-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Reduce pre flight mag inconsistency test false positives #12327
Conversation
Check horizontal and vertical angular discrepancy separately.
Under what circumstances would you advise relaxing the thresholds as an acceptable solution? I guess what I'm really wondering is if we want to expose these tuning knobs versus identifying a mag that might as well be disabled entirely or a switch to skip the check entirely (what most users are actually doing when "tuning" the parameter). |
* @decimal 2 | ||
* @increment 0.05 | ||
*/ | ||
PARAM_DEFINE_FLOAT(COM_ARM_MAG, 0.15f); | ||
PARAM_DEFINE_FLOAT(COM_ARM_MAG_V, 0.7f); |
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.
Why does there need to be a different angle for inclination and declination? As far as I understood the check is to find out if the mag is mounted the wrong way or the sensor orientation is set wrong. Not?
|
||
// calculate the horizontal angle between the two vectors to estimate the difference in magnetic heading thee sensors would produce | ||
matrix::Quatf q_delta(mag_hvec_selected, mag_hvec_alternate); | ||
matrix::Vector3f ang_hvec_delta = q_delta.to_axis_angle(); |
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.
to_axis_angle()
is "XXX DEPRECATED"
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.
I'm sorry but this check seems too complicated to me.
Please allow me to make a suggestion and let you have a look if it's viable.
@@ -1249,10 +1279,12 @@ void VotedSensorsUpdate::calc_mag_inconsistency(sensor_preflight_s &preflt) | |||
|
|||
// skip check if less than 2 sensors | |||
if (check_index < 1) { | |||
preflt.mag_inconsistency_ga = 0.0f; | |||
preflt.mag_inconsistency_h = 0.0f; | |||
preflt.mag_inconsistency_v = 0.0f; | |||
|
|||
} else { | |||
// get the vector length of the largest difference and write to the combined sensor struct |
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.
comment outdated
Here's my suggestion that I also successfully tested now: #12334 |
Work in progress for comment and review.
Raised in response to #12272.
@MaEtUgR are you able to test on your setup(s) and provide feedback? TXS