Commander: COM_MODE_ARM_CHK parameter to allow mode registration while armed #24249
+20
−6
Conversation
This file contains 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
…med with COM_MODE_ARM_CHK
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 192 byte (0.01 %)]px4_fmu-v6x [Total VM Diff: 80 byte (0 %)]Updated: 2025-01-24T13:21:14 |
bdilman
changed the title
bkueng
reviewed
Jan 24, 2025
| @@ -370,7 +370,7 @@ void ModeManagement::update(bool armed, uint8_t user_intended_nav_state, bool fa | |||
| _failsafe_action_active = failsafe_action_active; | |||
| _external_checks.update(); | |||
|
|
|||
| bool allow_update_while_armed = false; | |||
| bool allow_update_while_armed = _external_checks.allowUpdateWhileArmed(); | |||
| #if defined(CONFIG_ARCH_BOARD_PX4_SITL) | |||
There was a problem hiding this comment.
Can you remove this ifdef and set the param for sitl?
There was a problem hiding this comment.
@bkueng removed the ifdef and thanks for the pointer with the param for sitl, I wanted to keep the sitl exception.
Is this file adequate to set the sitl param?
| @@ -109,4 +110,7 @@ class ExternalChecks : public HealthAndArmingCheckBase | |||
| uORB::Subscription _arming_check_reply_sub{ORB_ID(arming_check_reply)}; | |||
|
|
|||
| uORB::Publication<arming_check_request_s> _arming_check_request_pub{ORB_ID(arming_check_request)}; | |||
| DEFINE_PARAMETERS( | |||
| (ParamInt<px4::params::COM_MODE_ARM_CHK>) _param_com_mode_arm_chk | |||
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 |
| /** | ||
| + * Allow external mode registration while armed. | ||
| + * | ||
| + * By default disabled. 0: Mode registration is not allowed while armed. 1: Mode registration is allowed while armed. |
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 |
src/modules/commander/HealthAndArmingChecks/checks/externalChecks.hpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Beat Küng <[email protected]>
Co-authored-by: Beat Küng <[email protected]>
Co-authored-by: Beat Küng <[email protected]>
Co-authored-by: Beat Küng <[email protected]>
|
@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
| @@ -4,6 +4,9 @@ | |||
| # Simulator IMU data provided at 250 Hz | |||
| param set-default IMU_INTEG_RATE 250 | |||
|
|
|||
| # For simulation, allow registering modes while armed for developer convenience | |||
| param set-default COM_MODE_ARM_CHK 1 | |||
There was a problem hiding this comment.
moved the sitl exception here
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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