feat: LP-Core ULP support for Arduino-only projects#425
feat: LP-Core ULP support for Arduino-only projects#425antoinecellerier wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds LP‑Core ULP coprocessor support for Arduino-only projects on ESP32‑C5/C6/P4 by auto-detecting Changes
Sequence Diagram(s)sequenceDiagram
participant ArduinoBuild as Arduino Build System
participant ULPConfig as ULP Auto-Config
participant FrameworkCheck as Framework/Library Validation
participant CMakeBuild as CMake/Ninja ULP Build
participant Assembly as Binary Embedding
participant MemoryPatch as Memory Layout Patching
participant SConsGraph as SCons Build Graph
ArduinoBuild->>ULPConfig: Detect `ulp/` directory with sources
ULPConfig->>ULPConfig: Match MCU (esp32c5/c6/p4)
ULPConfig->>FrameworkCheck: Inject sdkconfig & validate ESP‑IDF/libs
FrameworkCheck->>CMakeBuild: Provide sdkconfig.cmake and include paths
CMakeBuild->>CMakeBuild: Configure & compile ULP via CMake/Ninja
CMakeBuild->>Assembly: Produce ulp_main.bin
Assembly->>Assembly: Generate ulp_main.bin.S (embedded assembly)
Assembly->>MemoryPatch: Provide size for LP SRAM reservation
MemoryPatch->>SConsGraph: Emit patched memory.ld and UL P objects
SConsGraph->>ArduinoBuild: Integrate ulp_main.o into main build
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
|
@coderabbitai review |
✅ Actions performedReview 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: 3
🧹 Nitpick comments (3)
builder/frameworks/arduino.py (1)
1074-1076: Redundant Path construction.
_ulp_diris already defined at line 574 with the same value. Consider reusing it:♻️ Suggested refactor
# LP-Core ULP support for Arduino-only builds - if _has_ulp_sources(Path(env.subst("$PROJECT_DIR")) / "ulp"): + if _has_ulp_sources(_ulp_dir): SConscript("ulp_lp_core.py", exports="env")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/arduino.py` around lines 1074 - 1076, The code recomputes the ULP directory instead of reusing the previously defined _ulp_dir; update the conditional to call _has_ulp_sources(_ulp_dir) and pass SConscript("ulp_lp_core.py", exports="env") unchanged so you don't rebuild Path(env.subst("$PROJECT_DIR")) / "ulp"; locate the block using _has_ulp_sources and SConscript and replace the Path(...) expression with the existing _ulp_dir variable to remove redundancy.examples/arduino-ulp-blink/src/main.cpp (1)
14-14: Consider ULP variable naming convention (optional).In
ulp/blink.c, the variable is namedulp_led_state, which becomesulp_ulp_led_stateafter the ULP build system adds itsulp_prefix. The ESP-IDF convention is to name ULP variables without theulp_prefix (e.g.,led_state→ulp_led_state).This works correctly as-is, but aligning with the convention would avoid the double prefix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/arduino-ulp-blink/src/main.cpp` at line 14, The ULP variable in ulp/blink.c is named ulp_led_state which the ULP build system then prefixes again to produce ulp_ulp_led_state; rename the symbol in ulp/blink.c to led_state (or another name without an ulp_ prefix) so the build-time prefix produces the expected ulp_led_state; update any references to ulp_led_state in the C file (and any host-side references if present) to the new unprefixed name to avoid the double prefix.platform.py (1)
595-605: Unify LP-Core ULP detection with the toolchain gate.These branches and
builder/frameworks/ulp_lp_core.pynow use a project-root, source-based check, but_configure_mcu_toolchains()still installs the ULP toolchains offPath("ulp").is_dir(). That leaves package selection and toolchain selection on different rules. Please route all three call sites through one helper.Also applies to: 778-785
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform.py` around lines 595 - 605, Extract the ULP detection logic into a single helper (e.g., function named project_has_ulp_sources or uses_project_ulp) that encapsulates the current check: resolve ProjectConfig.get_instance().path parent/"ulp", ensure the directory exists, and return True if any file under it has suffix in (".c", ".S", ".s") using Path.rglob and is_file(); then replace the duplicated checks in platform.py (the block that sets self.packages["framework-espidf"]["optional"] = False and the other branch at lines ~778-785) and the call site in _configure_mcu_toolchains() to call this helper so package selection, builder/frameworks/ulp_lp_core.py usage, and toolchain installation all use the same project-root, source-based rule.
🤖 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/frameworks/arduino.py`:
- Around line 574-624: The ULP auto-config block has inconsistent indentation;
normalize all nested lines under the if that checks mcu in _lp_core_mcus and
_has_ulp_sources(_ulp_dir) to use 4-space indentation so Python block scoping is
correct. Update the entire block that manipulates _ulp_sdkconfig_entries,
entry_custom_sdkconfig, flag_custom_sdkconfig, _ulp_component_remove,
existing_removes/new_removes, flag_custom_component_remove, _ulp_lib_ignore,
existing_ignores/new_ignores, flag_lib_ignore and the final print to align with
4-space indentation and preserve the same logic and calls to config.set and
env.GetProjectOption.
In `@builder/frameworks/ulp_lp_core.py`:
- Around line 121-132: Update get_sdkconfig_value to prefer parsing the actual
SDKCONFIG header used by the ULP build (e.g., ulp/sdkconfig.h or the SDKCONFIG_H
selected by the build) before falling back to
env.GetProjectOption("custom_sdkconfig") or the hard-coded default: if that file
exists, read it and extract the requested key (such as
CONFIG_ULP_COPROC_RESERVE_MEM) and return its integer value; apply the same
change to the other call sites that compute ULP_RESERVE_MEM (references:
get_sdkconfig_value and the code that reads
CONFIG_ULP_COPROC_RESERVE_MEM/ULP_RESERVE_MEM in this module) so the reserved
SRAM comes from the same SDKCONFIG_H the ULP build uses.
In `@examples/arduino-ulp-blink/README.md`:
- Around line 31-38: The fenced code block showing runtime output ("Starting ULP
blink program..." through "ULP led_state: 1") needs a language specifier; edit
the README.md fenced block containing that output and change the opening fence
from ``` to ```text (or ```console) so syntax highlighters treat it as plain
text/console output.
---
Nitpick comments:
In `@builder/frameworks/arduino.py`:
- Around line 1074-1076: The code recomputes the ULP directory instead of
reusing the previously defined _ulp_dir; update the conditional to call
_has_ulp_sources(_ulp_dir) and pass SConscript("ulp_lp_core.py", exports="env")
unchanged so you don't rebuild Path(env.subst("$PROJECT_DIR")) / "ulp"; locate
the block using _has_ulp_sources and SConscript and replace the Path(...)
expression with the existing _ulp_dir variable to remove redundancy.
In `@examples/arduino-ulp-blink/src/main.cpp`:
- Line 14: The ULP variable in ulp/blink.c is named ulp_led_state which the ULP
build system then prefixes again to produce ulp_ulp_led_state; rename the symbol
in ulp/blink.c to led_state (or another name without an ulp_ prefix) so the
build-time prefix produces the expected ulp_led_state; update any references to
ulp_led_state in the C file (and any host-side references if present) to the new
unprefixed name to avoid the double prefix.
In `@platform.py`:
- Around line 595-605: Extract the ULP detection logic into a single helper
(e.g., function named project_has_ulp_sources or uses_project_ulp) that
encapsulates the current check: resolve ProjectConfig.get_instance().path
parent/"ulp", ensure the directory exists, and return True if any file under it
has suffix in (".c", ".S", ".s") using Path.rglob and is_file(); then replace
the duplicated checks in platform.py (the block that sets
self.packages["framework-espidf"]["optional"] = False and the other branch at
lines ~778-785) and the call site in _configure_mcu_toolchains() to call this
helper so package selection, builder/frameworks/ulp_lp_core.py usage, and
toolchain installation all use the same project-root, source-based rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1783da6e-0661-494d-bb75-36c70222e438
📒 Files selected for processing (10)
README.mdbuilder/frameworks/arduino.pybuilder/frameworks/espidf.pybuilder/frameworks/ulp_lp_core.pyexamples/arduino-ulp-blink/.gitignoreexamples/arduino-ulp-blink/README.mdexamples/arduino-ulp-blink/platformio.iniexamples/arduino-ulp-blink/src/main.cppexamples/arduino-ulp-blink/ulp/blink.cplatform.py
Add native LP-Core ULP coprocessor support for Arduino framework builds on ESP32-C5, ESP32-C6, and ESP32-P4. Users only need a ulp/ directory with C or assembly sources — the platform handles everything else. Build pipeline (ulp_lp_core.py): - Delegates to IDF's own ULP CMake modules for compilation - Generates ulp_main.h (symbol map) and ulp_main.bin.S (binary embedding) - Patches memory.ld to reserve LP SRAM for the ULP binary - Links libulp.a for ulp_lp_core_load_binary / ulp_lp_core_run APIs - Uses recompiled sdkconfig.h from libs, or user-provided ulp/sdkconfig.h Auto-configuration (arduino.py): - Detects ulp/ directory on LP-Core MCUs and injects custom_sdkconfig entries (CONFIG_ULP_COPROC_ENABLED, TYPE_LP_CORE, RESERVE_MEM=8192) - Removes components that break lib recompilation (esp_insights, etc.) - Sets lib_ignore for their Arduino wrapper libraries Lib recompilation fixes (espidf.py, arduino.py): - Copy sections.ld during recompilation (fixes orphan section errors) - Check chip-specific sdkconfig.orig instead of root sdkconfig to prevent cross-env flip-flopping between different MCU environments - ProcessUnFlags on saved build_flags to prevent user defines from leaking into lib recompilation Package management (platform.py): - Force-install framework-espidf when ulp/ sources detected - Install CMake and ninja for Arduino-only ULP builds Includes arduino-ulp-blink example project for ESP32-C6. Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
antoinecellerier
force-pushed
the
feature/lp-core-ulp-arduino
branch
from
b60479b to
4f084d9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
builder/frameworks/ulp_lp_core.py (2)
129-151: Fallback chain is correct; consider narrowing exception types.The priority order (SDKCONFIG_H → custom_sdkconfig → default) correctly addresses the past review concern. The broad
Exceptioncatches work but could be tightened to(IOError, ValueError, IndexError)for more precise error handling—though the current approach is pragmatically acceptable for a build script.♻️ Optional: Narrow exception types
try: for line in SDKCONFIG_H.read_text().splitlines(): line = line.strip() if line.startswith("#define %s " % key): return int(line.split(None, 2)[2]) - except Exception: + except (IOError, ValueError, IndexError): pass # Fall back to platformio.ini custom_sdkconfig (Kconfig-style key=value) try: custom = env.GetProjectOption("custom_sdkconfig", "") for line in custom.splitlines(): line = line.strip() if "://" in line: continue if line.startswith(key + "="): return int(line.split("=", 1)[1]) - except Exception: + except (ValueError, IndexError): pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/ulp_lp_core.py` around lines 129 - 151, The try/except blocks in get_sdkconfig_value are too broad; replace the generic except Exception handlers with narrower exception tuples to avoid masking unrelated errors: catch (IOError, OSError, ValueError, IndexError) when reading/parsing SDKCONFIG_H and when parsing env.GetProjectOption("custom_sdkconfig") (or the appropriate subset like (IOError, ValueError, IndexError) if OSError isn’t needed), keeping the same fallback logic and return default otherwise.
373-408: Consider warning if memory.ld is missing.The silent return on line 378 when
memory.lddoesn't exist could mask configuration issues. Since the libs validation already passed, a missingmemory.ldwould be unexpected.♻️ Optional: Add warning for missing memory.ld
if not src_ld.exists(): + print("Warning: %s not found — LP SRAM patching skipped" % src_ld) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@builder/frameworks/ulp_lp_core.py` around lines 373 - 408, In patch_memory_ld, do not silently return when the expected memory.ld (src_ld) is missing: detect the missing file (src_ld.exists() is False) and emit a WARNING or stderr message that includes the path (src_ld) and context (e.g., that LP-Core memory reservation will be skipped) before returning; keep the early return but add the log to aid debugging and reference ULP_RESERVE_MEM, FW_LIBS_DIR, and the function name patch_memory_ld so maintainers can find and verify the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@builder/frameworks/ulp_lp_core.py`:
- Around line 129-151: The try/except blocks in get_sdkconfig_value are too
broad; replace the generic except Exception handlers with narrower exception
tuples to avoid masking unrelated errors: catch (IOError, OSError, ValueError,
IndexError) when reading/parsing SDKCONFIG_H and when parsing
env.GetProjectOption("custom_sdkconfig") (or the appropriate subset like
(IOError, ValueError, IndexError) if OSError isn’t needed), keeping the same
fallback logic and return default otherwise.
- Around line 373-408: In patch_memory_ld, do not silently return when the
expected memory.ld (src_ld) is missing: detect the missing file (src_ld.exists()
is False) and emit a WARNING or stderr message that includes the path (src_ld)
and context (e.g., that LP-Core memory reservation will be skipped) before
returning; keep the early return but add the log to aid debugging and reference
ULP_RESERVE_MEM, FW_LIBS_DIR, and the function name patch_memory_ld so
maintainers can find and verify the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bc583a6-9907-4147-a95d-77e746745bf9
📒 Files selected for processing (10)
README.mdbuilder/frameworks/arduino.pybuilder/frameworks/espidf.pybuilder/frameworks/ulp_lp_core.pyexamples/arduino-ulp-blink/.gitignoreexamples/arduino-ulp-blink/README.mdexamples/arduino-ulp-blink/platformio.iniexamples/arduino-ulp-blink/src/main.cppexamples/arduino-ulp-blink/ulp/blink.cplatform.py
🚧 Files skipped from review as they are similar to previous changes (5)
- platform.py
- examples/arduino-ulp-blink/.gitignore
- README.md
- builder/frameworks/espidf.py
- examples/arduino-ulp-blink/platformio.ini
|
@antoinecellerier First thank you for providing a PR to pioarduino. But i will not merge. Honestly i dont get the benefit, since the PR needs and uses HybridCompile and injects there the needed changes for ULP compiling. So it is not a simple Arduino compile run. HybridCompile is already complex. Adding another "thing" makes maintain work really hard. Overall to much complex code for supporting ULP. For using ULP with Arduino there is already the variant "compile Arduino as a component of IDF". There are no changes at all in the platform needed. See example https://github.com/pioarduino/platform-espressif32/tree/main/examples/espidf-arduino-C6-ULP-blink |
|
Fair enough. Thanks for taking time to provide feedback. The intent was to provide an easier out of the box experience for people wanting to get going with ULP but I do understand the maintainability concerns. |
- Replace 23 hardcoded COMPONENT_INCLUDES paths with discover_component_includes() that scans framework-arduinoespressif32-libs for include/register/mcu dirs - Replace patch_memory_ld() with validate_memory_ld() since lib recompile already produces correct linker script via CONFIG_ULP_COPROC_ENABLED - Remove unused 'import re' Addresses PR pioarduino#425 feedback about fragile hardcoded paths and patching. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Hi @Jason2866, thanks again for the detailed feedback. I've been iterating on the branch to address the specific concerns you raised: COMPONENT_INCLUDES fragility — Replaced the 23 hardcoded paths with discover_component_includes() which auto-scans the libs package for include/, register/, and MCU-named directories. No longer breaks when IDF reorganizes headers. patch_memory_ld() — Eliminated entirely. Since the lib recompile already produces a correct memory.ld (IDF's template uses #if CONFIG_ULP_COPROC_ENABLED), we just validate the offset exists instead of patching. Hardcoded component excludes in arduino.py — These use the same custom_component_remove mechanism the existing C6-ULP-blink example uses. The same set of components are excluded there — they're unrelated to ULP itself, but needed for HybridCompile's lib recompilation to succeed. I understand the concern about maintainability and added complexity to HybridCompile. The motivation is the user experience gap: the existing espidf-arduino-C6-ULP-blink example requires users to write 3 boilerplate files (CMakeLists.txt, src/CMakeLists.txt, sdkconfig.defaults) and know about framework = arduino, espidf. With this PR, users just drop .c files in ulp/ and build — zero config. The branch is at https://github.com/antoinecellerier/platform-espressif32/tree/feature/lp-core-ulp-arduino with the improvements if you'd like to take another look. Totally fine if not — no reply expected. Thanks for maintaining pioarduino! |
|
@antoinecellerier The newest version is much better! Still some stuff left which needs to be discussed / worked on. There is a chance for a merge ;-) EDIT: Why is not the existing ulp script used? Code seems duplicated. |
|
Thanks for the second look @Jason2866 ! I've pushed changes addressing each point: Renamed to arduino_ulp.py — clearly distinct from the espidf ulp.py flow. TBD if that name is slightly misleading as it only addresses MCUs with LP-Core co-processors. Single source of truth for MCU list — added builder/frameworks/ulp_configs.py with LP_CORE_MCUS and ULP_SOURCE_SUFFIXES. Imported by arduino_ulp.py, arduino.py, and platform.py — no duplication. PR base — the branch is based on develop already. Note it seems I cannot change the target branch atm as the PR is closed. Should you re-open it that should be a trivial change. Why not reuse ulp.py — ulp.py depends on Import("env sdk_config project_config app_includes idf_variant"), all provided by the full IDF CMake build in espidf.py. In Arduino-only builds those variables don't exist. Synthesizing them would need ~200 lines of fragile glue to fake the IDF CMake codemodel output, which defeats the purpose. The actual overlap is just 3 cmake invocations — straightforward boilerplate not worth extracting into a shared module. |
2 participants
Summary
ulp/directory with C or assembly sources — the platform handles sdkconfig injection, lib recompilation, ULP compilation, binary embedding, and linker script patching automaticallyarduino-ulp-blinkexample projectHow it works
arduino.py): detectsulp/on LP-Core MCUs, injectscustom_sdkconfigentries to enable ULP in lib recompilation, removes components that break the recompileulp_lp_core.py): delegates to IDF's own ULP CMake modules — cmake configure → cmake build → assembly embedding viadata_file_embed_asm.cmakeespidf.py,arduino.py): copiessections.ldduring recompilation, prevents cross-env flip-flopping, fixesbuild_flagsleakage into lib recompilationplatform.py): force-installsframework-espidf, CMake, and ninja whenulp/sources are detectedAlternative approach: SCons-native builder
An earlier version of this feature used a fully SCons-native approach (no CMake delegation) and is preserved on the
feature/lp-core-ulp-scons-nativebranch.ulp_lp_core.pyThe CMake approach was chosen for long-term maintainability — IDF's ULP build system is a moving target and delegating to their modules avoids fragile flag/path mirroring.
Test plan
ulp/directory — verify one-time lib recompilation + ULP binary embeddedulp/directory on LP-Core MCU — verify no side effectsulp/directory on non-LP-Core MCU (e.g. ESP32-S3) — verify silent skipUse of AI
This was built using Opus 4.6, GPT 5.3-Code and Gemini 3 Pro on GitHub Copilot Cli & Claude Code. I do not have deep understanding of ESP-IDF, Arduino or PlatformIO code bases but the changes seem to make sense to me and the code looks reasonable. Please do let me know if it's garbage.
I've tested this on a Seeed Xiao ESP32C6 device both with the example project and a personal project using timer based ULP wake-up from deep sleep, and have validated with a Nordic Semiconductor PPK2 that ULP was working as expected.
Summary by CodeRabbit
New Features
Documentation