Conversation
DShot only. After the positions are known every output is spun for two seconds and the user confirms the direction against the arrows or reverses it, which flips that output's bit in dshot_reversed_motors. The firmware re-sends the direction commands on its own; nothing is written to the ESC. Hidden on PWM ESCs and on firmware without the setting.
|
Raffi1202
marked this pull request as ready for review
September 23, 2026 14:16
Contributor
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoAdd DShot motor direction checks to the mixer wizard
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1. A stale reversal spins the wrong motor
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+899
to
+903
| mspHelper.setSetting('dshot_reversed_motors', wizardState.reversedMask, function() { | ||
| // Give the firmware time to notice the change and get the command frames out | ||
| wizardState.directionTimer = setTimeout(function() { | ||
| wizardState.directionTimer = null; | ||
| spinMotorForDirection(motorIndex); |
There was a problem hiding this comment.
1. A stale reversal spins the wrong motor 🐞 Bug ☼ Reliability
The Reverse handler captures directionMotor and schedules a later spin without verifying that the wizard is still checking the same motor and run. If the user advances, clicks Reverse repeatedly, or closes and quickly reopens before setSetting completes, the callback can interrupt the current spin and energize an earlier output.
Agent Prompt
## Issue description
Asynchronous reversal callbacks can outlive the motor or wizard run that created them and subsequently spin the wrong output.
## Fix Focus Areas
- tabs/mixer.js[895-905]
- tabs/mixer.js[1015-1017]
## Recommended Fix
Track a wizard-run generation and the expected direction motor when starting the setting operation. Before scheduling or starting the follow-up spin, verify that the wizard is active, the generation still matches, and `directionMotor` still equals the captured motor; also disable or serialize direction actions while a reversal is pending.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+898
to
+901
| wizardState.reversedMask ^= (1 << motorIndex); | ||
| mspHelper.setSetting('dshot_reversed_motors', wizardState.reversedMask, function() { | ||
| // Give the firmware time to notice the change and get the command frames out | ||
| wizardState.directionTimer = setTimeout(function() { |
There was a problem hiding this comment.
2. Failed reversals look successful 🐞 Bug ☼ Reliability
The Reverse handler updates reversedMask and schedules the verification spin from setSetting's callback even when the setting write fails. A transport or encoding failure therefore leaves the controller unchanged while the wizard continues without an error and bases later full-mask writes on a value that was never applied.
Agent Prompt
## Issue description
The direction wizard proceeds as though a reversal succeeded even when the setting write failed, leaving its local mask inconsistent with the flight controller.
## Fix Focus Areas
- tabs/mixer.js[895-905]
- js/msp/MSPHelper.js[3802-3809]
## Recommended Fix
Use a setting-write API that exposes success or failure, and only commit `reversedMask` and schedule the verification spin after confirmed success. On failure, restore the previous mask, keep the current motor selected, show an actionable error, and leave the motor stopped.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+999
to
+1002
| Promise.all([ | ||
| mspHelper.getSetting('motor_pwm_protocol'), | ||
| mspHelper.getSetting('dshot_reversed_motors'), | ||
| ]).then(function([protocol, reversed]) { |
There was a problem hiding this comment.
3. Slow reads skip direction checks 🐞 Bug ≡ Correctness
The modal initializes directionAvailable to false and updates it only after two asynchronous setting reads, while the position-completion path immediately branches on its current value. If all positions are selected before those reads resolve, a supported DShot controller goes directly to completion and the later result cannot enter the omitted phase.
Agent Prompt
## Issue description
Position identification can finish before asynchronous DShot capability detection, causing a supported controller to skip the direction phase.
## Fix Focus Areas
- tabs/mixer.js[808-815]
- tabs/mixer.js[995-1011]
## Recommended Fix
Store the capability lookup Promise for the current wizard run and await it before choosing between `startDirectionCheck` and `wizardComplete`. Disable Start until detection settles or show a pending state at position completion, while preserving the fallback to completion when either setting is unavailable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Configurator test build ready — commit Download build artifacts for PR #2798 Available platforms (scroll to the Artifacts section at the bottom of the run page):
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #2794 after the review on iNavFlight/inav#12011: no separate dialog, no new MSP. Needs iNavFlight/inav#12022 (
dshot_reversed_motors).What it does
The Mixer tab motor wizard (#2580) gets one more phase after every motor has been located. Each output is spun for two seconds in turn, its position button turns orange, and the wizard asks whether the motor turns the way the arrow at that position shows.
dshot_reversed_motorsvia the settings MSP, waits 300 ms for the firmware to send the direction commands, and spins the motor again so the result is visible.The phase only appears when
motor_pwm_protocolis a DShot variant and the firmware answers fordshot_reversed_motors; on PWM ESCs or older firmware the wizard ends after the positions as before.The setting lives in FC RAM until the mixer is saved, same as the motor mapping the wizard produces. The hint text says so.
Tested
node --check, locale JSON valid.