fix(stm32wl): recover from littlefs internal corruption instead of hanging - #11230
fix(stm32wl): recover from littlefs internal corruption instead of hanging#11230ndoo wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughSTM32WL LittleFS assertions now invoke a recovery handler that records corruption, reboots the device, and formats the filesystem during the next boot. The STM32 build enables this behavior through ChangesSTM32WL LittleFS recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LittleFS
participant lfs_assert
participant SystemReset
participant preFSBegin
participant fsFormat
LittleFS->>lfs_assert: failed expression text
lfs_assert->>SystemReset: set corruption flag and request reset
SystemReset->>preFSBegin: next boot
preFSBegin->>fsFormat: format corrupted filesystem
🚥 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 |
⚡ 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-26. Updated for |
24f2137 to
6f72258
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/littlefs/lfs_util.h`:
- Around line 77-79: Update the LFS_ASSERT macro in lfs_util.h to use a
do-while(0) wrapper around its conditional assertion call, ensuring it expands
as a single statement and cannot capture a caller’s else branch.
In `@src/platform/stm32wl/main-stm32wl.cpp`:
- Around line 191-200: Replace the raw msUntilFormattingAgain and millis()
backoff logic in lfs_assert with the repository Throttle helper. Configure and
use Throttle to enforce the reformat delay safely across millis() rollover,
preserving the existing warning and delay behavior when formatting is throttled.
🪄 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: fe6a594a-6f9f-47d7-8b49-ab25df9420c1
📒 Files selected for processing (3)
src/platform/stm32wl/littlefs/lfs_util.hsrc/platform/stm32wl/main-stm32wl.cppvariants/stm32/stm32.ini
…nging LFS_ASSERT (src/platform/stm32wl/littlefs/lfs_util.h) was a plain assert(), which on STM32WL hangs forever with no diagnostic (__wrap___assert_func is while(true);, see main-stm32wl.cpp). STM32_LittleFS::begin() is already designed to treat corruption as recoverable - format and retry, see fsFormat()/NodeDB::saveToDisk() - but that only works if lfs_mount() cleanly returns an error. An internal littlefs consistency check failing (metadata pair/CRC/block-allocator invariants) never returns at all, so a bad flash sector or power loss mid-write could permanently brick a device that would otherwise have recovered via the existing reformat path. nRF52 already hit this and fixed it (LFS_NO_ASSERT + a custom lfs_assert() that reboots into a reformat, see meshtastic#3818). Port the same approach to STM32WL: LFS_NO_ASSERT routes LFS_ASSERT through a custom lfs_assert() instead of disabling the check outright, and lfs_assert() requests a reformat-on-next-boot via a .noinit SRAM magic value (the same mechanism already used for the DFU bootloader redirect in this file, chosen specifically because backup/TAMP registers don't reliably survive a soft reset in this toolchain) and reboots, rather than trying to reformat littlefs from inside its own possibly-mid-operation callback. Unlike nRF52 (a third-party Adafruit library patched via a -include override so as not to fork it), STM32WL's littlefs copy is already a project-owned vendored file, so lfs_util.h is edited directly. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg>
6f72258 to
230fcda
Compare
|
Addressed both review comments: wrapped `LFS_ASSERT` in `do { ... } while (0)` so it's statement-safe, and replaced the raw `millis()` deadline arithmetic in `lfs_assert()`'s reformat backoff with `Throttle::isWithinTimespanMs()` (rollover-safe). Rebuilt and reflashed to wio-e5 to confirm normal boot after the change. @coderabbitai review |
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedReview finished.
|
Hardware test: real on-disk corruption, not a synthetic triggerDUT setup
MethodThe goal was to trigger the actual Steps:
First attempt (see below) targeted a directory entry's own pointer instead of a file's, and got silently healed by littlefs's own commit sequence within the same boot before it was ever read back — informative (shows the depth of the existing CRC-validated design), but not a test of this PR. Retargeting a file actually read early every boot avoided that race. Result: the real internal assertion fired, with its genuine condition text
For contrast: the "already-handled" caseRandom-byte corruption of the same partition (not targeting a specific pointer) is caught earlier, by littlefs's own commit-CRC validation, and handled by the existing (pre-PR) code path: No hang either way, but this is a different (already-working) mechanism — protobuf-level validation catching garbage file contents. This PR specifically targets the narrower case where littlefs's own internal structure is corrupted in a way that still passes its CRC check. |
2 participants
Problem
LFS_ASSERT(src/platform/stm32wl/littlefs/lfs_util.h) was a plainassert(), which on STM32WL hangs forever with no diagnostic —__wrap___assert_func(src/platform/stm32wl/main-stm32wl.cpp) iswhile(true);, no reset, nothing printed.STM32_LittleFS::begin()is already designed to treat filesystem corruption as recoverable — format and retry, seefsFormat()/NodeDB::saveToDisk()— but that only works iflfs_mount()cleanly returns an error code. An internal littlefs consistency check failing (metadata pair/CRC/block-allocator invariants) never returns at all; it falls straight into the assert hang. A bad flash sector or power loss mid-write could permanently brick a device that would otherwise have recovered via the existing reformat path.nRF52 already hit this exact problem and fixed it:
LFS_NO_ASSERT+ a customlfs_assert()that reboots into a reformat (#3818). STM32WL had no equivalent — it was fully exposed to the bug nRF52 already solved.Fix
Port the nRF52 approach to STM32WL:
variants/stm32/stm32.ini: add-DLFS_NO_ASSERT.src/platform/stm32wl/littlefs/lfs_util.h: routeLFS_ASSERTthrough a customlfs_assert()hook instead of disabling the check outright (matching the semantics of nRF52's Adafruit-derived header, not just "turn assertions off").src/platform/stm32wl/main-stm32wl.cpp:lfs_assert()logs the corruption and requests a reformat on the next boot via a.noinitSRAM magic value — the same mechanism already used in this file for the DFU bootloader redirect, chosen specifically because backup/TAMP registers don't reliably survive a soft reset in this toolchain (see the existing comment aboveg_bootloaderMagic) — then reboots.preFSBegin()(the existing weak hookfsInit()already calls before mounting) sees the magic value, reformats via the existingfsFormat()path, and clears it. A plain (non-.noinit) retry-delay static throttles repeated reformat/reboot cycles within one power-on session, mirroring nRF52'slfs_assert().Unlike nRF52 (a third-party Adafruit library, patched via a
-includeoverride specifically to avoid forking it), STM32WL's littlefs copy is already a project-owned vendored file, solfs_util.his edited directly — no override hack needed.Test plan
pio run -e wio-e5/pio run -e rak3172— build clean.meshtastic --infohealthy, radio init succeeds).lfs_crc()/directory-commit format) to corrupt a real file's CTZ head pointer (channels.proto'sheadfield, 8 → 9999 — a block number pastblock_count=56) while recomputing a matching CRC-32 so the corrupted commit still passes littlefs's own validation. Wrote it back and reset with no gap. Confirmed over serial the actual internal littlefs assertion fired with its genuine condition text:"NOTE! Record critical error 13"→"LittleFS format complete; restoring default settings"→ filesystem came back empty and every file correctly fell back toInstall default ...→ normal boot continued, radio init succeeded. This is the exact bug being fixed, reproduced from real corrupted on-disk data rather than a manual function call, and the exact recovery path this PR adds.🤝 Attestations
Summary by CodeRabbit