Fix silent DSHOT loss on 11 F4/F7 targets from shared DMA request lines - #11803
Conversation
Several targets declared two motor outputs on DMA-option indices (dmavar) that resolve to the identical DMA stream. The firmware claims DMA in strict motor-init order and silently drops DSHOT on whichever output initializes second, with no boot-time error, so the affected output never spins under DSHOT. Fixed by moving the losing output to an alternate dmavar/timer-channel option on the same physical pin (no rewiring needed) where one exists, verified with simulate_pwm_roles.py's DMA-stream-collision check. BEEROTORF4 has two collisions where both sides are hard-wired to the same DMA stream with no alternative; documented with a comment instead since there is no fix available on this hardware revision.
|
ⓘ 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 QodoFix silent DSHOT loss from shared DMA request lines on F4/F7 targets
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1. Servo timer capacity reduced
|
| DEF_TIM(TIM8, CH4, PC9, TIM_USE_OUTPUT_AUTO, 0, 0), // S2 | ||
| DEF_TIM(TIM1, CH1, PA8, TIM_USE_OUTPUT_AUTO, 0, 0), // S3 | ||
| DEF_TIM(TIM1, CH3N, PB1, TIM_USE_OUTPUT_AUTO, 0, 0), // S4 | ||
| DEF_TIM(TIM3, CH4, PB1, TIM_USE_OUTPUT_AUTO, 0, 0), // S4 -- moved off TIM1_CH3N (DMA-stuck sharing S3's stream on both dmavar options) |
There was a problem hiding this comment.
1. Servo timer capacity reduced 🐞 Bug ≡ Correctness
On ATOMRCF405MINI and ATOMRCF405NAVI, moving S4 to TIM3 causes a 4-motor setup to claim TIM3 for motors, which blocks S8 (also on TIM3) from being allocated as a servo output. This reduces the remaining servo outputs after allocating 4 motors and can trigger PWM_INIT_ERROR_NOT_ENOUGH_SERVO_OUTPUTS for mixers requiring 4+ servos.
Agent Prompt
## Issue description
ATOMRCF405MINI and ATOMRCF405NAVI changed S4 from TIM1_CH3N to TIM3_CH4 (same PB1 pin), but S8 is also on TIM3. In a typical 4-motor configuration, INAV assigns motors in `timerHardware[]` order and uses `pwmClaimTimer()` to apply motor usage to all channels on that timer, which makes S8 unusable for servo outputs; this reduces the available servo output count and can cause `PWM_INIT_ERROR_NOT_ENOUGH_SERVO_OUTPUTS` on servo-heavy mixers.
## Issue Context
- Targets affected: `ATOMRCF405MINI`, `ATOMRCF405NAVI`
- Change: S4 now uses `TIM3` while S8 already uses `TIM3`.
- Allocation rule: servos cannot share a timer with any motor output, and motor claiming propagates to all channels of the timer.
## Fix Focus Areas
- src/main/target/ATOMRCF405MINI/target.c[36-47]
- src/main/target/ATOMRCF405NAVI/target.c[36-47]
- src/main/drivers/pwm_mapping.c[277-317]
- src/main/drivers/pwm_mapping.c[362-395]
- src/main/drivers/pwm_mapping.c[483-492]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
RAM / Flash usage vs. base branch — commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #11803 244 targets built. Find your board's
|
TIM2_CH1 (TAIL, PA15) and TIM3_CH2 (S2, PB5) both resolve to DMA1 Stream5 and both have exactly one DMA option in the F7 table -- no alternate dmavar or same-pin timer channel exists on either side, so this can't be fixed without a PCB rewire. Same known-unfixable pair already documented on BEEROTORF4's M4/M7. TAIL loses the claim and DSHOT silently doesn't work on it. Comment-only change; both targets build clean.
37 targets, comment-only: appended an explanatory comment to the losing DEF_TIM line for each undocumented NOTICE-tier motor-vs-motor collision (S5+ output silently loses DSHOT, basic quad unaffected) and each LED-strip collision (motor always wins over TIM_USE_LED, motors init before ledStripInit() in fc_init.c -- LED silently doesn't light). 6 targets (HGLRCF722, MATEKF411, MATEKF722SE, SPEEDYBEEF7MINI, TMOTORF7, TMOTORVELOXF7V2) were already fully documented and untouched. No DEF_TIM argument changed anywhere -- verified programmatically and by independent code review that every diff line is comment-only. MAMBAF405US excluded from this pass: its default (non-I2C) build variant has a genuine, previously-undetected CERTAIN hazard behind an undocumented #ifdef, found while isolating a same-(tim,ch) duplicate during this work. Needs the same fix-or-document workflow as the original 11 CERTAIN targets, not a NOTICE-tier comment -- tracked as a follow-up.
…ion) TIM8_CH3 (S1_OUT, PC8) used dmavar 0, resolving to DMA2 Stream2 -- the same stream TIM1_CH2 (S4_OUT, PA9, array position 0) claims with dmavar 1. Same stream, different channel selector, so on a basic quad build S1_OUT silently lost DSHOT since it initializes later. Fixed by switching TIM8_CH3 to dmavar 1 (DMA2 Stream4), matching the value the target's #ifdef MAMBAF405US_I2C build variant already used safely for the same pin. That made both branches byte-identical, so collapsed the #ifdef/#else/#endif into one unconditional block -- confirmed via grep that MAMBAF405US_I2C is otherwise only used for USB string and I2C bus/pin selection, unrelated to timerHardware[]. Both MAMBAF405US and MAMBAF405US_I2C build clean. classify_collisions.py and a full motorCount 1-12 sweep confirm zero hazards on MAMBAF405US now.
The default (#else) branch's old "S4_OUT..S1_OUT" labels had the convention backwards. Confirmed with the board owner: silkscreen labels are always assigned in top-to-bottom array declaration order, same as the MAMBAF405US_I2C branch's plain "S1".."S4" labels this target.c used to carry before the two branches were collapsed. Fixed to match: S1=TIM1_CH2, S2=TIM1_CH1, S3=TIM8_CH4, S4=TIM8_CH3. This also corrects which physical output was actually losing DSHOT in the previous commit's description: it's S4 (TIM8_CH3, array position 3), not S1. The dmavar fix itself (454391b) is unaffected -- comment-only change.
Summary
Several F4/F7 targets declare two motor outputs whose
dmavarindex (the DMA-option selector inDEF_TIM(tim, ch, pin, usage, flags, dmavar)) resolves to the identical DMA stream. INAV claims DMA in strict motor-init order and silently drops DSHOT on whichever output initializes second — no boot-time error, the output just never spins under DSHOT.Changes
AIKONF4,ATOMRCF405MINI,ATOMRCF405NAVI,FF_PIKOF4,FISHDRONEF4,FRSKYF4,FRSKYPILOT,PIXRACER,TMOTORF7,WARPF7: moved the losing output to an alternatedmavar/timer-channel option on the same physical pin (no PCB rewire needed).BEEROTORF4: two of its collisions have no alternative on either side (both timer/channel combinations are hard-wired to the same DMA stream on this MCU) — documented with a comment rather than a functional change, since there's no fix available on this hardware revision.FF_PIKOF4has two build variants (FF_PIKOF4OSDand default) sharing onetimerHardware[]array; both had the same collision and both are fixed identically.Testing
simulate_pwm_roles.py's DMA-stream-collision model (a deterministic port of the firmware's ownpwmClaimTimer/pwmBuildTimerOutputListDMA-claim logic) before applying it, confirming no new collision is introduced and the existing one is resolved.Related Issues
None filed yet — found via a broader audit of shared-DMA-request-line defects across F4/F7 targets.