Add per-motor DShot direction configuration and bounded test pulses by Raffi1202 · Pull Request #12011 · iNavFlight/inav

Raffi1202 · 2026-09-22T16:47:24Z

Problem and behavior

Add per-motor DShot direction configuration and a bounded test pulse for an integrated Configurator wizard. Commands 7/8 and SAVE_SETTINGS (12) target only the selected motor; other outputs stay at zero during the operation. Settings live in the ESC. Completion means commands sent, not confirmed persistence.

Existing arming rules are unchanged: no new arming flag or persistent lock. Requests are rejected while armed or ordinary motor testing is active. Arming cancels the operation before the next output frame and restores normal output ownership. Ordinary motor testing works normally after the operation. Cancellation retains tokens so delayed retries cannot restart it.

The fixed DShot120 test pulse expires after 1.5 seconds independently of USB/UI. Duplicate tokens cannot repeat a save or extend/restart a pulse. SITL uses the same sequencer and reports simulation explicitly.

Integration

Companion Configurator PR: Add an integrated multirotor motor direction wizard inav-configurator#2794.
Target: maintenance-10.x. Provisional MSP2 codes 0x2235-0x2237 require maintainer agreement.
Initialized STM32/AT32 DShot only; no reversible/3D mode; RP2350 unsupported. Compatible ESC direction/save support is required and cannot be detected over DShot.
UI is restricted to multirotor/tricopter platforms. Betaflight's Motor Direction Wizard inspired the workflow; no wizard code was copied.

Validation

Strict C99 tests passed: sequence order, repetitions, timing, both directions, timer wrap, expiry, stop, cancellation and token retention.
Builds passed: SITL, SPEEDYBEEF405V4, IFLIGHT_BLITZ_ATF435.
Native Electron Configurator with INAV SITL passed wizard/individual flow, reverse/next, release stop, expiry while held and unchanged ordinary motor-test availability.
Whitespace checks passed.

Hardware test plan and limitations

Physical hardware has not been tested. With all propellers removed:

Check selected-output mapping/isolation and command timing at DShot150/300/600 with burst/non-burst DMA on STM32 and AT32.
Verify armed/ordinary-test rejection, cancellation on arming and normal output ownership. Check delayed/duplicate requests after cancellation.
Verify pulse stop on release, timeout, dialog close and USB disconnection electrically.
Power-cycle ESCs and FC; check persistence and unchanged other motors. Check ordinary motor tests and turtle mode afterward.
Check unsupported ESC firmware, disabled outputs, analog PWM, 3D and missing ESC power. A fixed low pulse may not start every motor.

See docs/development/msp/esc-direction.md for protocol details.

qodo-code-review · 2026-09-22T16:47:30Z

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

qodo-free-for-open-source-projects · 2026-09-22T16:48:46Z

PR Summary by Qodo

Add per-motor DShot direction configuration and bounded test pulses

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Adds tokenized per-motor direction writes with isolated DShot command sequencing.
• Provides fixed DShot120 test pulses with firmware-enforced expiry and arming cancellation.
• Exposes MSP2 status and controls with hardware-independent SITL coverage.

Diagram

sequenceDiagram
    actor CFG as Configurator
    participant MSP as MSP handlers
    participant PWM as DShot output
    participant SEQ as Direction sequencer
    participant ESC as Selected ESC
    participant SITL as SITL simulator
    CFG->>MSP: Direction or test request
    alt Hardware output
        MSP->>PWM: Validate and dispatch
        PWM->>SEQ: Start tokenized operation
        loop Bounded sequence
            SEQ-->>PWM: Isolated command frame
            PWM->>ESC: Selected output only
        end
        PWM-->>MSP: Phase and token status
    else Simulated output
        MSP->>SITL: Validate and dispatch
        SITL->>SEQ: Run same sequencer
        SITL-->>MSP: Simulated status
    end
    MSP-->>CFG: Capability and progress

High-Level Assessment

The following are alternative approaches to this PR:

1. Extend the existing DShot command queue

➕ Reuses established command scheduling infrastructure.
➕ Could reduce the amount of dedicated driver state.
➖ The generic queue does not model selected-output isolation or bounded throttle pulses.
➖ Zero-frame suppression, cancellation, and retained token semantics would complicate shared behavior.
➖ Changes could increase regression risk for turtle mode and existing DShot commands.

2. Sequence commands from the Configurator

➕ Keeps firmware state smaller.
➕ Allows the UI to directly control workflow pacing.
➖ USB loss or UI stalls could leave testing behavior unsafe or incomplete.
➖ Cannot reliably enforce frame-level timing and output isolation.
➖ Duplicate requests and retries become harder to make idempotent.

Recommendation: Keep the dedicated firmware sequencer. Frame-level timing, selected-motor isolation, token idempotency, arming cancellation, and pulse expiry belong beside output generation rather than in the UI or generic command queue. Before merging, confirm the provisional MSP2 allocations and complete the documented physical hardware validation.

Files changed (10) +481 / -0

Enhancement (6) +244 / -0

dshot_direction.hAdd the tokenized DShot direction state machine +104/-0

Add the tokenized DShot direction state machine

• Introduces the shared state machine for zeroing outputs, repeating direction and save commands, enforcing timing gaps, and retaining request tokens. It also implements fixed DShot120 test pulses with a 1.5-second deadline and cancellation support.

src/main/drivers/dshot_direction.h

pwm_output.cIntegrate direction sequencing with physical DShot outputs +68/-0

Integrate direction sequencing with physical DShot outputs

• Adds capability checks and APIs for starting, stopping, and inspecting direction operations. Active operations isolate the selected motor, suppress competing DShot commands, expire test pulses, and yield immediately when arming occurs.

src/main/drivers/pwm_output.c

pwm_output.hExpose DShot direction configuration APIs +12/-0

Expose DShot direction configuration APIs

• Defines the feature guard and public status, capability, direction, test, and SITL update interfaces. RP2350 is excluded while SITL explicitly enables the shared interface.

src/main/drivers/pwm_output.h

fc_msp.cAdd MSP2 direction, test, and status handlers +53/-0

Add MSP2 direction, test, and status handlers

• Exposes versioned operation status and accepts tokenized direction or bounded-test requests. It rejects ordinary motor testing and disruptive reboot, persistence, reset, or passthrough commands while the sequencer owns outputs.

src/main/fc/fc_msp.c

mixer.cAdvance DShot direction operations in SITL +2/-0

Advance DShot direction operations in SITL
• Invokes the SITL direction update from the motor write path so simulated operations progress through the normal output cadence.
src/main/flight/mixer.c

msp_protocol_v2_inav.hReserve provisional MSP2 ESC direction commands +5/-0

Reserve provisional MSP2 ESC direction commands
• Defines experimental command identifiers for status, direction changes, and bounded test pulses. The comments flag that allocation requires maintainer coordination before merge.
src/main/msp/msp_protocol_v2_inav.h

Tests (2) +89 / -0

CMakeLists.txtRegister the standalone DShot direction test +6/-0

Register the standalone DShot direction test
• Builds the strict assertion-based sequencer test and adds it to both CTest and the aggregate check target.
src/test/CMakeLists.txt

dshot_direction_test.cTest direction timing, pulse bounds, and cancellation +83/-0

Test direction timing, pulse bounds, and cancellation

• Covers both directions, command repetition and ordering, minimum timing gaps, timer wraparound, and completion. It also verifies pulse expiry, explicit stop behavior, mutual exclusion, cancellation, and duplicate-token retention.

src/test/dshot_direction_test.c

Documentation (1) +92 / -0

esc-direction.mdDocument the ESC direction protocol and safety model +92/-0

Document the ESC direction protocol and safety model

• Documents platform scope, provisional MSP2 payloads, DShot timing, ESC persistence limitations, and unchanged arming behavior. It also records completed validation and the required propeller-free hardware test plan.

docs/development/msp/esc-direction.md

Other (1) +56 / -0

target.cImplement simulated ESC direction sequencing +56/-0

Implement simulated ESC direction sequencing

• Provides SITL implementations of the hardware-facing direction APIs using the shared sequencer. It applies the same validation, token, timeout, and arming-cancellation behavior while explicitly logging simulated commands.

src/main/target/SITL/target.c

qodo-free-for-open-source-projects · 2026-09-22T16:51:13Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~Turtle mode runs motors forward~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

sendDShotCommand() drops every command while directionConfig is busy, rather than preserving the
turtle-mode reverse command. A direction operation may start while disarmed, and turtle arming then
sets the armed and turtle flags immediately, so the next scheduler pass cancels the operation while
the ESCs never receive their required reverse command.

Code

src/main/drivers/pwm_output.c[R590-592]

+    if (dshotDirectionBusy(&directionConfig)) {
+        return;
+    }

Evidence
The new early return drops the reverse command needed by turtle mode. Turtle arming does not check
whether that command was accepted and immediately enables both armed and turtle states; the newly
added scheduler logic subsequently cancels the active direction sequence once armed.
src/main/drivers/pwm_output.c[560-573]
src/main/drivers/pwm_output.c[589-594]
src/main/drivers/pwm_output.c[637-642]
src/main/fc/fc_core.c[606-616]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
DShot commands are discarded while the direction sequencer is active. Turtle-mode arming depends on queuing the ESC reverse command before enabling turtle mode, so dropping that command lets the craft enter turtle mode with its ESC direction unchanged.
## Fix Focus Areas
- src/main/drivers/pwm_output.c[589-594]
- src/main/fc/fc_core.c[606-616]
## Recommended Fix
Do not silently discard DShot commands while a direction operation is active. Queue the command so that, after the armed-state cancellation releases sequencer ownership, the normal DShot command scheduler sends its required repetitions; alternatively explicitly cancel and enqueue before accepting turtle-mode arming. Preserve the existing exclusion that prevents a newly started direction operation from interrupting already queued commands.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Stopped tests can pulse once more~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

pwmDshotDirectionTest clears testActive without clearing the test value cached in
motors[].value, and arming cancellation has the same stale-cache behavior. If
pwmCompleteMotorUpdate runs before the mixer writes fresh values, it packages and transmits the
selected motor's previous DShot120 value after an explicit stop or arming cancellation.

Code

src/main/drivers/pwm_output.c[R578-580]

+    if (run == 0) {
+        directionConfig.testActive = false;
+        return true;

Evidence
The digital writer stores values for later transmission, the active-test path places 120 in that
cache, and cancellation only clears state. The subsequent no-command path can return successfully
without overwriting the cache, after which the update routine encodes and transmits it.
src/main/drivers/pwm_output.c[508-510]
src/main/drivers/pwm_output.c[578-580]
src/main/drivers/pwm_output.c[621-634]
src/main/drivers/pwm_output.c[637-657]
src/main/drivers/pwm_output.c[686-700]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Stopping or cancelling a direction test clears only the state flag, leaving DShot120 cached for transmission in the selected motor output.
## Fix Focus Areas
- src/main/drivers/pwm_output.c[578-580]
- src/main/drivers/pwm_output.c[621-634]
## Recommended Fix
On every explicit or arming-triggered cancellation, clear the cached test outputs or retain direction-output ownership long enough to emit a zero frame before returning to the normal command path. Ensure the first frame after cancellation cannot reuse the previous DShot120 value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~SITL cannot compile this feature~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

target.c declares dshotDirection_t and invokes its helpers without including
drivers/pwm_output.h, while its new calls to getMotorCount and areMotorsRunning also lack
flight/mixer.h. Compiling the SITL translation unit therefore encounters an unknown direction type
and undeclared APIs as soon as the new implementation is enabled.

Code

src/main/target/SITL/target.c[R588-590]

+// Exercise the same command sequencer through real MSP in the built-in demo.
+// SITL has no physical ESCs; it explicitly advertises simulated output.
+static dshotDirection_t sitlDirectionConfig;

Evidence
The SITL source's include list contains neither required header, while the direction type is exposed
through pwm_output.h and both motor APIs are declared by mixer.h. The newly added declarations
and calls consequently have no visible definitions in this translation unit.
src/main/target/SITL/target.c[25-53]
src/main/target/SITL/target.c[588-604]
src/main/target/SITL/target.c[617-636]
src/main/drivers/pwm_output.h[67-76]
src/main/flight/mixer.h[148-167]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new SITL implementation uses DShot direction and mixer APIs without including the headers that declare them.
## Fix Focus Areas
- src/main/target/SITL/target.c[45-53]
- src/main/target/SITL/target.c[588-636]
## Recommended Fix
Include `drivers/pwm_output.h` for the direction state and helper declarations and `flight/mixer.h` for `getMotorCount` and `areMotorsRunning`, then verify the SITL target compiles without implicit declarations.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~A new motor test fails after expiry~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

dshotDirectionTestBegin() checks testActive before applying the elapsed-time deadline, while
only dshotDirectionTestFrame() clears an expired pulse. A fresh-token request arriving after 1.5
seconds but before the next output update is rejected as conflicting, and status continues to expose
the pulse as active during that window.

Code

src/main/drivers/dshot_direction.h[R73-75]

+    if (token == 0 || (s->phase > 0 && s->phase < 6)) return false;
+    if (token == s->testToken) return motor == s->testMotor;
+    if (s->testActive) return false;

Evidence
The begin path rejects any request while testActive remains set, but elapsed-time expiration
occurs only when an output frame is generated. Both hardware and SITL MSP handlers invoke the begin
path directly, so an MSP request can observe stale active state between the deadline and the next
output update.
src/main/drivers/dshot_direction.h[71-85]
src/main/drivers/dshot_direction.h[91-94]
src/main/drivers/pwm_output.c[577-587]
src/main/target/SITL/target.c[619-637]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`dshotDirectionTestBegin()` can reject a new test after the previous pulse's 1.5-second deadline because expiration is only processed by frame generation.
## Fix Focus Areas
- src/main/drivers/dshot_direction.h[71-80]
## Recommended Fix
At the start of `dshotDirectionTestBegin()`, clear `testActive` when the unsigned elapsed time from `testStartedUs` has reached 1,500,000 microseconds. Perform duplicate-token and active-state checks afterward so fresh tokens are accepted once the prior pulse has expired without allowing duplicate tokens to restart it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Simulator tests never pulse a motor~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

sitlDshotDirectionUpdate() advances the test and direction state machines but discards their
resulting output values, only printing positive configuration commands. SITL's simulators read the
mixer's motor[] values directly, so a direction operation leaves ordinary simulated outputs in
place and a DShot120 test pulse produces no simulated motor output or selected-motor isolation.

Code

src/main/target/SITL/target.c[R635-638]

+    dshotDirectionTestFrame(&sitlDirectionConfig, micros());
+    const int16_t command = dshotDirectionFrame(&sitlDirectionConfig, micros());
+    if (command > 0) {
+        fprintf(stderr, "[ESC DEMO] motor=%u command=%d (simulated, no hardware)\n", sitlDirectionConfig.motor + 1, command);

Evidence
The added SITL code invokes both frame helpers but does not store either frame result. Unlike
hardware, where the new sequencer overwrites every output with the selected value or zero, SITL
deliberately has no PWM layer and its simulation backends consume motor[] directly.
src/main/target/SITL/target.c[630-640]
src/main/drivers/pwm_output.c[619-630]
src/main/fc/fc_init.c[361-367]
src/main/target/SITL/sim/realFlight.c[232-238]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The SITL hook advances the shared DShot direction sequencer but does not publish the resulting isolated motor output to the simulator. As a result, SITL does not model either the selected test pulse or the required zeroing of all other motors.
## Fix Focus Areas
- src/main/target/SITL/target.c[630-640]
- src/main/flight/mixer.c[520-601]
## Recommended Fix
In the SITL update hook, translate the sequencer's selected DShot test output to the simulator motor-value range and set every unselected `motor[]` entry to its stopped value while the sequence or pulse owns outputs. Preserve ordinary mixer values once the operation completes or is cancelled, and retain the existing simulated-command logging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

Review mode: 🧠 Deep: This is a broad, safety-sensitive runtime feature spanning DShot sequencing, motor-output ownership, arming cancellation, MSP protocol handling, SITL behavior, and multiple independent code paths, creating a high density of subtle defects that benefits from redundant review passes.

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 5018715 🧠 Deep

Results up to commit N/A

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Turtle mode runs motors forward~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

sendDShotCommand() drops every command while directionConfig is busy, rather than preserving the
turtle-mode reverse command. A direction operation may start while disarmed, and turtle arming then
sets the armed and turtle flags immediately, so the next scheduler pass cancels the operation while
the ESCs never receive their required reverse command.

Code

src/main/drivers/pwm_output.c[R590-592]

+    if (dshotDirectionBusy(&directionConfig)) {
+        return;
+    }

Evidence
The new early return drops the reverse command needed by turtle mode. Turtle arming does not check
whether that command was accepted and immediately enables both armed and turtle states; the newly
added scheduler logic subsequently cancels the active direction sequence once armed.
src/main/drivers/pwm_output.c[560-573]
src/main/drivers/pwm_output.c[589-594]
src/main/drivers/pwm_output.c[637-642]
src/main/fc/fc_core.c[606-616]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
DShot commands are discarded while the direction sequencer is active. Turtle-mode arming depends on queuing the ESC reverse command before enabling turtle mode, so dropping that command lets the craft enter turtle mode with its ESC direction unchanged.
## Fix Focus Areas
- src/main/drivers/pwm_output.c[589-594]
- src/main/fc/fc_core.c[606-616]
## Recommended Fix
Do not silently discard DShot commands while a direction operation is active. Queue the command so that, after the armed-state cancellation releases sequencer ownership, the normal DShot command scheduler sends its required repetitions; alternatively explicitly cancel and enqueue before accepting turtle-mode arming. Preserve the existing exclusion that prevents a newly started direction operation from interrupting already queued commands.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Stopped tests can pulse once more~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

pwmDshotDirectionTest clears testActive without clearing the test value cached in
motors[].value, and arming cancellation has the same stale-cache behavior. If
pwmCompleteMotorUpdate runs before the mixer writes fresh values, it packages and transmits the
selected motor's previous DShot120 value after an explicit stop or arming cancellation.

Code

src/main/drivers/pwm_output.c[R578-580]

+    if (run == 0) {
+        directionConfig.testActive = false;
+        return true;

Evidence
The digital writer stores values for later transmission, the active-test path places 120 in that
cache, and cancellation only clears state. The subsequent no-command path can return successfully
without overwriting the cache, after which the update routine encodes and transmits it.
src/main/drivers/pwm_output.c[508-510]
src/main/drivers/pwm_output.c[578-580]
src/main/drivers/pwm_output.c[621-634]
src/main/drivers/pwm_output.c[637-657]
src/main/drivers/pwm_output.c[686-700]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Stopping or cancelling a direction test clears only the state flag, leaving DShot120 cached for transmission in the selected motor output.
## Fix Focus Areas
- src/main/drivers/pwm_output.c[578-580]
- src/main/drivers/pwm_output.c[621-634]
## Recommended Fix
On every explicit or arming-triggered cancellation, clear the cached test outputs or retain direction-output ownership long enough to emit a zero frame before returning to the normal command path. Ensure the first frame after cancellation cannot reuse the previous DShot120 value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~SITL cannot compile this feature~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

target.c declares dshotDirection_t and invokes its helpers without including
drivers/pwm_output.h, while its new calls to getMotorCount and areMotorsRunning also lack
flight/mixer.h. Compiling the SITL translation unit therefore encounters an unknown direction type
and undeclared APIs as soon as the new implementation is enabled.

Code

src/main/target/SITL/target.c[R588-590]

+// Exercise the same command sequencer through real MSP in the built-in demo.
+// SITL has no physical ESCs; it explicitly advertises simulated output.
+static dshotDirection_t sitlDirectionConfig;

Evidence
The SITL source's include list contains neither required header, while the direction type is exposed
through pwm_output.h and both motor APIs are declared by mixer.h. The newly added declarations
and calls consequently have no visible definitions in this translation unit.
src/main/target/SITL/target.c[25-53]
src/main/target/SITL/target.c[588-604]
src/main/target/SITL/target.c[617-636]
src/main/drivers/pwm_output.h[67-76]
src/main/flight/mixer.h[148-167]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new SITL implementation uses DShot direction and mixer APIs without including the headers that declare them.
## Fix Focus Areas
- src/main/target/SITL/target.c[45-53]
- src/main/target/SITL/target.c[588-636]
## Recommended Fix
Include `drivers/pwm_output.h` for the direction state and helper declarations and `flight/mixer.h` for `getMotorCount` and `areMotorsRunning`, then verify the SITL target compiles without implicit declarations.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~A new motor test fails after expiry~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

dshotDirectionTestBegin() checks testActive before applying the elapsed-time deadline, while
only dshotDirectionTestFrame() clears an expired pulse. A fresh-token request arriving after 1.5
seconds but before the next output update is rejected as conflicting, and status continues to expose
the pulse as active during that window.

Code

src/main/drivers/dshot_direction.h[R73-75]

+    if (token == 0 || (s->phase > 0 && s->phase < 6)) return false;
+    if (token == s->testToken) return motor == s->testMotor;
+    if (s->testActive) return false;

Evidence
The begin path rejects any request while testActive remains set, but elapsed-time expiration
occurs only when an output frame is generated. Both hardware and SITL MSP handlers invoke the begin
path directly, so an MSP request can observe stale active state between the deadline and the next
output update.
src/main/drivers/dshot_direction.h[71-85]
src/main/drivers/dshot_direction.h[91-94]
src/main/drivers/pwm_output.c[577-587]
src/main/target/SITL/target.c[619-637]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`dshotDirectionTestBegin()` can reject a new test after the previous pulse's 1.5-second deadline because expiration is only processed by frame generation.
## Fix Focus Areas
- src/main/drivers/dshot_direction.h[71-80]
## Recommended Fix
At the start of `dshotDirectionTestBegin()`, clear `testActive` when the unsigned elapsed time from `testStartedUs` has reached 1,500,000 microseconds. Perform duplicate-token and active-state checks afterward so fresh tokens are accepted once the prior pulse has expired without allowing duplicate tokens to restart it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Simulator tests never pulse a motor~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

sitlDshotDirectionUpdate() advances the test and direction state machines but discards their
resulting output values, only printing positive configuration commands. SITL's simulators read the
mixer's motor[] values directly, so a direction operation leaves ordinary simulated outputs in
place and a DShot120 test pulse produces no simulated motor output or selected-motor isolation.

Code

src/main/target/SITL/target.c[R635-638]

+    dshotDirectionTestFrame(&sitlDirectionConfig, micros());
+    const int16_t command = dshotDirectionFrame(&sitlDirectionConfig, micros());
+    if (command > 0) {
+        fprintf(stderr, "[ESC DEMO] motor=%u command=%d (simulated, no hardware)\n", sitlDirectionConfig.motor + 1, command);

Evidence
The added SITL code invokes both frame helpers but does not store either frame result. Unlike
hardware, where the new sequencer overwrites every output with the selected value or zero, SITL
deliberately has no PWM layer and its simulation backends consume motor[] directly.
src/main/target/SITL/target.c[630-640]
src/main/drivers/pwm_output.c[619-630]
src/main/fc/fc_init.c[361-367]
src/main/target/SITL/sim/realFlight.c[232-238]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The SITL hook advances the shared DShot direction sequencer but does not publish the resulting isolated motor output to the simulator. As a result, SITL does not model either the selected test pulse or the required zeroing of all other motors.
## Fix Focus Areas
- src/main/target/SITL/target.c[630-640]
- src/main/flight/mixer.c[520-601]
## Recommended Fix
In the SITL update hook, translate the sequencer's selected DShot test output to the simulator motor-value range and set every unselected `motor[]` entry to its stopped value while the sequence or pulse owns outputs. Preserve ordinary mixer values once the operation completes or is cancelled, and retain the existing simulated-command logging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Raffi1202 · 2026-09-22T20:51:54Z

/agentic_review

github-actions · 2026-09-22T20:55:51Z

RAM / Flash usage vs. base commit 86a0441 — commit 5018715

Target	Flash Δ	RAM Δ
MATEKF405	+1416 B (+0.20%)	CCM: ±0 B (±0.00%) RAM: +48 B (+0.04%)
MATEKF722	+1304 B (+0.27%)	ITCM_RAM: -240 B (-1.93%) RAM: +40 B (+0.05%) TCM: ±0 B (±0.00%)
MATEKF765	+1768 B (+0.23%)	DTCM_RAM: ±0 B (±0.00%) SRAM1: +24 B (+0.02%)
MATEKH743	+1584 B (+0.20%)	D2_RAM: ±0 B (±0.00%) DTCM_RAM: ±0 B (±0.00%) ITCM_RAM: -384 B (-2.35%) RAM: -32 B (-0.02%)

See RAM/flash optimization guide for techniques to reduce usage.

qodo-free-for-open-source-projects · 2026-09-22T20:55:58Z

Code review by qodo was updated up to the latest commit 2af2025

github-actions · 2026-09-22T20:56:17Z

Test firmware build ready — commit 5018715

Download firmware for PR #12011

250 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.

Raffi1202 · 2026-09-22T22:32:16Z

/agentic_review

qodo-free-for-open-source-projects · 2026-09-22T22:36:05Z

Code review by qodo was updated up to the latest commit e57053d

Raffi1202 · 2026-09-23T06:56:23Z

/agentic_review

qodo-free-for-open-source-projects · 2026-09-23T07:00:13Z

Code review by qodo was updated up to the latest commit 5018715

b14ckyy · 2026-09-23T11:09:35Z

Hello @Raffi1202 and thank you for that PR. Its a very nice QoL feature for copter pilots. I would like to address a few things though.

Critical parts first

Feature freeze: INAV 10 is feature locked. But as I can see, there is no change in the Config scheme so its technically possible to consider it for 10.1. But this will need a version/feature gating in the INAV configurator so Configurator 10.1 will not show the Interface for a 10.0 FC. Compatibility has to be considered both ways. Alternatively rebase the PR to 11.x but you will need to followw up if conflicts happen.

Setting dshot config: that is the part that I do not like. Reading, writing and saving settings to an ESC should not be done by a 3rd party (that is INAV in that case). There are multiple different ESC firmware variants out there where we do not know their capabilities of each version. Writing the config without knowing what's there is a no-go.

The alternative: the reverse flag in the actual dshot init (DShot-Cmd 20/21 (SPIN_DIRECTION_NORMAL/REVERSED)). If the ESC can be reversed it should be able to reverse in runtime without config change. This would need a simple bitmask that defines what Motor ESC signal has to be inverted. Its one new Config parameter but adding config would be fine for a .1 release since it does not break existing.

Optimization

Condensse into a single MSP message: 0x2236 would become obsolete. The other two can be condensed to 1. Send it with a 1 byte payload. first bit defines what INAV should do, respond with the inversion bitmask or send the test signal to an ESC. the remaining 7 bit can address the Motor you want to test spin. Or make it 2 byte if you need additional information to send lke a throttloe value or time.

With these changes, the same thing can be achieved and the bitmask then only needs to be wired to the dshot control loop to define if each motor gets a normal or inverted direction signal with each dshot message. This can be defined at boot time. That should significantly reduce the flash demand for the same feature.

Some implications to consider

Since 20/21 are not persistent, the FC must reliably re-apply them whenever the ESCs power up (e.g. battery connected after USB, ESC brownout) and before arming is allowed. A motor spinning the wrong way on takeoff is a flip, so this needs to be robust.
Turtle mode currently broadcasts 20/21 to all motors. With a bitmask it needs to invert relative to the configured direction per motor, and restore the bitmask state on disarm instead of broadcasting "normal". The DShot command queue would need per-motor addressing for that.
The bitmask should be uint16 (up to 12 motors) and can be read/written via the standard settings MSP, so only the bounded test pulse needs its own message.

Happy to discuss before you start refactoring.

Raffi1202 · 2026-09-23T12:57:10Z

Agreed on all three points. Proposed rework, before I touch code:

Branch. Retarget both PRs to maintenance-11.x. The branch currently contains nothing that maintenance-10.x does not, and the PR merges clean there (checked with git merge-tree). That avoids version gating in the Configurator for a 10.1 backport; if you want it in 10.1 anyway, say so and I add the gate.

Setting instead of ESC config. Drop DShot 7/8/12 and all three MSP codes (0x2235-0x2237). New uint16 setting, bit N = motor N gets SPIN_DIRECTION_REVERSED (21), otherwise SPIN_DIRECTION_NORMAL (20). Read/written via MSP2_COMMON_SETTING, so the Configurator needs no new MSP handler. Placed in motorConfig unless you prefer a separate PG.

Per-motor addressing. The command queue in pwm_output.c writes one command to every motor (motors[i].value = currentExecutingCommand.cmd). I extend the queue entry to carry a per-motor value so one entry can send 20 to some outputs and 21 to others in the same frame. Repeats and inter-command delay stay as they are (10 repeats, 1 ms, 10 ms post delay). Beacon and the existing all-motor commands keep the old path.

When it is sent. 20/21 are volatile, so the FC has to re-apply them:

on every arm, before the first throttle frame; arming waits until the repeats are out (~20 ms, same idea as the beacon guard delay);
periodically while disarmed (every ~2 s), because without ESC telemetry the FC cannot see an ESC boot - battery plugged after USB, brownout on the bench;
immediately when the setting changes while disarmed, so the wizard sees the new direction on the next test spin.

An ESC that resets in flight falls back to its stored direction; it also will not re-arm while throttle is non-zero, so that motor is lost either way. No runtime scheme fixes that, and neither did the persistent variant - I will state it in the docs.

Turtle. tryArm() sends mask ^ 0xFFFF per motor instead of broadcasting 21; disarm() re-sends the mask instead of broadcasting 20. Behaviour for an all-zero mask is identical to today.

Configurator. No separate dialog. The mixer motor wizard (#2580) already spins each motor via MSP_SET_MOTOR; the direction check becomes a step in that flow ("spinning correctly / reverse"), toggling the bit in the setting. No test-pulse message needed, so no new MSP at all. I will close iNavFlight/inav-configurator#2794 and open the replacement against the wizard.

Flash. Removes the sequencer, the SITL simulation, the state machine test and the MSP handlers; adds one setting, per-motor queue entries and the arm/disarm hook. Net should be well under the current +900 lines.

Open question: periodic re-send while disarmed, or a hard arming block until the direction has been sent after battery detection (vbat present + ESC boot delay)? The re-send is simpler and covers the same cases without a new arming flag.

b14ckyy · 2026-09-23T13:11:40Z

This sounds good to me. Resending is fine It will also make sure that the setting is persistent if a EEPROM save interrupts dshot that was a topic in the past.

The Motor loss on brownout is acceptable in my opinion. Even right now when an ESC resets mid flight it would take a few seconds to come back and in 99% of cases the copter reaches the ground before that happens.

From my perspective, that's a solid plan and I am looking forward to the update.

Raffi1202 · 2026-09-23T13:47:49Z

Superseded by #12022 (maintenance-11.x, bitmask + DShot 20/21 as discussed above).

Add per-motor DShot direction configuration and bounded test pulses

2ac7813

Raffi1202 mentioned this pull request Sep 22, 2026

Add an integrated multirotor motor direction wizard iNavFlight/inav-configurator#2794

Closed

qodo-free-for-open-source-projects Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread src/main/drivers/pwm_output.c

Comment thread src/main/target/SITL/target.c

Fix motor direction MSP catalogue and F7 ITCM overflow

2af2025

qodo-free-for-open-source-projects Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread src/main/drivers/pwm_output.c Outdated

Comment thread src/main/target/SITL/target.c Outdated

Raffi1202 added 2 commits September 22, 2026 23:20

Keep direction test frames separate from normal motor outputs

49097aa

Keep DShot command sequencing out of F7 instruction RAM

e57053d

qodo-free-for-open-source-projects Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread src/main/drivers/dshot_direction.h

Expire DShot tests before handling requests and status reads

5018715

Raffi1202 mentioned this pull request Sep 23, 2026

Add dshot_reversed_motors: per-motor spin direction via DShot commands 20/21 #12022

Open

Raffi1202 closed this Sep 23, 2026

Raffi1202 mentioned this pull request Sep 23, 2026

Mixer wizard: check each motor's spin direction after locating it iNavFlight/inav-configurator#2798

Open

Conversation

Raffi1202 commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem and behavior

Integration

Validation

Hardware test plan and limitations

Uh oh!

qodo-code-review Bot commented Sep 22, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 22, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Raffi1202 commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Raffi1202 commented Sep 22, 2026

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 22, 2026

Uh oh!

Raffi1202 commented Sep 23, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 23, 2026

Uh oh!

b14ckyy commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Critical parts first

Optimization

Some implications to consider

Uh oh!

Raffi1202 commented Sep 23, 2026

Uh oh!

b14ckyy commented Sep 23, 2026

Uh oh!

Raffi1202 commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Raffi1202 commented Sep 22, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Sep 22, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading

b14ckyy commented Sep 23, 2026 •

edited

Loading