commander: ensure health report is always sent out before failsafe notificaation #23973

bkueng · 2024-11-19T12:02:36Z

As the failsafe message can reference the health report, the health report needs to be sent out first. This is generally the case, except there is a rate limiter set to 2 seconds. So if the report changes quickly, it is sent out delayed (potentially after the failsafe report).

…tificaation As the failsafe message can reference the health report, the health report needs to be sent out first. This is generally the case, except there is a rate limiter set to 2 seconds. So if the report changes quickly, it is sent out delayed (potentially after the failsafe report).

MaEtUgR

Thanks for looking itno it! A classic C callback setup to allow triggering a healt_and_arming_checks action based on a consition in the completely separate failsafe state machine.

It works and crosses the nice architectural separation to have the nice behavior to only update in the necessary case.

src/modules/commander/HealthAndArmingChecks/Common.cpp

sfuhrer

One CI failure is real: https://github.com/PX4/PX4-Autopilot/actions/runs/11934542772/job/33263873602?pr=23973

github-actions · 2024-11-20T14:47:21Z

FLASH Analysis

px4_fmu-v5x

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0% +1.47Ki  [ = ]       0    .debug_info
    +0.2%    +453  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +0.3%     +99  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
    +0.2%     +74  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/accelerometerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/airspeedCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/armPermissionCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/baroCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/batteryCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/cpuResourceCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/distanceSensorChecks.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/escCheck.cpp
    +0.0%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/estimatorCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/externalChecks.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/failureDetectorCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/flightTimeCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/geofenceCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/gyroCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/homePositionCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/imuConsistencyCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/loggerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/magnetometerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/manualControlCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/missionCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/modeCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/navigatorCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/offboardCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/openDroneIDCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/parachuteCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/powerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/rcAndDataLinkCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/rcCalibrationCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/sdcardCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/systemCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/vtolCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/windCheck.cpp
    +0.0%     +26  [ = ]       0    ../../src/modules/commander/UserModeIntention.cpp
    +0.0%     +22  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%    +389  [ = ]       0    .debug_str
    +0.3%    +345  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +0.8%     +41  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
    +0.0%     +11  [ = ]       0    [section .debug_str]
    -0.3%      -8  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
  +0.0%    +178  [ = ]       0    .debug_line
    +0.5%    +166  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +1.2%     +44  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.1%      +7  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
    -0.3%     -39  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%    +152  [ = ]       0    .strtab
    +7.5%     +60  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
    +1.3%     +49  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +5.2%     +43  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
  +0.0%    +120  +0.0%    +120    .text
    +0.4%     +92  +0.4%     +92    ../../src/modules/commander/Commander.cpp
    +0.7%     +20  +0.7%     +20    ../../src/modules/commander/failsafe/framework.cpp
    +1.1%     +16  +1.1%     +16    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.0%     +14  +0.0%     +14    [section .text]
    -0.6%     -22  -0.6%     -22    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%     +96  [ = ]       0    .debug_ranges
    +1.8%    +136  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +0.5%      +8  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    -0.9%     -48  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%     +96  [ = ]       0    .symtab
    +0.7%     +32  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +5.7%     +32  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +4.0%     +32  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%     +82  [ = ]       0    .debug_abbrev
    +0.0%     +82  [ = ]       0    armv7-m/arm_vectors.c
  +0.0%     +64  [ = ]       0    .debug_frame
  +0.0%     +32  [ = ]       0    .debug_aranges
  -0.5%    -117  [ = ]       0    [Unmapped]
  -0.0%    -690  [ = ]       0    .debug_loc
    +0.4%     +25  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.0%      +8  [ = ]       0    [section .debug_loc]
    -0.1%     -22  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
    -0.2%     -62  [ = ]       0    ../../src/modules/commander/Commander.cpp
    -9.6%    -639  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
  +0.0% +1.86Ki  +0.0%    +120    TOTAL

px4_fmu-v6x

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0% +1.47Ki  [ = ]       0    .debug_info
    +0.2%    +453  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +0.3%     +99  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
    +0.2%     +74  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/accelerometerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/airspeedCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/armPermissionCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/baroCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/batteryCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/cpuResourceCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/distanceSensorChecks.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/escCheck.cpp
    +0.0%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/estimatorCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/externalChecks.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/failureDetectorCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/flightTimeCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/geofenceCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/gyroCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/homePositionCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/imuConsistencyCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/loggerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/magnetometerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/manualControlCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/missionCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/modeCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/navigatorCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/offboardCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/openDroneIDCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/parachuteCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/powerCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/rcAndDataLinkCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/rcCalibrationCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/sdcardCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/systemCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/vtolCheck.cpp
    +0.1%     +26  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/windCheck.cpp
    +0.0%     +26  [ = ]       0    ../../src/modules/commander/UserModeIntention.cpp
    +0.0%     +22  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%    +389  [ = ]       0    .debug_str
    +0.3%    +345  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +0.8%     +41  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
    +0.0%     +11  [ = ]       0    [section .debug_str]
    -0.3%      -8  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
  +0.0%    +178  [ = ]       0    .debug_line
    +0.5%    +166  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +1.2%     +44  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.1%      +7  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
    -0.3%     -39  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%    +152  [ = ]       0    .strtab
    +7.5%     +60  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
    +1.3%     +49  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +5.2%     +43  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
  +0.0%    +120  +0.0%    +120    .text
    +0.4%     +92  +0.4%     +92    ../../src/modules/commander/Commander.cpp
    +0.7%     +20  +0.7%     +20    ../../src/modules/commander/failsafe/framework.cpp
    +1.1%     +16  +1.1%     +16    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +0.0%     +14  +0.0%     +14    [section .text]
    -0.6%     -22  -0.6%     -22    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%     +96  [ = ]       0    .debug_ranges
    +1.8%    +136  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +0.5%      +8  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    -0.9%     -48  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%     +96  [ = ]       0    .symtab
    +0.7%     +32  [ = ]       0    ../../src/modules/commander/Commander.cpp
    +5.7%     +32  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    +4.0%     +32  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
  +0.0%     +82  [ = ]       0    .debug_abbrev
    +0.0%     +82  [ = ]       0    armv7-m/arm_vectors.c
  +0.0%     +64  [ = ]       0    .debug_frame
  +0.0%     +32  [ = ]       0    .debug_aranges
  -0.2%    -120  [ = ]       0    [Unmapped]
  -0.0%    -735  [ = ]       0    .debug_loc
    +0.4%     +25  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/Common.cpp
    -0.0%     -11  [ = ]       0    [section .debug_loc]
    -0.1%     -13  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/checks/estimatorCheck.cpp
    -0.1%     -22  [ = ]       0    ../../src/modules/commander/HealthAndArmingChecks/HealthAndArmingChecks.cpp
    -0.3%     -75  [ = ]       0    ../../src/modules/commander/Commander.cpp
    -9.6%    -639  [ = ]       0    ../../src/modules/commander/failsafe/framework.cpp
  +0.0% +1.82Ki  +0.0%    +120    TOTAL

bkueng requested review from MaEtUgR and sfuhrer November 19, 2024 12:02

MaEtUgR previously approved these changes Nov 19, 2024

View reviewed changes

src/modules/commander/HealthAndArmingChecks/Common.cpp Outdated Show resolved Hide resolved

bkueng dismissed MaEtUgR’s stale review via d7189a3 November 20, 2024 13:37

sfuhrer previously approved these changes Nov 20, 2024

View reviewed changes

sfuhrer reviewed Nov 20, 2024

View reviewed changes

refactor commander: remove unused argument is_armed in Report class

921cd8a

bkueng dismissed sfuhrer’s stale review via 921cd8a November 20, 2024 14:36

bkueng force-pushed the failsafe_force_reporting branch from d7189a3 to 921cd8a Compare November 20, 2024 14:36

PX4 deleted a comment from github-actions bot Nov 21, 2024

MaEtUgR merged commit 8ccd43d into main Nov 21, 2024
60 of 62 checks passed

MaEtUgR deleted the failsafe_force_reporting branch November 21, 2024 13:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

commander: ensure health report is always sent out before failsafe notificaation #23973

commander: ensure health report is always sent out before failsafe notificaation #23973

bkueng commented Nov 19, 2024

MaEtUgR left a comment •

edited

Loading

sfuhrer left a comment

github-actions bot commented Nov 20, 2024

commander: ensure health report is always sent out before failsafe notificaation #23973

commander: ensure health report is always sent out before failsafe notificaation #23973

Conversation

bkueng commented Nov 19, 2024

MaEtUgR left a comment • edited Loading

Choose a reason for hiding this comment

sfuhrer left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 20, 2024

FLASH Analysis

MaEtUgR left a comment •

edited

Loading