fix(dshot): resynchronise serial ESC telemetry to its request - #28223
Merged
dakejahl merged 1 commit intoAug 14, 2026
Merged
dakejahl merged 1 commit into
dakejahl merged 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
A serial telemetry response is matched to a motor by which request was outstanding when it was decoded, not by anything in the frame itself. Nothing discarded stale bytes between requests, so a response that arrived after its own request had timed out stayed in the RX FIFO and was decoded as the *next* motor's response — an intact frame with a good CRC, simply attributed to the wrong ESC. That offset is self-sustaining: from then on every request finds a complete frame already buffered, decodes it immediately, and so never times out again to fall back into step. All ESC readings stay shifted by one motor until something else resets the stream. It is easy to miss on a multirotor, where the motors report similar voltage, current and temperature — but RPM is per-motor, so any test that spins one motor at a time reads that motor's RPM as zero while its voltage and temperature look perfectly healthy. Flush the RX buffer when a request is started, so a response can only ever be attributed to the motor it was asked of. Also restore the pre-rework 30 ms response timeout. 5 ms is short enough that a busy ESC regularly misses it, which is what produced the late responses in the first place; giving up early costs a whole round-robin pass, not a single frame. (cherry picked from commit a6c0d88)
Copy link
Copy Markdown
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: -8 byte (-0 %)] FILE SIZE VM SIZE
-------------- --------------
+0.0% +54 [ = ] 0 .debug_abbrev
+0.0% +63 [ = ] 0 .debug_info
-0.0% -5 [ = ] 0 .debug_line
[DEL] -4 [ = ] 0 [Unmapped]
-0.0% -1 [ = ] 0 [section .debug_line]
-0.0% -3 [ = ] 0 .debug_rnglists
+0.0% +27 [ = ] 0 .debug_str
+0.1% +8 [ = ] 0 [Unmapped]
-0.0% -8 -0.0% -8 .text
+20% +4 +20% +4 DShotTelemetry::startTelemetryRequest()
+3.6% +4 +3.6% +4 GPSHelper::storeUpdateRates()
+44% +4 +44% +4 g_nullstring
-4.5% -4 -4.5% -4 FlightTask
-0.0% -4 -0.0% -4 g_cromfs_image
-0.0% -12 -0.0% -12 [section .text]
+0.0% +136 -0.0% -8 TOTAL
FILE SIZE VM SIZE
-------------- --------------
+0.0% +54 [ = ] 0 .debug_abbrev
+0.0% +63 [ = ] 0 .debug_info
-0.0% -5 [ = ] 0 .debug_line
[DEL] -4 [ = ] 0 [Unmapped]
-0.0% -1 [ = ] 0 [section .debug_line]
+0.0% +1 [ = ] 0 .debug_rnglists
+0.0% +27 [ = ] 0 .debug_str
+0.4% +24 [ = ] 0 [Unmapped]
-0.0% -24 -0.0% -24 .text
+20% +4 +20% +4 DShotTelemetry::startTelemetryRequest()
-30.8% -4 -30.8% -4 g_nullstring
-11.1% -4 -11.1% -4 septentrio::SeptentrioDriver::set_clock()
-0.0% -8 -0.0% -8 g_cromfs_image
-0.0% -12 -0.0% -12 [section .text]
+0.0% +140 -0.0% -24 TOTAL
Updated: 2026-08-11T19:15:09 |
Sorry, something went wrong.
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.
Summary
Backport of #28222 to release/1.18. Cherry-pick, no conflicts and no changes from the original.
Problem
Serial ESC telemetry can end up permanently attributed to the wrong motor: a response arriving after its request timed out stays in the RX FIFO and is decoded as the next motor's, and the resulting one-ESC offset is self-sustaining. See #28222 for the full account.
This first shipped in 1.18 (the driver rework in f00e46f replaced the 30 ms response timeout and the pre-read bytesAvailable() check with an 800 µs–5 ms window), so 1.18 is the release that needs it. Measured on a 4-in-1 AM32 production fixture: spins reporting 0 rpm with valid voltage went from 1.9% on v1.17.0 to 14.1% on v1.18.0-beta1.
Solution
_uart.flush() when a telemetry request is started, so a response can only be attributed to the motor it was asked of, and the response timeout restored to 30 ms.