Bugfix: Let user take over from a degraded failsafe#26237
Merged
Conversation
8e28636 to
c081d2c
Compare
bkueng
previously approved these changes
Jan 9, 2026
Member
bkueng
left a comment
There was a problem hiding this comment.
Nice work, this is an important fix
| ASSERT_EQ(updated_user_intented_mode, state.user_intended_mode); | ||
| ASSERT_EQ(failsafe.selectedAction(), FailsafeBase::Action::Land); | ||
|
|
||
| // User wants takeover -> Land |
Member
There was a problem hiding this comment.
Suggested change
| // User wants takeover -> Land | |
| // User wants takeover -> Altitude Control |
Member
Author
There was a problem hiding this comment.
Thanks for reviewing and spotting, copy pasta comment and forgot adapting the second part 🤦♂️
Fixed in place: https://github.com/PX4/PX4-Autopilot/compare/c081d2cb807f6f5fa783fcc5ff9ce25f6d5c09c6..1a85c71245ee27b972e7971d2a3f727b1c70b88d
…ailsafes 2 not cause immediate takeover when failsafe happens because of mode switch The first test makes sure the user can take over when an RTL failsafe was triggered but degraded to a Land. The second test rules out the easiest fix of removing the condition `_selected_action == selected_action` which causes the problem for test one but is there for a reason.
…ction when configured RTL is not possible
c081d2c to
1a85c71
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Solved Problem
User can not take over from a degraded failsafe e.g.
NAV_DLL_ACT = 2)Solution
_selected_action == selected_actionwas originally added: When a failsafe happens immediately on mode switch because a mode requirement is not met. Then the mode switch that caused it should not be interpreted as the user taking over immediately. Note that the context should avoid switching to modes that cannot run but the framework should still handle the case correctly._selected_action == selected_actionto(_selected_action > Action::Warn)._selected_action == selected_actionbecause in the problem caseselected_actionis what we want to do: Return and_selected_actionis what we end up doing after the fallbacks in the same function below: Land and the condition is never fulfilled while degraded.(_selected_action > Action::Warn)to avoid immediately flaging a user takeover if there was no failsafe before and it's triggered at the same time as/caused by a mode switch.Big thanks to @bkueng who got me to understand the immediate takeover case and also was the one suggesting the fix 🙏
Changelog Entry
Test coverage
The failsafe simulation shows the problem and it being resolved and we have automated unit tests for the cases we could think of such that we hopefully can't break it again.