Add ESP32-S31 1 Gb RGMII Ethernet + fix build-script ESP32 gaps#34
Merged
Conversation
The ESP32-S31 CoreBoard's on-chip 1 Gb Ethernet (RGMII → YT8531 PHY) now works: it brings up a wired link, gets DHCP, and prefers Ethernet over WiFi via the existing cascade — bench-verified on the board. Getting there surfaced two build-script bugs that also affected other targets, fixed here.
KPI: 16384lights | PC:465KB | tick:416/206/12/345/46/5/919/324/47/60/612/322/56/3/86us(FPS:2403/4854/83333/2898/21739/200000/1088/3086/21276/16666/1633/3105/17857/333333/11627) | tick:5583us(FPS:179) | heap:31522KB | src:166(31724) | test:115(16438) | lizard:113w
Core:
- platform_config.h: added the isEsp32S31 chip flag, the ethYt8531 PHY type (RGMII, S31-only — the only target with SOC_EMAC_SUPPORT_1000M), and the S31 branch of ethConfigDefault (MDC 5 / MDIO 6 / reset 7, PHY addr auto-detect). RGMII data pins are the chip's fixed IO_MUX pads so they stay in ethInitEmac, not the struct — same as RMII's data pins.
- platform_esp32.cpp: renamed ethInitRmii → ethInitEmac and branched it on the chip (#ifdef CONFIG_IDF_TARGET_ESP32S31): the S31 sets the RGMII interface + CoreBoard IO_MUX data/clock pins, everything else uses the RMII path, and both share the driver-install / netif / DHCP tail (no duplicated init). YT8531 is driven by the generic IEEE-802.3 PHY ctor. ethInit() dispatches ethYt8531 to it.
- NetworkModule.h: added YT8531 to the ethType options and an isRgmii case so the phyAddr/reset rows show for it (MDC/MDIO/data are fixed IO_MUX, so those rows stay hidden).
Scripts / MoonDeck:
- build_esp32.py: MM_NO_ETH is now decided by reading each sdkconfig fragment's *content* for CONFIG_ETH_USE_ESP32_EMAC / CONFIG_ETH_USE_SPI_ETHERNET, not by pattern-matching the fragment *filename* for ".eth". The old filename check missed the S31 (its EMAC is enabled in sdkconfig.defaults.esp32s31, which has no ".eth" in the name), so it silently stubbed ethInit out — the bug that made the S31 fall back to WiFi with no Ethernet log. Verified identical to the old behaviour for all 7 existing firmwares.
- build_esp32.py: find_idf_python() now selects the ESP-IDF Python venv by matching the target IDF version (idf<major.minor>_) instead of picking the newest-mtime venv. With two IDFs installed, the last-activated venv is newest by mtime but belongs to whichever IDF was last sourced, so a 6.1 build could be handed the 5.5 venv (mismatched esptool → "requirements not satisfied"). Falls back to newest-overall only when no versioned venv matches, so single-IDF setups are unchanged.
Tests:
- test_build_esp32_venv_select.py: pins find_idf_python's version-matched selection — the regression (6.1 target must pick the 6.1 venv even when a 5.5 venv is newer), the symmetric 5.5 case, newest-python-minor tie-break within a version, and the no-match fallback.
Docs / CI:
- sdkconfig.defaults.esp32s31: set CONFIG_ETH_EMAC_RGMII_TX_CLK_SRC_APLL. RGMII needs a clean 125 MHz Tx clock; the default MPLL is owned by PSRAM (400 MHz, no integer path to 125) and CPLL can't hit it on the 40 MHz XTAL grid, so the fractional APLL synthesises it exactly.
- deviceModels.json: S31 Ethernet moved from planned to supported.
- esp32-s31-coreboard.md: corrected the Ethernet pin table to the bench-verified values (the prior table was systematically one GPIO low on the MDC/MDIO + TX pins) and documented the APLL clock choice.
- Plan-20260703 - S31 RGMII Ethernet.md: the approved implementation plan.
KPI Details:
PC:
tick: 416us, 206us, 12us, 345us, 46us, 5us, 919us, 324us, 47us, 60us, 612us, 322us, 56us, 3us, 86us (per scenario)
ESP32:
tick: 5583us (FPS: 179) heap free: 32278559 (live capture)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds RGMII Ethernet support for ESP32-S31 with a new YT8531 PHY type: platform config defaults, a consolidated EMAC init path handling both RMII and RGMII, NetworkModule UI updates, sdkconfig APLL clock selection, updated docs, device model change, and a separate pytest suite for Python venv selection. ChangesRGMII Ethernet Support for ESP32-S31
Build Script Venv Selection Tests
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant ethInit
participant ethInitEmac
participant PHY
App->>ethInit: request Ethernet init
ethInit->>ethInit: check phyType (ethYt8531 vs ethIp101/others)
ethInit->>ethInitEmac: dispatch to shared EMAC init
ethInitEmac->>ethInitEmac: configure RMII or RGMII clock/pins (isEsp32S31)
ethInitEmac->>PHY: instantiate generic PHY ctor
ethInitEmac-->>App: log "Ethernet init done" (RMII/RGMII)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/esp32/platform_config.h`:
- Around line 227-238: Update the stale S31 comment above ethConfigDefault so it
matches the actual board-verified pin mapping in the initializer and
docs/reference/esp32-s31-coreboard.md. The current note incorrectly says
MDC/MDIO/reset are 4/5/6, but the EthPinConfig values and the reference table
use mdc=5, mdio=6, rst=7; revise the comment text only so future edits aren’t
misled.
In `@test/python/test_build_esp32_venv_select.py`:
- Around line 49-102: Add a regression test in
test/python/test_build_esp32_venv_select.py that exercises ambiguous
version-prefix collisions for build_esp32.find_idf_python, such as a target v6.1
IDF with both idf6.1_py... and idf6.10_py... venvs present. Verify it selects
the exact matching versioned venv, not a naive prefix/substr match, and keep the
existing version-driven assertions consistent with the other find_idf_python
tests.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 00a1aaf8-7e55-440b-b775-a636bf29c9e4
⛔ Files ignored due to path filters (1)
scripts/build/build_esp32.pyis excluded by!**/build/**
📒 Files selected for processing (8)
docs/history/plans/Plan-20260703 - S31 RGMII Ethernet.mddocs/reference/esp32-s31-coreboard.mdesp32/sdkconfig.defaults.esp32s31src/core/NetworkModule.hsrc/platform/esp32/platform_config.hsrc/platform/esp32/platform_esp32.cpptest/python/test_build_esp32_venv_select.pyweb-installer/deviceModels.json
Comment on lines
+227
to
+238
| // - S31 → Function-CoreBoard-1: RGMII YT8531, MDC/MDIO 4/5, reset 6. The RGMII | ||
| // *data* pins (TX_CTL/TXD0-3, RX_CTL/RXD0-3, clocks) are board-fixed and live in | ||
| // ethInitRgmii(), not this struct (same reason RMII data pins don't — see above); | ||
| // rmiiClock* are unused for RGMII (clocks are set in ethInitRgmii). See | ||
| // docs/reference/esp32-s31-coreboard.md for the schematic pin map. | ||
| constexpr EthPinConfig ethConfigDefault = | ||
| isEsp32P4 ? EthPinConfig{ /*phyType*/ ethIp101, /*addr*/ 1, /*mdc*/ 31, /*mdio*/ 52, | ||
| /*rst*/ 51, /*rmiiClk*/ 50, /*extIn*/ true, | ||
| /*miso*/ -1, /*mosi*/ -1, /*sck*/ -1, /*cs*/ -1, /*irq*/ -1 } | ||
| : isEsp32S31 ? EthPinConfig{ /*phyType*/ ethYt8531, /*addr*/ -1, /*mdc*/ 5, /*mdio*/ 6, | ||
| /*rst*/ 7, /*rmiiClk*/ -1, /*extIn*/ false, | ||
| /*miso*/ -1, /*mosi*/ -1, /*sck*/ -1, /*cs*/ -1, /*irq*/ -1 } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Comment states wrong MDC/MDIO/reset pins.
The comment says MDC/MDIO 4/5, reset 6, but the actual struct values below are mdc=5, mdio=6, rst=7 — which matches the bench-verified table in docs/reference/esp32-s31-coreboard.md (PHY_MDC=5, PHY_MDIO=6, ETH_PHY_RST=7). The comment is stale/wrong and would mislead anyone changing these pins later.
📝 Proposed fix
-// - S31 → Function-CoreBoard-1: RGMII YT8531, MDC/MDIO 4/5, reset 6. The RGMII
+// - S31 → Function-CoreBoard-1: RGMII YT8531, MDC/MDIO 5/6, reset 7. The RGMII📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // - S31 → Function-CoreBoard-1: RGMII YT8531, MDC/MDIO 4/5, reset 6. The RGMII | |
| // *data* pins (TX_CTL/TXD0-3, RX_CTL/RXD0-3, clocks) are board-fixed and live in | |
| // ethInitRgmii(), not this struct (same reason RMII data pins don't — see above); | |
| // rmiiClock* are unused for RGMII (clocks are set in ethInitRgmii). See | |
| // docs/reference/esp32-s31-coreboard.md for the schematic pin map. | |
| constexpr EthPinConfig ethConfigDefault = | |
| isEsp32P4 ? EthPinConfig{ /*phyType*/ ethIp101, /*addr*/ 1, /*mdc*/ 31, /*mdio*/ 52, | |
| /*rst*/ 51, /*rmiiClk*/ 50, /*extIn*/ true, | |
| /*miso*/ -1, /*mosi*/ -1, /*sck*/ -1, /*cs*/ -1, /*irq*/ -1 } | |
| : isEsp32S31 ? EthPinConfig{ /*phyType*/ ethYt8531, /*addr*/ -1, /*mdc*/ 5, /*mdio*/ 6, | |
| /*rst*/ 7, /*rmiiClk*/ -1, /*extIn*/ false, | |
| /*miso*/ -1, /*mosi*/ -1, /*sck*/ -1, /*cs*/ -1, /*irq*/ -1 } | |
| // - S31 → Function-CoreBoard-1: RGMII YT8531, MDC/MDIO 5/6, reset 7. The RGMII |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/platform/esp32/platform_config.h` around lines 227 - 238, Update the
stale S31 comment above ethConfigDefault so it matches the actual board-verified
pin mapping in the initializer and docs/reference/esp32-s31-coreboard.md. The
current note incorrectly says MDC/MDIO/reset are 4/5/6, but the EthPinConfig
values and the reference table use mdc=5, mdio=6, rst=7; revise the comment text
only so future edits aren’t misled.
Comment on lines
+49
to
+102
| def test_picks_matching_version_even_when_other_is_newer(tmp_path, monkeypatch): | ||
| """The regression: a 6.1 build with a NEWER-mtime 5.5 venv present must still | ||
| pick the 6.1 venv.""" | ||
| home = tmp_path / "home" | ||
| # 6.1 venv created first (older), 5.5 venv created last (newest mtime). | ||
| _make_venvs(home, ["idf6.1_py3.12_env", "idf5.5_py3.12_env"]) | ||
| monkeypatch.setattr(build_esp32.Path, "home", staticmethod(lambda: home)) | ||
|
|
||
| idf61 = _make_idf(tmp_path / "esp-idf", "v6.1-dev-5215-g0d928780081") | ||
| picked = build_esp32.find_idf_python(idf61) | ||
| assert picked is not None | ||
| assert picked.name == "idf6.1_py3.12_env", ( | ||
| "must match the target IDF version (6.1), not the newest-mtime venv (5.5)" | ||
| ) | ||
|
|
||
|
|
||
| def test_picks_matching_version_for_the_older_idf(tmp_path, monkeypatch): | ||
| """Symmetric case: a 5.5 build must pick the 5.5 venv even if the 6.1 venv is | ||
| newer — proves it's version-driven, not just 'prefer 6.1'.""" | ||
| home = tmp_path / "home" | ||
| _make_venvs(home, ["idf5.5_py3.12_env", "idf6.1_py3.12_env"]) # 6.1 newest | ||
| monkeypatch.setattr(build_esp32.Path, "home", staticmethod(lambda: home)) | ||
|
|
||
| idf55 = _make_idf(tmp_path / "esp-idf-v5.5", "v5.5.4") | ||
| picked = build_esp32.find_idf_python(idf55) | ||
| assert picked is not None | ||
| assert picked.name == "idf5.5_py3.12_env" | ||
|
|
||
|
|
||
| def test_newest_python_minor_wins_within_a_version(tmp_path, monkeypatch): | ||
| """If several Python-minor venvs exist for the SAME IDF version, the newest one | ||
| wins (mtime is the tie-breaker only within the matched version).""" | ||
| home = tmp_path / "home" | ||
| _make_venvs(home, ["idf6.1_py3.12_env", "idf6.1_py3.13_env"]) # 3.13 newest | ||
| monkeypatch.setattr(build_esp32.Path, "home", staticmethod(lambda: home)) | ||
|
|
||
| idf61 = _make_idf(tmp_path / "esp-idf", "v6.1-dev-5215-g0d928780081") | ||
| picked = build_esp32.find_idf_python(idf61) | ||
| assert picked is not None | ||
| assert picked.name == "idf6.1_py3.13_env" | ||
|
|
||
|
|
||
| def test_falls_back_to_newest_when_no_version_match(tmp_path, monkeypatch): | ||
| """Unknown/absent version match → keep the old newest-mtime behaviour so a | ||
| single-IDF setup (the common case) is unaffected.""" | ||
| home = tmp_path / "home" | ||
| _make_venvs(home, ["idf6.1_py3.12_env", "idf6.1_py3.13_env"]) | ||
| monkeypatch.setattr(build_esp32.Path, "home", staticmethod(lambda: home)) | ||
|
|
||
| # An IDF whose version doesn't match any venv prefix (e.g. a 5.4 with no venv). | ||
| idf_other = _make_idf(tmp_path / "esp-idf-x", "v5.4.1") | ||
| picked = build_esp32.find_idf_python(idf_other) | ||
| assert picked is not None | ||
| assert picked.name == "idf6.1_py3.13_env", "newest overall when nothing matches" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Missing coverage for ambiguous version-prefix matching.
None of the tests cover a case where the target version string could ambiguously prefix-match multiple venvs (e.g., an idf6.1 target alongside both idf6.1_py... and idf6.10_py... venv directories, or a version string like v6.1-dev-... versus a stored idf6.10_py... name). If find_idf_python() uses naive substring/prefix matching on the version string, this class of collision could silently pick the wrong venv. Consider adding a regression test for this boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/python/test_build_esp32_venv_select.py` around lines 49 - 102, Add a
regression test in test/python/test_build_esp32_venv_select.py that exercises
ambiguous version-prefix collisions for build_esp32.find_idf_python, such as a
target v6.1 IDF with both idf6.1_py... and idf6.10_py... venvs present. Verify
it selects the exact matching versioned venv, not a naive prefix/substr match,
and keep the existing version-driven assertions consistent with the other
find_idf_python tests.
…on test)
Two CodeRabbit review findings addressed: a stale S31 pin comment corrected, and a regression test added for the venv version-prefix boundary (the code was already correct; the test pins it).
Core:
- platform_config.h: corrected the S31 ethConfigDefault comment — it said MDC/MDIO 4/5, reset 6 (the pre-bench-fix values) while the struct uses 5/6/7, and referenced the old ethInitRgmii name. Comment-only; the struct values were already right.
Tests:
- test_build_esp32_venv_select.py: added test_version_prefix_does_not_collide_across_minor — a 6.1 target must not match an idf6.10 venv. find_idf_python already guards this via the trailing '_' in the prefix ("idf6.1_"); the test pins that boundary so a refactor dropping the separator regresses loudly.
Reviews:
- 🐇 platform_config.h S31 comment says wrong MDC/MDIO/reset pins (4/5/6) — fixed to the bench-verified 5/6/7 and updated the stale ethInitRgmii → ethInitEmac reference.
- 🐇 missing coverage for ambiguous version-prefix matching (idf6.1 vs idf6.10) — verified the code already handles it (trailing '_' in the prefix); added a regression test pinning the boundary rather than a code change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai Both findings addressed in 885a551:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
git log --oneline -5
git show 885a551 --stat
git show 885a551 -- src/platform/esp32/platform_config.h src/platform/esp32/platform_esp32.cpp test/python/test_build_esp32_venv_select.pyLength of output: 6728 Both changes look correct.
Thanks for the quick turnaround — both findings are resolved as described. (๑˃̵ᴗ˂̵)ﻭ |
… docs PR-merge (Event 2) gate work: the branch's hard-won lessons land in main with the code that proved them, and the Ethernet-type docs catch up to the new YT8531/RGMII path. Docs / CI: - decisions.md: added "ESP32-S31 RGMII Ethernet bring-up: four bugs between compiles and link up" — the MM_NO_ETH filename-vs-content gate, the systematic reference-doc pin offset (trust the chip's IO_MUX table), the RGMII 125 MHz Tx clock as a PLL-contention decision (APLL, since PSRAM owns MPLL), and the Kconfig-choice-needs-clean-build trap. - NetworkModule.md (archive): ethType now documents the YT8531 / RGMII option (index 4) and that the S31 shows only PHY-address + reset (its data/clock pins are fixed IO_MUX pads); prose updated to include the internal-EMAC RGMII path alongside RMII/W5500. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…curacy)
The pre-merge Reviewer pass found one real user-facing contradiction (the installer still said S31 Ethernet wasn't enabled) plus comment/test accuracy gaps. All addressed; the architecture was judged sound.
Scripts / MoonDeck:
- build_esp32.py: rewrote the S31 firmware description — it said the 1 Gbps Ethernet was "wired in but not yet enabled (pin map pending)", which this branch disproves and which shipped verbatim to users via firmwares.json (a present-tense violation the check gate can't catch, since dict and projection agreed — both stale). Now: WiFi 6 + 1 Gbps Ethernet, YT8531 RGMII, Ethernet-preferred cascade.
- build_esp32.py: the eth-fragment content check now matches whole lines (line.strip() in {symbols}) instead of substring, so a commented-out CONFIG_...=y can't false-positive — the textbook Kconfig read.
Core:
- platform_esp32.cpp: reworded two comments that said ethInitEmac "branches on phyType" — it branches on the chip (a compile-time #ifdef, RGMII being S31-only).
- NetworkModule.h: corrected the ethType enum-invariant comment to include YT8531=4, and fixed the RGMII pin-source comment — MDC/MDIO are NOT fixed IO_MUX pads (only data/clock are); they come from the per-chip ethConfigDefault via the shared smi_gpio path, and the rows are hidden because that default covers them.
- platform_config.h: isEsp32S31 "used only for ethConfigDefault" corrected (also used in ethInitEmac's RGMII branch/log).
Tests:
- test_build_esp32_s31.py: added test_s31_gets_ethernet_not_MM_NO_ETH — pins the MM_NO_ETH content-check fix (the S31 must build with Ethernet enabled), the regression decisions.md elevated to a lesson but that had no test.
- test_build_esp32_venv_select.py: moved import os to module top; dropped a JS-frame-of-reference ("no Date.now") comment.
- web-installer/firmwares.json: regenerated from the FIRMWARES dict (new S31 description).
Reviews:
- 👾 (major) Installer S31 description contradicted this PR ("Ethernet not yet enabled") and violated present-tense — fixed in the FIRMWARES dict + regenerated firmwares.json.
- 👾 (minor) Two comments claimed ethInitEmac branches on phyType, not the chip — fixed.
- 👾 (minor) ethTypeOptions_ enum-invariant comment omitted YT8531=4 — fixed.
- 👾 (minor) RGMII comment misstated MDC/MDIO as fixed IO_MUX pads — corrected to the ethConfigDefault/smi_gpio path.
- 👾 (minor) No regression test for the MM_NO_ETH content-check fix — added.
- 👾 (nit) platform_config.h "used only", line-based Kconfig match, venv-test import/comment — fixed.
- 👾 (nit) APLL rationale duplicated in sdkconfig + reference doc — accepted as-is: the two serve different readers (build-config vs board reference) and the doc already points at the sdkconfig.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What this does
The ESP32-S31 CoreBoard's on-chip 1 Gb Ethernet (RGMII → YT8531 PHY) now works: it brings up a wired link, gets a DHCP lease, and prefers Ethernet over WiFi via the existing cascade — bench-verified on the board (link up,
eth ip: 192.168.1.216,MM-FEF3.local). The S31 is the only projectMM target withSOC_EMAC_SUPPORT_1000M(RGMII), so this path is S31-only by nature — the classic ESP32 / P4 EMACs are RMII, and S2/S3/C3/C6 have no EMAC.Getting the bring-up working surfaced two build-script bugs that also affected other targets, fixed here.
S31 Ethernet feature
platform_config.h—isEsp32S31chip flag,ethYt8531PHY type, S31ethConfigDefault(MDC 5 / MDIO 6 / reset 7, PHY addr auto-detect). RGMII data pins are the chip's fixed IO_MUX pads, so they live inethInitEmac(like RMII's data pins), not the struct.platform_esp32.cpp— renamedethInitRmii→ethInitEmac, branched on the chip: the S31 selects the RGMII interface + CoreBoard IO_MUX data/clock pins; everything else uses the RMII path; both share the driver-install / netif / DHCP tail (no duplicated init). YT8531 is a standard-register PHY → driven by the generic IEEE-802.3 ctor.sdkconfig.defaults.esp32s31—CONFIG_ETH_EMAC_RGMII_TX_CLK_SRC_APLL. RGMII needs a clean 125 MHz Tx clock; MPLL is owned by PSRAM (400 MHz, no path to 125) and CPLL can't hit it on the 40 MHz grid, so the fractional APLL synthesises it exactly.NetworkModule.h— YT8531 in the ethType options + anisRgmiiUI case (phyAddr/reset rows show; the fixed IO_MUX MDC/MDIO/data rows stay hidden).deviceModels.json— S31 Ethernetplanned→supported.esp32-s31-coreboard.md— pin table corrected to bench-verified values (the prior table was one GPIO low on the MDC/MDIO + TX pins) + APLL clock rationale.Build-script fixes (found during this work)
MM_NO_ETHis now decided by reading each sdkconfig fragment's content forCONFIG_ETH_USE_ESP32_EMAC/CONFIG_ETH_USE_SPI_ETHERNET, not by matching the fragment filename for.eth. The old check missed the S31 (its EMAC is enabled insdkconfig.defaults.esp32s31, no.ethin the name) and silently stubbedethInitout — the bug that made the S31 fall back to WiFi. Verified identical to the old behaviour for all 7 existing firmwares.find_idf_python()now selects the ESP-IDF Python venv by matching the target IDF version (idf<major.minor>_) instead of newest-mtime. With two IDFs installed, the last-activated venv is newest by mtime but belongs to whichever IDF was last sourced, so a 6.1 build could get the 5.5 venv (mismatched esptool). Falls back to newest-overall when nothing matches, so single-IDF setups are unchanged. Pinned bytest_build_esp32_venv_select.py(5 tests).Verification
All pre-commit gates green: spec / device / firmware / platform-boundary checks, JS (31) + Python (35, incl. the 5 new venv tests), desktop build (zero warnings) + ctest + scenarios, all 8 ESP32 variants build clean, KPI (PC + ESP32). Plus real-hardware: S31 boots on Ethernet, DHCP, Ethernet-preferred cascade confirmed.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Post-open updates
885a551): corrected a stale S31 pin comment (platform_config.h), and added a regression test pinning the venv version-prefix boundary (idf6.1must not matchidf6.10— the code already guarded this via the trailing_).docs/history/decisions.md(the four bugs between "compiles" and "link up"), and synceddocs/moonmodules/core/archive/NetworkModule.mdfor the newYT8531ethType option.