### Summary
`AP_Vehicle::reboot()` leaves the PWM timers running on their las…t commanded values right up to `NVIC_SystemReset()`, so the reset truncates whatever pulse is in flight and a servo reads the stub as a valid, much shorter command; add `AP_HAL::RCOutput::prepare_for_reboot()`, which zeroes the outputs and refuses every further write, and call it from `reboot()` for all vehicles rather than only Sub.
### Classification & Testing (check all that apply and add your own)
- [x] Checked by a human programmer
- [ ] Non-functional change
- [ ] No-binary change
- [ ] Infrastructure change (e.g. unit tests, helper scripts)
- [ ] Automated test(s) verify changes (e.g. unit test, autotest)
- [x] Tested manually, description below (e.g. SITL)
- [x] Tested on hardware
- [ ] Logs attached
- [x] Logs available on request
Tested on a YJUAV_A6SE_H743 running ArduPlane, with a flap servo on output 8 (PE14) commanded to `SERVO8_MAX` of 1850 us, outputs 1-8 in OneShot (`SERVO_BLH_MASK 255`, `SERVO_BLH_OTYPE 1`), safety off, captured with a Saleae on the servo signal pin.
The two firmware images used for the hardware A/B differ by exactly one byte, at flash address 0x8024924, which is the constant returned by `should_zero_rc_outputs_on_reboot()`. Nothing else in the images differs, so nothing else can account for the change in behaviour. That A/B tested the zeroing alone; the freeze that now accompanies it was added afterwards and was checked in simulation, described below.
| Test | Without the change | With the change |
|---|---|---|
| Hardware: flap movement on a commanded reboot | deflected on 2 of 3 reboots | no deflection in 10 consecutive reboots |
| Hardware: last pulse before the reset | cut to 1049 us from a commanded 1850 us | full width on all 10 reboots |
| Hardware: all pulses in a 262 s, 10-reboot capture | n/a | 52500 pulses per channel, every one at nominal width |
| Renode SITL-equivalent, plain PWM at 50 Hz | 1 truncated final pulse in 15 reboots | 30 of 30 full width |
| Renode, OneShot | not measured | 10 of 10 full width |
| Plane and Copter build for the test board | pass | pass |
| SITL: servo write before the freeze | n/a | applied, twice, at two values |
| SITL: output level after the freeze, no command | n/a | zero |
| SITL: servo write after the freeze | n/a | dropped, output stays at zero |
| Sub and Plane builds for SITL | pass | pass |
| Plane build for CubeOrange, which has an IOMCU | pass | pass |
| Every commit builds (Plane and Sub for SITL) | | 8 of 8 |
| `Tools/scripts/check_branch_conventions.py` | | all checks pass |
The observed hardware failure rate matches the geometry. The reset lands at a time uncorrelated with the output timing, so the chance of it falling inside a pulse is the duty cycle during the reboot delay: 1850 of 5850 us in OneShot, about 32%, against 1850 of 20000 us for plain PWM at 50 Hz, about 9%. That is also why the Renode plain-PWM rate came out at 1 in 15.
Output 8 is shown rather than output 1 because output 1 on this board is on PA15, whose separate reset-pull-up problem is addressed by a different branch. PE14 has no such behaviour, so its before and after are attributable to this change alone.
### Description
`AP_Vehicle::reboot()` runs, in order:
```
if (should_zero_rc_outputs_on_reboot()) {
SRV_Channels::zero_rc_outputs();
}
...
hal.rcout->force_safety_on();
AP_Param::flush();
hal.scheduler->register_delay_callback(nullptr, 5);
hal.scheduler->delay(200);
...
hal.scheduler->reboot(hold_in_bootloader); // -> NVIC_SystemReset()
```
`force_safety_on()` looks like it should make the outputs safe, but on a board without an IOMCU it only sets `safety_state = SAFETY_DISARMED`. The zeroing of channels outside `BRD_SAFETY_MASK` happens later, in `AP_HAL_ChibiOS::RCOutput::push_local()`:
```
if (safety_on && !(safety_mask & (1U<<(chan+chan_offset)))) {
// safety is on, overwride pwm
period_us = 0;
}
```
and that code only runs when something pushes the outputs. Nothing pushes between that call and the reset when the reboot came from the main thread, because `reboot()` is running on it and then blocks. The timers therefore keep their existing compare values and free-run until the reset stops them, wherever the counter happens to be.
The window is longer than the `delay(200)` suggests. `AP_Param::flush()` runs first and is two hundred iterations of a ten millisecond delay, its own comment saying "2 seconds max", so the gap between the zeroing and the reset can be about 2.2 seconds rather than 200 ms.
The result is not a malformed signal. It is a shorter pulse that is still inside the valid range, so a servo decodes it as a genuine command and drives there, then holds that position for the several seconds the board takes to boot. In the capture above an 1850 us pulse became 1049 us, which is near the minimum end of the range, and the flap slammed over.
`SRV_Channels::zero_rc_outputs()` already exists for precisely this, and its comment says so:
```
/* Send an invalid signal to the motors to prevent spinning due to
* neutral (1500) pwm pulse being cut short. For that matter,
* send an invalid signal to all channels to prevent
* undesired/unexpected behavior
*/
```
but only Sub was calling it, via an override of `should_zero_rc_outputs_on_reboot()`.
This PR makes it the default for every vehicle. Sub's override and the virtual it overrode both go, since nothing needs them once `reboot()` calls the new path unconditionally, and `zero_rc_outputs()` is renamed to `prepare_for_reboot()` because it now does more than write zeros. The commits are ordered so that each one builds on its own: the Linux backends get the bounds check they were missing, the HAL gains the call, the two backends implement it, `SRV_Channel` gains the wrapper, `AP_Vehicle` switches to it, then the override, the virtual and the old function are removed in turn.
The fix is not probabilistic. Zeroing happens as soon as the reboot is commanded; because `CCRx` is preloaded, the pulse in flight completes at its full width and no further pulse begins, so by the time the reset arrives there is nothing left to truncate. That is what the middle panel of the figure shows.
### Another thread writing the outputs after the zeroing
Zeroing the outputs is not enough on its own, because the window before the reset is up to about 2.2 seconds and the caller is not the only thing that can write a channel in it.
**Copter's rate thread is a writer in every case.** It runs at `PRIORITY_RCOUT`, above the main loop, calls `motors_output()` on every pass, and stands down only when `FSTRATE_ENABLE` is zero. It takes no scheduler semaphore, so nothing about how the reboot was commanded stops it. It will write channels again immediately after the zeroing and restart a pulse for the reset to truncate, which is exactly the fault the zeroing exists to prevent.
**Where the reboot came from decides whether the main loop is a writer too.** Three callers reach `AP_Vehicle::reboot()` from three different threads:
| Reboot from | Main loop during the window |
|---|---|
| A GCS command, on the main thread | it is the caller, so it is not outputting |
| Lua `vehicle:reboot()` | blocked, see below |
| FrSky MAVlite, on the `FrSky` thread | still running and pushing |
The Lua case is the subtle one. `vehicle` is declared `scheduler-semaphore` in `bindings.desc`, so the binding holds the scheduler semaphore, and `AP_Scheduler::loop()` takes that same semaphore blocking on each iteration. The main loop therefore stops for the duration. That removes the main loop as a writer on the Lua path, but not the rate thread, which never touches that semaphore.
**Some writers never pass through a `SRV_Channel` at all.** `AP_BLHeli` and Copter's toy mode load test call `hal.rcout->write()` directly.
The freeze therefore lives in the HAL rather than in `SRV_Channel`, for three reasons.
It is below every writer. `RCOutput::write()` drops everything once frozen, which catches the direct callers above and covers the forward to the IOMCU as well.
It can be made race-free. A freeze implemented by zeroing `period[]` and `period_corked[]` cannot be: another thread may be inside `push()`'s `memcpy` between those arrays, and a torn copy would put pre-reboot values back after the zeroing and then program them into the timers. Instead the flag is tested where the value is handed to the timer, in `push_local()` and `dshot_send()`, alongside the existing safety substitution. Every path from `period[]` to a compare register passes through one of those two tests, so there is no shared array to tear over and no dependence on whatever transaction another thread is part way through. The reboot path itself does not cork or push at all.
It owns the timers. Zero reaches `pwmEnableChannel(..., 0)` and the compare register is preloaded, so the pulse in flight completes at its full width and no further pulse begins. That is the property the fix depends on, and only the HAL can guarantee it.
The flag is one-way by design. There is nothing to clear it: the reset does that, along with the rest of RAM.
A thread that passes the test at the top of `write()` just before the freeze is harmless on an FMU channel. It stores its value in `period[]`, and its own `push_local()` then substitutes zero for it on the way to the register. The forward to an IOMCU sits past that test and has no second one, so the IOMCU is instead told to force safety on. It applies that itself in `pwm_out_update()`, which a late write cannot undo. Channels in `BRD_SAFETY_MASK` are exempt from that on the IOMCU and are covered only by the zeros written to it.
What remains is a thread already inside `push_local()` or `dshot_send()` that has passed the per-channel test and has not yet reached the register write, a window of a few instructions. That is why the test is per channel rather than hoisted out of the loop. Closing it entirely would need a lock covering the period arrays and both output loops, on the rate thread's path, which is not worth it for this.
`Tools/autotest/` has no coverage for this, so it was checked live against a SITL ArduPlane, driving an unassigned channel with `MAV_CMD_DO_SET_SERVO` and calling `prepare_for_reboot()` in the running process with gdb. Writes of 1600 and then 1400 took effect, the output went to zero on the call, and a following write of 1200 was dropped with the output still at zero.
### Why `force_safety_on()` does not already do this
`reboot()` calls `force_safety_on()` a few lines later, which looks like it should make the outputs safe. For a PWM or OneShot output on the FMU it does nothing at all. On a board without an IOMCU it only sets `safety_state`, and the substitution that acts on it lives in `push_local()`, which runs only from `write()` when uncorked and from `push()`. Nothing writes or pushes between that call and the reset, because the line above it nulls the delay callback and the thread then blocks in `delay(200)`. The compare registers keep their values and the timers free-run until the reset stops them.
It is not entirely inert. DShot channels do stop, because `dshot_send()` re-reads the safety state on every frame and the rcout thread keeps generating frames throughout the delay. On an IOMCU board the IO channels are cut in hardware once the IOMCU processes the queued event. Neither covers the case measured here, and both exempt every channel in `BRD_SAFETY_MASK`.
Things reviewers may want to weigh:
- On a traditional helicopter the swashplate servos now stop being driven when a reboot is commanded rather than holding their last position. This is on the ground, and it is the same behaviour Sub has had, but it is a real change for heli.
- On a board with an IOMCU the zeros written to it are corked and pushed as one transaction. Uncorked, `AP_IOMCU::write_channel()` pushes itself, the IOMCU thread runs above the calling thread, and its send is rate limited to one per 2 ms, so all but the first channel's zero would have been dropped.
- Under `AP_SIM_ENABLED` the simulation's output array is zeroed explicitly, because the freeze in `write()` returns above the point where that array is updated.
- The freeze covers actuator outputs. Serial LED output is deliberately not covered, since it drives no actuator.
- DShot channels stop receiving frames rather than being sent an explicit stop, because `dshot_send()` skips a channel whose value is zero. That is already what a disarmed vehicle's zeroed outputs do. Queued DShot *commands* take a different path that does not look at the channel value, so `dshot_send_groups()` also returns early once the outputs are frozen. Whether stopping the frame stream up to 2.2 seconds earlier changes any particular ESC's rearm timing was not measured.
- The freeze is one-way. Nothing after a commanded reboot can drive an output again, which is the intent, but a reboot that somehow failed to reset would leave the outputs dead until power cycled.
- Backends other than ChibiOS and SITL inherit a default that corks, writes zeros to every channel and pushes, which is what `zero_rc_outputs()` did before, so Linux, ESP32, QURT and Empty all zero their outputs now. No backend loses behaviour. That matters for Sub in particular, which is the one vehicle that had this on master and which builds for the Sysfs Linux boards navio2 and edge, where nothing else would zero the outputs: `RCOutput_Sysfs` implements neither this nor `force_safety_on()`, and `Linux::Scheduler::reboot()` is `exit(1)`, so the kernel PWM peripheral would otherwise hold its last duty cycle indefinitely.
- The default does not freeze, it only zeros, so on Linux a rate thread can re-drive a channel inside the window, and with `reboot()` being `exit(1)` the last value written stays latched in the kernel PWM. That is what master already did for Sub, so it is not a regression; the freeze is ChibiOS and SITL only.
- Three Linux backends wrote into their channel arrays with no bounds check, and the new default reaches channel 31. On `RCOutput_PRU` (pxf, erleboard) `pending[MAX_PWMS]` aliases the `corked` flag, so writing zero to channel 12 uncorked the backend and the following `push()` became a no-op, leaving all twelve real channels untouched. The first commit adds the missing `ch >=` check to `RCOutput_PRU`, `RCOutput_AioPRU_PB2` and `RCOutput_AeroIO`, which also closes the pre-existing overrun of channels 12-15 that `SRV_Channels::output_ch_all()` already causes on those boards.
- If a BLHeli passthrough session is active, `push_local()` returns early while in soft serial, so the zeroing would not reach the timers. That state is not reachable in normal flight preparation.
- The reboot that installs a firmware containing this change is still governed by the old firmware, so protection begins with the next reboot after an upgrade.
This PR fixes one of two distinct mechanisms that can deflect a servo on reboot. The other, a short pulse produced *after* the reset by the internal pull-up the STM32 applies to the JTAG pins JTDI and NJTRST, affects only boards routing a PWM output through PA15 or PB4 and is addressed separately.
This PR was AI-assisted: Claude (Claude Code) did the investigation, built the emulator instrumentation used to find and measure the fault, prepared the one-byte A/B images, analysed the logic-analyser captures, wrote the HAL output freeze and authored this description under direction; the original reboot change and all hardware testing were done by the submitter, who reviewed the diff.