feat: keep the config in NVS and add the cfg partition migration

.changes/config-nvs.md

@@ -0,0 +1,26 @@
+---
+kind: changed
+---
+Keep the configuration in NVS instead of a littlefs file
+
+## Release Note
+Settings are now stored in NVS
+
+The hub keeps its settings in the chip's NVS key-value store instead of a single config file, and saves each setting on its own when it changes. Your WiFi networks, account link and other settings from 1.x are imported automatically on the first boot after updating.
+
+- Each module owns its section of the config (`config/Section.h`); every section type meets the `SectionValue` concept, with its defaults defined in one place and its validation run on every value the section takes in
+- On the 2.x partition table the store is the `cfg` partition (the 1.x littlefs `config` partition retyped in place); on the 1.x table it lives in the shared `nvs` partition until the layout is migrated, and then moves into `cfg`
+- An empty store is filled once from the 1.x config file, which is never modified, otherwise from the defaults
+- WiFi credentials are stored one per key; the WiFi driver's own NVS store is compiled out
+- A section that can't be read runs on its defaults without erasing what is stored, and the save after a failed one rewrites every key; a single WiFi credential that doesn't read is dropped on its own, so the rest still load
+- The store is marked filled only once every section reads back from flash as written; a first fill cut short is redone on the next boot
+- A store written by newer firmware is loaded instead of reset, and a `cfg` partition in a newer NVS format is never erased
+- A section whose NVS namespace fails to open is a failed load, not a load of the defaults: the defaults are only taken when the namespace was never written. The store likewise never fills or cleans up when its own marker cannot be read
+- A boolean field of the wrong type in a `jsonconfig set` document rejects the document instead of storing the default, in every section
+- The config accessors return the value itself (a section always has one); the WiFi section hands a network saved without an id the lowest free one on every path, so `networks` over serial no longer allocates ids itself
+- An OTA check interval of zero and a hostname esp_netif would refuse become their defaults on every path (NVS, JSON and the setters), not only through the captive portal
+- `Config::MigrateToCfgPartition` retypes the `config` partition as `cfg`; nothing calls it yet
+
+## Notices
+- warning: **1.x firmware does not see settings changed after this update.** Downgrading to 1.x brings back the settings the hub had before updating, as long as the partition layout has not been migrated yet. Re-enter any WiFi networks or pairing you changed since.
+- warning: Until the partition layout is migrated the settings live in the chip's shared NVS partition. If that partition has to be erased (it fills up, or a newer firmware wrote it in a format this one cannot read), the hub imports the 1.x settings again and anything changed since the update is lost.

.github/actions/setup-esp-idf/action.yml

@@ -18,6 +18,13 @@ runs:
       shell: bash
       run: git submodule update --init --depth 1 components/flatbuffers/flatbuffers
 
+    # The install's Python venv is built on the runner image's Python, so an install from one image does not run on
+    # another (ubuntu-latest moving to a new release broke every job with "No module named 'rich_click'")
+    - name: Name the runner image
+      id: image
+      shell: bash
+      run: echo "name=${ImageOS:-unknown}" >> "$GITHUB_OUTPUT"
+
     # No restore-keys, a partial match would layer the installer over a stale tree. Bump the trailing number to invalidate.
     - name: Restore ESP-IDF install
       id: idf-cache
@@ -27,7 +34,7 @@ runs:
           /tmp/esp
           /opt/esp
           ~/.espressif
-        key: esp-idf-${{ runner.os }}-${{ inputs.idf-version }}-6
+        key: esp-idf-${{ runner.os }}-${{ steps.image.outputs.name }}-${{ inputs.idf-version }}-6
 
     - uses: espressif/install-esp-idf-action@8fc05d1470d5591417e7a3707a1f2bec178db4ae  # v1
       if: ${{ steps.idf-cache.outputs.cache-hit != 'true' }}
@@ -91,7 +98,7 @@ runs:
           /tmp/esp
           /opt/esp
           ~/.espressif
-        key: esp-idf-${{ runner.os }}-${{ inputs.idf-version }}-6
+        key: esp-idf-${{ runner.os }}-${{ steps.image.outputs.name }}-${{ inputs.idf-version }}-6
 
     - name: Put ccache on PATH
       shell: bash

.github/workflows/host-tests.yml

@@ -10,6 +10,7 @@ on:
       - 'test/**'
       - 'partitions/**'
       - 'CMakeLists.txt'
+      - 'cmake/**'
       - 'sdkconfig.defaults'
       - 'dependencies.lock'
       - '.gitmodules'

CMakeLists.txt

@@ -140,6 +140,7 @@ if(_os_idf_activated)
 endif()
 
 project(firmware)
+include(${CMAKE_CURRENT_LIST_DIR}/cmake/cxx_standard.cmake)
 
 # --- ccache by absolute path ---------------------------------------------------
 # idf.py configures with CCACHE_ENABLE, and ESP-IDF then launches compiles through a bare `ccache`,

cmake/cxx_standard.cmake

@@ -0,0 +1,10 @@
+# The C++ standard every OpenShock build compiles with. Included right after project() by the firmware's CMakeLists.txt
+# and by every components/*/host_test/CMakeLists.txt, so the firmware and its host tests speak the same language.
+#
+# ESP-IDF appends -std=gnu++26 for chip targets and the newest -std the host compiler accepts for the linux target.
+# GCC's C++26 mode is experimental and, from GCC 14 to 17, miscompiles a constexpr function that searches a string
+# (string_view::find and anything on top of it) when the call is folded with literal arguments: the folded value
+# disagrees with strict constant evaluation and with a runtime call (GCC bug 127126, -std=c++2c only). The device
+# toolchain is GCC 15.2, so the firmware is as exposed as the common host test that first showed it. This flag comes
+# after ESP-IDF's and the last -std on the command line wins. Drop it once the toolchain carries the fix.
+idf_build_set_property(CXX_COMPILE_OPTIONS "-std=gnu++23" APPEND)

components/captive_portal/CMakeLists.txt

@@ -9,14 +9,16 @@
 # esp_http_server and friends do not leak into consumers.
 idf_component_register(
   SRCS              "src/captiveportal/ApiHandlers.cpp"
+                    "src/captiveportal/CaptivePortalConfig.cpp"
                     "src/captiveportal/CaptivePortalInstance.cpp"
                     "src/captiveportal/HttpUtils.cpp"
                     "src/captiveportal/Manager.cpp"
                     "src/captiveportal/StaticFileServer.cpp"
                     "src/captiveportal/WebSocketEndpoint.cpp"
   INCLUDE_DIRS      "include"
   PRIV_INCLUDE_DIRS "src"
-  PRIV_REQUIRES     core config device_control                        # wifi/gateway/message handlers/AppHooks; Config::; CommandHandler/EStopManager
+  REQUIRES          config serialization                              # its config section (captiveportal/CaptivePortalConfig.h)
+  PRIV_REQUIRES     core device_control                        # wifi/gateway/message handlers/AppHooks; Config::; CommandHandler/EStopManager
                     common chipset logging temporal osjson serialization   # foundation utilities, json, HubToLocalMessage fbs
                     dns_server fs http hwutil rfc8908 www                # DNSServer, LfsPartition (1.x layout), ContentTypes, PartitionUtils, RFC8908Handler, the www image
                     esp_http_server esp_netif esp_timer esp_wifi

components/captive_portal/include/captiveportal/CaptivePortalConfig.h

@@ -0,0 +1,38 @@
+#pragma once
+
+// The `captivePortal` config section. Owned by captive_portal and kept in NVS through the config framework
+// (config/Section.h); namespace "portal", key always (u8).
+
+#include "config/Section.h"
+#include "serialization/_fbs/HubConfig_generated.h"
+
+#include <optional>
+
+namespace OpenShock::JSON {
+  class JsonView;
+}
+
+namespace OpenShock::Config {
+  struct CaptivePortalConfig {
+    CaptivePortalConfig();  // The defaults: the one place they are defined
+    CaptivePortalConfig(bool alwaysEnabled);
+
+    bool alwaysEnabled;
+
+    /// @brief No rules to apply yet. The section runs it on every value it takes in.
+    void Normalize() { }
+
+    [[nodiscard]] flatbuffers::Offset<Serialization::Configuration::CaptivePortalConfig> ToFlatbuffers(flatbuffers::FlatBufferBuilder& builder, bool withSensitiveData) const;
+
+    /// @brief Reads this section from `partition`; a missing key is its default, a read error fails.
+    bool LoadNvs(const char* partition);
+    /// @brief Writes the keys that differ from `previous` (every key when null) to `partition`.
+    bool SaveNvs(const char* partition, const CaptivePortalConfig* previous) const;
+
+    bool FromJSON(JSON::JsonView json);
+    void ToJSON(json_gen_str_t* gen, const char* name, bool withSensitiveData) const;
+  };
+
+  [[nodiscard]] CaptivePortalConfig GetCaptivePortalConfig();
+  bool SetCaptivePortalConfig(const CaptivePortalConfig& config);
+}  // namespace OpenShock::Config

components/captive_portal/src/captiveportal/ApiHandlers.cpp

@@ -11,12 +11,12 @@ const char* const TAG = "CaptivePortalApi";
 #include "captiveportal/HttpUtils.h"
 #include "captiveportal/Manager.h"
 #include "CommandHandler.h"
-#include "config/Config.h"
 #include "Convert.h"
 #include "enums/OtaUpdateChannel.h"
 #include "GatewayConnectionManager.h"
 #include "http/ContentTypes.h"
 #include "Logging.h"
+#include "OtaUpdateConfig.h"
 #include "RateLimiter.h"
 #include "util/TaskUtils.h"
 #include "wifi/WiFiManager.h"

components/captive_portal/src/captiveportal/CaptivePortalConfig.cpp

@@ -0,0 +1,83 @@
+#include <freertos/FreeRTOS.h>
+
+#include "captiveportal/CaptivePortalConfig.h"
+
+const char* const TAG = "Config::CaptivePortalConfig";
+
+#include "config/Nvs.h"
+#include "config/Parse.h"
+#include "Logging.h"
+
+#include "json/Json.h"
+
+using namespace OpenShock::Config;
+namespace Fbs = OpenShock::Serialization::Configuration;
+
+static constexpr const char* kNamespace = "portal";
+static constexpr const char* kKeyAlways = "always";
+
+static constexpr const char* kNamespaces[] = {kNamespace};
+static Section<CaptivePortalConfig> s_section({.name = "captivePortal", .order = 2, .hubConfigField = Fbs::HubConfig::VT_CAPTIVE_PORTAL, .namespaces = kNamespaces});
+
+CaptivePortalConfig::CaptivePortalConfig()
+  : alwaysEnabled(false)
+{
+}
+
+CaptivePortalConfig::CaptivePortalConfig(bool alwaysEnabled)
+  : alwaysEnabled(alwaysEnabled)
+{
+}
+
+flatbuffers::Offset<OpenShock::Serialization::Configuration::CaptivePortalConfig> CaptivePortalConfig::ToFlatbuffers(flatbuffers::FlatBufferBuilder& builder, bool withSensitiveData) const
+{
+  return Serialization::Configuration::CreateCaptivePortalConfig(builder, alwaysEnabled);
+}
+
+bool CaptivePortalConfig::FromJSON(JSON::JsonView json)
+{
+  if (auto done = Parse::ObjectOrDefault(*this, json, TAG)) {
+    return *done;
+  }
+
+  if (!Parse::JsonBool(alwaysEnabled, json, "alwaysEnabled", false, TAG)) {
+    return false;
+  }
+
+  return true;
+}
+
+void CaptivePortalConfig::ToJSON(json_gen_str_t* gen, const char* name, bool withSensitiveData) const
+{
+  JSON::objBegin(gen, name);
+  json_gen_obj_set_bool(gen, "alwaysEnabled", alwaysEnabled);
+  JSON::objEnd(gen, name);
+}
+
+bool CaptivePortalConfig::LoadNvs(const char* partition)
+{
+  auto handle = Nvs::Handle::OpenForLoad(partition, kNamespace);
+  if (!handle) {
+    return false;
+  }
+  return Nvs::Load(*handle, kKeyAlways, alwaysEnabled, false);
+}
+
+bool CaptivePortalConfig::SaveNvs(const char* partition, const CaptivePortalConfig* previous) const
+{
+  auto handle = Nvs::Handle::Open(partition, kNamespace, true);
+  if (!handle) {
+    return false;
+  }
+  return Nvs::Saver(*handle, *this, previous).Set(kKeyAlways, &CaptivePortalConfig::alwaysEnabled).Commit();
+}
+
+CaptivePortalConfig OpenShock::Config::GetCaptivePortalConfig()
+{
+  return s_section.Get();
+}
+
+bool OpenShock::Config::SetCaptivePortalConfig(const CaptivePortalConfig& config)
+{
+  return s_section.Set(config);
+}

components/captive_portal/src/captiveportal/Manager.cpp

@@ -7,13 +7,15 @@
 const char* const TAG = "CaptivePortal";
 
 #include "AppHooks.h"
+#include "BackendConfig.h"
+#include "captiveportal/CaptivePortalConfig.h"
 #include "captiveportal/CaptivePortalInstance.h"
 #include "CommandHandler.h"
-#include "config/Config.h"
 #include "FormatHelpers.h"
 #include "GatewayConnectionManager.h"
 #include "Logging.h"
 #include "Temporal.h"
+#include "wifi/WiFiConfig.h"
 
 #include <esp_mac.h>
 #include <esp_netif.h>
@@ -84,11 +86,7 @@ static void sendCommand(uint32_t command)
 
 static bool isDeviceFullyConfigured()
 {
-  auto credentialsList = Config::GetWiFiCredentials();
-  if (!credentialsList || credentialsList->empty()) {
-    return false;
-  }
-  return Config::HasBackendAuthToken();
+  return Config::HasWiFiCredentials() && Config::HasBackendAuthToken();
 }
 
 static std::shared_ptr<CaptivePortalInstance> GetInstance()
@@ -358,12 +356,9 @@ bool CaptivePortal::Init()
   auto* initialState = new ManagerState();
 
   // Restore the persisted always-enabled setting before the hook can change it, so it survives a reboot.
-  if (auto config = Config::GetCaptivePortalConfig()) {
-    initialState->alwaysEnabled = config->alwaysEnabled;
-    s_alwaysEnabled             = config->alwaysEnabled;
-  } else {
-    OS_LOGE(TAG, "Failed to get captive portal config");
-  }
+  const bool alwaysEnabled    = Config::GetCaptivePortalConfig().alwaysEnabled;
+  initialState->alwaysEnabled = alwaysEnabled;
+  s_alwaysEnabled             = alwaysEnabled;
 
   // If device is already fully configured, set a startup grace period before opening portal
   if (isDeviceFullyConfigured()) {

components/chipset/CMakeLists.txt

@@ -13,6 +13,6 @@ idf_component_register(
   SRCS          "src/BoardChecks.cpp"
   INCLUDE_DIRS  "include"
   REQUIRES      common          # Common.h, BoardDescriptor.h
-                esp_driver_gpio # driver/gpio.h
+                esp_hal_gpio    # hal/gpio_types.h (gpio_num_t, GPIO_IS_VALID_*)
   PRIV_REQUIRES logging         # OS_LOG* in BoardChecks.cpp
 )

components/chipset/include/Chipset.h

@@ -3,7 +3,7 @@
 #include "BoardDescriptor.h"
 #include "OpenShock.h"
 
-#include <driver/gpio.h>
+#include <hal/gpio_types.h>  // gpio_num_t, GPIO_IS_VALID_*: the types only, no driver
 
 #include <bitset>
 #include <limits>

components/common/host_test/CMakeLists.txt

@@ -13,3 +13,4 @@ list(APPEND EXTRA_COMPONENT_DIRS
      "${CMAKE_CURRENT_LIST_DIR}/../../../test/host_support")
 
 project(common_host_test)
+include(${CMAKE_CURRENT_LIST_DIR}/../../../cmake/cxx_standard.cmake)

components/config/CMakeLists.txt

@@ -1,20 +1,31 @@
-# OpenShock configuration model: typed config structs with JSON + flatbuffer
-# (de)serialization, persisted to littlefs via the fs component.
+# The config framework. It owns no settings: every module defines its own section (a value type plus a
+# Config::Section<T>, config/Section.h) and its own accessors, and keeps it in NVS through what is here:
 #
-# Extracted out of `core` into a low-layer component: it depends only on the
-# generated wire schema (serialization), JSON, the config filesystem and a few
-# foundation utilities; nothing in core. This is what lets device_control (estop/
-# radio/visual/CommandHandler, which read Config::) leave core in a later step
-# without a config <-> core cycle.
-FILE(GLOB_RECURSE config_sources
-  ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp)
-
+#   config/Nvs.h      typed access to one NVS namespace, the primitive every section's LoadNvs/SaveNvs uses
+#   config/Section.h  the section template: RAM cache, serialized writers, saving only what changed, the registry
+#   config/Store.h    picking the store (`cfg`, or the shared `nvs` on the 1.x layout), the move between them,
+#                     filling an empty store (the 1.x file, else defaults), factory reset, the whole-config JSON
+#   config/Wire.h     the whole config as the FlatBuffers HubConfig (local protocol Ready message, rawconfig), and
+#                     HubConfig -> JSON, which is also how the 1.x config file is imported
+#   config/Migration.h retyping the 1.x littlefs `config` partition as the `cfg` store (not wired up yet)
+#   config/Parse.h    field parsing helpers for sections' FromJSON
+#
+# The 1.x import (Store.cpp's readLegacyConfig, littlefs through fs) is 1.x compatibility code: it goes away on
+# master once the release/1.9 branch is cut.
 idf_component_register(
-  SRCS          ${config_sources}
+  SRCS          "src/Migration.cpp"
+                "src/Nvs.cpp"
+                "src/Parse.cpp"
+                "src/Section.cpp"
+                "src/Store.cpp"
+                "src/Wire.cpp"
   INCLUDE_DIRS  "include"
-  REQUIRES      common osjson serialization flatbuffers   # in public headers (enums, json/Json.h, HubConfig fbs, TinyVec, flatbuffers::Offset)
-                esp_wifi esp_hal_gpio                           # WiFiCredentials.h -> esp_wifi_types.h (wifi_auth_mode_t); RFConfig.h -> hal/gpio_types.h (gpio_num_t)
-  PRIV_REQUIRES chipset fs logging                              # .cpp only (GPIO validation, ConfigFs, OS_LOG*/OS_PANIC; SimpleMutex via common)
+  REQUIRES      common osjson                      # OpenShock.h, SimpleMutex.h, TinyVec.h, util/FunctionRef.h; json/Json.h and json_generator.h
+                nvs_flash                          # nvs.h in Nvs.h
+                serialization flatbuffers          # HubConfig in Wire.h
+                esp_hal_gpio                       # hal/gpio_types.h in Parse.h
+  PRIV_REQUIRES fs logging esp_partition           # LfsPartition (the 1.x import), OS_LOG*/OS_PANIC, the partition lookups
+                partition_layout esp_system        # the config partition retype, esp_restart
 )
 
 target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-error)

components/config/host_test/CMakeLists.txt

@@ -7,16 +7,22 @@ include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 set(COMPONENTS main)
 
 # No freertos mock: this suite runs on ESP-IDF's real FreeRTOS POSIX port, so the real
-# SimpleMutexes guard the Config:: store exactly as on the device. The driver mock
-# provides <driver/gpio.h> for the real Chipset.h, the esp_wifi mock the real
-# <esp_wifi_types.h> (wifi_auth_mode_t) - neither has a linux implementation; the
-# esp_wifi mock in turn needs the esp_netif and lwip mocks. Plus the shared host-test
-# stand-ins (Logging.h, openshock_board.h, CONFIG_OPENSHOCK_*).
+# SimpleMutexes guard the Config:: store exactly as on the device, and the real nvs_flash
+# and esp_partition (linux flash emulation) hold it. The esp_wifi mock provides the real
+# <esp_wifi_types.h> (wifi_auth_mode_t), which has no linux implementation; it in turn
+# needs the esp_netif and lwip mocks. Plus the shared host-test stand-ins (Logging.h,
+# openshock_board.h, CONFIG_OPENSHOCK_*).
 list(APPEND EXTRA_COMPONENT_DIRS
-     "$ENV{IDF_PATH}/tools/mocks/driver/"
      "$ENV{IDF_PATH}/tools/mocks/esp_wifi/"
      "$ENV{IDF_PATH}/tools/mocks/esp_netif/"
      "$ENV{IDF_PATH}/tools/mocks/lwip/"
      "${CMAKE_CURRENT_LIST_DIR}/../../../test/host_support")
 
 project(config_host_test)
+include(${CMAKE_CURRENT_LIST_DIR}/../../../cmake/cxx_standard.cmake)
+
+# glibc 2.42 (Ubuntu 26.04) pulls <linux/sockios.h> into <sys/ioctl.h>, so SIOCATMARK is already defined when lwip's
+# sockets.h defines it again under a guard that only checks SIOCSHIWAT. The netdb mock includes both, and -Werror
+# makes the redefinition a build failure. The mock is only here for esp_wifi_types.h's dependency chain.
+idf_component_get_property(_lwip_mock lwip COMPONENT_LIB)
+target_compile_options(${_lwip_mock} PRIVATE -Wno-error)