Commander: COM_MODE_ARM_CHK parameter to allow mode registration while armed#24249
Merged
Commander: COM_MODE_ARM_CHK parameter to allow mode registration while armed#24249
Conversation
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 200 byte (0.01 %)]px4_fmu-v6x [Total VM Diff: 80 byte (0 %)]Updated: 2025-01-28T13:17:50 |
bkueng
reviewed
Jan 24, 2025
Member
There was a problem hiding this comment.
Suggested change
| (ParamInt<px4::params::COM_MODE_ARM_CHK>) _param_com_mode_arm_chk | |
| (ParamBool<px4::params::COM_MODE_ARM_CHK>) _param_com_mode_arm_chk |
Member
There was a problem hiding this comment.
Suggested change
| + * By default disabled. 0: Mode registration is not allowed while armed. 1: Mode registration is allowed while armed. | |
| + * By default disabled for safety reasons |
Contributor
Author
|
@bkueng Thank you for the review! I'm open for parameter name suggestions and maybe we can consider to update this comment for consistency, " |
bdilman
commented
Jan 24, 2025
Contributor
Author
There was a problem hiding this comment.
moved the sitl exception here
Contributor
Author
There was a problem hiding this comment.
tested and verified the sitl exception, link to sitl log
Member
Right, this is exactly among the reasons why there could be problems (or at least unintended effects). I'd add something like Note that this might not be true if |
…med with COM_MODE_ARM_CHK
Co-authored-by: Beat Küng <beat-kueng@gmx.net>
Co-authored-by: Beat Küng <beat-kueng@gmx.net>
Co-authored-by: Beat Küng <beat-kueng@gmx.net>
Co-authored-by: Beat Küng <beat-kueng@gmx.net>
cda07d7 to
1d89b01
Compare
Contributor
Author
|
@bkueng latest changes are done and attached the sitl test results |
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
A new parameter
COM_MODE_ARM_CHKis defined to allow mode registrations while vehicle is armed. In the current implementation, by default, external mode registrations are only allowed while disarmed and therefore parameter default is set to reject mode registrations while armed.Fixes #{Github issue ID}
Solution
allow_update_while_armedvariable was previously hard-coded and disabled. To provide users the option to allow -external- mode registrations while vehicle is armed, this variable is parameterized asCOM_MODE_ARM_CHK, a boolean PX4 parameter under commander group.Changelog Entry
For release notes:
Alternatives
We could also ...
Test coverage
Context
Related links, screenshot before/after, video