fix(stm32): discover Arduino_Core_STM32 framework libraries (fixes #202)#203
fix(stm32): discover Arduino_Core_STM32 framework libraries (fixes #202)#203
Conversation
…e, ...) The STM32 orchestrator installs the STM32duino core but never walked libraries/*/ to surface bundled libraries to sketches. Any sketch that transitively included <SPI.h> (e.g. FastLED's STM32 fastspi path) failed with "fatal error: SPI.h: No such file or directory" despite SPI living right there in the cache. Mirror PR #164's Teensy fix: - Add a shared FrameworkLibrary model + discover_framework_libraries() walker in fbuild-packages, used by both TeensyCores and Stm32Cores. - Extract the #include-scanning / transitive-closure resolver out of the Teensy orchestrator into crate::framework_libs so both platforms share one code path. - Wire Stm32Cores::get_framework_libraries() + get_framework_library_include_dirs() into the STM32 orchestrator after SrcWrapper so the resolver picks up SPI/Wire/EEPROM/... on demand and their include dirs reach the sketch compile command. Fixes #202 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces a unified framework library discovery and resolution system for Arduino-style packages. It extracts shared infrastructure to resolve framework libraries by parsing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/fbuild-packages/src/library/framework_library.rs (1)
80-131: Source collection: minor filter inconsistency with the build-side scanner.
collect_library_sources_innerskipsexample(s),test(s), andextras, but the companion scanner incrates/fbuild-build/src/framework_libs.rs(should_scan_framework_entry, Lines 190–199) additionally skipsfontconvert. If a framework library ships afontconvert/tool tree (e.g. Adafruit-GFX style), its sources would be picked up here and fed to the linker, while its headers would be invisible to the resolver — a recipe for "undefined reference tomain" or silent bloat.Not a problem for the libraries currently shipped by Arduino_Core_STM32 / Teensyduino, so low urgency; worth aligning if you want the two filters to stay in lockstep.
Optional alignment
if matches!( name.as_str(), - "example" | "examples" | "test" | "tests" | "extras" + "example" | "examples" | "test" | "tests" | "extras" | "fontconvert" ) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-packages/src/library/framework_library.rs` around lines 80 - 131, collect_library_sources_inner currently filters out "example(s)", "test(s)" and "extras" but not "fontconvert", causing a mismatch with should_scan_framework_entry; update collect_library_sources_inner (used by collect_library_sources) to also skip directories named "fontconvert" (and its lowercase variants) alongside the existing matches so library tool trees under fontconvert/ aren't picked up as linkable sources.crates/fbuild-build/src/stm32/orchestrator.rs (1)
107-121: Core fix for issue#202looks right — one small dedup opportunity.The invocation correctly mirrors Teensy: resolve after
SrcWrappersources are in, extendcore_sources, and extendinclude_dirswith bundled library header paths so<SPI.h>resolves. The ordering (framework lib includes appended before CMSIS/sysroot) means bundled library headers can't be shadowed by toolchain-provided headers.Nit:
framework.get_framework_libraries()at Line 112 already walkslibraries/*; the later call toframework.get_framework_library_include_dirs()at Line 195 re-walks the same tree (seestm32_core.rsLines 113–118, which callsget_framework_libraries()again internally). You can skip the second walk by reusing the vector from Line 112:♻️ Avoid re-walking
libraries/let framework_libs = framework.get_framework_libraries(); let framework_library_sources = resolve_framework_library_sources(&framework_libs, ¶ms.project_dir, &ctx.src_dir); if !framework_library_sources.is_empty() { tracing::info!( "STM32 framework library sources added: {}", framework_library_sources.len() ); sources.core_sources.extend(framework_library_sources); }// Bundled framework library headers (SPI, Wire, EEPROM, ...) so // sketches can `#include <SPI.h>` etc. - include_dirs.extend(framework.get_framework_library_include_dirs()); + include_dirs.extend(framework_libs.iter().flat_map(|l| l.include_dirs.iter().cloned()));Low-impact since discovery is cheap, but saves one full directory walk per build.
Also applies to: 193-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-build/src/stm32/orchestrator.rs` around lines 107 - 121, The code currently calls framework.get_framework_libraries() to compute framework_libs and later re-invokes framework.get_framework_library_include_dirs(), which causes a duplicate directory walk; reuse the previously computed framework_libs when computing include dirs instead of calling get_framework_library_include_dirs() again: pass framework_libs into whatever helper or directly compute include dirs from that vector and extend sources.include_dirs with the result, leaving resolve_framework_library_sources(&framework_libs, ...) and extending sources.core_sources unchanged; update references around framework_libs, framework_library_sources, and sources.include_dirs to use the reused data.crates/fbuild-build/src/framework_libs.rs (2)
86-91:tracing::info!log level for per-library selection — considerdebug!.Each selected library emits an info-level line at build time. On a sketch that pulls SPI + Wire + EEPROM that's 3 lines, but on a fatter project this could add noise to the normal build log.
tracing::debug!would match the "detail" nature of the message while keeping it available underRUST_LOG=debug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-build/src/framework_libs.rs` around lines 86 - 91, The per-library selection log currently uses tracing::info! inside framework_libs.rs (around where libraries[idx].name and libraries[idx].source_files.len() are referenced and where sources.extend(libraries[idx].source_files.iter().cloned()) is called); change that tracing::info! call to tracing::debug! so these routine, detailed messages are only emitted under debug logging (RUST_LOG=debug) to avoid noisy normal build logs while keeping the same message and interpolation of libraries[idx].name and libraries[idx].source_files.len().
38-47: First-library-wins on duplicate headers — document or verify intent.
header_to_library.entry(header).or_insert(idx)means that if two bundled libraries both expose the same header basename, the first one inlibraries.iter()order wins silently. Sincediscover_framework_librariessorts alphabetically by library name, the winner is determined by alphabetical order, not precedence.For Arduino_Core_STM32 (and Teensyduino) today, header basenames are unique across
libraries/*, so this is a non-issue in practice. Still, worth a one-line doc note or a debug log on collision, since a futureLibraryA/SPI.h+LibraryB/SPI.hcould route the sketch to the wrong implementation without warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-build/src/framework_libs.rs` around lines 38 - 47, The current header_to_library population silently keeps the first library index on duplicate header basenames, which can mask collisions; update discover_framework_libraries to detect when header_to_library.entry(header).or_insert(idx) would skip an existing entry and emit a debug/warn log (including the header basename, the existing library name from libraries[existing_idx].name and the new library name from libraries[idx].name) or add a one-line documentation comment about first-library-wins behavior; locate the logic in header_to_library, use collect_header_names as-is, and ensure the log clarifies that alphabetical ordering determines precedence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fbuild-packages/src/library/framework_library.rs`:
- Around line 60-78: The function library_include_dirs currently adds a
non-standard "include" directory to the returned include paths; remove the block
that constructs and pushes lib_dir.join("include") (the `include` variable and
its is_dir() check) from library_include_dirs so only the standard "src" (and
optional "utility") dirs are exposed, or if non-standard support is required
make it opt-in via a clearly named feature/flag; update any tests/comments that
assume the "include" entry accordingly.
---
Nitpick comments:
In `@crates/fbuild-build/src/framework_libs.rs`:
- Around line 86-91: The per-library selection log currently uses tracing::info!
inside framework_libs.rs (around where libraries[idx].name and
libraries[idx].source_files.len() are referenced and where
sources.extend(libraries[idx].source_files.iter().cloned()) is called); change
that tracing::info! call to tracing::debug! so these routine, detailed messages
are only emitted under debug logging (RUST_LOG=debug) to avoid noisy normal
build logs while keeping the same message and interpolation of
libraries[idx].name and libraries[idx].source_files.len().
- Around line 38-47: The current header_to_library population silently keeps the
first library index on duplicate header basenames, which can mask collisions;
update discover_framework_libraries to detect when
header_to_library.entry(header).or_insert(idx) would skip an existing entry and
emit a debug/warn log (including the header basename, the existing library name
from libraries[existing_idx].name and the new library name from
libraries[idx].name) or add a one-line documentation comment about
first-library-wins behavior; locate the logic in header_to_library, use
collect_header_names as-is, and ensure the log clarifies that alphabetical
ordering determines precedence.
In `@crates/fbuild-build/src/stm32/orchestrator.rs`:
- Around line 107-121: The code currently calls
framework.get_framework_libraries() to compute framework_libs and later
re-invokes framework.get_framework_library_include_dirs(), which causes a
duplicate directory walk; reuse the previously computed framework_libs when
computing include dirs instead of calling get_framework_library_include_dirs()
again: pass framework_libs into whatever helper or directly compute include dirs
from that vector and extend sources.include_dirs with the result, leaving
resolve_framework_library_sources(&framework_libs, ...) and extending
sources.core_sources unchanged; update references around framework_libs,
framework_library_sources, and sources.include_dirs to use the reused data.
In `@crates/fbuild-packages/src/library/framework_library.rs`:
- Around line 80-131: collect_library_sources_inner currently filters out
"example(s)", "test(s)" and "extras" but not "fontconvert", causing a mismatch
with should_scan_framework_entry; update collect_library_sources_inner (used by
collect_library_sources) to also skip directories named "fontconvert" (and its
lowercase variants) alongside the existing matches so library tool trees under
fontconvert/ aren't picked up as linkable sources.
🪄 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
Run ID: 3c38786b-24ea-4828-8e1f-a679dbb888bb
📒 Files selected for processing (8)
crates/fbuild-build/src/framework_libs.rscrates/fbuild-build/src/lib.rscrates/fbuild-build/src/stm32/orchestrator.rscrates/fbuild-build/src/teensy/orchestrator.rscrates/fbuild-packages/src/library/framework_library.rscrates/fbuild-packages/src/library/mod.rscrates/fbuild-packages/src/library/stm32_core.rscrates/fbuild-packages/src/library/teensy_core.rs
| pub fn library_include_dirs(lib_dir: &Path) -> Vec<PathBuf> { | ||
| let mut dirs = Vec::new(); | ||
| let src = lib_dir.join("src"); | ||
| if src.is_dir() { | ||
| dirs.push(src); | ||
| } else { | ||
| dirs.push(lib_dir.to_path_buf()); | ||
| } | ||
|
|
||
| let utility = lib_dir.join("utility"); | ||
| if utility.is_dir() { | ||
| dirs.push(utility); | ||
| } | ||
| let include = lib_dir.join("include"); | ||
| if include.is_dir() { | ||
| dirs.push(include); | ||
| } | ||
| dirs | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Arduino library specification 1.5 layout src utility include directory
💡 Result:
Arduino library specification (1.5 format) layout rules for src / utility / include: 1) src/ is the root for compiling and for the library’s public headers - For “1.5.x+-only” libraries, all source code is placed under the src/ folder. - The builder compiles and links everything inside src/ and all its subfolders. - Only the src/ folder is added to the compiler’s include search path. 2) Which headers get auto-added for #include by the IDE - When a user imports the library via the IDE’s “Include Library”/web editor include action, the default behavior is to add an #include for each header (.h) that is located directly in src/ (but not in src subfolders). - Therefore, the headers at src/
Citations:
- 1: https://docs.arduino.cc/arduino-cli/library-specification/
- 2: https://arduino.github.io/arduino-cli/0.10/library-specification/
- 3: https://arduino.github.io/arduino-cli/latest/library-specification/
- 4: how to use
utility/folder in library development arduino/arduino-cli#2228
include/ subdirectory is non-standard per Arduino library specification.
The Arduino 1.5+ library spec standardizes only src/ for the compiler's include search path. The include/ directory is not mentioned in the official specification (neither in 1.5 format nor in the legacy 1.0 flat layout), making it speculative and potentially prone to polluting the include path by exposing unintended headers from foreign libraries.
Consider dropping include/ unless you have a concrete library that requires it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/fbuild-packages/src/library/framework_library.rs` around lines 60 -
78, The function library_include_dirs currently adds a non-standard "include"
directory to the returned include paths; remove the block that constructs and
pushes lib_dir.join("include") (the `include` variable and its is_dir() check)
from library_include_dirs so only the standard "src" (and optional "utility")
dirs are exposed, or if non-standard support is required make it opt-in via a
clearly named feature/flag; update any tests/comments that assume the "include"
entry accordingly.
1 participant
Summary
#include <SPI.h>(and friends) fail fbuild builds since fbuild became the default backend for STM32 in FastLED CI — the orchestrator installsArduino_Core_STM32but never walkslibraries/*/to expose bundled libs to the sketch.libraries/*/, resolve which ones the sketch transitively references, inject their include dirs, and compile their sources.#include-scanning/resolution logic into a sharedframework_libsmodule so both Teensy and STM32 go through one code path instead of two near-identical copies.Closes #202.
What changed
fbuild-packages: newFrameworkLibrarystruct +discover_framework_libraries()walker inlibrary/framework_library.rs.TeensyCoresnow uses it;Stm32Coresgainsget_framework_libraries()/get_framework_library_include_dirs().fbuild-build: newframework_libs.rsmodule withresolve_framework_library_sources()+ include parsing helpers extracted fromteensy/orchestrator.rs. Teensy orchestrator calls into it.stm32/orchestrator.rs: calls the shared resolver right after SrcWrapper (so selected library sources land incore_sources) and extendsinclude_dirswithframework.get_framework_library_include_dirs(). This is the user-visible fix for stm32: Arduino_Core_STM32 framework libraries (SPI, Wire, ...) not discovered — fatal error: SPI.h: No such file or directory #202.Test plan
uv run soldr cargo check --workspace --all-targets— cleanuv run soldr cargo clippy --workspace --all-targets -- -D warnings— cleanuv run soldr cargo test -p fbuild-packages --lib— 407 passed, including newframework_librarywalker testsuv run soldr cargo test -p fbuild-build --lib— 498 passed, including sharedframework_libsresolver tests carried over from the Teensy suite (transitive includes, local-shadow-over-framework)uv run soldr cargo fmt --all --check— cleanRUSTDOCFLAGS="-D warnings" uv run soldr cargo doc -p fbuild-packages -p fbuild-build --no-deps— cleanstm32f103c8_bluepill,stm32f411ce_blackpill) and confirm theSPI.hfailure no longer reproduces🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor