circle buffer rough fix #785
Conversation
| @@ -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++; } | |||
There was a problem hiding this comment.
500 should be SENSOR_BUFFER_SIZE instead
| // NOLINTNEXTLINE(div-by-zero) | ||
| int current_index = (i + sensor_buffer_index) % | ||
| static_cast<int>(SENSOR_BUFFER_SIZE); | ||
| if (pressure_transations >= 500) { |
There was a problem hiding this comment.
500 should be SENSOR_BUFFER_SIZE
There was a problem hiding this comment.
This logic is kind of weird and fiddly. We should make it something that can be more easily tested. I'd suggest putting the indexing logic in its own file and class (like DoubleBuffer) with some methods to do various things and like a function called get_read_index() for getting data out (really, I'd suggest using a custom iterator, but that's kind of a pain). Then, we need to test the hell out of it.
| .sensor_id = sensor_id, | ||
| .sensor_data = mmr920::reading_to_fixed_point( | ||
| (*sensor_buffer).at(i))}); | ||
| if (i % 10 == 0) { |
There was a problem hiding this comment.
let's add a todo here or something - send_can_message should return a success value so we can reactively do this when needed
| if (pressure_transations >= 500) { | ||
| for (int i = 0; i < static_cast<int>(SENSOR_BUFFER_SIZE); i++) { | ||
| // send over buffer and then clear buffer values | ||
| // NOLINTNEXTLINE(div-by-zero) |
There was a problem hiding this comment.
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); | ||
| if (pressure_transations >= 500) { | ||
| for (int i = 0; i < static_cast<int>(SENSOR_BUFFER_SIZE); i++) { |
There was a problem hiding this comment.
nit: things used as indices should be unsigned
Co-authored-by: Seth Foster <[email protected]>
Co-authored-by: Seth Foster <[email protected]>
No description provided.