feat: Toggle and dim LED's via terminal and captive portal

.changes/rgb-led-dithering.md

@@ -0,0 +1,10 @@
+---
+kind: added
+---
+Smoother RGB LED dimming
+
+## Release Note
+The RGB status LED now dims and blends colors in finer steps. Before, low brightness settings left only a few distinct levels per color, so dimming jumped visibly and nearby colors looked the same; now there are four times as many.
+
+- The LED takes 8 bits per color; the driver shows 10 by repeating 4 frames that alternate between the two nearest levels, looped by the RMT hardware so it costs no CPU (ESP32 and ESP32-S3; the ESP32-C3 lacks the RMT memory and stays at 8 bits)
+- On by default; `CONFIG_OPENSHOCK_RGB_LED_DITHERING` turns it off

.changes/status-led-toggle.md

@@ -0,0 +1,13 @@
+---
+kind: added
+---
+Turn the status LEDs off or dim them
+
+## Release Note
+The status LEDs can now be set to one of three modes: show every state (the default), light up only for an active E-Stop or a critical error, or stay off entirely. Their brightness can also be lowered, set separately for the built-in LED and the RGB LED, and separately for critical states (E-Stop, errors) and everything else.
+
+Change these with the `led mode` and `led brightness` serial commands, or in the captive portal's new LEDs section, which also lets you preview each state's LED pattern and try colors on the RGB LED. Only the LEDs your board has are shown. The portal shows what the LEDs are showing right now. Brightness you move but don't save is shown on the LEDs and goes back to the saved value after 10 seconds without changes. A color preview ends 2 seconds after you stop moving the picker.
+
+- Brightness is stored as 16 bits (0-65535) of the LED's full output; the portal shows it as a percent. Defaults match previous firmware exactly: built-in LED 65535 (100%), RGB LED 5140 (20/255, 7.8%); boards can change them in Kconfig (`OPENSHOCK_LED_*_BRIGHTNESS`)
+- Setting the RGB LED above 30% logs a warning
+- On boards with both LEDs, the built-in LED now also shows an E-Stop or a critical error, not only "connected"

components/captive_portal/CMakeLists.txt

@@ -25,4 +25,4 @@ idf_component_register(
 )
 
 # Match core: IDF compiles application components with -Werror, the legacy build did not.
-target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-error)
+target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-error -Werror=switch)  # -Wswitch stays fatal: exhaustive enum switches rely on it

components/captive_portal/src/captiveportal/Manager.cpp

@@ -11,9 +11,13 @@ const char* const TAG = "CaptivePortal";
 #include "captiveportal/CaptivePortalConfig.h"
 #include "captiveportal/CaptivePortalInstance.h"
 #include "CommandHandler.h"
+#include "events/Events.h"
+#include "FormatHelpers.h"
 #include "GatewayConnectionManager.h"
 #include "Logging.h"
+#include "serialization/WSLocal.h"
 #include "Temporal.h"
+#include "visual/LedOverrides.h"
 #include "wifi/AccessPoint.h"
 #include "wifi/WiFiConfig.h"
 
@@ -453,6 +457,19 @@ static void captiveportal_managertask(void* arg)
   }
 }
 
+// Open portals get the LED settings and status once on connect; a change from anywhere (another client, the serial
+// console, an override timing out) is sent to them as it happens. Broadcasting with no portal open is a no-op.
+static void handleLedConfigChanged(void*, esp_event_base_t, int32_t, void*)
+{
+  Serialization::Local::SerializeLedSettingsChangedEvent(CaptivePortal::BroadcastMessageBIN);
+}
+
+static void handleLedStatusChanged(void*, esp_event_base_t, int32_t, void* eventData)
+{
+  const auto& notice = *static_cast<const VisualStateManager::LedStatusNotice*>(eventData);
+  Serialization::Local::SerializeLedStatusEvent(notice, CaptivePortal::BroadcastMessageBIN);
+}
+
 // esp_timer callback (runs on the esp_timer task, 3.5KB stack). Must stay trivial:
 // it only kicks the manager task, which does the real work on its own stack.
 static void captiveportal_timernotify(void*)
@@ -492,6 +509,17 @@ bool CaptivePortal::Init()
 
   AppHooks::RegisterCaptivePortal(CaptivePortal::BroadcastMessageBIN, CaptivePortal::SetAlwaysEnabled, CaptivePortal::IsRunning);
 
+  esp_err_t err = esp_event_handler_register(OPENSHOCK_EVENTS, OPENSHOCK_EVENT_LED_CONFIG_CHANGED, handleLedConfigChanged, nullptr);
+  if (err != ESP_OK) {
+    // Portals still get the settings on connect
+    OS_LOGE(TAG, "Failed to register for LED settings changes: %s", esp_err_to_name(err));
+  }
+  err = esp_event_handler_register(OPENSHOCK_EVENTS, OPENSHOCK_EVENT_LED_STATUS_CHANGED, handleLedStatusChanged, nullptr);
+  if (err != ESP_OK) {
+    // Portals still get the status on connect
+    OS_LOGE(TAG, "Failed to register for LED status changes: %s", esp_err_to_name(err));
+  }
+
   esp_timer_create_args_t args = {
     .callback              = captiveportal_timernotify,
     .arg                   = nullptr,
@@ -500,8 +528,6 @@ bool CaptivePortal::Init()
     .skip_unhandled_events = true,
   };
 
-  esp_err_t err;
-
   err = esp_timer_create(&args, &s_captivePortalUpdateLoopTimer);
   if (err != ESP_OK) {
     OS_LOGE(TAG, "Failed to create captive portal update timer");

components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp

@@ -9,6 +9,7 @@ const char* const TAG = "CaptivePortalWs";
 #include "message_handlers/WebSocket.h"
 #include "rfc8908/RFC8908Handler.h"
 #include "serialization/WSLocal.h"
+#include "visual/VisualStateManager.h"
 #include "wifi/WiFiManager.h"
 #include "wifi/WiFiScanManager.h"
 
@@ -326,7 +327,7 @@ void CaptivePortal::WebSocketEndpoint::handleWebSocketClientConnected(httpd_req_
     connectedNetworkPtr = &connectedNetwork;
   }
 
-  Serialization::Local::SerializeReadyMessage(connectedNetworkPtr, GatewayConnectionManager::IsLinked(), sendBin);
+  Serialization::Local::SerializeReadyMessage(connectedNetworkPtr, GatewayConnectionManager::IsLinked(), socketId, sendBin);
 
   // Send all previously scanned wifi networks
   auto networks = OpenShock::WiFiManager::GetDiscoveredWiFiNetworks();
@@ -341,6 +342,10 @@ void CaptivePortal::WebSocketEndpoint::handleWebSocketClientConnected(httpd_req_
 void CaptivePortal::WebSocketEndpoint::handleWebSocketClientDisconnected(uint8_t socketId)
 {
   OS_LOGD(TAG, "WebSocket client #%u disconnected", socketId);
+
+  // A preview or unsaved brightness it left behind ends now rather than at its timeout. On the httpd task, like the
+  // connect handler, so no new client has taken the id yet.
+  VisualStateManager::ReleaseLedOverrides(socketId);
 }
 
 // ---------------------------------------------------------------------------

components/common/include/enums/StatusLed.h

@@ -0,0 +1,77 @@
+#pragma once
+
+#include "StringHelpers.h"
+
+#include <cstdint>
+#include <optional>
+#include <string_view>
+
+namespace OpenShock {
+  // Which states the status LEDs show
+  enum class LedMode : uint8_t {
+    All,           // Every state
+    CriticalOnly,  // Dark during normal operation; only an E-Stop or a critical error lights them
+    Off            // Always dark
+  };
+
+  enum class LedType : uint8_t {
+    Mono,  // The single-color built-in LED
+    Rgb    // The WS2812B RGB LED
+  };
+
+  // What a pattern signals: critical is an E-Stop or a critical error, normal is everything else
+  enum class LedSignal : uint8_t {
+    Critical,
+    Normal
+  };
+
+  // The name jsonconfig and the serial console use; TryParseLedMode reads it back.
+  [[nodiscard]] constexpr const char* LedModeName(LedMode mode)
+  {
+    switch (mode) {
+      case LedMode::All:
+        return "all";
+      case LedMode::CriticalOnly:
+        return "critical";
+      case LedMode::Off:
+        return "off";
+    }
+    return "";
+  }
+
+  [[nodiscard]] inline std::optional<LedMode> TryParseLedMode(std::string_view str)
+  {
+    if (StringIEquals(str, "all")) {
+      return LedMode::All;
+    }
+    if (StringIEquals(str, "critical")) {
+      return LedMode::CriticalOnly;
+    }
+    if (StringIEquals(str, "off")) {
+      return LedMode::Off;
+    }
+    return std::nullopt;
+  }
+
+  [[nodiscard]] inline std::optional<LedType> TryParseLedType(std::string_view str)
+  {
+    if (StringIEquals(str, "mono")) {
+      return LedType::Mono;
+    }
+    if (StringIEquals(str, "rgb")) {
+      return LedType::Rgb;
+    }
+    return std::nullopt;
+  }
+
+  [[nodiscard]] inline std::optional<LedSignal> TryParseLedSignal(std::string_view str)
+  {
+    if (StringIEquals(str, "critical")) {
+      return LedSignal::Critical;
+    }
+    if (StringIEquals(str, "normal")) {
+      return LedSignal::Normal;
+    }
+    return std::nullopt;
+  }
+}  // namespace OpenShock

components/common/include/util/PercentUtils.h

@@ -0,0 +1,27 @@
+#pragma once
+
+#include <concepts>
+#include <cstdint>
+#include <limits>
+
+namespace OpenShock::PercentUtils {
+  /// @brief `Percent` (0-100) of an unsigned type's full scale, rounded to nearest. The percent is a template argument
+  ///        so it can be checked with static_assert, and the function is consteval: the floating point math only ever
+  ///        runs in the compiler.
+  template<std::unsigned_integral T, double Percent>
+  consteval T OfMax()
+  {
+    static_assert(Percent >= 0.0 && Percent <= 100.0, "a percent is 0-100");
+    if constexpr (Percent == 100.0) {
+      // Exact: a 64-bit max rounds up to 2^64 as a double, which doesn't fit back in the type
+      return std::numeric_limits<T>::max();
+    } else {
+      return static_cast<T>(Percent * static_cast<double>(std::numeric_limits<T>::max()) / 100.0 + 0.5);
+    }
+  }
+
+  static_assert(OfMax<uint16_t, 0.0>() == 0);
+  static_assert(OfMax<uint16_t, 100.0>() == 65535);
+  static_assert(OfMax<uint8_t, 50.0>() == 128);
+  static_assert(OfMax<uint64_t, 100.0>() == std::numeric_limits<uint64_t>::max());
+}  // namespace OpenShock::PercentUtils

components/config/CMakeLists.txt

@@ -18,4 +18,4 @@ idf_component_register(
   PRIV_REQUIRES logging                            # OS_LOG*/OS_PANIC
 )
 
-target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-error)
+target_compile_options(${COMPONENT_LIB} PRIVATE -Wno-error -Werror=switch)  # -Wswitch stays fatal: exhaustive enum switches rely on it

components/config/host_test/main/CMakeLists.txt

@@ -20,6 +20,7 @@ idf_component_register(
                # Each module's section
                "${_components}/device_control/src/radio/RFConfig.cpp"
                "${_components}/device_control/src/estop/EStopConfig.cpp"
+               "${_components}/device_control/src/visual/LedConfig.cpp"
                "${_components}/core/src/wifi/WiFiConfig.cpp"
                "${_components}/core/src/wifi/WiFiCredentials.cpp"
                "${_components}/core/src/BackendConfig.cpp"
@@ -33,7 +34,7 @@ idf_component_register(
                "${_components}/common/src/StringHelpers.cpp"
   INCLUDE_DIRS "."
                "${_components}/config/include"                 # config/*.h
-               "${_components}/device_control/include"         # radio/RFConfig.h, estop/EStopConfig.h
+               "${_components}/device_control/include"         # radio/RFConfig.h, estop/EStopConfig.h, visual/LedConfig.h
                "${_components}/core/include"                   # wifi/WiFiConfig.h, BackendConfig.h, OtaUpdateConfig.h
                "${_components}/captive_portal/include"         # captiveportal/CaptivePortalConfig.h
                "${_components}/serial_console/include"         # serial_console/SerialInputConfig.h

components/config/host_test/main/config_test_helpers.cpp

@@ -16,7 +16,7 @@ namespace ConfigTest {
   bool Full::SaveNvs(const char* partition) const
   {
     return rf.SaveNvs(partition, nullptr) && wifi.SaveNvs(partition, nullptr) && captivePortal.SaveNvs(partition, nullptr) && backend.SaveNvs(partition, nullptr) && serialInput.SaveNvs(partition, nullptr) && otaUpdate.SaveNvs(partition, nullptr)
-        && estop.SaveNvs(partition, nullptr);
+        && estop.SaveNvs(partition, nullptr) && led.SaveNvs(partition, nullptr);
   }
 
   Full MakeSampleConfig()
@@ -36,6 +36,7 @@ namespace ConfigTest {
     config.serialInput   = Config::SerialInputConfig(false);
     config.otaUpdate     = Config::OtaUpdateConfig(false, "fw.example.org", OtaUpdateChannel::Develop, true, true, 120, false, true, 4242, OtaUpdateStep::Validating);
     config.estop         = Config::EStopConfig(true, GPIO_NUM_13, true);
+    config.led           = Config::LedConfig(LedMode::CriticalOnly, 40000, 0, 5140, 300);
 
     return config;
   }
@@ -89,6 +90,7 @@ namespace ConfigTest {
     config.serialInput   = Config::GetSerialInputConfig();
     config.otaUpdate     = Config::GetOtaUpdateConfig();
     config.estop         = Config::GetEStopConfig();
+    config.led           = Config::GetLedConfig();
     return config;
   }
 
@@ -137,5 +139,11 @@ namespace ConfigTest {
     TEST_ASSERT_EQUAL_MESSAGE(expected.estop.enabled, actual.estop.enabled, "estop.enabled");
     TEST_ASSERT_EQUAL_INT_MESSAGE(expected.estop.gpioPin, actual.estop.gpioPin, "estop.gpioPin");
     TEST_ASSERT_EQUAL_MESSAGE(expected.estop.latching, actual.estop.latching, "estop.latching");
+
+    TEST_ASSERT_EQUAL_INT_MESSAGE(static_cast<int>(expected.led.mode), static_cast<int>(actual.led.mode), "led.mode");
+    TEST_ASSERT_EQUAL_UINT16_MESSAGE(expected.led.monoCriticalBrightness, actual.led.monoCriticalBrightness, "led.monoCriticalBrightness");
+    TEST_ASSERT_EQUAL_UINT16_MESSAGE(expected.led.monoNormalBrightness, actual.led.monoNormalBrightness, "led.monoNormalBrightness");
+    TEST_ASSERT_EQUAL_UINT16_MESSAGE(expected.led.rgbCriticalBrightness, actual.led.rgbCriticalBrightness, "led.rgbCriticalBrightness");
+    TEST_ASSERT_EQUAL_UINT16_MESSAGE(expected.led.rgbNormalBrightness, actual.led.rgbNormalBrightness, "led.rgbNormalBrightness");
   }
 }  // namespace ConfigTest

components/config/host_test/main/config_test_helpers.h

@@ -13,6 +13,7 @@
 #include "OtaUpdateConfig.h"
 #include "radio/RFConfig.h"
 #include "serial_console/SerialInputConfig.h"
+#include "visual/LedConfig.h"
 #include "wifi/WiFiConfig.h"
 
 #include <cstddef>
@@ -34,7 +35,8 @@ namespace ConfigTest {
       inline constexpr const char* kPortal  = "portal";
       inline constexpr const char* kSerial  = "serial";
       inline constexpr const char* kOta     = "ota";
-      inline constexpr const char* kAll[]   = {kBackend, kWifi, kWifiCfg, kRf, kEstop, kPortal, kSerial, kOta};
+      inline constexpr const char* kLed     = "led";
+      inline constexpr const char* kAll[]   = {kBackend, kWifi, kWifiCfg, kRf, kEstop, kPortal, kSerial, kOta, kLed};
     }  // namespace Ns
     namespace Backend {
       inline constexpr const char* kDomain = "domain";
@@ -67,6 +69,7 @@ namespace ConfigTest {
     OpenShock::Config::SerialInputConfig serialInput;
     OpenShock::Config::OtaUpdateConfig otaUpdate;
     OpenShock::Config::EStopConfig estop;
+    OpenShock::Config::LedConfig led;
 
     // Every section's keys
     bool SaveNvs(const char* partition) const;

components/config/host_test/main/test_defaults.cpp

@@ -59,6 +59,11 @@ TEST_CASE("Default values come from Kconfig and the board descriptor", "[config]
   TEST_ASSERT_EQUAL_INT(OPENSHOCK_ESTOP_PIN, config.estop.gpioPin);
   TEST_ASSERT_FALSE(config.estop.enabled);
   TEST_ASSERT_FALSE(config.estop.latching);  // OPENSHOCK_ESTOP_LATCHING is 0 on the host-test board
+  TEST_ASSERT_EQUAL_INT(static_cast<int>(LedMode::All), static_cast<int>(config.led.mode));
+  TEST_ASSERT_EQUAL_UINT16(CONFIG_OPENSHOCK_LED_MONO_CRITICAL_BRIGHTNESS, config.led.monoCriticalBrightness);
+  TEST_ASSERT_EQUAL_UINT16(CONFIG_OPENSHOCK_LED_MONO_NORMAL_BRIGHTNESS, config.led.monoNormalBrightness);
+  TEST_ASSERT_EQUAL_UINT16(CONFIG_OPENSHOCK_LED_RGB_CRITICAL_BRIGHTNESS, config.led.rgbCriticalBrightness);
+  TEST_ASSERT_EQUAL_UINT16(CONFIG_OPENSHOCK_LED_RGB_NORMAL_BRIGHTNESS, config.led.rgbNormalBrightness);
 }
 
 TEST_CASE("An empty store reads as the defaults and Init writes nothing", "[config][defaults]")

components/config/host_test/main/test_roundtrip.cpp

@@ -1,6 +1,7 @@
 // Round trips: a config written by this firmware must read back unchanged after a reboot, through the NVS store
 // and through the jsonconfig (GetAsJSON / SaveFromJSON) path.
 #include <freertos/FreeRTOS.h>
+#include <utility>
 
 #include "unity.h"
 
@@ -41,8 +42,14 @@ TEST_CASE("Values set through the Config API survive a reload", "[config][roundt
   TEST_ASSERT_TRUE(Config::SetOtaUpdateStep(OtaUpdateStep::Updated));
   TEST_ASSERT_TRUE(Config::SetEStopGpioPin(GPIO_NUM_13));
   TEST_ASSERT_TRUE(Config::SetEStopEnabled(true));
+  TEST_ASSERT_TRUE(Config::UpdateLedConfig([](Config::LedConfig& led) {
+    led.mode                = LedMode::Off;
+    led.rgbNormalBrightness = 60000;  // Above 255: survives only as a u16
+    return true;
+  }));
 
   const Full before = Snapshot();
+  TEST_ASSERT_EQUAL_UINT16(60000, before.led.rgbNormalBrightness);
   TEST_ASSERT_EQUAL_UINT8_ARRAY(bssid, before.wifi.credentialsList[0].bssid.data(), 6);
 
   Reload();
@@ -76,12 +83,40 @@ TEST_CASE("Whole-section setters round trip through the store", "[config][roundt
   TEST_ASSERT_TRUE(Config::SetSerialInputConfig(sample.serialInput));
   TEST_ASSERT_TRUE(Config::SetOtaUpdateConfig(sample.otaUpdate));
   TEST_ASSERT_TRUE(Config::SetEStopConfig(sample.estop));
+  TEST_ASSERT_TRUE(Config::SetLedConfig(sample.led));
   AssertConfigEqual(sample, Snapshot());
 
   Reload();
   AssertConfigEqual(sample, Snapshot());
 }
 
+TEST_CASE("An unknown LED mode from a caller is reset to the default", "[config][roundtrip]")
+{
+  InitDefault();
+
+  // Not one of the LedMode values: a caller can hand the section anything
+  Config::LedConfig led = Config::GetLedConfig();
+  led.mode              = static_cast<LedMode>(7);
+  TEST_ASSERT_TRUE(Config::SetLedConfig(led));
+  TEST_ASSERT_EQUAL_UINT8(std::to_underlying(LedMode::All), std::to_underlying(Config::GetLedConfig().mode));
+
+  TEST_ASSERT_TRUE(Config::UpdateLedConfig([](Config::LedConfig& l) {
+    l.mode = static_cast<LedMode>(200);
+    return true;
+  }));
+  TEST_ASSERT_EQUAL_UINT8(std::to_underlying(LedMode::All), std::to_underlying(Config::GetLedConfig().mode));
+}
+
+TEST_CASE("An unknown OTA update channel from a caller is reset to stable", "[config][roundtrip]")
+{
+  InitDefault();
+
+  Config::OtaUpdateConfig ota = Config::GetOtaUpdateConfig();
+  ota.updateChannel           = static_cast<OtaUpdateChannel>(9);
+  TEST_ASSERT_TRUE(Config::SetOtaUpdateConfig(ota));
+  TEST_ASSERT_EQUAL_UINT8(std::to_underlying(OtaUpdateChannel::Stable), std::to_underlying(Config::GetOtaUpdateConfig().updateChannel));
+}
+
 TEST_CASE("Removed credentials are gone after a reload and their ids can be reused", "[config][roundtrip]")
 {
   InitDefault();

components/config/host_test/main/test_storage.cpp

@@ -20,7 +20,7 @@ TEST_CASE("The store is the cfg partition", "[config][storage]")
 
 TEST_CASE("Sections link themselves into the registry in document order", "[config][storage]")
 {
-  const char* expected[] = {"rf", "wifi", "captivePortal", "backend", "serialInput", "otaUpdate", "estop"};
+  const char* expected[] = {"rf", "wifi", "captivePortal", "backend", "serialInput", "otaUpdate", "estop", "led"};
   std::size_t i          = 0;
   for (auto* section = Config::SectionBase::First(); section != nullptr; section = section->Next(), ++i) {
     TEST_ASSERT_LESS_THAN_size_t(std::size(expected), i);