feature: relinker for Arduino#451
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Arduino-framework support for the relinker on ESP32-family chips: implements Arduino-specific relinker setup, copies/modifies linker script in the build dir, validates CSVs and toolchain paths, provides a fallback EntityDB for section parsing, and adds documentation and an example project. Changes
Sequence DiagramsequenceDiagram
participant PIO as PlatformIO Builder
participant AF as Arduino Framework<br/>(arduino.py)
participant AR as Arduino Relinker<br/>(arduino_relinker.py)
participant RL as Relinker Core<br/>(relinker.py)
participant Linker as Linker<br/>(ld)
PIO->>AF: Start build (Arduino)
AF->>AR: setup_arduino_relinker(env,platform,mcu,chip_variant)
AR->>AR: Read `custom_relinker_*` from env and validate CSVs
AR->>AR: Copy package `sections.ld` -> `$BUILD_DIR/sections.ld`
AR->>AR: Expand `$ARDUINO_LIBS_DIR`, create `sdkconfig.arduino`, locate `objdump`
AR->>RL: run_relinker(sections.ld, lib.csv, obj.csv, fn.csv, sdkconfig, objdump)
RL->>RL: Parse sections (EntityDB or fallback), match functions, modify sections.ld
RL-->>AR: Return modified `sections.ld`
AR->>AF: Signal configured
AF->>Linker: Link using `$BUILD_DIR/sections.ld`
Linker-->>PIO: Produce final binary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
examples/arduino-relinker-esp32c2/src/main.cpp (1)
54-56: Minor note: ESP32-C2 doesn't have PSRAM.
ESP.getFreePsram()will return 0 on ESP32-C2 since it lacks PSRAM. This won't cause errors, but you might want to add a comment or conditionally skip this line for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/arduino-relinker-esp32c2/src/main.cpp` around lines 54 - 56, The call to ESP.getFreePsram() in the Serial.printf block prints 0 on ESP32-C2 since that chip has no PSRAM; update the code around Serial.printf/ESP.getFreePsram to either (a) guard the PSRAM line with a compile-time or runtime check for PSRAM presence, or (b) replace it with a comment indicating ESP32-C2 has no PSRAM so the value will be 0—refer to the existing Serial.printf and ESP.getFreePsram symbols to locate and modify the line.builder/relinker/examples/esp32c6/README.md (1)
28-38: Consider adding Arduino framework example.The usage example only shows
framework = espidf, but this PR adds Arduino relinker support. Consider adding an alternative Arduino example for consistency with the new feature.; For Arduino framework: [env:esp32c6_arduino] platform = espressif32 board = esp32c6-devkitc-1 framework = arduino custom_relinker_library = ${platformio.platforms_dir}/espressif32/builder/relinker/examples/esp32c6/library.csv custom_relinker_object = ${platformio.platforms_dir}/espressif32/builder/relinker/examples/esp32c6/object.csv custom_relinker_function = ${platformio.platforms_dir}/espressif32/builder/relinker/examples/esp32c6/function.csv🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/relinker/examples/esp32c6/README.md` around lines 28 - 38, The README example only shows framework = espidf but the relinker now supports Arduino; add an alternative example block mirroring the existing [env:esp32c6] snippet but with a unique env name (e.g. [env:esp32c6_arduino]) and set framework = arduino while keeping the three custom_relinker_* keys (custom_relinker_library, custom_relinker_object, custom_relinker_function) pointing to the same example CSVs so users can see both espidf and Arduino usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ARDUINO_RELINKER_INTEGRATION.md`:
- Around line 63-68: The documented build banner is stale — update the snippet
in ARDUINO_RELINKER_INTEGRATION.md to match the actual output from
builder/frameworks/arduino_relinker.py which prints "*** Configuring Arduino
Relinker for ... ***" (e.g., replace "*** Arduino Relinker configured for
esp32c2 ***" with "*** Configuring Arduino Relinker for esp32c2 ***") so the doc
matches the real runtime banner emitted by the code.
- Around line 331-335: The printed label in setup() is misleading:
ESP.getFreeHeap() returns free heap (DRAM), not IRAM; update the Serial.printf
call in setup() to use a correct label such as "Free Heap" or "Free DRAM" (e.g.,
change "Free IRAM: %d bytes\n" to "Free Heap: %d bytes\n") so the output matches
the value returned by ESP.getFreeHeap().
In `@builder/frameworks/arduino_relinker.py`:
- Around line 60-62: The relinker currently mutates the shared framework linker
script (sections.ld) and returns early when
relinker_function/relinker_library/relinker_object are missing, skipping the
stale-backup recovery and leaving the framework file overridden; fix by (1)
writing the relinked linker script into the per-build directory
(BUILD_DIR/sections.ld) instead of replacing the package-owned script, or if
replacement is required, ensure an always-run restore path so any existing
backup is restored even when relinker_function/relinker_library/relinker_object
is falsy (ensure the early return in the relinker path performs restore if a
backup exists), and (2) serialize backup/replace/restore with an interprocess
lock (e.g., file lock) around the operations that create backup, replace the
framework file, and restore it (the code paths that handle backup/restore and
the checkprogsize-linked restore hook must be updated to use this lock) so
same-MCU concurrent builds cannot race.
- Around line 39-62: The relinker validation is being bypassed because the
caller only checks custom_relinker_function before invoking
setup_arduino_relinker; change the caller to call setup_arduino_relinker
whenever any of custom_relinker_function, custom_relinker_library or
custom_relinker_object is set (i.e., use config.get(...,
"custom_relinker_function", "") OR config.get(..., "custom_relinker_library",
"") OR config.get(..., "custom_relinker_object", "")), so the validation block
in setup_arduino_relinker runs and errors on incomplete sets; additionally, fix
the sections.ld backup/cleanup logic that currently uses MCU-scoped names (the
code that backs up and overwrites sections.ld) to avoid stale artifacts and
races by making backups unique per-build (include env identifier/PID/timestamp)
and ensure cleanup runs deterministically (perform removal in both pre- and
post-build actions or use a finally-style/atomic rename approach) so concurrent
builds do not clobber each other.
In `@builder/relinker/examples/arduino/README.md`:
- Around line 54-57: Replace the hardcoded "esp32c2" chip name in the README
examples with a generic "<chip>" placeholder so the shared documentation works
for all variants; locate the bash snippets that use paths like
"~/.platformio/platforms/espressif32/builder/relinker/examples/arduino/esp32c2/*.csv"
and change them to use
"~/.platformio/platforms/espressif32/builder/relinker/examples/arduino/<chip>/*.csv"
(and any other occurrences of "esp32c2" in the same README) so S2/S3/C6/H2 users
are pointed to the correct files.
In `@builder/relinker/examples/esp32s2/README.md`:
- Around line 64-65: Update the README wording to remove the contradiction by
clarifying what the example relocates versus what it excludes: change the "SPI
Flash" bullet (the heading and its description that currently reads "Flash
operation functions") to explicitly state whether flash read/write/erase calls
are relocated or left in place (e.g., "SPI Flash - SPI driver functions
(excluding flash programming/erase operations)") and add a parenthetical or note
near the earlier "flash operations are excluded from relocation" sentence to
reference the same terminology (use the exact phrase "flash programming/erase
operations" or "flash write/erase" to match). Make sure both the "SPI Flash"
bullet and the later exclusion sentence use identical terms so readers know
which flash-related operations are moved by the example.
---
Nitpick comments:
In `@builder/relinker/examples/esp32c6/README.md`:
- Around line 28-38: The README example only shows framework = espidf but the
relinker now supports Arduino; add an alternative example block mirroring the
existing [env:esp32c6] snippet but with a unique env name (e.g.
[env:esp32c6_arduino]) and set framework = arduino while keeping the three
custom_relinker_* keys (custom_relinker_library, custom_relinker_object,
custom_relinker_function) pointing to the same example CSVs so users can see
both espidf and Arduino usage.
In `@examples/arduino-relinker-esp32c2/src/main.cpp`:
- Around line 54-56: The call to ESP.getFreePsram() in the Serial.printf block
prints 0 on ESP32-C2 since that chip has no PSRAM; update the code around
Serial.printf/ESP.getFreePsram to either (a) guard the PSRAM line with a
compile-time or runtime check for PSRAM presence, or (b) replace it with a
comment indicating ESP32-C2 has no PSRAM so the value will be 0—refer to the
existing Serial.printf and ESP.getFreePsram symbols to locate and modify the
line.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a279598-8a1f-4de8-8bf4-890350a768bc
⛔ Files ignored due to path filters (36)
builder/relinker/examples/arduino/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/object.csvis excluded by!**/*.csv
📒 Files selected for processing (13)
ARDUINO_RELINKER_INTEGRATION.mdbuilder/frameworks/arduino.pybuilder/frameworks/arduino_relinker.pybuilder/frameworks/component_manager.pybuilder/relinker/examples/arduino/README.mdbuilder/relinker/examples/esp32/README.mdbuilder/relinker/examples/esp32c3/README.mdbuilder/relinker/examples/esp32c6/README.mdbuilder/relinker/examples/esp32h2/README.mdbuilder/relinker/examples/esp32s2/README.mdexamples/arduino-relinker-esp32c2/README.mdexamples/arduino-relinker-esp32c2/platformio.iniexamples/arduino-relinker-esp32c2/src/main.cpp
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
builder/relinker/examples/arduino/README.md (1)
54-57:⚠️ Potential issue | 🟡 MinorUse
<chip>placeholder in generic shell examples.Lines 56 and 115 hardcode
esp32c2in paths, but this README is shared across all chip variants. Users of S2/S3/C6/H2 will be directed to incorrect paths.📝 Suggested fix
-cp ~/.platformio/platforms/espressif32/builder/relinker/examples/arduino/esp32c2/*.csv relinker/ +cp ~/.platformio/platforms/espressif32/builder/relinker/examples/arduino/<chip>/*.csv relinker/-ls ~/.platformio/packages/framework-arduinoespressif32-libs/esp32c2/lib/ +ls ~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/Also applies to: 110-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/relinker/examples/arduino/README.md` around lines 54 - 57, Replace the hardcoded chip name in the shell examples that run "mkdir -p relinker" and "cp .../*.csv relinker/" with a generic "<chip>" placeholder so the README works for all variants; update both occurrences of the bash snippet (the "mkdir -p relinker" / "cp .../*.csv relinker/" example) to use "<chip>" instead of "esp32c2" so S2/S3/C6/H2 users are directed to the correct platformio paths.
🧹 Nitpick comments (4)
builder/frameworks/arduino_relinker.py (1)
13-17: Remove unused import.
DefaultEnvironmentis imported but never used in this module. Theenvis passed as a parameter to the function.📝 Suggested fix
import os import sys import shutil from pathlib import Path -from SCons.Script import DefaultEnvironment🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/arduino_relinker.py` around lines 13 - 17, Remove the unused DefaultEnvironment import from the top of the module: the symbol DefaultEnvironment is not referenced anywhere in builder/frameworks/arduino_relinker.py and the function(s) accept an env parameter instead, so delete "from SCons.Script import DefaultEnvironment" to eliminate the unused import and related linter warning.examples/arduino-relinker/README.md (1)
84-113: Add language specifier to fenced code block for consistency.The expected output code block lacks a language specifier. While this is terminal output, adding
textimproves markdown linting compliance.📝 Suggested fix
-``` +```text === Arduino Relinker Example ===🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/arduino-relinker/README.md` around lines 84 - 113, The fenced code block in README.md that begins with "=== Arduino Relinker Example ===" is missing a language specifier; update the opening triple-backtick fence to include "text" (```text) so the terminal output block is properly marked for markdown linters and renderers.builder/relinker/examples/arduino/README.md (1)
118-121: Add language specifier to the code block.The troubleshooting section's code block at line 120 lacks a language specifier.
📝 Suggested fix
-``` +```text A function was moved to flash that gets called during flash operations or from ISR. Remove it from `function.csv` or set its option to `FALSE`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/relinker/examples/arduino/README.md` around lines 118 - 121, The code block in the "Runtime Crash" troubleshooting section is missing a language specifier; update the fenced block that contains "A function was moved to flash..." (the code block around that sentence in README.md) to include a language tag such as ```text (i.e., change the opening fence from ``` to ```text) so the snippet is explicitly marked as plain text.ARDUINO_RELINKER_INTEGRATION.md (1)
119-122: Add language specifiers to code blocks for consistency.Several code blocks showing directory/path structures lack language specifiers. Consider using
textfor plain text output.📝 Suggested fix for line 120
-``` +```text ~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/📝 Suggested fix for line 302
-``` +```text my_arduino_project/📝 Suggested fix for lines 358 and 363
-``` +```text ~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/Also applies to: 301-311, 357-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ARDUINO_RELINKER_INTEGRATION.md` around lines 119 - 122, Several fenced code blocks showing directory paths (e.g., the $ARDUINO_LIBS_DIR expansion "~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/" and the example "my_arduino_project/") are missing language specifiers; fix by updating those fences to use "text" (i.e., replace ``` with ```text) for every block that contains plain path output so the blocks at/around the occurrences of $ARDUINO_LIBS_DIR, "~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/", and "my_arduino_project/" render consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ARDUINO_RELINKER_INTEGRATION.md`:
- Around line 209-212: Replace the German heading "Beispiel:" in
ARDUINO_RELINKER_INTEGRATION.md with the English equivalent "Example:"; search
for the exact string "Beispiel:" (the line preceding the ini block with
custom_relinker_missing_function_info) and update it to "Example:" so the
documentation language is consistent.
- Line 31: Replace the German word "mit" with English "with" in the device
description string '`esp32c6` - ESP32-C6 (RISC-V mit WiFi 6)' so it reads
'`esp32c6` - ESP32-C6 (RISC-V with WiFi 6)'; update that exact text in
ARDUINO_RELINKER_INTEGRATION.md to correct the language.
---
Duplicate comments:
In `@builder/relinker/examples/arduino/README.md`:
- Around line 54-57: Replace the hardcoded chip name in the shell examples that
run "mkdir -p relinker" and "cp .../*.csv relinker/" with a generic "<chip>"
placeholder so the README works for all variants; update both occurrences of the
bash snippet (the "mkdir -p relinker" / "cp .../*.csv relinker/" example) to use
"<chip>" instead of "esp32c2" so S2/S3/C6/H2 users are directed to the correct
platformio paths.
---
Nitpick comments:
In `@ARDUINO_RELINKER_INTEGRATION.md`:
- Around line 119-122: Several fenced code blocks showing directory paths (e.g.,
the $ARDUINO_LIBS_DIR expansion
"~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/" and the
example "my_arduino_project/") are missing language specifiers; fix by updating
those fences to use "text" (i.e., replace ``` with ```text) for every block that
contains plain path output so the blocks at/around the occurrences of
$ARDUINO_LIBS_DIR,
"~/.platformio/packages/framework-arduinoespressif32-libs/<chip>/lib/", and
"my_arduino_project/" render consistently.
In `@builder/frameworks/arduino_relinker.py`:
- Around line 13-17: Remove the unused DefaultEnvironment import from the top of
the module: the symbol DefaultEnvironment is not referenced anywhere in
builder/frameworks/arduino_relinker.py and the function(s) accept an env
parameter instead, so delete "from SCons.Script import DefaultEnvironment" to
eliminate the unused import and related linter warning.
In `@builder/relinker/examples/arduino/README.md`:
- Around line 118-121: The code block in the "Runtime Crash" troubleshooting
section is missing a language specifier; update the fenced block that contains
"A function was moved to flash..." (the code block around that sentence in
README.md) to include a language tag such as ```text (i.e., change the opening
fence from ``` to ```text) so the snippet is explicitly marked as plain text.
In `@examples/arduino-relinker/README.md`:
- Around line 84-113: The fenced code block in README.md that begins with "===
Arduino Relinker Example ===" is missing a language specifier; update the
opening triple-backtick fence to include "text" (```text) so the terminal output
block is properly marked for markdown linters and renderers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46688ab7-22af-4201-993e-455b3d43a30b
⛔ Files ignored due to path filters (36)
builder/relinker/examples/arduino/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/object.csvis excluded by!**/*.csv
📒 Files selected for processing (14)
ARDUINO_RELINKER_INTEGRATION.mdbuilder/frameworks/arduino.pybuilder/frameworks/arduino_relinker.pybuilder/frameworks/component_manager.pybuilder/relinker/examples/arduino/README.mdbuilder/relinker/examples/esp32/README.mdbuilder/relinker/examples/esp32c3/README.mdbuilder/relinker/examples/esp32c6/README.mdbuilder/relinker/examples/esp32h2/README.mdbuilder/relinker/examples/esp32s2/README.mdbuilder/relinker/relinker.pyexamples/arduino-relinker/README.mdexamples/arduino-relinker/platformio.iniexamples/arduino-relinker/src/main.cpp
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
builder/relinker/examples/esp32s2/README.md (1)
7-7: Minor: Consider hyphenating "LX7-based".For compound adjectives, "Xtensa LX7-based single-core chip" is grammatically preferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/relinker/examples/esp32s2/README.md` at line 7, Change the phrase "Xtensa LX7 based single-core chip" to the hyphenated form "Xtensa LX7-based single-core chip" in the README text so the compound adjective is grammatically correct; locate the sentence mentioning "The ESP32-S2 is an Xtensa LX7 based single-core chip..." and update it to use "LX7-based".builder/frameworks/arduino_relinker.py (1)
17-17: Unused import:DefaultEnvironment.
DefaultEnvironmentis imported but never used in this module.📝 Suggested fix
-from SCons.Script import DefaultEnvironment🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/arduino_relinker.py` at line 17, Remove the unused import DefaultEnvironment from the module: the import statement "from SCons.Script import DefaultEnvironment" in arduino_relinker.py is not referenced anywhere in the file (no functions/classes use DefaultEnvironment), so delete that import to clean up unused dependencies and satisfy linting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@builder/relinker/relinker.py`:
- Around line 55-68: The code reads and consumes first_line from
sections_info_dump to detect an "In archive ..." banner, but when that banner is
absent it drops that first_line before calling self._parse_content, losing the
first object's sections; fix by preserving first_line when no archive is found
(e.g., if archive is None, prepend first_line back onto the remainder before
calling self._parse_content or read the stream without discarding it), updating
the logic around variables first_line, archive, remaining and the call to
self._parse_content so self.sections[archive] receives the full content
including that initial line (also keep the existing fallback using
getattr(sections_info_dump, 'name')).
- Around line 76-80: The loop that sets current_obj in relinker.py stores
archive-member names with their full path/prefix, but _match_obj() expects
normalized basenames; update the storing step in the for-loop (where current_obj
is assigned) to normalize archive-member names the same way
_parse_all_obj_sections() does (strip archive prefixes and use basename) so that
get_sections() and _match_obj() can find members; locate the code around the for
line loop that assigns current_obj and change it to produce the same normalized
member name used by _parse_all_obj_sections().
---
Nitpick comments:
In `@builder/frameworks/arduino_relinker.py`:
- Line 17: Remove the unused import DefaultEnvironment from the module: the
import statement "from SCons.Script import DefaultEnvironment" in
arduino_relinker.py is not referenced anywhere in the file (no functions/classes
use DefaultEnvironment), so delete that import to clean up unused dependencies
and satisfy linting.
In `@builder/relinker/examples/esp32s2/README.md`:
- Line 7: Change the phrase "Xtensa LX7 based single-core chip" to the
hyphenated form "Xtensa LX7-based single-core chip" in the README text so the
compound adjective is grammatically correct; locate the sentence mentioning "The
ESP32-S2 is an Xtensa LX7 based single-core chip..." and update it to use
"LX7-based".
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa3703dc-dbbb-41d7-b5d3-ff97b86858f8
⛔ Files ignored due to path filters (36)
builder/relinker/examples/arduino/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/object.csvis excluded by!**/*.csv
📒 Files selected for processing (13)
ARDUINO_RELINKER_INTEGRATION.mdbuilder/frameworks/arduino.pybuilder/frameworks/arduino_relinker.pybuilder/relinker/examples/arduino/README.mdbuilder/relinker/examples/esp32/README.mdbuilder/relinker/examples/esp32c3/README.mdbuilder/relinker/examples/esp32c6/README.mdbuilder/relinker/examples/esp32h2/README.mdbuilder/relinker/examples/esp32s2/README.mdbuilder/relinker/relinker.pyexamples/arduino-relinker/README.mdexamples/arduino-relinker/platformio.iniexamples/arduino-relinker/src/main.cpp
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
builder/frameworks/arduino_relinker.py (1)
17-17: Unused import:DefaultEnvironment.
DefaultEnvironmentis imported but never used in this module.🧹 Suggested fix
-from SCons.Script import DefaultEnvironment🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/arduino_relinker.py` at line 17, The import DefaultEnvironment in arduino_relinker.py is unused; remove the unused import from the top of the module (the "from SCons.Script import DefaultEnvironment" line) so the module only imports what it actually uses, or if DefaultEnvironment is intended to be used, update the code to reference DefaultEnvironment where needed (e.g., in functions or initialization) and ensure lint passes.builder/relinker/examples/esp32s2/README.md (1)
7-7: Minor grammar nit: use hyphen in compound modifier."Xtensa LX7 based" should be "Xtensa LX7-based" when used as a compound adjective.
📝 Suggested fix
-The ESP32-S2 is an Xtensa LX7 based single-core chip with 320 KB of SRAM. +The ESP32-S2 is an Xtensa LX7-based single-core chip with 320 KB of SRAM.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/relinker/examples/esp32s2/README.md` at line 7, Replace the incorrect compound modifier "Xtensa LX7 based" with the hyphenated form "Xtensa LX7-based" in the README sentence describing the ESP32-S2, i.e., update the phrase "The ESP32-S2 is an Xtensa LX7 based single-core chip..." to read "The ESP32-S2 is an Xtensa LX7-based single-core chip..." to fix the grammar.builder/relinker/examples/esp32/README.md (1)
7-7: Minor grammar nit: use hyphen in compound modifier."Xtensa LX6 based" should be "Xtensa LX6-based" when used as a compound adjective before "dual-core chip".
📝 Suggested fix
-The ESP32 is the original Xtensa LX6 based dual-core chip with 520 KB of SRAM. +The ESP32 is the original Xtensa LX6-based dual-core chip with 520 KB of SRAM.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/relinker/examples/esp32/README.md` at line 7, Replace the compound modifier "Xtensa LX6 based" in the README (the phrase "Xtensa LX6 based" describing the dual-core chip) with the hyphenated form "Xtensa LX6-based" so it reads "Xtensa LX6-based dual-core chip".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/arduino-relinker/README.md`:
- Around line 89-93: The README expected output contains a "Free PSRAM: 0 bytes"
line that doesn't match the code in main.cpp (the ESP.getFreePsram() call is
commented out because ESP32-C2 has no PSRAM); remove the "Free PSRAM: 0 bytes"
line from the example output in README.md so the output matches the actual
behavior, and adjust any surrounding whitespace or line breaks so the sample
remains well-formatted.
---
Nitpick comments:
In `@builder/frameworks/arduino_relinker.py`:
- Line 17: The import DefaultEnvironment in arduino_relinker.py is unused;
remove the unused import from the top of the module (the "from SCons.Script
import DefaultEnvironment" line) so the module only imports what it actually
uses, or if DefaultEnvironment is intended to be used, update the code to
reference DefaultEnvironment where needed (e.g., in functions or initialization)
and ensure lint passes.
In `@builder/relinker/examples/esp32/README.md`:
- Line 7: Replace the compound modifier "Xtensa LX6 based" in the README (the
phrase "Xtensa LX6 based" describing the dual-core chip) with the hyphenated
form "Xtensa LX6-based" so it reads "Xtensa LX6-based dual-core chip".
In `@builder/relinker/examples/esp32s2/README.md`:
- Line 7: Replace the incorrect compound modifier "Xtensa LX7 based" with the
hyphenated form "Xtensa LX7-based" in the README sentence describing the
ESP32-S2, i.e., update the phrase "The ESP32-S2 is an Xtensa LX7 based
single-core chip..." to read "The ESP32-S2 is an Xtensa LX7-based single-core
chip..." to fix the grammar.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cb127aa-f321-420d-a6d1-b45626a48c6a
⛔ Files ignored due to path filters (36)
builder/relinker/examples/arduino/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/arduino/esp32s3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c3/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32c6/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32h2/object.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/function.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/library.csvis excluded by!**/*.csvbuilder/relinker/examples/esp32s2/object.csvis excluded by!**/*.csv
📒 Files selected for processing (13)
ARDUINO_RELINKER_INTEGRATION.mdbuilder/frameworks/arduino.pybuilder/frameworks/arduino_relinker.pybuilder/relinker/examples/arduino/README.mdbuilder/relinker/examples/esp32/README.mdbuilder/relinker/examples/esp32c3/README.mdbuilder/relinker/examples/esp32c6/README.mdbuilder/relinker/examples/esp32h2/README.mdbuilder/relinker/examples/esp32s2/README.mdbuilder/relinker/relinker.pyexamples/arduino-relinker/README.mdexamples/arduino-relinker/platformio.iniexamples/arduino-relinker/src/main.cpp
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.
Checklist:
Summary by CodeRabbit
New Features
Documentation