Skip to content

feat(failure_detector): unite motorfailure offsets#26990

Draft
ttechnick wants to merge 1 commit intomainfrom
pr-unite-motorfailure-offset
Draft

feat(failure_detector): unite motorfailure offsets#26990
ttechnick wants to merge 1 commit intomainfrom
pr-unite-motorfailure-offset

Conversation

@ttechnick
Copy link
Copy Markdown
Member

Solved Problem

This PR simplifies the motor over/under-current detection introduced in #26262.

Currently, overcurrent and undercurrent thresholds must be configured separately. In practice, estimating even one of these reliably is already difficult, making the feature unnecessarily complex to use.


Solution

Instead of defining separate offsets for overcurrent and undercurrent, this PR introduces a single symmetric offset.

The new parameter MOTFAIL_OFF is applied symmetrically around the expected current-to-throttle relationship (MOTFAIL_C2T). This simplifies configuration while preserving the ability to detect motor failures.

Using the original image from @MaEtUgR:

Untitled design
  • Purple: Ideal current-to-throttle relationship (as defined by MOTFAIL_C2T)
  • Green: Symmetric offset band (defined by MOTFAIL_OFF)

Changelog Entry

For release notes:

Feature: Use Symmetric motorfailure offset limit
Removed param `MOTFAIL_LOW_OFF` & `MOTFAIL_HIGH_OFF` replaced by `MOTFAIL_OFF`

Issue

Both the current and proposed implementations assume:

  • 0 current at 0 throttle
  • a linear increase in current with throttle

However, as shown in the original analysis by @MaEtUgR, this assumption does not hold in practice. There is typically a dead zone, where motor command increases before significant current is drawn.

This behavior cannot be represented in the current model—neither before nor after this change. Despite this limitation, the simplification introduced by this PR is considered a worthwhile trade-off.


Alternatives

To address the limitation mentioned above, the following options come to my mind:

  1. Introduce a parameter to better describe the current curve (e.g., current at 50% throttle or a similar reference point).
  2. Estimate the throttle offset at which current starts flowing and use it as a fixed approximation.

Open to hear your thoughts 👐

@ttechnick ttechnick requested a review from MaEtUgR April 7, 2026 14:15
@ttechnick ttechnick self-assigned this Apr 7, 2026
@ttechnick
Copy link
Copy Markdown
Member Author

Fun fact:

The image shown above—originally used to advocate for separate low and high limits—actually supports using a symmetric limit.

The key issue is that the yellow and red “lines” should be parallel to the purple line, not the measured blue “ideal” line. This follows from the assumption of 0 current at 0 command, which defines the reference slope.

As a result:

  • The overcurrent deviation is largest at high motor commands
  • The undercurrent deviation is largest at low motor commands

In this specific example, both deviations appear to be very similar in magnitude, based on the image provided by @MaEtUgR.

😀

@ttechnick ttechnick requested a review from dakejahl April 7, 2026 14:20
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

No broken links found in changed files.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: -192 byte (-0.01 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +55  [ = ]       0    .debug_abbrev
-0.0% -3.65Ki  [ = ]       0    .debug_info
-0.0%    -147  [ = ]       0    .debug_line
 -50.0%      -2  [ = ]       0    [Unmapped]
  -0.0%    -145  [ = ]       0    [section .debug_line]
-0.0%    -276  [ = ]       0    .debug_loclists
-0.0%     -33  [ = ]       0    .debug_rnglists
  [NEW]      +3  [ = ]       0    [Unmapped]
  -0.0%     -36  [ = ]       0    [section .debug_rnglists]
-0.0%    -975  [ = ]       0    .debug_str
-32.7% -3.81Ki  [ = ]       0    [Unmapped]
-0.0%    -192  -0.0%    -192    .text
  +3.4%     +12  +3.4%     +12    Commander::handleAutoDisarm()
  +1.5%      +8  +1.5%      +8    Commander::arm()
  [NEW]      +4  [NEW]      +4    CSWTCH.822
  +1.7%      +4  +1.7%      +4    Commander::landDetectorUpdate()
  +1.1%      +4  +1.1%      +4    Commander::~Commander()
  [DEL]      -4  [DEL]      -4    CSWTCH.824
  -1.1%      -4  -1.1%      -4    Commander::disarm()
  -1.0%      -4  -1.0%      -4    Commander::executeActionRequest()
  -5.3%      -4  -5.3%      -4    Commander::getPrearmState()
  -2.4%      -4  -2.4%      -4    Commander::handleCommandsFromModeExecutors()
  -1.6%      -4  -1.6%      -4    Commander::manualControlCheck()
  -2.2%      -4  -2.2%      -4    Commander::updateControlMode()
 -100.2%      -8 -100.2%      -8    [8 Others]
  -0.0%      -8  -0.0%      -8    px4::parameters
  -4.8%     -12  -4.8%     -12    Commander::checkForMissionUpdate()
 -11.1%     -12 -11.1%     -12    EscChecks::updateParamsImpl()
  -0.0%     -16  -0.0%     -16    [section .text]
  -7.4%     -20  -7.4%     -20    EscChecks::EscChecks()
  -0.0%     -32  -0.0%     -32    g_cromfs_image
  -7.2%     -40  -7.2%     -40    Commander::control_status_leds()
  -1.3%     -48  -1.3%     -48    Commander::handle_command()
-0.0% -9.00Ki  -0.0%    -192    TOTAL

px4_fmu-v6x [Total VM Diff: -248 byte (-0.01 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +55  [ = ]       0    .debug_abbrev
-0.0% -3.60Ki  [ = ]       0    .debug_info
-0.0%    -181  [ = ]       0    .debug_line
-0.0%    -299  [ = ]       0    .debug_loclists
-0.0%     -42  [ = ]       0    .debug_rnglists
  [DEL]      -2  [ = ]       0    [Unmapped]
  -0.0%     -40  [ = ]       0    [section .debug_rnglists]
-0.0%    -975  [ = ]       0    .debug_str
+3.5%    +248  [ = ]       0    [Unmapped]
-0.0%    -248  -0.0%    -248    .text
  +3.4%     +12  +3.4%     +12    Commander::handleAutoDisarm()
  +1.5%      +8  +1.5%      +8    Commander::arm()
  [NEW]      +4  [NEW]      +4    CSWTCH.822
  +1.7%      +4  +1.7%      +4    Commander::landDetectorUpdate()
  +1.1%      +4  +1.1%      +4    Commander::~Commander()
  [DEL]      -4  [DEL]      -4    CSWTCH.824
  -1.1%      -4  -1.1%      -4    Commander::disarm()
  -1.0%      -4  -1.0%      -4    Commander::executeActionRequest()
  -5.3%      -4  -5.3%      -4    Commander::getPrearmState()
  -2.4%      -4  -2.4%      -4    Commander::handleCommandsFromModeExecutors()
  -1.6%      -4  -1.6%      -4    Commander::manualControlCheck()
  -2.2%      -4  -2.2%      -4    Commander::updateControlMode()
 -25.0%      -4 -25.0%      -4    ConstLayer::contains()
  -0.0%      -8  -0.0%      -8    px4::parameters
  -4.8%     -12  -4.8%     -12    Commander::checkForMissionUpdate()
 -11.1%     -12 -11.1%     -12    EscChecks::updateParamsImpl()
  -7.4%     -20  -7.4%     -20    EscChecks::EscChecks()
  -0.0%     -24  -0.0%     -24    [section .text]
  -7.2%     -40  -7.2%     -40    Commander::control_status_leds()
  -1.3%     -48  -1.3%     -48    Commander::handle_command()
 -100.1%     -84 -100.1%     -84    [29 Others]
-0.0% -5.00Ki  -0.0%    -248    TOTAL

Updated: 2026-04-07T14:23:20

Comment thread docs/en/config/safety.md

### Motor Failure Trigger

The failure detector can be configured to detect a motor failure while armed (and trigger an associated action) if the ESC current falls outside expected bounds for more than [MOTFAIL_TIME](#MOTFAIL_TIME) seconds.
Copy link
Copy Markdown
Contributor

@hamishwillee hamishwillee Apr 9, 2026

Choose a reason for hiding this comment

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

Thanks for keeping the docs updated. Perhaps, given we specify a single value? UP to you.

Suggested change
The failure detector can be configured to detect a motor failure while armed (and trigger an associated action) if the ESC current falls outside expected threshold for more than [MOTFAIL_TIME](#MOTFAIL_TIME) seconds.

EDIT NOTE, I changed "outside expected bounds" to "outside expected threshold" in this suggestion. It isn't rendering properly for me, so if you agree you might need to apply separately.

@ttechnick ttechnick marked this pull request as draft April 14, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants