Skip to content

Conversation

@Jason2866
Copy link

@Jason2866 Jason2866 commented Nov 17, 2025

Description:

needs PR espressif/arduino-esp32#12036 merged first

Checklist:

  • The pull request is done against the latest develop branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR, more changes are allowed when changing boards.json
  • I accept the CLA

Summary by CodeRabbit

  • New Features

    • Added board variant configuration support for variant-specific builds and framework-aware library path resolution.
  • Updates

    • Updated three ESP32-P4 board names to indicate engineering-sample (ES) pre rev.300 status.
    • Added a new Espressif ESP32-P4 rev.300 generic board definition.

@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Added chip_variant support: board JSONs now include build.chip_variant and names updated; builder frameworks (arduino, component_manager, espidf) were updated to derive and propagate chip_variant and to use it for SDK and library path resolution.

Changes

Cohort / File(s) Change Summary
Board JSON updates
boards/esp32-p4-evboard.json, boards/esp32-p4.json, boards/m5stack-tab5-p4.json
Added build.chip_variant: "esp32p4_es" and updated name strings to indicate ES / pre-rev.300 variants.
New board definition
boards/esp32-p4_r3.json
Added a new PlatformIO board JSON for Espressif ESP32-P4 rev.300 generic with build, connectivity, debug, frameworks, and upload metadata.
Arduino framework changes
builder/frameworks/arduino.py
PathCache constructor extended to accept chip_variant; lowered default threshold for esp32c2; MCU values lowercased in threshold info; SDK include path computation switched to use chip_variant.
Component manager
builder/frameworks/component_manager.py
Read build.chip_variant (fallback to MCU), stored internally, and used chip_variant when resolving MCU-specific Arduino libraries paths.
ESP-IDF framework
builder/frameworks/espidf.py
Derived chip_variant from board config (fallback to MCU) and replaced MCU-based paths with chip_variant for Arduino ESP-IDF integration, SDK/lib paths, and sdkconfig file operations.

Sequence Diagram(s)

sequenceDiagram
  participant Board as Board JSON
  participant CM as component_manager
  participant AR as arduino.PathCache
  participant IDF as espidf

  Note over Board: Board contains build.mcu and optional build.chip_variant
  Board->>CM: provide board config
  CM->>CM: chip_variant = build.chip_variant or mcu
  CM->>AR: instantiate PathCache(platform, mcu, chip_variant)
  AR->>AR: use chip_variant to compute SDK/include paths
  CM->>IDF: pass chip_variant
  IDF->>IDF: use chip_variant for Arduino-IDF lib/sdk and sdkconfig paths
  Note over AR,IDF: All MCU-specific paths resolved using chip_variant
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Check every PathCache instantiation sites updated to pass the new chip_variant argument in arduino.py.
  • Verify consistent fallback logic (board.build.chip_variant → lowercase → mcu) across component_manager.py, arduino.py, and espidf.py.
  • Validate path computations that were switched from mcu to chip_variant, especially sdkconfig copy/move and Arduino libs directories.
  • Review new boards/esp32-p4_r3.json for accurate fields (upload/flash sizes, debug config).

Poem

🐰 A tiny hop, a variant named,
Paths rebloom where chips are framed,
SDKs now know which leaf to wear,
Builders bustle, tidy and fair,
Hoppity—builds run with care!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for ESP32-P4 revision 300 by introducing chip_variant configuration across multiple board definitions and framework files.

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 and usage tips.

@Jason2866
Copy link
Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

✅ Actions performed

Full review triggered.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

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

76-76: ESP32-C2 threshold adjusted to 31600.

The threshold reduction from 32000 to 31600 is a minor adjustment. Consider documenting the reason for this specific value in a comment or commit message to help future maintainers understand the rationale.


288-288: Duplicate chip_variant derivation in PathCache.

The chip_variant is derived twice with identical logic:

  1. In PathCache.__init__ (lines 291-293)
  2. At module level (lines 526-527)

Additionally, PathCache.__init__ receives chip_variant as a parameter but then re-derives it from env.BoardConfig(), ignoring the parameter.

Consider one of these approaches:

  1. Remove parameter and derive once inside __init__:
-    def __init__(self, platform, mcu, chip_variant):
+    def __init__(self, platform, mcu):
         self.platform = platform
         self.mcu = mcu
         chip_variant = env.BoardConfig().get("build.chip_variant", "").lower()
         chip_variant = chip_variant if chip_variant else mcu
         self.chip_variant = chip_variant
  1. Use the parameter directly:
     def __init__(self, platform, mcu, chip_variant):
         self.platform = platform
         self.mcu = mcu
-        chip_variant = env.BoardConfig().get("build.chip_variant", "").lower()
-        chip_variant = chip_variant if chip_variant else mcu
         self.chip_variant = chip_variant

Option 2 is cleaner if the caller always derives chip_variant correctly before instantiation.

Also applies to: 291-293, 316-316

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1316914 and 0cd337d.

📒 Files selected for processing (6)
  • boards/esp32-p4-evboard.json (2 hunks)
  • boards/esp32-p4.json (2 hunks)
  • boards/m5stack-tab5-p4.json (2 hunks)
  • builder/frameworks/arduino.py (6 hunks)
  • builder/frameworks/component_manager.py (2 hunks)
  • builder/frameworks/espidf.py (6 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 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: 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: 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-10-16T20:34:54.300Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 316
File: boards/esp32-p4.json:9-9
Timestamp: 2025-10-16T20:34:54.300Z
Learning: ESP32-P4 supports PSRAM frequencies of 20MHz, 40MHz, 80MHz, 120MHz, and 200MHz. The 200MHz frequency is valid and supported in the platform builder code.

Applied to files:

  • boards/esp32-p4-evboard.json
  • builder/frameworks/arduino.py
📚 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/arduino.py
  • 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/arduino.py
  • 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/arduino.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/arduino.py
📚 Learning: 2025-08-05T11:47:41.756Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 246
File: builder/main.py:0-0
Timestamp: 2025-08-05T11:47:41.756Z
Learning: In the ESP32 platform builder, subprocess calls correctly use the virtual environment Python by explicitly passing PYTHON_EXE (which is set to the venv python path) as the first argument in the command array, rather than relying on environment variable propagation. The env=os.environ parameter only affects environment variables, not the executable path itself.

Applied to files:

  • builder/frameworks/arduino.py
  • 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/arduino.py
  • 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-07-13T20:03:29.695Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 225
File: builder/main.py:99-108
Timestamp: 2025-07-13T20:03:29.695Z
Learning: In the ESP32 platform builder, the subprocess call to get site-packages directory in setup_python_paths() function is intentionally left without error handling because if this basic Python operation fails, it indicates a fundamental system issue that would cause the entire build process to fail anyway.

Applied to files:

  • builder/frameworks/espidf.py
🔇 Additional comments (10)
boards/m5stack-tab5-p4.json (1)

14-14: LGTM! Chip variant support added correctly.

The addition of chip_variant field and the updated board name clearly indicate support for ESP32-P4 ES (engineering sample) pre-rev.300 silicon. This aligns with the broader PR objective to enable variant-specific path resolution.

Also applies to: 31-31

builder/frameworks/component_manager.py (2)

48-49: LGTM! Chip variant fallback logic is correct.

The chip_variant initialization correctly falls back to mcu when not specified, ensuring backward compatibility while supporting new variant-specific configurations.


79-79: Path resolution updated to use chip_variant.

The change from self.mcu to self.chip_variant enables variant-specific MCU library paths. The property name arduino_libs_mcu remains unchanged for API stability, which is reasonable.

boards/esp32-p4.json (1)

12-12: LGTM! Consistent chip variant implementation.

The chip variant configuration matches the pattern used in other ESP32-P4 board definitions, with clear naming to indicate ES (engineering sample) silicon.

Also applies to: 26-26

boards/esp32-p4-evboard.json (1)

12-12: LGTM! Board definition updated consistently.

The chip variant addition and name update follow the same pattern as the other ESP32-P4 board definitions in this PR.

Also applies to: 29-29

builder/frameworks/espidf.py (3)

80-81: LGTM! Chip variant initialization with proper fallback.

The chip_variant derivation correctly falls back to mcu when not specified, maintaining backward compatibility.


185-185: Arduino C2 paths correctly updated for chip variant support.

The C2-specific library paths now use chip_variant instead of mcu, enabling proper variant-specific directory resolution for ESP32-C2 flows.

Also applies to: 192-192, 194-194


2605-2607: Arduino IDF library paths updated for chip variant support.

All library destination paths (lib_dst, ld_dst, mem_var) and sdkconfig handling now correctly use chip_variant, enabling the Arduino-as-IDF-component flow to work with variant-specific libraries.

Also applies to: 2630-2632

builder/frameworks/arduino.py (2)

239-239: LGTM! MCU lowercased for consistency.

Applying .lower() ensures consistent MCU keys in the threshold info payload.


526-527: LGTM! Chip variant integrated at module level.

The chip_variant is correctly derived and passed to PathCache, though see the comment on PathCache regarding duplicate derivation.

Also applies to: 530-530

@Jason2866
Copy link
Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

✅ Actions performed

Full review triggered.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1316914 and 716f579.

📒 Files selected for processing (7)
  • boards/esp32-p4-evboard.json (2 hunks)
  • boards/esp32-p4.json (2 hunks)
  • boards/esp32-p4_r3.json (1 hunks)
  • boards/m5stack-tab5-p4.json (2 hunks)
  • builder/frameworks/arduino.py (6 hunks)
  • builder/frameworks/component_manager.py (2 hunks)
  • builder/frameworks/espidf.py (6 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 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: 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: 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: 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.
📚 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/arduino.py
📚 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/arduino.py
  • boards/esp32-p4_r3.json
  • 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/arduino.py
📚 Learning: 2025-08-08T09:55:50.842Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 252
File: builder/frameworks/espidf.py:1382-1388
Timestamp: 2025-08-08T09:55:50.842Z
Learning: In pioarduino/platform-espressif32 (builder/frameworks/espidf.py, Python), for get_app_partition_offset: preferred behavior is to probe app/ota_0 first, then app/factory; avoid using the {"name":"boot"} sentinel; make probes non-fatal (don’t exit on missing partitions), optionally fall back to calling parttool.py with --partition-boot-default, then default to 0x10000.

Applied to files:

  • builder/frameworks/arduino.py
📚 Learning: 2025-10-16T20:34:54.300Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 316
File: boards/esp32-p4.json:9-9
Timestamp: 2025-10-16T20:34:54.300Z
Learning: ESP32-P4 supports PSRAM frequencies of 20MHz, 40MHz, 80MHz, 120MHz, and 200MHz. The 200MHz frequency is valid and supported in the platform builder code.

Applied to files:

  • builder/frameworks/arduino.py
  • boards/esp32-p4_r3.json
  • boards/esp32-p4-evboard.json
📚 Learning: 2025-08-05T11:47:41.756Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 246
File: builder/main.py:0-0
Timestamp: 2025-08-05T11:47:41.756Z
Learning: In the ESP32 platform builder, subprocess calls correctly use the virtual environment Python by explicitly passing PYTHON_EXE (which is set to the venv python path) as the first argument in the command array, rather than relying on environment variable propagation. The env=os.environ parameter only affects environment variables, not the executable path itself.

Applied to files:

  • builder/frameworks/arduino.py
  • 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/arduino.py
  • 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-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-07-13T20:03:29.695Z
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 225
File: builder/main.py:99-108
Timestamp: 2025-07-13T20:03:29.695Z
Learning: In the ESP32 platform builder, the subprocess call to get site-packages directory in setup_python_paths() function is intentionally left without error handling because if this basic Python operation fails, it indicates a fundamental system issue that would cause the entire build process to fail anyway.

Applied to files:

  • builder/frameworks/espidf.py
🔇 Additional comments (6)
boards/esp32-p4-evboard.json (1)

12-12: LGTM! Clean integration of chip variant support.

The addition of chip_variant and the clarified board name properly distinguish this as an ES (engineering sample) pre-rev.300 board, aligning with the builder framework changes.

Also applies to: 29-29

builder/frameworks/arduino.py (1)

288-291: LGTM! Well-structured chip variant integration.

The PathCache class correctly accepts and uses chip_variant for SDK path resolution, and the derivation logic (board config with fallback to MCU) is sound. This enables proper path handling for different chip variants like esp32p4_es vs esp32p4.

Also applies to: 314-314, 524-528

builder/frameworks/component_manager.py (1)

48-49: LGTM! Consistent chip variant integration for library path resolution.

The chip_variant derivation matches the pattern in arduino.py, and using it for the arduino_libs_mcu path (line 79) correctly enables variant-specific library directory selection.

Also applies to: 79-79

boards/esp32-p4.json (1)

12-12: LGTM! Consistent chip variant and naming convention.

The changes properly identify this as an ES pre-rev.300 variant, using the same naming convention as esp32-p4-evboard.json.

Also applies to: 26-26

builder/frameworks/espidf.py (1)

80-81: LGTM! Comprehensive chip variant integration for ESP-IDF Arduino paths.

The chip_variant derivation and its consistent use throughout for Arduino ESP-IDF library paths, sdkconfig locations, and skeleton directories correctly enables variant-specific builds. The pattern aligns with changes in arduino.py and component_manager.py.

Also applies to: 185-194, 2605-2607, 2630-2632

boards/esp32-p4_r3.json (1)

7-7: CPU frequency (400 MHz) is verified correct. Official Espressif specifications confirm the ESP32-P4 supports CPU frequency up to 400 MHz, matching the rev.300 board definition at line 7.

However, RAM and upload speed specifications cannot be conclusively verified from available sources. While the official Espressif specs indicate 768 KB total on-chip SRAM, the maximum_ram_size field in board definitions (327680 bytes / 320 KB) likely represents framework-available RAM after bootloader/system allocation—a common pattern in embedded systems. Notably:

  • rev.300's 327680 bytes RAM and 460800 bps match the pre-rev.300 generic definition (esp32-p4.json)
  • The ev-board variant uses different specs: 512000 bytes and 1500000 bps
  • Both 460800 bps and 1500000 bps are documented as valid upload speeds depending on hardware/driver capabilities

Without access to official rev.300 hardware specifications or the referenced upstream PR #12036, the RAM and upload speed values cannot be confirmed as correct or incorrect. The consistency with the pre-rev.300 generic definition suggests they may be intentional, but manual verification is recommended.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants