Static IP (WiFi+Ethernet), S31 RGMII eth link, /api/types LED-freeze fix - #54
Open
ewowi wants to merge 4 commits into
Open
Static IP (WiFi+Ethernet), S31 RGMII eth link, /api/types LED-freeze fix#54ewowi wants to merge 4 commits into
ewowi wants to merge 4 commits into
Conversation
… freeze
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 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/NetworkModule.h (1)
439-450: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winWiFi STA static addressing is never applied before/at first association — DHCP-less WiFi static will never connect.
WaitingStahas no analogue ofWaitingEth's static-apply poll (compare lines 400-401).applyStaticIfConfigured(NetIface::Sta)is only ever called fromonConnected()(line 921) andsyncAddressingLive()(line 867), both gated onstate_ == ConnectedSta— which itself requiresplatform::wifiStaConnected()to already be true. But for a Static-mode STA,wifiStaConnected()only becomes true insidenetSetStaticIPv4oncestaStatic_is already set — andstaStatic_is only ever set by that same call. This is circular: nothing callsapplyStaticIfConfigured(Sta)(or otherwise primesstaStatic_) right after any of the fivewifiStaInit()call sites succeed (setup, setWifiCredentials, the two eth-drop cascades, and the AP retry).Result: on a DHCP-less WiFi network with
addressing == Static,WIFI_EVENT_STA_CONNECTEDfires whilestaStatic_is still false, so the static IP is never pinned;wifiStaConnected()never flips;WaitingStatimes out afterkStaGraceMsand falls back to AP forever — exactly the "DHCP-less networks" capability this PR claims to add. On a network that does have DHCP, the device instead takes a leased address first and only self-corrects to the static IP once GOT_IP incidentally satisfies theConnectedStagate.This matches
docs/history/plans/Plan-20260726 - Static IP for WiFi and Ethernet (shipped).md's Stage 2, which specifies calling the static apply "right afterwifiStaInit()returns true" — that call appears to have been dropped from the shipped implementation. The class docstring (lines 67-68: "Applied at each interface's bring-up (applyStaticIfConfigured)") also describes the STA behavior that isn't actually wired.🐛 Proposed fix — mirror the Ethernet poll, or prime the apply right after wifiStaInit()
case State::WaitingSta: if constexpr (platform::hasWiFi) { + // Static mode: pin the IP now so a DHCP-less network never needs a lease + // (mirrors the Ethernet poll in WaitingEth). Safe to call every tick — the + // platform setter is idempotent and no-ops once already applied. + if (addressing_ == kAddressingStatic) + applyStaticIfConfigured(platform::NetIface::Sta); if (platform::wifiStaConnected()) { onConnected("WiFi STA"); } else if (elapsed > kStaGraceMs) {Alternatively (matching the plan doc exactly), call
applyStaticIfConfigured(platform::NetIface::Sta)immediately after each of the fivewifiStaInit()call sites returnstrue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/NetworkModule.h` around lines 439 - 450, Ensure WiFi STA static addressing is applied during STA bring-up, before connection-state checks can time out. Update the five successful wifiStaInit() call sites to invoke applyStaticIfConfigured(NetIface::Sta) immediately after initialization, or add an equivalent static-apply poll in WaitingSta mirroring WaitingEth; preserve normal DHCP behavior and the existing AP fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/Control.h`:
- Around line 255-262: Update the comment describing numberField and its related
setter documentation to use present-tense wording throughout: replace imperative
phrases such as “Render” and “Set” with statements such as “Renders” and “The
setter sets.” Preserve the existing meaning and scope of the comments.
In `@src/core/HttpServerModule.cpp`:
- Line 1081: Update the conditional that appends the numberField metadata in
HttpServerModule to use a braced block, keeping the existing append operation
inside the condition.
In `@src/light/drivers/ParallelLedDriver.h`:
- Line 1155: Update the quiesce notification in the swap logic near the
peripheral selection code to require explicit live ownership by the active
Drivers instance, rather than checking only peripheral_. Preserve notification
for attached drivers, suppress it for detached probes after default selection,
and add regression coverage for a detached probe’s second swap.
In `@src/platform/esp32/platform_esp32.cpp`:
- Around line 1279-1383: Protect the shared static-addressing state updated in
netSetStaticIPv4() and netSetDhcp()—including ethStatic_/staStatic_, connected
flags, and static IP/GW/mask/DNS arrays—with the existing cross-task
synchronization used for wifiStaStopping_ and apkClients_, covering both IDF
event handlers and NetworkModule::tick1s() reconfiguration paths. Ensure reads
in link-up handlers use the same protection so live Static↔DHCP toggles cannot
race with event processing.
In `@src/ui/app.js`:
- Around line 1374-1389: Update the input handler in the number-field control
block to assign the clamped value back to input.value before scheduling
sendControl, so displayed text always matches the value sent, including invalid
and out-of-range input.
- Around line 1392-1393: Update the appendResetButton flow so clicking reset
cancels any pending debounce timer, records the reset interaction in dragTs, and
then applies the reset value. Preserve the existing reset callback behavior
while ensuring delayed edits and stale WebSocket patches cannot overwrite the
reset.
In `@test/unit/core/unit_NetworkModule_ethernet.cpp`:
- Around line 88-111: The host tests do not cover the WaitingSta
static-addressing path because desktop wifiStaInit() always fails. Update the
NetworkModule state flow around WaitingSta so that, after a successful
platform::wifiStaInit() in Static mode, applyStaticIfConfigured(Sta) invokes
netSetStaticIPv4; add a lightweight seam test that verifies this invocation.
In `@test/unit/light/unit_Drivers_rendersplit.cpp`:
- Around line 208-230: Make the render-hook setup in the test case
“render-split: a detached ParallelLedDriver's peripheral swap does not disturb
the live split” exception-safe by declaring an RAII guard or fixture cleanup
before calling setQuiesceRenderHook. Ensure the guard always resets the global
hook, including assertion failures and exceptions during setup, while preserving
the existing normal release behavior.
---
Outside diff comments:
In `@src/core/NetworkModule.h`:
- Around line 439-450: Ensure WiFi STA static addressing is applied during STA
bring-up, before connection-state checks can time out. Update the five
successful wifiStaInit() call sites to invoke
applyStaticIfConfigured(NetIface::Sta) immediately after initialization, or add
an equivalent static-apply poll in WaitingSta mirroring WaitingEth; preserve
normal DHCP behavior and the existing AP fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2fed9df9-b551-4ac7-8723-001f2231a926
📒 Files selected for processing (16)
docs/backlog/backlog-core.mddocs/history/lessons.mddocs/history/plans/Plan-20260726 - S31 RGMII eth DHCP at 100M.mddocs/history/plans/Plan-20260726 - Static IP for WiFi and Ethernet (shipped).mdsrc/core/Control.hsrc/core/HttpServerModule.cppsrc/core/NetworkModule.hsrc/light/drivers/ParallelLedDriver.hsrc/platform/desktop/platform_desktop.cppsrc/platform/esp32/platform_esp32.cppsrc/platform/platform.hsrc/ui/app.jstest/scenarios/light/scenario_Layouts_mutation.jsontest/scenarios/light/scenario_peripheral_grid_sweep.jsontest/unit/core/unit_NetworkModule_ethernet.cpptest/unit/light/unit_Drivers_rendersplit.cpp
| // Emit optional flags only when set (common case is false; omit to save bytes). | ||
| if (c.readonly) sink.append(",\"readonly\":true"); | ||
| if (c.advanced) sink.append(",\"advanced\":true"); // UI shows it only in expert mode | ||
| if (c.numberField) sink.append(",\"numberField\":true"); // render a plain number input, not a slider |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add braces around the new conditional.
Clang-Tidy reports readability-braces-around-statements at Line 1081. Braces prevent a future metadata statement from accidentally escaping the condition.
Proposed fix
- if (c.numberField) sink.append(",\"numberField\":true"); // render a plain number input, not a slider
+ if (c.numberField) {
+ sink.append(",\"numberField\":true"); // render a plain number input, not a slider
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (c.numberField) sink.append(",\"numberField\":true"); // render a plain number input, not a slider | |
| if (c.numberField) { | |
| sink.append(",\"numberField\":true"); // render a plain number input, not a slider | |
| } |
🧰 Tools
🪛 Clang (14.0.6)
[note] 1081-1081: +2, including nesting penalty of 1, nesting level increased to 2
(clang)
[warning] 1081-1081: statement should be inside braces
(readability-braces-around-statements)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/HttpServerModule.cpp` at line 1081, Update the conditional that
appends the numberField metadata in HttpServerModule to use a braced block,
keeping the existing append operation inside the condition.
Source: Linters/SAST tools
| // in-flight DMA the worker must be quiesced away from before the free) and skips the harmless | ||
| // no-backend first build. (Not gated on parent(): a swap that frees a real backend must quiesce | ||
| // regardless of tree attachment — the guard is "is there a backend to protect", not "am I live".) | ||
| if (peripheral_) MoonModule::notifyQuiesceRender(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Gate quiescing on live ownership, not just peripheral_.
After a probe’s first default selection, peripheral_ is non-null. Any later swap on that detached probe therefore fires the global hook, which resolves to the live Drivers instance and can stop its render worker. The regression only covers the null-to-default swap. Gate this on explicit attachment/ownership by the active Drivers instance, and add a second-swap regression case. This is also the rule documented in docs/history/lessons.md Line 376.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/light/drivers/ParallelLedDriver.h` at line 1155, Update the quiesce
notification in the swap logic near the peripheral selection code to require
explicit live ownership by the active Drivers instance, rather than checking
only peripheral_. Preserve notification for attached drivers, suppress it for
detached probes after default selection, and add regression coverage for a
detached probe’s second swap.
The v4 "lean and mean" pass: CLAUDE.md shrinks from ~8,800 words to 124 lines with its detail moved into the docs that own it, and the gate lists it used to describe become three runnable scripts (pre-commit, pre-merge, pre-release) that report PASS/FAIL/SKIP/MANUAL in about ten seconds. Also raises the ESP32-S31 to its rated 320 MHz, converts the last British spellings repo-wide, and processes the CodeRabbit and Reviewer findings. KPI: 16384lights | Desktop:755KB | tick:140/116/3/6/146/336/19/3/332/71/20/26/188/138/21/5/43/5us(FPS:7142/8620/333333/166666/6849/2976/52631/333333/3012/14084/50000/38461/5319/7246/47619/200000/23255/200000) | ESP32:1499KB | tick:2705us(FPS:369) | heap:16582KB | src:195(46846) | test:139(25939) | lizard:162w Core: - NetworkModule: apply a configured static IP during WiFi STA bring-up (WaitingSta), mirroring the WaitingEth poll — a DHCP-less network never fires a lease event, so waiting for one stranded a static STA into the AP fallback - platform_esp32: publish the static-addressing state through std::atomic with an octets-before-flag release store, so an IDF event-task link-up re-pin cannot read a half-written config - platform: add the desktop test seams setTestWifiStaAvailable + testNetStaticApplyCount, so the STA cascade is drivable host-side - Control.h: present-tense wording on the numberField comment Light domain: - ESP32-S31: raise the CPU to its rated 320 MHz (esp32/sdkconfig.defaults.esp32s31); the shared defaults set 240, the rated max for the chips before it, so this target has to override or it silently runs a third slower. Bench-confirmed: fps 344 to 369 UI: - app.js: a number field clamps and writes back the value it sends, so the display can never disagree with the device; an empty field mid-edit sends nothing rather than silently sending the minimum - app.js: reset (the reset button) cancels any pending debounced edit and stamps the interaction, so a delayed keystroke or a stale WebSocket patch cannot overwrite the reset Scripts / MoonDeck: - event/: precommit, premerge and prerelease over a shared gate runner. The mechanical checks are defined once in _gates.py and parameterised by the two things that actually differ per event (compile the firmware or check its freshness; triggered or not), rather than re-declared per script. Each gate carries an objective trigger read from the changed-file set, so a docs-only change runs the spec check and skips the rest - check_hotpath: lint the render path for allocation and blocking (a lint, not a proof — it reads only the method's own source, and a justified exception is marked at the line) - check_esp32_built: report whether the firmware binary is newer than every source that feeds it, the cheap stand-in for a full build in the commit and merge gates. Freshness is measured against sources, not the clock, so an edit made twenty minutes after a build still fails it; generated headers and the desktop-only platform are excluded, since neither can stale an ESP32 image - collect_kpi: add --no-live-capture, which the gates pass to skip the 15 s serial read (about 4 s instead of 80, and no dependency on a bench board) - moondeck.py: heal a last_port breadcrumb that the present ports disprove, keyed on the drift-immune adapter serial — a stale path made the Live tab name a port the ESP32 tab knew was wrong - moondeck_ui: one Scan button replacing Discover + Refresh (the subnet scan already re-probes known devices and marks the unfound offline, so both were pressed every time); an "online only" filter with a hidden count; a link opening a device UI in a new browser tab, alongside the existing view pane; and a one-turn spin on the port-refresh button, which otherwise looked like it did nothing Tests: - unit_Effects_gridsweep: drive every effect through 0x0x0, 1x1x1, one strand and one row. The effect list is generated from src/light/effects/ at build time, so an effect added later is swept without touching the test; the generator refuses to emit an empty list, and the test fails on one, so it cannot pass vacuously - unit_NetworkModule_ethernet: pin that Static mode reaches the platform during STA bring-up - unit_Drivers_rendersplit: an RAII guard for the quiesce hook, so a failing assertion cannot leak it into later cases Docs / CI: - CLAUDE.md: rebuilt around five ordered principles and the change lifecycle (main, branch, build, test, document, commit, merge, release); the technical detail moved into architecture.md and coding-standards.md rather than being restated here - docs/principles-and-process.md: put CLAUDE.md on the docs site under Developer reference, embedded rather than copied, with its repo-root doc links rebased at build time (mkdocs_hooks) - Spelling: convert the remaining British spellings repo-wide (595 occurrences across 154 files) per the American-English rule; no wire key or external API name changed - coding-standards: the hot-path lint is a script now, not a hand convention; the "when checks run" list stops restating the gate lists and refers to the one definition - history/README: describe the plans/ archive, which the new temporary-plan rule left undocumented - backlog-core: the S31 gigabit test ran and Ethernet still does not lease, which contradicts the link-speed theory this item was opened on; the ARP evidence and the unresolved link-speed question are recorded - backlog-core: the AudioService sync suite is flaky (about one run in ten) on a fixed UDP port, with the failed per-run-port attempt recorded so it is not retried blind - backlog-core: ModuleFactory registration can fail silently at its uint8_t ceiling (it appends unconditionally, and no caller checks the returned bool) Reviews: - 🐇 CodeRabbit: number-field clamp not written back to the input — fixed - 🐇 CodeRabbit: reset button racing a pending debounce — fixed in appendResetButton, so every control type benefits - 🐇 CodeRabbit: unsynchronised cross-task static-addressing state — fixed with atomics and a documented publish order - 🐇 CodeRabbit: WaitingSta never applied a static IP — fixed with one poll in the state machine rather than editing five call sites - 🐇 CodeRabbit: render-hook test not exception-safe — fixed with an RAII guard - 🐇 CodeRabbit: present-tense comment wording in Control.h — fixed - 🐇 CodeRabbit: brace the single-statement numberField append in HttpServerModule — skipped; the adjacent readonly/advanced lines share the shape, and bracing one alone breaks local consistency - 🐇 CodeRabbit: require live ownership before the quiesce notify in ParallelLedDriver — skipped; a detached probe swaps once from a null backend and never notifies, the second-swap case is unreachable in production, and the gating is deliberate and documented at the site - 🐇 CodeRabbit: apply the static IP at the five wifiStaInit call sites — skipped as superseded by the WaitingSta poll above, which CodeRabbit itself offered as the alternative - 👾 Reviewer: coding-standards said the hot-path lint was enforced by hand while this diff ships the script — fixed - 👾 Reviewer: "when checks run" was a third, stale copy of the gate list (a pre-push event that no longer exists, future-tense CI) — fixed by referring to the one definition - 👾 Reviewer: the three event scripts re-declared the same mechanical gates — fixed by lifting them into _gates.py - 👾 Reviewer: MANUAL gates bypassed the trigger mechanism, so both scripts filtered them by display name — fixed in the runner; the name filters are gone - 👾 Reviewer: check_esp32_built scanned src/platform/desktop while the gate trigger excluded it — fixed - 👾 Reviewer: the effect-sweep comment misdescribed ModuleFactory (create returns nullptr, not garbage) — corrected to the real mechanism, and the underlying silent-failure issue is backlogged - 👾 Reviewer: the freshness and live-capture rationale was told in full in five places — reduced to one home plus pointers - 👾 Reviewer: the moonlive exclusion in the hot-path lint carried no reason — removed; it has no hot methods, so the exclusion was a no-op - 👾 Reviewer: single-call-site wrapper in collect_kpi — inlined - 👾 Reviewer: the KPI guardrail had been softened to "when measured" — restored to requiring a measured value per target - 👾 Reviewer: the 89-file plans archive was undocumented under the new plan rule — described in history/README - 👾 Reviewer: /api/ports mutates state on a GET — accepted; the self-heal has to run where the UI calls, it is idempotent, and this is a localhost dev dashboard Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measures the repo's size, complexity and docs footprint on every commit so growth is visible while it happens, and starts a proof-of-concept for the static-analysis tooling that would enforce our architectural principles. Nothing here gates a build yet: the ratchet reports, and the CodeQL workflow is manual until the POC judges it. KPI: 16384lights | Desktop:755KB | tick:180/131/3/7/164/372/23/5/349/76/22/27/195/151/22/6/45/5us(FPS:5555/7633/333333/142857/6097/2688/43478/200000/2865/13157/45454/37037/5128/6622/45454/166666/22222/200000) | ESP32:1499KB | src:195(46846) | test:139(25939) | lizard:162w The ESP32 tick/FPS figure is absent by design: this change touches no `src/`, so the firmware is byte-identical to the previous commit and its measured tick is unchanged. Flash size still reports. Scripts / MoonDeck: - repo_health: measure the current state into docs/metrics/ — flash per firmware variant, tick/FPS per target, lines of code and comment density by area, test counts, docs inventory. One small committed file whose git history IS the trend, so it never grows; a readable Markdown table is generated beside it with units, percentages and a per-metric delta against the last commit - repo_health: carry forward anything a run could not measure (a variant that was not built, a tick with no board attached) rather than blanking it, so the file's contents do not depend on which targets happened to be built - collect_kpi: write the snapshot at the end of a --commit run, reusing the tick/FPS it just measured - precommit: widen the KPI trigger beyond `src/` — a moondeck/ or docs-only commit still moves lines of code and the docs inventory, and a snapshot that skipped those would drift from the tree it claims to describe Docs / CI: - Plan-20260727: the static-analysis plan, shaped as a POC rather than a commitment — run every candidate against this codebase, then make one judgement about the final set. Records what each POC must produce (coverage, configurability, extensibility, cost), a scorecard to fill in, and the rule that overlaps are decided rather than accumulated - Plan-20260727: POC results so far for clang-tidy and clang-query, with the numbers rather than impressions - codeql.yml: a CodeQL workflow for the security question this project has never asked — six network packet parsers doing ~22 memcpy on LAN data, on a device with no MMU, and no security analysis today. build-mode: none, so no ESP-IDF cross-build needs reproducing in CI. Manual + weekly, deliberately not a gate: if it finds nothing actionable, that is a result and the file goes - docs/metrics/: a home for generated measurements, separate from prose. Verified while choosing it that performance.md is hand-written and the scenario JSONs are authored fixtures a script annotates in place — repo-health is the only fully generated file, so it gets a folder rather than the repo root Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The workflow could never have run as written: GitHub only shows the "Run workflow" button for workflows present on the DEFAULT branch, and this one lives on next-iteration until the POC is judged, so `workflow_dispatch` had nothing to attach to. Adds a push trigger scoped to the POC branch and to C/C++ (or the workflow itself) changing, which is the only way to get a first result without merging an unevaluated workflow into main. Still not a gate: it does not run on pull_request, so it cannot block a merge. Docs / CI: - codeql.yml: add a push trigger on next-iteration, scoped by path so a docs-only commit does not burn a runner Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // MDIO address is an identity, not a magnitude. Tests the addInt16 + setNumberField seam | ||
| // directly (the NetworkModule control is `if constexpr (hasEthernet)`-gated, absent on desktop), | ||
| // so a future edit that reverts to a slider or an unsigned type fails here, off-hardware. | ||
| TEST_CASE("ethPhyAddr-style control: signed int16, -1 sentinel in range, number field") { |
| // defaults to DHCP, the static fields exist with their documented defaults, and they are HIDDEN in | ||
| // DHCP mode (visible only when addressing==Static). These are always bound (not hasEthernet-gated), | ||
| // so the contract is testable on the desktop host. | ||
| TEST_CASE("addressing Select + static-IP controls: DHCP default, Static reveals the fields") { |
| // The desktop platform's static/DHCP setters are inert no-ops: addressing is OS-managed on the | ||
| // host, so netSetStaticIPv4 / netSetDhcp must accept any input and change nothing (no crash, no | ||
| // interface brought up). Mirrors the "desktop net seam is a safe no-op" guarantee for ethInit etc. | ||
| TEST_CASE("Desktop static-addressing seam is a safe no-op") { |
| // IP would strand a static STA into the AP fallback (the WaitingEth static poll's mirror). The test | ||
| // seam fakes an STA radio so the host can drive the cascade into WaitingSta; the platform apply | ||
| // counter pins that tick1s invoked netSetStaticIPv4(Sta). | ||
| TEST_CASE("Static mode pins the static IP during STA bring-up (WaitingSta)") { |
| #include <string> | ||
|
|
||
| TEST_CASE("HueDriver: a coloured pixel becomes an on/bri/hue/sat state body") { | ||
| TEST_CASE("HueDriver: a colored pixel becomes an on/bri/hue/sat state body") { |
| } | ||
|
|
||
| TEST_CASE("HueDriver: parseLights keeps only colour-capable, reachable lights") { | ||
| TEST_CASE("HueDriver: parseLights keeps only color-capable, reachable lights") { |
| #include "light/Palette.h" | ||
|
|
||
| TEST_CASE("Palette: gradient endpoints land on the first/last stop colours") { | ||
| TEST_CASE("Palette: gradient endpoints land on the first/last stop colors") { |
| // (hue, sat) is computed from its expanded entries; nearestForHue picks the closest. A vivid hue | ||
| // snaps to that hue's palette family; a low-saturation target snaps to the desaturated Rainbow. | ||
| TEST_CASE("Palettes::nearestForHue maps a colour to the closest palette") { | ||
| TEST_CASE("Palettes::nearestForHue maps a color to the closest palette") { |
| // table — no forEachCoord. Painting a known color at a kept column and finding it at the matching | ||
| // frame position pins the index math + the lattice order. | ||
| TEST_CASE("PreviewDriver dense downsample packs colours by closed-form index, in lattice order") { | ||
| TEST_CASE("PreviewDriver dense downsample packs colors by closed-form index, in lattice order") { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this lands
Static-IP addressing (DHCP / Static) that actually works — on both WiFi STA and Ethernet. The
addressingdropdown +ip/gateway/subnet/dnscontrols were previously inert on client interfaces (only the SoftAP ever applied a static IP). Now static is applied to the active interface on bring-up and live on a DHCP↔Static toggle (no reboot), and it works on a DHCP-less network (applied at L2 association for WiFi, at link-up for Ethernet — no wait for a DHCPGOT_IP). Bench-verified end-to-end on the Olimex classic, both toggle directions./api/typesprobe no longer freezes the LEDs on a UI refresh. A throwaway probe that/api/typesbuilds to read a type's default controls was tearing down the live render split through a global active-instance hook (the reported "refresh the UI and the LEDs freeze" bug).swapPeripheralnow fires the render-quiesce hook only when freeing a real backend.S31 RGMII Ethernet link brought up (
ethYt8531BoardInit: YT8531 auto-negotiation re-enable + RGMII Tx/Rx delays) andethPhyAddrfixed to a signed int16 so the-1auto-detect sentinel round-trips (auint8had mangled it to 31, so the PHY was never found).numberFieldcontrol type — renders a Uint8/Uint16/Int16 as a plain number input (for identity values like a PHY/MDIO address), not a slider.Highlights
netSetStaticIPv4/netSetDhcp(oneNetIfaceenum serves STA + Eth, mirroring the SoftAP static block); desktop stubs are no-ops.ethPhyAddrint16/-1/numberField contract, the addressing/static control contract + desktop no-op seam, and the detached-probe render-split guard.Verification
All commit gates green:
check_specs(87/87), desktop build (0 warnings),ctest(100%), scenarios, platform-boundary, and all four ESP32 variants (classic / S3 / P4 / S31) build clean. A Reviewer (Fable) pass over the diff surfaced 6 findings — 2 real bugs (DHCP-less WiFi static; a Static→DHCP toggle wedge) and 4 nits — all fixed; see the commit's Reviews section.tick:150us(FPS:6700)desktop ·tick:2454us(FPS:407)esp32(classic)Known / deferred (backlogged, not worked around)
backlog-core.mdas a bug to fix, never a reason to weaken the cascade. The 1 Gb-switch test is the next step.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests