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.

Sign up for GitHub

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

circle buffer rough fix #785

Closed
wants to merge 4 commits into from
Closed

circle buffer rough fix #785

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba748cc
circle buffer rough fix
caila-marashaj Jun 18, 2024
9e964a4
dont need both a bool and count
caila-marashaj Jun 18, 2024
fd4f309
Update include/sensors/core/tasks/pressure_driver.hpp
caila-marashaj Jun 24, 2024
54d4f0d
Update include/sensors/core/tasks/pressure_driver.hpp
caila-marashaj Jun 24, 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
61 changes: 44 additions & 17 deletions include/sensors/core/tasks/pressure_driver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class MMR920 {
echoing = should_echo;
if (should_echo) {
sensor_buffer_index = 0; // reset buffer index
pressure_transations = 0;
}
}

Expand Down Expand Up @@ -290,25 +291,48 @@ class MMR920 {
}

void send_accumulated_sensor_data(uint32_t message_index) {
for (int i = 0; i < static_cast<int>(SENSOR_BUFFER_SIZE); i++) {
// send over buffer and then clear buffer values
// NOLINTNEXTLINE(div-by-zero)
int current_index = (i + sensor_buffer_index) %
static_cast<int>(SENSOR_BUFFER_SIZE);
if (pressure_transations >= 500) {
Copy link
Contributor

Choose a reason for hiding this comment

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

500 should be SENSOR_BUFFER_SIZE

for (int i = 0; i < static_cast<int>(SENSOR_BUFFER_SIZE); i++) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: things used as indices should be unsigned

// send over buffer and then clear buffer values
// NOLINTNEXTLINE(div-by-zero)
Copy link
Member

Choose a reason for hiding this comment

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

is this warning because SENSOR_BUFFER_SIZE isn't defined in lint or something?

int current_index = (i + sensor_buffer_index) %
static_cast<int>(SENSOR_BUFFER_SIZE);

can_client.send_can_message(
can::ids::NodeId::host,
can::messages::ReadFromSensorResponse{
.message_index = message_index,
.sensor = can::ids::SensorType::pressure,
.sensor_id = sensor_id,
.sensor_data = mmr920::reading_to_fixed_point(
(*sensor_buffer).at(current_index))});
if (i % 10 == 0) {
// slow it down so the can buffer doesn't choke
vtask_hardware_delay(50);
can_client.send_can_message(
can::ids::NodeId::host,
can::messages::ReadFromSensorResponse{
.message_index = message_index,
.sensor = can::ids::SensorType::pressure,
.sensor_id = sensor_id,
.sensor_data = mmr920::reading_to_fixed_point(
(*sensor_buffer).at(current_index))});
if (i % 10 == 0) {
// slow it down so the can buffer doesn't choke
vtask_hardware_delay(50);
}
(*sensor_buffer).at(current_index) = 0;
}
}
// I think counting the transactions here might
// be preferable to having to ignore a bunch
// of 0's at the end in Python, because we might end up accidentally
// ignoring valid data that way
else {
for (int = 0; i < pressure_transations; i++) {
caila-marashaj marked this conversation as resolved.
Show resolved Hide resolved
can_client.send_can_message(
can::ids::NodeId::host,
can::messages::ReadFromSensorResponse{
.message_index = message_index,
.sensor = can::ids::SensorType::pressure,
.sensor_id = sensor_id,
.sensor_data = mmr920::reading_to_fixed_point(
(*sensor_buffer).at(i))});
if (i % 10 == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

let's add a todo here or something - send_can_message should return a success value so we can reactively do this when needed

// slow it down so the can buffer doesn't choke
vtask_hardware_delay(50);
}
}
(*sensor_buffer).at(current_index) = 0;
(*sensor_buffer).at(i) = 0;
}
}

Expand Down Expand Up @@ -371,6 +395,8 @@ class MMR920 {
if (echo_this_time) {
auto response_pressure = pressure - current_pressure_baseline_pa;
// do we want pressure or response pressure
if (pressure_transations < 500) { pressure_transactions++; }
Copy link
Contributor

Choose a reason for hiding this comment

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

500 should be SENSOR_BUFFER_SIZE instead


sensor_buffer_log(pressure);
can_client.send_can_message(
can::ids::NodeId::host,
Expand Down Expand Up @@ -532,6 +558,7 @@ class MMR920 {
float pressure_running_total = 0;
float temperature_running_total = 0;
uint16_t total_baseline_reads = 1;
uint16_t pressure_transations = 0;
caila-marashaj marked this conversation as resolved.
Show resolved Hide resolved

float current_pressure_baseline_pa = 0;
float current_temperature_baseline = 0;
Expand Down
Loading