From 7920ab4653d079b874e1198ea4b51c414d612e31 Mon Sep 17 00:00:00 2001 From: ewowi Date: Mon, 27 Jul 2026 08:34:22 +0200 Subject: [PATCH 1/5] Add static IP (WiFi+Ethernet), S31 RGMII eth link, fix /api/types LED freeze MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static-IP addressing (DHCP/Static) now actually applies on both WiFi and Ethernet β€” the dropdown and ip/gateway/subnet/dns controls were previously inert on client interfaces. The S31's RGMII link comes up (activity LED) via the YT8531 init, and a UI refresh no longer freezes the LEDs. Bench-verified: WiFi + Ethernet static work end-to-end on the Olimex classic (live DHCP<->Static both ways). tick:150us(FPS:6700) desktop Β· tick:2454us(FPS:407) esp32(classic) Core: - NetworkModule: static IP now applies to the active client interface (WiFi STA + Ethernet) via a new platform seam (dhcpc_stop + set_ip_info + DNS); applied on bring-up and live on a DHCP<->Static toggle (no reboot); a static apply marks the interface connected without waiting for a DHCP GOT_IP event (the DHCP-less case static exists for). Eth-degraded status ("Ethernet detected: no address assigned") when a cable is up but leaseless. ethPhyAddr fixed to a signed int16 so the -1 auto-detect sentinel round-trips (a uint8 mangled it to 31, so the PHY was never found). Named kAddressingStatic/Dhcp constants. - platform (esp32 + platform.h + desktop): new netSetStaticIPv4 / netSetDhcp seam (one NetIface enum serves STA + Eth, mirroring the SoftAP static block); ethYt8531BoardInit brings the S31 RGMII link up (auto-negotiation re-enable + RGMII Tx/Rx delays); the eth and STA link-up handlers re-pin static on reconnect instead of restarting DHCP (ethStatic_/staStatic_ + stored octets); desktop stubs are no-ops. - Control: numberField flag renders a Uint8/Uint16/Int16 as a plain number input (for identity values like a PHY/MDIO address), not a slider. - ParallelLedDriver: swapPeripheral fires the render-quiesce hook only when freeing a real backend (peripheral_ non-null), so an /api/types throwaway probe no longer tears down the live render split (the "UI refresh freezes the LEDs" bug). UI: - app.js: render a numberField control as a plain number input, on both the initial createControl path and the WS-patch path. Tests: - unit_NetworkModule_ethernet: ethPhyAddr int16/-1/numberField regression; the addressing Select + static-IP control contract; the desktop static-addressing seam is a safe no-op. - unit_Drivers_rendersplit: a detached ParallelLedDriver's peripheral swap leaves a live split untouched (pins the probe-freeze fix). Docs / CI: - backlog-core: S31 RGMII eth at 100M β€” bench-confirmed the failure is data-plane TX corruption (static bypasses DHCP; ARP resolves but unicast drops), not a DHCP-handshake quirk; a bug to fix (TXC reconfiguration or a gigabit link), never a reason to weaken the AP->STA->ETH promotion cascade. P4-400 rev-3 note. - lessons: /api/types probe tore down the live render split via a global active-instance hook (board-agnostic; the "only the S31 does it" was a timing artifact). - Saved plans: static-IP (shipped) + S31 eth-DHCP (in-flight); a peripheral-grid-sweep benchmark scenario. Reviews: - πŸ‘Ύ WiFi static only engaged after a DHCP lease (wifiStaConnected_ set on GOT_IP), so it was dead on a DHCP-less network: fixed β€” apply + mark connected at L2 association (WIFI_EVENT_STA_CONNECTED), mirroring the eth handler. - πŸ‘Ύ netSetDhcp didn't clear the connected flag, so a live Static->DHCP toggle could wedge at 0.0.0.0: fixed β€” clear ethConnected_/wifiStaConnected_ (GOT_IP re-sets it on a lease). - πŸ‘Ύ render-split probe guard: the peripheral_ gate is correct (a real backend-free must quiesce regardless of tree attachment); fixed the drifted test/code comments that said "parent" (the reviewer's parent-gate suggestion broke the backend-free quiesce test, confirmed by ctest). - πŸ‘Ύ addressing magic literal (x5): fixed β€” kAddressingStatic/Dhcp constants. - πŸ‘Ύ redundant forward decl splitting a doc comment: fixed β€” removed (platform.h already declares it). - πŸ‘Ύ connected flag set without a link/association check (dead-link race): fixed β€” gated on ethLinkUp_/wifiStaAssociated_. Co-Authored-By: Claude Opus 4.8 --- docs/backlog/backlog-core.md | 34 + docs/history/lessons.md | 14 + ...n-20260726 - S31 RGMII eth DHCP at 100M.md | 70 + ...atic IP for WiFi and Ethernet (shipped).md | 64 + src/core/Control.h | 19 +- src/core/HttpServerModule.cpp | 1 + src/core/NetworkModule.h | 189 ++- src/light/drivers/ParallelLedDriver.h | 15 +- src/platform/desktop/platform_desktop.cpp | 4 + src/platform/esp32/platform_esp32.cpp | 229 ++- src/platform/platform.h | 15 + src/ui/app.js | 27 + .../light/scenario_Layouts_mutation.json | 4 +- .../light/scenario_peripheral_grid_sweep.json | 1434 +++++++++++++++++ .../unit/core/unit_NetworkModule_ethernet.cpp | 72 + test/unit/light/unit_Drivers_rendersplit.cpp | 36 + 16 files changed, 2201 insertions(+), 26 deletions(-) create mode 100644 docs/history/plans/Plan-20260726 - S31 RGMII eth DHCP at 100M.md create mode 100644 docs/history/plans/Plan-20260726 - Static IP for WiFi and Ethernet (shipped).md create mode 100644 test/scenarios/light/scenario_peripheral_grid_sweep.json diff --git a/docs/backlog/backlog-core.md b/docs/backlog/backlog-core.md index 83f41bc6..9be45d98 100644 --- a/docs/backlog/backlog-core.md +++ b/docs/backlog/backlog-core.md @@ -505,3 +505,37 @@ Mitigated in practice: prime-only fires exactly one EOF per frame (no intra-fram **Fix:** widen the backstop's condition so a stuck prime-only ring finalizes too β€” likely a single "any ring frame whose wire time has elapsed with `busy` still set" oracle that subsumes both ring branches, calling the shared `finalizeStalledTransfer`. Verify on the expander wall (prime-only = the small-strand ring config), since the ring recovery is not desktop-testable. **Related (same file, same wall dependency):** the whole-frame backstop stops the hardware before draining the FIFO and the EOF ISR drops a firing against an empty FIFO, which rejects a late EOF that arrives after recovery. A tighter guarantee β€” an ISR that has *already passed* the empty-FIFO guard cannot then clear a freshly-recovered `busy` or give `wireFree` for the next transfer β€” would need explicit serialization (disable the EOF interrupt across finalize+rearm, or an explicit recovery/rearm state the ISR checks). The window is extremely narrow (an ISR in-flight at the instant of finalize) and stopping GDMA+LCD first already disables the source; the hardened version is an ISR-concurrency change that must be proven on the wall, not landed blind. + +## ESP32-P4 400 MHz on rev-3+ silicon + +**Found:** bench, 2026-07-25 (Quindor Discord question about the P4's 400 MHz). + +The P4 build runs at **360 MHz** because IDF's `Kconfig.cpu` caps a `SELECTS_REV_LESS_V3` build (which we set, so a stock binary boots on the v0.x/v1.x P4 chips in the field) at 360; 400 MHz is IDF's default only for rev β‰₯ 3. Forcing 400 on our bench P4 (a **rev v1.3** chip) was tried and **bootloops** β€” `assert failed: esp_clk_init clk.c:105` β€” so 400 is a genuine hardware limit on pre-rev-3 silicon, not marginal stability. The ~11% compute headroom is left on the table for rev-3+ owners. + +**Two ways to reach it, both deferred until rev-3 P4 hardware is on the bench to validate:** +- **A separate `esp32p4-400` variant** β€” `sdkconfig` with `ESP32P4_REV_MIN` = rev 3 + `CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ_400`, so IDF's normal boot-time clock init sets 400 and the bootloader refuses to run on a pre-rev-3 chip (safe). The web installer offers it only to rev-3+ boards. This is the projectMM-idiomatic per-variant path. +- **A runtime bump** β€” keep the 360 build (boots everywhere) and, at `app_main`, read `esp_chip_info().revision` and if β‰₯ 300 call `rtc_clk_cpu_freq_mhz_to_config(400)` + `rtc_clk_cpu_freq_set_config`. One binary, self-selecting. Riskier: a runtime CPU-PLL change on the P4 couples to the flash/PSRAM clock trees configured for 360, so it needs validation on real rev-3 hardware before shipping. + +Neither ships until a rev-3 P4 can prove 400 runs clean β€” no untested clock config, per the same rule the S31/320 and this P4/400 bootloop both taught. + +## ESP32-S31 RGMII Ethernet: DHCP at 100 Mbps (link-speed Tx-clock mismatch) + +**Found:** bench, 2026-07-26. Design record: [Plan-20260726 - S31 RGMII eth DHCP at 100M](../history/plans/Plan-20260726%20-%20S31%20RGMII%20eth%20DHCP%20at%20100M.md). + +The S31's 1 Gb RGMII EMAC (YT8531 PHY) leases fine on a **gigabit** switch but **never** completes DHCP on a **10/100** switch (the bench GL-AR300M). Confirmed on hardware that this is **not a regression** β€” it fails identically at the original S31 bring-up commit `d5ee07c` built against its exact pinned IDF (`0d928780081` / v6.1-dev-5215). The firmware logs `Ethernet no IP (DHCP timeout), cascading` and falls back to WiFi. Every "eth works" reading in the field was on a 1 Gb switch. + +**Root cause (IDF source-traced).** On RGMII the MAC's Tx clock (TXC) must be 125 MHz at 1000M but **25 MHz at 100M** (`emac_esp32_set_speed`). The MAC is reprogrammed only when the generic 802.3 PHY driver's poll sees a link_status *transition* (`updt_link_dup_spd` β†’ `on_state_changed(ETH_STATE_SPEED)`). Because the YT8531 needs its auto-negotiation manually re-enabled before `esp_eth_start` (it disables it on reset), the first negotiation can latch link-UP before the poll's first read, so the poll sees no transition, so `set_speed(100M)` never runs and TXC stays at its 125 MHz install default. At 100M every Tx frame then clocks out garbled and the switch drops it (Rx still works β€” it rides the PHY-recovered RXC): link up, DISCOVER sent, no OFFER ever. + +**Approaches tried on the bench (neither shipped):** +- **`esp_eth_stop()` + `esp_eth_start()` bounce after start** β€” DID produce the first-ever eth lease (`.125`), proving the mechanism, but `esp_eth_stop` tears the netif down (releases the lease, resets dhcpc to INIT, clears the IP), which races the NetworkModule 15 s cascade window and the double `applyHostname` β†’ non-deterministic (sometimes eth, sometimes WiFi; when eth, the netif IP was half-applied). +- **In-place `link_status = ETH_LINK_DOWN` + `phy->get_link()`** (netif-preserving, from the CONNECTED handler) β€” ran cleanly (no crash from the re-entrant poll) and forced a second link-up, but eth **still** DHCP-timed-out on the bench. Open question: whether `set_speed` actually fired (the `working in 100Mbps` proof line is `ESP_LOGD`, likely filtered by the esp_eth component log level β€” absence is not proof). + +**Already landed (the link-up half):** `ethYt8531BoardInit` (re-enables the YT8531's auto-negotiation β€” disabled on reset β€” plus the RGMII Tx/Rx clock delays) brings the RGMII **link** up (speed/duplex negotiated, activity LED lit); and the `ethPhyAddr` int16 fix (the `-1` auto-detect sentinel a `uint8` mangled to 31, with its regression test) makes the PHY addressable. What remains open is only the **100M Tx-clock (TXC) reconfiguration** so frames actually flow at 100M β€” the DHCP half below. + +**Next (needs hardware) β€” first, the gigabit test + a scope decision:** test the current firmware on a **1 Gb switch** (confirm `ETH-DIAG` reports "link up at 1000M full", then look for `Ethernet got IP`). At 1000M the MAC's Tx clock is already 125 MHz (the install default), so the 100M-specific TXC bug is absent β€” eth is expected to lease, matching the earlier field success on gigabit. **If it leases at 1000M, there is a product decision to make: whether to support 100M routers at all**, or to state that the S31 (a 1 Gb board) requires a gigabit switch and close this item as "won't-fix at 100M." Only if 100M support is deemed in-scope do the TXC steps below apply. + +**If 100M support is kept** (the TXC path): (1) if it links at 1000M but still no lease, sweep the RGMII delays (`MM_YT8531_{RX,TX}_DELAY`) for this board's trace lengths; (2) for 100M, add a decisive speed-readback diagnostic (`ETH_CMD_G_SPEED` before/after the resync + raise the esp_eth log level) to confirm whether `set_speed` runs; (3) if the in-place `link_status`-reset poke can't trigger `set_speed`, fall back to the stop/start bounce and make it deterministic by widening the S31 eth-DHCP cascade window (> 15 s) and suppressing the double `applyHostname`. + +**The failure is data-plane TX corruption, not a DHCP-handshake quirk (bench-confirmed 2026-07-26 via static IP).** Setting a static IP on the S31 at 100M bypasses DHCP entirely, yet the interface is still unreachable: ARP for the device resolves to the eth MAC (`30:ed:a0:f3:d4:68`) β€” so a broadcast round-trips β€” but unicast (ping / HTTP) is 100% dropped. That rules out "only the DHCP exchange is broken" and pins it to garbled unicast frames on the 100M Tx path (the TXC issue). So static addressing is NOT a workaround for 100M; the TXC fix (or a gigabit link) is genuinely required. **This is a bug to FIX, never a reason to weaken the AP β†’ STA β†’ ETH promotion cascade** β€” Ethernet always outranks WiFi when a cable is present, unconditionally. Interim behaviour on the S31 at 100M with a cable in Static mode: it promotes to Ethernet (as it must) but can't pass traffic; the user recovers by unplugging the cable (link-down cascades back to WiFi). Acceptable only as a temporary state for this one board's open bug, not a design. + +**Related symptom β€” the "Ethernet detected: no address assigned" degraded warning is intermittent on the S31.** In DHCP mode, that warning is meant to appear when the eth link is up but leaseless past the 15 s window (`NetworkModule::ConnectedSta`). On the S31 at 100M it shows only *sometimes*, because the marginal 100M link *flaps*: each `ETHERNET_EVENT_DISCONNECTED` makes `tick1s()` reset `ethLinkUpAt_` (the degraded clock), so the 15 s threshold is often never reached. This is the same root cause (a bad 100M physical link), so it resolves when the TXC/link issue is fixed. Two robustness follow-ups if it's ever decoupled: (a) don't reset the degraded clock on a *brief* link blip (debounce link-down), and (b) note that a lower `check_link_period_ms` makes the flapping more visible β€” the default 2000 ms is deliberately kept (a 500 ms poll surfaced the flaps and suppressed the warning entirely). diff --git a/docs/history/lessons.md b/docs/history/lessons.md index 0adc67cb..d6bd4212 100644 --- a/docs/history/lessons.md +++ b/docs/history/lessons.md @@ -362,3 +362,17 @@ The MoonI80 shift ring "wedged" after a loopback ONβ†’OFF (and after any control - **One control edit rebuilt the bus TWICE, and the rapid second rebuild wedged.** Within a single `Scheduler::prepareTree()` sweep, the Drivers container's `prepare()` pushes every child's correction (`passBufferToDrivers` β†’ `rebuildCorrection` β†’ the driver's `onCorrectionChanged()`, which did a full `reinit()`), and THEN the same sweep runs the child's own `prepare()` β†’ `reinit()` again. Two peripheral teardown+rebuilds back-to-back; the second came up before the first's ring had fired a single frame, and its GDMA EOF never fired. The fix is a dirty-gate: `onCorrectionChanged()` reinits only when `frameBytes_` actually changed (a real RGB↔RGBW switch), so a light-count/window edit β€” which the correction push does NOT resize β€” rebuilds once, in `prepare()`. **General: a cross-cutting "rebuild on change" that fires from two independent triggers in the same sweep must be idempotent or gated on a real delta; two reinits of a stateful peripheral in quick succession is its own failure mode, distinct from either reinit alone.** Both bugs presented identically ("output stalled, `wireUs=β€”`, reboot to fix"), which is why the peripheral-level theories stuck: the *symptom* was at the peripheral, the *cause* was two layers up in memory-fit routing and control-flow. **General: a symptom that manifests in an interrupt/peripheral is not evidence the bug lives there β€” instrument the decision that led to the peripheral call (which path was chosen, how many times it ran) before diffing registers.** The false-positive risk cuts the other way too: the bench probe flagged "WEDGE? first frame advanced EOF by 0" on every rebuild, which was just the first-frame-has-no-predecessor artifact of the probe, not a real wedge β€” the ring was driving fine underneath. + +## `/api/types` froze the LEDs on every UI refresh β€” a throwaway probe reached the LIVE render split through a global hook + +"Refresh the web UI and the LEDs freeze" β€” reported first on an ESP32-S31, so it read as S31-specific silicon: a new board on an IDF-beta. It was not. It reproduced on the P4 too (there the fast core recovered so fast the freeze was invisible to the eye, but the multicore worker task `mmEncode` provably vanished from the FreeRTOS task list on every refresh), and it fired with **every** LED driver, at the stock clock, on a flash-erased board. The one invariant: `multicore` ON + a UI refresh. + +**Root cause.** A browser refresh fetches `GET /api/types`, and `HttpServerModule::writeTypeDefaults` builds a **throwaway probe** of each registered type (`ModuleFactory::create` β†’ `defineControls` β†’ read defaults β†’ destroy) to publish its default control values. Constructing a `ParallelLedDriver` probe runs `selectDefaultPeripheral` β†’ `swapPeripheral`, which called `MoonModule::notifyQuiesceRender()` **unconditionally**. That hook is a process-global that resolves to the *live* Drivers via the static `ActiveInstance` seat (`Drivers::active()`), and it calls `stopEncodeTask()`. So a detached probe β€” an instance that was never in the tree β€” **tore down the running render split**, and no `prepareTree()` re-engaged it. `multicore` still read `true` (the switch state is not the engaged state), so the UI showed nothing wrong; on a slow-enough core the un-overlapped output visibly froze. + +**Fix (one line).** `swapPeripheral` fires the quiesce hook only when `peripheral_ != nullptr` β€” i.e. only when a real backend is about to be `delete`d, which is the exact use-after-free the hook exists to prevent. A fresh instance's first swap (the default-peripheral build, no prior backend) no longer touches the live worker; a genuine live swap (POST `/api/control` on `peripheral`, with a backend to free) still quiesces. The existing "a live swap quiesces the worker" test still passes; a new `unit_Drivers_rendersplit` case pins that a detached ParallelLedDriver's swap leaves a live split untouched (fails without the guard). + +**General lessons:** +- **A `create β†’ introspect β†’ destroy` probe must be side-effect-free, and "read the defaults" is not.** Any lifecycle call on a throwaway instance (`defineControls`, `release`, a constructor that selects a default) can reach global/static state β€” a registry, an active-instance seat, a hook β€” and mutate the *live* system. If a type publishes to a process-global on construction or teardown, probing it is not read-only. (Same family as the earlier registerType `T probe` stack-overflow: a probe instance is a real object with real side effects.) +- **A global hook keyed off a static "active" seat is invisible at its call sites.** `notifyQuiesceRender()` reads innocent β€” but it always hits whatever `Drivers::active()` currently points at, which is never the caller when the caller is a probe. A hook that reaches "the live one" needs an attached/owned check at the fire site, or it fires for instances that have no business touching the live system. +- **"Board-specific" was a timing artifact, not a hardware fact.** The freeze was identical on every board; only the *visibility* differed with core speed. Chasing "what's different about the S31" (clock, IDF-beta SMP, RMT refill) burned days. The break came from a deterministic, board-agnostic trigger (`GET /api/types` alone kills the worker) captured with a `this`-pointer printf that showed the teardown hitting the *live* Drivers, not the probe. **When "only board X does it" resists every hardware theory, find the software action that reproduces it on demand and instrument identity (which instance), not just occurrence (that it happened).** +- **The switch state is not the engaged state.** `multicore == true` while the split was actually disengaged sent the whole investigation sideways more than once. A control that can read ON while its mechanism is OFF is its own trap; the trustworthy signals were the FreeRTOS task list (`mmEncode` present?) and `renderWait` (a number vs `β€”`). diff --git a/docs/history/plans/Plan-20260726 - S31 RGMII eth DHCP at 100M.md b/docs/history/plans/Plan-20260726 - S31 RGMII eth DHCP at 100M.md new file mode 100644 index 00000000..34a20f11 --- /dev/null +++ b/docs/history/plans/Plan-20260726 - S31 RGMII eth DHCP at 100M.md @@ -0,0 +1,70 @@ +# Plan: ESP32-S31 RGMII Ethernet DHCP at 100 Mbps (fix the TXC-speed mismatch) + +## Context + +The ESP32-S31's on-chip EMAC is 1 Gb RGMII (YT8531 PHY). On a **gigabit** switch it leases fine (link at 1000M, MAC Tx clock TXC = 125 MHz, the driver's install default). On a **10/100** switch β€” like the bench GL-AR300M β€” the link negotiates to **100M**, where RGMII requires **TXC = 25 MHz**. The MAC never gets reconfigured to 25 MHz, so every Tx frame clocks out garbled and the switch drops it. Symptom: link up, DHCP DISCOVER sent, **no OFFER ever** β†’ `NetworkModule: Ethernet no IP (DHCP timeout), cascading` β†’ falls back to WiFi. + +**This is not a regression.** Verified on hardware at commit `d5ee07c` built against its exact pinned IDF (`0d928780081` / v6.1-dev-5215, confirmed via both `setup_esp_idf.py` and `release.yml:169`): eth DHCP times out on the 10/100 router there too. Eth has **never** worked at 100M on this firmware; the earlier "DHCP confirmed" success was on a 1 Gb switch. So the goal is a genuine new capability: **eth DHCP at 100M**, without regressing 1000M. + +**Root cause (confirmed by IDF source trace).** The MAC's `emac_esp32_set_speed` (`esp_eth_mac_esp.c:422`) reprograms TXC per link rate. It runs **only** via `on_state_changed(ETH_STATE_SPEED)`, emitted from the generic 802.3 PHY driver's `updt_link_dup_spd` (`esp_eth_phy_802_3.c:217`) β€” and that is gated on a link_status **transition** (`if (phy_802_3->link_status != link)`, `:235`). Our firmware manually re-enables the YT8531's auto-negotiation (a documented YT8531-reset quirk) *before* `esp_eth_start`; negotiation can complete and latch link-UP **before** the PHY poll task's first read, so the poll sees no transition, so `set_speed(100M)` never runs, so TXC stays at its 125 MHz install default. RX still works (it rides the PHY-recovered RXC), which is why DHCP OFFERs to the *WiFi* interface kept masking this. + +**Why the current WIP is not the final fix.** The stashed WIP corrects TXC with an `esp_eth_stop()` + `esp_eth_start()` bounce placed after the netif glue is attached. It *did* produce the first-ever eth lease (`.125`) β€” proving the mechanism β€” but `esp_eth_stop` unconditionally tears the netif down: it releases the DHCP lease, resets `dhcpc_status` to INIT, and sets the netif IP to `0.0.0.0` (`esp_netif_lwip.c:1308-1360, 1899-1948`). That re-acquisition races the NetworkModule cascade's 15 s eth-DHCP window and the double CONNECTED event (which re-runs `applyHostname`'s dhcp stop/restart), giving the non-deterministic "sometimes eth `.125` with a half-applied netif, sometimes WiFi `.212`" behaviour observed on the bench. + +## The fix: force one PHY link-state transition, without touching the netif + +IDF exposes exactly the public surface to re-drive the speed push without a driver stop/start (all confirmed present in the pinned IDF): +- `esp_eth_get_phy_instance(handle, &phy)` β€” `esp_eth_driver.h:403` +- `esp_eth_phy_into_phy_802_3(phy)` β€” public inline cast, `esp_eth_phy_802_3.h:372` +- `phy_802_3_t::link_status` β€” public member, `esp_eth_phy_802_3.h:27` +- `phy->get_link(phy)` β€” public fn-ptr, `esp_eth_phy.h:130` + +**Mechanism:** after `esp_eth_start` and once the link has settled UP, obtain the PHY instance, set `link_status = ETH_LINK_DOWN` **directly** (NOT via `esp_eth_phy_802_3_set_link`, which would post DISCONNECTED and disturb the netif), then call `phy->get_link(phy)`. The poll now sees a fresh DOWNβ†’UP transition, re-reads the negotiated 100M, emits `ETH_STATE_SPEED`, and `emac_esp32_set_speed` lands TXC at 25 MHz β€” **the netif, DHCP client, and any lease are never touched.** This removes both remaining problems (netif teardown race + IP-not-applied) by construction, because there is no teardown. + +A no-op at 1000M (the re-detected speed is 1000M, TXC already 125 MHz). S31-only (`#ifdef CONFIG_IDF_TARGET_ESP32S31`); the classic/P4 RMII path derives Tx clock from the fixed 50 MHz REF_CLK and has no per-speed TXC. + +## Design + +**File: `src/platform/esp32/platform_esp32.cpp`** (the S31 eth init `ethInitEmac`, ~line 703-744) + +1. **Remove** the stashed `esp_eth_stop()` + `esp_eth_start()` bounce block (lines ~723-740). +2. **Add** a new S31-only helper `ethYt8531ForceSpeedResync(esp_eth_handle_t)` that: + - `esp_eth_get_phy_instance(handle, &phy)`; on error, log-warn and return (non-fatal, as with `ethYt8531BoardInit`). + - `phy_802_3_t* p = esp_eth_phy_into_phy_802_3(phy);` + - `p->link_status = ETH_LINK_DOWN;` + - `phy->get_link(phy);` β€” re-detects and pushes speedβ†’MAC. + - Keep the existing `ETH-DIAG`/diagnostic prints controlled by the temp-debug flags until the PO signs off end-to-end. +3. **Call site:** the link settles UP a few hundred ms after `esp_eth_start`. Rather than a fixed long `vTaskDelay` in `ethInitEmac` (which eats the cascade's eth-DHCP budget and blocks boot), invoke the resync from the **`ETHERNET_EVENT_CONNECTED` handler** (`ethEventHandler`, ~line 485) on the S31 β€” that fires exactly when the link first comes UP, i.e. the first (speed-less) transition. Forcing `link_status = DOWN` + `get_link` there triggers the immediate second transition that carries the speed. Guard it to run **once** per link-up (a static/So-far flag reset on DISCONNECTED) so it doesn't recurse. This also means it self-heals on a live cable re-plug, not just at boot β€” consistent with the "no reboot to apply" principle. + - Confirm ordering against `applyHostname(ethNetif_)` already in that handler: run the speed resync **before** `applyHostname`, so the DHCP client that `applyHostname` (re)starts runs on a MAC whose TXC is already correct. + +**Keep the confirmed-good WIP fixes** (independent of this change, already validated): +- `ethYt8531BoardInit` (autoneg re-enable + RGMII delays) β€” unchanged; still needed. +- `ethPhyAddr` `int16_t` + `addInt16(-1,31)` + `numberField` β€” unchanged. +- The `/api/types` probe-freeze guard in `ParallelLedDriver.h` + its regression test β€” unrelated, keep. + +**Temp debug (remove only on PO end-to-end sign-off, per standing instruction):** the ARP/netif/IP/UDP/ETH/DHCP debug flags in `sdkconfig.defaults.esp32s31` + `sdkconfig.defaults`, and the `ETH-DIAG`/`ETH-MAC` printfs. Leave in place through bench verification. + +## Regression tests (per PO directive: every fix pinned in a test) + +The TXC-resync is platform/hardware logic (esp_eth calls) that can't run on the desktop host. Pin what *can* be pinned at the seam, host-side: +1. **`ethPhyAddr` sentinel** (`test/unit/core/` NetworkModule control test): assert the control is `int16` with range `[-1, 31]` and default `-1` (so a `uint8` cast can never again mangle `-1`β†’31), and that `numberField` is set on it. This is pure control-metadata, host-testable. +2. **Speed-resync contract** (documented + asserted where possible): the platform desktop stub already returns `ethConnected()/ethLinkUp() == false`; add a focused unit or a documented invariant that the S31 eth CONNECTED path calls the resync before `applyHostname`. Since the esp_eth calls are ESP32-only, pin the *ordering/contract* via a small seam (e.g. a testable free function or a documented sequence asserted by a comment + a platform-boundary check) rather than mocking IDF. Exact seam chosen during implementation; the bar is "a future edit that drops the resync or reorders it fails a check," not "mock the whole IDF." +3. Keep the existing render-split probe-freeze regression test (`test/unit/light/unit_Drivers_rendersplit.cpp`) β€” already green. + +Document the root cause + fix in `docs/history/lessons.md` (S31 RGMII 100M TXC), and update the memory note `s31-ethernet-dhcp-rx-open` outcome once bench-verified. + +## Verification + +1. **Host gates:** `cmake --build build` (zero warnings) + `ctest` + `uv run moondeck/scenario/run_scenario.py` green. +2. **S31 build:** `uv run moondeck/build/build_esp32.py --firmware esp32s31 --skip-idf-pin-check` (current IDF release/v6.1) β€” zero warnings. +3. **Bench (the real test), on the 10/100 GL-AR300M:** flash the S31, capture serial. Success criteria: + - Serial shows the speed resync running and (with debug on) `working in 100Mbps` from the EMAC driver, i.e. TXC reconfigured. + - `NetworkModule: Connected via Ethernet β€” Eth: ` and an `IP_EVENT_ETH_GOT_IP` β€” a **deterministic** eth lease across repeated reboots (not sometimes-WiFi). + - **PO-observed:** the S31 is reachable over the Ethernet cable at its leased IP (`http://` / `MM-S31.local`) β€” ARP resolves, ping replies, UI loads. This is the measurement; agent serial logs are supporting evidence only. + - Re-plug the cable live β†’ eth re-leases without reboot (self-heal). +4. **No 1000M regression:** when a gigabit switch is available (PO has one, not on hand now), confirm eth still leases at 1000M (the resync is a no-op there). + +## Notes / risks + +- **Direct `link_status` write** touches a public struct member but bypasses the `set_link` helper deliberately (to avoid the DISCONNECTED post that would disturb the netif). This is a bespoke touch of IDF internals β€” carry a one-line comment at the site naming *why* (netif-preserving) per the "bespoke choices carry their reason" principle. It uses only public headers (`esp_eth_phy_802_3.h`), not private ones. +- If `phy->get_link(phy)` inside the event handler proves to re-enter awkwardly (it runs on the event-loop task, same as the poll), fall back to just `link_status = DOWN` and let the **next scheduled poll** (`check_link_period_ms`) do the re-detect β€” one poll interval later, still well inside the 15 s cascade window. Decide by bench observation. +- The stashed WIP's `esp_eth_stop/start` bounce is **removed**, not kept as a fallback β€” it's the racy path this replaces (no-hacks floor). diff --git a/docs/history/plans/Plan-20260726 - Static IP for WiFi and Ethernet (shipped).md b/docs/history/plans/Plan-20260726 - Static IP for WiFi and Ethernet (shipped).md new file mode 100644 index 00000000..8728ebf8 --- /dev/null +++ b/docs/history/plans/Plan-20260726 - Static IP for WiFi and Ethernet (shipped).md @@ -0,0 +1,64 @@ +# Plan: Static IP support for WiFi STA + Ethernet (wire the existing controls to the netif) + +## Context + +The Network module already shows an `addressing` dropdown (**DHCP / Static**) and four IPv4 controls (`ip`, `gateway`, `subnet`, `dns`), stored as `staticIp_/staticGateway_/staticSubnet_/staticDns_` (uint8[4] octets) in `NetworkModule.h` and persisted. But **nothing applies them**: `esp_netif_set_ip_info` is called only for the SoftAP (`platform_esp32.cpp:1122`). Neither `wifiStaInit` nor `ethInitEmac` reads `addressing_` or the static octets, so both the STA and ETH netifs always run their DHCP client regardless of the dropdown. The controls are inert on both interfaces. NetworkModule's own docstring (`:89`) says the `addressing` selector + static-IP controls are meant to "remain" and apply to whichever interface is active, so this closes a gap that was designed for but never wired. + +The product owner asked for static support on Ethernet "just like wifi"; since WiFi static is *also* inert, the agreed scope is to make static actually work for **both** STA and ETH. + +**Bonus experiment (S31 at 100M).** A static IP bypasses DHCP entirely. The S31 links at 100M but can't complete the DHCP handshake (the open TXC issue, see backlog-core.md). If a static IP makes Ethernet reachable at 100M, that proves the RX/unicast path works and only the DHCP handshake was the 100M blocker: a simpler path than the TXC resync, and possibly enough to call 100M "supported via static." The plan flags this as a bench test. + +## Design + +Mirror the AP static path (`platform_esp32.cpp:1115-1124`: `dhcps_stop` β†’ build `esp_netif_ip_info_t` β†’ `set_ip_info` β†’ restart) in the **client** form for STA and ETH: `esp_netif_dhcpc_stop(netif)` β†’ `esp_netif_set_ip_info(netif, &info)` β†’ `esp_netif_set_dns_info(netif, ...)`. No dhcpc restart β€” static means the client stays stopped. + +**Stage 1 β€” platform seam.** Add one narrow platform function, `platform.h`: +``` +void netSetStaticIPv4(NetIface iface, const uint8_t ip[4], const uint8_t gw[4], + const uint8_t mask[4], const uint8_t dns[4]); +``` +`NetIface` is a tiny enum (`NetSta`, `NetEth`) so one function serves both, resolved to `staNetif_` / `ethNetif_` internally. If all-zero `ip` is passed, treat as "no static / use DHCP" (defensive). The ESP32 impl does the dhcpc_stop/set_ip_info/set_dns sequence on the resolved netif; the desktop stub is a no-op (like the other net stubs). This is a domain-neutral, single-purpose primitive β€” the recognizable "set static addressing on an interface" call, not a bespoke per-interface duplicate. (Naming: `netSetStaticIPv4` matches the existing `wifiStaGetIPv4` / `ethGetIPv4` octet-getter convention in `platform.h`.) + +**Stage 2 β€” apply on bring-up.** In `NetworkModule`, after each interface is initialized and its netif exists, when `addressing_ == 1` (Static) call `platform::netSetStaticIPv4(...)` for that interface: +- **STA:** right after `wifiStaInit` succeeds (the STA netif exists once `esp_wifi_start` ran; apply before/instead of the DHCP client taking a lease). Because the static apply stops the dhcp client, it must run once the netif is created β€” call it from NetworkModule right after `wifiStaInit()` returns true, guarded by `addressing_ == 1`. +- **ETH:** the eth netif exists after `ethInit()`; apply when Static. The eth DHCP client is started from the IDF CONNECTED handler, so for a clean static setup the apply must also run on link-up. Simplest robust approach: NetworkModule calls `netSetStaticIPv4` for eth when it observes eth link-up in Static mode (in the `WaitingEth` / cascade path), and the platform's eth CONNECTED handler skips `applyHostname`'s dhcpc_start when a static IP is set. Exact wiring settled in implementation; the invariant is "Static mode β†’ dhcpc stopped + ip_info set, on whichever netif is active, without a DHCP round." +- On Static, `ethConnected_` / the STA "got IP" state must be considered connected **without** waiting for a DHCP `GOT_IP` event (there won't be one). NetworkModule treats "Static + netif has the static IP applied" as connected: set the connected state directly after applying, rather than waiting on `ethConnected()` / `wifiStaConnected()` which key off DHCP/association-IP events. + +**Stage 3 β€” apply live on toggle.** Toggling `addressing` (or editing a static field) already triggers `rebuildControls()` (the Selectβ†’hidden re-eval). Extend the live path: when `addressing_` flips to Static, apply the static config to the currently-active interface immediately; when it flips back to DHCP, restart the DHCP client (`esp_netif_dhcpc_start`) so the device re-leases without a reboot (the "no reboot to apply" principle). This routes through the existing `onControlChanged` / dirty path NetworkModule already uses for live network reconfig. + +**Stage 4 β€” status.** `updateStatusIP()` already reads the netif IP via `currentIp()` β†’ `ethGetIPv4`/`wifiStaGetIPv4`, which returns the *applied* static IP (set_ip_info makes it the netif address). So the status line shows the static IP with no change. The eth-degraded warning path only fires in DHCP mode (Static won't hit the DHCP-timeout branch), so it composes cleanly. + +## Files + +- `src/platform/platform.h` β€” declare `NetIface` enum + `netSetStaticIPv4(...)`. +- `src/platform/esp32/platform_esp32.cpp` β€” implement it (dhcpc_stop/set_ip_info/set_dns on the resolved netif); teach the eth CONNECTED handler + `applyHostname` to skip dhcpc_start when static is active (a `bool ethStatic_` / STA equivalent set by the setter, or a param). +- `src/platform/desktop/platform_desktop.cpp` β€” no-op stub. +- `src/core/NetworkModule.h` β€” call `netSetStaticIPv4` on bring-up + on live toggle for STA and ETH; treat Static as connected without a DHCP event; DHCP-restart on toggle back. + +## Reuse (don't reinvent) + +- The AP static block (`platform_esp32.cpp:1115-1124`) is the exact server-side shape; the client side is the same minus dhcpsβ†’dhcpc and plus DNS. +- `formatDottedQuad` / the `ControlType::IPv4` octet storage already parse/hold the addresses; the octets go straight into `esp_netif_ip_info_t` via `IP4_ADDR` / `esp_netif_set_ip_info`. +- `currentIp()` / `updateStatusIP()` already surface the netif IP β€” no status rework. + +## Tests (regression, per the project rule) + +Host-testable seam (`test/unit/core/unit_NetworkModule_ethernet.cpp` or a new `unit_NetworkModule_static.cpp`): +- The `addressing` Select is index 0=DHCP / 1=Static, defaults to DHCP; the four static IPv4 controls exist with the documented defaults (subnet `255.255.255.0`), and are hidden when `addressing_ != 1` (pins the control contract + the visibility rule). +- The desktop `netSetStaticIPv4` stub is a safe no-op (accepts any octets, doesn't bring an interface up) β€” mirrors the existing "desktop net seam is inert" tests. +The actual dhcpc_stop/set_ip_info is ESP32-only (bench-verified, not host-mockable) β€” pin the control/seam contract host-side, verify the apply on hardware. + +## Verification + +1. **Host:** `cmake --build build` (0 warnings) + `ctest` + scenarios green. +2. **S31 build:** `uv run moondeck/build/build_esp32.py --firmware esp32s31 --skip-idf-pin-check` (0 warnings). +3. **Bench (WiFi static):** on a WiFi board, set addressing=Static + a valid static IP/gw/mask/dns on the LAN, confirm the device comes up at that IP (reachable, UI loads), and that toggling back to DHCP re-leases live (no reboot). PO-observed. +4. **Bench (Ethernet static) + the 100M experiment:** on the S31 at 100M, set a static IP in the LAN's range and confirm Ethernet becomes **reachable** (ping + UI at the static IP) β€” the decisive test: if it works, unicast/RX is fine and DHCP-handshake was the only 100M gap. PO-observed; this is the measurement, serial is supporting evidence. +5. **No-reboot toggle both ways** and **persistence across reboot** (the octets already persist; confirm they re-apply on boot in Static mode). + +## Risks / notes + +- **Static "connected" without a DHCP event** is the main behavioral change: the cascade's connected-detection keys off DHCP/association IP events today; Static must mark connected right after the apply. Keep this contained in NetworkModule's state machine (don't fake a platform event). +- **DNS optional:** if `dns` is all-zero, skip `set_dns_info` (leave whatever's there) rather than setting 0.0.0.0. +- **Gateway/subnet sanity:** a static IP outside the gateway's subnet silently won't route; out of scope to validate, but the status still shows the applied IP so the user can tell it took. +- **Platform boundary:** all IDF calls stay in `platform_esp32.cpp`; NetworkModule only calls `platform::netSetStaticIPv4` + the existing init functions (no `#ifdef` leaks). diff --git a/src/core/Control.h b/src/core/Control.h index bc0da580..b01a70aa 100644 --- a/src/core/Control.h +++ b/src/core/Control.h @@ -252,6 +252,14 @@ struct ControlDescriptor { // so tooling and the API reach it regardless. Set via setAdvanced(). (The // client composes the two: expertMode is one global toggle in SystemModule, // read UI-side, so no module needs to reach into System's state.) + bool numberField = false; // Render a numeric control (Uint8/Uint16/Int16) as a plain NUMBER INPUT, + // never a drag-slider β€” for a value where each integer is a discrete address + // (a PHY MDIO address, an I2C address, a channel number), not a magnitude you + // sweep. A pure UI rendering hint like hidden/advanced; the value, range, and + // persistence are unchanged. Set via setNumberField(). (The Pin type already + // renders number-only for the same reason β€” a GPIO is an identity, not a + // magnitude; this extends that to non-Pin numerics without the Pin type's + // pin-ownership-map claim.) // Optional per-control input validator (Text/Password only; nullptr = accept anything // that fits the buffer). applyControlValue calls it on the incoming string BEFORE the // write and returns ApplyResult::Malformed on reject, so the check covers EVERY write @@ -353,7 +361,7 @@ class ControlList { void addText(const char* name, char* var, uint16_t bufSize = 16, bool (*validate)(const char*) = nullptr) { grow(); - controls_[count_++] = {var, name, 0, ControlType::Text, 0, bufSize, false, false, false, validate}; + controls_[count_++] = {var, name, 0, ControlType::Text, 0, bufSize, false, false, false, false, validate}; } // Like addText but the UI renders a resizable multi-line