-
Notifications
You must be signed in to change notification settings - Fork 18.5k
All vehicles: Fix non fatal rc failsafe bug #29846
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
base: master
Are you sure you want to change the base?
Conversation
ArduPlane/radio.cpp
Outdated
@@ -402,7 +402,7 @@ bool Plane::rc_throttle_value_ok(void) const | |||
*/ | |||
bool Plane::rc_failsafe_active(void) const | |||
{ | |||
if (!rc_throttle_value_ok()) { | |||
if (!rc_throttle_value_ok() || (ThrFailsafe(g.throttle_fs_enabled.get()) == ThrFailsafe::Disabled)) { |
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.
this is a bit unintuitive for someone reading the code because I think it means that the RC failsafe is active when it's actually been disabled.
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 think this is already covered in Plane::rc_throttle_value_ok() implementation?
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.
it is not....rc_throttle_value_ok() early returns, yes, but ALWAYS when throttle failsafe is disabled, even when failsafe flag is still set ....rc_failsafe_active will return true and never return false after RC sis restored
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.
@rmackay9 I think I understand...I will early return on the disable instead of letting of letting it test rc_throttle value first and oring...and removing from rc_throttle_value_ok
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.
the change in plane radio.cpp doesn't look right to me
8a4a28a
to
dbc5250
Compare
dbc5250
to
299d031
Compare
I discovered this while working on adding RC to Sub...easy to reproduce
Sim vehicle with defaults...happens disarmed or in the air
wont clear until reboot or resetting failsafe enable
non fatal but really annoying if you run into it...
hope its doesn't make CI uhappy