From 8c599ff22b85a51b9363a3d383b49520092ef386 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 10:38:09 +0200 Subject: [PATCH 1/7] docs(007): spec, plan, tasks for WiFi provisioning (PR-07) Spec-kit artifacts for PR-07 (WiFi station management + first-boot SoftAP provisioning), approved at Checkpoint 2. Records the pre-made FR9 decision (custom SoftAP portal), parity reconnect timing, and the host-testable IWifiDriver seam. No implementation yet. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../checklists/requirements.md | 42 +++ .../contracts/IWifiDriver.md | 44 +++ .../contracts/provisioning-portal.md | 42 +++ .../contracts/wifi-manager-states.md | 49 +++ specs/007-wifi-provisioning/data-model.md | 104 +++++++ specs/007-wifi-provisioning/plan.md | 151 +++++++++ specs/007-wifi-provisioning/quickstart.md | 78 +++++ specs/007-wifi-provisioning/research.md | 127 ++++++++ specs/007-wifi-provisioning/spec.md | 248 +++++++++++++++ specs/007-wifi-provisioning/tasks.md | 288 ++++++++++++++++++ 10 files changed, 1173 insertions(+) create mode 100644 specs/007-wifi-provisioning/checklists/requirements.md create mode 100644 specs/007-wifi-provisioning/contracts/IWifiDriver.md create mode 100644 specs/007-wifi-provisioning/contracts/provisioning-portal.md create mode 100644 specs/007-wifi-provisioning/contracts/wifi-manager-states.md create mode 100644 specs/007-wifi-provisioning/data-model.md create mode 100644 specs/007-wifi-provisioning/plan.md create mode 100644 specs/007-wifi-provisioning/quickstart.md create mode 100644 specs/007-wifi-provisioning/research.md create mode 100644 specs/007-wifi-provisioning/spec.md create mode 100644 specs/007-wifi-provisioning/tasks.md diff --git a/specs/007-wifi-provisioning/checklists/requirements.md b/specs/007-wifi-provisioning/checklists/requirements.md new file mode 100644 index 0000000..2ae1c9a --- /dev/null +++ b/specs/007-wifi-provisioning/checklists/requirements.md @@ -0,0 +1,42 @@ +# Specification Quality Checklist: WiFi Provisioning & Station Management + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-07-04 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Items marked incomplete require spec updates before `/speckit-clarify` or `/speckit-plan`. +- The FR9 provisioning-method decision, parity reconnection constants (10 s / 60 s / 5 attempts / + 5 s monitor), credential bounds (SSID 1–32, password empty or ≥ 8), and the empty-SSID unconfigured + representation are treated as pre-decided inputs from `docs/prd/PR-07-wifi-provisioning.md` and + `docs/parity-checklist.md` §7 — recorded as requirements/assumptions rather than open questions, so no + [NEEDS CLARIFICATION] markers were needed. +- Concrete parity constants (192.168.4.1, WPA2, timing values) appear in requirements because they are + behavioral parity facts, not implementation choices; the FR9 note in Success Criteria keeps the + method-level decision documented without leaking code structure. diff --git a/specs/007-wifi-provisioning/contracts/IWifiDriver.md b/specs/007-wifi-provisioning/contracts/IWifiDriver.md new file mode 100644 index 0000000..758ba72 --- /dev/null +++ b/specs/007-wifi-provisioning/contracts/IWifiDriver.md @@ -0,0 +1,44 @@ +# Contract: `IWifiDriver` (driver seam) + +Location: `firmware/components/interfaces/include/interfaces/IWifiDriver.h` (NEW). Pure C++ header — **no +IDF includes** (mirrors `IModbusClient`, `II2cBus`). Implemented on target by `EspWifiDriver`; a +`MockWifiDriver` under `network/include/network/testing/` backs host tests. + +## Types + +```cpp +enum class WifiEvent { None, Connected, GotIp, Disconnected, ConnectFailed }; +``` + +`Connected` = association succeeded (L2); `GotIp` = DHCP lease acquired (the "usable" signal the manager +treats as fully connected); `Disconnected`/`ConnectFailed` = drop / association failure. + +## Methods (proposed signatures — final naming at implementation) + +| Method | Contract | +|---|---| +| `bool staConnect(const std::string& ssid, const std::string& password)` | Configure STA + begin association. Returns false only on a synchronous config error; success/failure of the *attempt* arrives later as an event. Non-blocking. | +| `void staStop()` | Stop STA / disconnect. Idempotent. | +| `bool apStart(const std::string& ssid, const std::string& password)` | Start SoftAP (WPA2) at 192.168.4.1. Non-blocking; returns false on synchronous config error. | +| `void apStop()` | Stop SoftAP. Idempotent. | +| `WifiEvent pollEvent()` | Drain the next queued event, or `None` if the queue is empty. Called once per manager tick; thread-safe (driver enqueues from the esp_event callback). | +| `int8_t rssi() const` | Last known RSSI in dBm; unspecified when not connected. | + +## Behavioral contract + +- **Non-blocking**: no method blocks on network I/O; results are delivered via `pollEvent()`. This is what + lets the pure `WifiManager` stay deterministic and the wifi task never stall. +- **Event ordering**: a successful connect delivers `Connected` then `GotIp`. A failure delivers + `ConnectFailed` or `Disconnected`. The manager treats `GotIp` as "connected", `Disconnected`/ + `ConnectFailed` as "attempt failed". +- **No timing**: the driver owns no timers; all cadence lives in `WifiManager` via `ITimeProvider`. +- **Isolation**: the driver touches only WiFi/netif/event-loop resources — never pump/sensor state. + +## Mock (`MockWifiDriver`) requirements + +- Scriptable event queue: `queueEvent(WifiEvent)` (or a helper like `scriptConnectSuccess()` / + `scriptConnectFailure()`), consumed by `pollEvent()`. +- Records calls: counts of `staConnect`/`staStop`/`apStart`/`apStop`, last ssid/password passed (for + assertions; the mock may store them since it is host-only test code). +- Settable `rssi()` return. +- No real networking. Deterministic under `FakeTimeProvider`. diff --git a/specs/007-wifi-provisioning/contracts/provisioning-portal.md b/specs/007-wifi-provisioning/contracts/provisioning-portal.md new file mode 100644 index 0000000..4f35913 --- /dev/null +++ b/specs/007-wifi-provisioning/contracts/provisioning-portal.md @@ -0,0 +1,42 @@ +# Contract: Provisioning portal (standalone HTTP) + +Target-only class `ProvisioningPortal` (`firmware/components/network/.../ProvisioningPortal.{h,cpp}`), +excluded from the linux build. Runs a **minimal standalone `esp_http_server`** while the device is in +`Provisioning` (AP) mode. The full JSON status API and `/api/wifi/scan` are **out of scope** (PR-09). + +## Endpoints + +| Method | Path | Behavior | +|---|---|---| +| `GET` | `/` (and unknown paths) | Serve the WiFi setup page (HTML form: SSID text field, password field, submit). Small, self-contained, English. | +| `POST` | `/wifi/config` (path finalized at impl; parity used `/wifi/config`) | Accept form params `ssid`, `password`. See flow below. | + +## POST /wifi/config flow + +1. Parse `ssid` and `password` from the form body. +2. Call pure `validateWifiCredentials(ssid, password)`: + - **Reject** (SSID not 1–32, or non-empty password < 8, or > 64): respond 4xx with a short error, do + **not** persist, do **not** restart. Device stays in Provisioning (FR-005). +3. On accept: `IConfigStore::setWifiCredentials(ssid, password)`. + - If persist fails (`false`): respond 5xx, stay provisionable. +4. On persist success: respond 200 with a success page indicating `restartRequired`, then **schedule a + restart ~3 s later** (so the response is delivered before reboot) — FR-007. + +## Guards + +- Credential submission is honored **only in AP/provisioning mode** (FR-006). (In this design the portal + server only runs while provisioning, which structurally enforces it; if the server could ever be up in + STA mode, the handler must reject.) +- The portal never echoes the submitted password back and never logs credential values (FR-004 / PR-06 + convention). +- Setup page + assets must keep the app binary within the 1.5 MiB OTA slot (research R2). + +## Parity / overlap notes + +- Legacy served `wifi_setup.html` as the AP-mode default file at 192.168.4.1 with a ~3 s post-save + restart (`docs/parity-checklist.md` §7). This portal reproduces that reachable-provisioning-UI parity + requirement; exact page markup is an implementation detail. +- The diag console `config wifi ` / `wifi-clear` path writes the **same** `IConfigStore` + credentials — both paths must agree (HIL check). +- Captive-portal DNS redirect and mDNS/hostname niceties are **not** implemented unless free with the + chosen components (PR brief: do not gold-plate). diff --git a/specs/007-wifi-provisioning/contracts/wifi-manager-states.md b/specs/007-wifi-provisioning/contracts/wifi-manager-states.md new file mode 100644 index 0000000..825b543 --- /dev/null +++ b/specs/007-wifi-provisioning/contracts/wifi-manager-states.md @@ -0,0 +1,49 @@ +# Contract: `WifiManager` state machine & reconnect timing + +Pure class (`firmware/components/network/include/network/WifiManager.h`, compiled on host + target). +Dependencies (constructor-injected): `IWifiDriver&`, `IConfigStore&`, `ITimeProvider&`, `ReconnectPolicy` +(defaults = parity constants). **No watering/pump/sensor dependency** — this is the FR-014 guarantee and a +host-test assertion (the type does not compile with such a reference; tests document the dependency set). + +## Public surface (proposed) + +| Member | Contract | +|---|---| +| `void begin(WifiBootMode mode)` | Enter `Provisioning` or `Station` (from `decideBootMode`). In Station, issues the first `staConnect`. In Provisioning, `apStart` + (portal started by caller). | +| `void tick()` | Advance the state machine using `ITimeProvider::nowMs()` and drained `pollEvent()`s. Non-blocking; called at a fixed cadence from the wifi task. | +| `WifiConnectionSnapshot snapshot() const` | Single-acquisition consistent copy of state+counters+rssi for status/LED consumers. | + +## Timing contract (parity — the core host-test target) + +Using `FakeTimeProvider` (no wall clock), with `MockWifiDriver` scripting outcomes: + +1. **Connect success**: `begin(Station)` → `staConnect` called once → script `Connected`+`GotIp` → + `tick()` → state `Connected`, `consecutiveFailures == 0`. +2. **Single retry cadence**: from `Connecting`, script `ConnectFailed` → state `Reconnecting`; **no** new + `staConnect` until `advance(10_000)`; at exactly 10 s a new attempt is issued. Assert no attempt at + 9 999 ms, one attempt at 10 000 ms. +3. **Pause after 5 failures**: script 5 consecutive `ConnectFailed` (advancing 10 s each) → after the 5th, + state `ReconnectPaused`; assert **no** `staConnect` during the pause; the next attempt occurs only + after `advance(60_000)`, and `consecutiveFailures` resets to 0 for the new round. +4. **Monitor cadence**: in `Connected`, monitoring evaluates every 5 s; a scripted `Disconnected` → + `Reconnecting`, `disconnectCount` incremented. +5. **No boot loop (FR-013)**: script an infinite `ConnectFailed` stream over many rounds → the machine + only ever cycles `Reconnecting`↔`ReconnectPaused`; assert it never requests a restart and memory/state + stays bounded. +6. **AP monitoring suspended**: in `Provisioning`, `tick()` performs no STA monitoring or reconnect + attempts regardless of elapsed time. +7. **Isolation (FR-014)**: a driver whose `pollEvent()` always returns `None` and whose `staConnect` + "hangs" (never events) must not cause `tick()` to block — `tick()` returns promptly every call. + +## Boot-mode contract + +`decideBootMode(bool credentialsPresent, bool configButtonHeld) -> WifiBootMode`: + +| credentialsPresent | configButtonHeld | result | +|---|---|---| +| false | false | Provisioning | +| false | true | Provisioning | +| true | false | Station | +| true | true | Provisioning (emergency; caller clears credentials first) | + +All four rows are host-tested. diff --git a/specs/007-wifi-provisioning/data-model.md b/specs/007-wifi-provisioning/data-model.md new file mode 100644 index 0000000..4f96766 --- /dev/null +++ b/specs/007-wifi-provisioning/data-model.md @@ -0,0 +1,104 @@ +# Phase 1 Data Model: WiFi Provisioning & Station Management + +Entities are in-memory firmware state (no schema/DB). Credentials persist via `IConfigStore` (PR-06). + +## Entities + +### WifiCredentials (persisted, reused from PR-06) + +| Field | Type | Rules | +|---|---|---| +| ssid | string | 1–32 chars to be valid; **empty string = unconfigured (factory state)** | +| password | string | empty (open network) OR ≥ 8 chars; max 64 (`kWifiPasswordMaxLen`) | + +- Source of truth: `IConfigStore` (NVS namespace `wscfg`, keys `wifi_ssid`/`wifi_pass`). Values never + logged. Written atomically by `setWifiCredentials()` (both keys + one commit); cleared by + `clearWifiCredentials()`. Wiped by `factoryReset()` (whole default NVS partition). + +### WifiConnectionState (in-memory, exposed via snapshot) + +| Field | Type | Notes | +|---|---|---| +| state | enum `WifiState` | see state machine below | +| rssi | int8 (dBm) | valid only in `Connected`; weak-signal note < −80 dBm (parity) | +| consecutiveFailures | uint8 | 0–5; drives the pause transition | +| disconnectCount | uint32 | monotonic, for diagnostics/status | +| ipAcquired | bool | true after `GotIp` | + +- Exposed to consumers as an immutable **snapshot** (single mutex acquisition), per research D9. + +### ProvisioningApConfig (in-memory, from Kconfig + board) + +| Field | Source | Value | +|---|---|---| +| ssid | Kconfig / constant | fixed setup SSID (e.g. `WateringSystem-Setup`), confirmed at impl | +| ip | fixed | 192.168.4.1 (ESP32 softAP default) | +| authmode | fixed | WPA2 | +| password | `CONFIG_WS_PROV_AP_PASSWORD` | documented non-secret; brand-new; legacy never reused | + +### ReconnectPolicy (construction parameters, defaults = parity) + +| Param | Default | Meaning | +|---|---|---| +| retryIntervalMs | 10 000 | delay between STA connect attempts while reconnecting | +| failuresBeforePause | 5 | consecutive failures that trigger the long pause | +| pauseMs | 60 000 | extra wait after `failuresBeforePause` before next round | +| monitorIntervalMs | 5 000 | connection-health check cadence while connected (suspended in AP) | + +## WifiManager state machine + +States (`enum class WifiState`): `Provisioning`, `Connecting`, `Connected`, `Reconnecting`, +`ReconnectPaused`. + +```text + decideBootMode == Provisioning + [boot] ───────────────────────────────────────────────► Provisioning + │ │ (credentials submitted+persisted → restart) + │ decideBootMode == Station └───► [restart] → [boot] + ▼ + Connecting ──── GotIp ─────────────────────────────────► Connected + │ ▲ │ + │ │ retryIntervalMs elapsed │ monitor(5s): link lost / Disconnected event + │ │ ▼ + │ └───────────────────────── Reconnecting ◄────────────┘ + │ │ ▲ + │ ConnectFailed / Disconnected │ │ retryIntervalMs elapsed (attempt++) + └─────────────────────────────────┘ │ + │ consecutiveFailures == failuresBeforePause + ▼ + ReconnectPaused ── pauseMs elapsed ──► Reconnecting (failures reset) +``` + +Transition rules: + +- **boot → Provisioning**: `decideBootMode(credentialsPresent, buttonHeld)` returns Provisioning when + credentials absent OR config button held at boot. On button-forced provisioning, credentials are cleared + first. In Provisioning: AP up + portal serving; STA monitoring suspended. +- **Provisioning → restart**: a valid credential submission persists to NVS and schedules a restart + (~3 s) so the HTTP success response is delivered first. There is no timed abandonment of provisioning + (device stays provisionable indefinitely — edge case in spec). +- **boot → Connecting**: Station mode; issue STA connect with stored credentials. +- **Connecting → Connected**: on `GotIp`. Resets `consecutiveFailures` to 0. +- **Connecting/Reconnecting → Reconnecting**: on `ConnectFailed`/`Disconnected`, increment + `consecutiveFailures`; schedule next attempt after `retryIntervalMs`. +- **Reconnecting → ReconnectPaused**: when `consecutiveFailures` reaches `failuresBeforePause` (5). +- **ReconnectPaused → Reconnecting**: after `pauseMs` (60 s); reset `consecutiveFailures` to 0 and begin a + new round. Never transitions to a reboot (FR-013 — no boot loop). +- **Connected → Reconnecting**: monitor tick (every 5 s) detects link loss, or a `Disconnected` event + arrives; `disconnectCount++`. + +Invariants: + +- The manager only ever calls `IWifiDriver` and reads `ITimeProvider`/`IConfigStore`. It holds **no + reference to any watering/pump/sensor object** (FR-014, Constitution I) — enforced by its constructor + signature and asserted structurally in host tests. +- All timers are computed from `ITimeProvider::nowMs()` deltas; the manager never sleeps or blocks. +- A driver that never reports `GotIp` (wrong password) keeps the machine cycling + Reconnecting↔ReconnectPaused forever — bounded memory, no reboot. + +## Validation rules (pure, host-tested) + +- `validateWifiCredentials(ssid, password)`: reject if `ssid.empty()` or `ssid.size() > 32`; reject if + `!password.empty() && password.size() < 8`; reject if `password.size() > 64`; else accept. +- `decideBootMode(credentialsPresent, buttonHeld)`: `buttonHeld || !credentialsPresent ⇒ Provisioning`, + else `Station`. (Truth table is a host-test target — all four rows.) diff --git a/specs/007-wifi-provisioning/plan.md b/specs/007-wifi-provisioning/plan.md new file mode 100644 index 0000000..cf5989d --- /dev/null +++ b/specs/007-wifi-provisioning/plan.md @@ -0,0 +1,151 @@ +# Implementation Plan: WiFi Provisioning & Station Management + +**Branch**: `007-wifi-provisioning` | **Date**: 2026-07-04 | **Spec**: [spec.md](./spec.md) + +**Input**: Feature specification from `specs/007-wifi-provisioning/spec.md`; PR brief +`docs/prd/PR-07-wifi-provisioning.md`; parity contract `docs/parity-checklist.md` §7. + +## Summary + +Add WiFi station management with parity-baseline reconnection and first-boot SoftAP provisioning to +the ESP-IDF firmware, without ever letting network activity touch watering. The design keeps all timing +and state-machine logic in a **pure `WifiManager`** driven by `ITimeProvider` and an injected +**`IWifiDriver`** seam, so the reconnection schedule (10 s retry, +60 s pause after 5 fails, 5 s monitor) +and the boot-mode decision are host-tested against a mock driver + `FakeTimeProvider`. The IDF surface +(`esp_wifi`/`esp_netif`/`esp_event` and the standalone provisioning `esp_http_server`) lives in thin +hardware classes excluded from the linux build. WiFi runs on its own FreeRTOS task, structurally +isolated from the 10 Hz pump/level loop. Credentials reuse the existing `IConfigStore` WiFi API from +PR-06 (empty SSID = unconfigured); no new credential store is introduced. + +## Technical Context + +**Language/Version**: C++17 on native ESP-IDF v6.0.1 (Docker `espressif/idf:v6.0.1`); no Arduino layers. + +**Primary Dependencies**: `esp_wifi`, `esp_netif`, `esp_event`, `esp_http_server`, `nvs_flash` (already +initialized), FreeRTOS. Pure layer depends only on project interfaces (`IWifiDriver`, `IConfigStore`, +`ITimeProvider`) — no IDF headers. + +**Storage**: Reuses `IConfigStore` (PR-06) WiFi API — `getWifiSsid()/getWifiPassword()/ +setWifiCredentials()/clearWifiCredentials()`; NVS namespace `wscfg`, keys `wifi_ssid`/`wifi_pass`; +credential values never logged. Unconfigured = empty SSID string. + +**Testing**: Unity host suite on the IDF linux preview target (`test_apps/host/`); new `run_wifi_tests()`. +Hardware paths (`EspWifiDriver`, `ProvisioningPortal`) are excluded from the linux build and verified by +HIL on the rev1 devkit rig. + +**Target Platform**: ESP32-WROOM-32E, board targets `BOARD_REV1_DEVKIT` and `BOARD_REV2` (rev2 LED/button +pins are `TODO(SYNC1)` provisional). + +**Project Type**: Embedded firmware component + `main/` wiring; single-repo, component-based. + +**Performance Goals**: Reconnection cadence exactly per parity (10 s / +60 s after 5 / 5 s monitor). +Watering loop cadence (10 Hz poll, pump timing) suffers zero measurable deviation during WiFi events. + +**Constraints**: Provisioning portal binary must fit the 1.5 MiB OTA app slot; NVS partition is 16 KiB; +brownout detector stays enabled (parity QUIRK 4 watch-item); AP uses WPA2 with a Kconfig-sourced, +documented non-secret password (brand-new, legacy never reused). + +**Scale/Scope**: One WiFi manager, one provisioning portal with a single setup page and one POST handler, +one new FreeRTOS task, one new `network` component + one interface + one mock. Minimal standalone portal — +the full JSON/status API and `/api/wifi/scan` are PR-09. + +## Constitution Check + +*GATE: evaluated before Phase 0 and re-checked after Phase 1 design. Result: PASS (no violations).* + +- **I. Safety First (NON-NEGOTIABLE)** — PASS. WiFi runs on a dedicated task that shares **no mutex or + state with the pump/level watering path**; `pumps_force_off()` remains the first action in `app_main`. + The pure `WifiManager` depends only on `IWifiDriver`/`IConfigStore`/`ITimeProvider` — it is + *structurally incapable* of blocking or influencing watering. Host tests assert this dependency set and + that a stuck/failing driver never stalls the manager's tick contract (FR-014). **Scoping note:** the + watering *controller* (pump control loop) lands in PR-11; at PR-07 the isolation is verified + structurally (dependency set + separate task) plus HIL on the rig, and full watering-cycle-during-outage + is a PR-07 HIL item and re-validated in PR-12. This scoping is recorded here deliberately so it does not + surface as a Checkpoint-3 finding. +- **II. Host-Testability** — PASS. All logic (reconnect state machine, boot-mode decision, credential + validation) is pure and host-tested; hardware access is behind the new `IWifiDriver` interface in the + `interfaces` component, mirroring `IModbusClient`/`II2cBus`. IDF-touching classes contain no business + logic and are excluded from the linux build via the `if(${IDF_TARGET} STREQUAL "linux")` CMake guard. +- **III. Reproducible Builds** — PASS. Both board targets build in the pinned container; the new + `network` component adds only IDF-provided dependencies (no new managed components), so + `dependencies.lock` and the two `esp-modbus` pins are untouched. +- **IV. Frozen Legacy** — PASS. No change under `src/`, `include/`, `data/`, `test/`, `platformio.ini`. +- **V. Checkpoint-Gated AI Workflow** — PASS. Plan authored by the orchestrator; implementation will be + delegated to the `implementer` subagent after CP2 approval. +- **VI. English Outward** — PASS. All artifacts, code, and the setup page are in English. + +**Additional-constraints note (documented, compliant — not a deviation):** the AP password in +`CONFIG_WS_PROV_AP_PASSWORD` is an intentional *documented non-secret*, not a secret in the +"no secrets in the repo" sense. It only shields the brief unconfigured-device window; WPA2 is kept so the +operator's real home-WiFi credentials are never sent in cleartext during setup, and those real +credentials live only in NVS (constitution-compliant). A brand-new password is chosen at implementation. + +## Project Structure + +### Documentation (this feature) + +```text +specs/007-wifi-provisioning/ +├── plan.md # This file +├── research.md # Phase 0 — decisions & rationale +├── data-model.md # Phase 1 — entities + WifiManager state machine +├── quickstart.md # Phase 1 — host-test + HIL validation guide +├── contracts/ +│ ├── IWifiDriver.md # driver seam contract +│ ├── wifi-manager-states.md # state machine + reconnect timing contract +│ └── provisioning-portal.md # HTTP setup-page + POST contract +├── checklists/ +│ └── requirements.md # spec quality checklist (from /speckit-specify) +└── tasks.md # Phase 2 — /speckit-tasks output (NOT created here) +``` + +### Source Code (repository root) + +```text +firmware/ +├── components/ +│ ├── interfaces/include/interfaces/ +│ │ ├── IWifiDriver.h # NEW — pure driver seam (STA/AP control + event poll), no IDF +│ │ └── (IConfigStore.h, ITimeProvider.h — reused unchanged) +│ ├── network/ # NEW component +│ │ ├── CMakeLists.txt # linux-guarded: pure sources on host, +hardware on target +│ │ ├── Kconfig.projbuild? (see note) # or options live in main/Kconfig.projbuild (chosen: main/) +│ │ ├── include/network/ +│ │ │ ├── WifiManager.h # pure state machine (STA connect + reconnect schedule) +│ │ │ ├── WifiBootMode.h # pure decideBootMode(credsPresent, buttonHeld) +│ │ │ ├── WifiCredentialValidation.h # pure validate(ssid,password) +│ │ │ ├── EspWifiDriver.h # thin IWifiDriver impl (target-only) +│ │ │ ├── ProvisioningPortal.h # standalone esp_http_server (target-only) +│ │ │ └── testing/MockWifiDriver.h # host mock (scriptable events) +│ │ └── src/ +│ │ ├── WifiManager.cpp # pure (host + target) +│ │ ├── EspWifiDriver.cpp # target-only (esp_wifi/esp_netif/esp_event) +│ │ └── ProvisioningPortal.cpp # target-only (esp_http_server + setup page asset) +│ └── board/include/board/board.h # reused: BOARD_PIN_STATUS_LED, BOARD_PIN_BTN_CONFIG +├── main/ +│ ├── app_main.cpp # EDIT — add esp_netif_init + esp_event_loop_create_default, +│ │ # construct driver+manager, start wifi_task +│ ├── wifi_task.cpp / .h # NEW — task wrapper (mirrors sensor_task.cpp idiom) +│ └── Kconfig.projbuild # EDIT — add CONFIG_WS_PROV_* options +└── test_apps/host/main/ + ├── test_wifi.cpp # NEW — Unity suite: reconnect schedule, boot mode, validation + ├── test_main.cpp # EDIT — declare + call run_wifi_tests() + └── CMakeLists.txt # EDIT — add test_wifi.cpp to SRCS, `network` to REQUIRES +``` + +**Structure Decision**: A new `network` component holds the WiFi logic and its hardware drivers, +following the established `sensors`/`actuators`/`storage` component shape (pure sources compiled on both +targets; `esp_wifi`/`esp_netif`/`esp_event`/`esp_http_server` sources compiled only when +`IDF_TARGET != linux`, with those IDF deps in `PRIV_REQUIRES`). `IWifiDriver` goes in the existing +`interfaces` component so the pure `WifiManager` never sees an IDF header, exactly as `IModbusClient` +and `II2cBus` do. Kconfig options are added to `main/Kconfig.projbuild` (the existing `menu +"WateringSystem"`) to match how `CONFIG_WS_*` options are declared today, rather than adding a component +`Kconfig`. The task wrapper lives in `main/wifi_task.cpp` mirroring `main/sensor_task.cpp`. + +## Complexity Tracking + +> No constitution violations. Table intentionally empty. + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|-----------|------------|--------------------------------------| +| — | — | — | diff --git a/specs/007-wifi-provisioning/quickstart.md b/specs/007-wifi-provisioning/quickstart.md new file mode 100644 index 0000000..71a04f9 --- /dev/null +++ b/specs/007-wifi-provisioning/quickstart.md @@ -0,0 +1,78 @@ +# Quickstart / Validation Guide: WiFi Provisioning & Station Management + +How to prove PR-07 works. CI-tagged items gate the merge; HIL-tagged items run on the rev1 devkit rig at +Checkpoint 3. + +## Prerequisites + +- ESP-IDF v6.0.1 via Docker. **Docker cannot mount the OneDrive tree** — rsync the firmware to `/tmp` + first: + ```bash + rsync -a --delete --exclude build --exclude managed_components --exclude sdkconfig \ + "$PWD/firmware/" /tmp/ws007-firmware/ + ``` +- On board switch: `idf.py fullclean` + `rm -f sdkconfig`. + +## CI validation (host tests + both target builds) + +### Host tests (`test_apps/host`, IDF linux preview target) + +Run the Unity suite (exit code = failure count). The new `run_wifi_tests()` must cover: + +- **Reconnect schedule** (contract `wifi-manager-states.md`): retry only at 10 s; pause 60 s after 5 + failures; monitor at 5 s; no attempts during pause; no boot loop under infinite failure. +- **Boot-mode truth table**: all four `decideBootMode` rows. +- **Credential validation**: SSID 1–32 accept / empty + >32 reject; password empty accept, 1–7 reject, + ≥8 accept, >64 reject. +- **Connect happy path**: `Connected` after `GotIp`, failures reset to 0. +- **Isolation (FR-014)**: `tick()` never blocks on a hung/silent driver; manager has no watering + dependency. + +```bash +# from the rsync'd copy +docker run --rm -v /tmp/ws007-firmware:/project -w /project espressif/idf:v6.0.1 \ + bash -lc "idf.py --preview set-target linux && idf.py -C test_apps/host build && \ + ./test_apps/host/build/*.elf" +``` + +### Both board targets build + +```bash +for b in BOARD_REV1_DEVKIT BOARD_REV2; do + docker run --rm -v /tmp/ws007-firmware:/project -w /project espressif/idf:v6.0.1 \ + bash -lc "idf.py fullclean; rm -f sdkconfig; \ + idf.py -DSDKCONFIG_DEFAULTS='' set-target esp32 && idf.py build" # + select $b via Kconfig +done +``` + +- **Acceptance [CI]**: both targets build; `idf.py size` confirms the portal fits the 1.5 MiB slot + (research R2); `dependencies.lock` and the two `esp-modbus` pins are unchanged. +- **Acceptance [CI]**: the FR9 SoftAP-portal decision is documented in this spec directory and referenced + from `firmware/CLAUDE.md`. + +## HIL validation (rev1 devkit rig — Checkpoint 3) + +From `docs/prd/PR-07-wifi-provisioning.md` acceptance criteria + parity §7: + +1. **Fresh device provisioning**: flash with empty credentials → device boots into AP mode, SSID visible, + setup page reachable at 192.168.4.1 (WPA2, using `CONFIG_WS_PROV_AP_PASSWORD`). Submit real + credentials → success page → device restarts (~3 s) → joins the LAN. +2. **AP power-cycle / outage isolation**: with the device connected (and a watering-relevant task + running), cut the AP → confirm sensor polling / pump-control cadence continue unaffected; restore AP → + device reconnects on the 10 s/60 s schedule. +3. **Config-button-at-boot**: on an already-configured device, hold `BOARD_PIN_BTN_CONFIG` (GPIO18) at + boot → credentials cleared → device enters provisioning mode. +4. **Wrong-password**: provision a wrong home-WiFi password → device keeps retrying with the schedule, + stays provisionable, **no boot loop**. +5. **Console/portal equivalence**: credentials set via the portal and via diag console + `config wifi ` produce the same `wifi=configured` state (research D10). +6. **LED (parity §7/§9)**: connect-attempt toggle ~500 ms; config-button-hold blink ~100 ms. +7. **Brownout watch (QUIRK 4)**: no spike-induced resets during the AP power-cycle test with brownout + detection left enabled. + +## Definition of done + +- All host-test items above pass in CI (green, 0 failures) and both board targets build. +- HIL checklist executed by Paul on the rig (or explicitly deferred with rationale if rig/hardware + unavailable, mirroring the deferred-HIL register in the handover). +- `firmware/CLAUDE.md` references the FR9 decision recorded here. diff --git a/specs/007-wifi-provisioning/research.md b/specs/007-wifi-provisioning/research.md new file mode 100644 index 0000000..030b6b3 --- /dev/null +++ b/specs/007-wifi-provisioning/research.md @@ -0,0 +1,127 @@ +# Phase 0 Research: WiFi Provisioning & Station Management + +All decisions grounded in the verified codebase map (origin/main) and the pre-made project decisions +recorded in `docs/prd/PR-07-wifi-provisioning.md`, `docs/parity-checklist.md` §7, and the Fable→Opus +handover. No open `NEEDS CLARIFICATION` items remain. + +## D1 — Provisioning mechanism: custom SoftAP portal + +- **Decision**: Custom SoftAP portal (AP at 192.168.4.1, WPA2, minimal setup page + one POST handler on a + standalone `esp_http_server`). +- **Rationale**: Pre-decided by Paul (2026-06-10). Feature parity with the Arduino unit; works from any + phone browser; no dependency on Espressif provisioning apps. IDF `wifi_provisioning` is a documented + fallback ONLY if the portal proves fragile during implementation — in which case re-escalate to Paul. +- **Alternatives considered**: IDF `wifi_provisioning` (SoftAP/BLE transport) — deferred fallback; BLE + rejected earlier (NimBLE ~300–400 KB would not fit the 1.5 MiB slot budget). + +## D2 — Host-testable seam: pure WifiManager + IWifiDriver + +- **Decision**: Put all timing/state logic in a pure `WifiManager` (compiled on host + target) behind a + new `IWifiDriver` interface in the `interfaces` component; keep `esp_wifi`/`esp_netif`/`esp_event` in a + thin `EspWifiDriver` excluded from the linux build. +- **Rationale**: Mirrors the enforced triad already used by `ModbusSoilSensor`/`IModbusClient` and + `Bme280Sensor`/`II2cBus`. Satisfies Constitution II (host-testability) and lets the reconnect schedule + be tested deterministically with `FakeTimeProvider` instead of wall-clock waits. +- **Alternatives considered**: Driving `esp_wifi` directly from a task with inline timing — rejected: not + host-testable, violates the interface-layer rule, and hard to assert the 10 s/60 s schedule in CI. + +## D3 — Reconnection strategy = parity fixed-interval (NOT exponential backoff) + +- **Decision**: On loss, retry every **10 s**; after **5** consecutive failures, wait an extra **60 s** + before the next round; monitor connection health every **5 s** while in STA; monitoring suspended in AP + mode. +- **Rationale**: This is the *actual* Arduino behavior (`docs/parity-checklist.md` §7, + `src/main.cpp:102-105, 922-968`). The "exponential backoff" phrasing in older docs is a myth from a code + comment. Constants are runtime-relevant and captured as `WifiManager` construction parameters (defaults + = parity values) so they can be tuned without touching logic. +- **Alternatives considered**: Exponential backoff — would be a deliberate divergence; rejected to hold + parity unless a concrete reason emerges (would be flagged explicitly if chosen). + +## D4 — Event-driven driver, tick-driven manager + +- **Decision**: `esp_wifi` is event-driven; `EspWifiDriver` translates `esp_event` callbacks + (`WIFI_EVENT_STA_*`, `IP_EVENT_STA_GOT_IP`) into discrete `WifiEvent`s that the pure `WifiManager` + drains on each `tick(nowMs)`. The manager owns all timers via `ITimeProvider`; the driver owns no + timing. +- **Rationale**: Keeps the pure/hardware split clean and the state machine fully deterministic in host + tests (the mock driver scripts events; `FakeTimeProvider.advance()` drives timers). The default event + loop + netif are initialized once in `app_main` (currently absent — see D7). +- **Alternatives considered**: Blocking `esp_wifi_connect()` + polling status — rejected; less faithful to + IDF, harder to isolate. + +## D5 — Boot-mode decision is pure + +- **Decision**: A pure `decideBootMode(credentialsPresent, configButtonHeld) -> {Station, Provisioning}` + (config button OR empty credentials ⇒ Provisioning; else Station). The GPIO read of + `BOARD_PIN_BTN_CONFIG` and the credential presence check (`!IConfigStore::getWifiSsid().empty()`) feed + it from `app_main`; forcing provisioning on a configured device clears credentials + (`clearWifiCredentials()`) then enters AP mode. +- **Rationale**: The decision table is the part worth host-testing (all four combinations); the GPIO read + itself is trivial hardware. Matches parity entry conditions (unconfigured→AP; button-held-at-boot→ + emergency provisioning). +- **Alternatives considered**: Embedding the decision inside the driver — rejected (not host-testable). + +## D6 — Credential validation is pure and layered over IConfigStore + +- **Decision**: A pure `validateWifiCredentials(ssid, password)` enforces SSID length 1–32 and password + empty-or-≥8, returning a typed result; the portal calls it before `IConfigStore::setWifiCredentials()`. +- **Rationale**: `IConfigStore` enforces only max lengths (`kWifiSsidMaxLen=32`, `kWifiPasswordMaxLen=64`) + and never the WPA2 min-8 rule or the SSID-non-empty rule, so the feature owns the min bounds. Pure ⇒ + host-tested (accept/reject table incl. empty-password open-network case and 1–7-char reject). +- **Alternatives considered**: Relying solely on `setWifiCredentials()` validation — rejected; it does not + reject a 1–7-char password or an empty SSID submit. + +## D7 — app_main must add event loop + netif (NVS already present) + +- **Decision**: Add `esp_netif_init()` and `esp_event_loop_create_default()` in `app_main` after the + existing `nvs_flash_init()` and before constructing the WiFi driver; create the default STA and AP + netifs inside `EspWifiDriver`. +- **Rationale**: The research confirmed neither is called anywhere today, but both are prerequisites for + `esp_wifi`. NVS is already initialized (`app_main.cpp:~173`) and can be relied upon. `pumps_force_off()` + stays the very first action (safety invariant untouched). +- **Alternatives considered**: Initializing netif/event-loop lazily inside the driver — acceptable, but + centralizing one-time system init in `app_main` matches the existing I2C-bus "one owner" precedent. + +## D8 — Status exposure & LED scope + +- **Decision**: Expose the current `WifiConnectionState` via a thread-safe snapshot getter on the WiFi + manager (single read, no cross-call gap), consumable by status reporting and the status LED. PR-07 owns + only the **WiFi/provisioning-related** LED behavior on `BOARD_PIN_STATUS_LED`: connect-attempt toggle + (~500 ms) and config-button-hold blink (~100 ms), per parity §7/§9. Any non-WiFi LED semantics are left + to later PRs. +- **Rationale**: FR-015 requires state exposure, not a full LED subsystem. Scoping the LED to WiFi events + avoids gold-plating and keeps the parity §9 non-WiFi LED items with their owning features. LED patterns + are HIL-verified (not host-testable). +- **Alternatives considered**: A general LED/indicator abstraction now — rejected as premature; no other + consumer exists yet (this would be the first). + +## D9 — Concurrency: snapshot over Locked decorator for status reads + +- **Decision**: The WiFi manager is owned and mutated by exactly one task (the wifi task). Status readers + (diag console / future status API) read a **snapshot** guarded by a single mutex acquisition, avoiding + the documented cross-call atomicity gap of the `Locked*` per-call decorators. +- **Rationale**: Handover explicitly flags that `Locked*` wrappers are per-call atomic only; a snapshot + getter is the load-bearing pattern for consumers. Single-writer/many-reader with a snapshot avoids a + read-modify-write race. +- **Alternatives considered**: `LockedWifiManager` per-call decorator — insufficient for a consistent + multi-field status read; snapshot chosen instead. + +## D10 — Portal ↔ existing console overlap (bookkeeping) + +- **Decision**: Note that the diag console already exposes `config wifi ` / `wifi-clear` and + reports `wifi=configured|unconfigured` without echoing values (`diag_console.cpp`). The portal is an + additional path to the same `IConfigStore` credentials; both must agree. Flag for the HIL checklist that + console-set and portal-set credentials are equivalent. +- **Rationale**: Avoids a surprise at review/HIL where two provisioning paths exist. No code conflict — + both funnel through `setWifiCredentials()`. + +## Open risks carried to implementation + +- **R1 — Portal fragility**: if the SoftAP portal proves unreliable during HIL, the fallback is IDF + `wifi_provisioning`; that is a re-escalation to Paul, not an autonomous switch (per D1). +- **R2 — Binary size**: confirm the portal + WiFi stack still fit the 1.5 MiB OTA slot after + implementation (`idf.py size`); flag if margin is tight. +- **R3 — Brownout**: parity QUIRK 4 (Arduino disables brownout to mask WiFi power spikes). Target keeps + brownout enabled; watch for spike-induced resets on the rev1 rig during the AP-power-cycle HIL test. +- **R4 — rev2 pins provisional**: `BOARD_PIN_STATUS_LED`/`BTN_CONFIG` on rev2 are `TODO(SYNC1)`; rev2 LED + behavior is not finalized until the pin map freezes. diff --git a/specs/007-wifi-provisioning/spec.md b/specs/007-wifi-provisioning/spec.md new file mode 100644 index 0000000..b84126a --- /dev/null +++ b/specs/007-wifi-provisioning/spec.md @@ -0,0 +1,248 @@ +# Feature Specification: WiFi Provisioning & Station Management + +**Feature Branch**: `007-wifi-provisioning` + +**Created**: 2026-07-04 + +**Status**: Draft + +**Input**: PR-07 (`docs/prd/PR-07-wifi-provisioning.md`) — WiFi station management with robust +reconnection plus first-boot provisioning, implementing the FR9 baseline decision (custom SoftAP +portal). Parity source: `docs/parity-checklist.md` §7. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Provision a fresh device onto home WiFi (Priority: P1) + +An operator powers on a device that has never been configured. With no stored credentials the device +brings up its own WiFi access point with a known name and a reachable setup page. The operator connects +a phone or laptop to that access point, opens the setup page, picks/enters their home network name and +password, and submits. The device stores the credentials and restarts, after which it joins the home +network as a station. + +**Why this priority**: Without provisioning, a device out of the box cannot be reached at all — every +other network-dependent capability (status UI, remote control, time sync) is unreachable. This is the +minimum viable slice: a device the operator can get onto their network from any phone browser, with no +app install and no cable. + +**Independent Test**: Flash a device with empty credentials, confirm it exposes the setup access point +at the known address, submit valid credentials from a browser, and confirm the device restarts and +associates with the target network. + +**Acceptance Scenarios**: + +1. **Given** a device with an empty stored SSID, **When** it boots, **Then** it enters provisioning mode + and exposes a WPA2-protected access point at a fixed address serving a WiFi setup page. +2. **Given** the device is in provisioning mode, **When** the operator submits a valid SSID + (1–32 characters) and password (empty, or 8 or more characters), **Then** the credentials are persisted + and the device confirms success and restarts shortly after so the confirmation is delivered first. +3. **Given** the device restarted with freshly stored valid credentials for a reachable network, + **When** it boots, **Then** it starts in station mode and connects to that network. +4. **Given** the device is in provisioning mode, **When** the operator submits an SSID outside 1–32 + characters or a non-empty password shorter than 8 characters, **Then** the submission is rejected, no + credentials are stored, no restart occurs, and the device stays provisionable. + +--- + +### User Story 2 - Stay connected and survive network outages without affecting watering (Priority: P2) + +A configured device runs day to day on the greenhouse network. The home access point occasionally +reboots, drops, or briefly loses power. The device must notice the loss, keep retrying on a predictable +schedule until it reconnects, and expose its current connection state for the status LED and status +reporting — all without ever pausing, delaying, or otherwise disturbing sensor polling or pump control. + +**Why this priority**: The device's primary job is watering, which must never depend on the network. +Reconnection resilience protects the remote-visibility experience, but the hard requirement is that a +WiFi outage of any duration has zero effect on watering safety and timing. + +**Independent Test**: With the device connected and (in a bench scenario) an active watering cycle +running, cut the access point; confirm watering continues uninterrupted and the pump control loop and +sensor polling keep their cadence, then restore the access point and confirm the device reconnects +automatically on schedule. + +**Acceptance Scenarios**: + +1. **Given** a configured device connected in station mode, **When** the connection is lost, **Then** the + device retries connecting on a fixed 10-second interval. +2. **Given** repeated reconnection failures, **When** 5 consecutive attempts have failed, **Then** the + device waits an additional 60 seconds before beginning the next round of attempts. +3. **Given** the device is connected, **When** connection monitoring runs, **Then** it checks connection + health every 5 seconds; monitoring is suspended while the device is in provisioning (AP) mode. +4. **Given** an active watering cycle and running sensor polling, **When** WiFi is lost for any duration + and later restored, **Then** watering behavior, pump control timing, and sensor polling are completely + unaffected throughout, and the device reconnects when the network returns. +5. **Given** any point in the connect/reconnect lifecycle, **When** status is queried, **Then** the + current connection state (e.g. connecting, connected, disconnected/retrying, provisioning) is available + for status reporting and for driving the status LED. + +--- + +### User Story 3 - Recover a device with wrong or changed credentials (Priority: P3) + +The operator has moved the device to a new network, or mistyped the password during setup. They need a +reliable, cable-free way to force the device back into provisioning mode, and a device that mistyped its +password must never get stuck in a reboot loop. + +**Why this priority**: This is the recovery path. It is less frequent than first-boot setup and normal +operation, but without it a device with bad credentials becomes unrecoverable in the field without a +reflash. + +**Independent Test**: On an already-configured device, hold the config button while powering on and +confirm the device enters provisioning mode with its stored credentials cleared; separately, provision a +wrong password and confirm the device keeps retrying on schedule and remains provisionable rather than +rebooting repeatedly. + +**Acceptance Scenarios**: + +1. **Given** an already-configured device, **When** the config button is held during boot for the + required hold time, **Then** the stored WiFi credentials are cleared and the device restarts into + provisioning mode. +2. **Given** a device configured with a wrong home-WiFi password, **When** it boots and fails to connect, + **Then** it keeps retrying using the fixed-interval reconnection strategy and does not enter a boot + loop. +3. **Given** a device that cannot connect with its stored credentials, **When** the operator wants to + correct them, **Then** the device remains provisionable (reachable to receive new credentials) via the + config-button recovery path. + +--- + +### Edge Cases + +- **Empty vs. open network**: An empty submitted password is valid (open home network); a non-empty + password must be 8+ characters (WPA2 minimum). A 1–7 character password is rejected. +- **Submit while already configured**: Credential submission is only honored while the device is in + provisioning (AP) mode; a submission attempt outside provisioning mode is rejected. +- **Loss of the setup client mid-provisioning**: If the operator's phone disconnects from the setup AP + before submitting, the device stays in provisioning mode indefinitely (no timeout that abandons setup). +- **Network returns during the 60-second pause**: Reconnection resumes on the next scheduled round; a + returned network does not need to wait out the full pause beyond the normal retry cadence. +- **Config button held on an unconfigured device**: Already provisionable; the forced-provisioning path + is a no-op beyond confirming provisioning mode. +- **Credentials submitted, restart interrupted by power loss before it completes**: On next boot the + device reads whatever was persisted; if the write completed it connects, otherwise it is unconfigured + and re-enters provisioning. + +## Requirements *(mandatory)* + +### Functional Requirements + +**Provisioning entry & mode** + +- **FR-001**: The system MUST treat a device with an empty stored SSID as unconfigured and boot it into + provisioning mode. (The legacy `CONFIGURE_ME` sentinel is retired; unconfigured is represented as an + empty SSID string in stored configuration.) +- **FR-002**: In provisioning mode the system MUST expose a WiFi access point at a fixed, known address + (192.168.4.1) with a known SSID, serving a WiFi setup page. +- **FR-003**: The provisioning access point MUST use WPA2 with a password sourced from a build-time + configuration option (a documented non-secret), not a hard-coded source constant. A brand-new password + MUST be chosen at implementation; the legacy password (exposed in public git history) MUST NOT be reused. +- **FR-004**: The system MUST allow an operator to force provisioning mode on an already-configured device + by holding the config button during boot; doing so MUST clear the stored WiFi credentials and restart + into provisioning mode. + +**Credential capture & persistence** + +- **FR-005**: The setup page MUST accept an SSID of 1–32 characters and a password that is either empty or + at least 8 characters; submissions violating these bounds MUST be rejected without persisting anything. +- **FR-006**: Credential submission MUST be honored only while the device is in provisioning (AP) mode and + rejected otherwise. +- **FR-007**: On a valid submission the system MUST persist the credentials to non-volatile storage, return + a success confirmation to the client, and then restart the device after a short delay (~3 seconds) so the + confirmation is delivered before the restart. +- **FR-008**: Stored WiFi credentials MUST persist across reboots and MUST be read from non-volatile storage + at boot to decide between provisioning mode and station mode. + +**Station connection & reconnection** + +- **FR-009**: When configured, the system MUST start in station mode and attempt to connect using the + stored credentials. +- **FR-010**: On connection loss the system MUST retry connecting on a fixed 10-second interval (parity + baseline; explicitly NOT exponential backoff). +- **FR-011**: After 5 consecutive failed connection attempts the system MUST wait an additional 60 seconds + before starting the next round of attempts. +- **FR-012**: The system MUST monitor connection health every 5 seconds while in station mode, and MUST + suspend this monitoring while in provisioning (AP) mode. +- **FR-013**: A device configured with credentials that never succeed (e.g. wrong password) MUST keep + retrying under the reconnection strategy and MUST NOT enter a reboot loop, remaining provisionable via the + config-button recovery path. + +**Isolation from watering (safety)** + +- **FR-014**: WiFi connection, disconnection, reconnection, and provisioning activity MUST NOT block, + delay, or otherwise influence watering tasks (sensor polling and pump control). Loss of WiFi for any + duration MUST have zero effect on watering behavior or timing. + +**State exposure** + +- **FR-015**: The current WiFi connection state MUST be exposed for consumption by status reporting and by + the status LED indicator. + +### Key Entities *(include if feature involves data)* + +- **WiFi credentials**: The stored network name (SSID) and password used for station-mode connection. + Persisted in non-volatile storage (from the storage feature, PR-06). Unconfigured is represented by an + empty SSID. +- **Connection state**: The device's current network posture — e.g. provisioning, connecting, connected, + disconnected/retrying — together with the counters/flags needed to drive the reconnection schedule and + status reporting. +- **Provisioning AP configuration**: The known SSID, fixed address, and WPA2 password (from build-time + configuration) used when the device exposes its setup access point. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: An operator can take a factory-fresh (unconfigured) device and get it onto their home + network entirely from a phone or laptop browser, with no app install and no cable. +- **SC-002**: During a WiFi outage of any duration, an in-progress watering cycle completes on its normal + schedule and sensor polling keeps its normal cadence — measurably zero deviation attributable to the + network event. +- **SC-003**: After the home network returns from an outage, the device re-associates automatically within + one reconnection round of it becoming available, with no operator action. +- **SC-004**: A device given a wrong home-WiFi password stays powered and provisionable indefinitely + without rebooting on a loop, and can be corrected via the config-button recovery path. +- **SC-005**: An operator can force an already-configured device back into provisioning mode using only the + config button at boot, with no cable and no reflash. +- **SC-006**: Both firmware board targets build with the provisioning feature included, and the FR9 + provisioning-method decision is documented in the feature directory and referenced from + `firmware/CLAUDE.md`. + +## Assumptions + +- **FR9 method is pre-decided (not re-opened here)**: The provisioning mechanism is a custom SoftAP portal, + resolved by Paul on 2026-06-10. IDF's `wifi_provisioning` component is a documented fallback ONLY if the + portal proves fragile during implementation, in which case the choice is re-escalated to Paul. This spec + records and validates the portal choice; it does not re-evaluate it. +- **Minimal, standalone portal**: The provisioning setup page runs on a minimal standalone web server + instance. The full JSON status API and the `/api/wifi/scan` and `/api/wifi/config` endpoints belong to + PR-09 and are out of scope here. +- **AP SSID name**: The setup access point keeps a known, stable SSID consistent with the legacy behavior + (a fixed setup SSID such as `WateringSystem-Setup`); the exact string is confirmed at implementation. +- **Config-button hold time**: The forced-provisioning button hold at boot uses the established hold + duration (legacy: ~5 seconds during startup) unless bring-up shows a reason to change it. +- **STA and AP are mutually exclusive**: The device is either connected/attempting in station mode OR + serving the provisioning AP, not both simultaneously (matches legacy behavior). +- **No pre-persist credential validation**: The device does not test the submitted credentials against the + target network before persisting and restarting (matches legacy behavior); a wrong password is handled by + the reconnection strategy + config-button recovery, not by rejecting it at submit time. +- **Brownout detector stays enabled**: Unlike the legacy firmware (which disabled the brownout detector to + mask WiFi power spikes on the rev1 devkit), the target keeps brownout protection enabled unless bring-up + reproduces the symptom (parity checklist QUIRK 4). This is a watch-item during HIL, not a requirement of + this feature. + +### Dependencies + +- **PR-06 (NVS/storage)** — provides the persisted-configuration store from which WiFi credentials are read + and to which they are written. Unconfigured state is an empty SSID string in that store. +- **Board component (config button, status LED GPIO)** — provides the config-button input used for forced + provisioning and the status LED used for connection-state indication. + +### Out of Scope + +- HTTP status/control API and the `/api/wifi/scan`, `/api/wifi/config`, and full status JSON endpoints + (PR-09). +- SNTP / time synchronization (PR-08). +- The full web frontend and its LittleFS assets (PR-10). +- OTA updates (PR-13). +- mDNS or fixed-hostname conveniences beyond what is free with the chosen components (explicitly not + gold-plated). diff --git a/specs/007-wifi-provisioning/tasks.md b/specs/007-wifi-provisioning/tasks.md new file mode 100644 index 0000000..f16aa88 --- /dev/null +++ b/specs/007-wifi-provisioning/tasks.md @@ -0,0 +1,288 @@ +--- + +description: "Task list for WiFi Provisioning & Station Management (PR-07)" +--- + +# Tasks: WiFi Provisioning & Station Management + +**Input**: Design documents from `specs/007-wifi-provisioning/` + +**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/ + +**Tests**: INCLUDED. Host-testability is a non-negotiable constitution principle (II) and the CI host +suite is the merge gate. Pure-logic tasks are written test-first (test task precedes implementation task). +Hardware classes (`EspWifiDriver`, `ProvisioningPortal`) are excluded from the linux build and verified by +HIL, not host tests. + +**Organization**: Grouped by user story (US1 provisioning = MVP, US2 station+reconnect, US3 recovery). + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no incomplete dependencies) +- **[Story]**: US1 / US2 / US3 (setup, foundational, polish have no story label) +- All paths are under the repo root; firmware lives in `firmware/`. + +--- + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Scaffold the new `network` component and Kconfig; wire the host test suite. + +- [ ] T001 Create the `network` component skeleton: `firmware/components/network/CMakeLists.txt` with the + `if(${IDF_TARGET} STREQUAL "linux")` guard (pure sources — `WifiManager.cpp` — on host; pure + hardware + sources on target), `REQUIRES interfaces board`, IDF deps (`esp_wifi esp_netif esp_event + esp_http_server`) in `PRIV_REQUIRES` on the target branch only; add `include/` dir. +- [ ] T002 Add Kconfig options in `firmware/main/Kconfig.projbuild` under `menu "WateringSystem"` + (`WS_` prefix, mirror `WS_INA226_SHUNT_MILLIOHM` style): `WS_PROV_AP_SSID` (string, default + `"WateringSystem-Setup"`), `WS_PROV_AP_PASSWORD` (string; documented non-secret; help text notes WPA2 + + choose-new-at-impl), and optional int options for the reconnect constants + (`WS_WIFI_RETRY_INTERVAL_MS`=10000, `WS_WIFI_FAILS_BEFORE_PAUSE`=5, `WS_WIFI_PAUSE_MS`=60000, + `WS_WIFI_MONITOR_INTERVAL_MS`=5000) with `range`s. +- [ ] T003 [P] Register a WiFi host-test suite entry point: add `test_wifi.cpp` to `SRCS` and `network` to + `REQUIRES` in `firmware/test_apps/host/main/CMakeLists.txt`; declare + call `run_wifi_tests()` in + `firmware/test_apps/host/main/test_main.cpp` (empty suite stub compiles and links). + +**Checkpoint**: `network` component and host suite compile empty on both targets + linux. + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: The seam + shared types + system init that BOTH provisioning and station work build on. + +**⚠️ CRITICAL**: No user story work can begin until this phase is complete. + +- [ ] T004 Define `IWifiDriver` in `firmware/components/interfaces/include/interfaces/IWifiDriver.h` + (pure, NO IDF includes) per `contracts/IWifiDriver.md`: `enum class WifiEvent`, and virtual methods + `staConnect`, `staStop`, `apStart`, `apStop`, `pollEvent`, `rssi`. Include guard + `WATERINGSYSTEM_INTERFACES_IWIFIDRIVER_H`. +- [ ] T005 [P] Define shared WiFi state types in + `firmware/components/network/include/network/WifiState.h`: `enum class WifiState` (Provisioning, + Connecting, Connected, Reconnecting, ReconnectPaused), `struct WifiConnectionSnapshot` (state, rssi, + consecutiveFailures, disconnectCount, ipAcquired), and `struct ReconnectPolicy` (defaults = parity / + Kconfig constants). Pure header. +- [ ] T006 [P] Create `MockWifiDriver` in + `firmware/components/network/include/network/testing/MockWifiDriver.h`: scriptable event queue + (`queueEvent` + helpers `scriptConnectSuccess`/`scriptConnectFailure`), call counters, last-ssid/last- + password capture, settable `rssi()`. Host-only; no IDF. +- [ ] T007 Add system network init to `firmware/main/app_main.cpp`: call `esp_netif_init()` and + `esp_event_loop_create_default()` AFTER the existing `nvs_flash_init()` and BEFORE any WiFi construction. + Do NOT move `pumps_force_off()` — it stays the first action. Log via `ESP_LOG*` with a tag; treat + failures as non-fatal-but-logged consistent with existing init style. + +**Checkpoint**: interface + mock + shared types available; app_main initializes netif + default event +loop. User stories can now proceed. + +--- + +## Phase 3: User Story 1 - Provision a fresh device onto home WiFi (Priority: P1) 🎯 MVP + +**Goal**: An unconfigured device exposes a reachable WPA2 SoftAP setup page at 192.168.4.1; a valid SSID + +password submission is validated, persisted to NVS, and the device restarts to apply it. + +**Independent Test**: Flash empty credentials → device enters AP mode → submit valid credentials from a +browser → success response → device persists and restarts. (Host tests cover the pure validation + boot- +mode decision; the AP/HTTP path is HIL.) + +### Tests for User Story 1 (write first, ensure they FAIL) + +- [ ] T008 [P] [US1] Host tests for `validateWifiCredentials` in + `firmware/test_apps/host/main/test_wifi.cpp`: SSID empty→reject, 1–32→accept, >32→reject; password + empty→accept, 1–7→reject, ≥8→accept, >64→reject (per data-model validation rules). +- [ ] T009 [P] [US1] Host tests for `decideBootMode` in `firmware/test_apps/host/main/test_wifi.cpp`: all + four rows of the truth table (contract `wifi-manager-states.md`). + +### Implementation for User Story 1 + +- [ ] T010 [P] [US1] Implement pure `validateWifiCredentials(ssid, password)` in + `firmware/components/network/include/network/WifiCredentialValidation.h` (+ `.cpp` if needed) returning a + typed accept/reject result; enforces SSID 1–32 and password empty-or-8..64. +- [ ] T011 [P] [US1] Implement pure `decideBootMode(credentialsPresent, configButtonHeld)` and + `enum class WifiBootMode` in `firmware/components/network/include/network/WifiBootMode.h`. +- [ ] T012 [US1] Implement `ProvisioningPortal` (target-only) in + `firmware/components/network/include/network/ProvisioningPortal.h` + `src/ProvisioningPortal.cpp`: + standalone `esp_http_server`; `GET /` serves a minimal self-contained English setup page (SSID + + password form); `POST /wifi/config` parses form, calls `validateWifiCredentials`, then + `IConfigStore::setWifiCredentials`. Never logs/echoes the password. Keep the page small (OTA-slot + budget, research R2). Per `contracts/provisioning-portal.md`. +- [ ] T013 [US1] Implement the post-save restart: on persist success, respond 200 (success page, + `restartRequired`) then schedule `esp_restart()` ~3 s later so the response is delivered first + (`ProvisioningPortal.cpp`). On validation reject → 4xx no-persist-no-restart; on persist failure → 5xx, + stay provisionable. +- [ ] T014 [US1] Wire provisioning into `app_main`: when `decideBootMode` → Provisioning, start the AP via + the driver (`apStart` using `CONFIG_WS_PROV_AP_SSID` / `CONFIG_WS_PROV_AP_PASSWORD`, WPA2) and start + `ProvisioningPortal`; do not start STA connect in this mode. + +**Checkpoint**: Fresh/empty-credential device serves a reachable setup page and persists+restarts on valid +submit. Pure validation + boot-mode logic green in CI. MVP demoable (HIL). + +--- + +## Phase 4: User Story 2 - Stay connected and survive outages without affecting watering (Priority: P2) + +**Goal**: A configured device connects in STA mode, reconnects on the parity fixed-interval schedule +(10 s / +60 s after 5 / 5 s monitor), exposes its connection state, and never lets WiFi activity touch +watering. + +**Independent Test**: Connect the device; drop the AP (with a watering-relevant task running on the rig) +→ watering cadence unaffected → restore AP → device reconnects on schedule. Host tests cover the schedule +and the isolation property deterministically. + +### Tests for User Story 2 (write first, ensure they FAIL) + +- [ ] T015 [P] [US2] Host tests for the reconnect schedule in `firmware/test_apps/host/main/test_wifi.cpp` + using `MockWifiDriver` + `FakeTimeProvider` (contract `wifi-manager-states.md` §Timing): connect happy + path; retry only at 10 s (assert none at 9999 ms); pause 60 s after 5 consecutive failures with no + attempts during the pause and failures reset after; monitor evaluates at 5 s; AP-mode suspends + monitoring. +- [ ] T016 [P] [US2] Host test for FR-014 isolation in `firmware/test_apps/host/main/test_wifi.cpp`: + `tick()` returns promptly with a silent/hung driver (never blocks); assert `WifiManager`'s constructor + takes only `IWifiDriver`/`IConfigStore`/`ITimeProvider`/`ReconnectPolicy` (no watering type) — document + the dependency set. + +### Implementation for User Story 2 + +- [ ] T017 [US2] Implement the pure `WifiManager` state machine in + `firmware/components/network/include/network/WifiManager.h` + `src/WifiManager.cpp`: constructor injects + `IWifiDriver&`, `IConfigStore&`, `ITimeProvider&`, `ReconnectPolicy`; `begin(WifiBootMode)`, `tick()` + (drains `pollEvent()`, advances timers via `nowMs()`), `snapshot()`. Implements all transitions in + `data-model.md` (Connecting→Connected on GotIp; retry cadence; pause after 5; monitor; no boot loop). +- [ ] T018 [US2] Implement `EspWifiDriver` (target-only) in + `firmware/components/network/include/network/EspWifiDriver.h` + `src/EspWifiDriver.cpp`: create STA + AP + netifs, register `esp_event` handlers translating `WIFI_EVENT_*`/`IP_EVENT_STA_GOT_IP` into a + thread-safe `WifiEvent` queue drained by `pollEvent()`; implement `staConnect/staStop/apStart/apStop/ + rssi` non-blocking per `contracts/IWifiDriver.md`. No timing, no business logic. +- [ ] T019 [US2] Implement the WiFi task in `firmware/main/wifi_task.h` + `firmware/main/wifi_task.cpp` + mirroring `main/sensor_task.cpp`: `xTaskCreate` with its own stack/priority, `[[noreturn]]`, manager + injected via `void* arg`, fixed-cadence `tick()` loop, non-fatal creation failure. Separate task from + the pump/level 10 Hz loop (FR-014). +- [ ] T020 [US2] Wire station mode into `app_main`: when `decideBootMode` → Station, construct + `EspWifiDriver` + `WifiManager`, `begin(Station)`, and `wifi_task_start(manager)`. Keep everything after + `pumps_force_off()` and the existing sensor/console setup; ensure no shared mutex with watering. +- [ ] T021 [US2] Expose connection state: implement `snapshot()` consumers hook — make the manager + snapshot readable by the diag console (add/extend a `wifi` status line reading the snapshot without + echoing credentials). Drive `BOARD_PIN_STATUS_LED` connect-attempt toggle (~500 ms) from the wifi task + (parity §7/§9). Single-acquisition snapshot (research D9). + +**Checkpoint**: Configured device connects and reconnects on the parity schedule; state exposed; schedule ++ isolation green in CI. + +--- + +## Phase 5: User Story 3 - Recover a device with wrong or changed credentials (Priority: P3) + +**Goal**: Config-button-held-at-boot forces provisioning (clearing stored credentials); a wrong password +never causes a boot loop and the device stays provisionable. + +**Independent Test**: Hold `BOARD_PIN_BTN_CONFIG` (GPIO18) at boot on a configured device → credentials +cleared → provisioning mode. Separately, provision a wrong password → device keeps retrying, stays +provisionable, no reboot loop. + +### Tests for User Story 3 (write first, ensure they FAIL) + +- [ ] T022 [P] [US3] Host test "no boot loop under permanent failure" in + `firmware/test_apps/host/main/test_wifi.cpp`: script an infinite `ConnectFailed` stream across many + rounds; assert the machine only cycles Reconnecting↔ReconnectPaused, never requests a restart, bounded + state (FR-013). (Extends the US2 machine; validated here as the US3 acceptance.) +- [ ] T023 [P] [US3] Host test for the emergency-reset decision in + `firmware/test_apps/host/main/test_wifi.cpp`: `decideBootMode(credentialsPresent=true, buttonHeld=true)` + → Provisioning (already in T009's table; here assert the paired "clear credentials" intent is invoked in + the boot flow via a small testable helper if one is extracted). + +### Implementation for User Story 3 + +- [ ] T024 [US3] Read the config button at boot in `app_main`: sample `BOARD_PIN_BTN_CONFIG` (GPIO18) with + the required hold semantics (parity: held during startup) and feed `configButtonHeld` into + `decideBootMode`. Keep the GPIO read minimal (no new abstraction required). +- [ ] T025 [US3] Emergency reset flow in `app_main`: when the button forces provisioning on a configured + device, call `IConfigStore::clearWifiCredentials()` before entering AP mode (per `data-model.md` boot + rule). +- [ ] T026 [US3] Config-button-hold LED feedback: blink `BOARD_PIN_STATUS_LED` ~100 ms during the button + hold window (parity §7/§9). HIL-verified. + +**Checkpoint**: All three stories independently functional; recovery paths validated. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +- [ ] T027 Reference the FR9 SoftAP-portal decision from `firmware/CLAUDE.md` (acceptance-criteria [CI] + requirement), pointing at `specs/007-wifi-provisioning/`. (Root/agent CLAUDE.md is edited by the + implementer as an explicit task, not via the spec-kit agent-context hook.) +- [ ] T028 Run `idf.py size` on both targets; confirm the app (incl. portal + WiFi stack) fits the 1.5 MiB + OTA slot; record the margin in the PR description / note if tight (research R2). +- [ ] T029 [P] Confirm `dependencies.lock` and both `esp-modbus` pins (main/ + components/sensors/) are + unchanged by this PR (no new managed components introduced). +- [ ] T030 Author the HIL checklist file for Paul (rev1 rig) from `quickstart.md` §HIL: fresh-device + provisioning, AP power-cycle isolation, config-button-at-boot, wrong-password no-boot-loop, console/ + portal credential equivalence, LED patterns, brownout watch (QUIRK 4). Note deferral path if hardware + is unavailable. +- [ ] T031 Run the full host suite on the linux target; confirm 0 failures and both board targets build + green (quickstart.md CI section). + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: no dependencies. +- **Foundational (Phase 2)**: after Setup — BLOCKS all stories (interface, mock, types, netif/event-loop). +- **US1 (Phase 3)**: after Foundational. MVP. +- **US2 (Phase 4)**: after Foundational. Shares `app_main` wiring with US1 (T014/T020 touch the same + file — sequence them, not parallel). +- **US3 (Phase 5)**: after US2's state machine exists (T022 extends it) and after US1's provisioning entry + (T024/T025 route into the provisioning path). Sequence after US1+US2. +- **Polish (Phase 6)**: after all desired stories. + +### Story independence & coupling notes + +- US1 and US2 both edit `firmware/main/app_main.cpp` (boot-mode branch) — treat those tasks as sequential. +- US3's "no boot loop" (T022) is a property of the US2 `WifiManager`; it is listed under US3 because it is + US3's acceptance criterion, but it needs T017 done first. +- Pure-logic tasks (T008–T011, T015–T017, T022–T023) are host-tested; hardware tasks (T012–T014, T018, + T024–T026) are HIL-only. + +### Parallel opportunities + +- T005 / T006 (Foundational) are parallel. +- Within US1: T008/T009 (tests) parallel; T010/T011 (pure impl) parallel. +- Within US2: T015/T016 (tests) parallel; then T017 before T018–T021. +- T029 parallel with other polish. + +--- + +## Parallel Example: User Story 1 + +```bash +# Tests first (parallel): +Task: "Host tests for validateWifiCredentials in firmware/test_apps/host/main/test_wifi.cpp" # T008 +Task: "Host tests for decideBootMode in firmware/test_apps/host/main/test_wifi.cpp" # T009 +# Then pure implementations (parallel): +Task: "Implement validateWifiCredentials (WifiCredentialValidation.h)" # T010 +Task: "Implement decideBootMode (WifiBootMode.h)" # T011 +``` + +--- + +## Implementation Strategy + +### MVP first (US1) + +1. Phase 1 Setup → 2. Phase 2 Foundational → 3. Phase 3 US1 → **STOP & VALIDATE** provisioning path + (host: validation + boot mode; HIL: reachable setup page persists+restarts). + +### Incremental delivery + +- Setup + Foundational → foundation ready. +- + US1 → provisioning MVP (device is configurable from a browser). +- + US2 → device actually joins the LAN and survives outages without touching watering. +- + US3 → field recovery (button reset, no boot loop). + +### Notes + +- Commit after each task or logical group (handover: "design missions to die cheaply — commit often"). +- Verify host tests fail before implementing the pure logic they cover. +- Builds run via Docker on an rsync'd `/tmp/ws007-firmware` copy (Docker can't mount OneDrive); host tests + on the linux preview target. +- Never modify frozen legacy paths (`src/`, `include/`, `data/`, `test/`, `platformio.ini`). From dc4496eb409ecf1698c240bbb63a35ecb86f4dac Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 10:46:45 +0200 Subject: [PATCH 2/7] =?UTF-8?q?feat(network):=20PR-07=20setup=20+=20founda?= =?UTF-8?q?tional=20=E2=80=94=20IWifiDriver=20seam,=20netif/event-loop=20i?= =?UTF-8?q?nit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1+2 of PR-07 (WiFi provisioning). Adds: - network component (linux-guarded CMake; header-only for now) - IWifiDriver pure interface (interfaces/) + WifiState types + MockWifiDriver - Kconfig: WS_PROV_AP_SSID/PASSWORD + WS_WIFI_* reconnect constants (parity defaults) - app_main: esp_netif_init() + esp_event_loop_create_default() after NVS, pumps_force_off() preserved as first action; no WiFi objects constructed yet - host wifi test suite registered (run_wifi_tests stub) Verified: host tests 164/0, both board targets build + board-config verified. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../include/interfaces/IWifiDriver.h | 95 ++++++++++++++ firmware/components/network/CMakeLists.txt | 34 +++++ .../network/include/network/WifiState.h | 65 ++++++++++ .../include/network/testing/MockWifiDriver.h | 117 ++++++++++++++++++ firmware/main/CMakeLists.txt | 2 +- firmware/main/Kconfig.projbuild | 54 ++++++++ firmware/main/app_main.cpp | 20 +++ firmware/test_apps/host/main/CMakeLists.txt | 3 +- firmware/test_apps/host/main/test_main.cpp | 2 + firmware/test_apps/host/main/test_wifi.cpp | 18 +++ 10 files changed, 408 insertions(+), 2 deletions(-) create mode 100644 firmware/components/interfaces/include/interfaces/IWifiDriver.h create mode 100644 firmware/components/network/CMakeLists.txt create mode 100644 firmware/components/network/include/network/WifiState.h create mode 100644 firmware/components/network/include/network/testing/MockWifiDriver.h create mode 100644 firmware/test_apps/host/main/test_wifi.cpp diff --git a/firmware/components/interfaces/include/interfaces/IWifiDriver.h b/firmware/components/interfaces/include/interfaces/IWifiDriver.h new file mode 100644 index 0000000..016250a --- /dev/null +++ b/firmware/components/interfaces/include/interfaces/IWifiDriver.h @@ -0,0 +1,95 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file IWifiDriver.h + * @brief WiFi hardware seam (STA/AP control + non-blocking event poll). + * + * The host-test seam for WiFi (feature 007): the pure WifiManager holds all + * timing and state-machine logic above this interface and is host-tested + * against MockWifiDriver; EspWifiDriver is the only hardware-touching + * implementation (esp_wifi/esp_netif/esp_event) and is excluded from the + * linux build. Mirrors IModbusClient / II2cBus. Normative contract: + * specs/007-wifi-provisioning/contracts/IWifiDriver.md. + * + * Part of the header-only `interfaces` component: no IDF includes allowed. + */ + +#ifndef WATERINGSYSTEM_INTERFACES_IWIFIDRIVER_H +#define WATERINGSYSTEM_INTERFACES_IWIFIDRIVER_H + +#include +#include + +/** + * @brief WiFi lifecycle events, drained one per tick via pollEvent(). + * + * `Connected` = association succeeded (L2). `GotIp` = DHCP lease acquired — + * the "usable" signal the manager treats as fully connected. + * `Disconnected` / `ConnectFailed` = drop / association failure (the manager + * treats both as "attempt failed"). `None` = the event queue is empty. + * + * Event ordering (contract): a successful connect delivers `Connected` then + * `GotIp`; a failure delivers `ConnectFailed` or `Disconnected`. + */ +enum class WifiEvent { None, Connected, GotIp, Disconnected, ConnectFailed }; + +/** + * @brief STA/AP control plus a non-blocking event queue. + * + * Behavioral contract: no method blocks on network I/O — the outcome of a + * connect/AP-start attempt arrives later as an event via pollEvent(). This + * keeps the pure WifiManager deterministic and lets the wifi task never + * stall (FR-014). The driver owns no timers (all cadence lives in + * WifiManager via ITimeProvider) and touches only WiFi/netif/event-loop + * resources — never pump/sensor state. + */ +class IWifiDriver { +public: + virtual ~IWifiDriver() = default; + + /** + * @brief Configure STA and begin association (non-blocking). + * + * @param ssid Network SSID to join. + * @param password Network password (empty for an open network). + * @return false only on a synchronous config error; success/failure of + * the attempt itself arrives later as a WifiEvent. + */ + virtual bool staConnect(const std::string& ssid, + const std::string& password) = 0; + + /** + * @brief Stop STA / disconnect. Idempotent. + */ + virtual void staStop() = 0; + + /** + * @brief Start the SoftAP (WPA2) at 192.168.4.1 (non-blocking). + * + * @param ssid AP SSID to advertise. + * @param password AP password (WPA2). + * @return false only on a synchronous config error. + */ + virtual bool apStart(const std::string& ssid, + const std::string& password) = 0; + + /** + * @brief Stop the SoftAP. Idempotent. + */ + virtual void apStop() = 0; + + /** + * @brief Drain the next queued event, or None if the queue is empty. + * + * Called once per manager tick; thread-safe (the driver enqueues from + * the esp_event callback). + */ + virtual WifiEvent pollEvent() = 0; + + /** + * @brief Last known RSSI in dBm; unspecified when not connected. + */ + virtual int8_t rssi() const = 0; +}; + +#endif /* WATERINGSYSTEM_INTERFACES_IWIFIDRIVER_H */ diff --git a/firmware/components/network/CMakeLists.txt b/firmware/components/network/CMakeLists.txt new file mode 100644 index 0000000..cbbc9ae --- /dev/null +++ b/firmware/components/network/CMakeLists.txt @@ -0,0 +1,34 @@ +# network — WiFi station management + first-boot SoftAP provisioning. +# +# Same split as sensors/actuators/storage: all timing and state-machine +# logic lives in pure C++ (WifiManager, decideBootMode, credential +# validation) driven by ITimeProvider + the IWifiDriver seam, so it builds +# and is unit-tested on the linux preview target. The IDF-touching classes +# (EspWifiDriver over esp_wifi/esp_netif/esp_event, ProvisioningPortal over +# esp_http_server) carry no business logic and are excluded from the linux +# build (research.md, same mechanism as the other components). +# +# Phase 1/2 scaffolding (tasks T001–T007): no pure sources exist yet — +# WifiManager.cpp and the hardware sources arrive with US1/US2. The +# component therefore registers header-only for now (INCLUDE_DIRS only) and +# still compiles cleanly on both targets. When pure sources land, add them +# to SRCS on BOTH branches; hardware sources go to SRCS on the target +# branch only. +if(${IDF_TARGET} STREQUAL "linux") + # Host build: pure sources only (none yet — header-only for now). + idf_component_register( + INCLUDE_DIRS "include" + REQUIRES interfaces board + ) +else() + # Target build: pure sources + the hardware touchpoints (none yet). The + # WiFi/HTTP-server IDF deps are private — they appear only in the + # EspWifiDriver / ProvisioningPortal .cpp files, never in this + # component's public headers (same rule as storage's littlefs and + # sensors' esp-modbus). + idf_component_register( + INCLUDE_DIRS "include" + REQUIRES interfaces board + PRIV_REQUIRES esp_wifi esp_netif esp_event esp_http_server + ) +endif() diff --git a/firmware/components/network/include/network/WifiState.h b/firmware/components/network/include/network/WifiState.h new file mode 100644 index 0000000..bd86217 --- /dev/null +++ b/firmware/components/network/include/network/WifiState.h @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file WifiState.h + * @brief Shared WiFi state types (pure — host + target). + * + * The state enum, the immutable status snapshot exposed to consumers, and + * the reconnect-policy construction parameters for the pure WifiManager + * (feature 007). No IDF includes; part of the header-only surface of the + * `network` component. Normative contract: + * specs/007-wifi-provisioning/contracts/wifi-manager-states.md and + * data-model.md. + */ + +#ifndef WATERINGSYSTEM_NETWORK_WIFISTATE_H +#define WATERINGSYSTEM_NETWORK_WIFISTATE_H + +#include + +/** + * @brief WifiManager state (data-model.md state machine). + * + * `Provisioning` = SoftAP up + portal serving, STA monitoring suspended. + * `Connecting` = first STA attempt in flight. `Connected` = GotIp acquired. + * `Reconnecting` = attempt failed, waiting retryIntervalMs before the next. + * `ReconnectPaused` = failuresBeforePause consecutive failures reached, + * waiting pauseMs before a fresh round (never reboots — FR-013). + */ +enum class WifiState { + Provisioning, + Connecting, + Connected, + Reconnecting, + ReconnectPaused +}; + +/** + * @brief Immutable status copy for status/LED consumers. + * + * Produced by WifiManager::snapshot() in a single mutex acquisition + * (research D9) so consumers see a consistent view of state + counters. + */ +struct WifiConnectionSnapshot { + WifiState state; ///< current state machine state + int8_t rssi; ///< last RSSI dBm; valid only in Connected + uint8_t consecutiveFailures; ///< 0..failuresBeforePause; drives the pause + uint32_t disconnectCount; ///< monotonic drop count, for diagnostics + bool ipAcquired; ///< true after GotIp +}; + +/** + * @brief Reconnect timing parameters (defaults = parity constants). + * + * Kept as a plain struct so the wiring site can override from Kconfig + * (CONFIG_WS_WIFI_*). Defaults match docs/parity-checklist.md §7: + * 10 s retry, +60 s pause after 5 consecutive failures, 5 s health monitor. + */ +struct ReconnectPolicy { + uint32_t retryIntervalMs = 10000; ///< delay between STA attempts + uint8_t failuresBeforePause = 5; ///< failures that trigger the pause + uint32_t pauseMs = 60000; ///< extra wait before the next round + uint32_t monitorIntervalMs = 5000; ///< health-check cadence (Connected) +}; + +#endif /* WATERINGSYSTEM_NETWORK_WIFISTATE_H */ diff --git a/firmware/components/network/include/network/testing/MockWifiDriver.h b/firmware/components/network/include/network/testing/MockWifiDriver.h new file mode 100644 index 0000000..20cdab6 --- /dev/null +++ b/firmware/components/network/include/network/testing/MockWifiDriver.h @@ -0,0 +1,117 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file MockWifiDriver.h + * @brief Scriptable IWifiDriver test double (header-only). + * + * Backs the WifiManager host tests (feature 007): a scriptable event queue + * consumed by pollEvent() (queueEvent + scriptConnectSuccess / + * scriptConnectFailure helpers), call counters for the STA/AP methods, the + * last ssid/password passed to staConnect/apStart, and a settable rssi(). + * No real networking; deterministic under FakeTimeProvider. Never compiled + * into target builds (only included from test code). No IDF includes. + * Mirrors MockConfigStore / MockModbusClient. + */ + +#ifndef WATERINGSYSTEM_NETWORK_TESTING_MOCKWIFIDRIVER_H +#define WATERINGSYSTEM_NETWORK_TESTING_MOCKWIFIDRIVER_H + +#include +#include +#include + +#include "interfaces/IWifiDriver.h" + +/** + * @brief IWifiDriver over a scripted event queue, instrumented for tests. + * + * pollEvent() pops the front of `events` (FIFO), or returns + * WifiEvent::None when the queue is empty — modelling the driver contract + * that a "hung" attempt simply never delivers an event. + */ +class MockWifiDriver : public IWifiDriver { +public: + // -- Instrumentation (public, MockConfigStore style) ------------------ + int staConnectCalls = 0; + int staStopCalls = 0; + int apStartCalls = 0; + int apStopCalls = 0; + + std::string lastStaSsid; ///< ssid of the most recent staConnect + std::string lastStaPassword; ///< password of the most recent staConnect + std::string lastApSsid; ///< ssid of the most recent apStart + std::string lastApPassword; ///< password of the most recent apStart + + bool staConnectResult = true; ///< false: synchronous staConnect failure + bool apStartResult = true; ///< false: synchronous apStart failure + + // -- Scripting -------------------------------------------------------- + + /** + * @brief Queue one event to be returned by a later pollEvent() (FIFO). + */ + void queueEvent(WifiEvent event) { events.push_back(event); } + + /** + * @brief Queue a successful connect: Connected followed by GotIp. + */ + void scriptConnectSuccess() + { + events.push_back(WifiEvent::Connected); + events.push_back(WifiEvent::GotIp); + } + + /** + * @brief Queue a failed connect: a single ConnectFailed. + */ + void scriptConnectFailure() { events.push_back(WifiEvent::ConnectFailed); } + + /** + * @brief Set the value returned by rssi(). + */ + void setRssi(int8_t value) { rssi_ = value; } + + /// Scripted event queue (public for direct assertions/manipulation). + std::deque events; + + // -- IWifiDriver ------------------------------------------------------ + + bool staConnect(const std::string& ssid, + const std::string& password) override + { + ++staConnectCalls; + lastStaSsid = ssid; + lastStaPassword = password; + return staConnectResult; + } + + void staStop() override { ++staStopCalls; } + + bool apStart(const std::string& ssid, + const std::string& password) override + { + ++apStartCalls; + lastApSsid = ssid; + lastApPassword = password; + return apStartResult; + } + + void apStop() override { ++apStopCalls; } + + WifiEvent pollEvent() override + { + if (events.empty()) { + return WifiEvent::None; + } + const WifiEvent event = events.front(); + events.pop_front(); + return event; + } + + int8_t rssi() const override { return rssi_; } + +private: + int8_t rssi_ = 0; +}; + +#endif /* WATERINGSYSTEM_NETWORK_TESTING_MOCKWIFIDRIVER_H */ diff --git a/firmware/main/CMakeLists.txt b/firmware/main/CMakeLists.txt index 448c4da..738d0d7 100644 --- a/firmware/main/CMakeLists.txt +++ b/firmware/main/CMakeLists.txt @@ -2,7 +2,7 @@ idf_component_register( SRCS "app_main.cpp" "diag_console.cpp" "sensor_task.cpp" PRIV_REQUIRES board esp_driver_gpio esp_app_format actuators interfaces console esp_timer - storage nvs_flash sensors + storage nvs_flash sensors esp_netif esp_event ) # Build-time littlefs image of the committed seed directory diff --git a/firmware/main/Kconfig.projbuild b/firmware/main/Kconfig.projbuild index 6ed8726..5ff2775 100644 --- a/firmware/main/Kconfig.projbuild +++ b/firmware/main/Kconfig.projbuild @@ -20,4 +20,58 @@ menu "WateringSystem" 0.5 mA — 2048 at the default 5 mOhm, rev2 BOM R1 5 mOhm 2512). Only change this when the fitted shunt changes; the current resolution is a compile-time driver constant (research.md R9). + + config WS_PROV_AP_SSID + string "Provisioning SoftAP SSID" + default "WateringSystem-Setup" + help + SSID advertised by the first-boot SoftAP setup portal + (feature 007). Shown to the operator during provisioning; not + a secret. + + config WS_PROV_AP_PASSWORD + string "Provisioning SoftAP password (WPA2)" + default "change-me-setup" + help + WPA2 password for the first-boot provisioning SoftAP + (feature 007). This is a DOCUMENTED NON-SECRET: it only shields + the brief unconfigured-device window, and WPA2 is kept so the + operator's real home-WiFi credentials are never sent in + cleartext during setup (those live only in NVS). CHOOSE A NEW + PASSWORD at build/deploy time — do NOT ship the placeholder and + do NOT reuse the legacy AP password. Must be 8..63 characters + for WPA2 (an empty value yields an open AP). + + config WS_WIFI_RETRY_INTERVAL_MS + int "WiFi reconnect retry interval (ms)" + default 10000 + range 1000 600000 + help + Delay between STA connect attempts while reconnecting + (parity default 10 s, docs/parity-checklist.md §7). + + config WS_WIFI_FAILS_BEFORE_PAUSE + int "WiFi consecutive failures before long pause" + default 5 + range 1 100 + help + Consecutive failed connect attempts that trigger the long + reconnect pause (parity default 5). + + config WS_WIFI_PAUSE_MS + int "WiFi long reconnect pause (ms)" + default 60000 + range 1000 3600000 + help + Extra wait after WS_WIFI_FAILS_BEFORE_PAUSE consecutive + failures before starting a fresh reconnect round (parity + default 60 s). Never reboots (FR-013). + + config WS_WIFI_MONITOR_INTERVAL_MS + int "WiFi connection-health monitor interval (ms)" + default 5000 + range 500 600000 + help + Health-check cadence while connected; suspended in + provisioning/AP mode (parity default 5 s). endmenu diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index d2e4726..f22b93b 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -23,8 +23,10 @@ #include "board/board.h" #include "esp_app_desc.h" +#include "esp_event.h" #include "esp_idf_version.h" #include "esp_log.h" +#include "esp_netif.h" #include "driver/gpio.h" #include "freertos/FreeRTOS.h" #include "freertos/task.h" @@ -185,6 +187,24 @@ extern "C" void app_main(void) esp_err_to_name(nvs_err)); } + // System network init (feature 007). The TCP/IP stack and the default + // event loop must exist before any WiFi driver is constructed (US1/US2); + // they are created once, here, after NVS (esp_wifi persists calibration + // in NVS) and before any WiFi object. Not safety-critical: a failure is + // logged and the system keeps running — WiFi is simply unavailable, and + // the watering path never depends on the network (FR-014). No WiFi + // objects are constructed yet in this phase. + esp_err_t netif_err = esp_netif_init(); + if (netif_err != ESP_OK) { + ESP_LOGE(TAG, "esp_netif_init failed: %s (WiFi unavailable)", + esp_err_to_name(netif_err)); + } + esp_err_t event_err = esp_event_loop_create_default(); + if (event_err != ESP_OK) { + ESP_LOGE(TAG, "default event loop create failed: %s (WiFi unavailable)", + esp_err_to_name(event_err)); + } + // littlefs mount-or-format of the `storage` partition at /storage // (FR-007; a corrupted filesystem is reformatted, never bricks). const esp_err_t mount_err = StorageMount::mount(); diff --git a/firmware/test_apps/host/main/CMakeLists.txt b/firmware/test_apps/host/main/CMakeLists.txt index a0fc93b..548a037 100644 --- a/firmware/test_apps/host/main/CMakeLists.txt +++ b/firmware/test_apps/host/main/CMakeLists.txt @@ -7,10 +7,11 @@ idf_component_register( "test_bme280.cpp" "test_level_sensor.cpp" "test_ina226.cpp" + "test_wifi.cpp" # Compile-time board-contract TUs (T016): each defines one board # selector before including the REAL board/board.h — passing = # compiling (static_asserts only, nothing registers with Unity). "test_board_contract_rev1.cpp" "test_board_contract_rev2.cpp" - REQUIRES unity actuators interfaces storage nvs_flash sensors board + REQUIRES unity actuators interfaces storage nvs_flash sensors board network ) diff --git a/firmware/test_apps/host/main/test_main.cpp b/firmware/test_apps/host/main/test_main.cpp index 6889b5f..c0ac4db 100644 --- a/firmware/test_apps/host/main/test_main.cpp +++ b/firmware/test_apps/host/main/test_main.cpp @@ -20,6 +20,7 @@ void run_soil_sensor_tests(void); void run_bme280_tests(void); void run_level_sensor_tests(void); void run_ina226_tests(void); +void run_wifi_tests(void); // Unity requires setUp/tearDown definitions (shared by all suites). extern "C" void setUp(void) {} @@ -35,5 +36,6 @@ extern "C" void app_main(void) run_bme280_tests(); run_level_sensor_tests(); run_ina226_tests(); + run_wifi_tests(); std::exit(UNITY_END()); } diff --git a/firmware/test_apps/host/main/test_wifi.cpp b/firmware/test_apps/host/main/test_wifi.cpp new file mode 100644 index 0000000..e40f460 --- /dev/null +++ b/firmware/test_apps/host/main/test_wifi.cpp @@ -0,0 +1,18 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file test_wifi.cpp + * @brief Unity host suite for WiFi provisioning & station management + * (feature 007). + * + * Phase 1/2 scaffolding (task T003): the suite entry point exists and links + * so the runner and CMake wiring are in place. The real tests — credential + * validation, boot-mode truth table, reconnect schedule, isolation and the + * no-boot-loop property — arrive with the US1/US2/US3 phases (T008+), + * exercising the pure WifiManager over MockWifiDriver + FakeTimeProvider. + */ + +void run_wifi_tests(void) +{ + // Intentionally empty until the US1+ phases add real cases (T008+). +} From 68b1d759ee6a37f0a23839c415a4d1c7bfacb4af Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 11:00:38 +0200 Subject: [PATCH 3/7] =?UTF-8?q?feat(network):=20PR-07=20US1=20=E2=80=94=20?= =?UTF-8?q?SoftAP=20provisioning=20portal=20+=20credential=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 / User Story 1 of PR-07: - WifiCredentialValidation (pure): SSID 1-32, password empty-or-8..64, typed reasons - decideBootMode (pure): button-held or no-credentials => Provisioning - ProvisioningPortal (target-only): standalone esp_http_server, GET / setup page, POST /wifi/config -> validate -> IConfigStore::setWifiCredentials -> 200 + deferred esp_restart (~3s, injectable RestartHook); never logs credential values - app_main: boot-mode decision + portal start in Provisioning mode (SoftAP radio bring-up left as TODO(US2/T018); config-button read TODO(US3/T024)) Verified: host tests 168/0 (+4 wifi cases), both board targets build + verified. Co-Authored-By: Claude Opus 4.8 (1M context) --- firmware/components/network/CMakeLists.txt | 26 +- .../include/network/ProvisioningPortal.h | 110 +++++++ .../network/include/network/WifiBootMode.h | 51 +++ .../network/WifiCredentialValidation.h | 81 +++++ .../network/src/ProvisioningPortal.cpp | 309 ++++++++++++++++++ firmware/main/CMakeLists.txt | 1 + firmware/main/app_main.cpp | 38 +++ firmware/test_apps/host/main/test_wifi.cpp | 133 +++++++- 8 files changed, 731 insertions(+), 18 deletions(-) create mode 100644 firmware/components/network/include/network/ProvisioningPortal.h create mode 100644 firmware/components/network/include/network/WifiBootMode.h create mode 100644 firmware/components/network/include/network/WifiCredentialValidation.h create mode 100644 firmware/components/network/src/ProvisioningPortal.cpp diff --git a/firmware/components/network/CMakeLists.txt b/firmware/components/network/CMakeLists.txt index cbbc9ae..6cb5ef8 100644 --- a/firmware/components/network/CMakeLists.txt +++ b/firmware/components/network/CMakeLists.txt @@ -8,25 +8,27 @@ # esp_http_server) carry no business logic and are excluded from the linux # build (research.md, same mechanism as the other components). # -# Phase 1/2 scaffolding (tasks T001–T007): no pure sources exist yet — -# WifiManager.cpp and the hardware sources arrive with US1/US2. The -# component therefore registers header-only for now (INCLUDE_DIRS only) and -# still compiles cleanly on both targets. When pure sources land, add them -# to SRCS on BOTH branches; hardware sources go to SRCS on the target -# branch only. +# US1 (feature 007): the pure credential-validation and boot-mode logic is +# header-only (WifiCredentialValidation.h, WifiBootMode.h — no .cpp), so the +# host branch stays INCLUDE_DIRS-only. The provisioning HTTP portal +# (ProvisioningPortal.cpp) is the first hardware touchpoint and is target- +# only. WifiManager.cpp and EspWifiDriver.cpp arrive with US2 — pure sources +# then go to SRCS on BOTH branches, hardware sources on the target branch +# only. if(${IDF_TARGET} STREQUAL "linux") - # Host build: pure sources only (none yet — header-only for now). + # Host build: pure sources only (validation + boot-mode are header-only). idf_component_register( INCLUDE_DIRS "include" REQUIRES interfaces board ) else() - # Target build: pure sources + the hardware touchpoints (none yet). The - # WiFi/HTTP-server IDF deps are private — they appear only in the - # EspWifiDriver / ProvisioningPortal .cpp files, never in this - # component's public headers (same rule as storage's littlefs and - # sensors' esp-modbus). + # Target build: pure sources + the hardware touchpoints. The WiFi/HTTP- + # server IDF deps are private — they appear only in the EspWifiDriver / + # ProvisioningPortal .cpp files, never in this component's public headers + # (same rule as storage's littlefs and sensors' esp-modbus): the portal + # header holds the server handle as an opaque pointer. idf_component_register( + SRCS "src/ProvisioningPortal.cpp" INCLUDE_DIRS "include" REQUIRES interfaces board PRIV_REQUIRES esp_wifi esp_netif esp_event esp_http_server diff --git a/firmware/components/network/include/network/ProvisioningPortal.h b/firmware/components/network/include/network/ProvisioningPortal.h new file mode 100644 index 0000000..bf0523f --- /dev/null +++ b/firmware/components/network/include/network/ProvisioningPortal.h @@ -0,0 +1,110 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file ProvisioningPortal.h + * @brief First-boot SoftAP setup portal (target-only HTTP server). + * + * A minimal standalone esp_http_server that runs while the device is in + * Provisioning (AP) mode (feature 007, US1). It serves a small self-contained + * setup page and accepts a POST that validates + persists WiFi credentials, + * then schedules a restart so the operator's new settings take effect. The + * full JSON status API and /api/wifi/scan are out of scope (PR-09). + * + * This is the only HTTP touchpoint of the `network` component and is excluded + * from the linux build (esp_http_server has no host port); all reusable + * policy (credential validation) lives in the pure WifiCredentialValidation + * header so it stays host-tested. Normative contract: + * specs/007-wifi-provisioning/contracts/provisioning-portal.md. + * + * PRIV rule (same as EspI2cBus / StorageMount): esp_http_server.h appears + * only in the .cpp; the server handle is held as an opaque pointer and the + * HTTP route handlers are file-local in the .cpp — so consumers (app_main) + * that include this header pull in no HTTP-server dependency. Credential + * values are never logged or echoed back (FR-004 / PR-06 convention). + */ + +#ifndef WATERINGSYSTEM_NETWORK_PROVISIONINGPORTAL_H +#define WATERINGSYSTEM_NETWORK_PROVISIONINGPORTAL_H + +#include +#include +#include + +#include "esp_err.h" + +#include "interfaces/IConfigStore.h" + +/** + * @brief Standalone provisioning HTTP server over IConfigStore. + * + * Lifetime: construct once at the wiring site (app_main), call start() when + * entering provisioning mode and keep the instance alive for as long as the + * portal serves. The config store is held by reference and outlives the + * portal (it is an app_main function-local static). + */ +class ProvisioningPortal { +public: + /// Delay between the credential-save HTTP response and the restart, so + /// the response reaches the browser before the device reboots (FR-007). + static constexpr uint32_t kRestartDelayMs = 3000; + + /// Hook invoked to trigger the (deferred) restart after a successful + /// credential save. Kept injectable so esp_restart stays out of the + /// directly-called handler path; the default schedules it kRestartDelayMs + /// later on a short-lived task. + using RestartHook = std::function; + + /** + * @brief Construct the portal over a config store and a restart hook. + * + * @param configStore Credential sink; must outlive the portal. + * @param restartHook Called once on a successful save. When null (the + * default), a built-in scheduled restart is used. + */ + explicit ProvisioningPortal(IConfigStore& configStore, + RestartHook restartHook = nullptr); + + ~ProvisioningPortal(); + + ProvisioningPortal(const ProvisioningPortal&) = delete; + ProvisioningPortal& operator=(const ProvisioningPortal&) = delete; + + /** + * @brief Start the HTTP server and register the routes. + * + * @return ESP_OK on success; an esp_err_t on server start / route + * registration failure (already-started is a successful no-op). + */ + esp_err_t start(); + + /** + * @brief Stop the HTTP server. Idempotent. + */ + void stop(); + + // -- Submission API used by the POST route handler (.cpp) -------------- + // Validation happens in the handler via the pure validateWifiCredentials; + // these two steps stay on the instance so the handler can interleave them + // with the HTTP response (persist, respond 200, then restart). + + /** + * @brief Persist already-validated credentials. + * @return false on a persistence failure (handler responds 5xx and the + * device stays provisionable). + */ + bool persistCredentials(const std::string& ssid, + const std::string& password); + + /** + * @brief Invoke the restart hook (default: schedule a deferred reboot). + * Called after the success response has been sent. + */ + void scheduleRestart(); + +private: + IConfigStore& configStore_; + RestartHook restartHook_; + void* server_ = nullptr; ///< opaque httpd_handle_t (see .cpp) +}; + +#endif /* WATERINGSYSTEM_NETWORK_PROVISIONINGPORTAL_H */ diff --git a/firmware/components/network/include/network/WifiBootMode.h b/firmware/components/network/include/network/WifiBootMode.h new file mode 100644 index 0000000..2d5d595 --- /dev/null +++ b/firmware/components/network/include/network/WifiBootMode.h @@ -0,0 +1,51 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file WifiBootMode.h + * @brief Pure boot-mode decision (host + target). + * + * Decides whether the device comes up in station mode or first-boot/recovery + * provisioning (feature 007). No IDF includes — part of the header-only + * surface of the `network` component, compiled on the linux preview target + * and host-tested against the boot-mode truth table. + * + * Normative contract: specs/007-wifi-provisioning/contracts/ + * wifi-manager-states.md ("Boot-mode contract") and data-model.md. + */ + +#ifndef WATERINGSYSTEM_NETWORK_WIFIBOOTMODE_H +#define WATERINGSYSTEM_NETWORK_WIFIBOOTMODE_H + +/** + * @brief The mode the WiFi subsystem enters at boot. + * + * `Station` joins the stored network; `Provisioning` brings up the SoftAP + * setup portal (first boot, missing credentials, or an operator-forced + * recovery via the config button). + */ +enum class WifiBootMode { Station, Provisioning }; + +/** + * @brief Decide the boot mode from stored-credential presence and the + * config-button hold state at boot. + * + * Rule (truth table, all four rows host-tested): a held config button OR the + * absence of stored credentials forces `Provisioning`; only a configured + * device with the button released comes up in `Station`. On a button-forced + * provisioning the caller clears the stored credentials first (data-model.md + * boot rule) — that side effect lives at the wiring site, not here. + * + * @param credentialsPresent true when a non-empty SSID is stored. + * @param configButtonHeld true when the config button is held at boot. + * @return WifiBootMode::Provisioning or WifiBootMode::Station. + */ +inline WifiBootMode decideBootMode(bool credentialsPresent, + bool configButtonHeld) +{ + if (configButtonHeld || !credentialsPresent) { + return WifiBootMode::Provisioning; + } + return WifiBootMode::Station; +} + +#endif /* WATERINGSYSTEM_NETWORK_WIFIBOOTMODE_H */ diff --git a/firmware/components/network/include/network/WifiCredentialValidation.h b/firmware/components/network/include/network/WifiCredentialValidation.h new file mode 100644 index 0000000..f95e559 --- /dev/null +++ b/firmware/components/network/include/network/WifiCredentialValidation.h @@ -0,0 +1,81 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file WifiCredentialValidation.h + * @brief Pure WiFi credential validation (host + target). + * + * The single source of truth for "is this SSID/password pair acceptable" + * (feature 007). Shared by the provisioning portal (POST /wifi/config) and + * the host test suite. No IDF includes — part of the header-only surface of + * the `network` component, compiled on the linux preview target. + * + * Rules (data-model.md "Validation rules"): + * - SSID must be 1..kWifiSsidMaxLen (32) bytes. + * - Password is either empty (open network) or kWifiPasswordMinLen (8) .. + * kWifiPasswordMaxLen (64) bytes. + * + * The result is a typed reason code so callers can render a specific error + * without re-deriving the rule. Credential VALUES are never logged here. + */ + +#ifndef WATERINGSYSTEM_NETWORK_WIFICREDENTIALVALIDATION_H +#define WATERINGSYSTEM_NETWORK_WIFICREDENTIALVALIDATION_H + +#include +#include + +#include "interfaces/IConfigStore.h" + +/** + * @brief Outcome of validateWifiCredentials(). + * + * `Ok` is the only accepting value; every other value is a specific reject + * reason (data-model.md validation rules). + */ +enum class CredentialCheck { + Ok, ///< SSID 1..32 and password empty-or-8..64 + SsidEmpty, ///< SSID is the empty string (unconfigured input) + SsidTooLong, ///< SSID exceeds kWifiSsidMaxLen (32) bytes + PasswordTooShort, ///< non-empty password below kWifiPasswordMinLen (8) + PasswordTooLong ///< password exceeds kWifiPasswordMaxLen (64) bytes +}; + +/// Minimum length of a non-empty WPA2 passphrase (empty = open network). +inline constexpr std::size_t kWifiPasswordMinLen = 8; + +/** + * @brief Validate a candidate SSID/password pair. + * + * Rejection order (first failing rule wins): empty SSID, over-long SSID, + * too-short non-empty password, over-long password. An empty password is + * accepted (open network). + * + * @param ssid Candidate network name. + * @param password Candidate passphrase (empty for an open network). + * @return CredentialCheck::Ok when acceptable, otherwise the reject reason. + */ +inline CredentialCheck validateWifiCredentials(const std::string& ssid, + const std::string& password) +{ + if (ssid.empty()) { + return CredentialCheck::SsidEmpty; + } + if (ssid.size() > IConfigStore::kWifiSsidMaxLen) { + return CredentialCheck::SsidTooLong; + } + if (!password.empty() && password.size() < kWifiPasswordMinLen) { + return CredentialCheck::PasswordTooShort; + } + if (password.size() > IConfigStore::kWifiPasswordMaxLen) { + return CredentialCheck::PasswordTooLong; + } + return CredentialCheck::Ok; +} + +/// Convenience predicate: true only for CredentialCheck::Ok. +inline bool isValid(CredentialCheck check) +{ + return check == CredentialCheck::Ok; +} + +#endif /* WATERINGSYSTEM_NETWORK_WIFICREDENTIALVALIDATION_H */ diff --git a/firmware/components/network/src/ProvisioningPortal.cpp b/firmware/components/network/src/ProvisioningPortal.cpp new file mode 100644 index 0000000..886f3f7 --- /dev/null +++ b/firmware/components/network/src/ProvisioningPortal.cpp @@ -0,0 +1,309 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file ProvisioningPortal.cpp + * @brief First-boot SoftAP setup portal implementation (target-only). + * + * See ProvisioningPortal.h and specs/007-wifi-provisioning/contracts/ + * provisioning-portal.md. Excluded from the linux build (esp_http_server has + * no host port). The only reusable policy — credential validation — is the + * pure, host-tested validateWifiCredentials(); this file holds the HTTP + * plumbing (file-local route handlers, urlencoded form parsing) and the + * post-save restart scheduling. The server handle is the only IDF type that + * touches this class and is kept opaque in the header (PRIV rule). Credential + * VALUES are never logged or echoed (FR-004). + */ + +#include "network/ProvisioningPortal.h" + +#include +#include +#include +#include + +#include "esp_http_server.h" +#include "esp_log.h" +#include "esp_system.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" + +#include "network/WifiCredentialValidation.h" + +namespace { + +const char* TAG = "prov_portal"; + +/// Cap on the accepted POST body: two short fields plus urlencoding overhead. +/// Larger bodies are rejected rather than buffered (the fields are bounded by +/// the SSID/password max lengths, PR-06). +constexpr std::size_t kMaxFormBodyLen = 512; + +/// Priority for the short-lived restart task (just above idle; it only +/// sleeps then reboots). +constexpr UBaseType_t kRestartTaskPriority = tskIDLE_PRIORITY + 1; + +// -- Setup / result pages ---------------------------------------------------- +// Small, self-contained (no external assets) to keep the app within the OTA +// slot budget (research R2). English only. + +const char* kSetupPage = + "" + "" + "WateringSystem Setup" + "

WateringSystem WiFi Setup

" + "

Enter the credentials of the WiFi network the device should join.

" + "
" + "

" + "

" + "

" + "
"; + +const char* kSuccessPage = + "" + "" + "Saved" + "

Credentials saved

" + "

The device will restart shortly to join the network. You can close " + "this page.

" + ""; + +/// Decode one application/x-www-form-urlencoded value ('+' -> space, %XX -> +/// byte). Malformed trailing escapes are copied literally. +std::string urlDecode(const std::string& in) +{ + std::string out; + out.reserve(in.size()); + for (std::size_t i = 0; i < in.size(); ++i) { + const char c = in[i]; + if (c == '+') { + out.push_back(' '); + } else if (c == '%' && i + 2 < in.size() && + std::isxdigit(static_cast(in[i + 1])) && + std::isxdigit(static_cast(in[i + 2]))) { + const std::string hex = in.substr(i + 1, 2); + out.push_back(static_cast(std::stoi(hex, nullptr, 16))); + i += 2; + } else { + out.push_back(c); + } + } + return out; +} + +/// Extract the urldecoded value of `key` from an urlencoded form body. +/// Returns true when the key is present (its value may be empty). +bool formField(const std::string& body, const std::string& key, + std::string& out) +{ + const std::string needle = key + "="; + std::size_t pos = 0; + while (pos < body.size()) { + // A field starts at the beginning or right after a '&'. + if (body.compare(pos, needle.size(), needle) == 0) { + const std::size_t valueStart = pos + needle.size(); + const std::size_t amp = body.find('&', valueStart); + const std::size_t end = + (amp == std::string::npos) ? body.size() : amp; + out = urlDecode(body.substr(valueStart, end - valueStart)); + return true; + } + const std::size_t amp = body.find('&', pos); + if (amp == std::string::npos) { + break; + } + pos = amp + 1; + } + return false; +} + +/// Short-lived task: wait kRestartDelayMs, then reboot. +[[noreturn]] void restartTask(void* /*arg*/) +{ + vTaskDelay(pdMS_TO_TICKS(ProvisioningPortal::kRestartDelayMs)); + ESP_LOGI(TAG, "restarting to apply new WiFi credentials"); + esp_restart(); + for (;;) { + // esp_restart() never returns; guard against a hypothetical return. + vTaskDelay(portMAX_DELAY); + } +} + +/// Default restart hook: schedule the deferred reboot on a short-lived task so +/// the HTTP success response is delivered first (esp_restart stays off the +/// handler's direct call path). +void scheduleDeferredRestart() +{ + if (xTaskCreate(restartTask, "prov_restart", 2048, nullptr, + kRestartTaskPriority, nullptr) != pdPASS) { + // The task could not be created: fall back to an immediate restart. + // The credentials are already persisted, so a reboot is still the + // correct outcome even without the response-delivery delay. + ESP_LOGW(TAG, "restart task create failed; restarting now"); + esp_restart(); + } +} + +/// Send a small text/html response with an explicit status line. +esp_err_t sendHtml(httpd_req_t* req, const char* status, const char* body) +{ + httpd_resp_set_status(req, status); + httpd_resp_set_type(req, "text/html"); + return httpd_resp_sendstr(req, body); +} + +// -- Route handlers (file-local; recover the portal via req->user_ctx) ------- + +esp_err_t rootGetHandler(httpd_req_t* req) +{ + // GET / (and, via wildcard matching, any unknown path) serves the setup + // page (contracts/provisioning-portal.md). + return sendHtml(req, "200 OK", kSetupPage); +} + +esp_err_t wifiConfigPostHandler(httpd_req_t* req) +{ + auto* portal = static_cast(req->user_ctx); + if (portal == nullptr) { + return sendHtml(req, "500 Internal Server Error", + "Server misconfigured."); + } + + if (req->content_len == 0 || req->content_len > kMaxFormBodyLen) { + ESP_LOGW(TAG, "rejecting form: body length %d out of range", + static_cast(req->content_len)); + return sendHtml(req, "400 Bad Request", "Invalid form submission."); + } + + std::string body(req->content_len, '\0'); + std::size_t received = 0; + while (received < req->content_len) { + const int r = httpd_req_recv(req, &body[received], + req->content_len - received); + if (r <= 0) { + ESP_LOGW(TAG, "form body receive failed (%d)", r); + return sendHtml(req, "400 Bad Request", + "Could not read the form."); + } + received += static_cast(r); + } + + std::string ssid; + std::string password; + formField(body, "ssid", ssid); + formField(body, "password", password); // absent = open network (empty) + + // Validate with the same pure rule the host tests cover. Never log the + // values — only the typed reason code. + const CredentialCheck check = validateWifiCredentials(ssid, password); + if (!isValid(check)) { + ESP_LOGW(TAG, "credential submission rejected (reason %d)", + static_cast(check)); + return sendHtml(req, "400 Bad Request", + "Invalid credentials: the network name must be " + "1-32 characters and the password must be empty or " + "8-64 characters."); + } + + if (!portal->persistCredentials(ssid, password)) { + ESP_LOGE(TAG, "credential persist failed"); + return sendHtml(req, "500 Internal Server Error", + "Could not save the credentials. Please try again."); + } + + // Deliver the success page, THEN schedule the deferred restart so the + // response reaches the browser before the reboot (FR-007). + const esp_err_t sendErr = sendHtml(req, "200 OK", kSuccessPage); + portal->scheduleRestart(); + return sendErr; +} + +} // namespace + +ProvisioningPortal::ProvisioningPortal(IConfigStore& configStore, + RestartHook restartHook) + : configStore_(configStore), + restartHook_(restartHook ? std::move(restartHook) + : RestartHook(&scheduleDeferredRestart)) +{ +} + +ProvisioningPortal::~ProvisioningPortal() +{ + stop(); +} + +bool ProvisioningPortal::persistCredentials(const std::string& ssid, + const std::string& password) +{ + return configStore_.setWifiCredentials(ssid, password); +} + +void ProvisioningPortal::scheduleRestart() +{ + if (restartHook_) { + restartHook_(); + } +} + +esp_err_t ProvisioningPortal::start() +{ + if (server_ != nullptr) { + return ESP_OK; // already started (idempotent) + } + + httpd_handle_t server = nullptr; + httpd_config_t config = HTTPD_DEFAULT_CONFIG(); + // Wildcard matching so GET on any unknown path falls through to the setup + // page (contracts/provisioning-portal.md). + config.uri_match_fn = httpd_uri_match_wildcard; + + esp_err_t err = httpd_start(&server, &config); + if (err != ESP_OK) { + ESP_LOGE(TAG, "httpd_start failed: %s", esp_err_to_name(err)); + return err; + } + + const httpd_uri_t postConfig = { + .uri = "/wifi/config", + .method = HTTP_POST, + .handler = wifiConfigPostHandler, + .user_ctx = this, + }; + err = httpd_register_uri_handler(server, &postConfig); + if (err != ESP_OK) { + ESP_LOGE(TAG, "register POST /wifi/config failed: %s", + esp_err_to_name(err)); + httpd_stop(server); + return err; + } + + // Registered after the specific POST route above; the GET wildcard only + // matches GET requests, so it never shadows POST /wifi/config. + const httpd_uri_t getRoot = { + .uri = "/*", + .method = HTTP_GET, + .handler = rootGetHandler, + .user_ctx = this, + }; + err = httpd_register_uri_handler(server, &getRoot); + if (err != ESP_OK) { + ESP_LOGE(TAG, "register GET /* failed: %s", esp_err_to_name(err)); + httpd_stop(server); + return err; + } + + server_ = server; + ESP_LOGI(TAG, "provisioning portal started"); + return ESP_OK; +} + +void ProvisioningPortal::stop() +{ + if (server_ != nullptr) { + httpd_stop(static_cast(server_)); + server_ = nullptr; + ESP_LOGI(TAG, "provisioning portal stopped"); + } +} diff --git a/firmware/main/CMakeLists.txt b/firmware/main/CMakeLists.txt index 738d0d7..d408d5f 100644 --- a/firmware/main/CMakeLists.txt +++ b/firmware/main/CMakeLists.txt @@ -3,6 +3,7 @@ idf_component_register( PRIV_REQUIRES board esp_driver_gpio esp_app_format actuators interfaces console esp_timer storage nvs_flash sensors esp_netif esp_event + network ) # Build-time littlefs image of the committed seed directory diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index f22b93b..2618386 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -50,6 +50,8 @@ #include "sensors/Ina226Sensor.h" #include "sensors/LockedPowerSensor.h" #endif +#include "network/ProvisioningPortal.h" +#include "network/WifiBootMode.h" #include "storage/LittleFsDataStorage.h" #include "storage/LockedConfigStore.h" #include "storage/LockedDataStorage.h" @@ -230,6 +232,42 @@ extern "C" void app_main(void) static_cast(stats.usedBytes / 1024), static_cast(stats.totalBytes / 1024)); + // WiFi boot-mode decision (feature 007, US1). A missing stored SSID (the + // factory/unconfigured state) OR a held config button forces first-boot/ + // recovery provisioning; a configured device otherwise comes up in + // station mode. The config-button read is US3/T024 — until then the + // button is reported as released. Credential VALUES are never logged: we + // only test whether an SSID is present. WiFi never touches the watering + // path (FR-014); everything below stays after the pump fail-safe. + const bool wifi_credentials_present = !config.getWifiSsid().empty(); + const bool config_button_held = false; // TODO(US3/T024): read BOARD_PIN_BTN_CONFIG at boot + const WifiBootMode wifi_boot_mode = + decideBootMode(wifi_credentials_present, config_button_held); + + if (wifi_boot_mode == WifiBootMode::Provisioning) { + ESP_LOGI(TAG, "WiFi: provisioning mode (SoftAP setup portal)"); + // TODO(US2/T018): bring up the SoftAP radio via EspWifiDriver::apStart + // using CONFIG_WS_PROV_AP_SSID / CONFIG_WS_PROV_AP_PASSWORD (WPA2). + // EspWifiDriver arrives with US2; until the radio is up the portal + // is started here but not yet reachable over the air. The portal + // only depends on the config store, so it is wired now. + // Function-local static (boot fail-safe rule: no non-trivial + // static/global constructors). Constructed only on this branch, kept + // alive for the program lifetime so it keeps serving. + static ProvisioningPortal provisioning_portal(config); + const esp_err_t prov_err = provisioning_portal.start(); + if (prov_err != ESP_OK) { + ESP_LOGE(TAG, "provisioning portal failed to start: %s", + esp_err_to_name(prov_err)); + } + } else { + ESP_LOGI(TAG, "WiFi: station mode (stored credentials present)"); + // TODO(US2/T020): construct EspWifiDriver + WifiManager, + // begin(WifiBootMode::Station) and wifi_task_start(manager). The + // station connect/reconnect path is deferred to US2; no STA connect + // is issued in this PR. + } + // RS485 Modbus soil sensor (feature 004). Not safety-critical: a failed // client init is logged and the system keeps running — the sensor layer // reports invalid data and recovers on later attempts (US2 semantics). diff --git a/firmware/test_apps/host/main/test_wifi.cpp b/firmware/test_apps/host/main/test_wifi.cpp index e40f460..63c8cee 100644 --- a/firmware/test_apps/host/main/test_wifi.cpp +++ b/firmware/test_apps/host/main/test_wifi.cpp @@ -5,14 +5,135 @@ * @brief Unity host suite for WiFi provisioning & station management * (feature 007). * - * Phase 1/2 scaffolding (task T003): the suite entry point exists and links - * so the runner and CMake wiring are in place. The real tests — credential - * validation, boot-mode truth table, reconnect schedule, isolation and the - * no-boot-loop property — arrive with the US1/US2/US3 phases (T008+), - * exercising the pure WifiManager over MockWifiDriver + FakeTimeProvider. + * US1 (tasks T008/T009): the pure credential validation rules and the + * boot-mode truth table, the two host-testable pieces of the provisioning + * MVP. Normative sources: specs/007-wifi-provisioning/data-model.md + * ("Validation rules") and contracts/wifi-manager-states.md ("Boot-mode + * contract"). The reconnect schedule, isolation and no-boot-loop tests + * arrive with US2/US3 (T015+), exercising the pure WifiManager over + * MockWifiDriver + FakeTimeProvider. + * + * Scoped-enum comparisons are wrapped in static_cast (project Unity + * convention, see test_soil_sensor.cpp). */ +#include + +#include "unity.h" + +#include "network/WifiBootMode.h" +#include "network/WifiCredentialValidation.h" + +namespace { + +/// static_cast a CredentialCheck for Unity's integer comparison. +int checkInt(CredentialCheck c) { return static_cast(c); } + +/// static_cast a WifiBootMode for Unity's integer comparison. +int modeInt(WifiBootMode m) { return static_cast(m); } + +} // namespace + +// --------------------------------------------------------------------------- +// T008 — validateWifiCredentials: SSID length rules +// (empty→reject, 1..32→accept, >32→reject; data-model.md validation rules) +// --------------------------------------------------------------------------- +static void test_validate_ssid_length_rules(void) +{ + // Empty SSID is rejected regardless of the password (checked first). + TEST_ASSERT_EQUAL(checkInt(CredentialCheck::SsidEmpty), + checkInt(validateWifiCredentials("", ""))); + + // A 1-char SSID with an (accepted) empty password is Ok. + TEST_ASSERT_EQUAL(checkInt(CredentialCheck::Ok), + checkInt(validateWifiCredentials("a", ""))); + + // The 32-byte upper bound is accepted. + TEST_ASSERT_EQUAL(checkInt(CredentialCheck::Ok), + checkInt(validateWifiCredentials(std::string(32, 's'), ""))); + + // 33 bytes is one over the bound → SsidTooLong. + TEST_ASSERT_EQUAL(checkInt(CredentialCheck::SsidTooLong), + checkInt(validateWifiCredentials(std::string(33, 's'), ""))); +} + +// --------------------------------------------------------------------------- +// T008 — validateWifiCredentials: password length rules +// (empty→accept, 1..7→reject, 8→accept, 64→accept, 65→reject) +// --------------------------------------------------------------------------- +static void test_validate_password_length_rules(void) +{ + const std::string ssid = "net"; + + // Empty password = open network → accepted. + TEST_ASSERT_EQUAL(checkInt(CredentialCheck::Ok), + checkInt(validateWifiCredentials(ssid, ""))); + + // Every non-empty length 1..7 is too short for WPA2. + for (std::size_t len = 1; len <= 7; ++len) { + TEST_ASSERT_EQUAL( + checkInt(CredentialCheck::PasswordTooShort), + checkInt(validateWifiCredentials(ssid, std::string(len, 'p')))); + } + + // 8 bytes is the minimum accepted passphrase. + TEST_ASSERT_EQUAL( + checkInt(CredentialCheck::Ok), + checkInt(validateWifiCredentials(ssid, std::string(8, 'p')))); + + // 64 bytes is the upper bound and accepted. + TEST_ASSERT_EQUAL( + checkInt(CredentialCheck::Ok), + checkInt(validateWifiCredentials(ssid, std::string(64, 'p')))); + + // 65 bytes is one over the bound → PasswordTooLong. + TEST_ASSERT_EQUAL( + checkInt(CredentialCheck::PasswordTooLong), + checkInt(validateWifiCredentials(ssid, std::string(65, 'p')))); +} + +// --------------------------------------------------------------------------- +// T008 — isValid() convenience predicate agrees with the reason codes +// --------------------------------------------------------------------------- +static void test_validate_isvalid_predicate(void) +{ + TEST_ASSERT_TRUE(isValid(validateWifiCredentials("net", "password1"))); + TEST_ASSERT_TRUE(isValid(CredentialCheck::Ok)); + TEST_ASSERT_FALSE(isValid(CredentialCheck::SsidEmpty)); + TEST_ASSERT_FALSE(isValid(CredentialCheck::SsidTooLong)); + TEST_ASSERT_FALSE(isValid(CredentialCheck::PasswordTooShort)); + TEST_ASSERT_FALSE(isValid(CredentialCheck::PasswordTooLong)); +} + +// --------------------------------------------------------------------------- +// T009 — decideBootMode: all four rows of the truth table +// (contract wifi-manager-states.md "Boot-mode contract") +// --------------------------------------------------------------------------- +static void test_boot_mode_truth_table(void) +{ + // credentialsPresent=false, buttonHeld=false → Provisioning. + TEST_ASSERT_EQUAL(modeInt(WifiBootMode::Provisioning), + modeInt(decideBootMode(false, false))); + + // credentialsPresent=false, buttonHeld=true → Provisioning. + TEST_ASSERT_EQUAL(modeInt(WifiBootMode::Provisioning), + modeInt(decideBootMode(false, true))); + + // credentialsPresent=true, buttonHeld=false → Station. + TEST_ASSERT_EQUAL(modeInt(WifiBootMode::Station), + modeInt(decideBootMode(true, false))); + + // credentialsPresent=true, buttonHeld=true → Provisioning (emergency). + TEST_ASSERT_EQUAL(modeInt(WifiBootMode::Provisioning), + modeInt(decideBootMode(true, true))); +} + void run_wifi_tests(void) { - // Intentionally empty until the US1+ phases add real cases (T008+). + // T008 — credential validation. + RUN_TEST(test_validate_ssid_length_rules); + RUN_TEST(test_validate_password_length_rules); + RUN_TEST(test_validate_isvalid_predicate); + // T009 — boot-mode truth table. + RUN_TEST(test_boot_mode_truth_table); } From c3182cc345b5980f49942b2cf460dccf8debff8a Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 11:09:32 +0200 Subject: [PATCH 4/7] =?UTF-8?q?feat(network):=20PR-07=20US2=20core=20?= =?UTF-8?q?=E2=80=94=20pure=20WifiManager=20reconnect=20state=20machine?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 / US2 (host-testable core): - WifiManager (pure, no IDF): STA connect + fixed-interval reconnect (10s retry, +60s pause after 5 fails, 5s rssi monitor), Connecting/Connected/Reconnecting/ ReconnectPaused; GotIp=connected. Constructor injects only IWifiDriver/ IConfigStore/ITimeProvider/ReconnectPolicy (FR-014: no watering dependency). Overflow-guarded failure counter; never reboots (FR-013 no boot loop). - 6 host tests (MockWifiDriver + FakeTimeProvider): happy path, 10s retry boundary, 60s pause after 5 fails, disconnect->reconnect, AP-mode suspends, no-block isolation. Hardware wiring (EspWifiDriver, wifi_task, app_main STA, LED) is the next mission. Verified: host tests 174/0, both board targets build + verified. Co-Authored-By: Claude Opus 4.8 (1M context) --- firmware/components/network/CMakeLists.txt | 18 +- .../network/include/network/WifiManager.h | 114 ++++++++ .../components/network/src/WifiManager.cpp | 159 +++++++++++ firmware/test_apps/host/main/test_wifi.cpp | 263 ++++++++++++++++++ 4 files changed, 546 insertions(+), 8 deletions(-) create mode 100644 firmware/components/network/include/network/WifiManager.h create mode 100644 firmware/components/network/src/WifiManager.cpp diff --git a/firmware/components/network/CMakeLists.txt b/firmware/components/network/CMakeLists.txt index 6cb5ef8..d8e88f0 100644 --- a/firmware/components/network/CMakeLists.txt +++ b/firmware/components/network/CMakeLists.txt @@ -9,15 +9,16 @@ # build (research.md, same mechanism as the other components). # # US1 (feature 007): the pure credential-validation and boot-mode logic is -# header-only (WifiCredentialValidation.h, WifiBootMode.h — no .cpp), so the -# host branch stays INCLUDE_DIRS-only. The provisioning HTTP portal -# (ProvisioningPortal.cpp) is the first hardware touchpoint and is target- -# only. WifiManager.cpp and EspWifiDriver.cpp arrive with US2 — pure sources -# then go to SRCS on BOTH branches, hardware sources on the target branch -# only. +# header-only (WifiCredentialValidation.h, WifiBootMode.h — no .cpp). US2 adds +# the pure WifiManager state machine (WifiManager.cpp) — a pure source that +# goes to SRCS on BOTH branches (host tests + target). The provisioning HTTP +# portal (ProvisioningPortal.cpp) and EspWifiDriver.cpp are hardware +# touchpoints and stay target-only. if(${IDF_TARGET} STREQUAL "linux") - # Host build: pure sources only (validation + boot-mode are header-only). + # Host build: pure sources only (validation + boot-mode are header-only; + # WifiManager is the pure state machine, host-tested over MockWifiDriver). idf_component_register( + SRCS "src/WifiManager.cpp" INCLUDE_DIRS "include" REQUIRES interfaces board ) @@ -28,7 +29,8 @@ else() # (same rule as storage's littlefs and sensors' esp-modbus): the portal # header holds the server handle as an opaque pointer. idf_component_register( - SRCS "src/ProvisioningPortal.cpp" + SRCS "src/WifiManager.cpp" + "src/ProvisioningPortal.cpp" INCLUDE_DIRS "include" REQUIRES interfaces board PRIV_REQUIRES esp_wifi esp_netif esp_event esp_http_server diff --git a/firmware/components/network/include/network/WifiManager.h b/firmware/components/network/include/network/WifiManager.h new file mode 100644 index 0000000..f2d3c41 --- /dev/null +++ b/firmware/components/network/include/network/WifiManager.h @@ -0,0 +1,114 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file WifiManager.h + * @brief Pure WiFi station state machine + reconnect scheduler (host + target). + * + * All timing and state-machine logic for feature 007 lives here, above the + * IWifiDriver seam: it never touches esp_wifi/esp_netif/esp_event and holds + * NO reference to any watering/pump/sensor object (FR-014, Constitution I) — + * the constructor injects only IWifiDriver, IConfigStore, ITimeProvider and a + * ReconnectPolicy. That isolation is enforced structurally by this signature + * and asserted in the host tests. Compiles on the linux preview target and is + * unit-tested against MockWifiDriver + FakeTimeProvider + MockConfigStore. + * + * tick() is strictly non-blocking: it advances purely from + * ITimeProvider::nowMs() deltas and drained IWifiDriver::pollEvent()s, never + * sleeps or waits, and never triggers a restart/reboot (FR-013, no boot loop). + * + * Normative contract: + * specs/007-wifi-provisioning/contracts/wifi-manager-states.md and + * data-model.md. Part of the header-only public surface of the `network` + * component: no IDF includes. + */ + +#ifndef WATERINGSYSTEM_NETWORK_WIFIMANAGER_H +#define WATERINGSYSTEM_NETWORK_WIFIMANAGER_H + +#include + +#include "interfaces/IConfigStore.h" +#include "interfaces/ITimeProvider.h" +#include "interfaces/IWifiDriver.h" +#include "network/WifiBootMode.h" +#include "network/WifiState.h" + +/** + * @brief Station-mode connection lifecycle + reconnect scheduler. + * + * Drive it by calling begin() once at boot and tick() at a fixed cadence from + * the wifi task (T019). All external effects go through the injected + * IWifiDriver; time comes from ITimeProvider; credentials are read from + * IConfigStore. Unsynchronized by design — cross-task readers consume the + * immutable snapshot() (single acquisition at the wiring site). + */ +class WifiManager { +public: + /** + * @brief Inject the hardware seam, config store, clock and reconnect policy. + * + * @param driver WiFi control + event queue seam. + * @param config Credential source (SSID/password) for STA attempts. + * @param time Monotonic clock; all cadences are nowMs() deltas. + * @param policy Retry/pause/monitor timing (defaults = parity constants). + */ + WifiManager(IWifiDriver& driver, IConfigStore& config, ITimeProvider& time, + ReconnectPolicy policy = {}) + : driver_(driver), config_(config), time_(time), policy_(policy) + { + } + + /** + * @brief Enter the boot mode decided by decideBootMode(). + * + * Station: read the stored credentials, issue the first staConnect + * (attempt #1) and enter Connecting. Provisioning: enter Provisioning and + * make NO driver calls — the AP radio and portal are brought up at the + * wiring site (T018), and tick() is a no-op in this mode. + */ + void begin(WifiBootMode mode); + + /** + * @brief Advance the state machine once (non-blocking). + * + * Drains every queued IWifiDriver event, then applies time-based + * transitions (retry cadence, pause release, connected-health monitor) + * from nowMs() deltas. No-op while Provisioning. Never sleeps or reboots. + */ + void tick(); + + /** + * @brief Consistent copy of state + counters + rssi for status/LED + * consumers (single acquisition at the wiring site). + */ + WifiConnectionSnapshot snapshot() const; + +private: + /// Read credentials and issue a staConnect, entering Connecting. + void startConnect(); + + /// Apply one drained lifecycle event to the state machine. + void handleEvent(WifiEvent event); + + /// Handle a ConnectFailed/Disconnected event (schedule retry or pause). + void handleFailure(int64_t now); + + IWifiDriver& driver_; + IConfigStore& config_; + ITimeProvider& time_; + ReconnectPolicy policy_; + + WifiState state_ = WifiState::Connecting; + int8_t rssi_ = 0; + uint8_t consecutiveFailures_ = 0; + uint32_t disconnectCount_ = 0; + bool ipAcquired_ = false; + + /// Deadline for the next STA attempt (Reconnecting) or pause release + /// (ReconnectPaused), an absolute nowMs() value. + int64_t nextAttemptMs_ = 0; + /// Last time the connected-health monitor refreshed rssi (Connected). + int64_t lastMonitorMs_ = 0; +}; + +#endif /* WATERINGSYSTEM_NETWORK_WIFIMANAGER_H */ diff --git a/firmware/components/network/src/WifiManager.cpp b/firmware/components/network/src/WifiManager.cpp new file mode 100644 index 0000000..018915b --- /dev/null +++ b/firmware/components/network/src/WifiManager.cpp @@ -0,0 +1,159 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file WifiManager.cpp + * @brief Pure WifiManager state machine (host + target; see WifiManager.h). + * + * No IDF/FreeRTOS includes and no logging — logging belongs to the wifi-task + * wrapper (T019), keeping this translation unit host-buildable and the + * FR-014 isolation intact. + */ + +#include "network/WifiManager.h" + +#include +#include + +void WifiManager::begin(WifiBootMode mode) +{ + ipAcquired_ = false; + disconnectCount_ = 0; + consecutiveFailures_ = 0; + rssi_ = 0; + + if (mode == WifiBootMode::Provisioning) { + // SoftAP + portal are brought up at the wiring site (T018); here we + // only record the mode so tick() suspends all STA monitoring. + state_ = WifiState::Provisioning; + return; + } + + // Station: issue the first STA attempt (attempt #1) and wait for events. + startConnect(); +} + +void WifiManager::tick() +{ + // AP/provisioning mode suspends STA monitoring and reconnect entirely, + // regardless of elapsed time (contract §6). + if (state_ == WifiState::Provisioning) { + return; + } + + // Drain the entire event queue this tick. The loop terminates as soon as + // the driver reports None, so a "hung" driver (never any events) simply + // does no work — tick() never blocks (FR-014, contract §7). + for (WifiEvent event = driver_.pollEvent(); event != WifiEvent::None; + event = driver_.pollEvent()) { + handleEvent(event); + } + + const int64_t now = time_.nowMs(); + + switch (state_) { + case WifiState::Reconnecting: + // Fixed retry cadence: re-attempt only once the interval elapsed. + // consecutiveFailures is NOT reset here — it keeps climbing toward + // the pause threshold across the round. + if (now >= nextAttemptMs_) { + startConnect(); + } + break; + + case WifiState::ReconnectPaused: + // After the long pause, begin a fresh round with failures reset. + if (now >= nextAttemptMs_) { + consecutiveFailures_ = 0; + startConnect(); + } + break; + + case WifiState::Connected: + // Health monitor: refresh rssi on the fixed cadence. Link loss + // itself arrives as a Disconnected event, so no active poll is + // needed to detect it here. + if (now - lastMonitorMs_ >= + static_cast(policy_.monitorIntervalMs)) { + rssi_ = driver_.rssi(); + lastMonitorMs_ = now; + } + break; + + case WifiState::Connecting: + case WifiState::Provisioning: + // Connecting waits for an event; Provisioning handled above. + break; + } +} + +WifiConnectionSnapshot WifiManager::snapshot() const +{ + return WifiConnectionSnapshot{state_, rssi_, consecutiveFailures_, + disconnectCount_, ipAcquired_}; +} + +void WifiManager::startConnect() +{ + const std::string ssid = config_.getWifiSsid(); + const std::string password = config_.getWifiPassword(); + // Non-blocking: success/failure of the attempt arrives later as an event. + // A synchronous config error (return false) leaves us in Connecting; the + // machine never reboots, it just waits — the operator can re-provision. + driver_.staConnect(ssid, password); + state_ = WifiState::Connecting; +} + +void WifiManager::handleEvent(WifiEvent event) +{ + const int64_t now = time_.nowMs(); + + switch (event) { + case WifiEvent::GotIp: + // DHCP lease acquired: fully connected. Reset the failure count + // and prime the health monitor with a fresh rssi reading. + state_ = WifiState::Connected; + consecutiveFailures_ = 0; + ipAcquired_ = true; + rssi_ = driver_.rssi(); + lastMonitorMs_ = now; + break; + + case WifiEvent::Connected: + // L2 association only — stay Connecting until GotIp confirms a + // usable link. + break; + + case WifiEvent::Disconnected: + case WifiEvent::ConnectFailed: + handleFailure(now); + break; + + case WifiEvent::None: + // Unreachable: the drain loop stops on None. + break; + } +} + +void WifiManager::handleFailure(int64_t now) +{ + // A drop from an established connection is a diagnostic disconnect; + // failures that occur while merely (re)connecting are not counted here. + if (state_ == WifiState::Connected || ipAcquired_) { + ++disconnectCount_; + ipAcquired_ = false; + } + + if (consecutiveFailures_ < UINT8_MAX) { + ++consecutiveFailures_; + } + + if (consecutiveFailures_ >= policy_.failuresBeforePause) { + // Threshold reached: pause a full round before retrying, but never + // reboot (FR-013 — the machine only cycles Reconnecting↔Paused). + state_ = WifiState::ReconnectPaused; + nextAttemptMs_ = now + static_cast(policy_.pauseMs); + } else { + state_ = WifiState::Reconnecting; + nextAttemptMs_ = now + static_cast(policy_.retryIntervalMs); + } +} diff --git a/firmware/test_apps/host/main/test_wifi.cpp b/firmware/test_apps/host/main/test_wifi.cpp index 63c8cee..976f625 100644 --- a/firmware/test_apps/host/main/test_wifi.cpp +++ b/firmware/test_apps/host/main/test_wifi.cpp @@ -21,8 +21,13 @@ #include "unity.h" +#include "actuators/testing/FakeTimeProvider.h" #include "network/WifiBootMode.h" #include "network/WifiCredentialValidation.h" +#include "network/WifiManager.h" +#include "network/WifiState.h" +#include "network/testing/MockWifiDriver.h" +#include "storage/testing/MockConfigStore.h" namespace { @@ -32,6 +37,9 @@ int checkInt(CredentialCheck c) { return static_cast(c); } /// static_cast a WifiBootMode for Unity's integer comparison. int modeInt(WifiBootMode m) { return static_cast(m); } +/// static_cast a WifiState for Unity's integer comparison. +int stateInt(WifiState s) { return static_cast(s); } + } // namespace // --------------------------------------------------------------------------- @@ -128,6 +136,253 @@ static void test_boot_mode_truth_table(void) modeInt(decideBootMode(true, true))); } +// =========================================================================== +// US2 — pure WifiManager over MockWifiDriver + FakeTimeProvider (T015/T016). +// +// FR-014 isolation, documented and structurally enforced: WifiManager's +// constructor takes ONLY (IWifiDriver&, IConfigStore&, ITimeProvider&, +// ReconnectPolicy) — no watering/pump/sensor type. Every construction below +// is that exact four-argument signature; the type does not compile with a +// watering reference (there is no such parameter to pass one to). +// =========================================================================== + +namespace { + +/// Seed a MockConfigStore with a valid SSID/password pair for STA attempts. +void seedCredentials(MockConfigStore& config) +{ + config.setWifiCredentials("net", "password1"); +} + +} // namespace + +// --------------------------------------------------------------------------- +// T015 #1 — connect happy path + connected-health rssi monitor cadence. +// (contract wifi-manager-states.md §Timing 1 and §Timing 4 rssi refresh) +// --------------------------------------------------------------------------- +static void test_wifi_connect_happy_path(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + WifiManager manager(driver, config, clock, ReconnectPolicy{}); + + // begin(Station) issues exactly one STA attempt (#1) with the stored creds. + manager.begin(WifiBootMode::Station); + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); + TEST_ASSERT_EQUAL_STRING("net", driver.lastStaSsid.c_str()); + TEST_ASSERT_EQUAL_STRING("password1", driver.lastStaPassword.c_str()); + + // Connected (L2) then GotIp → fully connected, failures reset, ip acquired. + driver.setRssi(-50); + driver.scriptConnectSuccess(); + manager.tick(); + + WifiConnectionSnapshot snap = manager.snapshot(); + TEST_ASSERT_EQUAL(stateInt(WifiState::Connected), stateInt(snap.state)); + TEST_ASSERT_EQUAL_UINT8(0, snap.consecutiveFailures); + TEST_ASSERT_TRUE(snap.ipAcquired); + TEST_ASSERT_EQUAL_INT8(-50, snap.rssi); // primed on GotIp + + // Health monitor refreshes rssi only once monitorIntervalMs (5 s) elapses. + driver.setRssi(-60); + clock.advance(4999); + manager.tick(); + TEST_ASSERT_EQUAL_INT8(-50, manager.snapshot().rssi); // not yet + clock.advance(1); // exactly 5000 ms since GotIp + manager.tick(); + TEST_ASSERT_EQUAL_INT8(-60, manager.snapshot().rssi); // refreshed + // No spurious STA attempts while connected. + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); +} + +// --------------------------------------------------------------------------- +// T015 #2 — retry only at 10 s: no attempt at 9999 ms, one at 10000 ms. +// (contract §Timing 2) +// --------------------------------------------------------------------------- +static void test_wifi_retry_only_at_10s(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + WifiManager manager(driver, config, clock, ReconnectPolicy{}); + manager.begin(WifiBootMode::Station); // attempt #1 + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); + + // First failure → Reconnecting; the retry is scheduled 10 s out, so the + // same tick does NOT re-attempt. + driver.scriptConnectFailure(); + manager.tick(); + TEST_ASSERT_EQUAL(stateInt(WifiState::Reconnecting), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT8(1, manager.snapshot().consecutiveFailures); + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); + + // 1 ms short of the interval: still no new attempt. + clock.advance(9999); + manager.tick(); + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); + + // At exactly 10 000 ms: exactly one new attempt, back to Connecting, and + // the failure count is NOT reset on a retry. + clock.advance(1); + manager.tick(); + TEST_ASSERT_EQUAL(2, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::Connecting), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT8(1, manager.snapshot().consecutiveFailures); +} + +// --------------------------------------------------------------------------- +// T015 #3 — pause after 5 consecutive failures: no attempts during the 60 s +// pause; a fresh attempt with failures reset only after it elapses. +// (contract §Timing 3) +// --------------------------------------------------------------------------- +static void test_wifi_pause_after_5_failures(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + WifiManager manager(driver, config, clock, ReconnectPolicy{}); + manager.begin(WifiBootMode::Station); // attempt #1 + + // Drive 5 consecutive failures, advancing 10 s to issue each retry between + // them. After the 5th failure the machine enters ReconnectPaused. + for (int i = 1; i <= 5; ++i) { + driver.scriptConnectFailure(); + manager.tick(); + TEST_ASSERT_EQUAL_UINT8(static_cast(i), + manager.snapshot().consecutiveFailures); + if (i < 5) { + TEST_ASSERT_EQUAL(stateInt(WifiState::Reconnecting), + stateInt(manager.snapshot().state)); + clock.advance(10000); + manager.tick(); // issues retry #(i+1) + } + } + + // Attempts so far: begin (#1) + four retries (#2..#5) = 5. + TEST_ASSERT_EQUAL(stateInt(WifiState::ReconnectPaused), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL(5, driver.staConnectCalls); + + // No attempt anywhere inside the pause window (1 ms short of 60 s). + clock.advance(59999); + manager.tick(); + TEST_ASSERT_EQUAL(5, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::ReconnectPaused), + stateInt(manager.snapshot().state)); + + // At 60 000 ms: exactly one new attempt, failures reset for the new round. + clock.advance(1); + manager.tick(); + TEST_ASSERT_EQUAL(6, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::Connecting), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT8(0, manager.snapshot().consecutiveFailures); +} + +// --------------------------------------------------------------------------- +// T015 #4 — monitor/disconnect: a drop from Connected returns to Reconnecting +// and increments disconnectCount. (contract §Timing 4) +// --------------------------------------------------------------------------- +static void test_wifi_monitor_disconnect(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + WifiManager manager(driver, config, clock, ReconnectPolicy{}); + manager.begin(WifiBootMode::Station); + driver.scriptConnectSuccess(); + manager.tick(); + TEST_ASSERT_EQUAL(stateInt(WifiState::Connected), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT32(0, manager.snapshot().disconnectCount); + + // A link drop while connected: Reconnecting, disconnectCount++, ip lost. + driver.queueEvent(WifiEvent::Disconnected); + manager.tick(); + + WifiConnectionSnapshot snap = manager.snapshot(); + TEST_ASSERT_EQUAL(stateInt(WifiState::Reconnecting), stateInt(snap.state)); + TEST_ASSERT_EQUAL_UINT32(1, snap.disconnectCount); + TEST_ASSERT_EQUAL_UINT8(1, snap.consecutiveFailures); + TEST_ASSERT_FALSE(snap.ipAcquired); +} + +// --------------------------------------------------------------------------- +// T015 #5 — AP/provisioning mode suspends all STA monitoring and reconnect, +// regardless of elapsed time, and never leaves Provisioning. (contract §6) +// --------------------------------------------------------------------------- +static void test_wifi_ap_mode_suspends(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + WifiManager manager(driver, config, clock, ReconnectPolicy{}); + + // begin(Provisioning) makes NO driver calls (AP radio is brought up in + // T018 at the wiring site). + manager.begin(WifiBootMode::Provisioning); + TEST_ASSERT_EQUAL(0, driver.staConnectCalls); + TEST_ASSERT_EQUAL(0, driver.apStartCalls); + + // Many ticks with large time advances issue ZERO STA attempts and never + // leave Provisioning. + for (int i = 0; i < 20; ++i) { + clock.advance(60000); + manager.tick(); + } + TEST_ASSERT_EQUAL(0, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::Provisioning), + stateInt(manager.snapshot().state)); +} + +// --------------------------------------------------------------------------- +// T016 — FR-014 isolation: tick() never blocks with a silent/"hung" driver +// (pollEvent() always None, staConnect yields no events) and issues no phantom +// retries while Connecting; the state stays sane over many advances. +// +// Dependency set (documented FR-014 guarantee): the WifiManager below is +// constructed with exactly IWifiDriver&, IConfigStore&, ITimeProvider& and a +// ReconnectPolicy value — there is no watering/pump/sensor parameter, so the +// manager cannot reference watering state even in principle. +// --------------------------------------------------------------------------- +static void test_wifi_isolation_no_block_no_watering_dep(void) +{ + MockWifiDriver driver; // empty queue: pollEvent() always returns None + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + WifiManager manager(driver, config, clock, ReconnectPolicy{}); + manager.begin(WifiBootMode::Station); + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); + + // 1000 ticks over ~1000 s: each returns promptly (no hang — tick() has no + // loops that wait on the driver). With no events the machine sits in + // Connecting and issues no further attempts. + for (int i = 0; i < 1000; ++i) { + clock.advance(1000); + manager.tick(); + } + + WifiConnectionSnapshot snap = manager.snapshot(); + TEST_ASSERT_EQUAL(stateInt(WifiState::Connecting), stateInt(snap.state)); + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); // no phantom retries + TEST_ASSERT_EQUAL_UINT8(0, snap.consecutiveFailures); +} + void run_wifi_tests(void) { // T008 — credential validation. @@ -136,4 +391,12 @@ void run_wifi_tests(void) RUN_TEST(test_validate_isvalid_predicate); // T009 — boot-mode truth table. RUN_TEST(test_boot_mode_truth_table); + // T015 — WifiManager reconnect schedule. + RUN_TEST(test_wifi_connect_happy_path); + RUN_TEST(test_wifi_retry_only_at_10s); + RUN_TEST(test_wifi_pause_after_5_failures); + RUN_TEST(test_wifi_monitor_disconnect); + RUN_TEST(test_wifi_ap_mode_suspends); + // T016 — FR-014 isolation / non-blocking tick. + RUN_TEST(test_wifi_isolation_no_block_no_watering_dep); } From 89dc1830d5689053179a8976ad8d3115831d1688 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 11:29:16 +0200 Subject: [PATCH 5/7] =?UTF-8?q?feat(network):=20PR-07=20US2=20wiring=20?= =?UTF-8?q?=E2=80=94=20EspWifiDriver,=20wifi=5Ftask,=20STA/AP=20bring-up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 / US2 hardware layer (target-only): - EspWifiDriver: esp_wifi/esp_netif/esp_event; WIFI_EVENT/IP_EVENT handlers push WifiEvents onto a FreeRTOS queue (lock-free task hand-off, no watering coupling); non-blocking staConnect/staStop/apStart/apStop/pollEvent/rssi; STA/AP exclusive; WIFI_STORAGE_RAM (real creds stay in wscfg NVS) - wifi_task: own FreeRTOS task (mirrors sensor_task) ticking WifiManager @250ms; drives BOARD_PIN_STATUS_LED (steady=connected, 500ms toggle=connecting/reconnecting) - app_main: builds ReconnectPolicy from CONFIG_WS_WIFI_*; provisioning branch apStart + portal; station branch WifiManager.begin(Station) + wifi_task_start; resolves the US1 apStart and US2 station TODOs. Reuses existing EspTimeProvider. - diag console: 'wifi' status line from snapshot (no credential values) Verified: host tests 174/0, both board targets build + verified. Co-Authored-By: Claude Opus 4.8 (1M context) --- firmware/components/network/CMakeLists.txt | 1 + .../network/include/network/EspWifiDriver.h | 81 +++++ .../components/network/src/EspWifiDriver.cpp | 311 ++++++++++++++++++ firmware/main/CMakeLists.txt | 2 +- firmware/main/app_main.cpp | 73 +++- firmware/main/diag_console.cpp | 73 ++++ firmware/main/diag_console.h | 16 + firmware/main/wifi_task.cpp | 123 +++++++ firmware/main/wifi_task.h | 39 +++ 9 files changed, 707 insertions(+), 12 deletions(-) create mode 100644 firmware/components/network/include/network/EspWifiDriver.h create mode 100644 firmware/components/network/src/EspWifiDriver.cpp create mode 100644 firmware/main/wifi_task.cpp create mode 100644 firmware/main/wifi_task.h diff --git a/firmware/components/network/CMakeLists.txt b/firmware/components/network/CMakeLists.txt index d8e88f0..77ac3b8 100644 --- a/firmware/components/network/CMakeLists.txt +++ b/firmware/components/network/CMakeLists.txt @@ -31,6 +31,7 @@ else() idf_component_register( SRCS "src/WifiManager.cpp" "src/ProvisioningPortal.cpp" + "src/EspWifiDriver.cpp" INCLUDE_DIRS "include" REQUIRES interfaces board PRIV_REQUIRES esp_wifi esp_netif esp_event esp_http_server diff --git a/firmware/components/network/include/network/EspWifiDriver.h b/firmware/components/network/include/network/EspWifiDriver.h new file mode 100644 index 0000000..a97eb64 --- /dev/null +++ b/firmware/components/network/include/network/EspWifiDriver.h @@ -0,0 +1,81 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file EspWifiDriver.h + * @brief IWifiDriver implementation over esp_wifi/esp_netif/esp_event + * (target-only, feature 007 US2). + * + * The single hardware touchpoint of the WiFi subsystem: it owns the STA and + * AP netifs, drives esp_wifi (mode/config/start/connect) and translates the + * esp_event callbacks into a thread-safe WifiEvent queue drained by + * pollEvent(). It carries NO timing and NO business logic — all cadence and + * state-machine decisions live in the pure WifiManager above the IWifiDriver + * seam (contracts/IWifiDriver.md). Excluded from the linux build (esp_wifi + * has no host port; host tests use MockWifiDriver). + * + * PRIV rule (same as EspI2cBus / StorageMount / ProvisioningPortal): the + * heavy IDF headers (esp_wifi.h, esp_netif.h, esp_event.h, freertos/queue.h) + * appear only in the .cpp. The netif handles and the FreeRTOS queue handle are + * held here as opaque void* so a consumer that includes this header (only + * app_main, on target) pulls in no WiFi/netif dependency. Credential values + * are never logged (FR-004 / PR-06 convention): the SSID may be logged, the + * password never. + */ + +#ifndef WATERINGSYSTEM_NETWORK_ESPWIFIDRIVER_H +#define WATERINGSYSTEM_NETWORK_ESPWIFIDRIVER_H + +#include + +#include "esp_err.h" + +#include "interfaces/IWifiDriver.h" + +/** + * @brief esp_wifi-backed WiFi driver (STA/AP control + non-blocking events). + * + * Lifetime: construct once at the wiring site (app_main function-local static, + * so no non-trivial constructor runs before the boot pump fail-safe — the + * constructor is trivial and all IDF work happens in init()). init() must be + * called exactly once, after esp_netif_init()/esp_event_loop_create_default() + * and NVS init already done in app_main, before the first staConnect/apStart. + */ +class EspWifiDriver : public IWifiDriver { +public: + EspWifiDriver() = default; + ~EspWifiDriver() override; + + EspWifiDriver(const EspWifiDriver&) = delete; + EspWifiDriver& operator=(const EspWifiDriver&) = delete; + + /** + * @brief One-time init: create the default STA + AP netifs, esp_wifi_init, + * register the WIFI_EVENT / IP_EVENT handlers and create the event + * queue. Idempotent (a second call is a successful no-op). + * + * @return ESP_OK on success; the first failing esp_err_t otherwise. On a + * failure the driver stays uninitialized and every control method + * becomes a logged no-op / false — WiFi is simply unavailable and + * the watering path is unaffected (FR-014). + */ + esp_err_t init(); + + bool staConnect(const std::string& ssid, + const std::string& password) override; + void staStop() override; + bool apStart(const std::string& ssid, + const std::string& password) override; + void apStop() override; + WifiEvent pollEvent() override; + int8_t rssi() const override; + +private: + // Held opaque to keep esp_wifi/esp_netif/freertos headers in the .cpp. + void* eventQueue_ = nullptr; ///< QueueHandle_t of WifiEvent (thread-safe) + void* staNetif_ = nullptr; ///< esp_netif_t* for the STA interface + void* apNetif_ = nullptr; ///< esp_netif_t* for the AP interface + bool initialized_ = false; ///< true once init() succeeded + bool started_ = false; ///< true between esp_wifi_start and stop +}; + +#endif /* WATERINGSYSTEM_NETWORK_ESPWIFIDRIVER_H */ diff --git a/firmware/components/network/src/EspWifiDriver.cpp b/firmware/components/network/src/EspWifiDriver.cpp new file mode 100644 index 0000000..6b1ad7d --- /dev/null +++ b/firmware/components/network/src/EspWifiDriver.cpp @@ -0,0 +1,311 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file EspWifiDriver.cpp + * @brief esp_wifi/esp_netif/esp_event implementation of IWifiDriver + * (target-only; see EspWifiDriver.h). + * + * Error handling is explicit (not ESP_ERROR_CHECK): a WiFi bring-up failure + * must be visible in the logs but must never abort — WiFi is not boot-critical + * and the watering path never depends on it (FR-014). The esp_event callbacks + * run on the default event-loop task and only push a WifiEvent onto a FreeRTOS + * queue; pollEvent() drains it from the wifi task. The FreeRTOS queue is the + * thread-safe hand-off between the two tasks (xQueueSendFromISR is not needed — + * esp_event handlers run in task context, so a plain xQueueSend/xQueueReceive + * pair is the correct, lock-free primitive here). + */ + +#include "network/EspWifiDriver.h" + +#include +#include + +#include "esp_event.h" +#include "esp_log.h" +#include "esp_netif.h" +#include "esp_wifi.h" +#include "freertos/FreeRTOS.h" +#include "freertos/queue.h" + +static const char *TAG = "espwifidriver"; + +namespace { + +/// Bounded event queue: a handful of lifecycle transitions between two ticks +/// is plenty; the manager drains the whole queue every tick (250 ms). +constexpr UBaseType_t kEventQueueLen = 8; + +/// Copy a std::string into a fixed-size, NUL-terminated esp_wifi field +/// (ssid/password are uint8_t[] arrays in wifi_config_t). Never logs the +/// content — callers decide what is safe to log. +void copyField(uint8_t *dst, std::size_t dstSize, const std::string &src) +{ + const std::size_t n = std::min(src.size(), dstSize - 1); + std::memcpy(dst, src.data(), n); + dst[n] = '\0'; +} + +/// Translate a raw WIFI_EVENT id to a WifiEvent, or None for events the +/// manager does not track. +WifiEvent translateWifiEvent(int32_t id) +{ + switch (id) { + case WIFI_EVENT_STA_CONNECTED: + return WifiEvent::Connected; + case WIFI_EVENT_STA_DISCONNECTED: + // A disconnect also serves as the "connect attempt failed" signal: + // the manager treats Disconnected/ConnectFailed identically. + return WifiEvent::Disconnected; + default: + return WifiEvent::None; + } +} + +/// WIFI_EVENT handler: arg is the WifiEvent queue handle (registered in +/// init()). Runs on the default event-loop task; pushes non-blocking. +void wifiEventHandler(void *arg, esp_event_base_t /*base*/, int32_t id, + void * /*data*/) +{ + QueueHandle_t queue = static_cast(arg); + const WifiEvent event = translateWifiEvent(id); + if (event != WifiEvent::None && queue != nullptr) { + // Non-blocking send: if the queue is somehow full the oldest news is + // simply the manager's problem to catch up on next tick — never block + // the event loop. + (void)xQueueSend(queue, &event, 0); + } +} + +/// IP_EVENT handler: only IP_EVENT_STA_GOT_IP matters — it is the "usable +/// link" signal the manager treats as fully connected. +void ipEventHandler(void *arg, esp_event_base_t /*base*/, int32_t id, + void * /*data*/) +{ + if (id != IP_EVENT_STA_GOT_IP) { + return; + } + QueueHandle_t queue = static_cast(arg); + const WifiEvent event = WifiEvent::GotIp; + if (queue != nullptr) { + (void)xQueueSend(queue, &event, 0); + } +} + +} // namespace + +EspWifiDriver::~EspWifiDriver() +{ + // Function-local static in app_main: this never actually runs (the program + // lives forever). Kept correct for completeness / unit reasoning. + if (started_) { + esp_wifi_stop(); + } + if (initialized_) { + esp_wifi_deinit(); + } + if (eventQueue_ != nullptr) { + vQueueDelete(static_cast(eventQueue_)); + } +} + +esp_err_t EspWifiDriver::init() +{ + if (initialized_) { + return ESP_OK; // idempotent + } + + // The default STA + AP netifs. esp_netif_init() and the default event loop + // are created earlier in app_main (T007); creating both interfaces up front + // lets staConnect/apStart switch modes without re-plumbing netifs. + staNetif_ = esp_netif_create_default_wifi_sta(); + apNetif_ = esp_netif_create_default_wifi_ap(); + if (staNetif_ == nullptr || apNetif_ == nullptr) { + ESP_LOGE(TAG, "failed to create default WiFi netifs"); + return ESP_FAIL; + } + + const wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT(); + esp_err_t err = esp_wifi_init(&cfg); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_init failed: %s", esp_err_to_name(err)); + return err; + } + + // Thread-safe hand-off between the esp_event task (producer) and the wifi + // task (consumer, via pollEvent()). + QueueHandle_t queue = xQueueCreate(kEventQueueLen, sizeof(WifiEvent)); + if (queue == nullptr) { + ESP_LOGE(TAG, "failed to create WiFi event queue"); + esp_wifi_deinit(); + return ESP_ERR_NO_MEM; + } + eventQueue_ = queue; + + // The handlers receive the queue handle as their arg, so they need no + // access to the driver instance (keeps them file-local and header-clean). + err = esp_event_handler_instance_register(WIFI_EVENT, ESP_EVENT_ANY_ID, + &wifiEventHandler, queue, nullptr); + if (err != ESP_OK) { + ESP_LOGE(TAG, "WIFI_EVENT handler register failed: %s", + esp_err_to_name(err)); + return err; + } + err = esp_event_handler_instance_register(IP_EVENT, IP_EVENT_STA_GOT_IP, + &ipEventHandler, queue, nullptr); + if (err != ESP_OK) { + ESP_LOGE(TAG, "IP_EVENT handler register failed: %s", + esp_err_to_name(err)); + return err; + } + + // Store WiFi calibration/config in RAM only: NVS holds our own config + // (wscfg namespace) and we never want esp_wifi to persist scratch there. + err = esp_wifi_set_storage(WIFI_STORAGE_RAM); + if (err != ESP_OK) { + ESP_LOGW(TAG, "esp_wifi_set_storage(RAM) failed: %s", + esp_err_to_name(err)); + } + + initialized_ = true; + ESP_LOGI(TAG, "WiFi driver initialized (STA + AP netifs, event queue)"); + return ESP_OK; +} + +bool EspWifiDriver::staConnect(const std::string &ssid, + const std::string &password) +{ + if (!initialized_) { + ESP_LOGE(TAG, "staConnect before init — ignored"); + return false; + } + + // STA and AP are mutually exclusive here: switching to STA mode tears down + // any running SoftAP. + esp_err_t err = esp_wifi_set_mode(WIFI_MODE_STA); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_set_mode(STA) failed: %s", + esp_err_to_name(err)); + return false; + } + + wifi_config_t wc = {}; + copyField(wc.sta.ssid, sizeof(wc.sta.ssid), ssid); + copyField(wc.sta.password, sizeof(wc.sta.password), password); + err = esp_wifi_set_config(WIFI_IF_STA, &wc); + if (err != ESP_OK) { + // Password value never logged (FR-004). + ESP_LOGE(TAG, "esp_wifi_set_config(STA) failed: %s", + esp_err_to_name(err)); + return false; + } + + if (!started_) { + err = esp_wifi_start(); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_start failed: %s", esp_err_to_name(err)); + return false; + } + started_ = true; + } + + // Non-blocking: the outcome arrives later as a WifiEvent. A synchronous + // failure here is reported false; the manager stays deterministic. + err = esp_wifi_connect(); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_connect failed: %s", esp_err_to_name(err)); + return false; + } + ESP_LOGI(TAG, "STA connect requested (ssid=%s)", ssid.c_str()); + return true; +} + +void EspWifiDriver::staStop() +{ + if (!initialized_) { + return; + } + // Idempotent: esp_wifi_disconnect returns an error when not connected, + // which is a benign no-op here. + (void)esp_wifi_disconnect(); +} + +bool EspWifiDriver::apStart(const std::string &ssid, + const std::string &password) +{ + if (!initialized_) { + ESP_LOGE(TAG, "apStart before init — ignored"); + return false; + } + + esp_err_t err = esp_wifi_set_mode(WIFI_MODE_AP); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_set_mode(AP) failed: %s", + esp_err_to_name(err)); + return false; + } + + wifi_config_t wc = {}; + copyField(wc.ap.ssid, sizeof(wc.ap.ssid), ssid); + wc.ap.ssid_len = static_cast( + std::min(ssid.size(), sizeof(wc.ap.ssid))); + copyField(wc.ap.password, sizeof(wc.ap.password), password); + wc.ap.channel = 1; + wc.ap.max_connection = 4; + // WPA2 by default; an empty password yields an open AP (documented + // non-secret provisioning window — Kconfig WS_PROV_AP_PASSWORD help). + wc.ap.authmode = + password.empty() ? WIFI_AUTH_OPEN : WIFI_AUTH_WPA2_PSK; + err = esp_wifi_set_config(WIFI_IF_AP, &wc); + if (err != ESP_OK) { + // Password value never logged (FR-004). + ESP_LOGE(TAG, "esp_wifi_set_config(AP) failed: %s", + esp_err_to_name(err)); + return false; + } + + if (!started_) { + err = esp_wifi_start(); + if (err != ESP_OK) { + ESP_LOGE(TAG, "esp_wifi_start failed: %s", esp_err_to_name(err)); + return false; + } + started_ = true; + } + // Default AP address is 192.168.4.1 (esp_netif_create_default_wifi_ap). + ESP_LOGI(TAG, "SoftAP started (ssid=%s, %s) at 192.168.4.1", ssid.c_str(), + password.empty() ? "open" : "WPA2"); + return true; +} + +void EspWifiDriver::apStop() +{ + if (!initialized_ || !started_) { + return; // idempotent + } + (void)esp_wifi_stop(); + started_ = false; +} + +WifiEvent EspWifiDriver::pollEvent() +{ + if (eventQueue_ == nullptr) { + return WifiEvent::None; + } + WifiEvent event = WifiEvent::None; + // Non-blocking (timeout 0): return the next queued event or None. + if (xQueueReceive(static_cast(eventQueue_), &event, 0) == + pdTRUE) { + return event; + } + return WifiEvent::None; +} + +int8_t EspWifiDriver::rssi() const +{ + wifi_ap_record_t ap = {}; + if (esp_wifi_sta_get_ap_info(&ap) == ESP_OK) { + return ap.rssi; + } + // Unspecified when not connected (contract): report 0. + return 0; +} diff --git a/firmware/main/CMakeLists.txt b/firmware/main/CMakeLists.txt index d408d5f..0bbc12c 100644 --- a/firmware/main/CMakeLists.txt +++ b/firmware/main/CMakeLists.txt @@ -1,5 +1,5 @@ idf_component_register( - SRCS "app_main.cpp" "diag_console.cpp" "sensor_task.cpp" + SRCS "app_main.cpp" "diag_console.cpp" "sensor_task.cpp" "wifi_task.cpp" PRIV_REQUIRES board esp_driver_gpio esp_app_format actuators interfaces console esp_timer storage nvs_flash sensors esp_netif esp_event diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index 2618386..3286bb6 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -50,8 +50,11 @@ #include "sensors/Ina226Sensor.h" #include "sensors/LockedPowerSensor.h" #endif +#include "network/EspWifiDriver.h" #include "network/ProvisioningPortal.h" #include "network/WifiBootMode.h" +#include "network/WifiManager.h" +#include "network/WifiState.h" #include "storage/LittleFsDataStorage.h" #include "storage/LockedConfigStore.h" #include "storage/LockedDataStorage.h" @@ -60,6 +63,7 @@ #include "diag_console.h" #include "sensor_task.h" +#include "wifi_task.h" static const char *TAG = "app_main"; @@ -194,8 +198,8 @@ extern "C" void app_main(void) // they are created once, here, after NVS (esp_wifi persists calibration // in NVS) and before any WiFi object. Not safety-critical: a failure is // logged and the system keeps running — WiFi is simply unavailable, and - // the watering path never depends on the network (FR-014). No WiFi - // objects are constructed yet in this phase. + // the watering path never depends on the network (FR-014). The WiFi driver + // and manager are constructed below, after this init. esp_err_t netif_err = esp_netif_init(); if (netif_err != ESP_OK) { ESP_LOGE(TAG, "esp_netif_init failed: %s (WiFi unavailable)", @@ -244,13 +248,36 @@ extern "C" void app_main(void) const WifiBootMode wifi_boot_mode = decideBootMode(wifi_credentials_present, config_button_held); + // The single WiFi driver (one hardware touchpoint) — a function-local + // static after pumps_force_off() (boot fail-safe rule: the constructor is + // trivial, all IDF work is in init()). Both boot modes use it: provisioning + // brings up the SoftAP through it, station drives STA connect/reconnect. + // init() creates the STA + AP netifs, esp_wifi_init and the event queue on + // top of the netif/event-loop init done above; a failure is non-fatal + // (logged) — WiFi is simply unavailable and the watering path is + // unaffected (FR-014). + static EspWifiDriver wifi_driver; + const esp_err_t wifi_init_err = wifi_driver.init(); + if (wifi_init_err != ESP_OK) { + ESP_LOGE(TAG, "WiFi driver init failed: %s (WiFi unavailable)", + esp_err_to_name(wifi_init_err)); + } + + // Set later in the station branch; used both to start the wifi task and to + // register the manager with the diag console below. Stays nullptr in + // provisioning mode (the console `wifi` command then reports unavailable). + WifiManager *wifi_manager = nullptr; + if (wifi_boot_mode == WifiBootMode::Provisioning) { ESP_LOGI(TAG, "WiFi: provisioning mode (SoftAP setup portal)"); - // TODO(US2/T018): bring up the SoftAP radio via EspWifiDriver::apStart - // using CONFIG_WS_PROV_AP_SSID / CONFIG_WS_PROV_AP_PASSWORD (WPA2). - // EspWifiDriver arrives with US2; until the radio is up the portal - // is started here but not yet reachable over the air. The portal - // only depends on the config store, so it is wired now. + // Bring up the SoftAP radio BEFORE the portal starts serving so the + // page is reachable over the air the moment it is registered (T018). + // Credential VALUES are never logged (FR-004): the AP SSID is not a + // secret, the WPA2 password is. + if (!wifi_driver.apStart(CONFIG_WS_PROV_AP_SSID, + CONFIG_WS_PROV_AP_PASSWORD)) { + ESP_LOGE(TAG, "SoftAP failed to start (portal may be unreachable)"); + } // Function-local static (boot fail-safe rule: no non-trivial // static/global constructors). Constructed only on this branch, kept // alive for the program lifetime so it keeps serving. @@ -262,10 +289,32 @@ extern "C" void app_main(void) } } else { ESP_LOGI(TAG, "WiFi: station mode (stored credentials present)"); - // TODO(US2/T020): construct EspWifiDriver + WifiManager, - // begin(WifiBootMode::Station) and wifi_task_start(manager). The - // station connect/reconnect path is deferred to US2; no STA connect - // is issued in this PR. + // Reconnect timing from Kconfig (parity defaults, docs/parity- + // checklist.md §7): 10 s retry, +60 s pause after 5 consecutive + // failures, 5 s health monitor. The pure WifiManager owns all cadence + // above the IWifiDriver seam. + const ReconnectPolicy wifi_policy = { + .retryIntervalMs = + static_cast(CONFIG_WS_WIFI_RETRY_INTERVAL_MS), + .failuresBeforePause = + static_cast(CONFIG_WS_WIFI_FAILS_BEFORE_PAUSE), + .pauseMs = static_cast(CONFIG_WS_WIFI_PAUSE_MS), + .monitorIntervalMs = + static_cast(CONFIG_WS_WIFI_MONITOR_INTERVAL_MS), + }; + // Function-local static (boot fail-safe rule). Reuses the app_main + // EspTimeProvider (the same monotonic clock the pump/level layer uses) + // and the shared config store; holds NO watering/pump/sensor reference + // (FR-014, structurally enforced by the constructor signature). begin() + // issues the first STA connect; the wifi task then ticks it. + static WifiManager wifi_manager_inst(wifi_driver, config, time_provider, + wifi_policy); + wifi_manager_inst.begin(WifiBootMode::Station); + wifi_manager = &wifi_manager_inst; + + // Separate FreeRTOS task from the 10 Hz pump/level loop — no shared + // mutex with watering (FR-014). Drives the status LED too (T021). + wifi_task_start(wifi_manager_inst); } // RS485 Modbus soil sensor (feature 004). Not safety-critical: a failed @@ -409,6 +458,8 @@ extern "C" void app_main(void) #if BOARD_HAS_INA226 diag_console_register_power(power_sensor); #endif + // nullptr in provisioning mode — the `wifi` command reports unavailable. + diag_console_register_wifi(wifi_manager); esp_err_t err = diag_console_start(); if (err != ESP_OK) { // Console is a diagnostic aid, not a safety function: log and keep diff --git a/firmware/main/diag_console.cpp b/firmware/main/diag_console.cpp index 101719c..c5bf459 100644 --- a/firmware/main/diag_console.cpp +++ b/firmware/main/diag_console.cpp @@ -91,6 +91,7 @@ #include "interfaces/IPowerSensor.h" #include "interfaces/ISoilSensor.h" #include "interfaces/IWaterPump.h" +#include "network/WifiManager.h" namespace { @@ -152,6 +153,11 @@ ILevelSensor *s_level_high = nullptr; IPowerSensor *s_power = nullptr; #endif +// WiFi station manager (set from app_main; nullptr when the device booted in +// provisioning mode — no station manager exists then). Same trivial- +// initialization rule. The `wifi` command reads only its snapshot(). +WifiManager *s_wifi = nullptr; + const char *stop_reason_str(StopReason reason) { switch (reason) { @@ -779,6 +785,52 @@ int power_cmd(int argc, char **argv) #endif // BOARD_HAS_INA226 +// --- wifi command (feature 007 US2 status path) -------------------------- + +/// WifiState -> stable console word (never a credential value). +const char *wifi_state_str(WifiState state) +{ + switch (state) { + case WifiState::Provisioning: + return "provisioning"; + case WifiState::Connecting: + return "connecting"; + case WifiState::Connected: + return "connected"; + case WifiState::Reconnecting: + return "reconnecting"; + case WifiState::ReconnectPaused: + return "reconnect_paused"; + default: + return "unknown"; + } +} + +/// `wifi`: one immutable snapshot of the station manager — state, RSSI and the +/// reconnect/disconnect counters. Never echoes SSID or password (FR-004); the +/// snapshot carries no credential fields by design. +int wifi_cmd(int argc, char **argv) +{ + (void)argv; + if (argc != 1) { + printf("ERR usage: wifi\n"); + return 1; + } + if (s_wifi == nullptr) { + // Provisioning/unconfigured boot: no station manager to report. + printf("ERR wifi station manager not available (provisioning or " + "unconfigured)\n"); + return 1; + } + const WifiConnectionSnapshot snap = s_wifi->snapshot(); + printf("OK state=%s ip=%s rssi=%d dBm failures=%u disconnects=%lu\n", + wifi_state_str(snap.state), snap.ipAcquired ? "yes" : "no", + static_cast(snap.rssi), + static_cast(snap.consecutiveFailures), + static_cast(snap.disconnectCount)); + return 0; +} + } // namespace #if BOARD_HAS_RESERVOIR_PUMP @@ -824,6 +876,12 @@ void diag_console_register_power(IPowerSensor& sensor) } #endif +void diag_console_register_wifi(WifiManager* manager) +{ + // nullptr is valid: provisioning/unconfigured boot has no station manager. + s_wifi = manager; +} + esp_err_t diag_console_start(void) { esp_console_repl_t *repl = nullptr; @@ -1002,5 +1060,20 @@ esp_err_t diag_console_start(void) } #endif + const esp_console_cmd_t cmd_wifi = { + .command = "wifi", + .help = "wifi — station connection status (state / rssi / counters; " + "no credentials)", + .hint = nullptr, + .func = &wifi_cmd, + .argtable = nullptr, + .func_w_context = nullptr, + .context = nullptr, + }; + err = esp_console_cmd_register(&cmd_wifi); + if (err != ESP_OK) { + return err; + } + return esp_console_start_repl(repl); } diff --git a/firmware/main/diag_console.h b/firmware/main/diag_console.h index 0c66ce7..c783cee 100644 --- a/firmware/main/diag_console.h +++ b/firmware/main/diag_console.h @@ -21,6 +21,7 @@ #include "interfaces/IPowerSensor.h" #include "interfaces/ISoilSensor.h" #include "interfaces/IWaterPump.h" +#include "network/WifiManager.h" /** * @brief Register the pump instances the console commands operate on. @@ -102,6 +103,21 @@ void diag_console_register_level(ILevelSensor& low, ILevelSensor& high); void diag_console_register_power(IPowerSensor& sensor); #endif +/** + * @brief Register the WiFi manager the `wifi` status command reads (feature + * 007 US2). + * + * Pass the station-mode WifiManager, or nullptr when the device booted in + * provisioning mode (no station manager exists) — the `wifi` command then + * reports "not available". The command only reads the manager's immutable + * snapshot() (single acquisition), never the raw state, and never echoes + * credentials (FR-004). Must be called before diag_console_start(); plain + * pointer registration. + * + * @param manager Station manager, or nullptr in provisioning/unconfigured mode. + */ +void diag_console_register_wifi(WifiManager* manager); + /** * @brief Start the UART REPL (prompt "ws>") and register the commands. * diff --git a/firmware/main/wifi_task.cpp b/firmware/main/wifi_task.cpp new file mode 100644 index 0000000..54bf733 --- /dev/null +++ b/firmware/main/wifi_task.cpp @@ -0,0 +1,123 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file wifi_task.cpp + * @brief WiFi station tick task + status LED (feature 007 US2, T019/T021). + * + * Mirrors sensor_task.cpp: its own 4096 B stack at priority 1, a [[noreturn]] + * fixed-cadence loop, the manager injected via the void* task arg, and a + * non-fatal creation failure. This is a SEPARATE task from the 10 Hz + * pump/level loop and holds no watering mutex (FR-014); all it does is advance + * the pure WifiManager (non-blocking tick()) and mirror the snapshot on the + * status LED. + * + * LED policy (parity §7/§9): ~500 ms toggle while Connecting/Reconnecting, + * steady ON when Connected, OFF while ReconnectPaused (or any non-station + * state). The 100 ms config-button-hold blink is US3 (T026), not here. + */ + +#include "wifi_task.h" + +#include "board/board.h" +#include "driver/gpio.h" +#include "esp_log.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" + +#include "network/WifiState.h" + +static const char *TAG = "wifi_task"; + +namespace { + +constexpr uint32_t kTickPeriodMs = 250; ///< manager tick cadence +constexpr uint32_t kBlinkHalfPeriodMs = 500; ///< LED toggle half-period +constexpr uint32_t kStackBytes = 4096; ///< match sensor_task (R7) +constexpr UBaseType_t kPriority = 1; ///< match sensor_task (R7) + +/// Configure the status LED as a driven, initially-off output. Non-fatal: a +/// failure only costs the visual indicator, never the tick loop. +void status_led_init() +{ + const gpio_config_t led_cfg = { + .pin_bit_mask = 1ULL << BOARD_PIN_STATUS_LED, + .mode = GPIO_MODE_OUTPUT, + .pull_up_en = GPIO_PULLUP_DISABLE, + .pull_down_en = GPIO_PULLDOWN_DISABLE, + .intr_type = GPIO_INTR_DISABLE, + }; + const esp_err_t err = gpio_config(&led_cfg); + if (err != ESP_OK) { + ESP_LOGW(TAG, "status LED gpio_config failed: %s (LED disabled)", + esp_err_to_name(err)); + return; + } + gpio_set_level(static_cast(BOARD_PIN_STATUS_LED), 0); +} + +[[noreturn]] void wifi_task(void *arg) +{ + WifiManager &manager = *static_cast(arg); + + status_led_init(); + bool led_on = false; + uint32_t blink_accum_ms = 0; + + const auto set_led = [&led_on](bool on) { + if (led_on != on) { + led_on = on; + gpio_set_level(static_cast(BOARD_PIN_STATUS_LED), + on ? 1 : 0); + } + }; + + TickType_t last_wake = xTaskGetTickCount(); + while (true) { + vTaskDelayUntil(&last_wake, pdMS_TO_TICKS(kTickPeriodMs)); + + // Non-blocking: advances from time deltas + drained driver events; + // never sleeps, waits or reboots (FR-013/FR-014). + manager.tick(); + + const WifiConnectionSnapshot snap = manager.snapshot(); + switch (snap.state) { + case WifiState::Connecting: + case WifiState::Reconnecting: + // ~500 ms toggle: attempting to (re)join the network. + blink_accum_ms += kTickPeriodMs; + if (blink_accum_ms >= kBlinkHalfPeriodMs) { + blink_accum_ms = 0; + set_led(!led_on); + } + break; + case WifiState::Connected: + // Steady on: usable link. + blink_accum_ms = 0; + set_led(true); + break; + case WifiState::ReconnectPaused: + case WifiState::Provisioning: + // Off: paused between rounds (or not station-managed at all). + blink_accum_ms = 0; + set_led(false); + break; + } + } +} + +} // namespace + +void wifi_task_start(WifiManager &manager) +{ + const BaseType_t created = + xTaskCreate(wifi_task, "wifi_task", kStackBytes, &manager, kPriority, + nullptr); + if (created != pdPASS) { + // Not a safety function: log and continue without the WiFi tick loop + // (the watering path is unaffected — FR-014). + ESP_LOGE(TAG, "failed to create wifi task"); + return; + } + ESP_LOGI(TAG, "wifi task started (%lu ms tick cadence)", + static_cast(kTickPeriodMs)); +} diff --git a/firmware/main/wifi_task.h b/firmware/main/wifi_task.h new file mode 100644 index 0000000..0801ed1 --- /dev/null +++ b/firmware/main/wifi_task.h @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: 2026 Cryptotomte +// SPDX-License-Identifier: AGPL-3.0-or-later +/** + * @file wifi_task.h + * @brief WiFi station tick task (app wiring, feature 007 US2). + * + * App-level FreeRTOS task, not a component — mirrors main/sensor_task.h. It is + * a SEPARATE task from the 10 Hz pump/level loop and shares no mutex with the + * watering path (FR-014): all it does is call WifiManager::tick() at a fixed + * cadence and reflect the connection state on the status LED. Task/behavior + * contract: specs/007-wifi-provisioning/contracts/wifi-manager-states.md + * (tick cadence) and docs/parity-checklist.md §7/§9 (LED). + */ + +#ifndef WATERINGSYSTEM_MAIN_WIFI_TASK_H +#define WATERINGSYSTEM_MAIN_WIFI_TASK_H + +#include "network/WifiManager.h" + +/** + * @brief Start the WiFi station tick task (station mode only). + * + * The task calls manager.tick() every ~250 ms (non-blocking; the manager + * advances purely from time deltas + drained driver events) and drives + * BOARD_PIN_STATUS_LED: ~500 ms toggle while Connecting/Reconnecting, steady + * on when Connected, off otherwise. Started only when the device boots in + * station mode — provisioning brings up the AP + portal instead and does not + * run this task. + * + * Call once, from app_main, after WifiManager::begin(Station). A task-creation + * failure is logged and swallowed — WiFi is not a safety function and the + * watering path is unaffected. + * + * @param manager Ticked forever; must outlive the task (pass a function-local + * static from app_main). + */ +void wifi_task_start(WifiManager& manager); + +#endif /* WATERINGSYSTEM_MAIN_WIFI_TASK_H */ From 68d74da5a68f87b6dc27d0e5318be725a5e6d2ce Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 11:40:53 +0200 Subject: [PATCH 6/7] =?UTF-8?q?feat(network):=20PR-07=20US3=20=E2=80=94=20?= =?UTF-8?q?config-button=20emergency=20provisioning=20+=20no-boot-loop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 / US3 + polish docs: - app_main: read BOARD_PIN_BTN_CONFIG (GPIO18, active-LOW/pull-up) at boot; held >=5s forces provisioning with 100ms status-LED blink; clears WiFi credentials first on an already-configured device (emergency reset). pumps_force_off() still first; hold loop bounded and only entered when pressed at boot. - WifiBootMode: pure shouldClearCredentialsOnBoot(credsPresent, buttonHeld) - host tests: no-boot-loop under permanent failure (800 rounds, bounded counters, never Connected/Provisioning), emergency-reset decision -> 176/0 - firmware/CLAUDE.md: reference the FR9 SoftAP-portal decision (specs/007-*) - HIL checklist specs/007-wifi-provisioning/checklists/hil.md for the rev1 rig Verified: host 176/0; rev1 1011536B (36% slot free) + rev2 1013920B (35% free), both board-config verified; dependencies.lock + esp-modbus pins unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- firmware/CLAUDE.md | 37 ++++++ .../network/include/network/WifiBootMode.h | 24 ++++ firmware/main/app_main.cpp | 125 ++++++++++++++++-- firmware/test_apps/host/main/test_wifi.cpp | 90 +++++++++++++ specs/007-wifi-provisioning/checklists/hil.md | 105 +++++++++++++++ 5 files changed, 373 insertions(+), 8 deletions(-) create mode 100644 specs/007-wifi-provisioning/checklists/hil.md diff --git a/firmware/CLAUDE.md b/firmware/CLAUDE.md index e256d92..a2a1abc 100644 --- a/firmware/CLAUDE.md +++ b/firmware/CLAUDE.md @@ -343,6 +343,43 @@ code. **Hardware validation is deferred to PR-14** (no INA226 on the rev1 rig): the driver is host-verified only, and the written config value carries a `TODO(PR-14)` bench confirmation. +## WiFi provisioning & station management + +Feature 007 (PR-07). **FR9 decision: a custom SoftAP provisioning portal** (AP +at 192.168.4.1, WPA2, a minimal self-contained setup page + one POST handler on +a standalone `esp_http_server`), chosen over IDF `wifi_provisioning` — full spec +and rationale in `specs/007-wifi-provisioning/` (`spec.md` + research decision +**D1**; `wifi_provisioning` is the documented re-escalation fallback if the +portal proves unreliable at HIL, research R1). The `network` component splits at +the `IWifiDriver` seam, same pattern as the sensor drivers: + +- **Pure, host-tested** (`components/network/include/network/`): the + `WifiManager` state machine (Provisioning / Connecting / Connected / + Reconnecting / ReconnectPaused, parity reconnect cadence — 10 s retry, +60 s + pause after 5 consecutive failures, 5 s health monitor; **never reboots** — + FR-013 no boot loop), `validateWifiCredentials` (SSID 1–32, password + empty-or-8..64), `decideBootMode` + `shouldClearCredentialsOnBoot` + (`WifiBootMode.h`). Driven over `MockWifiDriver` + `FakeTimeProvider` in + `test_apps/host/main/test_wifi.cpp`. +- **Hardware touchpoints** (target-only, excluded from the linux build): + `EspWifiDriver` (STA + AP netifs, `esp_event` → `WifiEvent` queue) and + `ProvisioningPortal` (`esp_http_server`). + +**Isolation (FR-014):** `WifiManager`'s constructor takes only +`IWifiDriver&`/`IConfigStore&`/`ITimeProvider&`/`ReconnectPolicy` — no +watering/pump/sensor reference — and the wifi task is a SEPARATE FreeRTOS task +from the 10 Hz pump/level loop with no shared mutex; WiFi outages never touch +watering. Credentials come from PR-06's `IConfigStore` (never logged, FR-004). + +**Boot flow in `app_main`** (all strictly after `pumps_force_off()`): read the +config button (`BOARD_PIN_BTN_CONFIG`, GPIO18, active LOW, >= 5 s hold, 100 ms +LED blink) → `decideBootMode` → provisioning (button-forced on a configured +device clears credentials first, per the data-model boot rule) or station +(`begin(Station)` + `wifi_task_start`). Kconfig: `WS_PROV_AP_SSID`, +`WS_PROV_AP_PASSWORD`, `WS_WIFI_*` reconnect constants. LED scope (parity +§7/§9): 500 ms connect-attempt toggle (wifi task) + 100 ms config-button-hold +blink (app_main); HIL checklist in `specs/007-wifi-provisioning/checklists/hil.md`. + ## Partition layout (4MB flash) nvs (0x9000, 16K) | otadata (0xd000, 8K) | phy_init (0xf000, 4K) | diff --git a/firmware/components/network/include/network/WifiBootMode.h b/firmware/components/network/include/network/WifiBootMode.h index 2d5d595..56bbeca 100644 --- a/firmware/components/network/include/network/WifiBootMode.h +++ b/firmware/components/network/include/network/WifiBootMode.h @@ -48,4 +48,28 @@ inline WifiBootMode decideBootMode(bool credentialsPresent, return WifiBootMode::Station; } +/** + * @brief Whether the stored WiFi credentials must be cleared before entering + * provisioning at boot. + * + * The emergency-reset path (data-model.md boot rule): only when a device that + * DOES have stored credentials is forced into provisioning by a held config + * button do we wipe them first, so re-provisioning starts from a clean + * unconfigured state and cannot silently keep the old network. An already + * unconfigured device has nothing to clear, and a normal station boot (button + * released) must never touch the credentials. + * + * Pure boolean intent, host-tested; the actual `clearWifiCredentials()` side + * effect lives at the app_main wiring site (T025). + * + * @param credentialsPresent true when a non-empty SSID is stored. + * @param configButtonHeld true when the config button is held at boot. + * @return true iff (credentialsPresent && configButtonHeld). + */ +inline bool shouldClearCredentialsOnBoot(bool credentialsPresent, + bool configButtonHeld) +{ + return credentialsPresent && configButtonHeld; +} + #endif /* WATERINGSYSTEM_NETWORK_WIFIBOOTMODE_H */ diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index 3286bb6..abc4a4d 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -67,6 +67,15 @@ static const char *TAG = "app_main"; +// Config-button emergency-provisioning hold (feature 007, US3). Parity +// (docs/parity-checklist.md §7): the config button held >= 5 s during startup +// forces WiFi provisioning; the status LED blinks every 100 ms while the hold +// is being confirmed so the operator sees the reset registering (§7/§9). This +// 100 ms button-hold blink is distinct from the wifi task's 500 ms +// connect-attempt toggle (that one runs later, from wifi_task.cpp). +static constexpr uint32_t kConfigButtonHoldMs = 5000; // hold to force prov. +static constexpr uint32_t kConfigButtonBlinkMs = 100; // LED toggle interval + /** * @brief Drive every pump GPIO that exists on this board to a safe OFF * state (output, level 0). @@ -119,6 +128,88 @@ static void pumps_force_off(void) } } +/** + * @brief Read the config button at boot and confirm a >= 5 s hold (feature + * 007, US3/T024/T026). + * + * The config button (BOARD_PIN_BTN_CONFIG, GPIO18) is wired to GND and read + * with an internal pull-up, so it is active LOW: held == logic 0 (parity: the + * legacy INPUT_PULLUP idiom, same as the level-sensor inputs). Semantics: + * + * - Not pressed at boot → return false immediately, no delay (the common, + * fast path: a normal boot must not stall). + * - Pressed at boot → enter a bounded hold-confirm loop of at most + * kConfigButtonHoldMs (5 s), sampling every kConfigButtonBlinkMs (100 ms) + * and toggling BOARD_PIN_STATUS_LED each sample so the operator sees the + * reset registering (parity §7/§9). Released before the window elapses → + * return false (treated as not held). Held for the whole window → return + * true (forced provisioning). + * + * The loop is bounded by kConfigButtonHoldMs and runs strictly AFTER + * pumps_force_off() (pumps already safe) and before the WiFi/sensor tasks + * start. Not safety-critical: a GPIO-config failure is logged and reported as + * "released" so a stuck read can never wedge boot into provisioning. + */ +static bool config_button_held_at_boot(void) +{ + const gpio_config_t btn_cfg = { + .pin_bit_mask = 1ULL << BOARD_PIN_BTN_CONFIG, + .mode = GPIO_MODE_INPUT, + .pull_up_en = GPIO_PULLUP_ENABLE, + .pull_down_en = GPIO_PULLDOWN_DISABLE, + .intr_type = GPIO_INTR_DISABLE, + }; + const esp_err_t err = gpio_config(&btn_cfg); + if (err != ESP_OK) { + ESP_LOGW(TAG, "config button gpio_config failed: %s (treated as released)", + esp_err_to_name(err)); + return false; + } + + // Active LOW: a released button reads 1 (pulled up). Fast path — no delay + // on a normal boot. + if (gpio_get_level(static_cast(BOARD_PIN_BTN_CONFIG)) != 0) { + return false; + } + + ESP_LOGI(TAG, "config button down at boot — hold %lu ms to force provisioning", + static_cast(kConfigButtonHoldMs)); + + // Drive the status LED for the hold-confirm blink. Non-fatal: a failure + // only costs the visual cue, never the hold decision. + const gpio_config_t led_cfg = { + .pin_bit_mask = 1ULL << BOARD_PIN_STATUS_LED, + .mode = GPIO_MODE_OUTPUT, + .pull_up_en = GPIO_PULLUP_DISABLE, + .pull_down_en = GPIO_PULLDOWN_DISABLE, + .intr_type = GPIO_INTR_DISABLE, + }; + if (gpio_config(&led_cfg) != ESP_OK) { + ESP_LOGW(TAG, "status LED gpio_config failed during config-button hold"); + } + + // Bounded hold-confirm loop: at most kConfigButtonHoldMs, sampled every + // kConfigButtonBlinkMs. Toggle the LED each sample (100 ms blink, parity + // §7/§9). Any early release aborts. + bool led_on = false; + for (uint32_t elapsed_ms = 0; elapsed_ms < kConfigButtonHoldMs; + elapsed_ms += kConfigButtonBlinkMs) { + vTaskDelay(pdMS_TO_TICKS(kConfigButtonBlinkMs)); + if (gpio_get_level(static_cast(BOARD_PIN_BTN_CONFIG)) != 0) { + gpio_set_level(static_cast(BOARD_PIN_STATUS_LED), 0); + ESP_LOGI(TAG, "config button released early — not forcing provisioning"); + return false; + } + led_on = !led_on; + gpio_set_level(static_cast(BOARD_PIN_STATUS_LED), + led_on ? 1 : 0); + } + gpio_set_level(static_cast(BOARD_PIN_STATUS_LED), 0); + ESP_LOGI(TAG, "config button held >= %lu ms — forcing WiFi provisioning", + static_cast(kConfigButtonHoldMs)); + return true; +} + extern "C" void app_main(void) { // Fail-safe first: every pump that exists on this board off before @@ -236,15 +327,18 @@ extern "C" void app_main(void) static_cast(stats.usedBytes / 1024), static_cast(stats.totalBytes / 1024)); - // WiFi boot-mode decision (feature 007, US1). A missing stored SSID (the - // factory/unconfigured state) OR a held config button forces first-boot/ - // recovery provisioning; a configured device otherwise comes up in - // station mode. The config-button read is US3/T024 — until then the - // button is reported as released. Credential VALUES are never logged: we - // only test whether an SSID is present. WiFi never touches the watering - // path (FR-014); everything below stays after the pump fail-safe. + // WiFi boot-mode decision (feature 007, US1 + US3). A missing stored SSID + // (the factory/unconfigured state) OR a held config button forces + // first-boot/recovery provisioning; a configured device otherwise comes up + // in station mode. The config-button read (US3/T024) samples GPIO18 and + // confirms a >= 5 s hold, blinking the status LED at 100 ms while it does + // (T026); it runs here — strictly after pumps_force_off() (pumps already + // safe) and before the WiFi/sensor tasks start — and is bounded by the + // hold window. Credential VALUES are never logged: we only test whether an + // SSID is present. WiFi never touches the watering path (FR-014); + // everything below stays after the pump fail-safe. const bool wifi_credentials_present = !config.getWifiSsid().empty(); - const bool config_button_held = false; // TODO(US3/T024): read BOARD_PIN_BTN_CONFIG at boot + const bool config_button_held = config_button_held_at_boot(); const WifiBootMode wifi_boot_mode = decideBootMode(wifi_credentials_present, config_button_held); @@ -270,6 +364,21 @@ extern "C" void app_main(void) if (wifi_boot_mode == WifiBootMode::Provisioning) { ESP_LOGI(TAG, "WiFi: provisioning mode (SoftAP setup portal)"); + // Emergency reset (US3/T025): when the config button forced + // provisioning on an ALREADY-configured device, wipe the stored + // credentials BEFORE the AP/portal comes up, so re-provisioning starts + // from a clean unconfigured state and cannot silently keep the old + // network (data-model.md boot rule). An unconfigured device has nothing + // to clear. Credential VALUES are never logged (FR-004). + if (shouldClearCredentialsOnBoot(wifi_credentials_present, + config_button_held)) { + ESP_LOGI(TAG, "config button forced provisioning on a configured " + "device — clearing stored WiFi credentials"); + if (!config.clearWifiCredentials()) { + ESP_LOGW(TAG, "failed to clear WiFi credentials " + "(continuing to provisioning)"); + } + } // Bring up the SoftAP radio BEFORE the portal starts serving so the // page is reachable over the air the moment it is registered (T018). // Credential VALUES are never logged (FR-004): the AP SSID is not a diff --git a/firmware/test_apps/host/main/test_wifi.cpp b/firmware/test_apps/host/main/test_wifi.cpp index 976f625..4c12e8f 100644 --- a/firmware/test_apps/host/main/test_wifi.cpp +++ b/firmware/test_apps/host/main/test_wifi.cpp @@ -383,6 +383,92 @@ static void test_wifi_isolation_no_block_no_watering_dep(void) TEST_ASSERT_EQUAL_UINT8(0, snap.consecutiveFailures); } +// =========================================================================== +// US3 — recovery paths (T022/T023). +// =========================================================================== + +// --------------------------------------------------------------------------- +// T022 — no boot loop under permanent failure (FR-013). An infinite +// ConnectFailed stream over many rounds keeps the machine in the failure cycle +// (Connecting / Reconnecting / ReconnectPaused) forever: it never reaches +// Connected, never falls back to Provisioning, never requests a restart (the +// pure WifiManager has no reboot surface at all), and its counters stay bounded +// (consecutiveFailures never exceeds failuresBeforePause — reset to 0 when the +// pause elapses; no overflow/UB). (contract §Timing 5) +// --------------------------------------------------------------------------- +static void test_wifi_no_boot_loop_under_permanent_failure(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + const ReconnectPolicy policy{}; // parity defaults (10 s / 5 / 60 s) + WifiManager manager(driver, config, clock, policy); + manager.begin(WifiBootMode::Station); // attempt #1 → Connecting + + // Drive 800 rounds of pure failure. Each round either fails an in-flight + // Connecting attempt or advances past whatever wait the machine is in + // (60 s covers both the 10 s retry and the 60 s pause), so the machine + // marches Connecting → Reconnecting → … → ReconnectPaused → Connecting + // indefinitely. One ReconnectPaused occurs roughly every 10 rounds (5 + // failures + their retries), so 800 rounds drives well over 50 retry/pause + // cycles. + int pausesSeen = 0; + for (int round = 0; round < 800; ++round) { + if (manager.snapshot().state == WifiState::Connecting) { + driver.scriptConnectFailure(); + } else { + clock.advance(60000); + } + manager.tick(); + + const WifiConnectionSnapshot snap = manager.snapshot(); + // Only the three failure-cycle states are ever legal here: no + // Connected (nothing ever succeeds) and no Provisioning (the machine + // never reboots into a different boot mode) — FR-013 no boot loop. + const bool inCycle = snap.state == WifiState::Connecting || + snap.state == WifiState::Reconnecting || + snap.state == WifiState::ReconnectPaused; + TEST_ASSERT_TRUE(inCycle); + // Counter never exceeds the pause threshold (bounded — no overflow). + TEST_ASSERT_TRUE(snap.consecutiveFailures <= policy.failuresBeforePause); + if (snap.state == WifiState::ReconnectPaused) { + ++pausesSeen; + } + } + + // The pause path was exercised many times (proving real retry/pause + // cycling, not a stuck state), and the machine is still in the failure + // cycle with a bounded counter at the end. + TEST_ASSERT_TRUE(pausesSeen >= 50); + const WifiConnectionSnapshot finalSnap = manager.snapshot(); + TEST_ASSERT_TRUE(finalSnap.state != WifiState::Connected && + finalSnap.state != WifiState::Provisioning); + TEST_ASSERT_TRUE(finalSnap.consecutiveFailures <= policy.failuresBeforePause); +} + +// --------------------------------------------------------------------------- +// T023 — emergency-reset boot decision + the paired clear-credentials intent. +// decideBootMode forces Provisioning when the config button is held on a +// configured device; the pure shouldClearCredentialsOnBoot helper (wired into +// app_main's provisioning branch, T025) is true ONLY when both hold, so a +// forced re-provisioning starts from a clean unconfigured state. (contract +// "Boot-mode contract" row 4 + data-model.md boot rule) +// --------------------------------------------------------------------------- +static void test_wifi_emergency_reset_clear_intent(void) +{ + // The emergency-reset entry: configured device + button held → Provisioning. + TEST_ASSERT_EQUAL(modeInt(WifiBootMode::Provisioning), + modeInt(decideBootMode(true, true))); + + // Clear-credentials intent: true only for (configured && button held). + TEST_ASSERT_TRUE(shouldClearCredentialsOnBoot(true, true)); // forced reset + TEST_ASSERT_FALSE(shouldClearCredentialsOnBoot(true, false)); // normal station + TEST_ASSERT_FALSE(shouldClearCredentialsOnBoot(false, true)); // nothing stored + TEST_ASSERT_FALSE(shouldClearCredentialsOnBoot(false, false)); // first boot +} + void run_wifi_tests(void) { // T008 — credential validation. @@ -399,4 +485,8 @@ void run_wifi_tests(void) RUN_TEST(test_wifi_ap_mode_suspends); // T016 — FR-014 isolation / non-blocking tick. RUN_TEST(test_wifi_isolation_no_block_no_watering_dep); + // T022 — no boot loop under permanent failure (FR-013). + RUN_TEST(test_wifi_no_boot_loop_under_permanent_failure); + // T023 — emergency-reset boot decision + clear-credentials intent. + RUN_TEST(test_wifi_emergency_reset_clear_intent); } diff --git a/specs/007-wifi-provisioning/checklists/hil.md b/specs/007-wifi-provisioning/checklists/hil.md new file mode 100644 index 0000000..1f3629e --- /dev/null +++ b/specs/007-wifi-provisioning/checklists/hil.md @@ -0,0 +1,105 @@ +# HIL Checklist: WiFi Provisioning & Station Management (007) — rev1 bench rig + +**Purpose**: hardware-in-the-loop verification of PR-07 at Checkpoint 3 (Paul, bench rig) +**Rig**: ESP32 devkit + status LED on GPIO2, config button on GPIO18 (to GND, internal pull-up ⇒ held == LOW); pumps, BME280 and RS485 soil sensor as wired for the previous features. A second WiFi AP (phone hotspot or a spare router you can power-cycle) plus a phone/laptop to reach the setup page. +**Build**: rev1 target (`sdkconfig.board.rev1_devkit`), flash per `firmware/CLAUDE.md` +**Reference**: acceptance criteria `docs/prd/PR-07-wifi-provisioning.md`; spec FR-004/FR-013/FR-014/FR-015; `quickstart.md` §HIL; parity `docs/parity-checklist.md` §7/§9 +**AP credentials**: setup SSID `CONFIG_WS_PROV_AP_SSID` (default `WateringSystem-Setup`), WPA2 password `CONFIG_WS_PROV_AP_PASSWORD` (documented non-secret, set at build). Never a legacy password. +**Note**: credential VALUES must never appear in the serial log or the portal response (FR-004) — watch for leaks while testing. + +## A. Fresh-device provisioning (US1, acceptance [HIL] #1) + +- [ ] A1. Flash a device with NO stored credentials (or run `config + wifi-clear` on the console, then reboot) → serial shows + `WiFi: provisioning mode (SoftAP setup portal)`; the SoftAP SSID + `WateringSystem-Setup` is visible from a phone +- [ ] A2. Join the AP (WPA2, `CONFIG_WS_PROV_AP_PASSWORD`); browse to + `http://192.168.4.1/` → the English setup page loads (SSID + + password form), self-contained, no external assets +- [ ] A3. Submit REAL home-WiFi credentials → success page indicating a + restart is required; device restarts (~3 s later, response delivered + first) and on reboot enters station mode and joins the LAN +- [ ] A4. Confirm the submitted password never appears in the serial log + or the HTTP response body (FR-004) +- [ ] A5. Invalid submits are rejected without persisting or restarting: + empty SSID → 4xx; a 1–7 char password → 4xx; device stays + provisionable (page still reachable) + +## B. AP power-cycle / outage isolation (US2, acceptance [HIL] #2, FR-014) + +- [ ] B1. With the device connected in station mode, start a watering- + relevant activity (e.g. `pump plant start 30`) and let the 5 s + environmental poll run +- [ ] B2. Cut the home AP (power it off) → the pump timed run and its + self-stop, and the `env`/`level` polling cadence, continue + UNAFFECTED throughout the outage (WiFi never touches watering) +- [ ] B3. Status LED toggles ~500 ms while the device is reconnecting; + serial shows the reconnect cadence (retry every 10 s; after 5 + consecutive failures an extra 60 s pause before the next round) +- [ ] B4. Restore the AP → device reconnects on schedule; LED goes steady + ON when the link is back; `wifi` console command reports + `state=Connected` with an RSSI +- [ ] B5. No watchdog resets, no missed pump self-stops across the whole + outage window + +## C. Config-button-at-boot forces provisioning (US3, acceptance [HIL] #3) + +- [ ] C1. On an ALREADY-configured device (station mode confirmed), power- + cycle while holding the config button (GPIO18) → serial shows the + "config button down at boot — hold 5000 ms" message and the status + LED blinks every 100 ms during the hold +- [ ] C2. Keep holding ≥ 5 s → serial shows "config button held … forcing + WiFi provisioning" then "clearing stored WiFi credentials"; device + enters provisioning mode (AP up), credentials cleared +- [ ] C3. Release the button BEFORE 5 s on a configured device → serial + shows "config button released early — not forcing provisioning"; + device proceeds to station mode with credentials intact (no clear) +- [ ] C4. Normal boot (button not pressed) proceeds immediately with no + startup delay and no LED blink from the button path + +## D. Wrong-password / no boot loop (US3, acceptance [HIL] #4, FR-013) + +- [ ] D1. Provision a WRONG home-WiFi password (valid length, wrong value) + → device keeps retrying on the 10 s / +60 s-after-5 schedule and + NEVER reboots (no boot loop): the serial banner appears exactly once, + not repeatedly +- [ ] D2. The device stays reachable/recoverable: hold the config button at + boot (section C) to force provisioning and re-enter correct + credentials → joins the LAN + +## E. Console / portal credential equivalence (research D10) + +- [ ] E1. Set credentials via the diag console `config wifi ` + then reboot → device reaches the same connected state as a + portal-provisioned device (`config get` shows `wifi=configured`; + values not echoed) +- [ ] E2. `config wifi-clear` returns the device to the unconfigured state + → next boot enters provisioning (parity with a fresh device) + +## F. LED patterns (parity §7/§9) + +- [ ] F1. Connect-attempt (Connecting/Reconnecting): status LED toggles + ~500 ms +- [ ] F2. Config-button hold: status LED blinks every 100 ms during the + hold window (distinct from the 500 ms connect toggle) +- [ ] F3. Connected: LED steady ON; ReconnectPaused: LED off + +## G. Brownout watch (QUIRK 4) + +- [ ] G1. During the AP power-cycle test (section B), watch for brownout- + detector-induced resets on the rev1 devkit with brownout detection + left ENABLED. Record whether any WiFi power-spike reset is observed — + if reproduced, note it (the legacy firmware disabled the brownout + detector to mask this; the ESP-IDF target keeps it on, QUIRK 4) + +## Deferral path + +If the rig or a spare/power-cyclable AP is unavailable, defer the affected +items with a written rationale and add them to the deferred-HIL register +(mirroring the handover's deferred-HIL pattern, e.g. PR-04's +`hil-004`). The host-test suite (`test_wifi.cpp`) already covers the +reconnect schedule, boot-mode truth table, credential validation, +isolation and the no-boot-loop property deterministically; the items above +are the hardware-only confirmations. + +**Sign-off**: date + result per item as a PR comment (pattern from PR #7). From 4e97becc5bc8d6686b023c5f91b0644e2cd740a2 Mon Sep 17 00:00:00 2001 From: Cryptotomte Date: Sat, 4 Jul 2026 16:20:29 +0200 Subject: [PATCH 7/7] =?UTF-8?q?fix(network):=20PR-07=20CP3=20review=20fixe?= =?UTF-8?q?s=20=E2=80=94=20reconnect/init=20robustness=20+=20diagnostics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CP3 review findings (all verified, host 178/0, both boards green): - F1: WifiManager routes synchronous staConnect() failure through handleFailure so the reconnect cadence advances instead of wedging in Connecting forever - F4: handleFailure ignores stray failure events while ReconnectPaused (counter stays <= failuresBeforePause; 60s deadline not re-armed) - F3: EspWifiDriver::init() unwinds cleanly on event-handler registration failure (unregister handler, esp_wifi_deinit, delete queue) — no leak/dangling handler - F2: app_main gates provisioning AND station bring-up on WiFi-init success; wifi_manager stays null on failure (console reports 'not available') - F5: correct the WifiConnectionSnapshot concurrency comment (no mutex exists; single-writer + benign cross-task diagnostic read); TODO(PR-09) LockedWifiManager - F6: qualify credential-never-logged citations to (PR-06 FR-004) - M1: count + WARN dropped WiFi events (queue full drops the newest, corrected) - M2: log a terminal 'provisioning unavailable' line if AP/portal start fails - M3: distinct 400 for a missing ssid form field (vs length error) - host tests: synchronous-staConnect-failure cadence + paused-ignores-stray-disconnect Known minor (out of scope, pre-existing, bounded): STA/AP netifs are not destroyed on the init failure paths (idempotent once-called static; WiFi-unavailable outcome). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../network/include/network/EspWifiDriver.h | 2 + .../network/include/network/WifiState.h | 11 +- .../components/network/src/EspWifiDriver.cpp | 40 ++++-- .../network/src/ProvisioningPortal.cpp | 8 +- .../components/network/src/WifiManager.cpp | 20 ++- firmware/main/app_main.cpp | 41 ++++-- firmware/test_apps/host/main/test_wifi.cpp | 119 ++++++++++++++++++ 7 files changed, 217 insertions(+), 24 deletions(-) diff --git a/firmware/components/network/include/network/EspWifiDriver.h b/firmware/components/network/include/network/EspWifiDriver.h index a97eb64..f704bf4 100644 --- a/firmware/components/network/include/network/EspWifiDriver.h +++ b/firmware/components/network/include/network/EspWifiDriver.h @@ -74,6 +74,8 @@ class EspWifiDriver : public IWifiDriver { void* eventQueue_ = nullptr; ///< QueueHandle_t of WifiEvent (thread-safe) void* staNetif_ = nullptr; ///< esp_netif_t* for the STA interface void* apNetif_ = nullptr; ///< esp_netif_t* for the AP interface + void* wifiHandlerInstance_ = nullptr; ///< esp_event_handler_instance_t (WIFI_EVENT) + void* ipHandlerInstance_ = nullptr; ///< esp_event_handler_instance_t (IP_EVENT) bool initialized_ = false; ///< true once init() succeeded bool started_ = false; ///< true between esp_wifi_start and stop }; diff --git a/firmware/components/network/include/network/WifiState.h b/firmware/components/network/include/network/WifiState.h index bd86217..b085bec 100644 --- a/firmware/components/network/include/network/WifiState.h +++ b/firmware/components/network/include/network/WifiState.h @@ -35,10 +35,15 @@ enum class WifiState { }; /** - * @brief Immutable status copy for status/LED consumers. + * @brief Point-in-time, by-value status copy for status/LED consumers. * - * Produced by WifiManager::snapshot() in a single mutex acquisition - * (research D9) so consumers see a consistent view of state + counters. + * Produced by WifiManager::snapshot() as a plain by-value copy of the state + + * counters. WifiManager is unsynchronized with a single writer (the wifi + * task's tick()); a cross-task diagnostic reader (the diag console) may observe + * a momentarily inconsistent tuple, which is acceptable for status display + * only. Consistent with WifiManager.h ("Unsynchronized by design"). + * TODO(PR-09): introduce a LockedWifiManager decorator when the HTTP status + * reader lands. */ struct WifiConnectionSnapshot { WifiState state; ///< current state machine state diff --git a/firmware/components/network/src/EspWifiDriver.cpp b/firmware/components/network/src/EspWifiDriver.cpp index 6b1ad7d..8c6bbdb 100644 --- a/firmware/components/network/src/EspWifiDriver.cpp +++ b/firmware/components/network/src/EspWifiDriver.cpp @@ -35,6 +35,11 @@ namespace { /// is plenty; the manager drains the whole queue every tick (250 ms). constexpr UBaseType_t kEventQueueLen = 8; +/// Diagnostic count of events dropped because the queue was full. Both event +/// handlers run on the single default event-loop task, so a plain counter is +/// race-free here. +uint32_t droppedEvents = 0; + /// Copy a std::string into a fixed-size, NUL-terminated esp_wifi field /// (ssid/password are uint8_t[] arrays in wifi_config_t). Never logs the /// content — callers decide what is safe to log. @@ -69,10 +74,14 @@ void wifiEventHandler(void *arg, esp_event_base_t /*base*/, int32_t id, QueueHandle_t queue = static_cast(arg); const WifiEvent event = translateWifiEvent(id); if (event != WifiEvent::None && queue != nullptr) { - // Non-blocking send: if the queue is somehow full the oldest news is - // simply the manager's problem to catch up on next tick — never block - // the event loop. - (void)xQueueSend(queue, &event, 0); + // Non-blocking send: xQueueSend(..., 0) drops the NEWEST event (this + // one) when the queue is full rather than blocking the event loop. + // A dropped lifecycle edge stays diagnosable via droppedEvents. + if (xQueueSend(queue, &event, 0) != pdTRUE) { + ++droppedEvents; + ESP_LOGW(TAG, "WiFi event queue full — dropped event (total %lu)", + static_cast(droppedEvents)); + } } } @@ -143,18 +152,33 @@ esp_err_t EspWifiDriver::init() // The handlers receive the queue handle as their arg, so they need no // access to the driver instance (keeps them file-local and header-clean). + // Capture the instance handles so a failure below can unregister cleanly. err = esp_event_handler_instance_register(WIFI_EVENT, ESP_EVENT_ANY_ID, - &wifiEventHandler, queue, nullptr); + &wifiEventHandler, queue, + &wifiHandlerInstance_); if (err != ESP_OK) { ESP_LOGE(TAG, "WIFI_EVENT handler register failed: %s", esp_err_to_name(err)); + // Unwind everything created so far: nothing is registered yet, so just + // tear down esp_wifi and the queue and stay uninitialized. + esp_wifi_deinit(); + vQueueDelete(queue); + eventQueue_ = nullptr; return err; } err = esp_event_handler_instance_register(IP_EVENT, IP_EVENT_STA_GOT_IP, - &ipEventHandler, queue, nullptr); + &ipEventHandler, queue, + &ipHandlerInstance_); if (err != ESP_OK) { ESP_LOGE(TAG, "IP_EVENT handler register failed: %s", esp_err_to_name(err)); + // The WIFI_EVENT handler is already registered against `queue`; drop it + // before the queue is deleted so no handler ever fires on a dead queue. + esp_event_handler_instance_unregister(WIFI_EVENT, ESP_EVENT_ANY_ID, + wifiHandlerInstance_); + esp_wifi_deinit(); + vQueueDelete(queue); + eventQueue_ = nullptr; return err; } @@ -193,7 +217,7 @@ bool EspWifiDriver::staConnect(const std::string &ssid, copyField(wc.sta.password, sizeof(wc.sta.password), password); err = esp_wifi_set_config(WIFI_IF_STA, &wc); if (err != ESP_OK) { - // Password value never logged (FR-004). + // Password value never logged (PR-06 FR-004). ESP_LOGE(TAG, "esp_wifi_set_config(STA) failed: %s", esp_err_to_name(err)); return false; @@ -257,7 +281,7 @@ bool EspWifiDriver::apStart(const std::string &ssid, password.empty() ? WIFI_AUTH_OPEN : WIFI_AUTH_WPA2_PSK; err = esp_wifi_set_config(WIFI_IF_AP, &wc); if (err != ESP_OK) { - // Password value never logged (FR-004). + // Password value never logged (PR-06 FR-004). ESP_LOGE(TAG, "esp_wifi_set_config(AP) failed: %s", esp_err_to_name(err)); return false; diff --git a/firmware/components/network/src/ProvisioningPortal.cpp b/firmware/components/network/src/ProvisioningPortal.cpp index 886f3f7..e01d9f4 100644 --- a/firmware/components/network/src/ProvisioningPortal.cpp +++ b/firmware/components/network/src/ProvisioningPortal.cpp @@ -191,7 +191,13 @@ esp_err_t wifiConfigPostHandler(httpd_req_t* req) std::string ssid; std::string password; - formField(body, "ssid", ssid); + // A missing ssid key is a malformed form (not a too-short SSID): report it + // distinctly rather than falling through to the credential length error. + if (!formField(body, "ssid", ssid)) { + ESP_LOGW(TAG, "rejecting form: ssid field missing"); + return sendHtml(req, "400 Bad Request", + "Invalid form submission: the ssid field is missing."); + } formField(body, "password", password); // absent = open network (empty) // Validate with the same pure rule the host tests cover. Never log the diff --git a/firmware/components/network/src/WifiManager.cpp b/firmware/components/network/src/WifiManager.cpp index 018915b..a21d9f5 100644 --- a/firmware/components/network/src/WifiManager.cpp +++ b/firmware/components/network/src/WifiManager.cpp @@ -96,11 +96,16 @@ void WifiManager::startConnect() { const std::string ssid = config_.getWifiSsid(); const std::string password = config_.getWifiPassword(); - // Non-blocking: success/failure of the attempt arrives later as an event. - // A synchronous config error (return false) leaves us in Connecting; the - // machine never reboots, it just waits — the operator can re-provision. - driver_.staConnect(ssid, password); state_ = WifiState::Connecting; + // Non-blocking: on success the outcome arrives later as an event. A + // synchronous config error (staConnect returns false) never yields an + // event, so route it straight through the failure path — that advances the + // reconnect cadence (Connecting → Reconnecting → … → ReconnectPaused) and + // makes the error visible in consecutiveFailures instead of wedging the + // machine in Connecting forever. It still never reboots (FR-013). + if (!driver_.staConnect(ssid, password)) { + handleFailure(time_.nowMs()); + } } void WifiManager::handleEvent(WifiEvent event) @@ -136,6 +141,13 @@ void WifiManager::handleEvent(WifiEvent event) void WifiManager::handleFailure(int64_t now) { + // A stray Disconnected/ConnectFailed that arrives while already paused must + // be ignored: it must not push consecutiveFailures past failuresBeforePause + // nor re-arm (delay) the pause deadline. Bail out before touching either. + if (state_ == WifiState::ReconnectPaused) { + return; + } + // A drop from an established connection is a diagnostic disconnect; // failures that occur while merely (re)connecting are not counted here. if (state_ == WifiState::Connected || ipAcquired_) { diff --git a/firmware/main/app_main.cpp b/firmware/main/app_main.cpp index abc4a4d..e6391a5 100644 --- a/firmware/main/app_main.cpp +++ b/firmware/main/app_main.cpp @@ -297,7 +297,11 @@ extern "C" void app_main(void) esp_err_to_name(netif_err)); } esp_err_t event_err = esp_event_loop_create_default(); - if (event_err != ESP_OK) { + if (event_err == ESP_ERR_INVALID_STATE) { + // The default loop already exists (created elsewhere) — that is a + // usable state, not a failure, so treat it as success below. + event_err = ESP_OK; + } else if (event_err != ESP_OK) { ESP_LOGE(TAG, "default event loop create failed: %s (WiFi unavailable)", esp_err_to_name(event_err)); } @@ -357,19 +361,32 @@ extern "C" void app_main(void) esp_err_to_name(wifi_init_err)); } + // WiFi is usable only when the full bring-up chain succeeded: the TCP/IP + // stack, the default event loop (already-created counts as success) and the + // driver init. If any step failed, both provisioning and station bring-up + // are skipped below — no apStart, no WifiManager, no wifi task — so the + // system runs headless. WiFi never touches the watering path (FR-014). + const bool wifi_stack_ok = (netif_err == ESP_OK) && + (event_err == ESP_OK) && + (wifi_init_err == ESP_OK); + // Set later in the station branch; used both to start the wifi task and to // register the manager with the diag console below. Stays nullptr in - // provisioning mode (the console `wifi` command then reports unavailable). + // provisioning mode and whenever WiFi init failed (the console `wifi` + // command then reports unavailable). WifiManager *wifi_manager = nullptr; - if (wifi_boot_mode == WifiBootMode::Provisioning) { + if (!wifi_stack_ok) { + ESP_LOGE(TAG, "WiFi unavailable (init failed) — skipping " + "station/provisioning bring-up"); + } else if (wifi_boot_mode == WifiBootMode::Provisioning) { ESP_LOGI(TAG, "WiFi: provisioning mode (SoftAP setup portal)"); // Emergency reset (US3/T025): when the config button forced // provisioning on an ALREADY-configured device, wipe the stored // credentials BEFORE the AP/portal comes up, so re-provisioning starts // from a clean unconfigured state and cannot silently keep the old // network (data-model.md boot rule). An unconfigured device has nothing - // to clear. Credential VALUES are never logged (FR-004). + // to clear. Credential VALUES are never logged (PR-06 FR-004). if (shouldClearCredentialsOnBoot(wifi_credentials_present, config_button_held)) { ESP_LOGI(TAG, "config button forced provisioning on a configured " @@ -381,10 +398,11 @@ extern "C" void app_main(void) } // Bring up the SoftAP radio BEFORE the portal starts serving so the // page is reachable over the air the moment it is registered (T018). - // Credential VALUES are never logged (FR-004): the AP SSID is not a - // secret, the WPA2 password is. - if (!wifi_driver.apStart(CONFIG_WS_PROV_AP_SSID, - CONFIG_WS_PROV_AP_PASSWORD)) { + // Credential VALUES are never logged (PR-06 FR-004): the AP SSID is not + // a secret, the WPA2 password is. + const bool ap_ok = wifi_driver.apStart(CONFIG_WS_PROV_AP_SSID, + CONFIG_WS_PROV_AP_PASSWORD); + if (!ap_ok) { ESP_LOGE(TAG, "SoftAP failed to start (portal may be unreachable)"); } // Function-local static (boot fail-safe rule: no non-trivial @@ -396,6 +414,13 @@ extern "C" void app_main(void) ESP_LOGE(TAG, "provisioning portal failed to start: %s", esp_err_to_name(prov_err)); } + // Terminal state: with neither the AP radio nor the portal up there is + // no way to enter credentials, and provisioning does not fall back to + // station. Say so explicitly — the only recovery is a power cycle. + if (!ap_ok || prov_err != ESP_OK) { + ESP_LOGE(TAG, "Provisioning unavailable (AP/portal failed) — " + "power-cycle to retry"); + } } else { ESP_LOGI(TAG, "WiFi: station mode (stored credentials present)"); // Reconnect timing from Kconfig (parity defaults, docs/parity- diff --git a/firmware/test_apps/host/main/test_wifi.cpp b/firmware/test_apps/host/main/test_wifi.cpp index 4c12e8f..9541ce3 100644 --- a/firmware/test_apps/host/main/test_wifi.cpp +++ b/firmware/test_apps/host/main/test_wifi.cpp @@ -383,6 +383,121 @@ static void test_wifi_isolation_no_block_no_watering_dep(void) TEST_ASSERT_EQUAL_UINT8(0, snap.consecutiveFailures); } +// --------------------------------------------------------------------------- +// F1 — a synchronous staConnect failure (driver returns false, never yields an +// event) is routed through the failure path, so the machine advances the +// reconnect cadence instead of wedging in Connecting with failures=0. +// (WifiManager::startConnect false-branch → handleFailure) +// --------------------------------------------------------------------------- +static void test_wifi_sync_staconnect_failure_advances_cadence(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + driver.staConnectResult = false; // every staConnect fails synchronously + + const ReconnectPolicy policy{}; // parity defaults (10 s / 5 / 60 s) + WifiManager manager(driver, config, clock, policy); + + // begin(Station) issues attempt #1; the synchronous false is routed straight + // through handleFailure, so the machine is already Reconnecting with one + // counted failure — NOT stuck in Connecting with failures=0. + manager.begin(WifiBootMode::Station); + TEST_ASSERT_EQUAL(1, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::Reconnecting), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT8(1, manager.snapshot().consecutiveFailures); + + // Each retry fails synchronously too, so consecutiveFailures climbs toward + // the pause threshold across successive intervals. + clock.advance(10000); + manager.tick(); // retry #2 fails → failures=2 + TEST_ASSERT_EQUAL(2, driver.staConnectCalls); + TEST_ASSERT_EQUAL_UINT8(2, manager.snapshot().consecutiveFailures); + + clock.advance(10000); + manager.tick(); // retry #3 → failures=3 + clock.advance(10000); + manager.tick(); // retry #4 → failures=4 + clock.advance(10000); + manager.tick(); // retry #5 → failures=5 → ReconnectPaused + TEST_ASSERT_EQUAL(5, driver.staConnectCalls); + + const WifiConnectionSnapshot snap = manager.snapshot(); + TEST_ASSERT_EQUAL(stateInt(WifiState::ReconnectPaused), stateInt(snap.state)); + TEST_ASSERT_EQUAL_UINT8(policy.failuresBeforePause, snap.consecutiveFailures); + // Never wedged in Connecting, never Connected, and never rebooted into + // Provisioning (the pure manager has no reboot surface). + TEST_ASSERT_TRUE(snap.state != WifiState::Connecting); + TEST_ASSERT_TRUE(snap.state != WifiState::Connected); + TEST_ASSERT_TRUE(snap.state != WifiState::Provisioning); +} + +// --------------------------------------------------------------------------- +// F4 — a stray Disconnected during ReconnectPaused is ignored: it neither pushes +// consecutiveFailures past the threshold nor delays the pause deadline. The +// fresh attempt still fires at the ORIGINAL deadline. (handleFailure paused +// guard) +// --------------------------------------------------------------------------- +static void test_wifi_paused_ignores_stray_disconnect(void) +{ + MockWifiDriver driver; + MockConfigStore config; + FakeTimeProvider clock; + seedCredentials(config); + + const ReconnectPolicy policy{}; // parity defaults (10 s / 5 / 60 s) + WifiManager manager(driver, config, clock, policy); + manager.begin(WifiBootMode::Station); // attempt #1 → Connecting + + // Drive 5 consecutive failures (10 s between retries) into ReconnectPaused. + // The 5th failure ticks at t = 4*10000 = 40000, so the fresh attempt is due + // at 40000 + pauseMs(60000) = 100000. + for (int i = 1; i <= 5; ++i) { + driver.scriptConnectFailure(); + manager.tick(); + if (i < 5) { + clock.advance(10000); + manager.tick(); // issues retry #(i+1) + } + } + TEST_ASSERT_EQUAL(stateInt(WifiState::ReconnectPaused), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT8(policy.failuresBeforePause, + manager.snapshot().consecutiveFailures); + TEST_ASSERT_EQUAL(5, driver.staConnectCalls); + + // Part-way into the pause (t = 70000), a stray Disconnected arrives. The + // paused guard bails before touching either counter or the deadline. + clock.advance(30000); + driver.queueEvent(WifiEvent::Disconnected); + manager.tick(); + TEST_ASSERT_EQUAL(stateInt(WifiState::ReconnectPaused), + stateInt(manager.snapshot().state)); + // Not 6 — never pushed past the threshold. + TEST_ASSERT_EQUAL_UINT8(policy.failuresBeforePause, + manager.snapshot().consecutiveFailures); + TEST_ASSERT_EQUAL(5, driver.staConnectCalls); + + // 1 ms short of the ORIGINAL deadline (t = 99999): still paused, still no + // attempt — proving the stray event did not push the deadline out by pauseMs. + clock.advance(29999); + manager.tick(); + TEST_ASSERT_EQUAL(5, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::ReconnectPaused), + stateInt(manager.snapshot().state)); + + // At the original deadline (t = 100000): exactly one fresh attempt, failures + // reset for the new round. + clock.advance(1); + manager.tick(); + TEST_ASSERT_EQUAL(6, driver.staConnectCalls); + TEST_ASSERT_EQUAL(stateInt(WifiState::Connecting), + stateInt(manager.snapshot().state)); + TEST_ASSERT_EQUAL_UINT8(0, manager.snapshot().consecutiveFailures); +} + // =========================================================================== // US3 — recovery paths (T022/T023). // =========================================================================== @@ -485,6 +600,10 @@ void run_wifi_tests(void) RUN_TEST(test_wifi_ap_mode_suspends); // T016 — FR-014 isolation / non-blocking tick. RUN_TEST(test_wifi_isolation_no_block_no_watering_dep); + // F1 — synchronous staConnect failure advances the reconnect cadence. + RUN_TEST(test_wifi_sync_staconnect_failure_advances_cadence); + // F4 — a stray Disconnected during ReconnectPaused is ignored. + RUN_TEST(test_wifi_paused_ignores_stray_disconnect); // T022 — no boot loop under permanent failure (FR-013). RUN_TEST(test_wifi_no_boot_loop_under_permanent_failure); // T023 — emergency-reset boot decision + clear-credentials intent.