Make idf compile possible for MCUs which have no precompiled Arduino libraries #353
+13
−9
Conversation
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
Build bootloader even when custom_sdkconfig is set. Use set frameworks as condition to check
check platform entry in platformio.ini If there is espidf it is not HybridCompile
WalkthroughReads the project's framework option into Changes
Sequence Diagram(s)sequenceDiagram
participant Env as Project env
participant Platform as platform.py
participant Builder as builder/frameworks/espidf.py
participant FS as Filesystem
Env->>Builder: GetProjectOption("framework") -> pio_orig_frwrk
note right of Builder `#DDEBF7`: pio_orig_frwrk used to control flows
alt pio_orig_frwrk contains "espidf"
Builder->>Builder: set flag_custom_sdkconfig = False
note right of Builder `#FCE9D5`: disable custom sdkconfig handling
else
Builder->>Builder: leave flag_custom_sdkconfig as-is
end
Platform->>Platform: _configure_arduino_framework(frameworks, mcu)
Platform->>Builder: pass mcu context for decisions
Builder->>Builder: check MCU type (mcu)
alt mcu is esp32c2 or esp32c61
alt pio_orig_frwrk contains "espidf"
Builder-->>FS: skip copying Arduino skeleton libs
else
Builder-->>FS: copy Arduino skeleton libs
end
else
Builder-->>FS: existing copy/merge behavior
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-09-23T14:15:00.476ZApplied to files:
📚 Learning: 2025-08-20T14:13:13.842ZApplied to files:
📚 Learning: 2025-09-23T14:15:00.476ZApplied to files:
📚 Learning: 2025-11-14T14:46:51.257ZApplied to files:
📚 Learning: 2025-08-10T19:12:17.988ZApplied to files:
📚 Learning: 2025-09-09T18:07:14.583ZApplied to files:
📚 Learning: 2025-09-23T16:55:39.788ZApplied to files:
📚 Learning: 2025-09-05T11:58:49.681ZApplied to files:
📚 Learning: 2025-09-04T15:27:18.112ZApplied to files:
📚 Learning: 2025-07-13T20:03:29.695ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (5)
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: 0
🧹 Nitpick comments (1)
builder/frameworks/espidf.py (1)
236-241: Consider removing debug print statements and adding error handling.The implementation achieves the stated goal of disabling HybridCompile for pure ESP-IDF projects, but has a few refinements needed:
- Debug output: Lines 237 and 241 appear to be debug print statements that should be removed or gated behind the verbose flag check used elsewhere in the file.
- Exception handling:
GetProjectOption("framework")could raise an exception if the framework option is not set in the project configuration.- String comparison: The substring check
"espidf" in pio_orig_frwrkis case-sensitive and could theoretically match unintended strings, though this is unlikely in practice.Apply this diff to address these concerns:
-pio_orig_frwrk = env.GetProjectOption("framework") -print("***** Framework read from Project Config:", pio_orig_frwrk) - -if "espidf" in pio_orig_frwrk: +try: + pio_orig_frwrk = env.GetProjectOption("framework") + if int(ARGUMENTS.get("PIOVERBOSE", 0)): + print("Framework read from Project Config:", pio_orig_frwrk) +except Exception: + pio_orig_frwrk = "" + +if "espidf" in pio_orig_frwrk.lower(): flag_custom_sdkonfig = False - print("**** HybridCompile switched off ****") + if int(ARGUMENTS.get("PIOVERBOSE", 0)): + print("HybridCompile disabled for ESP-IDF-only framework")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
builder/frameworks/espidf.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 268
File: builder/penv_setup.py:87-96
Timestamp: 2025-08-26T14:52:29.044Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to trust system executables when falling back to PATH-based command resolution (like "uv"), rather than implementing additional validation checks for security concerns.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-10-19T17:30:42.403Z
Learning: The tool-esp-rom-elfs package must remain in COMMON_IDF_PACKAGES in platform.py because espidf.py requires it to set the ESP_ROM_ELF_DIR environment variable for the ESP-IDF build system, independent of the exception decoder monitor filter usage.
📚 Learning: 2025-09-23T14:15:00.476Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates and manages its own separate IDF virtual environment (accessed via get_python_exe() from get_idf_venv_dir()). This IDF venv is completely isolated from the penv setup, so different cryptography versions between penv_setup.py and espidf.py don't cause conflicts as they exist in separate virtual environments.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-23T14:15:00.476Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates an extra/separate virtual environment from the penv setup, so cryptography version differences between penv_setup.py and espidf.py do not cause conflicts since they operate in isolated environments.
Applied to files:
builder/frameworks/espidf.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (macos-15, examples/espidf-ulp-lp)
- GitHub Check: build (macos-15, examples/espidf-ulp)
- GitHub Check: build (macos-15, examples/arduino-blink)
- GitHub Check: build (windows-latest, examples/tasmota)
- GitHub Check: build (windows-latest, examples/espidf-arduino-blink)
- GitHub Check: build (windows-latest, examples/arduino-blink)
Remove print statements related to framework configuration.
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
builder/frameworks/espidf.py (2)
185-185: Use more Pythonic membership test.The condition
not ("espidf" in pio_orig_frwrk)should be rewritten as"espidf" not in pio_orig_frwrkfor better readability and to follow Python style conventions.Apply this diff:
- if mcu == "esp32c2" and not ("espidf" in pio_orig_frwrk): + if mcu == "esp32c2" and "espidf" not in pio_orig_frwrk:
196-196: Use more Pythonic membership test.The condition
not ("espidf" in pio_orig_frwrk)should be rewritten as"espidf" not in pio_orig_frwrkfor better readability and to follow Python style conventions.Apply this diff:
- if mcu == "esp32c61" and not ("espidf" in pio_orig_frwrk): + if mcu == "esp32c61" and "espidf" not in pio_orig_frwrk:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
builder/frameworks/espidf.py(4 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 268
File: builder/penv_setup.py:87-96
Timestamp: 2025-08-26T14:52:29.044Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to trust system executables when falling back to PATH-based command resolution (like "uv"), rather than implementing additional validation checks for security concerns.
📚 Learning: 2025-08-20T14:13:13.842Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-23T14:15:00.476Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates an extra/separate virtual environment from the penv setup, so cryptography version differences between penv_setup.py and espidf.py do not cause conflicts since they operate in isolated environments.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-23T14:15:00.476Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T14:15:00.476Z
Learning: In the pioarduino/platform-espressif32 project, espidf.py creates and manages its own separate IDF virtual environment (accessed via get_python_exe() from get_idf_venv_dir()). This IDF venv is completely isolated from the penv setup, so different cryptography versions between penv_setup.py and espidf.py don't cause conflicts as they exist in separate virtual environments.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-09T18:07:14.583Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 285
File: platform.py:670-683
Timestamp: 2025-09-09T18:07:14.583Z
Learning: In the pioarduino/platform-espressif32 project, when tools.json exists in mklittlefs tool directories, it triggers a reinstall via install_tool(). This is intentional behavior to convert tools installed via idf_tools.py method into the platform's tool management system, rather than a redundant check that should be avoided.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-08-08T09:50:01.969Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 252
File: builder/frameworks/espidf.py:1382-1388
Timestamp: 2025-08-08T09:50:01.969Z
Learning: Preference in pioarduino/platform-espressif32 (builder/frameworks/espidf.py): Avoid using {"name": "boot"} as a sentinel when resolving the default app partition offset. Instead, call parttool.py with --partition-boot-default directly (and optionally fall back to factory/ota_0) to prevent confusion with non-existent CSV names.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-10-19T17:30:42.403Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-10-19T17:30:42.403Z
Learning: The tool-esp-rom-elfs package must remain in COMMON_IDF_PACKAGES in platform.py because espidf.py requires it to set the ESP_ROM_ELF_DIR environment variable for the ESP-IDF build system, independent of the exception decoder monitor filter usage.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-04T15:27:18.112Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-08-10T19:12:17.988Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 255
File: builder/penv_setup.py:19-19
Timestamp: 2025-08-10T19:12:17.988Z
Learning: In PlatformIO ESP32 platform builds, `semantic_version` is a dependency provided by PlatformIO itself and is guaranteed to be available when the build scripts run. The module is already imported at the top level in existing files like `builder/frameworks/espidf.py`, so lazy-loading it is unnecessary.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-23T16:55:39.788Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 296
File: builder/penv_setup.py:612-647
Timestamp: 2025-09-23T16:55:39.788Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers no fallback mechanisms in penv setup functions. The penv virtual environment setup must work properly and should crash immediately rather than falling back to host Python environments, to avoid using "unknown env" configurations.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-09-05T11:58:49.681Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/ulp.py:165-178
Timestamp: 2025-09-05T11:58:49.681Z
Learning: In the pioarduino/platform-espressif32 project, cmake executable paths in build scripts (like ulp.py) are provided as paths to the build engine (SCons/PlatformIO) rather than direct executable calls. The build engine handles cross-platform executable resolution automatically, so there's no need to add Windows .exe suffixes or special quoting - the current approach with str(Path(...) / "bin" / "cmake") works correctly across platforms.
Applied to files:
builder/frameworks/espidf.py
📚 Learning: 2025-11-14T14:46:51.257Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 338
File: builder/frameworks/component_manager.py:1312-1380
Timestamp: 2025-11-14T14:46:51.257Z
Learning: In the pioarduino/platform-espressif32 project, the `pioarduino-build.py` file has a fixed format that does not change, so exact string matching (e.g., `'CCFLAGS=['`) is acceptable and regex-based flexible matching is not necessary when modifying this file.
Applied to files:
builder/frameworks/espidf.py
🪛 Ruff (0.14.5)
builder/frameworks/espidf.py
185-185: Test for membership should be not in
Convert to not in
(E713)
196-196: Test for membership should be not in
Convert to not in
(E713)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: build (macos-15, examples/espidf-arduino-blink)
- GitHub Check: build (windows-latest, examples/arduino-zigbee-switch)
- GitHub Check: build (windows-latest, examples/espidf-storage-sdcard)
- GitHub Check: build (windows-latest, examples/espidf-http-request)
- GitHub Check: build (windows-latest, examples/espidf-hello-world)
- GitHub Check: build (windows-latest, examples/espidf-exceptions)
- GitHub Check: build (windows-latest, examples/arduino-usb-keyboard)
- GitHub Check: build (windows-latest, examples/espidf-blink)
- GitHub Check: build (windows-latest, examples/arduino-zigbee-light)
- GitHub Check: build (windows-latest, examples/tasmota)
- GitHub Check: build (windows-latest, examples/arduino-wifiscan)
- GitHub Check: build (windows-latest, examples/arduino-rmt-blink)
- GitHub Check: build (ubuntu-latest, examples/espidf-arduino-blink)
- GitHub Check: build (windows-latest, examples/espidf-arduino-blink)
- GitHub Check: build (windows-latest, examples/arduino-matter-light)
- GitHub Check: build (ubuntu-latest, examples/espidf-ulp-riscv)
- GitHub Check: build (ubuntu-latest, examples/espidf-ulp-lp)
- GitHub Check: build (ubuntu-latest, examples/espidf-hello-world)
- GitHub Check: build (ubuntu-latest, examples/espidf-ulp)
- GitHub Check: build (ubuntu-latest, examples/arduino-wifiscan)
🔇 Additional comments (1)
builder/frameworks/espidf.py (1)
237-240: Verify the logic correctly disables Arduino IDF settings for pure ESP-IDF projects.The code disables custom sdkconfig handling when "espidf" appears in the framework string. This achieves the PR goal of allowing IDF compilation without Arduino-specific processing. The logic will affect:
- Pure
framework = espidfprojects- Hybrid
framework = arduino, espidfprojectsCombined with the guard at line 799 (which requires
arduinoin framework ANDespidfnot in framework), this ensuresHandleArduinoIDFsettingsis only called for pure Arduino framework builds, which is correct.
Added 'mcu' parameter to the '_configure_arduino_framework' method.
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
without the need to use a extra boards manifest for idf
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.