From 9314ba23077726a23a1fe075c163464d649f748e Mon Sep 17 00:00:00 2001 From: ewowi Date: Fri, 3 Jul 2026 18:36:32 +0200 Subject: [PATCH 1/4] Add ESP32-S31 1 Gb RGMII Ethernet + fix build-script ESP32 gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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_) 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) --- .../Plan-20260703 - S31 RGMII Ethernet.md | 115 ++++++++++++++++++ docs/reference/esp32-s31-coreboard.md | 34 ++++-- esp32/sdkconfig.defaults.esp32s31 | 16 ++- scripts/build/build_esp32.py | 68 ++++++++--- src/core/NetworkModule.h | 17 +-- src/platform/esp32/platform_config.h | 19 +++ src/platform/esp32/platform_esp32.cpp | 54 ++++++-- test/python/test_build_esp32_venv_select.py | 110 +++++++++++++++++ web-installer/deviceModels.json | 4 +- 9 files changed, 382 insertions(+), 55 deletions(-) create mode 100644 docs/history/plans/Plan-20260703 - S31 RGMII Ethernet.md create mode 100644 test/python/test_build_esp32_venv_select.py diff --git a/docs/history/plans/Plan-20260703 - S31 RGMII Ethernet.md b/docs/history/plans/Plan-20260703 - S31 RGMII Ethernet.md new file mode 100644 index 00000000..f8c9f560 --- /dev/null +++ b/docs/history/plans/Plan-20260703 - S31 RGMII Ethernet.md @@ -0,0 +1,115 @@ +# Plan — ESP32-S31 RGMII Ethernet (1 Gb) with Ethernet-preferred cascade + +## Context + +The bench ESP32-S31 board (Espressif Function-CoreBoard-1) has an on-chip 1 Gb EMAC wired +through an **RGMII** interface to a **YT8531** PHY → RJ45. The product owner connected an +Ethernet cable to it and it isn't used yet: the S31 firmware only brings up WiFi. The goal is +Ethernet-preferred networking — use Ethernet when the cable is up at boot, fall back to WiFi +otherwise — matching how the classic ESP32 (Olimex) and P4 boards already behave. + +**Why S31-only:** among projectMM's targets, the S31 is the *only* chip whose EMAC advertises +`SOC_EMAC_SUPPORT_1000M` + RGMII. The classic ESP32 and P4 EMACs are RMII (100 Mb); S2/S3/C3/C6 +have no EMAC at all (Ethernet only via an external W5500 SPI chip). RGMII/1000M is intrinsic to +the SoC — no extension board can add it to a non-S31 — so the RGMII path is S31-only by nature, +not a selectable per-board option. (Product owner confirmed: "otherwise S31 only".) + +**Failover is already built:** `NetworkModule` runs `ethInit()` first and only starts WiFi if it +returns false (no PHY/cable). So "use Ethernet when available, WiFi otherwise" needs **no new +failover code** — only a new RGMII init path that the existing cascade calls. (Product owner +confirmed: Ethernet-preferred cascade at boot, not live hot-swap.) + +## Design (mirrors the existing RMII path, adds an RGMII sibling) + +The Ethernet layer already dispatches on `ethConfig_.phyType`: `ethInitRmii()` (on-chip EMAC, +RMII) and `ethInitSpi()` (W5500). This adds a third sibling, `ethInitRgmii()`, selected by a new +`ethYt8531` phyType — same shape, same cascade, same on-chip-EMAC compile guard. + +**RGMII data pins are hardwired, not runtime config.** Exactly like RMII (whose TX/RX data lines +live in the IDF EMAC macro, not `EthPinConfig` — see the comment at +[platform_config.h:196-198](src/platform/esp32/platform_config.h#L196)), the S31 CoreBoard's RGMII +data pins are fixed by the board schematic. So they go straight into `ethInitRgmii()` as literals +from the schematic — **no new `EthPinConfig` fields, no NetworkModule controls, no deviceModels.json +eth block.** This keeps the struct and the UI untouched; the whole feature is one board's wiring. + +**Pins (from `docs/reference/esp32-s31-coreboard.md`, sourced from the official schematic):** +MDC 4, MDIO 5, PHY reset 6, PHY int 2; TX_CTL 11, TXD0-3 = 7/8/9/10; RX_CTL 15, RXD0-3 = 19/18/17/16; +clock_tx 13, clock_rx 14. PHY = YT8531 via `esp_eth_phy_new_generic` (IEEE-standard registers). + +## Files to change + +1. **`src/platform/esp32/platform_config.h`** + - Add `isEsp32S31` constexpr flag (keyed on `CONFIG_IDF_TARGET_ESP32S31`), following the + `isEsp32P4`/`isEsp32S3` pattern at [L33-46](src/platform/esp32/platform_config.h#L33). + - Add `ethYt8531 = 4` to the `EthPhyType` enum ([L175](src/platform/esp32/platform_config.h#L175)), + with a one-line "RGMII, YT8531 PHY, S31 on-chip 1 Gb EMAC" comment. + - Add an `isEsp32S31` branch to the `ethConfigDefault` ternary + ([L216](src/platform/esp32/platform_config.h#L216)): `phyType ethYt8531`, `phyAddr` (from the + YT8531 strap — default 0, confirm on bench), `rstGpio 6`, MDC/MDIO 4/5. RGMII data + clock + pins are NOT struct fields (hardwired in `ethInitRgmii`); pass -1 for the unused RMII/SPI + fields. **No struct change.** + +2. **`src/platform/esp32/platform_esp32.cpp`** + - Add `static bool ethInitRgmii()` mirroring `ethInitRmii()` + ([L457-543](src/platform/esp32/platform_esp32.cpp#L457)) under the same + `#ifdef CONFIG_ETH_USE_ESP32_EMAC` guard. Differences from the RMII version: + - `emac_config.interface = EMAC_DATA_INTERFACE_RGMII` + - set `emac_config.clock_config.rgmii.clock_tx_gpio/clock_rx_gpio` (13/14) and the + `emac_config.emac_dataif_gpio.rgmii` struct (tx_ctl/txd0-3/rx_ctl/rxd0-3 = the schematic + pins) — the RGMII fields IDF exposes in `esp_eth_mac_esp.h`. + - PHY: `esp_eth_phy_new_generic(&phy_config)` (YT8531 is standard-register; same generic ctor + LAN8720 uses — no managed component needed). + - reuse the identical `fail()` cleanup lambda, driver-install, netif-attach, event-register, + non-blocking `esp_eth_start`, and the link-up hostname handling. Log "Ethernet init done + (RGMII, S31)". + - Add the dispatch case to `ethInit()` + ([the switch, ~L661](src/platform/esp32/platform_esp32.cpp#L661)): + `#ifdef CONFIG_ETH_USE_ESP32_EMAC` → `case ethYt8531: return ethInitRgmii();` (alongside the + existing `ethLan8720`/`ethIp101` RMII cases). + +3. **`esp32/sdkconfig.defaults.esp32s31`** — `CONFIG_ETH_USE_ESP32_EMAC=y` + DMA buffers are + already present ([L26-32](esp32/sdkconfig.defaults.esp32s31)). RGMII is selected at runtime via + the struct `interface` field (not a sdkconfig symbol), so likely **no change** — but verify at + configure time that no `CONFIG_ETH_*RGMII*`/1000M symbol is required; add it only if the build + demands it. + +4. **`web-installer/deviceModels.json`** — move `"Ethernet"` from the S31's `planned` list to + `supported` (the S31 entry). No eth `NetworkModule` control block needed (pins are the + compile-time default). `check_devices.py` allows `Ethernet` in `supported` (it's in + `SUPPORTED_VOCAB`). + +5. **`docs/reference/esp32-s31-coreboard.md`** — update the Ethernet section's "wiring the S31 eth + needs an RGMII branch — not a drop-in" note to present-tense "driven by `ethInitRgmii`", since + it now ships. (Small doc sync, per the present-tense rule.) + +## Not doing (deliberately, keeps it minimal) + +- No `EthPinConfig` struct fields for RGMII data pins (hardwired, like RMII). +- No NetworkModule UI controls, no `syncEthConfig` change (nothing new to sync). +- No deviceModels.json eth-pin block for the S31 (compile-time default covers it). +- No failover/route-switching code (the eth→WiFi cascade already exists). +- No new managed component (generic PHY driver covers YT8531). + +## Verification + +- **Build:** `esp32s31` builds clean on 6.1 with `-Werror` (the RGMII branch is behind the + already-set `CONFIG_ETH_USE_ESP32_EMAC`; other targets unaffected — the case is chip-guarded). +- **Non-regression:** classic/P4/S3 eth paths untouched (RMII/SPI code unchanged); a quick + `esp32p4-eth` + `esp32` build stays green. `check_devices.py` green with `Ethernet` under S31 + `supported`. `ctest` + scenarios unaffected (platform-only change). +- **Bench (the real test), on the connected S31 (`/dev/cu.usbserial-20213420`), cable plugged in:** + 1. Flash `esp32s31`, capture boot log → expect `Ethernet init done (RGMII, S31)` then an + **Ethernet DHCP lease** (an `MM_IP=` from the wired subnet, like the P4 eth test showed + `192.168.1.133`), and mDNS `MM-S31.local`. + 2. Confirm the render loop still runs (FPS line present) and heap is healthy. + 3. **Failover check:** unplug the cable, reboot → it should fall back to WiFi (the existing + cascade). Plug back in, reboot → Ethernet again. (Boot-time cascade, per the chosen model.) +- Save the approved plan to `docs/history/plans/Plan-20260703 - S31 RGMII Ethernet.md` as the first + implementation step. + +## Open items to confirm on the bench during implementation + +- **YT8531 PHY address** — default strap is usually 0; if `esp_eth` can't find the PHY at addr 0, + scan/try 1 (the reference doc doesn't pin the strap). One-line fix in `ethConfigDefault`. +- **RGMII clock direction / delay** — YT8531 boards sometimes need RX/TX clock delay config; if the + link comes up but no packets flow, revisit the RGMII clock config. (Bench will show.) diff --git a/docs/reference/esp32-s31-coreboard.md b/docs/reference/esp32-s31-coreboard.md index 9ad8a93e..07baa063 100644 --- a/docs/reference/esp32-s31-coreboard.md +++ b/docs/reference/esp32-s31-coreboard.md @@ -48,22 +48,34 @@ The onboard electret mic (J6) and speaker connect through an **ES8311 mono codec On-chip EMAC → **YT8531** (Motorcomm) PHY (U8) → RJ45, **RGMII** with a 25 MHz crystal (Y2). +Pin map — **bench-verified** (link + DHCP confirmed on the CoreBoard). The RGMII data + clock +GPIOs are the chip's fixed IO_MUX pads (the only ones the EMAC accepts; from IDF's +`esp32s31/emac_periph.c`); MDC/MDIO are IDF's S31 SMI defaults (`ETH_ESP32_EMAC_DEFAULT_CONFIG`): + | Signal | GPIO | | Signal | GPIO | |---|---|---|---|---| -| ETH_INTN | 2 | | ETH_TXD3 | 10 | -| PHY_MDC | 4 | | ETH_TX_CTL | 11 | -| PHY_MDIO | 5 | | ETH_TXCLK | 13 | -| ETH_PHY_RST | 6 | | ETH_RX_CLK | 14 | -| ETH_TXD0 | 7 | | ETH_RX_CTL | 15 | -| ETH_TXD1 | 8 | | ETH_RXD3 | 16 | -| ETH_TXD2 | 9 | | ETH_RXD2 | 17 | +| ETH_INTN | 2 | | ETH_TXD3 | 11 | +| PHY_MDC | 5 | | ETH_TX_CTL | 12 | +| PHY_MDIO | 6 | | ETH_TXCLK | 13 | +| ETH_PHY_RST | 7 | | ETH_RX_CLK | 14 | +| ETH_TXD0 | 8 | | ETH_RX_CTL | 15 | +| ETH_TXD1 | 9 | | ETH_RXD3 | 16 | +| ETH_TXD2 | 10 | | ETH_RXD2 | 17 | | | | | ETH_RXD1 | 18 | | | | | ETH_RXD0 | 19 | -> **RGMII, not RMII.** projectMM's classic/P4 Ethernet path (`ethInit` in -> `src/platform/esp32/platform_esp32.cpp`) is RMII (fewer data lines, 50 MHz ref clock). The S31's -> 1 Gbps EMAC is RGMII (4-bit data each way + TX/RX clocks). Wiring the S31 eth needs an RGMII MAC -> config branch — it is not a drop-in of the RMII pin struct. +> **RGMII, not RMII.** projectMM's classic/P4 Ethernet is RMII (fewer data lines, 50 MHz ref clock); +> the S31's 1 Gbps EMAC is RGMII (4-bit data each way + TX/RX clocks). The shared `ethInitEmac()` in +> `src/platform/esp32/platform_esp32.cpp` drives both: an `#ifdef CONFIG_IDF_TARGET_ESP32S31` block +> selects the RGMII interface and sets the CoreBoard's data/clock pins from the table above, then +> shares the driver-install / netif / DHCP tail with the RMII path. The board's default eth config +> (`ethConfigDefault` in `platform_config.h`) uses PHY type `ethYt8531` (PHY address auto-detected), +> driven by the generic IEEE-802.3 PHY ctor since the YT8531 is a standard-register PHY. +> +> **RGMII 125 MHz Tx clock = APLL.** The EMAC 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 +> `sdkconfig.defaults.esp32s31` sets `CONFIG_ETH_EMAC_RGMII_TX_CLK_SRC_APLL` — the fractional PLL +> synthesises 125 MHz exactly. ## Other onboard features diff --git a/esp32/sdkconfig.defaults.esp32s31 b/esp32/sdkconfig.defaults.esp32s31 index 2061a53a..58824356 100644 --- a/esp32/sdkconfig.defaults.esp32s31 +++ b/esp32/sdkconfig.defaults.esp32s31 @@ -23,10 +23,20 @@ CONFIG_ESPTOOLPY_FLASHSIZE_16MB=y # not relied on at boot. CONFIG_SPIRAM=y -# Ethernet — on-chip EMAC + RMII (SOC_EMAC_SUPPORTED, SOC_EMAC_SUPPORT_1000M). Pins -# are runtime config (ethConfig_ in C), not sdkconfig. RMII is the EMAC's default -# interface, so no CONFIG_ETH_PHY_INTERFACE_RMII (a non-existent symbol in IDF v6). +# Ethernet — on-chip EMAC (SOC_EMAC_SUPPORTED, SOC_EMAC_SUPPORT_1000M → RGMII 1 Gb). +# The interface (RGMII on the S31) and PHY pins (YT8531 map) are set in C — the RGMII +# branch of ethInitEmac() in src/platform/esp32/platform_esp32.cpp, keyed off the +# ETH_ESP32_EMAC_DEFAULT_CONFIG() macro which IDF fixes to RGMII on the S31 — not via a +# sdkconfig symbol, so the pin map stays in one place. CONFIG_ETH_USE_ESP32_EMAC=y + +# RGMII Tx clock source = APLL. The 1 Gb RGMII EMAC needs a clean 125 MHz Tx clock. The +# default (AUTO) picks the MPLL, but PSRAM owns the MPLL at 400 MHz (SPIRAM_SPEED_200M) — +# 400 MHz has no integer path to 125 MHz ("unusable frequency 133 MHz"). CPLL can't hit it +# either ("No CPLL on 40 MHz grid divides 125 MHz"): both are integer PLLs off the 40 MHz +# XTAL. The APLL is a *fractional* PLL built for exact frequencies (its reason to exist), so +# it synthesises 125 MHz precisely. Bench-proven on the S31 CoreBoard. +CONFIG_ETH_EMAC_RGMII_TX_CLK_SRC_APLL=y CONFIG_ETH_DMA_BUFFER_SIZE=512 CONFIG_ETH_DMA_RX_BUFFER_NUM=10 CONFIG_ETH_DMA_TX_BUFFER_NUM=10 diff --git a/scripts/build/build_esp32.py b/scripts/build/build_esp32.py index 92583aa6..87803188 100644 --- a/scripts/build/build_esp32.py +++ b/scripts/build/build_esp32.py @@ -209,20 +209,44 @@ def find_idf() -> Path | None: _PYTHON_EXE = "python.exe" if sys.platform == "win32" else "python" -def find_idf_python() -> Path | None: - """Find the ESP-IDF Python venv. Prefers most recently modified.""" +def find_idf_python(idf_path: Path | None = None) -> Path | None: + """Find the ESP-IDF Python venv for the target IDF version. + + ESP-IDF names each venv `idf_py_env` (e.g. + `idf6.1_py3.12_env`) — one per IDF version × Python version — exactly as + `idf_tools.py` computes it. We select by matching the *target* IDF version, + NOT by mtime: with two IDFs installed (e.g. a 5.5 alongside 6.1 for a + version fallback), the most-recently-activated venv is the newest by mtime + but belongs to whichever IDF was last sourced, so an mtime pick silently + hands a 6.1 build the 5.5 venv (mismatched esptool → "requirements not + satisfied"). Matching the version makes selection a function of what we're + building, not what was last run. + + Among venvs for the right IDF version (there can be several Python-minor + variants) the newest wins. Falls back to newest-overall only when the IDF + version can't be determined or no versioned venv matches — preserving the + single-IDF behaviour where mtime is unambiguous anyway. + """ venv_dir = Path.home() / ".espressif" / "python_env" if not venv_dir.exists(): return None candidates = [] for d in venv_dir.iterdir(): - python = d / _VENV_BIN / _PYTHON_EXE - if python.exists(): + if (d / _VENV_BIN / _PYTHON_EXE).exists(): candidates.append((d.stat().st_mtime, d)) - if candidates: - candidates.sort(reverse=True) - return candidates[0][1] - return None + if not candidates: + return None + + if idf_path is not None: + # idf_version() → "6.1.0"; the venv prefix uses only major.minor. + prefix = "idf" + ".".join(idf_version(idf_path).split(".")[:2]) + "_" + matched = [c for c in candidates if c[1].name.startswith(prefix)] + if matched: + matched.sort(reverse=True) + return matched[0][1] + + candidates.sort(reverse=True) + return candidates[0][1] def idf_version(idf_path: Path) -> str: @@ -250,7 +274,7 @@ def idf_env(idf_path: Path) -> dict: env["PYTHONUTF8"] = "1" env["PYTHONIOENCODING"] = "utf-8" - venv_path = find_idf_python() + venv_path = find_idf_python(idf_path) if venv_path: env["IDF_PYTHON_ENV_PATH"] = str(venv_path) @@ -301,7 +325,7 @@ def idf_cmd(idf_path: Path) -> list[str]: (`locale.getlocale()`) pass on Windows installs whose system locale is non-UTF-8 (e.g. Dutch / German / French). See the shim's docstring. """ - venv_path = find_idf_python() + venv_path = find_idf_python(idf_path) python_exe = (str(venv_path / _VENV_BIN / _PYTHON_EXE) if venv_path else "python") if sys.platform == "win32": @@ -345,16 +369,20 @@ def firmware_cmake_args(firmware: str, release: str = "", version: str = "") -> # Firmwares that have no Ethernet driver at all (no EMAC sdkconfig and no # SPI-PHY sdkconfig) lack the headers platform_esp32.cpp's ethInit() needs, # so it won't compile — set MM_NO_ETH and the source provides stubs instead. - # A variant "has Ethernet" when any sdkconfig fragment enables a PHY driver: - # * RMII EMAC (classic/P4): `sdkconfig.defaults.eth` (".eth"), board-specific - # ones append "-eth" (e.g. ".esp32p4-eth"). - # * W5500 SPI (S3, no EMAC): `sdkconfig.defaults.eth-spi` (".eth-spi"). - # Match the "eth" segment in any of these forms so a new eth board (RMII or - # SPI) doesn't silently stub Ethernet out. The hyphen-suffix forms (`-eth`, - # `.eth-spi`) are why a bare endswith(".eth") isn't enough. - has_eth_fragment = any(".eth" in f or f.endswith("-eth") - for f in spec["fragments"]) - if not has_eth_fragment: + # A variant "has Ethernet" when any of its sdkconfig fragments enables a PHY + # driver — the RMII EMAC (CONFIG_ETH_USE_ESP32_EMAC, classic/P4/S31) or the + # W5500 SPI PHY (CONFIG_ETH_USE_SPI_ETHERNET, S3). We read the fragment *files* + # and check for the actual enabling line rather than pattern-matching the + # filename: the S31 enables EMAC in `sdkconfig.defaults.esp32s31` (no ".eth" in + # the name), which a filename heuristic would miss and silently stub eth out. + eth_symbols = ("CONFIG_ETH_USE_ESP32_EMAC=y", "CONFIG_ETH_USE_SPI_ETHERNET=y") + has_eth = any( + (ESP32_DIR / frag).exists() + and any(sym in (ESP32_DIR / frag).read_text(encoding="utf-8") + for sym in eth_symbols) + for frag in spec["fragments"] + ) + if not has_eth: args.append("-DMM_NO_ETH=1") return args diff --git a/src/core/NetworkModule.h b/src/core/NetworkModule.h index d7ca5eee..96348734 100644 --- a/src/core/NetworkModule.h +++ b/src/core/NetworkModule.h @@ -289,17 +289,20 @@ class NetworkModule : public MoonModule { // Ethernet pin/PHY config — only on builds with an Ethernet driver. The // board's deviceModels.json eth block writes these; an un-provisioned board keeps // the per-chip default. ethType picks the PHY (and which pin set applies): - // 1=LAN8720(RMII), 2=IP101(RMII), 3=W5500(SPI). The RMII vs SPI pin rows are - // shown by type so the UI isn't cluttered with the inapplicable set. + // 1=LAN8720(RMII), 2=IP101(RMII), 3=W5500(SPI), 4=YT8531(RGMII). The RMII/SPI pin + // rows are shown by type so the UI isn't cluttered with the inapplicable set. if constexpr (platform::hasEthernet) { // ethType is the switch (always shown on an eth-capable build). When it // is 0 (no Ethernet) NO pin rows show; choosing LAN8720/IP101 reveals // the RMII rows, W5500 the SPI rows — only the applicable set is ever // visible. (Same "show only what's relevant" shape as the LED drivers.) - controls_.addSelect("ethType", ethType_, ethTypeOptions_, 4); - const bool isRmii = (ethType_ == 1 || ethType_ == 2); - const bool isSpi = (ethType_ == 3); - const bool isEth = isRmii || isSpi; + controls_.addSelect("ethType", ethType_, ethTypeOptions_, 5); + const bool isRmii = (ethType_ == 1 || ethType_ == 2); + const bool isSpi = (ethType_ == 3); + const bool isRgmii = (ethType_ == 4); + // RGMII (S31): MDC/MDIO + the data/clock pins are the chip's fixed IO_MUX pads, + // set in ethInitEmac() — not user-editable — so only phyAddr + reset show for it. + const bool isEth = isRmii || isSpi || isRgmii; // GPIO controls use addPin → a plain number input (ControlType::Pin), // not a slider: a GPIO has no meaningful range to drag. -1 = unused. // phyAddr is a PHY MDIO address (0..31), not a GPIO, but it's likewise @@ -663,7 +666,7 @@ class NetworkModule : public MoonModule { static constexpr const char* addressingOptions_[] = {"DHCP", "Static"}; // ethType dropdown options — index order MUST match the EthPhyType enum // (None=0, LAN8720=1, IP101=2, W5500=3) since the Select stores the index. - static constexpr const char* ethTypeOptions_[] = {"None", "LAN8720", "IP101", "W5500"}; + static constexpr const char* ethTypeOptions_[] = {"None", "LAN8720", "IP101", "W5500", "YT8531"}; void startAP() { // Same identity as the DHCP hostname and the mDNS .local name — all three read diff --git a/src/platform/esp32/platform_config.h b/src/platform/esp32/platform_config.h index 5a460566..c9aa12ce 100644 --- a/src/platform/esp32/platform_config.h +++ b/src/platform/esp32/platform_config.h @@ -45,6 +45,16 @@ constexpr bool isEsp32S3 = true; constexpr bool isEsp32S3 = false; #endif +// isEsp32S31: the S31 is the only target whose EMAC is RGMII / 1 Gb (SOC_EMAC_SUPPORT_1000M), +// where classic/P4 are RMII — so its Ethernet default is a distinct RGMII PHY (YT8531) with a +// different pin set. Not derivable from a SOC flag (the RGMII data pins are board wiring, not a +// chip property). Used only for ethConfigDefault. +#ifdef CONFIG_IDF_TARGET_ESP32S31 +constexpr bool isEsp32S31 = true; +#else +constexpr bool isEsp32S31 = false; +#endif + // RMT TX channels this chip offers (8 on classic ESP32, 4 on the S3 / P4 / S31, // straight from the IDF RMT HAL — `RMT_LL_TX_CANDIDATES_PER_INST`, included above). // Doubles as the RMT capability flag: the RMT LED driver and its main.cpp @@ -177,6 +187,7 @@ enum EthPhyType { ethLan8720 = 1, // RMII, generic PHY (Olimex Gateway, QuinLED Dig-Octa) ethIp101 = 2, // RMII, IP101 PHY (Waveshare P4-NANO; managed component, P4-only) ethW5500 = 3, // SPI, external W5500 module (ESP32-S3 boards — SE16, LightCrafter) + ethYt8531 = 4, // RGMII, YT8531 PHY (ESP32-S31 CoreBoard; on-chip 1 Gb EMAC, S31-only) }; // Per-board Ethernet pin/PHY map — runtime-configurable (no longer a fixed @@ -213,10 +224,18 @@ struct EthPinConfig { // - S3 → no built-in EMAC, so the default is W5500 SPI but with no pins set // (phyType ethW5500, pins -1): a W5500 S3 board MUST provide its SPI pins via // deviceModels.json — there's no universal S3 default to guess. +// - 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 } : isEsp32S3 ? EthPinConfig{ /*phyType*/ ethW5500, /*addr*/ 1, /*mdc*/ -1, /*mdio*/ -1, /*rst*/ -1, /*rmiiClk*/ -1, /*extIn*/ false, /*miso*/ -1, /*mosi*/ -1, /*sck*/ -1, /*cs*/ -1, /*irq*/ -1 } diff --git a/src/platform/esp32/platform_esp32.cpp b/src/platform/esp32/platform_esp32.cpp index 3c137836..38ec2760 100644 --- a/src/platform/esp32/platform_esp32.cpp +++ b/src/platform/esp32/platform_esp32.cpp @@ -450,30 +450,57 @@ static EthPinConfig ethConfig_ = ethConfigDefault; void setEthConfig(const EthPinConfig& cfg) { ethConfig_ = cfg; } -// Internal-EMAC RMII path — only on chips with an on-chip EMAC (classic ESP32, -// P4). The S3 has no EMAC, so esp_eth_mac_new_esp32 / eth_esp32_emac_config_t / -// EMAC_CLK_* don't exist there; gating on CONFIG_ETH_USE_ESP32_EMAC keeps this -// function out of the S3 build (where Ethernet is W5500-over-SPI instead). +// Internal-EMAC path (RMII on classic ESP32 / P4, RGMII on the S31) — only on chips +// with an on-chip EMAC. The S3 has no EMAC, so esp_eth_mac_new_esp32 / +// eth_esp32_emac_config_t / EMAC_CLK_* don't exist there; gating on +// CONFIG_ETH_USE_ESP32_EMAC keeps this function out of the S3 build (where Ethernet is +// W5500-over-SPI instead). RMII and RGMII share the same MAC ctor + driver/netif/event +// tail; only the interface-select + clock + data-pin config block differs, so they live +// in one function branched on phyType rather than two near-identical copies. #ifdef CONFIG_ETH_USE_ESP32_EMAC -static bool ethInitRmii() { +static bool ethInitEmac() { esp_netif_config_t netif_cfg = ESP_NETIF_DEFAULT_ETH(); ethNetif_ = esp_netif_new(&netif_cfg); - // RMII / PHY pins from the runtime ethConfig_ (the default LAN8720 map by default, the - // P4-NANO's IP101 map on the P4, or a board override pushed from deviceModels.json). + // PHY pins from the runtime ethConfig_ (the default LAN8720 map by default, the + // P4-NANO's IP101 map on the P4, the S31 CoreBoard's YT8531 map on the S31, or a + // board override pushed from deviceModels.json). ETH_ESP32_EMAC_DEFAULT_CONFIG() is + // chip-fixed: RMII on the classic ESP32 / P4, RGMII on the S31 (the S31's on-chip EMAC + // is 1 Gb, RGMII-only). So the interface-specific config below branches on the chip at + // compile time — the union member (.rmii / .rgmii) that exists is the one the macro set. eth_mac_config_t mac_config = ETH_MAC_DEFAULT_CONFIG(); eth_esp32_emac_config_t emac_config = ETH_ESP32_EMAC_DEFAULT_CONFIG(); + // Interface-specific config. #ifdef (not if constexpr) because the RGMII union members + // (.clock_config.rgmii, .emac_dataif_gpio.rgmii) only exist in the IDF header on chips + // with SOC_EMAC_USE_MULTI_IO_MUX (S31, P4) — on the classic ESP32 they're absent, so an + // if-constexpr S31 branch would still fail to compile there. The macro's `interface` + // field is likewise chip-fixed (RGMII on S31, RMII elsewhere). +#ifdef CONFIG_IDF_TARGET_ESP32S31 + // RGMII (S31): the on-chip 1 Gb EMAC drives the YT8531 over a 4-bit data path + TX/RX + // clocks. These are the chip's fixed RGMII IO_MUX pads — the ONLY GPIOs the EMAC accepts + // for each signal (validated against the IO_MUX table in IDF's esp32s31/emac_periph.c; + // a non-IO_MUX pin fails "invalid ... GPIO number"). They also match the CoreBoard + // schematic wiring (docs/reference/esp32-s31-coreboard.md). Passing GPIO_NUM_MAX (-1) + // here would make IDF pick these same defaults; we list them explicitly for clarity. + emac_config.clock_config.rgmii.clock_tx_gpio = 13; + emac_config.clock_config.rgmii.clock_rx_gpio = 14; + emac_config.emac_dataif_gpio.rgmii = eth_mac_rgmii_gpio_config_t{ + /*tx_ctl*/ 12, /*txd0*/ 8, /*txd1*/ 9, /*txd2*/ 10, /*txd3*/ 11, + /*rx_ctl*/ 15, /*rxd0*/ 19, /*rxd1*/ 18, /*rxd2*/ 17, /*rxd3*/ 16, + }; +#else emac_config.clock_config.rmii.clock_mode = ethConfig_.rmiiClockExtIn ? EMAC_CLK_EXT_IN : EMAC_CLK_OUT; emac_config.clock_config.rmii.clock_gpio = static_cast(ethConfig_.rmiiClockGpio); - if (ethConfig_.mdcGpio >= 0) emac_config.smi_gpio.mdc_num = ethConfig_.mdcGpio; - if (ethConfig_.mdioGpio >= 0) emac_config.smi_gpio.mdio_num = ethConfig_.mdioGpio; // NOTE: the RMII *data* GPIOs (TX_EN/TXD0/TXD1/CRS_DV/RXD0/RXD1) are left at the // ETH_ESP32_EMAC_DEFAULT_CONFIG() defaults. On the classic ESP32 they're fixed in // silicon; on the P4 the macro already defaults them to 49/34/35/28/29/30 (the // NANO wiring) — the proven round-1 P4 build relied on exactly these defaults, so // we don't override them. (deviceModels.json doesn't carry them either.) +#endif + if (ethConfig_.mdcGpio >= 0) emac_config.smi_gpio.mdc_num = ethConfig_.mdcGpio; + if (ethConfig_.mdioGpio >= 0) emac_config.smi_gpio.mdio_num = ethConfig_.mdioGpio; eth_phy_config_t phy_config = ETH_PHY_DEFAULT_CONFIG(); phy_config.phy_addr = ethConfig_.phyAddr; @@ -504,7 +531,9 @@ static bool ethInitRmii() { if (ethConfig_.phyType == ethIp101) phy = esp_eth_phy_new_ip101(&phy_config); else phy = esp_eth_phy_new_generic(&phy_config); #else - phy = esp_eth_phy_new_generic(&phy_config); // LAN8720 / generic RMII + // LAN8720 (classic RMII) and YT8531 (S31 RGMII) are both IEEE-802.3-standard-register + // PHYs → the generic ctor drives both; no PHY-specific managed component needed. + phy = esp_eth_phy_new_generic(&phy_config); #endif if (!phy) return fail("PHY create failed", mac, nullptr); @@ -537,7 +566,7 @@ static bool ethInitRmii() { // clobber a name set at init time). ethHandle_ = eth_handle; // retained (ethStop is W5500-only today, but keep it set) - ESP_LOGI(NET_TAG, "Ethernet init done (RMII, non-blocking)"); + ESP_LOGI(NET_TAG, "Ethernet init done (%s, non-blocking)", isEsp32S31 ? "RGMII, S31" : "RMII"); return true; } #endif // CONFIG_ETH_USE_ESP32_EMAC @@ -666,7 +695,8 @@ bool ethInit() { #endif #ifdef CONFIG_ETH_USE_ESP32_EMAC case ethLan8720: - case ethIp101: return ethInitRmii(); + case ethIp101: // RMII PHYs (classic ESP32, P4) + case ethYt8531: return ethInitEmac(); // RGMII PHY (S31); ethInitEmac branches on phyType #endif default: return false; // ethNone, or a PHY this firmware can't drive } diff --git a/test/python/test_build_esp32_venv_select.py b/test/python/test_build_esp32_venv_select.py new file mode 100644 index 00000000..2ba25393 --- /dev/null +++ b/test/python/test_build_esp32_venv_select.py @@ -0,0 +1,110 @@ +"""build_esp32.find_idf_python() must select the venv by IDF *version*, not mtime. + +ESP-IDF makes one Python venv per IDF version (`idf_py_env`) under +`~/.espressif/python_env/`. With two IDFs installed — e.g. a 5.5 kept alongside 6.1 for +a version-fallback experiment — sourcing either one's `export.sh` touches its venv, so +the last-activated venv is the newest by mtime. The old logic picked newest-by-mtime, +which meant a 6.1 build could be handed the 5.5 venv (its esptool is too old → idf.py +aborts with "The following Python requirements are not satisfied"). This pins that the +selection follows the *target* IDF version, so it's a function of what we build, not of +what was last sourced. + +Imports the real function from scripts/build/build_esp32.py (no ESP-IDF needed). +Run: `uv run --with pytest pytest test/python -q`. +""" + +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(ROOT / "scripts" / "build")) + +import build_esp32 # noqa: E402 + + +def _make_venvs(home: Path, names_by_age: list[str]) -> None: + """Create fake venvs under /.espressif/python_env/, each with a python + executable. `names_by_age` is oldest-first; later entries get a newer mtime so + the LAST name is the newest (the mtime a naive picker would choose).""" + venv_root = home / ".espressif" / "python_env" + for i, name in enumerate(names_by_age): + d = venv_root / name / build_esp32._VENV_BIN + d.mkdir(parents=True) + exe = d / build_esp32._PYTHON_EXE + exe.write_text("#!fake python\n") + # Stagger mtimes deterministically (no Date.now equivalent needed): index i. + import os + ts = 1_000_000 + i # strictly increasing → last name is newest + os.utime(venv_root / name, (ts, ts)) + + +def _make_idf(idf_path: Path, version_line: str) -> Path: + idf_path.mkdir(parents=True, exist_ok=True) + (idf_path / "version.txt").write_text(version_line) + return idf_path + + +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" + + +def test_none_when_no_venvs(tmp_path, monkeypatch): + home = tmp_path / "home" + (home / ".espressif" / "python_env").mkdir(parents=True) + monkeypatch.setattr(build_esp32.Path, "home", staticmethod(lambda: home)) + idf61 = _make_idf(tmp_path / "esp-idf", "v6.1") + assert build_esp32.find_idf_python(idf61) is None diff --git a/web-installer/deviceModels.json b/web-installer/deviceModels.json index f6a0ded2..8119bfb1 100644 --- a/web-installer/deviceModels.json +++ b/web-installer/deviceModels.json @@ -915,11 +915,11 @@ "url": "https://docs.espressif.com/projects/esp-dev-kits/en/latest/esp32s31/esp32-s31-function-coreboard-1/user_guide.html", "supported": [ "LEDs", - "WiFi" + "WiFi", + "Ethernet" ], "planned": [ "Audio", - "Ethernet", "Bluetooth", "Thread/Zigbee", "SD card", From 885a551f8d2b0c3aabb04f65acf1b8b01b53f2cd Mon Sep 17 00:00:00 2001 From: ewowi Date: Fri, 3 Jul 2026 18:55:45 +0200 Subject: [PATCH 2/4] Process CodeRabbit findings on PR #34 (stale comment + prefix-collision test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/platform/esp32/platform_config.h | 6 +++--- test/python/test_build_esp32_venv_select.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/platform/esp32/platform_config.h b/src/platform/esp32/platform_config.h index c9aa12ce..68c25757 100644 --- a/src/platform/esp32/platform_config.h +++ b/src/platform/esp32/platform_config.h @@ -224,10 +224,10 @@ struct EthPinConfig { // - S3 → no built-in EMAC, so the default is W5500 SPI but with no pins set // (phyType ethW5500, pins -1): a W5500 S3 board MUST provide its SPI pins via // deviceModels.json — there's no universal S3 default to guess. -// - 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 // *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 +// ethInitEmac()'s S31 branch, not this struct (same reason RMII data pins don't — +// see above); rmiiClock* are unused for RGMII (clocks are set there too). 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, diff --git a/test/python/test_build_esp32_venv_select.py b/test/python/test_build_esp32_venv_select.py index 2ba25393..e80c8963 100644 --- a/test/python/test_build_esp32_venv_select.py +++ b/test/python/test_build_esp32_venv_select.py @@ -102,6 +102,25 @@ def test_falls_back_to_newest_when_no_version_match(tmp_path, monkeypatch): assert picked.name == "idf6.1_py3.13_env", "newest overall when nothing matches" +def test_version_prefix_does_not_collide_across_minor(tmp_path, monkeypatch): + """A 6.1 target must NOT match an idf6.10 venv. The prefix carries a trailing + '_' ('idf6.1_') precisely so 'idf6.1' can't prefix-match 'idf6.10_py...'; this + pins that boundary so a refactor that drops the separator regresses loudly. + With no genuine idf6.1 venv present, it falls back to newest-overall (idf6.10) + rather than wrongly treating idf6.10 as a 6.1 match.""" + home = tmp_path / "home" + _make_venvs(home, ["idf6.1_py3.12_env", "idf6.10_py3.12_env"]) # 6.10 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 + # Must pick the exact idf6.1 venv, NOT idf6.10 (even though idf6.10 is newer). + assert picked.name == "idf6.1_py3.12_env", ( + "the trailing '_' must keep idf6.1 from matching idf6.10" + ) + + def test_none_when_no_venvs(tmp_path, monkeypatch): home = tmp_path / "home" (home / ".espressif" / "python_env").mkdir(parents=True) From a97920970f694877cfb7c769991da8db70ea7280 Mon Sep 17 00:00:00 2001 From: ewowi Date: Fri, 3 Jul 2026 19:09:33 +0200 Subject: [PATCH 3/4] Pre-merge: carry S31 lessons to decisions.md + sync NetworkModule eth docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/history/decisions.md | 12 ++++++++++++ docs/moonmodules/core/archive/NetworkModule.md | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/history/decisions.md b/docs/history/decisions.md index 2de6dcdf..ea56101a 100644 --- a/docs/history/decisions.md +++ b/docs/history/decisions.md @@ -803,3 +803,15 @@ The docs overhaul first tried to *shrink* the per-module `.md` files, then inver - **A generator that's *present but failing* must fail loud, not return empty.** `gen_api.generate()` first returned `{}` on any error — so a transient `npx` registry hiccup would ship a docs site with **zero** API pages and no red X. Split the two cases: toolchain *absent* → graceful `{}` (a contributor without Doxygen still builds the rest); toolchain *present but failing* (or under-producing vs a floor) → raise. Silent degradation in a generated artifact is worse than a hard failure, because nobody notices until a reader hits a 404. - **Enrichment by parallel agents needs an adversarial read, not just a build.** Worker agents enriched 19 headers; all compiled and generated. But the pre-merge reviewer + CodeRabbit still found real content bugs a build can't catch: a future-tense roadmap sentence (violates present-tense), a core-affinity claim describing plumbing that doesn't exist (inherited verbatim from the old `.md` — the inaccuracy pre-dated the enrichment), and an intentional default-value change (`TextEffect hue 0→128`) riding in unremarked and untested. "Compiles + generates" is table stakes; prose accuracy needs a human/reviewer pass, and an *intentional* behaviour change (even a good one) needs a test pinning it. - **A migration is only net-subtractive once the old thing is deleted — stage the deletion, but don't call it done early.** Moving the old `.md` to `archive/` (instead of deleting) kept them for cross-check but left the branch net-positive and shipped a temporary migration banner on every generated page. That's fine as an explicit *stage*, but the banner and archive are debt with a name and a removal plan, not a resting state. Mark such scaffolding "temporary / removed at Stage N" at every site so the cleanup is mechanical. + +## ESP32-S31 RGMII Ethernet bring-up: four bugs between "code compiles" and "link up" + +Bringing up the S31's on-chip 1 Gb RGMII Ethernet took four fixes, and none of them was the C++ — each was a layer *below* the feature code that a build can't catch. The general lesson: **for a hardware bring-up, "it compiles" tells you almost nothing; the truth is only in the boot log on the actual board.** + +- **A `MM_NO_ETH`-style capability gate keyed on a *filename* silently omits a new board.** `build_esp32.py` decided "does this firmware have Ethernet?" by pattern-matching the sdkconfig fragment *filename* for `.eth`. The S31 enables its EMAC in `sdkconfig.defaults.esp32s31` (no `.eth` in the name), so the gate said "no Ethernet" → compiled the `ethInit()` stub → the board booted with **zero** eth log and fell back to WiFi. The symptom (silent WiFi fallback) was maximally far from the cause (a filename heuristic in a build script). Rule: a gate that asks "does X enable feature Y?" must read what X *contains* (`CONFIG_ETH_USE_*=y`), never what X is *named*. Names drift out of the heuristic's blind spot the moment a new case doesn't follow the naming convention. + +- **A schematic/reference pin table is a hypothesis until the boot log confirms it — and ours was systematically off by one.** The reference-doc pin table (hand-transcribed from the schematic) had MDC/MDIO/TXD all one GPIO low. Wrong data pins failed loudly (`invalid TX_CTL GPIO number`), but wrong *MDIO* failed as "No PHY device detected" — three inference-steps from the typo. The chip's own IDF IO_MUX table (`esp32s31/emac_periph.c`) is the authority for RGMII data pins (they're fixed pads, not arbitrary GPIOs); trust it over any transcribed table, and verify the SMI pins against IDF's `ETH_ESP32_EMAC_DEFAULT_CONFIG` for that chip. Use PHY-addr auto-detect (`-1`) so a wrong strap assumption can't mask a working bus. + +- **A shared clock is a contended resource — the RGMII 125 MHz Tx clock can't come from a PLL that's already spoken for.** The default (AUTO) sourced the Tx clock from the MPLL, but PSRAM already ran the MPLL at 400 MHz (no integer path to 125), and CPLL couldn't synthesise 125 MHz on the 40 MHz XTAL grid either. Only the *fractional* APLL (built for exact frequencies) works. The lesson beyond Ethernet: on an SoC where one PLL feeds multiple peripherals, "pick a clock source" is a *conflict-resolution* decision, not a default — check what already owns each PLL before claiming one. + +- **Changing a Kconfig *choice* in a defaults fragment needs a clean build.** Editing `CONFIG_ETH_EMAC_RGMII_TX_CLK_SRC_*` in the fragment and doing an incremental build silently kept the *old* choice (the build dir's `sdkconfig` already had a value; defaults don't override an existing one). Two flash cycles were spent "testing APLL" that were actually still CPLL. When a sdkconfig *choice* (not a plain `=y`) changes, `rm -rf` the build dir or the test is a lie. diff --git a/docs/moonmodules/core/archive/NetworkModule.md b/docs/moonmodules/core/archive/NetworkModule.md index 3f5e81b2..dd425567 100644 --- a/docs/moonmodules/core/archive/NetworkModule.md +++ b/docs/moonmodules/core/archive/NetworkModule.md @@ -28,8 +28,8 @@ When a higher-priority connection becomes available, lower ones are torn down to - When Static: `ip`, `gateway`, `subnet`, `dns` (ipv4 controls — 4 bytes of storage each, not 16-char strings; the wire shape is still a dotted-quad string). Shown dynamically via onBuildControls. - `mDNS` (bool) — enable/disable mDNS responder -**Ethernet PHY/pin controls** (only on builds with an Ethernet driver — `platform::hasEthernet`). The PHY *driver* is compiled into the firmware per chip (internal-EMAC RMII on classic/P4, W5500 SPI on the S3); these controls pick *which* PHY a board uses and *on which pins* — runtime config, set per board in [`deviceModels.json`](../../../../web-installer/deviceModels.json) (→ `setEthConfig` → `ethInit`), seeded from the per-chip default in `platform_config.h`. `ethType` is the switch: with it at 0 no pin rows show; choosing a type reveals only that type's pins (RMII rows for LAN8720/IP101, SPI rows for W5500). A W5500 change applies **live** (the SPI driver tears down + re-inits, no reboot); an RMII change saves and applies on the next boot (status hints "restart to apply"). See [architecture.md § Config provenance](../../../architecture.md#config-provenance-mcu-devicemodel). -- `ethType` (select) — PHY type dropdown, options `None` / `LAN8720` / `IP101` / `W5500` (stored as the index 0..3, matching the `EthPhyType` enum: 0 = none, 1 = LAN8720 RMII, 2 = IP101 RMII, 3 = W5500 SPI). +**Ethernet PHY/pin controls** (only on builds with an Ethernet driver — `platform::hasEthernet`). The PHY *driver* is compiled into the firmware per chip (internal-EMAC RMII on classic/P4, internal-EMAC RGMII on the S31, W5500 SPI on the S3); these controls pick *which* PHY a board uses and *on which pins* — runtime config, set per board in [`deviceModels.json`](../../../../web-installer/deviceModels.json) (→ `setEthConfig` → `ethInit`), seeded from the per-chip default in `platform_config.h`. `ethType` is the switch: with it at 0 no pin rows show; choosing a type reveals only that type's editable pins (RMII rows for LAN8720/IP101, SPI rows for W5500; the S31's YT8531 RGMII shows just PHY-address + reset, since its data/clock pins are the chip's fixed IO_MUX pads). A W5500 change applies **live** (the SPI driver tears down + re-inits, no reboot); an RMII/RGMII change saves and applies on the next boot (status hints "restart to apply"). See [architecture.md § Config provenance](../../../architecture.md#config-provenance-mcu-devicemodel). +- `ethType` (select) — PHY type dropdown, options `None` / `LAN8720` / `IP101` / `W5500` / `YT8531` (stored as the index 0..4, matching the `EthPhyType` enum: 0 = none, 1 = LAN8720 RMII, 2 = IP101 RMII, 3 = W5500 SPI, 4 = YT8531 RGMII — the S31's on-chip 1 Gb EMAC). - `ethPhyAddr` (pin) — SMI/PHY address (typically 0 or 1). - `ethRstGpio` (pin) — PHY reset GPIO (−1 = none / module self-resets). - `ethMdcGpio`, `ethMdioGpio` (pin) — RMII SMI clock / data GPIOs (−1 = IDF default). RMII only. From 4cc5c0a1eea6384c83b9b6019e1743b045980d20 Mon Sep 17 00:00:00 2001 From: ewowi Date: Fri, 3 Jul 2026 19:18:58 +0200 Subject: [PATCH 4/4] Process Reviewer findings on PR #34 (installer desc + comment/test accuracy) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- scripts/build/build_esp32.py | 30 ++++++++++++--------- src/core/NetworkModule.h | 8 +++--- src/platform/esp32/platform_config.h | 2 +- src/platform/esp32/platform_esp32.cpp | 5 ++-- test/python/test_build_esp32_s31.py | 23 +++++++++++++++- test/python/test_build_esp32_venv_select.py | 4 +-- web-installer/firmwares.json | 2 +- 7 files changed, 52 insertions(+), 22 deletions(-) diff --git a/scripts/build/build_esp32.py b/scripts/build/build_esp32.py index 87803188..92241f44 100644 --- a/scripts/build/build_esp32.py +++ b/scripts/build/build_esp32.py @@ -150,10 +150,11 @@ "chip": "esp32s31", "fragments": ["sdkconfig.defaults", "sdkconfig.defaults.esp32s31"], "eth_only": False, - "description": "Espressif ESP32-S31 Function-CoreBoard-1 — WiFi 6 LED control " - "(RISC-V, 16 MB flash, PSRAM). The board's on-chip 1 Gbps Ethernet " - "is wired in but not yet enabled (pin map pending). esp32s31 is a " - "preview target on the v6.1 IDF line.", + "description": "Espressif ESP32-S31 Function-CoreBoard-1 — WiFi 6 + 1 Gbps " + "Ethernet LED control (RISC-V, 16 MB flash, PSRAM). The on-chip " + "EMAC drives the board's YT8531 RGMII PHY; Ethernet is preferred " + "when a cable is present, WiFi otherwise. esp32s31 is a preview " + "target on the v6.1 IDF line.", "ships": True, }, } @@ -375,14 +376,19 @@ def firmware_cmake_args(firmware: str, release: str = "", version: str = "") -> # and check for the actual enabling line rather than pattern-matching the # filename: the S31 enables EMAC in `sdkconfig.defaults.esp32s31` (no ".eth" in # the name), which a filename heuristic would miss and silently stub eth out. - eth_symbols = ("CONFIG_ETH_USE_ESP32_EMAC=y", "CONFIG_ETH_USE_SPI_ETHERNET=y") - has_eth = any( - (ESP32_DIR / frag).exists() - and any(sym in (ESP32_DIR / frag).read_text(encoding="utf-8") - for sym in eth_symbols) - for frag in spec["fragments"] - ) - if not has_eth: + eth_symbols = {"CONFIG_ETH_USE_ESP32_EMAC=y", "CONFIG_ETH_USE_SPI_ETHERNET=y"} + + def fragment_enables_eth(frag: str) -> bool: + path = ESP32_DIR / frag + if not path.exists(): + return False + # Match a whole line, not a substring: a disabled symbol is written + # "# CONFIG_ETH_USE_ESP32_EMAC is not set" (no "=y"), and a commented-out + # "# CONFIG_...=y" would substring-match but is not actually enabled. + return any(line.strip() in eth_symbols + for line in path.read_text(encoding="utf-8").splitlines()) + + if not any(fragment_enables_eth(frag) for frag in spec["fragments"]): args.append("-DMM_NO_ETH=1") return args diff --git a/src/core/NetworkModule.h b/src/core/NetworkModule.h index 96348734..1af4372d 100644 --- a/src/core/NetworkModule.h +++ b/src/core/NetworkModule.h @@ -300,8 +300,10 @@ class NetworkModule : public MoonModule { const bool isRmii = (ethType_ == 1 || ethType_ == 2); const bool isSpi = (ethType_ == 3); const bool isRgmii = (ethType_ == 4); - // RGMII (S31): MDC/MDIO + the data/clock pins are the chip's fixed IO_MUX pads, - // set in ethInitEmac() — not user-editable — so only phyAddr + reset show for it. + // RGMII (S31): the data/clock pins are the chip's fixed IO_MUX pads, set in + // ethInitEmac() (not user config); MDC/MDIO come from the per-chip ethConfigDefault + // (5/6) via the shared smi_gpio path. Neither needs a UI row, so RGMII shows only + // phyAddr + reset (the rest of the RMII rows stay hidden — isRmii-gated below). const bool isEth = isRmii || isSpi || isRgmii; // GPIO controls use addPin → a plain number input (ControlType::Pin), // not a slider: a GPIO has no meaningful range to drag. -1 = unused. @@ -665,7 +667,7 @@ class NetworkModule : public MoonModule { static constexpr const char* addressingOptions_[] = {"DHCP", "Static"}; // ethType dropdown options — index order MUST match the EthPhyType enum - // (None=0, LAN8720=1, IP101=2, W5500=3) since the Select stores the index. + // (None=0, LAN8720=1, IP101=2, W5500=3, YT8531=4) since the Select stores the index. static constexpr const char* ethTypeOptions_[] = {"None", "LAN8720", "IP101", "W5500", "YT8531"}; void startAP() { diff --git a/src/platform/esp32/platform_config.h b/src/platform/esp32/platform_config.h index 68c25757..7bff2eb9 100644 --- a/src/platform/esp32/platform_config.h +++ b/src/platform/esp32/platform_config.h @@ -48,7 +48,7 @@ constexpr bool isEsp32S3 = false; // isEsp32S31: the S31 is the only target whose EMAC is RGMII / 1 Gb (SOC_EMAC_SUPPORT_1000M), // where classic/P4 are RMII — so its Ethernet default is a distinct RGMII PHY (YT8531) with a // different pin set. Not derivable from a SOC flag (the RGMII data pins are board wiring, not a -// chip property). Used only for ethConfigDefault. +// chip property). Used by ethConfigDefault and ethInitEmac's RGMII branch/log. #ifdef CONFIG_IDF_TARGET_ESP32S31 constexpr bool isEsp32S31 = true; #else diff --git a/src/platform/esp32/platform_esp32.cpp b/src/platform/esp32/platform_esp32.cpp index 38ec2760..807a7786 100644 --- a/src/platform/esp32/platform_esp32.cpp +++ b/src/platform/esp32/platform_esp32.cpp @@ -456,7 +456,8 @@ void setEthConfig(const EthPinConfig& cfg) { ethConfig_ = cfg; } // CONFIG_ETH_USE_ESP32_EMAC keeps this function out of the S3 build (where Ethernet is // W5500-over-SPI instead). RMII and RGMII share the same MAC ctor + driver/netif/event // tail; only the interface-select + clock + data-pin config block differs, so they live -// in one function branched on phyType rather than two near-identical copies. +// in one function branched on the chip (a compile-time #ifdef, since the RGMII +// interface is S31-only) rather than two near-identical copies. #ifdef CONFIG_ETH_USE_ESP32_EMAC static bool ethInitEmac() { esp_netif_config_t netif_cfg = ESP_NETIF_DEFAULT_ETH(); @@ -696,7 +697,7 @@ bool ethInit() { #ifdef CONFIG_ETH_USE_ESP32_EMAC case ethLan8720: case ethIp101: // RMII PHYs (classic ESP32, P4) - case ethYt8531: return ethInitEmac(); // RGMII PHY (S31); ethInitEmac branches on phyType + case ethYt8531: return ethInitEmac(); // RGMII PHY (S31); ethInitEmac's RGMII block is #ifdef'd to the S31 chip #endif default: return false; // ethNone, or a PHY this firmware can't drive } diff --git a/test/python/test_build_esp32_s31.py b/test/python/test_build_esp32_s31.py index 256cf8e7..68ef23d7 100644 --- a/test/python/test_build_esp32_s31.py +++ b/test/python/test_build_esp32_s31.py @@ -23,7 +23,12 @@ ROOT = Path(__file__).resolve().parents[2] sys.path.insert(0, str(ROOT / "scripts" / "build")) -from build_esp32 import FIRMWARES, TARGET_TO_FAMILY, PREVIEW_TARGETS # noqa: E402 +from build_esp32 import ( # noqa: E402 + FIRMWARES, + TARGET_TO_FAMILY, + PREVIEW_TARGETS, + firmware_cmake_args, +) def test_s31_firmware_entry_is_well_formed(): @@ -38,6 +43,22 @@ def test_s31_firmware_entry_is_well_formed(): assert (ROOT / "esp32" / frag).is_file(), f"{frag} is referenced but missing on disk" +def test_s31_gets_ethernet_not_MM_NO_ETH(): + """firmware_cmake_args must NOT stub Ethernet out on the S31. The S31 enables its + on-chip EMAC in sdkconfig.defaults.esp32s31 (whose filename has no ".eth"), so the + eth-detection reads the fragment *content* for CONFIG_ETH_USE_ESP32_EMAC — not the + filename. This pins the regression: a filename heuristic set MM_NO_ETH and silently + compiled the ethInit() stub, so the board booted with no Ethernet (see decisions.md). + """ + assert "-DMM_NO_ETH=1" not in firmware_cmake_args("esp32s31"), ( + "the S31 firmware must build with Ethernet enabled — its EMAC is set in " + "sdkconfig.defaults.esp32s31, which the content-based check must detect" + ) + # Sanity anchor: a variant whose fragments enable an eth driver stays eth-enabled too, + # and the mechanism (content-read) is exercised for a ".eth"-named fragment as well. + assert "-DMM_NO_ETH=1" not in firmware_cmake_args("esp32-eth") + + def test_every_firmware_chip_has_a_family_label(): # generate_manifest.py builds CHIP_FAMILIES = {fw: TARGET_TO_FAMILY[chip]} — a chip # without a family entry would KeyError there. Pin that none is missing (S31 included). diff --git a/test/python/test_build_esp32_venv_select.py b/test/python/test_build_esp32_venv_select.py index e80c8963..5e613ff1 100644 --- a/test/python/test_build_esp32_venv_select.py +++ b/test/python/test_build_esp32_venv_select.py @@ -13,6 +13,7 @@ Run: `uv run --with pytest pytest test/python -q`. """ +import os import sys from pathlib import Path @@ -34,8 +35,7 @@ def _make_venvs(home: Path, names_by_age: list[str]) -> None: d.mkdir(parents=True) exe = d / build_esp32._PYTHON_EXE exe.write_text("#!fake python\n") - # Stagger mtimes deterministically (no Date.now equivalent needed): index i. - import os + # Stagger mtimes deterministically by index so the last name is newest. ts = 1_000_000 + i # strictly increasing → last name is newest os.utime(venv_root / name, (ts, ts)) diff --git a/web-installer/firmwares.json b/web-installer/firmwares.json index 28247fe6..c3da04b8 100644 --- a/web-installer/firmwares.json +++ b/web-installer/firmwares.json @@ -54,7 +54,7 @@ "chip": "esp32s31", "eth_only": false, "ships": true, - "description": "Espressif ESP32-S31 Function-CoreBoard-1 — WiFi 6 LED control (RISC-V, 16 MB flash, PSRAM). The board's on-chip 1 Gbps Ethernet is wired in but not yet enabled (pin map pending). esp32s31 is a preview target on the v6.1 IDF line." + "description": "Espressif ESP32-S31 Function-CoreBoard-1 — WiFi 6 + 1 Gbps Ethernet LED control (RISC-V, 16 MB flash, PSRAM). The on-chip EMAC drives the board's YT8531 RGMII PHY; Ethernet is preferred when a cable is present, WiFi otherwise. esp32s31 is a preview target on the v6.1 IDF line." } ] }