PR-06: NVS configuration and LittleFS data storage

.github/workflows/firmware-build.yml

@@ -46,13 +46,16 @@ jobs:
 
       # if-no-files-found: error only fires when ZERO files match, so a
       # partial match could silently upload an incomplete artifact set.
-      # Verify all three binaries exist before uploading.
+      # Verify all binaries exist before uploading.
       - name: Verify all binaries exist
         run: |
           test -f firmware/build/wateringsystem.bin
           test -f firmware/build/bootloader/bootloader.bin
           test -f firmware/build/partition_table/partition-table.bin
           test -f firmware/build/ota_data_initial.bin
+          # littlefs image for the `storage` partition, built by
+          # littlefs_create_partition_image() (feature 003, FR-007/SC-006).
+          test -f firmware/build/storage.bin
 
       - name: Upload binaries
         uses: actions/upload-artifact@v4

.specify/feature.json

@@ -1,3 +1,3 @@
 {
-  "feature_directory": "specs/002-pump-gpio-board"
+  "feature_directory": "specs/003-nvs-littlefs-storage"
 }

docs/parity-checklist.md

@@ -172,6 +172,50 @@ Route list extracted from `src/communication/WateringSystemWebServer.cpp:83-321`
 - [ ] `[HOST]` Pruning of old readings exists in the storage API but is **never called** by the application — history grows until reads start failing on the 16 KB JSON parse buffer (`src/storage/LittleFSStorage.cpp:374-440`, buffer `:237`, `:298`). Target: equivalent or better retention behavior with explicit bounding (new storage design may differ internally; behavior coverage = history endpoint keeps working over time)
 - [ ] `[HOST]` Storage stats (total/used bytes) available and reported in `/status` and serial status (`src/storage/LittleFSStorage.cpp:442-459`)
 
+### Deliberate divergences in the ESP-IDF port (feature 003, PR-06)
+
+These are intentional behavior/format changes from the Arduino storage layer,
+not parity targets. The master PRD authorizes a clean redesign (no migration);
+each item below is the new contract behavior, host-tested in
+`firmware/test_apps/host/`.
+
+- [ ] `[HOST]` **Configuration moves from `/config.json` to NVS** (namespace
+  `wscfg`, one typed entry per item). The legacy string-keyed JSON blob is
+  gone; defaults are compiled in and applied on missing/erased/out-of-range
+  entries (FR-002/FR-013). Factory reset = erase the `nvs` partition. The seven
+  watering items keep their legacy defaults and ranges (section 1); divergence:
+  `sensorReadInterval`/`dataLogInterval` are now first-class settable items
+  (legacy persisted them but exposed no setter) with new lower bounds (≥1 s /
+  ≥1 min) to prevent log-storm misconfiguration.
+- [ ] `[HOST]` **WiFi "unconfigured" representation changes**: legacy used the
+  sentinel SSID `CONFIGURE_ME` in `/wifi_config.json`; the port stores
+  credentials in NVS and represents unconfigured as an **empty SSID string**
+  (factory state). PR-07 reads this for its AP-fallback decision; the legacy
+  password is never reused.
+- [ ] `[HOST]` **Sensor history format redesigned and explicitly bounded**:
+  per-metric append-only chunk files of fixed 8-byte records
+  (`/storage/hist/<metric>/<first_epoch>.dat`, 8 KiB chunks, max 10 chunks per
+  metric, oldest-chunk eviction), replacing the unbounded JSON arrays. Resolves
+  the legacy defect at line 172 (history grew until the 16 KB parse buffer
+  failed). Retention target ≥30 days for all metrics at the default log
+  interval; max 10 distinct metrics (sized to the legacy metric set), an 11th
+  is rejected. Range-query behavior (inclusive filter, empty result on
+  no-data/error) is preserved.
+- [ ] `[HOST]` **Event log is new surface** (no legacy equivalent): rotating
+  two-file log (`/storage/events/0.log`+`1.log`, 16 KiB each, oldest-half
+  rotation, newest always retained) for pump/fail-safe/connectivity/OTA/reset
+  events. Satisfies the constitution's "safety-relevant events MUST be
+  persisted"; producers are wired in PR-08.
+- [ ] `[HOST]` **Interface split**: the legacy single `IDataStorage`
+  (config + history + stats, Arduino `String`) is redesigned into `IConfigStore`
+  + `IDataStorage` (history + events + stats, `std::string`). The two
+  never-called legacy methods (`getLastSensorReading`, `pruneOldReadings`) are
+  dropped; bounded retention is an internal guarantee, not a caller obligation.
+- [ ] `[HOST]` Reservoir feature flags remain **not persisted** in this PR
+  (unchanged from legacy, line 171); the NVS-persistence decision is deferred to
+  PR-05 (reservoir board flag). The config store is extensible per-key so PR-05
+  can add them without a contract change.
+
 ## 7. WiFi / network / time
 
 - [ ] `[HIL]` STA connect at boot using saved credentials: STA mode, auto-reconnect off (handled manually), WiFi modem sleep disabled, clean disconnect first, **60 s** connect timeout with LED toggling every 500 ms (`src/main.cpp:46`, `168-212`)

firmware/CLAUDE.md

@@ -58,14 +58,23 @@ firmware/
 │   ├── board/                  # Board abstraction (header-only)
 │   │   └── include/board/board.h
 │   ├── interfaces/             # Header-only, NO IDF deps (host-includable)
-│   │   └── include/interfaces/ # IActuator, IWaterPump, ITimeProvider
-│   └── actuators/              # Pump drivers
-│       ├── include/actuators/  # WaterPump (pure C++ logic), GpioWaterPump,
-│       │                       # EspTimeProvider (esp32-only header),
-│       │                       # testing/ (MockWaterPump, FakeTimeProvider)
-│       └── src/                # GpioWaterPump.cpp excluded on linux target
+│   │   └── include/interfaces/ # IActuator, IWaterPump, ITimeProvider,
+│   │                           # IConfigStore, IDataStorage
+│   ├── actuators/              # Pump drivers
+│   │   ├── include/actuators/  # WaterPump (pure C++ logic), GpioWaterPump,
+│   │   │                       # EspTimeProvider (esp32-only header),
+│   │   │                       # testing/ (MockWaterPump, FakeTimeProvider)
+│   │   └── src/                # GpioWaterPump.cpp excluded on linux target
+│   └── storage/                # Config + data persistence (feature 003)
+│       ├── include/storage/    # NvsConfigStore, LittleFsDataStorage (POSIX,
+│       │                       # host-runnable), StorageMount (esp32-only),
+│       │                       # LockedConfigStore/LockedDataStorage,
+│       │                       # testing/ (MockConfigStore, MockDataStorage)
+│       └── src/                # StorageMount.cpp + littlefs REQUIRES excluded
+│                               # on linux target (esp_littlefs has no port)
 └── test_apps/
-    └── host/                   # Host test app (linux preview target, Unity)
+    └── host/                   # Host test app (linux preview target, Unity):
+                                # pump + config store + data storage suites
 ```
 
 Future components (drivers, controllers, web server) are added as siblings
@@ -97,6 +106,46 @@ pump <plant|reservoir> status
 pump status                              # both pumps
 ```
 
+Feature 003 adds `config` and `storage` subcommands (HIL verification path; the
+handlers are thin interface calls, no logic). Credential values are never echoed
+(FR-004):
+
+```
+config get | set <item> <value> | wifi <ssid> <password> | wifi-clear | factory-reset
+storage stats | log <metric> <value> | query <metric> [t0 t1] | event <category> <detail> | events [n]
+```
+
+## Storage (config + data persistence)
+
+Feature 003 (PR-06). Two redesigned, host-includable interfaces in
+`components/interfaces/`:
+
+- **`IConfigStore`** → `NvsConfigStore`: typed configuration in the `nvs`
+  partition (namespace `wscfg`, one entry per item), compiled-in factory
+  defaults applied on missing/erased/out-of-range entries (FR-013), explicit
+  `factoryReset()` (erases the partition). NVS runs natively on the linux
+  target, so the real store is host-tested — no mock skew.
+- **`IDataStorage`** → `LittleFsDataStorage`: bounded sensor history
+  (per-metric 8-byte-record chunk files, ring eviction, ≥30-day retention, max
+  10 metrics), a rotating event log (two 16 KiB files, newest always retained),
+  and filesystem usage stats. Implemented over **POSIX stdio with an injectable
+  base path**, so it builds and runs on the linux host; only `StorageMount`
+  (mount-or-format of the `storage` partition at `/storage`, `esp_littlefs_info`
+  stats) is esp32-only and excluded from the linux build via the component
+  CMakeLists `if(NOT ${IDF_TARGET} STREQUAL "linux")` guard.
+
+Concurrency: both base implementations are unsynchronized; anything accessed
+from more than one task (main loop + console REPL) is wrapped in
+`LockedConfigStore`/`LockedDataStorage` and accessed only through the wrapper —
+the same pattern as `LockedWaterPump`. A committed seed directory
+(`firmware/storage_image/`) feeds `littlefs_create_partition_image()`, which
+emits `build/storage.bin` on every build (CI verifies it exists). No Arduino
+data is migrated; on-disk formats diverge from legacy by design — see
+`docs/parity-checklist.md` §6 "Deliberate divergences".
+
+The diagnostic console (`ws>`) exposes `config` and `storage` subcommands for
+the HIL verification path (see below).
+
 ## Board abstraction
 
 Two board revisions exist, selected via Kconfig (`main/Kconfig.projbuild`):

firmware/components/interfaces/include/interfaces/IConfigStore.h

@@ -0,0 +1,167 @@
+// SPDX-FileCopyrightText: 2026 Cryptotomte
+// SPDX-License-Identifier: AGPL-3.0-or-later
+/**
+ * @file IConfigStore.h
+ * @brief Typed, validated access to persisted configuration.
+ *
+ * Replaces the config half of the legacy IDataStorage (string-keyed
+ * storeConfig/getConfig over one JSON blob) — a deliberate contract
+ * redesign, same approach as PR-02's IWaterPump. Normative contract:
+ * specs/003-nvs-littlefs-storage/contracts/IConfigStore.md; item schema,
+ * defaults and ranges: specs/003-nvs-littlefs-storage/data-model.md.
+ *
+ * Invariants:
+ *  1. A getter never returns an out-of-range value — the compiled-in
+ *     factory default shadows missing or invalid storage (FR-002).
+ *  2. A failed/rejected write never alters the stored value (FR-003):
+ *     old-or-new, never torn.
+ *  3. No operation blocks on or is affected by network state.
+ *  4. WiFi credential values never appear in any diagnostic or log
+ *     output (FR-004).
+ *
+ * Part of the header-only `interfaces` component: no IDF includes allowed
+ * (compiled on the host in the linux-target test suite).
+ */
+
+#ifndef WATERINGSYSTEM_INTERFACES_ICONFIGSTORE_H
+#define WATERINGSYSTEM_INTERFACES_ICONFIGSTORE_H
+
+#include <cstddef>
+#include <cstdint>
+#include <string>
+
+/**
+ * @brief Typed configuration store with compiled-in factory defaults.
+ *
+ * Implementations are unsynchronized by design; cross-task consumers wrap
+ * them in the Locked* decorator (research.md D9, PR-02 CP3 precedent).
+ */
+class IConfigStore {
+public:
+    // Factory defaults and valid ranges (data-model.md table) — the single
+    // source of truth shared by implementations, mocks and contract tests.
+    // Defaults mirror the frozen firmware (src/WateringController.cpp:16-21)
+    // except the documented divergences: the two interval items are settable
+    // and have new range floors (1 s / 1 min) against log-storm
+    // misconfiguration.
+    static constexpr float kMoistureThresholdMin = 0.0f;    ///< %
+    static constexpr float kMoistureThresholdMax = 100.0f;  ///< %
+    static constexpr float kDefaultMoistureThresholdLow = 30.0f;
+    static constexpr float kDefaultMoistureThresholdHigh = 55.0f;
+
+    static constexpr uint32_t kWateringDurationMinS = 1;
+    static constexpr uint32_t kWateringDurationMaxS = 300;
+    static constexpr uint32_t kDefaultWateringDurationS = 20;
+
+    static constexpr uint32_t kMinWateringIntervalFloorS = 1;
+    static constexpr uint32_t kDefaultMinWateringIntervalS = 300;
+
+    static constexpr bool kDefaultWateringEnabled = true;
+
+    static constexpr uint32_t kSensorReadIntervalFloorMs = 1000;
+    static constexpr uint32_t kDefaultSensorReadIntervalMs = 5000;
+
+    static constexpr uint32_t kDataLogIntervalFloorMs = 60000;
+    static constexpr uint32_t kDefaultDataLogIntervalMs = 300000;
+
+    static constexpr std::size_t kWifiSsidMaxLen = 32;      ///< bytes
+    static constexpr std::size_t kWifiPasswordMaxLen = 64;  ///< bytes
+
+    virtual ~IConfigStore() = default;
+
+    /**
+     * @brief Lower soil-moisture threshold in percent (0–100).
+     *
+     * Never fails: returns the stored value if present and in range,
+     * otherwise kDefaultMoistureThresholdLow.
+     */
+    virtual float getMoistureThresholdLow() const = 0;
+
+    /**
+     * @brief Set the lower soil-moisture threshold.
+     *
+     * In range (0–100, not NaN): persisted atomically, survives a power
+     * cycle, returns true. Out of range: returns false, stored value
+     * untouched. May also return false on a persistence failure.
+     */
+    virtual bool setMoistureThresholdLow(float percent) = 0;
+
+    /// Upper soil-moisture threshold in percent (0–100); same semantics.
+    virtual float getMoistureThresholdHigh() const = 0;
+
+    /// Set the upper soil-moisture threshold; same semantics as the low one.
+    virtual bool setMoistureThresholdHigh(float percent) = 0;
+
+    /// Watering run duration in seconds (1–300).
+    virtual uint32_t getWateringDurationS() const = 0;
+
+    /// Set the watering run duration; rejects values outside 1–300 s.
+    virtual bool setWateringDurationS(uint32_t seconds) = 0;
+
+    /// Minimum interval between watering runs (soak pause) in seconds (>= 1).
+    virtual uint32_t getMinWateringIntervalS() const = 0;
+
+    /// Set the soak pause; rejects 0.
+    virtual bool setMinWateringIntervalS(uint32_t seconds) = 0;
+
+    /// Whether automatic watering is enabled.
+    virtual bool getWateringEnabled() const = 0;
+
+    /// Enable/disable automatic watering (no range to violate; false only
+    /// on a persistence failure).
+    virtual bool setWateringEnabled(bool enabled) = 0;
+
+    /// Sensor read interval in milliseconds (>= 1000).
+    virtual uint32_t getSensorReadIntervalMs() const = 0;
+
+    /// Set the sensor read interval; rejects values below 1000 ms.
+    virtual bool setSensorReadIntervalMs(uint32_t ms) = 0;
+
+    /// Data log interval in milliseconds (>= 60000).
+    virtual uint32_t getDataLogIntervalMs() const = 0;
+
+    /// Set the data log interval; rejects values below 60000 ms.
+    virtual bool setDataLogIntervalMs(uint32_t ms) = 0;
+
+    /**
+     * @brief Stored WiFi SSID; empty string = unconfigured (factory state).
+     *
+     * Divergence from legacy: the Arduino firmware marked the unconfigured
+     * state with a CONFIGURE_ME sentinel in /wifi_config.json.
+     */
+    virtual std::string getWifiSsid() const = 0;
+
+    /// Stored WiFi password; empty string in the factory state.
+    virtual std::string getWifiPassword() const = 0;
+
+    /**
+     * @brief Store WiFi credentials as one pair.
+     *
+     * Length-validated (SSID <= 32 bytes, password <= 64 bytes); over-long
+     * input is rejected with false, stored values untouched.
+     * Implementations MUST NOT log the values (FR-004).
+     */
+    virtual bool setWifiCredentials(const std::string& ssid,
+                                    const std::string& password) = 0;
+
+    /// Return both credential items to the factory (empty) state.
+    virtual bool clearWifiCredentials() = 0;
+
+    /**
+     * @brief Factory reset: erase the underlying config storage.
+     *
+     * Afterwards every item reads its factory default and the credentials
+     * are removed (FR-005). The store remains usable without
+     * re-construction.
+     *
+     * WARNING: the NvsConfigStore implementation erases the ENTIRE default
+     * NVS partition (nvs_flash_erase_partition), not just the `wscfg`
+     * namespace. Any other namespace's data in that partition (future
+     * provisioning/OTA state, etc.) is therefore also cleared. This
+     * whole-partition erase is the documented data-model decision
+     * (data-model.md "Factory reset"), not an implementation accident.
+     */
+    virtual bool factoryReset() = 0;
+};
+
+#endif /* WATERINGSYSTEM_INTERFACES_ICONFIGSTORE_H */