feat(nxplpc): find_core_root + LPC845 RELEASE defines + pin bump to ACLPC#29#576
Merged
Merged
Conversation
…CLPC#29 Three fbuild-side changes the LPC845-BRK bring-up surfaced. Each one is a real bug in the post-vendoring (#570) nxplpc path; together they are the minimum needed for `fbuild build --environment lpc845brk` to produce a linkable firmware that fits in 64 KB flash for a sketch that uses the upstream Arduino core. 1. crates/fbuild-packages/src/library/arduino_core_lpc8xx.rs `find_core_root` helper — GitHub's archive tarball wraps repo contents in `<repo>-<sha>/<contents>`; without stripping that the package validator's `install_dir.join("cores/lpc8xx/main.cpp").exists()` returns false even though the file is present at `install_dir/ArduinoCore-LPC8xx-<sha>/ cores/lpc8xx/main.cpp`. Other core packages (arduino_core.rs etc) already carry the same helper; this PR ports it. Also: applies it to `install_path()`, `core_dir()`, `variant_dir()`, `linker_script()`, and `is_installed()` so every accessor sees the unwrapped layout. 2. crates/fbuild-packages/src/library/arduino_core_lpc8xx.rs Pin bump: ACLPC#23 head (6031232) → ACLPC#29 merge (195a2eddd31eba84). The six intervening upstream PRs (#24–#29) shipped: #24: operator new/new[], .ARM.exidx, forced heap base, F_CPU=24MHz #25: drop unsigned-long operator new overloads (32-bit ABI) #26: collapse operator delete variants into one free thunk #27: Wire/SPI proxy + function-local-static singleton + Stream& conv #28: discard .ARM.exidx on M0+ (it was overflowing flash on parts where .text already filled most of FLASH) #29: pack libgcc __aeabi_fdiv into the pre-CRP gap (0xC0..0x2FC), reclaiming 564 bytes of flash that was 0xFF padding Without these the LPC845 link fails with "FLASH overflowed by 564 bytes" (#29 fix) on a basic JSON-RPC sketch; with them it fits with margin. 3. crates/fbuild-config/assets/boards/json/lpc845brk.json Adds `-DRELEASE=1 -DFASTLED_DISABLE_DBG=1` to the `extra_flags` of the built-in LPC845-BRK board entry. Without these, FastLED's log.h auto-defines FASTLED_FORCE_DBG=1 in non-RELEASE builds, expanding every FL_DBG callsite (including those inside fl::Remote) through the fl::println formatting machinery — ~4 KB of flash bloat that doesn't fit on this 64 KB part. PlatformIO `[env:*]` `build_flags` are *not* propagated by the nxplpc orchestrator today (this is part of what #574 calls out architecturally), so the only place to put board-wide defines that reach FastLED's library compile is here. Validated end-to-end on real LPC845-BRK hardware: TX `{"jsonrpc":"2.0","method":"echo","params":[4242],"id":1}` → RX `REMOTE: {"id":1,"result":4242,"jsonrpc":"2.0"}` round-trip works. Related ------- - #570 — original nxplpc vendoring (this PR consumes the result and fixes the wrapping-dir bug it surfaced) - #574 — env-namespaced routing design (this PR's board-JSON `extra_flags` workaround is a stopgap for the missing build_flags propagation) - FastLED/framework-arduino-lpc8xx#24 through #29 — the upstream fixes the pin bump consumes Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 9 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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 (2)
✨ 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 was referenced Jun 14, 2026
zackees
added a commit
that referenced
this pull request
Jul 10, 2026
…574) (#1022) Two concrete pieces of the env-driven namespacing design: 1. **Typed `EnvNamespace { env_id, platform, board, framework }`** in fbuild-core — the `platformio.ini [env:*]` routing triplet, with `slug()` (per-env build output / lib cache key) and `framework_segment()` helpers. Constructed via `BuildContext::env_namespace(env_id, platform)`. 2. **Single-source-of-truth compile overlays.** The (user_overlay, src_overlay) assembly that decides which flags reach each translation unit was copy-pasted in three places (`pipeline::sequential`, esp32 orchestrator, nxplpc orchestrator) — a drift risk. Extracted `BuildContext::compile_overlays()` (+ `_with_base` for esp32's SDK-defines prepend) over a pure, unit-tested `assemble_compile_overlays`. All three now call it, so `[env:*] build_flags` reach framework/core + library + sketch compiles uniformly by construction. Fixes a real inconsistency along the way: `params.extra_build_flags` (caller-injected one-off flags, e.g. QEMU emulation defines) were applied on the ESP32 path but DROPPED by the shared sequential pipeline — the same build behaved differently across platforms. They're now folded into `user_flags` in `BuildContext::new`, so every orchestrator applies them. Plus `docs/namespacing.md` documenting the namespace layout + propagation rule. Note: the nxplpc "drops build_flags" symptom that motivated #574 was already retired on main (#576/#587); the residual work is the typing + the structural de-duplication that prevents that class of bug from recurring. Threading EnvNamespace into every fetcher/cache-path (criterion 2's full breadth) and `fbuild cache gc --env` are follow-ups on this foundation. Verified: EnvNamespace (3) + overlay-assembly (1) unit tests; workspace compiles; clippy -D warnings + fmt clean. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
Three fbuild-side fixes the LPC845-BRK bring-up surfaced. Each is a real bug in the post-vendoring (#570) nxplpc path.
1.
find_core_roothelperGitHub's archive tarball wraps repo contents in
<repo>-<sha>/<contents>. Without stripping that, the package validator'sinstall_dir.join("cores/lpc8xx/main.cpp").exists()returns false even though the file is present atinstall_dir/ArduinoCore-LPC8xx-<sha>/cores/lpc8xx/main.cpp. Other core packages (arduino_core.rsetc) already carry the same helper; this ports it. Applied toinstall_path(),core_dir(),variant_dir(),linker_script(), andis_installed()so every accessor sees the unwrapped layout.2. Pin bump: ACLPC#23 head → ACLPC#29 merge
The six intervening upstream PRs all shipped:
esp32devpost-build crash testing #25 — drop unsigned-long operator new overloads (32-bit ABI)__aeabi_fdivinto the pre-CRP gap (0xC0..0x2FC), reclaiming 564 bytes of flash that was 0xFF paddingWithout these the LPC845 link fails on a basic JSON-RPC sketch; with them it fits.
3. LPC845 board defines:
-DRELEASE=1 -DFASTLED_DISABLE_DBG=1Added to
extra_flagsin the built-in board entry. Without them, FastLED'sfl/log/log.hauto-definesFASTLED_FORCE_DBG=1in non-RELEASE builds, expanding every FL_DBG callsite (including those inside fl::Remote) through thefl::printlnformatting machinery — ~4 KB of flash bloat that doesn't fit on a 64 KB part.PlatformIO
[env:*]build_flagsare not propagated by the nxplpc orchestrator today — surfaced as part of #574's broader architectural concern about env-namespaced routing. Until #574 lands, the only place to put board-wide defines that reach FastLED's library compile is the board JSON.Validation
End-to-end on real LPC845-BRK hardware (
COM10, VID:PID 1FC9:0132):Related
extra_flagsworkaround is a stopgap for the missingbuild_flagspropagation)🤖 Generated with Claude Code