Skip to content

safety: allow for level logic in safety switch #25156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

safety: allow for level logic in safety switch #25156

wants to merge 1 commit into from

Conversation

alexcekay
Copy link
Member

Solved Problem

The safety button system currently only allows for a button that disables the safety when pressed for one second. For some usecases it can be practical to have a level based switch instead of this pulse style switch.

Solution

  • Keep the whole button logic, so this stays unchanged
  • Add a second independent topic that publishes the "raw" safety GPIO value
  • Depending on the value of a newly added parameter either keep the old logic (default) or set the safety depending on the state of the safety GPIO

Changelog Entry

For release notes:

Feature: Allow for level logic in the safety switch

Test coverage

  • Tested on a v6x (with a PX4IO)
  • TODO: Test on a ARK v6x (without a PX4IO)

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 3, 2025 •
edited
Loading

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 192 byte (0.01 %)] 
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +184  +0.0%    +184    .text
  +233%     +84  +233%     +84    SafetyButton::CheckSafetyRequest()
   +67%     +80   +67%     +80    PX4IO::io_handle_status()
  +2.3%     +76  +2.3%     +76    Commander::handle_command()
 -99.9%     +60 -99.9%     +60    [20 Others]
  [NEW]     +44  [NEW]     +44    Safety::handleModeLevel()
  +0.0%     +44  +0.0%     +44    [section .text]
   +77%     +40   +77%     +40    Safety::Safety()
  [NEW]     +38  [NEW]     +38    Safety::handleModeButton()
  [NEW]     +38  [NEW]     +38    Safety::safetySwitchHandler()
  +2.2%     +24  +2.2%     +24    Commander::dataLinkCheck()
  +3.4%     +20  +3.4%     +20    PX4IO::PX4IO()
   +11%     +16   +11%     +16    SafetyButton::SafetyButton()
  [NEW]     +16  [NEW]     +16    __orb_safety_switch
  +3.3%     +12  +3.3%     +12    Commander::disarm()
  +2.5%      +8  +2.5%      +8    Commander::executeActionRequest()
  +2.8%      +8  +2.8%      +8    Commander::manualControlCheck()
  +1.8%      +8  +1.8%      +8    Commander::~Commander()
  -0.5%      -8  -0.5%      -8    Commander::Commander()
  -2.3%     -12  -2.3%     -12    Commander::control_status_leds()
  [DEL]     -80  [DEL]     -80    Safety::safetyButtonHandler()
  -0.2%    -332  -0.2%    -332    g_cromfs_image
+0.1%      +8  +0.1%      +8    .ramfunc
  +1.2%      +4  +1.2%      +4    Ekf::measurementUpdate()
   +20%      +4   +20%      +4    get_orb_meta()
   +14%      +1   +14%      +1    __px4_log_modulename_veneer
 -12.5%      -1 -12.5%      -1    __stm32_rxfifo_discard.isra.0_veneer
+0.0%    +895  [ = ]       0    .debug_abbrev
+0.0%     +64  [ = ]       0    .debug_aranges
+0.0%    +176  [ = ]       0    .debug_frame
+0.1% +16.7Ki  [ = ]       0    .debug_info
+0.0% +2.02Ki  [ = ]       0    .debug_line
 -33.3%      -2  [ = ]       0    [Unmapped]
  +0.0% +2.02Ki  [ = ]       0    [section .debug_line]
+0.0%    +530  [ = ]       0    .debug_loclists
+0.0%     +71  [ = ]       0    .debug_rnglists
  [DEL]      -1  [ = ]       0    [Unmapped]
  +0.0%     +72  [ = ]       0    [section .debug_rnglists]
+0.0%    +835  [ = ]       0    .debug_str
-1.2%      -3  [ = ]       0    .shstrtab
+0.0%     +99  [ = ]       0    .strtab
  [NEW]     +31  [ = ]       0    Safety::handleModeButton()
  [NEW]     +30  [ = ]       0    Safety::handleModeLevel()
  [DEL]     -34  [ = ]       0    Safety::safetyButtonHandler()
  [NEW]     +34  [ = ]       0    Safety::safetySwitchHandler()
  +0.1%     +18  [ = ]       0    [section .strtab]
   +36%     +16  [ = ]       0    ___ZL19param_get_cplusplustPf.isra.0_veneer
 -37.2%     -16  [ = ]       0    __hrt_absolute_time_veneer
  [NEW]     +20  [ = ]       0    __orb_safety_switch
+0.0%    +144  [ = ]       0    .symtab
 -25.0%     -16  [ = ]       0    Commander::control_status_leds()
  +100%     +16  [ = ]       0    Commander::updateControlMode()
 -25.0%     -16  [ = ]       0    Commander::updateParamsImpl()
  +100%     +16  [ = ]       0    HealthAndArmingCheckBase::updateParams()
 -33.3%     -16  [ = ]       0    PX4IO::io_set_arming_state()
   +33%     +16  [ = ]       0    PX4IO::ioctl()
 -33.3%     -32  [ = ]       0    RcvTopicsPubs::init()
  [NEW]     +32  [ = ]       0    Safety::handleModeButton()
  [NEW]     +32  [ = ]       0    Safety::handleModeLevel()
  [DEL]     -48  [ = ]       0    Safety::safetyButtonHandler()
  [NEW]     +48  [ = ]       0    Safety::safetySwitchHandler()
  +0.3%     +32  [ = ]       0    [section .symtab]
   +13%     +32  [ = ]       0    ___ZL19param_get_cplusplustPf.isra.0_veneer
   +33%     +16  [ = ]       0    ___ZN4ListIP13MavlinkStreamE8IteratorppEv.isra.0_veneer
 -40.0%     -32  [ = ]       0    __hrt_absolute_time_veneer
  [NEW]     +48  [ = ]       0    __orb_safety_switch
   +50%     +16  [ = ]       0    __px4_log_modulename_veneer
 -33.3%     -16  [ = ]       0    __stm32_rxfifo_discard.isra.0_veneer
 -25.0%     -16  [ = ]       0    __up_flush_dcache_veneer
   +11%     +32  [ = ]       0    uORB::PublicationBase::get_topic()
-2.1%    -192  [ = ]       0    [Unmapped]
+0.0% +21.5Ki  +0.0%    +192    TOTAL
px4_fmu-v6x [Total VM Diff: 240 byte (0.01 %)] 
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +240  +0.0%    +240    .text
 -99.7%    +128 -99.7%    +128    [39 Others]
  +233%     +84  +233%     +84    SafetyButton::CheckSafetyRequest()
   +67%     +80   +67%     +80    PX4IO::io_handle_status()
  +2.3%     +76  +2.3%     +76    Commander::handle_command()
  [NEW]     +44  [NEW]     +44    Safety::handleModeLevel()
  +0.0%     +44  +0.0%     +44    [section .text]
   +77%     +40   +77%     +40    Safety::Safety()
  [NEW]     +38  [NEW]     +38    Safety::handleModeButton()
  [NEW]     +38  [NEW]     +38    Safety::safetySwitchHandler()
  +2.2%     +24  +2.2%     +24    Commander::dataLinkCheck()
  +3.4%     +20  +3.4%     +20    PX4IO::PX4IO()
   +11%     +16   +11%     +16    SafetyButton::SafetyButton()
  [NEW]     +16  [NEW]     +16    __orb_safety_switch
  +3.3%     +12  +3.3%     +12    Commander::disarm()
  +2.5%      +8  +2.5%      +8    Commander::executeActionRequest()
  +2.8%      +8  +2.8%      +8    Commander::manualControlCheck()
  +1.8%      +8  +1.8%      +8    Commander::~Commander()
  -0.5%      -8  -0.5%      -8    Commander::Commander()
  -2.3%     -12  -2.3%     -12    Commander::control_status_leds()
  [DEL]     -80  [DEL]     -80    Safety::safetyButtonHandler()
  -0.2%    -344  -0.2%    -344    g_cromfs_image
+0.0%    +895  [ = ]       0    .debug_abbrev
+0.0%     +64  [ = ]       0    .debug_aranges
+0.0%    +176  [ = ]       0    .debug_frame
+0.1% +16.5Ki  [ = ]       0    .debug_info
+0.0% +2.06Ki  [ = ]       0    .debug_line
 -25.0%      -1  [ = ]       0    [Unmapped]
  +0.0% +2.06Ki  [ = ]       0    [section .debug_line]
+0.0%    +519  [ = ]       0    .debug_loclists
+0.0%     +74  [ = ]       0    .debug_rnglists
 -66.7%      -2  [ = ]       0    [Unmapped]
  +0.0%     +76  [ = ]       0    [section .debug_rnglists]
+0.0%    +835  [ = ]       0    .debug_str
+0.4%      +1  [ = ]       0    .shstrtab
+0.0%     +99  [ = ]       0    .strtab
  [NEW]     +31  [ = ]       0    Safety::handleModeButton()
  [NEW]     +30  [ = ]       0    Safety::handleModeLevel()
  [DEL]     -34  [ = ]       0    Safety::safetyButtonHandler()
  [NEW]     +34  [ = ]       0    Safety::safetySwitchHandler()
  +0.1%     +18  [ = ]       0    [section .strtab]
  [NEW]     +20  [ = ]       0    __orb_safety_switch
+0.0%    +144  [ = ]       0    .symtab
 -25.0%     -16  [ = ]       0    Commander::control_status_leds()
  +100%     +16  [ = ]       0    Commander::updateControlMode()
 -25.0%     -16  [ = ]       0    Commander::updateParamsImpl()
 -50.0%     -16  [ = ]       0    ConstLayer::contains()
   +50%     +16  [ = ]       0    ConstLayer::store()
 -20.0%     -16  [ = ]       0    DynamicSparseLayer::DynamicSparseLayer()
  +100%     +16  [ = ]       0    HealthAndArmingCheckBase::updateParams()
 -33.3%     -16  [ = ]       0    PX4IO::io_set_arming_state()
   +33%     +16  [ = ]       0    PX4IO::ioctl()
 -33.3%     -32  [ = ]       0    RcvTopicsPubs::init()
  [NEW]     +32  [ = ]       0    Safety::handleModeButton()
  [NEW]     +32  [ = ]       0    Safety::handleModeLevel()
  [DEL]     -48  [ = ]       0    Safety::safetyButtonHandler()
  [NEW]     +48  [ = ]       0    Safety::safetySwitchHandler()
 -99.3%     +32  [ = ]       0    [3 Others]
  +0.4%     +48  [ = ]       0    [section .symtab]
  [NEW]     +48  [ = ]       0    __orb_safety_switch
   +50%     +16  [ = ]       0    atanf
   +33%     +16  [ = ]       0    atanlo
 -50.0%     -16  [ = ]       0    param_for_index
 -50.0%     -16  [ = ]       0    param_get_index
-3.1%    -240  [ = ]       0    [Unmapped]
+0.0% +21.3Ki  +0.0%    +240    TOTAL

Updated: 2025-07-04T16:10:06

@alexcekay alexcekay force-pushed the pr-safety-switch-level branch from ae9539c to 50652ac Compare July 4, 2025 16:03
@alexcekay alexcekay force-pushed the pr-safety-switch-level branch from 50652ac to 8cecaca Compare July 4, 2025 16:03
@alexcekay alexcekay requested a review from niklaut July 4, 2025 16:19
@dakejahl
Copy link
Contributor

dakejahl commented Jul 6, 2025

Should also test with a CAN GPS

@github-actions github-actions bot added the stale label Aug 6, 2025
dagar
dagar reviewed Aug 8, 2025
View reviewed changes
src/modules/commander/Safety.cpp

button_event_s button_event;

while (_safety_button_sub.update(&button_event)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upper bound on loops just to be paranoid.

@dagar
Copy link
Member

dagar commented Aug 8, 2025

I like the idea of optionally exposing GPIO events as publications. If used beyond this instead of SafetySwitch.msg we could have something like ButtonEvent.msg and then topics like safety_button, power_button, etc.

@github-actions github-actions bot removed the stale label Aug 8, 2025
@github-actions github-actions bot added the stale label Sep 13, 2025
@github-actions GitHub Actions
Copy link

This pull request has been closed after being marked as stale with no further activity. Thank you for the time and effort you put into this contribution. If you’d like to continue the discussion or update the work, please feel free to reopen it or submit a new PR.

@github-actions github-actions bot closed this Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dagar dagar dagar left review comments

@niklaut niklaut Awaiting requested review from niklaut

Assignees

No one assigned

Labels

stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexcekay @dakejahl @dagar