Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sensors): Self baselining during sensor moves #786

Merged
merged 26 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
70e8734
Use logic or instead of hardcoded enums
ryanthecoder Jun 17, 2024
d6c1372
when filling the bufffer auto baseline after the first 10
ryanthecoder Jun 17, 2024
c00db25
the new method that lets us increase senstivity uncovered an issue wi…
ryanthecoder Jun 17, 2024
4e17786
few tweaks to the way this works
ryanthecoder Jun 17, 2024
d44cfa7
only compute the first 10 elements to self-baselien
ryanthecoder Jun 18, 2024
ac0c76e
don't immediatly turn of the recording when move completes
ryanthecoder Jun 18, 2024
095fc71
save response pressure
ryanthecoder Jun 18, 2024
fb63bf4
fix the pressure leveling
ryanthecoder Jun 18, 2024
7b0e300
rebase fixups
ryanthecoder Jun 18, 2024
920b861
fix the hardware delay hack and get rid of another instance of it
ryanthecoder Jun 20, 2024
544d9c0
send ack after sending buffer
ryanthecoder Jun 21, 2024
6c30b6e
fixes to the circular buffer
ryanthecoder Jun 21, 2024
372383b
don't need this and can cause the process to choke
ryanthecoder Jun 21, 2024
4b7b24d
send the can messages faster
ryanthecoder Jun 21, 2024
994d6ef
format
ryanthecoder Jun 21, 2024
92bc679
make the moving baseline log clearer
ryanthecoder Jun 24, 2024
0c0e09b
use the real world sensor speed
ryanthecoder Jun 25, 2024
5ba41c1
change auto baseline slightly to ignore the first N samples
ryanthecoder Jun 25, 2024
bbb5df4
add a new sensor sync value
ryanthecoder Jun 25, 2024
3109317
don't trigger before autobaseline is finished
ryanthecoder Jun 25, 2024
7a26107
add new binding type for sensors and use the auto-baselineing during …
ryanthecoder Jun 25, 2024
787b983
forgot to change an old reference
ryanthecoder Jun 25, 2024
fdd6637
format lint
ryanthecoder Jun 25, 2024
5a60de6
preserve old behavior
ryanthecoder Jun 25, 2024
ecd943b
reduce the complexity in the handle function to satisfy lint
ryanthecoder Jun 25, 2024
a9cce7b
format
ryanthecoder Jun 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions include/common/core/hardware_delay.hpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
#pragma once
// Not my favorite way to check this, but if we don't have access
// to vTaskDelay during host compilation so just dummy the function

#ifdef ENABLE_CROSS_ONLY_HEADERS
#include "FreeRTOS.h"
#endif

template <typename T>
requires std::is_integral_v<T>
static void vtask_hardware_delay(T ticks) {
#ifndef INC_TASK_H
std::ignore = ticks;
#else
#ifdef ENABLE_CROSS_ONLY_HEADERS
vTaskDelay(ticks);
#else
std::ignore = ticks;
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,11 @@ class MotorInterruptHandler {
hardware.get_encoder_pulses();
#ifdef USE_SENSOR_MOVE
if (buffered_move.sensor_id != can::ids::SensorId::UNUSED) {
auto binding = static_cast<uint8_t>(0x3); // sync and report
auto binding =
static_cast<uint8_t>(can::ids::SensorOutputBinding::sync) |
static_cast<uint8_t>(
can::ids::SensorOutputBinding::report); // sync and
// report
if (buffered_move.sensor_id == can::ids::SensorId::BOTH) {
send_bind_message(buffered_move.sensor_type,
can::ids::SensorId::S0, binding);
Expand Down Expand Up @@ -502,21 +506,6 @@ class MotorInterruptHandler {
tick_count = 0x0;
stall_handled = false;
build_and_send_ack(ack_msg_id);
#ifdef USE_SENSOR_MOVE
if (buffered_move.sensor_id != can::ids::SensorId::UNUSED) {
auto binding = static_cast<uint8_t>(
can::ids::SensorOutputBinding::sync); // make none?!
if (buffered_move.sensor_id == can::ids::SensorId::BOTH) {
send_bind_message(buffered_move.sensor_type,
can::ids::SensorId::S0, binding);
send_bind_message(buffered_move.sensor_type,
can::ids::SensorId::S1, binding);
} else {
send_bind_message(buffered_move.sensor_type,
buffered_move.sensor_id, binding);
}
}
#endif
set_buffered_move(MotorMoveMessage{});
// update the stall check ideal encoder counts based on
// last known location
Expand Down
24 changes: 7 additions & 17 deletions include/sensors/core/tasks/capacitive_driver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,11 @@

#include <array>

#ifdef ENABLE_CROSS_ONLY_HEADERS
// TODO(fps 7/12/2023): This is super hacky and I hate throwing #ifdefs
// in our nicely host-independent code but for now we really just need
// the vTaskDelay function and hopefully sometime in the near future I
// can refactor this file with a nice templated sleep function.
#include "FreeRTOS.h"
// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
#define HACKY_TASK_SLEEP(___timeout___) vTaskDelay(___timeout___)

#else

// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
#define HACKY_TASK_SLEEP(___timeout___) (void)(___timeout___)
#endif

#include "can/core/can_writer_task.hpp"
#include "can/core/ids.hpp"
#include "can/core/messages.hpp"
#include "common/core/bit_utils.hpp"
#include "common/core/hardware_delay.hpp"
#include "common/core/logging.h"
#include "common/core/message_queue.hpp"
#include "common/core/sensor_buffer.hpp"
Expand Down Expand Up @@ -63,11 +49,11 @@ class FDC1004 {
// holding off for this PR.

// Initial delay to avoid I2C bus traffic.
HACKY_TASK_SLEEP(100);
vtask_hardware_delay(100);
update_capacitance_configuration();
// Second delay to ensure IC is ready to start
// readings (and also to avoid I2C bus traffic).
HACKY_TASK_SLEEP(100);
vtask_hardware_delay(100);
set_sample_rate();
_initialized = true;
}
Expand Down Expand Up @@ -231,6 +217,10 @@ class FDC1004 {
}
(*sensor_buffer).at(i) = 0;
}
can_client.send_can_message(
can::ids::NodeId::host,
can::messages::Acknowledgment{
.message_index = message_index});
#else
std::ignore = message_index;
#endif
Expand Down
65 changes: 47 additions & 18 deletions include/sensors/core/tasks/pressure_driver.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include <cmath>
#include <numeric>

#include "can/core/can_writer_task.hpp"
#include "can/core/ids.hpp"
Expand Down Expand Up @@ -70,6 +71,8 @@ class MMR920 {
echoing = should_echo;
if (should_echo) {
sensor_buffer_index = 0; // reset buffer index
crossed_buffer_index=false;
sensor_buffer->fill(0.0);
}
}

Expand Down Expand Up @@ -230,12 +233,12 @@ class MMR920 {
}

auto sensor_buffer_log(float data) -> void {
sensor_buffer->at(sensor_buffer_index) = data;
sensor_buffer_index++;
if (sensor_buffer_index == SENSOR_BUFFER_SIZE) {
sensor_buffer_index = 0;
crossed_buffer_index=true;
}

sensor_buffer->at(sensor_buffer_index) = data;
sensor_buffer_index++;
}

auto save_temperature(int32_t data) -> bool {
Expand Down Expand Up @@ -290,10 +293,21 @@ class MMR920 {
}

void send_accumulated_sensor_data(uint32_t message_index) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this overlap with #785 ?

for (int i = 0; i < static_cast<int>(SENSOR_BUFFER_SIZE); i++) {
auto start = 0;
auto count = sensor_buffer_index;
if (crossed_buffer_index == true) {
start = sensor_buffer_index;
count = SENSOR_BUFFER_SIZE;
}

can_client.send_can_message(
can::ids::NodeId::host,
can::messages::Acknowledgment{
.message_index = count});
for (int i = 0; i < count; i++) {
// send over buffer and then clear buffer values
// NOLINTNEXTLINE(div-by-zero)
int current_index = (i + sensor_buffer_index) %
int current_index = (i + start) %
static_cast<int>(SENSOR_BUFFER_SIZE);

can_client.send_can_message(
Expand All @@ -306,10 +320,13 @@ class MMR920 {
(*sensor_buffer).at(current_index))});
if (i % 10 == 0) {
// slow it down so the can buffer doesn't choke
vtask_hardware_delay(50);
vtask_hardware_delay(20);
}
(*sensor_buffer).at(current_index) = 0;
}
can_client.send_can_message(
can::ids::NodeId::host,
can::messages::Acknowledgment{
.message_index = message_index});
}

auto handle_ongoing_pressure_response(i2c::messages::TransactionResponse &m)
Expand Down Expand Up @@ -355,31 +372,42 @@ class MMR920 {
.message_index = m.message_index,
.severity = can::ids::ErrorSeverity::unrecoverable,
.error_code = can::ids::ErrorCode::over_pressure});
} else {
} else if (!bind_sync) {
// if we're not using bind sync turn off the sync line
// we don't do this during bind sync because if it's triggering
// the sync line on purpose this causes bouncing on the line
// that turns off the sync and then immediately turns it back on
// and this can cause disrupt the behavior
hardware.reset_sync();
}
}
if (bind_sync) {
if (std::fabs(pressure - current_pressure_baseline_pa) >
threshold_pascals) {
hardware.set_sync();
// force this to stay set long enough to debounce on other nodes
} else {
hardware.reset_sync();
}
}

if (echo_this_time) {
auto response_pressure = pressure - current_pressure_baseline_pa;
// do we want pressure or response pressure
sensor_buffer_log(pressure);
can_client.send_can_message(
can::ids::NodeId::host,
can::messages::ReadFromSensorResponse{
.message_index = m.message_index,
.sensor = can::ids::SensorType::pressure,
.sensor_id = sensor_id,
.sensor_data =
mmr920::reading_to_fixed_point(response_pressure)});

sensor_buffer_log(response_pressure);

if (sensor_buffer_index == 10 && !crossed_buffer_index) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this 10 is setting the baselining window size, right? let's drop that in a define or a constexpr or something


current_pressure_baseline_pa =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we incrementing instead of setting this? won't this make it run away? also we could put 10*current_pressure_baseline_pa in the init argument to accumulate if we do want the accumulation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment that explains why this won't run away

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-worked it to make it more clear

std::accumulate(sensor_buffer->begin(), sensor_buffer->begin()+10, 0) /
10 + current_pressure_baseline_pa;
for (auto i = sensor_buffer_index - 10; i < sensor_buffer_index;
i++) {
sensor_buffer->at(sensor_buffer_index) =
sensor_buffer->at(sensor_buffer_index) -
current_pressure_baseline_pa;
}
}
}
}

Expand Down Expand Up @@ -565,6 +593,7 @@ class MMR920 {
}
std::array<float, SENSOR_BUFFER_SIZE> *sensor_buffer;
uint16_t sensor_buffer_index = 0;
bool crossed_buffer_index = false;
};

} // namespace tasks
Expand Down
Loading