Copter: clear takeoff spool-up block on boards without ESC telemetry

[yuiseki]

Summary

Copter cannot leave GROUND_IDLE when built without ESC telemetry (HAL_WITH_ESC_TELEM == 0), which currently includes every ESP32 board in this tree.

It arms, passes prearm, spins the motors, accepts full throttle, and then declines the optional advanced feature known as taking off.

This has been present since 2022.

Classification & Testing

• This pull request was carefully written by a human
• Checked by a human programmer
• Non-functional change
• No-binary change
• Infrastructure change
• Automated test(s) verify changes
• Tested manually
• Tested on hardware
• Logs attached
• Logs available on request

Tested before/after in feature-disabled SITL and on a real M5Stack StampFly. Default SITL disassembly is unchanged.

Problem

First, thank you for ArduPilot.

Watching EKF3, compass calibration and MAVLink over WiFi all come up on a 27.6-gram toy quadcopter made for a genuinely good weekend, right up until the part described above.

AP_MotorsMulticopter::output_logic() sets the spool-up block when idle spin-up completes:

if (!_spin_up_complete) {
    _spin_up_complete = true;
    set_spoolup_block(true);
}

For Copter, outside THROW and TURTLE, Copter::takeoff_check() is what clears it.

But the whole function body is currently inside:

#if HAL_WITH_ESC_TELEM && FRAME_CONFIG != HELI_FRAME

On a board without ESC telemetry, takeoff_check() therefore does nothing.

Nothing clears the block.

The aircraft stays in GROUND_IDLE at MOT_SPIN_MIN forever, and because the warnings are inside the same #if, it says nothing about why.

HAL_WITH_ESC_TELEM is (NUM_SERVO_CHANNELS > 0) && (HAL_SUPPORT_RCOUT_SERIAL || HAL_MAX_CAN_PROTOCOL_DRIVERS).
HAL_SUPPORT_RCOUT_SERIAL is ChibiOS only and no ESP32 hwdef declares CAN, so this is 0 for all 11 boards under libraries/AP_HAL_ESP32/hwdef/.

I confirmed it on esp32s3m5stampfly by putting an #error in each branch of the #if and seeing which one fired, rather than trusting my own reading of the macro.

This is not a recent regression. set_spoolup_block(true) arrived in commit f3dc805 on 2022-09-06, and the code before commit 0deeede had the same hole: the block was set during ramp-up, and with no ESC telemetry nothing ever set it false.

It survived three and a half years because essentially every real flight controller has CAN or serial RCOut, so the macro is 1 and the clearing code exists.

This is particularly convincing on ESP32 because "my cheap ESP32 flight controller doesn't work" sounds like a perfectly reasonable root cause.

In this case, the cheap ESP32 was innocent.

I spent most of a weekend following the throttle path.

RC3_*, MANUAL_CONTROL, failsafes, interlock, motor parameters, upstream master, all looked plausible.

Eventually I instrumented read_radio():

ci=1000  tz=0  armed=1  mode=STABILIZE
-> spool=GROUND_IDLE, throttle_thrust_max=0.00

The throttle input was correct.

At that point the throttle path had an alibi.

The state machine was one grep away.

Only afterwards, reading the history to write this up, did I find that the #if had already been seen. #33601 touched exactly this line and was closed.

#32404 mentioned in passing that we skip the CPU checks without ESC telemetry, and that it had been that way since 2022.

Described as a missing CPU load check it is a small thing, easy to leave for later.

The part that was not followed through is that takeoff_check() is also the only code that clears the block, so compiling it out does not skip a check. It grounds the aircraft.

Fix

Narrow the outer #if so that only the ESC-telemetry-specific checks remain conditional.

The CPU-load check does not depend on ESC telemetry and now runs everywhere, which is what #32404 wanted.

I considered removing the remaining HAL_WITH_ESC_TELEM guards entirely, but g2.takeoff_rpm_min/max are themselves conditional.

This is therefore the smallest change I could make that fixes Copter without widening the scope.

With ESC telemetry enabled, the generated Copter::takeoff_check() is byte-for-byte unchanged.

If you would rather have those parameters made unconditional, or the whole thing restructured through AP_Motors, say which shape you want and I will redo it.

Why no automated test

The failure is selected at compile time.

Default SITL behaves identically before and after, so the existing runtime tests cannot catch it. CI does build feature-disabled configurations, but does not run them.

So the current CI successfully verifies that ESC telemetry is disabled.

Unfortunately, the aircraft is also disabled.

Catching this class automatically would require running a feature-disabled SITL configuration. I can add that separately if wanted.

Scope

Hardware testing was on one M5Stack StampFly v1.1 using esp32s3m5stampfly, in STABILIZE and ALT_HOLD.

I do not have hardware with ESC telemetry. That path is covered by builds and the disassembly comparison, not by flight.

Side note, not part of this PR

--disable-ESC_TELEM alone currently does not build. AP_EXTENDED_ESC_TELEM_ENABLED defaults to HAL_ENABLE_DRONECAN_DRIVERS and trips the #error in AP_ESC_Telem_config.h, and AP_PiccoloCAN_ESC.cpp calls update_rpm() while PICCOLOCAN in build_options.py lists only DroneCAN as its dependency.

I can send that separately rather than turning a small takeoff fix into an expedition.

AI assistance disclosure

Per AGENTS.md, this contribution was AI assisted.

I used Claude Code to inspect the spool-up state machine, help write instrumentation and the SITL harness, and draft this description.

Its first several root-cause hypotheses were wrong.

Mine were too.

The useful part was eventually asking the aircraft what its variables actually contained.

Every number below comes from a real run.

Raw SITL output, MAVLink logs and both disassemblies are available on request.

I understand the change and will answer review comments myself.

Evidence

This reproduces without hardware. HAL_WITH_ESC_TELEM can be turned off through the existing build option in Tools/scripts/build_options.py:

./waf configure --board sitl --disable-ESC_TELEM --disable-ESC_EXTENDED_TELM --disable-PICCOLOCAN
./waf copter
build/sitl/bin/arducopter --model quad --speedup 5 --defaults Tools/autotest/default_params/copter.parm --home 35.6,139.7,0,0

Then arm in GUIDED and send MAV_CMD_NAV_TAKEOFF for 10 m.

configuration	before	after
SITL, ESC telemetry disabled	motors stuck at 1150, altitude 0.00 m	motors 1587, reaches 10.00 m
StampFly	GROUND_IDLE, motors capped at 1150	reaches THROTTLE_UNLIMITED, flies
SITL, ESC telemetry enabled	builds	builds, disassembly identical
SITL heli	builds	builds

Before is master at commit e0652af, unmodified. After is the same tree with this patch and nothing else.

1150 is MOT_SPIN_MIN, held for the full 24 seconds I watched it.

For the SITL takeoff command, the requested altitude was 10 m.

Before the patch, the achieved altitude was 0.00 m.

Or, in hardware units: table.