Skip to content

fix: link ESP-IDF 4.4 radio archives from SDK ld - #1650

Merged
zackees merged 1 commit into
mainfrom
fix/4686-idf44-radio-link
Oct 4, 2026
Merged

zackees merged 1 commit into
mainfrom
fix/4686-idf44-radio-link

Conversation

@zackees

@zackees zackees commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Arduino-ESP32 2.x keeps radio archives (libphy.a, librtc.a, and related libraries) in tools/sdk/esp32/ld, outside the common SDK lib directory. The fallback SDK linker list now includes that archive directory. Bump fbuild to 2.5.36 for the FastLED pin.

The focused regression test failed before the fix and passes after it. fbuild-library tests, formatting, Clippy, and the source-bound local gate pass. With the patched binary, esp32dev_idf44 OTA and the full 97-example catalog pass; the published 2.5.35 build reproduces OTA's missing radio symbols.

Tracks FastLED/FastLED#4686.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed library linking for legacy Arduino-ESP32 2.x SDKs so radio archives in the ld directory are included in builds.

Local-Gate: v1 tree=53c36b735851f2b72a84eacfcd91ae5b4e3bb563 secs=764 lanes=linux-minimal:run,dylint:run
Ci-Attestation: {"at":1791129158,"gate":"general/all/ubuntu-ci-guards","host":"linux-x86_64","key":"2ea1a9d6b92ced3df74ca7d7e209de580f0dbb6c5519809bcb63fc1f23cbaaba","lane":"linux-minimal","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":400,"stamp":"457b60e3db34caa853d6b863db6ae25d","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
Ci-Attestation: {"at":1791129158,"gate":"rust/x86_64-unknown-linux-gnu/workspace-clippy","host":"linux-x86_64","key":"2ea1a9d6b92ced3df74ca7d7e209de580f0dbb6c5519809bcb63fc1f23cbaaba","lane":"linux-minimal","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":400,"stamp":"19f505a76cf6cc68295111d6c56e368f","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
Ci-Attestation: {"at":1791129158,"gate":"rust/x86_64-unknown-linux-gnu/workspace-test","host":"linux-x86_64","key":"2ea1a9d6b92ced3df74ca7d7e209de580f0dbb6c5519809bcb63fc1f23cbaaba","lane":"linux-minimal","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":400,"stamp":"c5f34ff4f07e6c8e5fd1ba74bb43141b","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
Ci-Attestation: {"at":1791129158,"gate":"rust/x86_64-unknown-linux-gnu/python-facade-test","host":"linux-x86_64","key":"2ea1a9d6b92ced3df74ca7d7e209de580f0dbb6c5519809bcb63fc1f23cbaaba","lane":"linux-minimal","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":400,"stamp":"50752d4521027425954e1e1f239d96ab","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
Ci-Attestation: {"at":1791129158,"gate":"general/all/dylint-policy","host":"linux-x86_64","key":"2cc46a15c0aafa04ceb41b640ddd53391c795edc07ef7f606686a7656c4c8d58","lane":"dylint","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":364,"stamp":"c8a99b9577a65597f77df897eef083e1","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
Ci-Attestation: {"at":1791129158,"gate":"rust/x86_64-unknown-linux-gnu/dylint-library-check","host":"linux-x86_64","key":"2cc46a15c0aafa04ceb41b640ddd53391c795edc07ef7f606686a7656c4c8d58","lane":"dylint","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":364,"stamp":"e357d67a244e3496cb3abe50e3ec5a94","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
Ci-Attestation: {"at":1791129158,"gate":"rust/x86_64-unknown-linux-gnu/workspace-dylint","host":"linux-x86_64","key":"2cc46a15c0aafa04ceb41b640ddd53391c795edc07ef7f606686a7656c4c8d58","lane":"dylint","parents":["c38329f2e90f5878f939159d818ffcb3f5e5358f"],"secs":364,"stamp":"e4ef5e6509837af42d9f376f5446e8e4","tree":"53c36b735851f2b72a84eacfcd91ae5b4e3bb563","v":1,"via":"run"}
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: FastLED/fbuild/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d43cd2e0-4330-4a9e-8a70-b68a6b3392a0
📥 Commits

Reviewing files that changed from the base of the PR and between c38329f and dff6d1c.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • crates/fbuild-library/src/library/esp32_framework/sdk_paths.rs
  • crates/fbuild-library/src/library/esp32_framework/tests.rs
  • pyproject.toml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Legacy Arduino-ESP32 2.x library flag generation now searches the SDK’s ld/ directory and includes archives found there. A test checks the phy and rtc archives. The workspace and Python project versions change to 2.5.36.

Changes

Legacy ESP32 SDK library linking

Layer / File(s) Summary
Discover and test legacy SDK archives
crates/fbuild-library/src/library/esp32_framework/sdk_paths.rs, crates/fbuild-library/src/library/esp32_framework/tests.rs
When the legacy SDK has an ld/ directory, library flag generation adds it as a search path and includes its archives. The test checks for the -lphy and -lrtc flags when those archives are in ld/.
Update project versions
Cargo.toml, pyproject.toml
The workspace and Python project versions change from 2.5.35 to 2.5.36.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to dff6d

Legacy ESP32 builds now receive linker flags for radio archives stored in the SDK’s ld/ directory. No actionable merge-blocking issue is established, so the remaining merge risk is minimal.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to dff6d

The fix adds radio archives from the existing SDK installation to the established linking process. The reviewed change introduces no demonstrated new privilege, external entrypoint, or security-control bypass.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated incremental exposure is firmware built through the legacy SDK fallback: additional SDK archive code can participate in linking. The inspected change does not establish broader tenant, service, credential, or environment exposure.

Trust Boundaries and Controls

  • inferred — The new ld inputs inherit the existing trust in framework SDK contents. SDK directories derive from the resolved framework installation, and variant and common archives were already accepted through the same mechanism. No new authority transition or control bypass was demonstrated; installation provenance and integrity remain outside the verified scope.
🚥 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 describes the main change: linking ESP-IDF 4.4 radio archives from the SDK ld directory.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 …
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 docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@zackees
zackees merged commit 6fbc0f9 into main Oct 4, 2026
24 of 25 checks passed
@zackees
zackees deleted the fix/4686-idf44-radio-link branch October 4, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant