Maintenance 9.x to 10#11561
Merged
Merged
Conversation
This approach doesn't prevent DShot interruption during EEPROM writes. Committing for potential future refinement. Changes: - Added impl_timerPWMSetDMACircular() to switch DMA mode at runtime - Modified processDelayedSave() to use circular mode during writeEEPROM() - Called pwmCompleteMotorUpdate() 3x to latch DShot 0 packets Issue: DShot still shows gaps during settings save on oscilloscope. Next approach: Test with simple GPIO high instead of DShot.
Move circular DShot DMA code from processDelayedSave() to writeConfigToEEPROM(). This ensures the fix works for MSP_EEPROM_WRITE commands, not just delayed saves. The MSP call path is: MSP_EEPROM_WRITE → writeEEPROM() → writeConfigToEEPROM() → writeSettingsToEEPROM() The previous commit (a6ba116) had circular DMA in processDelayedSave(), which is only called for delayed saves (on disarm), not MSP commands. Changes: - Move circular DMA setup to writeConfigToEEPROM() in config_eeprom.c - Remove unused pwmSetMotorPinsHigh() function - Add pwm_output.h include to config_eeprom.c Test method: - MSP_EEPROM_WRITE command sent once per second - DShot signal monitored on oscilloscope - Confirmed: DShot no longer interrupted during settings save Issue: #10913 Related: #9441
Based on code-reviewer agent feedback: 1. Add missing AT32 platform implementation - Implement impl_timerPWMSetDMACircular() for AT32F43x targets - Uses AT32 loop_mode (ctrl_bit.lm) instead of DMA_SxCR_CIRC 2. Remove duplicate circular DMA code from config.c - processDelayedSave() calls writeEEPROM() which calls writeConfigToEEPROM() - writeConfigToEEPROM() already has circular DMA protection - Removed redundant nested enable/disable from config.c 3. Add ATOMIC_BLOCK protection to DMA mode switch - Consistent with existing impl_timerPWMStopDMA() pattern - Prevents interrupt interference during DMA reconfiguration - Applied to HAL, StdPeriph, and AT32 implementations Issue: #10913
Critical fixes for STM32H7 DMA circular mode: - Wait for EN bit to actually clear before changing mode (was the primary bug) - Disable/re-enable timer DMA requests during reconfiguration - Reload DMA transfer count after mode change - Clear pending DMA flags Without these changes, the mode change was being ignored because the DMA stream was still active when we tried to modify the configuration.
SITL doesn't have real PWM/motor hardware, so pwmSetMotorDMACircular() and pwmCompleteMotorUpdate() don't exist in SITL builds. Wrap these calls with #if \!defined(SITL_BUILD) to allow SITL builds to compile while preserving the ESC spinup fix for hardware builds.
Address qodo-code-review feedback: Add defensive timeout checks when waiting for DMA streams/channels to disable before reconfiguring. Changes: - H7 (timer_impl_hal.c): Check if timeout expired and abort if DMA still enabled - F4/F7 (timer_impl_stdperiph.c): Add wait loop for EN bit to clear with timeout check - AT32 (timer_impl_stdperiph_at32.c): Add wait loop for chen bit to clear with timeout check This prevents potential race conditions where DMA configuration could be modified while the stream is still active, which could cause unstable behavior.
Some hardware targets don't compile DShot support, causing linker errors when trying to call pwmSetMotorDMACircular() and pwmCompleteMotorUpdate(). Change guard from: #if \!defined(SITL_BUILD) To: #if \!defined(SITL_BUILD) && defined(USE_DSHOT) This ensures the functions are only called on targets that actually have DShot compiled in, fixing build failures on targets like BEEROTORF4.
- config.c: Remove comment about circular DMA protection location (obvious from context) - timer_impl_stdperiph_at32.c: Remove redundant comment about loop mode (already clear from 'Enable loop mode' / 'Disable loop mode' comments)
pidController is FAST_CODE. Its callees that lacked FAST_CODE created
SRAM veneers (trampolines from ITCM to flash) on every call. Annotate
each callee that was audited as genuinely hot:
pid.c:
pTermProcess, dTermProcess, applyItermLimiting,
pidApplySetpointRateLimiting, checkItermLimitingActive,
checkItermFreezingActive, pidRcCommandToAngle, pidRcCommandToRate
smith_predictor.c: applySmithPredictor
filter.c: pt1ComputeRC (static; called from FAST_CODE pt1FilterApply4)
maths.c: constrain, constrainf, scaleRangef
(called from the pid.c functions above; candidates for static inline
in a future refactor, but FAST_CODE resolves the veneer for now)
fc_core.c: getAxisRcCommand
Functions removed from FAST_CODE where no FAST_CODE caller existed
(sin_approx, cos_approx, fast_fsqrtf, failsafeShouldApplyControlInput)
were never added in this tree; no net change for those.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two independent optimisations for the main PID loop: rcCommand caching (fc_core.c) getAxisRcCommand() and failsafeUpdateRcCommandValues() are now gated on isRXDataNew so they only run when the RX task delivers a fresh frame (~50 Hz) instead of every PID cycle (1 kHz / multirotor rate). rcCommand[] is updated from a small cache on every cycle so the rest of the code is unaffected. pidSumLimit precomputation (pid.c) getPidSumLimit() returns 400 or 500 based on vehicle type and axis — values that are constant for the lifetime of a flight. Add a pidSumLimit field to pidState_t, initialise it once in pidInit(), and replace the two hot-path calls in pidApplyFixedWingRateController and pidApplyMulticopterRateController with a struct field read. getPidSumLimit() is retained for pid_autotune.c (not hot). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per STM32F7 Reference Manual (RM0410 Section 8.3.5), writes to DMA_SxNDTR and DMA_SxM0AR are silently ignored while the EN bit is asserted. After calling LL_DMA_DisableStream(), EN does not clear synchronously -- the hardware may still be completing an in-progress burst. impl_timerPWMPrepareDMA() previously called LL_DMA_SetDataLength() and LL_DMA_ConfigAddresses() immediately after LL_DMA_DisableStream() with no wait. In the race window where EN is still 1, both writes are discarded, leaving stale count and address in the DMA registers. The subsequent LL_DMA_EnableStream() then fires DMA with the old (now incorrect) transfer count and/or buffer address, producing garbled DShot frames. Fix: add a bounded wait loop for EN to clear before reconfiguring. If EN has not cleared after the timeout (indicating a hardware fault), skip the reconfiguration entirely rather than proceeding with stale register values. This race is most visible on STM32F765 targets, where the larger 16 KB I-Cache keeps the interrupt handler hot path in cache, resulting in faster execution that consistently lands within the ~5-18 ns EN clear window. On STM32F745 (4 KB I-Cache), cache misses add stall cycles that naturally extend the window enough for EN to clear before reconfiguration begins.
…unter underflow Two issues in the EN-bit wait loop added to impl_timerPWMPrepareDMA(): 1. On timeout, the early return skipped setting tch->dmaState, leaving it in whatever state it had on entry (could be TCH_DMA_ACTIVE if the DMA TC interrupt was suppressed by the preceding ATOMIC_BLOCK). Subsequent calls to impl_timerPWMStartDMA() check for TCH_DMA_READY, so an ACTIVE or stale state would not cause a spurious fire, but timer.c timerIsMotorBusy() uses dmaState != TCH_DMA_IDLE as a busy check, which would return true forever on a stuck channel. Fix: explicitly set TCH_DMA_IDLE on timeout. 2. The while-loop condition used post-decrement (timeout--), causing the counter to wrap from 0 to UINT32_MAX on the final iteration. The counter is not used after the loop so this was harmless, but the pattern is fragile and easy to misread. Fix: use a for-loop where timeout decrements cleanly to 0 and the loop exit condition is unambiguous.
… fault The timeout (~140-230 us at 216 MHz) is long enough that any in-progress DMA transfer has certainly completed or been aborted by the time it expires. The stuck EN bit means we cannot reconfigure DMA registers this cycle, not that the hardware is permanently broken. Update the comment to reflect that: the ESC holds its last command for one missed frame, and EN will almost certainly have cleared before the next PrepareDMA call (~1-2 ms later).
…DMAR support The circular DMA protection was unreliable because: - DMA TC interrupt handler immediately disabled the stream on every circular cycle completion, defeating the circular mode entirely - H7 could enable circular mode with zero data length (undefined per RM) - pwmCompleteMotorUpdate() rate limiter silently swallowed the zero-throttle latching calls - Burst DMA (USE_DSHOT_DMAR) targets were not handled Fixes: - Add TCH_DMA_CIRCULAR state so IRQ handlers skip stream-disable - Disable TC interrupt during circular mode as belt-and-suspenders - Always reload transfer count to buffer size when enabling circular - Load zero-throttle directly into DMA buffers, bypassing rate limiter - Add burst DMA circular support for DMAR targets (F4/F7/H7) - Disable timer DMA request during reconfiguration on all platforms - Add __DSB() barrier before re-enabling DMA streams - Add configured check consistent with pwmCompleteMotorUpdate()
Please test: OSD stats fixes and code cleanup
TIM11/CH1 is the correct timer mapping for PB9 on STM32F405. Without this DEF_TIM entry, beeperPwmInit() silently fails because timerGetByTag(PB9, TIM_USE_BEEPER) returns NULL, leaving the beeper non-functional when beeper_pwm_mode is enabled. Fixes beeper_pwm_mode regression on this target. Same fix was previously applied to BLUEBERRYH743 (commit a66b346).
Flash supports MX35LF2G model
…ession BLUEBERRYF405: add missing PWM beeper timer entry for PB9
Replace the per-chip dynamic opcode dispatch (g_readStatusReg / g_writeStatusReg) with static use of the NAND-standard "Get Features" (0x0F) and "Set Features" (0x1F) opcodes for all supported chips. Winbond W25N01GV and W25N02KV accept both the legacy NOR-style aliases (0x05/0x01) and the NAND-standard opcodes (0x0F/0x1F). Macronix MX35LF2GE4AD only accepts the NAND-standard opcodes; using 0x01 leaves its block-protection register at 0x38 (fully write-protected), causing all blackbox writes to silently fail. Tested on W25N01G (NexusXR) and W25N02K (AOCODARCH7DUAL): 2.2 MB written to flash in a 30-second arm cycle with no errors.
…opcode drivers: use NAND-standard opcodes for W25N register read/write
uint32_t post-decrement from 0 wraps to UINT32_MAX, so timeout == 0 can never be true after the loop exits. The stream-still-enabled check alone correctly identifies whether the timeout was exhausted.
…dshot Fix ESC spinup during settings save using circular DMA
Multifunction improvement
…wait drivers: wait for DMA EN bit to clear before reconfiguring DShot DMA (F7/H7)
…ache pid/filter: FAST_CODE annotations and RC command caching
Multifunction fixes
Allow yaw reset to north when disarmed - for multirotors with no compass
Multifunction refactor
…rs" (#11445) Reverts PR #11445 (commit f6281a6, merge commit 4955d49) for 9.x compatibility. The two-pass priority algorithm is correct behavior but introduces a backward-compatibility break: Configurator 9.0.1 re-implements the firmware assignment algorithm in JS. Old Configurator + new firmware (or vice versa) shows wrong pin assignments for targets with explicit timerOverrides (e.g. OUTPUT_MODE_MOTORS / SERVOS in config.c). To re-implement for 10.x without this problem: 1. Merge this revert commit into maintenance-10.x (so both branches share the reverted state as their common ancestor). 2. Apply the two-pass algorithm again as new commits on top of 10.x. 3. Add MSP2_INAV_OUTPUT_ASSIGNMENT so the firmware reports final assignments directly — eliminates the JS/C algorithm duplication that caused this issue. See: claude/developer/workspace/pwm-compat-project/project-plan-output-assignment-api.md
Revert "use explicitly assigned motor and servo timers before auto timers" (#11445)
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Test firmware build ready — commit Download firmware for PR #11561 234 targets built. Find your board's
|
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
4 participants
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.
No description provided.