-
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
Pre-flight check: add parameter to en/disable mag field strength check #13766
Conversation
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.
just nit-picking for readability
src/modules/commander/Arming/PreFlightCheck/checks/ekf2Check.cpp
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
success = false; | ||
goto out; | ||
} | ||
|
||
param_get(param_find("COM_ARM_MAG_STR"), &is_mag_strength_check); |
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 would fill the local variable when it's declared. Ideally here but if you have to declare it earlier because of the **** gotos I would move the param_get()
there as well.
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 hope we can get rid of those ugly goto at some point...
src/modules/commander/Arming/PreFlightCheck/checks/ekf2Check.cpp
Outdated
Show resolved
Hide resolved
9f5927e
to
8c1ad76
Compare
@MaEtUgR I addressed your comments, can you review again please? |
This new parameter:
COM_ARM_MAG_STR
defines if thepre_flt_fail_mag_field_disturbed
flag sent by the estimator should prevent arming or not.