Skip to content

fix(commander): use DroneCAN Remote ID status for arming checks - #28770

Open
amovlgf wants to merge 1 commit into
PX4:mainfrom
amovlgf:pr/dronecan-rid-arming-check
Open

amovlgf wants to merge 1 commit into
PX4:mainfrom
amovlgf:pr/dronecan-rid-arming-check

Conversation

@amovlgf

@amovlgf amovlgf commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

With COM_ARM_ODID=2, DroneCAN RID faults and status timeouts prevent normal arming without triggering an in-flight action; existing preflight bypasses remain available.

Problem

PX4 already receives dronecan.remoteid.ArmStatus, but Commander derives RID health only from MAVLink heartbeats, so the CAN module's readiness does not reach the existing arming check.

Solution

Consume the existing status, accept only GOOD_TO_ARM, expire it after three seconds, and refresh it before normal arming checks; after the first valid CAN message, use CAN until restart so MAVLink cannot mask a fault or disconnect (before that, MAVLink remains the fallback; only a single CAN RID is supported).
Compile this CAN integration only when the DroneCAN Remote ID controller is enabled, or for tests; other firmware retains periodic MAVLink RID checks without the new pre-arm refresh.
Parameter definitions/defaults, existing COM_ARM_ODID=3/4/5 flight policies, other arming checks, flight-control algorithms, board configuration, and the CAN driver/protocol are unchanged by this PR.

Validation: freshly built and passed 44 RID/health/failsafe tests, px4_sitl_default, and FMU-v2, Flycore, and FMU-v6x firmware with Arm GCC 13.2.1; changed-file formatting and whitespace checks pass. FMU-v2 uses 1,019,996 of 1,032,192 Flash bytes. Hardware bench testing has not been performed.

@github-actions github-actions Bot added kind:bug Something is broken or behaving incorrectly. kind:test Adds or improves tests. scope:commander Arming, modes, failsafe, health checks, or vehicle state. scope:testing Unit, integration, fuzzing, or test data. scope:docs labels Sep 20, 2026
@amovlgf
amovlgf marked this pull request as ready for review September 20, 2026 06:49
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 264 byte (0.01 %)]
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +264  +0.0%    +264    .text
    [NEW]    +312  [NEW]    +312    Commander::updateOpenDroneIDStatus()
    +175%     +56  +175%     +56    CSWTCH.245
    [NEW]     +56  [NEW]     +56    Commander::reportOpenDroneIDRegained()
    [NEW]     +32  [NEW]     +32    CSWTCH.241
    +1.7%     +24  +1.7%     +24    Commander::Commander()
    +2.3%     +12  +2.3%     +12    Commander::arm()
    +2.2%      +8  +2.2%      +8    Commander::~Commander()
    [NEW]      +4  [NEW]      +4    CSWTCH.800
    +2.0%      +4  +2.0%      +4    Commander::handleCommandActuatorTest()
    +7.7%      +4  +7.7%      +4    matrix::detail::wrap_floating<>()
    [DEL]      -4  [DEL]      -4    CSWTCH.799
    -9.1%      -4  -9.1%      -4    Commander::offboardControlCheck()
    -0.4%      -4  -0.4%      -4    Commander::run()
    -2.9%      -4  -2.9%      -4    Commander::updateControlMode()
    -2.0%      -4  -2.0%      -4    Commander::updateParameters()
   -102.3%      -4 -102.3%      -4    [1 Others]
    -0.0%      -8  -0.0%      -8    [section .text]
    -2.8%     -16  -2.8%     -16    Commander::control_status_leds()
    [DEL]     -32  [DEL]     -32    CSWTCH.242
    [DEL]     -56  [DEL]     -56    CSWTCH.246
    -9.0%    -112  -9.0%    -112    Commander::dataLinkCheck()
  +0.0%     +77  [ = ]       0    .debug_abbrev
  +0.0%     +16  [ = ]       0    .debug_aranges
  +0.0%     +68  [ = ]       0    .debug_frame
  +0.0%    +550  [ = ]       0    .debug_info
  +0.0%    +293  [ = ]       0    .debug_line
  +0.0%     +84  [ = ]       0    .debug_loclists
     +50%      +1  [ = ]       0    [Unmapped]
    +0.0%     +83  [ = ]       0    [section .debug_loclists]
  +0.0%     +10  [ = ]       0    .debug_rnglists
  +0.0%    +170  [ = ]       0    .debug_str
  -0.4%      -1  [ = ]       0    .shstrtab
  +0.0%     +73  [ = ]       0    .strtab
    [NEW]     +11  [ = ]       0    CSWTCH.241
    [DEL]     -11  [ = ]       0    CSWTCH.242
    [DEL]     -11  [ = ]       0    CSWTCH.246
    [DEL]     -11  [ = ]       0    CSWTCH.799
    [NEW]     +11  [ = ]       0    CSWTCH.800
    [NEW]     +43  [ = ]       0    Commander::reportOpenDroneIDRegained()
    [NEW]     +41  [ = ]       0    Commander::updateOpenDroneIDStatus()
   -32.7%     -16  [ = ]       0    ___Z12get_orb_meta6ORB_ID_veneer
     +67%     +16  [ = ]       0    __stm32_dmastart_veneer
  +0.0%     +96  [ = ]       0    .symtab
    [NEW]     +32  [ = ]       0    CSWTCH.241
    [DEL]     -32  [ = ]       0    CSWTCH.242
     +67%     +32  [ = ]       0    CSWTCH.245
    [DEL]     -32  [ = ]       0    CSWTCH.246
    [DEL]     -32  [ = ]       0    CSWTCH.799
    [NEW]     +32  [ = ]       0    CSWTCH.800
     +17%     +16  [ = ]       0    Commander::Commander()
   -50.0%     -16  [ = ]       0    Commander::checkForMissionUpdate()
   -25.0%     -16  [ = ]       0    Commander::control_status_leds()
   -25.0%     -16  [ = ]       0    Commander::handleCommandActuatorTest()
   -25.0%     -16  [ = ]       0    Commander::manualControlLossModeSwitch()
    +100%     +16  [ = ]       0    Commander::offboardControlCheck()
    [NEW]     +48  [ = ]       0    Commander::reportOpenDroneIDRegained()
    +100%     +16  [ = ]       0    Commander::updateControlMode()
    [NEW]     +48  [ = ]       0    Commander::updateOpenDroneIDStatus()
    +100%     +16  [ = ]       0    Commander::updateParameters()
   -40.0%     -32  [ = ]       0    ___Z12get_orb_meta6ORB_ID_veneer
     +33%     +16  [ = ]       0    ____aeabi_l2f_veneer
   -25.0%     -16  [ = ]       0    __sem_post_veneer
     +67%     +32  [ = ]       0    __stm32_dmastart_veneer
  -2.5%    -264  [ = ]       0    [Unmapped]
  +0.0% +1.40Ki  +0.0%    +264    TOTAL

px4_fmu-v6x [Total VM Diff: 272 byte (0.01 %)]
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +272  +0.0%    +272    .text
    [NEW]    +312  [NEW]    +312    Commander::updateOpenDroneIDStatus()
    +175%     +56  +175%     +56    CSWTCH.245
    [NEW]     +56  [NEW]     +56    Commander::reportOpenDroneIDRegained()
    [NEW]     +32  [NEW]     +32    CSWTCH.241
    +1.7%     +24  +1.7%     +24    Commander::Commander()
    +2.3%     +12  +2.3%     +12    Commander::arm()
    +2.2%      +8  +2.2%      +8    Commander::~Commander()
    [NEW]      +4  [NEW]      +4    CSWTCH.800
    +2.0%      +4  +2.0%      +4    Commander::handleCommandActuatorTest()
    +7.7%      +4  +7.7%      +4    matrix::detail::wrap_floating<>()
    [DEL]      -4  [DEL]      -4    CSWTCH.799
    -9.1%      -4  -9.1%      -4    Commander::offboardControlCheck()
    -0.4%      -4  -0.4%      -4    Commander::run()
    -2.9%      -4  -2.9%      -4    Commander::updateControlMode()
    -2.0%      -4  -2.0%      -4    Commander::updateParameters()
    -2.3%      -4  -2.3%      -4    send_vehicle_command()
    -2.8%     -16  -2.8%     -16    Commander::control_status_leds()
    [DEL]     -32  [DEL]     -32    CSWTCH.242
    [DEL]     -56  [DEL]     -56    CSWTCH.246
    -9.0%    -112  -9.0%    -112    Commander::dataLinkCheck()
  +0.0%     +77  [ = ]       0    .debug_abbrev
  +0.0%     +16  [ = ]       0    .debug_aranges
  +0.0%     +68  [ = ]       0    .debug_frame
  +0.0%    +550  [ = ]       0    .debug_info
  +0.0%    +293  [ = ]       0    .debug_line
  +0.0%     +60  [ = ]       0    .debug_loclists
    [NEW]      +1  [ = ]       0    [Unmapped]
    +0.0%     +59  [ = ]       0    [section .debug_loclists]
  +0.0%     +10  [ = ]       0    .debug_rnglists
  +0.0%    +170  [ = ]       0    .debug_str
  -0.4%      -1  [ = ]       0    .shstrtab
  +0.0%     +73  [ = ]       0    .strtab
    [NEW]     +11  [ = ]       0    CSWTCH.241
    [DEL]     -11  [ = ]       0    CSWTCH.242
    [DEL]     -11  [ = ]       0    CSWTCH.246
    [DEL]     -11  [ = ]       0    CSWTCH.799
    [NEW]     +11  [ = ]       0    CSWTCH.800
    [NEW]     +43  [ = ]       0    Commander::reportOpenDroneIDRegained()
    [NEW]     +41  [ = ]       0    Commander::updateOpenDroneIDStatus()
  +0.0%     +96  [ = ]       0    .symtab
    [NEW]     +32  [ = ]       0    CSWTCH.241
    [DEL]     -32  [ = ]       0    CSWTCH.242
     +67%     +32  [ = ]       0    CSWTCH.245
    [DEL]     -32  [ = ]       0    CSWTCH.246
    [DEL]     -32  [ = ]       0    CSWTCH.799
    [NEW]     +32  [ = ]       0    CSWTCH.800
     +17%     +16  [ = ]       0    Commander::Commander()
   -50.0%     -16  [ = ]       0    Commander::checkForMissionUpdate()
   -25.0%     -16  [ = ]       0    Commander::control_status_leds()
   -25.0%     -16  [ = ]       0    Commander::handleCommandActuatorTest()
   -25.0%     -16  [ = ]       0    Commander::manualControlLossModeSwitch()
    +100%     +16  [ = ]       0    Commander::offboardControlCheck()
    [NEW]     +48  [ = ]       0    Commander::reportOpenDroneIDRegained()
    +100%     +16  [ = ]       0    Commander::updateControlMode()
    [NEW]     +48  [ = ]       0    Commander::updateOpenDroneIDStatus()
    +100%     +16  [ = ]       0    Commander::updateParameters()
  -4.4%    -272  [ = ]       0    [Unmapped]
  +0.0% +1.38Ki  +0.0%    +272    TOTAL

Updated: 2026-09-22T04:07:48

@github-actions

Copy link
Copy Markdown
Contributor

No broken links found in changed files.

DroneCAN ArmStatus is already published internally but never reaches the
RID health check. Consume it in Commander and refresh readiness before
normal arming checks, rejecting faults and status older than three seconds.

Keep CAN as the source after its first valid status so MAVLink heartbeats
cannot hide a fault or disconnect. Preserve existing parameter policies
and preflight bypasses, and add focused regression tests and documentation.

Compile the CAN-specific integration only with the DroneCAN Remote ID
controller or tests. Propagate the internal flag to Commander consumers
so non-CAN targets avoid the subscription and pre-arm refresh while test
class layouts remain consistent.

Signed-off-by: lvguofei <1969354053@qq.com>
Assisted-by: Codex
@amovlgf
amovlgf force-pushed the pr/dronecan-rid-arming-check branch from 1475736 to 020aacd Compare September 22, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug Something is broken or behaving incorrectly. kind:test Adds or improves tests. scope:commander Arming, modes, failsafe, health checks, or vehicle state. scope:docs scope:testing Unit, integration, fuzzing, or test data.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant