Skip to content

Comments

ControlAllocator: Always respect output slew rate#26530

Open
mbjd wants to merge 10 commits intomainfrom
pr-always-respect-output-slewrate
Open

ControlAllocator: Always respect output slew rate#26530
mbjd wants to merge 10 commits intomainfrom
pr-always-respect-output-slewrate

Conversation

@mbjd
Copy link
Contributor

@mbjd mbjd commented Feb 19, 2026

Solved Problem

It is possible to override the slew rates CA_Rx_SLEW by sending a very low thrust setpoint, which then causes the motor to be labelled stopped, which immediately gives it a motor output of NaN and thus its disarmed PWM. In some cases the ESC is not able to spin back up, even after only a very short spike of disarmed PWM.

More details

Such zero-thrust spikes currently happen via this path:

  • Navigator resets triplet on mode switch to idle setpoints, and takes a split second before updating the triplet to the correct one (only known case is when switching to RTL)
  • FixedWingModeManager commands zero thrust as a consequence
  • Allocator decides based on the low thrust that the motor should be completely stopped
  • And then overwrites the motor ouput with NaN, turning off the motor and overriding any configured output slew rate

While this issue should be addressed at its root in a general navigator cleanup ™️, setting an output slew rate limit provides an easy hotfix after this PR for this and similar issues.

But also generally, it is very unintuitive (and possibly dangerous if the propulsion system actually requires a limited slew rate...) for the output slew rate to have this type of exception.

Solution

  • move stopped motor handling from publish_actuator_controls to main run, crucially before applying the slew rate. publish_actuator_controls now only does what it says
  • modify slew rate to treat NaN as zero, more specifically behaving like this on different input transitions:
    • between 0 and NaN: immediately match input
    • nonzero to NaN: sink to zero with slew rate, then replace zero by NaN
    • NaN to nonzero: replace NaN by zero, then rise with slew rate to input
    • between nonzero and 0: slew limit, then match input
  • adapt ice shedding to modify the actuator setpoint between stopping motors and applying slew rate (previously modified also motor outputs, after slew rate limit)

Context

This is one part of the changes I pushed on to #24684.

  • The slew rate changes are copied almost 1:1 from there
  • But not the definition that also in the thrust setpoint, NaN means stop the motor and 0 means spin at min RPM.

The changes are actually quite orthogonal, they just happen to touch some of the same lines in CA, so I propose to do it in two steps.

Changelog Entry

For release notes:

ControlAllocator: Handle stopped motors before slew rate limit. Fixes possibility to override output slew rate by briefly giving very low thrust setpoint.

Open questions

  • The slew rate now applies after EVERYTHING, so the stopped motors mask cannot immediately apply if a slew limit is set. This may be a slight regression when it comes to motor failures, which also apply through the stopped motors mask. @MaEtUgR as our resident expert on motor failure (injection) do you have an opinion on it?
  • The NaN-handling slew rate implementation is currently allocator-specific. I have a draft implementation in src/lib/slew_rate/SlewRate.hpp, which we could use instead. Then we would also have to fix the current parameterisation inconsistency:
    • most slew rates (FW_THR_SLEW_MAX, FW_PN_R_SLEW_MAX, VT_PSHER_SLEW, everything handled by the SlewRate lib) are in units of [unit]/s or 1/s, specifying the max absolute rate of change either in absolute units, or in "normalised" units as a fraction of the relevant max-min range.
    • the CA_Rx_SLEW rates are parameterised in s (the min allowed time to traverse the range). This is the inverse of the default.
    • If am in favour of using one convention and adapting the CA slew rates to the standard (and using the library)
    • But it would be a breaking change: If somebody now sets a very low CA "slew rate" like 0.1 seconds (meaning actually a high slew limit that is mostly unrestrictive), the change makes it now a very slow and restrictive 0.1 / second, making it unflyable. Possible solutions:
      • Rename the params and introduce a param translation
      • Just write a very big warning in the release note
      • Use the library BUT invert the parameter to match old convention
  • I have another version that makes the slight flash increase a slight decrease, but comes at the cost of readability (inlining some functions, ditching some intermediate variables and simplifying branching). If needed I can include it

Test coverage

Sim sanity check: gz_standard_vtol in altitude mode (MC), with fast pitch inputs to enable/disable pusher assist, CA_R4_SLEW=1 and high rate logging.

before: actuator_motors/control.05 switches immediately between NaN and large positive values. End result is that the motor switches immediately from disarmed PWM to high PWM without respecting slew rate
image

after: actuator_motors/control.05 switches between NaN and 0 (not directly to nonzero values), and otherwise always respects slew rate. End result: physical motor respects slew rate.
image

mbjd added 3 commits February 19, 2026 09:59
Will also be handled by main motor slew rate in later commit
introducing more variables than strictly necessary to clearly specify
what is input and what is output.

no functional change
@github-actions
Copy link

github-actions bot commented Feb 19, 2026

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 64 byte (0 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +64  +0.0%     +64    .text
  [NEW]    +204  [NEW]    +204    ControlAllocator::handle_stopped_motors()
   +59%     +80   +59%     +80    ControlAllocation::applySlewRateLimit()
  +1.2%     +12  +1.2%     +12    ControlAllocator::Run()
  +0.0%     +12  +0.0%     +12    [section .text]
   +44%      +4   +44%      +4    g_nullstring
 -16.7%      -4 -16.7%      -4    _file_shutdown()
  -2.6%     -24  -2.6%     -24    ControlAllocator::ControlAllocator()
 -40.4%     -92 -40.4%     -92    ControlAllocator::get_ice_shedding_output()
 -23.9%    -128 -23.9%    -128    ControlAllocator::publish_actuator_controls()
+0.0%    +104  [ = ]       0    .debug_abbrev
+0.0%      +8  [ = ]       0    .debug_aranges
+0.0%     +24  [ = ]       0    .debug_frame
-0.0%     -44  [ = ]       0    .debug_info
+0.0%    +156  [ = ]       0    .debug_line
  [DEL]      -6  [ = ]       0    [Unmapped]
  +0.0%    +162  [ = ]       0    [section .debug_line]
+0.0%    +203  [ = ]       0    .debug_loclists
-0.0%     -39  [ = ]       0    .debug_rnglists
+0.0%     +24  [ = ]       0    .debug_str
-0.8%      -2  [ = ]       0    .shstrtab
+0.0%     +46  [ = ]       0    .strtab
  -2.0%      -1  [ = ]       0    ControlAllocator::get_ice_shedding_output()
  [NEW]     +54  [ = ]       0    ControlAllocator::handle_stopped_motors()
 -15.6%      -7  [ = ]       0    MavlinkStreamTimesync::get_size()
+0.0%     +48  [ = ]       0    .symtab
  [NEW]     +64  [ = ]       0    ControlAllocator::handle_stopped_motors()
 -25.0%     -16  [ = ]       0    ControlAllocator::print_usage()
   +33%     +16  [ = ]       0    ControlAllocator::publish_actuator_controls()
 -33.3%     -16  [ = ]       0    MavlinkStreamTimesync::get_size()
 -33.3%     -16  [ = ]       0    ___ZN4ListIP13MavlinkStreamE8IteratorppEv.isra.0_veneer
 -25.0%     -16  [ = ]       0    __arp_out_veneer
   +50%     +16  [ = ]       0    __nxsem_restore_baseprio_irq_veneer
   +33%     +16  [ = ]       0    __stm32_recvfifo_veneer
-0.5%     -64  [ = ]       0    [Unmapped]
+0.0%    +528  +0.0%     +64    TOTAL

px4_fmu-v6x [Total VM Diff: 56 byte (0 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +56  +0.0%     +56    .text
  [NEW]    +204  [NEW]    +204    ControlAllocator::handle_stopped_motors()
   +59%     +80   +59%     +80    ControlAllocation::applySlewRateLimit()
  +1.2%     +12  +1.2%     +12    ControlAllocator::Run()
  +0.0%     +12  +0.0%     +12    [section .text]
 -16.7%      -4 -16.7%      -4    _file_shutdown()
 -30.8%      -4 -30.8%      -4    g_nullstring
  -2.6%     -24  -2.6%     -24    ControlAllocator::ControlAllocator()
 -40.4%     -92 -40.4%     -92    ControlAllocator::get_ice_shedding_output()
 -23.9%    -128 -23.9%    -128    ControlAllocator::publish_actuator_controls()
+0.0%    +104  [ = ]       0    .debug_abbrev
+0.0%      +8  [ = ]       0    .debug_aranges
+0.0%     +24  [ = ]       0    .debug_frame
-0.0%     -44  [ = ]       0    .debug_info
+0.0%    +164  [ = ]       0    .debug_line
   +67%      +2  [ = ]       0    [Unmapped]
  +0.0%    +162  [ = ]       0    [section .debug_line]
+0.0%    +257  [ = ]       0    .debug_loclists
-0.0%     -41  [ = ]       0    .debug_rnglists
  [DEL]      -2  [ = ]       0    [Unmapped]
  -0.0%     -39  [ = ]       0    [section .debug_rnglists]
+0.0%     +24  [ = ]       0    .debug_str
+0.9%      +2  [ = ]       0    .shstrtab
+0.0%     +46  [ = ]       0    .strtab
  -2.0%      -1  [ = ]       0    ControlAllocator::get_ice_shedding_output()
  [NEW]     +54  [ = ]       0    ControlAllocator::handle_stopped_motors()
 -15.6%      -7  [ = ]       0    MavlinkStreamTimesync::get_size()
+0.0%     +48  [ = ]       0    .symtab
  [NEW]     +64  [ = ]       0    ControlAllocator::handle_stopped_motors()
 -25.0%     -16  [ = ]       0    ControlAllocator::print_usage()
   +33%     +16  [ = ]       0    ControlAllocator::publish_actuator_controls()
 -33.3%     -16  [ = ]       0    MavlinkStreamTimesync::get_size()
-0.9%     -56  [ = ]       0    [Unmapped]
+0.0%    +592  +0.0%     +56    TOTAL

Updated: 2026-02-20T09:13:23

mbjd added 2 commits February 19, 2026 11:55
 - get stopped motors mask in main Run()
 - introduce ApplyNanToActuators in ControlAllocation to apply it
 - refactor ice shedding with extra function that modifies the
   actuator_sp (passed by ref)
@mbjd mbjd force-pushed the pr-always-respect-output-slewrate branch from f3fe0ec to 88f7191 Compare February 19, 2026 10:56
@mbjd mbjd requested review from MaEtUgR and sfuhrer February 19, 2026 10:56
@mbjd mbjd marked this pull request as ready for review February 19, 2026 10:56
Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Not extremely elegant but I can't think of a more straight forward way. The issue is real, you expect the slew rate also to work when stopping a motor suddenly as it often is added to protect the hardware (e.g. from large back currents).

@mbjd mbjd force-pushed the pr-always-respect-output-slewrate branch from f169b75 to d18a652 Compare February 20, 2026 08:04
@mbjd mbjd force-pushed the pr-always-respect-output-slewrate branch from d18a652 to c9f7522 Compare February 20, 2026 08:06
@sfuhrer
Copy link
Contributor

sfuhrer commented Feb 20, 2026

Have you tested it in hardware? Given that this is a very low level change I would recommend you do that. Ideally even check performance (CPU).
RE using the slew rate library: Agree with you that it is confusing for the user (rise time vs slew rate), and sub-optimal for the code as we have to maintain both implementations. At a minimum we should update the field in the UI, and then consider the next steps. Think this is big enough for a separate PR!
image

@mbjd
Copy link
Contributor Author

mbjd commented Feb 20, 2026

no, will do next week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants