UAVCAN ESCs pre-arm checks #12153
Closed
UAVCAN ESCs pre-arm checks #12153
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
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
…shed. Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
cmic0
force-pushed
the
pr-escstatus_checks
branch
from
9aafbdf to
0c0acc4
Compare
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
|
Uh apparently the CI is not happy 🤕 |
RomanBapst
reviewed
Jul 29, 2019
| @@ -1010,6 +1010,13 @@ bool prearm_check(orb_advert_t *mavlink_log_pub, const vehicle_status_flags_s &s | |||
|
|
|||
| } | |||
|
|
|||
| if (!status_flags.condition_escs_valid) { | |||
There was a problem hiding this comment.
@cmic0 This will fail for any vehicle which does not support the esc_status message.
Also check CI, SITL is actually complaining about ESC not being online :-D
RomanBapst
reviewed
Jul 29, 2019
src/modules/commander/Commander.cpp
Outdated
| // Check if ALL the ESCs are online | ||
| if (online_bitmask == esc_status.esc_online_flags) { | ||
| if (_last_esc_online_flags != esc_status.esc_online_flags) { | ||
| mavlink_log_info(&mavlink_log_pub, "All ESCs are ready"); |
There was a problem hiding this comment.
@cmic0 I think we can get rid of this message. No need to tell the user what he does not need to know.
RomanBapst
reviewed
Jul 29, 2019
src/lib/mixer/mixer_group.cpp
Outdated
| rotor_count = mixer->get_multirotor_count(); | ||
|
|
||
| if (rotor_count > 0) { | ||
| mixer = nullptr; |
There was a problem hiding this comment.
@cmic0 I think you can use "break"
… report status). - Added circuit breaker for disabling/enabling the logic. Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
Signed-off-by: Claudio Micheli <[email protected]>
…pr-escstatus_checks
|
Closing this one in favor as the cleaned-up PR #12641. |
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.
This PR introduces a new pre-arm check aimed for vehicles running UAVCAN ESCs.
Each actuator is monitored through a status flag introduced in the
esc_statusmessage:If an ESC does not provide any data within a timeout threshold then is considered offline.
During pre-arm checks, if one or more motors are offline a warning message is published and arming is denied.
The feature is enabled by setting the
COM_ACT_COUNTto the number of UAVCAN actuators.Test data / coverage
Tested on a vehicle running Zubax Myxa ESCs. Reporting below the warning message that is triggered. In this case ESC3 and ESC2 were disconnected.
Describe possible alternatives
Actually the solution could be improved by taking the
number of motorsinformation from the mixer and use a circuit breaker for enabling/disabling the pre-arm check. Adding the parameter was the MVP solution for getting the pre-arm check to work.Update 07/26:
Did some logic refactoring and got rid of
COM_ACT_COUNTparameter.Now the number of rotors is obtained through the mixer at boot.
Screenshot of bench testing:
Logs:
https://logs.px4.io/plot_app?log=6bfacd4a-9b37-413c-a333-8618dff6f3fc
The PR is now in a good shape. Only thing I am thinking that maybe is worth to add is a circuit breaker for disabling the feature?
@dagar @MaEtUgR FYI