fix(stm32wl): make HardFault UART reporting resilient to a frozen tick count - #11196
fix(stm32wl): make HardFault UART reporting resilient to a frozen tick count#11196ndoo wants to merge 4 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughHardFault diagnostic output on STM32WL now bypasses the HAL UART path, uses direct USART register access with DWT cycle-counter timeouts, and waits for final transmission completion. ChangesFault-safe UART diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…k count uart_debug_write() (STM32duino SrcWrapper) times its "wait for UART idle" loop, and HAL_UART_Transmit()'s own internal timeout, off HAL_GetTick(), which only advances via the SysTick interrupt. HardFault is fixed at NVIC priority -1 (always the highest in the system), so SysTick can never preempt it: if the UART happens to be genuinely mid-transmission when the fault lands (e.g. a log line was still draining out), HAL_GetTick() is frozen for as long as we're inside the fault handler, the timeout can never trip, and HardFault_Handler_C hangs forever inside HAL_UART_GetState instead of printing the register dump or reaching the SOS blink loop - reproduced on wio-e5 hardware under a burst of incoming DMs. Bypass the framework TX path entirely for the fault handler's own output: write straight to the debug UART peripheral registers, using the DWT cycle counter (a free-running hardware counter that keeps incrementing regardless of interrupt state) to bound the wait instead of HAL_GetTick(). Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg>
ecdf166 to
f1952df
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/stm32wl/main-stm32wl.cpp`:
- Around line 223-225: Shorten the comment above the direct UART debug write to
two lines or fewer, retaining only that HAL_GetTick-based timeout is unsafe in
the fault handler and DWT->CYCCNT provides timing that continues during faults.
Remove the extra explanatory detail while preserving the implementation
unchanged.
- Around line 235-250: Update faultSafeUartWrite so it initializes the DWT cycle
counter deadline once before the byte loop and reuses that single timeout budget
for both TXE/TXFNF waits and the final TC wait. Remove the per-byte start reset,
and return when the elapsed cycles from the shared start reach timeoutCycles,
preserving the existing bounded-failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5444b2aa-0e9b-438a-b7f9-79d3331bd969
📒 Files selected for processing (1)
src/platform/stm32wl/main-stm32wl.cpp
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (31)
Build artifacts expire on 2026-08-25. Updated for |
Per-byte timer reset meant a wedged-but-slowly-progressing UART could stretch the total write out to size * timeoutCycles instead of being bounded by one deadline. Share a single start point across the byte loop and the final TC wait instead. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg>
|
@coderabbitai addressed feedback, please review again |
|
✅ Action performedReview finished.
|
|
@t-miura reports that this fixes the hardfault crash report, it can now print fully: Before: After: |
|
Closing this - I will raise this as an upstream bug report in STM32duino if it continues to trouble us during hardfault debugging, but currently SWD is a more reliable way to see what is going on during a hardfault anyway. |
2 participants
Problem
On STM32WL (reproduced on wio-e5 hardware, under a burst of incoming DMs), a HardFault can leave the device completely locked up with no diagnostic output at all — not even the
HardFault!register dumpHardFault_Handler_Cis supposed to print over serial before blinking SOS.debug_printf()(src/platform/stm32wl/main-stm32wl.cpp) calls SrcWrapper'suart_debug_write(), whose "wait for the UART to go idle" loop — andHAL_UART_Transmit()'s own internal timeout — are both bounded usingHAL_GetTick(), which only advances via the SysTick interrupt.HardFaultis fixed at NVIC priority -1 (always the highest priority in the system), so SysTick can never preempt it. If the UART happens to be genuinely mid-transmission when the fault lands (e.g. aLOG_HEAPline was still draining out over serial, which is exactly what was observed),HAL_GetTick()is frozen for as long as we're inside the fault handler — the timeout condition can never become true, andHardFault_Handler_Cspins forever insideHAL_UART_GetState()instead of ever printing anything or reaching the SOS blink loop. Confirmed with GDB: the device was found parked inHAL_UART_GetState<-serial_tx_active<-uart_debug_write<-debug_printf<-HardFault_Handler_C, mid-fault, having never printed the"HardFault!\r\n"already sitting in its own message buffer.This doesn't fix whatever originally triggers the HardFault (that's a separate, harder problem under active investigation) — it fixes the fact that when one does happen, the device hangs silently instead of giving us anything to debug from.
Fix
Bypass the framework's UART TX path entirely for the fault handler's own diagnostic output.
faultSafeUartWrite()writes straight to the debug UART peripheral registers (Serial.getHandle()->Instance, which resolves to whateverDEBUG_UART/SERIAL_UART_INSTANCEthe variant is actually configured for), pollingUSART_ISR_TXE_TXFNF/USART_ISR_TCdirectly and bounding the wait with the DWT cycle counter (DWT->CYCCNT) instead ofHAL_GetTick(). The DWT cycle counter is a free-running hardware counter that keeps incrementing regardless of interrupt/exception state, so it's safe to use as a deadline from inside a fault handler where nothing else in the system is running.Test plan
pio run) on all three affected STM32WL environments with differentSERIAL_UART_INSTANCEconfigs:wio-e5(instance 1),rak3172(instance 1, default),CDEBYTE_E77-MBL(instance 2) — confirms theSerial-derived peripheral lookup generalizes correctly.🤝 Attestations
Summary by CodeRabbit