fix(esp32): boot ESP32-P4 eco2 + validate flash offsets (#278, #279)#281
Merged
Conversation
ESP32-P4 (eco2, rev v1.3) wouldn't boot under fbuild. Two distinct bugs, the second masked by the first; both fixed here, both verified on hardware (boot console captured from COM25: SPI_FAST_FLASH_BOOT + clean entry, 80+ `Hello from ESP32-P4!` lines from the demo .ino). Bug 1 -- bootloader flashed at the wrong offset. The ROM expects the second-stage bootloader at 0x2000 on ESP32-P4 (and ESP32-C5); fbuild was flashing it at 0x0, so the ROM read garbage and reboot-looped with `invalid header: 0x47550ff7`. Set the correct offset in esp32p4.json and esp32c5.json, and added test_bootloader_flash_offsets_match_rom asserting every esp32 config against the known ROM values. Bug 2 -- bootloader + app linked for the wrong ROM revision. The framework ships two ESP32-P4 SDK variants: `esp32p4` (eco5, chip rev 301-399) and `esp32p4_es` (eco0-eco2, rev 1-199). fbuild always picked `esp32p4`, so the eco5-linked bootloader's entry was an illegal instruction on eco2 silicon, and the app's segments overlapped the eco2 bootloader stack. Mirror upstream arduino-esp32 by introducing a `build.chip_variant` board field + `BoardConfig::sdk_variant()` accessor: the "ES pre rev.300" boards (esp32-p4-evboard, esp32-p4, m5stack-tab5-p4) select `esp32p4_es`; `esp32-p4_r3` keeps `esp32p4`. All ESP32 SDK accessors (includes, libs, ld_flags/scripts/libs, defines, bootloader ELF, partitions) now resolve through sdk_variant(); the build fingerprint includes board_chip_variant so the cache invalidates correctly. esptool `--chip` and the per-MCU compiler/linker config still key on `mcu`. The P4 acceptance test (tests/platform/esp32p4) now sets ARDUINO_USB_CDC_ON_BOOT=1 + ARDUINO_USB_MODE=1 so Serial reaches the board's USB-Serial/JTAG port (the only host interface), making the "output on the monitor" success path actually observable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lash
The ESP32 deploy path silently swallowed `get_mcu_config` errors and fell
back to the esp32 MCU config, meaning an unsupported/new ESP32 variant
got flashed with esp32's 0x1000 bootloader offset -- wrong for every
RISC-V variant (need 0x0) and C5/P4 (need 0x2000). The result was a
device that wouldn't boot (`invalid header` reboot loop until reflashed).
Replace the `unwrap_or_else(get_mcu_config("esp32"))` fallback with
`map_err(|e| FbuildError::DeployFailed(...))?` so the deploy fails with
a clear "unsupported ESP32 MCU '<x>' for board '<id>' ... cannot
determine flash offsets" instead of silently misflashing. The build path
already propagates this error with `?`; only the deploy path was unsafe.
This is the highest-value item in #279 -- it converts every coverage gap
(e.g. esp32-c61-devkitc1-n8r2 today, any future variant tomorrow) from a
silent brick-until-reflash into a loud error.
Refs #279.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds `ci/check_flash_offsets.py`, a CI guardrail that cross-checks every `crates/fbuild-build/src/esp32/configs/esp32*.json` bootloader / partitions / firmware offset against the authoritative arduino-esp32 `boards.txt` (`<chip>.build.bootloader_addr`, with the `platform.txt` default applied to chips that don't override -- e.g. esp32/esp32s2). Hard-fails on mismatch, on a config chip unknown to boards.txt, on missing offsets, and on `partitions != 0x8000` / `firmware != 0x10000`. Prints a per-chip PASS/FAIL table. The authoritative source is resolved in priority order: `--boards-txt` explicit, `$FBUILD_ESP32_BOARDS_TXT`, fbuild's installed framework cache, or `--download` from the pioarduino release tag for CI. Wired into `.github/workflows/validate-boards.yml` via `--download` since the validate-boards job doesn't pre-install the ESP32 framework. Closes the recurring class of bug behind #278: a future ESP32 variant shipping a wrong/absent bootloader offset can no longer reach CI green. Refs #279. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`cargo test --workspace` was failing on `Doc-tests fbuild_python` with "can't find crate for `pyo3`": rustdoc compiles the cdylib's lib.rs as a doctest harness, but PyO3's `extension-module` feature suppresses the libpython link, so the pyo3 macros fail to resolve. This is the standard PyO3 cdylib limitation; there are no `///` examples in this crate anyway. Setting `doctest = false` on `[lib]` is PyO3's documented fix and lets the full workspace test suite pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Multi-line `assert_eq!` reformatting picked up by `cargo fmt` under the current toolchain. Pure formatting, no behavior change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 39 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
✨ 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 |
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
0x0→0x2000for P4 and C5, matching the ROM), and the eco0–eco2 boards now select theesp32p4_esSDK (base ROM) via a newchip_variant/sdk_variant()board mechanism, so the bootloader entry no longer hits anIllegal instructionpanic and the app's segments don't overlap the eco2 bootloader stack. Verified live on the attached eco2 board: cleanboot:0x30f (SPI_FAST_FLASH_BOOT)+entry+ 80+Hello from ESP32-P4!lines fromtests/platform/esp32p4over COM25.0x1000on an unknown MCU (it would brick every RISC-V variant); it now errors clearly. A newci/check_flash_offsets.pycross-checks everyesp32*.jsonbootloader/partitions/firmware offset against the authoritative arduino-esp32boards.txt(build.bootloader_addr, with theplatform.txtdefault applied for chips that don't override). Wired intovalidate-boards.ymlvia--download.cargo test --workspacepasses; one rustfmt drift onzccache.rs.Test plan
soldr cargo check -p fbuild-daemon -p fbuild-build --all-targets— clean.soldr cargo test -p fbuild-config -p fbuild-build --lib— passes, incl.test_bootloader_flash_offsets_match_rom,test_esp32p4_evboard_uses_es_chip_variant,test_esp32p4_r3_uses_eco5_chip_variant,test_sdk_variant_falls_back_to_mcu.soldr cargo test -p fbuild-daemon deploy— 10 passed.uv run python ci/check_flash_offsets.py— all 9 chips PASS against the installed framework.esp32p4bootloader back to0x0makes the check exit 1 withesp32p4: bootloader offset '0x0' != authoritative '0x2000'; reverted.fbuild deploy tests/platform/esp32p4 -e esp32p4 -p COM25succeeds (full flash) on the attached ESP32-P4-EV (eco2, rev v1.3); the demo .ino runs and printsHello from ESP32-P4!continuously on the USB-Serial/JTAG console.Closes #278.
Refs #279 (this PR implements items 1, 3, and 4 of the sub-issue checklist; item 2 — adding an
esp32c61config — was intentionally deferred because the installed arduino-esp32 framework ships noesp32c61SDK libs, so a config would be unbuildable; the deploy fail-loud change handles c61 safely by erroring with a clear message).🤖 Generated with Claude Code