Skip to content

feat(builder): add per-MCU CMSIS PAL + CMSIS-Core to include path#11

Open
phatpaul wants to merge 1 commit into
FastLED:mainfrom
phatpaul:feat/cmsis-pal-include-path
Open

feat(builder): add per-MCU CMSIS PAL + CMSIS-Core to include path#11
phatpaul wants to merge 1 commit into
FastLED:mainfrom
phatpaul:feat/cmsis-pal-include-path

Conversation

@phatpaul

@phatpaul phatpaul commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

The PlatformIO Arduino builder only put the core dir and the selected board-pins variant (variants/<build.variant>) on CPPPATH. But the vendor CMSIS device header (e.g. LPC845.h) is a chip-level artifact that lives in variants/<build.mcu>/, which for most boards is a different directory than the pins variant (board lpc845brk selects variants/lpc845brk/, while the PAL ships in variants/lpc845/). As a result, code that includes the vendor device header (<LPC845.h>) failed with "No such file or directory", and the transitively-included CMSIS-Core headers (core_cm0plus.h, cmsis_gcc.h) — which live at the framework root in CMSIS/ — were also missing.

Add variants/<build.mcu>/ and CMSIS/ to CPPPATH when they exist, so any board on a given MCU can consume the vendor device header regardless of which pins variant it selects. Both additions are guarded by os.path.isdir and the MCU dir is skipped when it coincides with the selected variant, so boards/frameworks without this layout are unaffected.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Arduino build support so the correct CMSIS headers are included automatically for the selected board.
    • This helps ensure boards with different MCU variants compile more reliably, even when using different pin mappings or variants.

The PlatformIO Arduino builder only put the core dir and the selected
board-pins variant (variants/<build.variant>) on CPPPATH. But the vendor
CMSIS device header (e.g. LPC845.h) is a chip-level artifact that lives in
variants/<build.mcu>/, which for most boards is a *different* directory
than the pins variant (board lpc845brk selects variants/lpc845brk/, while
the PAL ships in variants/lpc845/). As a result, code that includes the
vendor device header (<LPC845.h>) failed with "No such file or directory",
and the transitively-included CMSIS-Core headers (core_cm0plus.h,
cmsis_gcc.h) — which live at the framework root in CMSIS/ — were also
missing.

Add variants/<build.mcu>/ and CMSIS/ to CPPPATH when they exist, so any
board on a given MCU can consume the vendor device header regardless of
which pins variant it selects. Both additions are guarded by os.path.isdir
and the MCU dir is skipped when it coincides with the selected variant, so
boards/frameworks without this layout are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This change adds CMSIS include-path resolution to the Arduino framework builder. New variables (MCU, CMSIS_DIR, CMSIS_CORE_DIR) are derived from board.get("build.mcu") and FRAMEWORK_DIR, then conditionally appended to the CPPPATH list used during compilation.

Changes

CMSIS Include Path Resolution

Layer / File(s) Summary
CMSIS directory variables
builder/frameworks/arduino.py
Computes MCU from board.get("build.mcu"), derives CMSIS_DIR under variants/<mcu>/, and sets CMSIS_CORE_DIR under the framework root CMSIS/.
CPPPATH wiring
builder/frameworks/arduino.py
Updates env.Append(CPPPATH=[...]) to conditionally add CMSIS_DIR (if non-empty, distinct from VARIANT_DIR, and existing) and CMSIS_CORE_DIR (if it exists).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

A rabbit hops through headers deep,
Finding CMSIS paths while others sleep. 🐇
MCU whispers where to look,
CPPPATH grows by just one nook.
Compile clean, no missing trace—
Hooray, each chip now finds its place!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding per-MCU CMSIS and CMSIS-Core include paths.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
builder/frameworks/arduino.py (1)

67-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Misleading variable names: CMSIS_DIR/CMSIS_CORE_DIR don't match their content.

CMSIS_DIR actually resolves to variants/<mcu>/ (the chip-level device-PAL directory), while CMSIS_CORE_DIR resolves to the framework-root CMSIS/ directory. The naming is swapped relative to what each path represents — a reader expecting CMSIS_DIR to point at the CMSIS/ folder (as the PR description implies) will be confused.

♻️ Suggested rename
 MCU = board.get("build.mcu", "")
-CMSIS_DIR = os.path.join(FRAMEWORK_DIR, "variants", MCU) if MCU else ""
-CMSIS_CORE_DIR = os.path.join(FRAMEWORK_DIR, "CMSIS")
+MCU_DIR = os.path.join(FRAMEWORK_DIR, "variants", MCU) if MCU else ""
+CMSIS_DIR = os.path.join(FRAMEWORK_DIR, "CMSIS")

(and update the two usages at lines 119-123 accordingly)

🤖 Prompt for 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.

In `@builder/frameworks/arduino.py` around lines 67 - 70, The path variable names
in arduino.py are misleading because CMSIS_DIR points to variants/<mcu> while
CMSIS_CORE_DIR points to the framework CMSIS folder. Rename these locals in the
module-level setup so each name matches its actual path, and update the two
references in the later install/copy logic that consume CMSIS_DIR and
CMSIS_CORE_DIR. Use the surrounding symbols MCU, FRAMEWORK_DIR, and the related
path-handling block to locate the rename consistently.
🤖 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.

Nitpick comments:
In `@builder/frameworks/arduino.py`:
- Around line 67-70: The path variable names in arduino.py are misleading
because CMSIS_DIR points to variants/<mcu> while CMSIS_CORE_DIR points to the
framework CMSIS folder. Rename these locals in the module-level setup so each
name matches its actual path, and update the two references in the later
install/copy logic that consume CMSIS_DIR and CMSIS_CORE_DIR. Use the
surrounding symbols MCU, FRAMEWORK_DIR, and the related path-handling block to
locate the rename consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9bf0a7d0-4f86-4e04-967d-73e71860bd13

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6d395 and 71d85bf.

📒 Files selected for processing (1)
  • builder/frameworks/arduino.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant