-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
Prearm check to validate backend read rates against scheduler loop rate #28672
base: master
Are you sure you want to change the base?
Changes from all commits
097095b
70db719
5939782
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -39,6 +39,7 @@ | |||||||
#include "AP_InertialSensor_Invensensev3.h" | ||||||||
#include "AP_InertialSensor_NONE.h" | ||||||||
#include "AP_InertialSensor_SCHA63T.h" | ||||||||
#include <AP_Scheduler/AP_Scheduler.h> | ||||||||
|
||||||||
/* Define INS_TIMING_DEBUG to track down scheduling issues with the main loop. | ||||||||
* Output is on the debug console. */ | ||||||||
|
@@ -1492,6 +1493,24 @@ bool AP_InertialSensor::gyro_calibrated_ok_all() const | |||||||
return (get_gyro_count() > 0); | ||||||||
} | ||||||||
|
||||||||
// Prearm check to verify that we have sane sched loop rates set based on Gyro backend rates | ||||||||
bool AP_InertialSensor::pre_arm_check_gyro_backend_rate_hz(char* fail_msg, uint16_t fail_msg_len) const | ||||||||
{ | ||||||||
#if AP_SCHEDULER_ENABLED | ||||||||
for (uint8_t i=0; i<get_gyro_count(); i++) { | ||||||||
if (!_use(i) || _backends[i] == nullptr) { | ||||||||
continue; | ||||||||
} | ||||||||
if (_backends[i]->get_gyro_backend_rate_hz() < (2*_loop_rate)) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
... used in two places |
||||||||
hal.util->snprintf(fail_msg, fail_msg_len, "Gyro %d backend rate %dHz < sched loop ratex2 %dHz", | ||||||||
i, _backends[i]->get_gyro_backend_rate_hz(), 2 * _loop_rate); | ||||||||
return false; | ||||||||
} | ||||||||
} | ||||||||
#endif | ||||||||
return true; | ||||||||
} | ||||||||
|
||||||||
// return true if gyro instance should be used (must be healthy and have it's use parameter set to 1) | ||||||||
bool AP_InertialSensor::use_gyro(uint8_t instance) const | ||||||||
{ | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,11 @@ class AP_InertialSensor_Invensensev3 : public AP_InertialSensor_Backend | |
bool accumulate_samples(const struct FIFOData *data, uint8_t n_samples); | ||
bool accumulate_highres_samples(const struct FIFODataHighRes *data, uint8_t n_samples); | ||
|
||
// get the gyro backend rate in Hz at which the FIFO is being read | ||
uint16_t get_gyro_backend_rate_hz() const override { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should take the opportunity to come up with a better name than "backend". Perhaps "get_gyro_read_rate_hz()" at least conveys what is going on and why it might be important. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That needs to be much larger and a separate change, I don't want to widen the scope of this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could just remove the word "backend" from the method name maybe? |
||
return backend_rate_hz; | ||
} | ||
|
||
// reset FIFO configure1 register | ||
uint8_t fifo_config1; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to 1.8 multiplier, so LSM 760Hz IMUs are OK