From 5c53cb86408b7498a5797a02245d5033db7b2e71 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 10:28:18 +0000 Subject: [PATCH 01/57] refactor(adapters): migrate MQTT, Web and UI to commands and snapshots Co-authored-by: stritti --- .../.openspec.yaml | 2 + .../design.md | 52 +++ .../proposal.md | 27 ++ .../tasks.md | 19 + .../tasks.md | 1 + src/ControllerCommandHandler.cpp | 349 ++++++++++++++++++ src/ControllerCommandHandler.hpp | 65 ++++ src/ControllerSnapshotPublisher.cpp | 156 ++++++++ src/ControllerSnapshotPublisher.hpp | 51 +++ src/MqttRuntimeCommandAdapter.cpp | 252 +++++++++++++ src/MqttRuntimeCommandAdapter.hpp | 56 +++ src/WebPortal.cpp | 65 +++- src/WebPortal.hpp | 8 + src/main.cpp | 70 +++- test/native/CMakeLists.txt | 24 +- test/native/mocks/DallasTemperatureNode.hpp | 25 +- test/native/mocks/ESP32TemperatureNode.hpp | 2 + test/native/mocks/OperationModeNode.hpp | 45 ++- test/native/mocks/TimeClientHelper.hpp | 25 +- .../tests/test_controller_command_handler.cpp | 197 ++++++++++ .../test_controller_snapshot_publisher.cpp | 206 +++++++++++ test/native/tests/test_main.cpp | 8 + .../test_mqtt_runtime_command_adapter.cpp | 141 +++++++ .../tests/test_web_runtime_commands.cpp | 183 +++++++++ 24 files changed, 1970 insertions(+), 59 deletions(-) create mode 100644 openspec/changes/adapter-command-snapshot-migration/.openspec.yaml create mode 100644 openspec/changes/adapter-command-snapshot-migration/design.md create mode 100644 openspec/changes/adapter-command-snapshot-migration/proposal.md create mode 100644 openspec/changes/adapter-command-snapshot-migration/tasks.md create mode 100644 src/ControllerCommandHandler.cpp create mode 100644 src/ControllerCommandHandler.hpp create mode 100644 src/ControllerSnapshotPublisher.cpp create mode 100644 src/ControllerSnapshotPublisher.hpp create mode 100644 src/MqttRuntimeCommandAdapter.cpp create mode 100644 src/MqttRuntimeCommandAdapter.hpp create mode 100644 test/native/tests/test_controller_command_handler.cpp create mode 100644 test/native/tests/test_controller_snapshot_publisher.cpp create mode 100644 test/native/tests/test_mqtt_runtime_command_adapter.cpp create mode 100644 test/native/tests/test_web_runtime_commands.cpp diff --git a/openspec/changes/adapter-command-snapshot-migration/.openspec.yaml b/openspec/changes/adapter-command-snapshot-migration/.openspec.yaml new file mode 100644 index 00000000..f98e139f --- /dev/null +++ b/openspec/changes/adapter-command-snapshot-migration/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-04 diff --git a/openspec/changes/adapter-command-snapshot-migration/design.md b/openspec/changes/adapter-command-snapshot-migration/design.md new file mode 100644 index 00000000..42059eb4 --- /dev/null +++ b/openspec/changes/adapter-command-snapshot-migration/design.md @@ -0,0 +1,52 @@ +# Design: Adapter command and snapshot migration + +## Ownership + +The Core-1 application loop is the only owner allowed to mutate controller/domain state and runtime controller settings. Adapter callbacks may parse/validate protocol input, but they must not mutate nodes, runtime settings or relay state directly. + +Provisioning/authentication data is deliberately separate: WiFi/MQTT credentials and admin-password flows remain owned by dedicated configuration/auth services and are not copied through the general controller command queue. Likewise, specialized calibration workflows remain behind their dedicated service API until the composition-root/configuration refactor gives them an explicit ownership boundary. + +## Inbound path + +```text +MQTT callback -> protocol validation -> ControllerCommandQueue +Web handler -> protocol validation -> ControllerCommandQueue +Local UI -> interaction mapping -> ControllerCommandQueue + -> Core-1 command handler -> state mutation +``` + +The typed path covers runtime controller mutations: mode, setpoints, circulation settings, independent timer start/end updates, loop interval, timezone/time-loss thresholds, NORVI button thresholds, sensor mappings and manual relay commands. + +For synchronous Web responses, the first migration may process the typed command immediately when the handler is already executing on the owning loop task. The command handler remains the single mutation API for that runtime state. + +## Outbound path + +```text +Core-1 state -> SystemSnapshot -> ControllerSnapshotStore + -> MQTT + -> Web status/config projections + -> OLED/TFT +``` + +Adapters receive value copies. They do not receive node pointers or mutable runtime-config references. The snapshot must preserve the existing external projection, including timer runtime state, sensor mapping identity/configured/found state and GREEN/YELLOW/RED time degradation. + +## Migration order + +1. Add a Core-1 command handler for the complete runtime `ControllerCommand` contract. +2. Route processed MQTT runtime commands through typed commands. +3. Route Web runtime mutations through the same handler. +4. Route local UI runtime mutations through the same handler. +5. Build and publish `SystemSnapshot` from the owning loop. +6. Migrate status/read paths one adapter at a time. +7. Remove adapter dependencies on `Nodes.hpp` and mutable runtime config where no longer needed. +8. Remove `MqttCommandQueue` after the typed queue owns callback hand-over. + +## Verification + +- native tests for command validation/dispatch and queue overflow +- regression tests for independent timer-start/timer-end updates +- existing MQTT/Web compatibility tests remain unchanged +- PlatformIO builds for all supported boards +- regression test proving MQTT callbacks cannot mutate operation mode or runtime settings directly +- grep/code-search gate showing migrated adapters no longer include `Nodes.hpp` for mutable state access +- verify provisioning/auth flows remain isolated from the general runtime command queue diff --git a/openspec/changes/adapter-command-snapshot-migration/proposal.md b/openspec/changes/adapter-command-snapshot-migration/proposal.md new file mode 100644 index 00000000..aafcdc24 --- /dev/null +++ b/openspec/changes/adapter-command-snapshot-migration/proposal.md @@ -0,0 +1,27 @@ +# Change: Migrate adapters to typed commands and snapshots + +## Why + +MQTT, Web and local UI still depend directly on global nodes and static managers. That keeps protocol code coupled to mutable application state and makes the single-writer ownership model from #170 difficult to enforce structurally. + +## What changes + +- translate inbound MQTT/Web/local-UI runtime-controller mutations into `ControllerCommand` values +- process runtime commands only on the Core-1 application owner +- preserve independent timer-start/timer-end update behavior +- publish a complete `SystemSnapshot` after relevant state changes +- make MQTT/Web/display runtime output consume snapshots rather than `Nodes.hpp` or mutable controller settings +- retain existing external MQTT topics, Web payloads and behavior +- keep WiFi/MQTT credentials, admin-password flows and specialized calibration workflows in their dedicated services rather than transporting secrets through the general controller queue +- remove `MqttCommandQueue` only after the typed command path has equivalent coverage and bounded behavior + +## Dependencies + +- #214 controller architecture contracts +- #216 typed command queue foundation +- #217 snapshot-store foundation +- #170 multicore ownership boundary for sensor I/O + +## Impact + +The external interfaces remain compatible. The change is internal ownership/coupling cleanup and should reduce the number of translation units that can access mutable nodes and runtime controller settings directly. diff --git a/openspec/changes/adapter-command-snapshot-migration/tasks.md b/openspec/changes/adapter-command-snapshot-migration/tasks.md new file mode 100644 index 00000000..775fe864 --- /dev/null +++ b/openspec/changes/adapter-command-snapshot-migration/tasks.md @@ -0,0 +1,19 @@ +# Tasks + +- [x] Introduce a single Core-1 controller command handler for the complete runtime command contract. +- [x] Route processed MQTT runtime commands through typed `ControllerCommand` values. +- [x] Preserve independent MQTT timer-start and timer-end updates without stale-state reconstruction. +- [x] Route `/api/mode` and `/api/pump` Web mutations through the same command handler using owner-resolved relative actions. +- [ ] Route the remaining Web runtime mutations through the same command handler. +- [ ] Route local UI runtime mutations through the same command handler. +- [x] Keep provisioning/authentication secrets and specialized calibration workflows behind their dedicated service boundaries. +- [x] Publish `SystemSnapshot` from the Core-1 owner after state updates. +- [x] Populate sensor mapping address/configured/found state in the snapshot. +- [x] Populate GREEN/YELLOW/RED time degradation and runtime controller settings in the snapshot. +- [ ] Migrate MQTT state/discovery reads to snapshot values where applicable. +- [ ] Migrate Web status/config read paths to snapshot values where applicable. +- [ ] Migrate OLED/TFT read paths to snapshot values. +- [ ] Remove direct mutable `Nodes.hpp` and runtime-setting access from migrated adapters. +- [ ] Remove `MqttCommandQueue` after typed callback hand-over has equivalent tests. +- [x] Add native regression tests for dispatch, overflow, partial timer updates and MQTT compatibility. +- [ ] Verify all PlatformIO environments and existing API/MQTT contract tests. diff --git a/openspec/changes/controller-architecture-contracts/tasks.md b/openspec/changes/controller-architecture-contracts/tasks.md index 3d4e209d..436f37fd 100644 --- a/openspec/changes/controller-architecture-contracts/tasks.md +++ b/openspec/changes/controller-architecture-contracts/tasks.md @@ -21,6 +21,7 @@ - [x] Project the detected inventory with a 20-device cap, ROMs and independent temperature validity. - [x] Add native tests for compatibility, text boundaries, full/empty inventory, diagnostics and copy semantics. - [x] Document the minimal dependency graph, current #218 progress, #170 ownership gate and sensor projection bridge. +- [x] Store operation mode internally as the typed enum while preserving String compatibility at boundaries. - [x] Add a bounded, allocation-free `ControllerCommandQueue` for adapter-to-loop hand-over. - [x] Add a synchronized `ControllerSnapshotStore` for complete read-model publication. - [ ] In #216/#217, verify the expanded payloads and transport resource budgets. diff --git a/src/ControllerCommandHandler.cpp b/src/ControllerCommandHandler.cpp new file mode 100644 index 00000000..dca46056 --- /dev/null +++ b/src/ControllerCommandHandler.cpp @@ -0,0 +1,349 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include "ControllerCommandHandler.hpp" + +#include +#include + +#include "ConfigManager.hpp" +#include "DallasTemperatureNode.hpp" +#include "ESP32TemperatureNode.hpp" +#include "LogCapture.hpp" +#include "OperationModeNode.hpp" +#include "RelayModuleNode.hpp" +#include "TimeClientHelper.hpp" + +#ifdef NORVI_AE01_R +#include "NorviButtonHandler.hpp" +#endif + +namespace PoolController { + +namespace { + +bool inRange(std::int32_t value, std::int32_t minimum, std::int32_t maximum) { + return value >= minimum && value <= maximum; +} + +bool inRange(float value, float minimum, float maximum) { + return value >= minimum && value <= maximum; +} + +OperationMode nextOperationMode(OperationMode current) { + switch (current) { + case OperationMode::AUTO: + return OperationMode::MANUAL; + case OperationMode::MANUAL: + return OperationMode::BOOST; + case OperationMode::BOOST: + return OperationMode::TIMER; + case OperationMode::TIMER: + return OperationMode::AUTO; + } + return OperationMode::AUTO; +} + +} // namespace + +const char *ControllerCommandHandler::sourceName(CommandSource source) { + switch (source) { + case CommandSource::MQTT: + return "mqtt:typed-command"; + case CommandSource::WEB: + return "web:typed-command"; + case CommandSource::LOCAL_UI: + return "local-ui:typed-command"; + case CommandSource::INTERNAL: + default: + return "internal:typed-command"; + } +} + +bool ControllerCommandHandler::saveSettings() const { + if (ConfigManager::save()) { + return true; + } + LOG_ERROR("Controller command: failed to persist runtime settings\n"); + return false; +} + +bool ControllerCommandHandler::applySensorMapping(const ControllerCommand &command, bool clear) { + std::uint8_t solarAddress[8] = {0}; + std::uint8_t poolAddress[8] = {0}; + ConfigManager::loadSensorMapping(solarAddress, poolAddress); + + std::uint8_t *target = command.sensorRole == SensorRole::SOLAR ? solarAddress : poolAddress; + for (std::size_t i = 0; i < command.sensorAddress.size(); ++i) { + target[i] = clear ? 0 : command.sensorAddress[i]; + } + + ConfigManager::saveSensorMapping(solarAddress, poolAddress); + + DallasTemperatureNode &node = + command.sensorRole == SensorRole::SOLAR ? dependencies_.solarTemperature : dependencies_.poolTemperature; + if (clear) { + node.clearAddressFilter(); + } else { + node.setAddressFilter(target); + } + return true; +} + +bool ControllerCommandHandler::handle(const ControllerCommand &command, ControllerCommandResult *result) { + auto &settings = ConfigManager::getSettings(); + + switch (command.type) { + case ControllerCommandType::SET_MODE: + if (!dependencies_.operationMode.setMode(command.mode, sourceName(command.source))) { + return false; + } + settings.opMode = toString(command.mode); + if (!saveSettings()) { + return false; + } + if (result != nullptr) { + result->hasMode = true; + result->mode = command.mode; + } + return true; + + case ControllerCommandType::CYCLE_MODE: { + const OperationMode next = nextOperationMode(dependencies_.operationMode.getTypedMode()); + if (!dependencies_.operationMode.setMode(next, sourceName(command.source))) { + return false; + } + settings.opMode = toString(next); + if (!saveSettings()) { + return false; + } + if (result != nullptr) { + result->hasMode = true; + result->mode = next; + } + return true; + } + + case ControllerCommandType::SET_POOL_MAX_TEMPERATURE: + if (!inRange(command.value, 0.0F, 40.0F)) { + return false; + } + dependencies_.operationMode.setPoolMaxTemperature(command.value); + settings.tempMaxPool = command.value; + return saveSettings(); + + case ControllerCommandType::SET_SOLAR_MIN_TEMPERATURE: + if (!inRange(command.value, 0.0F, 100.0F)) { + return false; + } + dependencies_.operationMode.setSolarMinTemperature(command.value); + settings.tempMinSolar = command.value; + return saveSettings(); + + case ControllerCommandType::SET_TEMPERATURE_HYSTERESIS: + if (!inRange(command.value, 0.0F, 10.0F)) { + return false; + } + dependencies_.operationMode.setTemperatureHysteresis(command.value); + settings.tempHysteresis = command.value; + return saveSettings(); + + case ControllerCommandType::SET_TEMPERATURE_CIRCULATION_THRESHOLD: + if (!inRange(command.value, 0.0F, 40.0F)) { + return false; + } + settings.tempCircThreshold = command.value; + return saveSettings(); + + case ControllerCommandType::SET_TEMPERATURE_CIRCULATION_FACTOR: + if (!inRange(command.integerValue, 0, 120)) { + return false; + } + settings.tempCircFactor = static_cast(command.integerValue); + return saveSettings(); + + case ControllerCommandType::SET_TEMPERATURE_CIRCULATION_MAX_RUNTIME: + if (!inRange(command.integerValue, 60, 1440)) { + return false; + } + settings.tempCircMaxRuntime = static_cast(command.integerValue); + return saveSettings(); + + case ControllerCommandType::SET_TIMER_START: { + if (command.hour > 23 || command.minute > 59) { + return false; + } + TimerSetting timer = dependencies_.operationMode.getTimerSetting(); + timer.timerStartHour = command.hour; + timer.timerStartMinutes = command.minute; + dependencies_.operationMode.setTimerSetting(timer); + return true; + } + + case ControllerCommandType::SET_TIMER_END: { + if (command.hour > 23 || command.minute > 59) { + return false; + } + TimerSetting timer = dependencies_.operationMode.getTimerSetting(); + timer.timerEndHour = command.hour; + timer.timerEndMinutes = command.minute; + dependencies_.operationMode.setTimerSetting(timer); + return true; + } + + case ControllerCommandType::SET_LOOP_INTERVAL: + if (!inRange(command.integerValue, 1, 3600)) { + return false; + } + settings.loopInterval = command.integerValue; + dependencies_.solarTemperature.setMeasurementInterval(command.integerValue); + dependencies_.poolTemperature.setMeasurementInterval(command.integerValue); + dependencies_.controllerTemperature.setMeasurementInterval(command.integerValue); + dependencies_.poolPump.setMeasurementInterval(command.integerValue); + dependencies_.solarPump.setMeasurementInterval(command.integerValue); + dependencies_.operationMode.setMeasurementInterval(command.integerValue); + return saveSettings(); + + case ControllerCommandType::SET_TIMEZONE: + if (!inRange(command.integerValue, 0, getTzCount() - 1)) { + return false; + } + settings.timezoneIndex = command.integerValue; + if (!saveSettings()) { + return false; + } + setTimezoneIndex(command.integerValue); + return true; + + case ControllerCommandType::SET_TIME_LOSS_GREEN_HOURS: + if (!inRange(command.integerValue, 0, 255)) { + return false; + } + settings.timeLossGreenHours = command.integerValue; + if (!saveSettings()) { + return false; + } + setTimeDegradationGreenHours(static_cast(command.integerValue)); + return true; + + case ControllerCommandType::SET_TIME_LOSS_RED_HOURS: + if (!inRange(command.integerValue, 1, 255)) { + return false; + } + settings.timeLossRedHours = command.integerValue; + if (!saveSettings()) { + return false; + } + setTimeDegradationRedHours(static_cast(command.integerValue)); + return true; + + case ControllerCommandType::SET_BUTTON_1_MIN: + case ControllerCommandType::SET_BUTTON_1_MAX: + case ControllerCommandType::SET_BUTTON_2_MIN: + case ControllerCommandType::SET_BUTTON_2_MAX: + case ControllerCommandType::SET_BUTTON_3_MIN: + case ControllerCommandType::SET_BUTTON_3_MAX: + case ControllerCommandType::SET_BUTTON_NO_PRESS: { + if (!inRange(command.integerValue, 0, 4096)) { + return false; + } + const std::uint16_t value = static_cast(command.integerValue); + switch (command.type) { + case ControllerCommandType::SET_BUTTON_1_MIN: + settings.btn1Min = value; + break; + case ControllerCommandType::SET_BUTTON_1_MAX: + settings.btn1Max = value; + break; + case ControllerCommandType::SET_BUTTON_2_MIN: + settings.btn2Min = value; + break; + case ControllerCommandType::SET_BUTTON_2_MAX: + settings.btn2Max = value; + break; + case ControllerCommandType::SET_BUTTON_3_MIN: + settings.btn3Min = value; + break; + case ControllerCommandType::SET_BUTTON_3_MAX: + settings.btn3Max = value; + break; + case ControllerCommandType::SET_BUTTON_NO_PRESS: + settings.btnNoPress = value; + break; + default: + break; + } + if (!saveSettings()) { + return false; + } +#ifdef NORVI_AE01_R + NorviButtonHandler::applySettings(); +#endif + return true; + } + + case ControllerCommandType::SET_SENSOR_MAPPING: + return applySensorMapping(command, false); + + case ControllerCommandType::CLEAR_SENSOR_MAPPING: + return applySensorMapping(command, true); + + case ControllerCommandType::SET_POOL_PUMP_MANUAL: + case ControllerCommandType::SET_SOLAR_PUMP_MANUAL: + if (dependencies_.operationMode.getTypedMode() != OperationMode::MANUAL) { + LOG_WARN("Controller command: manual pump request rejected outside manual mode\n"); + return false; + } + if (command.type == ControllerCommandType::SET_POOL_PUMP_MANUAL) { + dependencies_.poolPump.setSwitch(command.enabled); + } else { + dependencies_.solarPump.setSwitch(command.enabled); + } + return true; + + case ControllerCommandType::TOGGLE_POOL_PUMP: + case ControllerCommandType::TOGGLE_SOLAR_PUMP: { + const OperationMode currentMode = dependencies_.operationMode.getTypedMode(); + if (command.toggleModePolicy == PumpToggleModePolicy::REQUIRE_MANUAL && currentMode != OperationMode::MANUAL) { + LOG_WARN("Controller command: Web pump toggle rejected outside manual mode\n"); + return false; + } + if (command.toggleModePolicy == PumpToggleModePolicy::ENTER_MANUAL && currentMode != OperationMode::MANUAL) { + if (!dependencies_.operationMode.setMode(OperationMode::MANUAL, sourceName(command.source))) { + return false; + } + settings.opMode = toString(OperationMode::MANUAL); + if (!saveSettings()) { + return false; + } + } + + RelayModuleNode &pump = + command.type == ControllerCommandType::TOGGLE_POOL_PUMP ? dependencies_.poolPump : dependencies_.solarPump; + const bool newState = !pump.getSwitch(); + pump.setSwitch(newState); + if (result != nullptr) { + result->hasPumpState = true; + result->pumpState = newState; + } + return true; + } + + case ControllerCommandType::FACTORY_RESET: { + static const char *const kOperationalNamespaces[] = {"pool-pump", "solar-pump", "pool-controller"}; + Preferences preferences; + for (const char *name : kOperationalNamespaces) { + if (preferences.begin(name, false)) { + preferences.clear(); + preferences.end(); + } + } + ConfigManager::reset(); + return saveSettings(); + } + } + + return false; +} + +} // namespace PoolController diff --git a/src/ControllerCommandHandler.hpp b/src/ControllerCommandHandler.hpp new file mode 100644 index 00000000..c4941bfb --- /dev/null +++ b/src/ControllerCommandHandler.hpp @@ -0,0 +1,65 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +/** + * @file ControllerCommandHandler.hpp + * @brief Single Core-1 mutation API for runtime controller commands. + */ + +#pragma once + +#include "ControllerCommand.hpp" + +class DallasTemperatureNode; +class ESP32TemperatureNode; +class OperationModeNode; +class RelayModuleNode; + +namespace PoolController { + +/** @brief Optional resolved owner state returned to synchronous adapters. */ +struct ControllerCommandResult final { + bool hasMode{false}; + OperationMode mode{OperationMode::AUTO}; + bool hasPumpState{false}; + bool pumpState{false}; +}; + +/** + * @brief Applies validated runtime commands to the legacy controller model. + * + * This service is the transitional application boundary between protocol + * adapters and the existing nodes/static configuration services. It is owned + * by the composition root and must only be invoked from the Core-1 Arduino + * loop task. + */ +class ControllerCommandHandler final { +public: + /** @brief Explicit legacy runtime dependencies supplied by the composition root. */ + struct Dependencies final { + OperationModeNode &operationMode; + RelayModuleNode &poolPump; + RelayModuleNode &solarPump; + DallasTemperatureNode &solarTemperature; + DallasTemperatureNode &poolTemperature; + ESP32TemperatureNode &controllerTemperature; + }; + + explicit ControllerCommandHandler(Dependencies dependencies) : dependencies_(dependencies) {} + + /** + * @brief Apply one command on the owning application task. + * @return true if the command was accepted and applied, false if invalid. + */ + bool handle(const ControllerCommand &command) { return handle(command, nullptr); } + bool handle(const ControllerCommand &command, ControllerCommandResult *result); + +private: + static const char *sourceName(CommandSource source); + bool saveSettings() const; + bool applySensorMapping(const ControllerCommand &command, bool clear); + + Dependencies dependencies_; +}; + +} // namespace PoolController diff --git a/src/ControllerSnapshotPublisher.cpp b/src/ControllerSnapshotPublisher.cpp new file mode 100644 index 00000000..24a64755 --- /dev/null +++ b/src/ControllerSnapshotPublisher.cpp @@ -0,0 +1,156 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include "ControllerSnapshotPublisher.hpp" + +#include +#include +#include +#include + +#include "ConfigManager.hpp" +#include "ControllerSnapshot.hpp" +#include "ControllerSnapshotStore.hpp" +#include "DallasTemperatureNode.hpp" +#include "DegradationManager.hpp" +#include "ESP32TemperatureNode.hpp" +#include "NetworkManager.hpp" +#include "OperationModeNode.hpp" +#include "RelayModuleNode.hpp" +#include "Rule.hpp" +#include "SystemMonitor.hpp" +#include "TimeClientHelper.hpp" + +namespace PoolController { + +namespace { + +bool isZeroAddress(const std::array &address) { + for (const std::uint8_t value : address) { + if (value != 0) { + return false; + } + } + return true; +} + +SensorReadingSnapshot sensorReading(float value, bool found) { + SensorReadingSnapshot reading{}; + reading.value = value; + reading.valid = found && std::isfinite(value); + return reading; +} + +SensorReadingSnapshot controllerReading(float value) { + SensorReadingSnapshot reading{}; + reading.value = value; + reading.valid = std::isfinite(value); + return reading; +} + +SensorMappingSnapshot mappingSnapshot(const std::array &configuredAddress, const DallasTemperatureNode &node) { + SensorMappingSnapshot mapping{}; + mapping.address = configuredAddress; + mapping.configured = !isZeroAddress(configuredAddress); + mapping.found = node.isSensorFound(); + + if (mapping.configured && mapping.found) { + const std::uint8_t *selectedAddress = node.getDeviceAddress(); + const bool addressMatches = std::memcmp(selectedAddress, configuredAddress.data(), configuredAddress.size()) == 0; + mapping.found = selectedAddress != nullptr && addressMatches; + } + return mapping; +} + +TimeDegradationState snapshotTimeDegradation(TimeDegradation degradation) { + switch (degradation) { + case TimeDegradation::GREEN: + return TimeDegradationState::GREEN; + case TimeDegradation::YELLOW: + return TimeDegradationState::YELLOW; + case TimeDegradation::RED: + default: + return TimeDegradationState::RED; + } +} + +} // namespace + +void ControllerSnapshotPublisher::publish() { + SystemSnapshot snapshot{}; + const std::uint32_t nowMs = millis(); + + const float poolTemperature = dependencies_.poolTemperature.getTemperature(); + const float solarTemperature = dependencies_.solarTemperature.getTemperature(); + const float controllerTemperature = dependencies_.controllerTemperature.getTemperature(); + + snapshot.sensors.pool = sensorReading(poolTemperature, dependencies_.poolTemperature.isSensorFound()); + snapshot.sensors.solar = sensorReading(solarTemperature, dependencies_.solarTemperature.isSensorFound()); + snapshot.sensors.controller = controllerReading(controllerTemperature); + + std::array solarAddress{}; + std::array poolAddress{}; + ConfigManager::loadSensorMapping(solarAddress.data(), poolAddress.data()); + snapshot.sensors.solarMapping = mappingSnapshot(solarAddress, dependencies_.solarTemperature); + snapshot.sensors.poolMapping = mappingSnapshot(poolAddress, dependencies_.poolTemperature); + + // Transitional generation: this identifies complete Core-1 read-model + // publications. Once #170 provides a coherent Core-0 acquisition snapshot, + // these two fields should use that sensor acquisition generation/timestamp. + snapshot.sensors.measuredAtMs = nowMs; + snapshot.sensors.generation = ++generation_; + + snapshot.network.wifiConnected = NetworkManager::isWiFiConnected(); + snapshot.network.mqttConnected = NetworkManager::isMqttConnected(); + snapshot.network.apMode = NetworkManager::isApMode(); + snapshot.network.wifiRssi = static_cast(NetworkManager::getWiFiRSSI()); + + snapshot.health.safeMode = DegradationManager::isSafe(); + snapshot.health.timeValid = isTimeSyncValid(); + snapshot.health.timeDegradation = snapshotTimeDegradation(getTimeDegradation()); + snapshot.health.poolSensorValid = snapshot.sensors.pool.valid; + snapshot.health.solarSensorValid = snapshot.sensors.solar.valid; + + const auto &settings = ConfigManager::getSettings(); + snapshot.circulation.threshold = settings.tempCircThreshold; + snapshot.circulation.factorMinutesPerDegree = static_cast(settings.tempCircFactor); + snapshot.circulation.maxRuntimeMinutes = static_cast(settings.tempCircMaxRuntime); + + const TimerSetting timer = dependencies_.operationMode.getTimerSetting(); + snapshot.timer.startHour = timer.timerStartHour; + snapshot.timer.startMinute = timer.timerStartMinutes; + snapshot.timer.endHour = timer.timerEndHour; + snapshot.timer.endMinute = timer.timerEndMinutes; + + Rule *activeRule = dependencies_.operationMode.getRule(); + if (activeRule != nullptr) { + snapshot.timer.effectiveRuntimeMinutes = activeRule->getEffectiveRuntimeMinutes(); + snapshot.timer.circulationExtensionMinutes = activeRule->getCirculationExtensionMinutes(); + snapshot.timer.activeEndMinutes = activeRule->getActiveEndMinutes(); + } + + snapshot.settings.loopInterval = static_cast(settings.loopInterval); + snapshot.settings.timezoneIndex = static_cast(settings.timezoneIndex); + snapshot.settings.timeLossGreenHours = static_cast(settings.timeLossGreenHours); + snapshot.settings.timeLossRedHours = static_cast(settings.timeLossRedHours); + snapshot.settings.button1Min = settings.btn1Min; + snapshot.settings.button1Max = settings.btn1Max; + snapshot.settings.button2Min = settings.btn2Min; + snapshot.settings.button2Max = settings.btn2Max; + snapshot.settings.button3Min = settings.btn3Min; + snapshot.settings.button3Max = settings.btn3Max; + snapshot.settings.buttonNoPress = settings.btnNoPress; + + snapshot.mode = dependencies_.operationMode.getTypedMode(); + snapshot.poolPumpOn = dependencies_.poolPump.getSwitch(); + snapshot.solarPumpOn = dependencies_.solarPump.getSwitch(); + snapshot.poolMaxTemperature = dependencies_.operationMode.getPoolMaxTemperature(); + snapshot.solarMinTemperature = dependencies_.operationMode.getSolarMinTemperature(); + snapshot.temperatureHysteresis = dependencies_.operationMode.getTemperatureHysteresis(); + snapshot.uptimeMs = nowMs; + snapshot.freeHeapBytes = SystemMonitor::getFreeHeap(); + + dependencies_.store.publish(snapshot); +} + +} // namespace PoolController diff --git a/src/ControllerSnapshotPublisher.hpp b/src/ControllerSnapshotPublisher.hpp new file mode 100644 index 00000000..a898cacf --- /dev/null +++ b/src/ControllerSnapshotPublisher.hpp @@ -0,0 +1,51 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +/** + * @file ControllerSnapshotPublisher.hpp + * @brief Core-1 builder and publisher for immutable controller read models. + */ + +#pragma once + +#include + +class DallasTemperatureNode; +class ESP32TemperatureNode; +class OperationModeNode; +class RelayModuleNode; + +namespace PoolController { + +class ControllerSnapshotStore; + +/** + * @brief Builds one coherent `SystemSnapshot` from Core-1-owned runtime state. + * + * The publisher is invoked only after the application loop has updated the + * legacy runtime model. Adapters can then read a value copy from the snapshot + * store instead of dereferencing mutable nodes and settings directly. + */ +class ControllerSnapshotPublisher final { +public: + struct Dependencies final { + OperationModeNode &operationMode; + RelayModuleNode &poolPump; + RelayModuleNode &solarPump; + DallasTemperatureNode &solarTemperature; + DallasTemperatureNode &poolTemperature; + ESP32TemperatureNode &controllerTemperature; + ControllerSnapshotStore &store; + }; + + explicit ControllerSnapshotPublisher(Dependencies dependencies) : dependencies_(dependencies) {} + + /** @brief Build and atomically publish the latest application read model. */ + void publish(); + +private: + Dependencies dependencies_; + std::uint32_t generation_{0}; +}; + +} // namespace PoolController diff --git a/src/MqttRuntimeCommandAdapter.cpp b/src/MqttRuntimeCommandAdapter.cpp new file mode 100644 index 00000000..cc20e979 --- /dev/null +++ b/src/MqttRuntimeCommandAdapter.cpp @@ -0,0 +1,252 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include "MqttRuntimeCommandAdapter.hpp" + +#include +#include + +#include "ControllerCommandHandler.hpp" +#include "LogCapture.hpp" +#include "MqttPublisher.hpp" +#include "NetworkManager.hpp" +#include "OperationMode.hpp" +#include "TimeClientHelper.hpp" + +namespace PoolController { + +namespace { + +bool topicEndsWith(const char *topic, const char *suffix) { + const std::size_t topicLength = std::strlen(topic); + const std::size_t suffixLength = std::strlen(suffix); + return topicLength >= suffixLength && std::memcmp(topic + topicLength - suffixLength, suffix, suffixLength) == 0; +} + +bool isDigit(char value) { + return value >= '0' && value <= '9'; +} + +bool parseTime(const char *value, std::size_t length, std::uint8_t &hour, std::uint8_t &minute) { + if (length < 5 || value[2] != ':' || !isDigit(value[0]) || !isDigit(value[1]) || !isDigit(value[3]) || !isDigit(value[4])) { + return false; + } + const unsigned parsedHour = static_cast((value[0] - '0') * 10 + (value[1] - '0')); + const unsigned parsedMinute = static_cast((value[3] - '0') * 10 + (value[4] - '0')); + if (parsedHour > 23 || parsedMinute > 59) { + return false; + } + hour = static_cast(parsedHour); + minute = static_cast(parsedMinute); + return true; +} + +bool parseSensorAddress(const char *value, std::size_t length, std::array &address) { + if (length < 16) { + return false; + } + for (std::size_t i = 0; i < address.size(); ++i) { + const char hi = value[i * 2]; + const char lo = value[i * 2 + 1]; + const auto hexValue = [](char c) -> int { + if (c >= '0' && c <= '9') + return c - '0'; + if (c >= 'a' && c <= 'f') + return 10 + c - 'a'; + if (c >= 'A' && c <= 'F') + return 10 + c - 'A'; + return -1; + }; + const int high = hexValue(hi); + const int low = hexValue(lo); + if (high < 0 || low < 0) { + return false; + } + address[i] = static_cast((high << 4) | low); + } + return true; +} + +} // namespace + +void MqttRuntimeCommandAdapter::begin() { + NetworkManager::setMqttCallback( + [this](char *topic, char *payload, AsyncMqttClientMessageProperties properties, std::size_t len, std::size_t index, + std::size_t total) { onMqttMessage(topic, payload, properties, len, index, total); }); +} + +void MqttRuntimeCommandAdapter::onMqttMessage(char *topic, char *payload, AsyncMqttClientMessageProperties properties, + std::size_t len, std::size_t index, std::size_t total) { + (void)properties; + if (index != 0 || len != total) { + LOG_WARN("MQTT runtime adapter: ignoring chunked message on %s\n", topic); + return; + } + + switch (rawQueue_.push(topic, payload, len)) { + case MqttCommandQueue::PushResult::OK: + break; + case MqttCommandQueue::PushResult::FULL: + LOG_WARN("MQTT runtime adapter: queue full — dropping %s\n", topic); + break; + case MqttCommandQueue::PushResult::TOO_LARGE: + LOG_WARN("MQTT runtime adapter: message too large — dropping %s\n", topic); + break; + } +} + +MqttRuntimeCommandAdapter::ParseResult MqttRuntimeCommandAdapter::parseRuntimeCommand( + const MqttCommandQueue::Message &message, ControllerCommand &command) const { + command = ControllerCommand{}; + command.source = CommandSource::MQTT; + const char *topic = message.topic; + const char *value = message.payload; + + if (topicEndsWith(topic, "/thermostat/preset/set")) { + command.type = ControllerCommandType::SET_MODE; + if (std::strcmp(value, "none") == 0) + command.mode = OperationMode::AUTO; + else if (std::strcmp(value, "manual") == 0) + command.mode = OperationMode::MANUAL; + else if (std::strcmp(value, "schedule") == 0) + command.mode = OperationMode::TIMER; + else if (std::strcmp(value, "boost") == 0) + command.mode = OperationMode::BOOST; + else + return ParseResult::INVALID; + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/thermostat/mode/set")) { + command.type = ControllerCommandType::SET_MODE; + if (std::strcmp(value, "off") == 0) + command.mode = OperationMode::MANUAL; + else if (std::strcmp(value, "auto") == 0) + command.mode = OperationMode::AUTO; + else if (std::strcmp(value, "heat") == 0) + command.mode = OperationMode::BOOST; + else + return ParseResult::INVALID; + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/thermostat/temperature/set") || topicEndsWith(topic, "/pool-max-temp/set")) { + command.type = ControllerCommandType::SET_POOL_MAX_TEMPERATURE; + command.value = std::strtof(value, nullptr); + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/solar-min-temp/set")) { + command.type = ControllerCommandType::SET_SOLAR_MIN_TEMPERATURE; + command.value = std::strtof(value, nullptr); + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/hysteresis/set")) { + command.type = ControllerCommandType::SET_TEMPERATURE_HYSTERESIS; + command.value = std::strtof(value, nullptr); + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/temp-circ-threshold/set")) { + command.type = ControllerCommandType::SET_TEMPERATURE_CIRCULATION_THRESHOLD; + command.value = std::strtof(value, nullptr); + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/temp-circ-factor/set")) { + command.type = ControllerCommandType::SET_TEMPERATURE_CIRCULATION_FACTOR; + command.integerValue = std::atoi(value); + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/temp-circ-max-runtime/set")) { + command.type = ControllerCommandType::SET_TEMPERATURE_CIRCULATION_MAX_RUNTIME; + command.integerValue = std::atoi(value); + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/pool-pump/set") || topicEndsWith(topic, "/solar-pump/set")) { + if (std::strcmp(value, "ON") != 0 && std::strcmp(value, "OFF") != 0) { + return ParseResult::INVALID; + } + if (topicEndsWith(topic, "/pool-pump/set")) { + command.type = ControllerCommandType::SET_POOL_PUMP_MANUAL; + } else { + command.type = ControllerCommandType::SET_SOLAR_PUMP_MANUAL; + } + command.enabled = std::strcmp(value, "ON") == 0; + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/mode/set")) { + command.type = ControllerCommandType::SET_MODE; + if (!tryParseOperationMode(value, command.mode)) { + return ParseResult::INVALID; + } + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/timer-start/set") || topicEndsWith(topic, "/timer-end/set")) { + if (topicEndsWith(topic, "/timer-start/set")) { + command.type = ControllerCommandType::SET_TIMER_START; + } else { + command.type = ControllerCommandType::SET_TIMER_END; + } + if (!parseTime(value, message.payloadLen, command.hour, command.minute)) { + return ParseResult::INVALID; + } + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/timezone/set")) { + const int timezoneIndex = getTimezoneIndexFromLabel(value); + if (timezoneIndex < 0) { + return ParseResult::INVALID; + } + command.type = ControllerCommandType::SET_TIMEZONE; + command.integerValue = timezoneIndex; + return ParseResult::VALID; + } + + if (topicEndsWith(topic, "/solar-sensor/set") || topicEndsWith(topic, "/pool-sensor/set")) { + command.sensorRole = topicEndsWith(topic, "/solar-sensor/set") ? SensorRole::SOLAR : SensorRole::POOL; + if (std::strstr(value, "Not configured") != nullptr || message.payloadLen < 16) { + command.type = ControllerCommandType::CLEAR_SENSOR_MAPPING; + return ParseResult::VALID; + } + if (!parseSensorAddress(value, message.payloadLen, command.sensorAddress)) { + return ParseResult::INVALID; + } + command.type = ControllerCommandType::SET_SENSOR_MAPPING; + return ParseResult::VALID; + } + + return ParseResult::NOT_RUNTIME; +} + +void MqttRuntimeCommandAdapter::processPendingCommands() { + MqttCommandQueue::Message message; + for (std::size_t handled = 0; handled < kMaxCommandsPerLoop && rawQueue_.pop(message); ++handled) { + ControllerCommand command{}; + const ParseResult result = parseRuntimeCommand(message, command); + if (result == ParseResult::NOT_RUNTIME) { + AsyncMqttClientMessageProperties properties{}; + MqttPublisher::handleMqttMessage(message.topic, message.payload, properties, message.payloadLen, 0, message.payloadLen); + continue; + } + + if (result == ParseResult::INVALID) { + LOG_WARN("MQTT runtime adapter: invalid command on %s\n", message.topic); + MqttPublisher::publishStates(); + continue; + } + + if (!handler_.handle(command)) { + LOG_WARN("MQTT runtime adapter: command rejected on %s\n", message.topic); + } + MqttPublisher::publishStates(); + } +} + +} // namespace PoolController diff --git a/src/MqttRuntimeCommandAdapter.hpp b/src/MqttRuntimeCommandAdapter.hpp new file mode 100644 index 00000000..fe45c085 --- /dev/null +++ b/src/MqttRuntimeCommandAdapter.hpp @@ -0,0 +1,56 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +/** + * @file MqttRuntimeCommandAdapter.hpp + * @brief MQTT protocol adapter for typed Core-1 controller commands. + */ + +#pragma once + +#include +#include + +#include "ControllerCommand.hpp" +#include "MqttCommandQueue.hpp" + +namespace PoolController { + +class ControllerCommandHandler; + +/** + * @brief Moves MQTT callback data onto the loop task and translates runtime + * mutations into allocation-free `ControllerCommand` values. + * + * Commands outside the runtime-controller scope (for example OTA and NTP + * server provisioning) are delegated to the existing MQTT implementation on + * the loop task until their dedicated service boundaries are extracted. + */ +class MqttRuntimeCommandAdapter final { +public: + explicit MqttRuntimeCommandAdapter(ControllerCommandHandler &handler) : handler_(handler) {} + + /** @brief Replace the MQTT callback with this adapter after MqttPublisher::begin(). */ + void begin(); + + /** @brief Drain a bounded number of queued messages on the Core-1 loop task. */ + void processPendingCommands(); + + static constexpr std::size_t kMaxCommandsPerLoop = 2; + +private: + enum class ParseResult : std::uint8_t { + NOT_RUNTIME, + VALID, + INVALID, + }; + + void onMqttMessage(char *topic, char *payload, AsyncMqttClientMessageProperties properties, std::size_t len, std::size_t index, + std::size_t total); + ParseResult parseRuntimeCommand(const MqttCommandQueue::Message &message, ControllerCommand &command) const; + + ControllerCommandHandler &handler_; + MqttCommandQueue rawQueue_; +}; + +} // namespace PoolController diff --git a/src/WebPortal.cpp b/src/WebPortal.cpp index eb914216..dfabb1d0 100644 --- a/src/WebPortal.cpp +++ b/src/WebPortal.cpp @@ -52,6 +52,7 @@ static File fsUploadFile; WebServer WebPortal::server_(80); DNSServer WebPortal::dnsServer_; +WebPortal::CommandDispatcher WebPortal::commandDispatcher_ = nullptr; bool WebPortal::dnsServerStarted_ = false; String WebPortal::activeSessionToken_ = ""; String WebPortal::csrfToken_ = ""; @@ -937,14 +938,37 @@ void WebPortal::apiSetMode() { return; } - String mode = server_.arg("mode"); - if (operationModeNode.setMode(mode, "web:apiSetMode")) { - ConfigManager::getSettings().opMode = mode; - ConfigManager::save(); - server_.send(200, "application/json", "{\"status\":\"ok\",\"mode\":\"" + mode + "\"}"); + const String requestedMode = server_.arg("mode"); + ControllerCommand command{}; + command.source = CommandSource::WEB; + + if (requestedMode == "cycle") { + command.type = ControllerCommandType::CYCLE_MODE; } else { + OperationMode parsedMode; + if (!tryParseOperationMode(requestedMode.c_str(), parsedMode)) { + server_.send(400, "application/json", "{\"status\":\"error\",\"message\":\"Invalid mode\"}"); + return; + } + command.type = ControllerCommandType::SET_MODE; + command.mode = parsedMode; + } + + ControllerCommandResult result{}; + if (commandDispatcher_ == nullptr || !commandDispatcher_(command, &result)) { server_.send(400, "application/json", "{\"status\":\"error\",\"message\":\"Invalid mode\"}"); + return; + } + + const char *resolvedMode = requestedMode.c_str(); + if (command.type == ControllerCommandType::CYCLE_MODE) { + if (!result.hasMode) { + server_.send(500, "application/json", "{\"status\":\"error\",\"message\":\"Missing command result\"}"); + return; + } + resolvedMode = toString(result.mode); } + server_.send(200, "application/json", "{\"status\":\"ok\",\"mode\":\"" + String(resolvedMode) + "\"}"); } void WebPortal::apiTogglePump() { @@ -953,27 +977,30 @@ void WebPortal::apiTogglePump() { return; } - // Only allow pump toggling in manual mode - if (operationModeNode.getMode() != "manu") { - server_.send(400, "application/json", "{\"status\":\"error\",\"message\":\"Pump control only available in manual mode\"}"); - return; - } - - String pump = server_.arg("pump"); - bool newState; - + const String pump = server_.arg("pump"); + ControllerCommand command{}; + command.source = CommandSource::WEB; + command.toggleModePolicy = PumpToggleModePolicy::REQUIRE_MANUAL; if (pump == "pool") { - newState = !poolPumpNode.getSwitch(); - poolPumpNode.setSwitch(newState); + command.type = ControllerCommandType::TOGGLE_POOL_PUMP; } else if (pump == "solar") { - newState = !solarPumpNode.getSwitch(); - solarPumpNode.setSwitch(newState); + command.type = ControllerCommandType::TOGGLE_SOLAR_PUMP; } else { server_.send(400, "application/json", "{\"status\":\"error\",\"message\":\"Invalid pump. Use 'pool' or 'solar'\"}"); return; } - String json = "{\"status\":\"ok\",\"state\":" + String(newState ? "true" : "false") + "}"; + ControllerCommandResult result{}; + if (commandDispatcher_ == nullptr || !commandDispatcher_(command, &result)) { + server_.send(400, "application/json", "{\"status\":\"error\",\"message\":\"Pump control only available in manual mode\"}"); + return; + } + if (!result.hasPumpState) { + server_.send(500, "application/json", "{\"status\":\"error\",\"message\":\"Missing command result\"}"); + return; + } + + String json = "{\"status\":\"ok\",\"state\":" + String(result.pumpState ? "true" : "false") + "}"; server_.send(200, "application/json", json); } diff --git a/src/WebPortal.hpp b/src/WebPortal.hpp index 581bfa79..9495e3c5 100644 --- a/src/WebPortal.hpp +++ b/src/WebPortal.hpp @@ -13,6 +13,7 @@ #include #include +#include "ControllerCommandHandler.hpp" #include "LogCapture.hpp" namespace PoolController { @@ -29,6 +30,12 @@ class WebPortal { public: WebPortal() = default; + using CommandDispatcher = bool (*)(const ControllerCommand &, ControllerCommandResult *); + static void setCommandDispatcher(CommandDispatcher dispatcher) { commandDispatcher_ = dispatcher; } +#if !defined(ESP32) && !defined(ARDUINO_ARCH_ESP32) + static bool invokeRouteForTest(const char *uri, int method) { return server_.invokeRoute(uri, method); } +#endif + /** @brief Start the HTTP server and optional DNS captive portal. @return true if server started successfully. */ static bool begin(); /** @brief Handle incoming HTTP client requests. Call periodically from main loop(). */ @@ -141,6 +148,7 @@ class WebPortal { static WebServer server_; static DNSServer dnsServer_; + static CommandDispatcher commandDispatcher_; static bool dnsServerStarted_; static String activeSessionToken_; diff --git a/src/main.cpp b/src/main.cpp index 56bf860b..5339ffa3 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4,25 +4,71 @@ /** * @file main.cpp - * @brief Main entry point for the Pool Controller firmware. + * @brief Main entry point and composition root for the Pool Controller firmware. * * Arduino entry point: setup() initializes all subsystems, loop() runs * the control, monitoring, and network stack indefinitely. */ #include + +#include "ControllerCommandHandler.hpp" +#include "ControllerSnapshotPublisher.hpp" +#include "ControllerSnapshotStore.hpp" #include "LogCapture.hpp" +#include "MqttRuntimeCommandAdapter.hpp" +#include "Nodes.hpp" #include "PoolController.hpp" +#include "WebPortal.hpp" -/** @brief Singleton context owning all controller subsystems. */ +/** @brief Singleton context owning the existing controller lifecycle. */ static PoolController::PoolControllerContext context{}; +namespace { + +/** + * Construct application services only after the legacy global nodes completed + * static initialization. Function-local statics avoid cross-TU initialization + * order dependencies while keeping ownership explicit at the composition root. + */ +PoolController::ControllerCommandHandler &controllerCommandHandler() { + static PoolController::ControllerCommandHandler::Dependencies dependencies{PoolController::operationModeNode, + PoolController::poolPumpNode, PoolController::solarPumpNode, PoolController::solarTemperatureNode, + PoolController::poolTemperatureNode, PoolController::ctrlTemperatureNode}; + static PoolController::ControllerCommandHandler handler(dependencies); + return handler; +} + +bool dispatchWebCommand(const PoolController::ControllerCommand &command, PoolController::ControllerCommandResult *result) { + return controllerCommandHandler().handle(command, result); +} + +PoolController::MqttRuntimeCommandAdapter &mqttRuntimeCommandAdapter() { + static PoolController::MqttRuntimeCommandAdapter adapter(controllerCommandHandler()); + return adapter; +} + +PoolController::ControllerSnapshotStore &controllerSnapshotStore() { + static PoolController::ControllerSnapshotStore store; + return store; +} + +PoolController::ControllerSnapshotPublisher &controllerSnapshotPublisher() { + static PoolController::ControllerSnapshotPublisher::Dependencies dependencies{PoolController::operationModeNode, + PoolController::poolPumpNode, PoolController::solarPumpNode, PoolController::solarTemperatureNode, + PoolController::poolTemperatureNode, PoolController::ctrlTemperatureNode, controllerSnapshotStore()}; + static PoolController::ControllerSnapshotPublisher publisher(dependencies); + return publisher; +} + +} // namespace + /** * @brief Arduino setup() — initializes serial and delegates to PoolControllerContext. * * Waits up to 3 seconds for a USB serial connection (non-blocking fallback * for headless operation), then calls context.setup() to initialize all - * subsystems. + * subsystems and installs the typed MQTT runtime-command adapter. */ auto setup() -> void { Serial.begin(SERIAL_SPEED); @@ -39,15 +85,27 @@ auto setup() -> void { // export watermark sees the pre-boot sequence. Serial is already up here. PoolController::LogCapture::begin(); + PoolController::WebPortal::setCommandDispatcher(dispatchWebCommand); context.setup(); + + // MqttPublisher::begin() is part of context.setup(). Replace its callback + // afterwards so new runtime mutations flow through typed commands. The + // adapter callback only copies bytes; mutation happens in loop(). + mqttRuntimeCommandAdapter().begin(); + + // Publish a first immutable read model after setup established runtime state. + controllerSnapshotPublisher().publish(); } /** - * @brief Arduino loop() — runs the control, monitoring, and network stack. + * @brief Arduino loop() — runs typed adapter hand-over and controller stack. * - * Called continuously after setup(). Delegates to context.loop() which - * handles watchdog feeding, memory checks, node updates, MQTT, and OTA. + * Runtime MQTT commands are drained on the same Arduino/Core-1 loop task that + * owns mutable controller state. After the legacy controller loop completes, + * one immutable read model is published for outbound adapters. */ auto loop() -> void { + mqttRuntimeCommandAdapter().processPendingCommands(); context.loop(); + controllerSnapshotPublisher().publish(); } diff --git a/test/native/CMakeLists.txt b/test/native/CMakeLists.txt index 35acb612..2eae6516 100644 --- a/test/native/CMakeLists.txt +++ b/test/native/CMakeLists.txt @@ -32,15 +32,16 @@ include_directories( ) # ── Wrappers ────────────────────────────────────────────────────────────── -# WebPortal, MqttPublisher, Timer, and CalibrationManager include class -# headers whose mock layout differs from production. Compiling from -# wrappers/ avoids GCC's "current-file-directory first" rule for -# #include "..." so the mock headers take effect. -file(COPY ${PROJ_ROOT}/src/WebPortal.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) +# Sources that include production class headers are compiled from wrappers/ +# so mocks/ remains first in the include search path. +file(COPY ${PROJ_ROOT}/src/WebPortal.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) file(COPY ${PROJ_ROOT}/src/MqttPublisher.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) -file(COPY ${PROJ_ROOT}/src/Timer.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) +file(COPY ${PROJ_ROOT}/src/Timer.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) file(COPY ${PROJ_ROOT}/src/CalibrationManager.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) file(COPY ${PROJ_ROOT}/src/NorviButtonHandler.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) +file(COPY ${PROJ_ROOT}/src/ControllerCommandHandler.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) +file(COPY ${PROJ_ROOT}/src/MqttRuntimeCommandAdapter.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) +file(COPY ${PROJ_ROOT}/src/ControllerSnapshotPublisher.cpp DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/wrappers) set(WRAPPER_SOURCES ${CMAKE_CURRENT_BINARY_DIR}/wrappers/WebPortal.cpp @@ -48,11 +49,12 @@ set(WRAPPER_SOURCES ${CMAKE_CURRENT_BINARY_DIR}/wrappers/Timer.cpp ${CMAKE_CURRENT_BINARY_DIR}/wrappers/CalibrationManager.cpp ${CMAKE_CURRENT_BINARY_DIR}/wrappers/NorviButtonHandler.cpp + ${CMAKE_CURRENT_BINARY_DIR}/wrappers/ControllerCommandHandler.cpp + ${CMAKE_CURRENT_BINARY_DIR}/wrappers/MqttRuntimeCommandAdapter.cpp + ${CMAKE_CURRENT_BINARY_DIR}/wrappers/ControllerSnapshotPublisher.cpp ) # ── Service classes ─────────────────────────────────────────────────────── -# Compiled from src/ directly (their own headers are in src/, so they find -# the production header first — only Arduino-level mocks are needed). set(SERVICE_SOURCES ${PROJ_ROOT}/src/StateManager.cpp ${PROJ_ROOT}/src/DegradationManager.cpp @@ -60,7 +62,7 @@ set(SERVICE_SOURCES ${PROJ_ROOT}/src/OtaUpdater.cpp ${PROJ_ROOT}/src/LogCapture.cpp ${PROJ_ROOT}/src/LocalSettingsMenu.cpp -${PROJ_ROOT}/src/Ky040Decoder.cpp + ${PROJ_ROOT}/src/Ky040Decoder.cpp ${PROJ_ROOT}/src/OlimexEncoderHandler.cpp ) @@ -79,6 +81,7 @@ set(TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_rules.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_config_manager.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_webportal_json.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_web_runtime_commands.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_mqttpublisher.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_security.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_state_manager.cpp @@ -93,6 +96,9 @@ set(TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_snapshot_diagnostics.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_snapshot_store.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_command_queue.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_command_handler.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_mqtt_runtime_command_adapter.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_snapshot_publisher.cpp ) add_executable(test_runner diff --git a/test/native/mocks/DallasTemperatureNode.hpp b/test/native/mocks/DallasTemperatureNode.hpp index 2c76682a..4f5ea44a 100644 --- a/test/native/mocks/DallasTemperatureNode.hpp +++ b/test/native/mocks/DallasTemperatureNode.hpp @@ -1,6 +1,7 @@ #pragma once #include "Arduino.h" #include "DallasTemperature.h" +#include class DallasTemperatureNode { public: @@ -22,11 +23,25 @@ class DallasTemperatureNode { unsigned long getMeasurementInterval() const { return _measurementInterval; } void setMeasurementInterval(unsigned long interval) { _measurementInterval = interval; } - void setAddressFilter(const DeviceAddress) {} - void clearAddressFilter() {} - bool hasAddressFilter() const { return false; } + void setAddressFilter(const DeviceAddress address) { + std::memcpy(_filterAddress, address, sizeof(_filterAddress)); + _hasFilter = true; + } + void clearAddressFilter() { + std::memset(_filterAddress, 0, sizeof(_filterAddress)); + _hasFilter = false; + } + bool hasAddressFilter() const { return _hasFilter; } + const uint8_t *getDeviceAddress() const { return _filterAddress; } - void getDeviceAddressString(char *buf, size_t size) const { snprintf(buf, size, "NONE"); } + void getDeviceAddressString(char *buf, size_t size) const { + if (!_hasFilter) { + snprintf(buf, size, "NONE"); + return; + } + snprintf(buf, size, "%02X%02X%02X%02X%02X%02X%02X%02X", _filterAddress[0], _filterAddress[1], _filterAddress[2], + _filterAddress[3], _filterAddress[4], _filterAddress[5], _filterAddress[6], _filterAddress[7]); + } bool getDetectedDeviceAddress(uint8_t, DeviceAddress) const { return false; } float getDetectedDeviceTemperature(uint8_t) const { return 0.0f; } @@ -38,4 +53,6 @@ class DallasTemperatureNode { unsigned long _lastMeasurement = 0; bool _sensorFound = false; float _temperature = 25.0f; + bool _hasFilter = false; + DeviceAddress _filterAddress{}; }; diff --git a/test/native/mocks/ESP32TemperatureNode.hpp b/test/native/mocks/ESP32TemperatureNode.hpp index 0a858adc..b8506093 100644 --- a/test/native/mocks/ESP32TemperatureNode.hpp +++ b/test/native/mocks/ESP32TemperatureNode.hpp @@ -10,6 +10,8 @@ class ESP32TemperatureNode { void setTemperature(float t) { _temperature = t; } const char *getId() const { return _id; } void setMeasurementInterval(unsigned long interval) { _measurementInterval = interval; } + unsigned long getMeasurementInterval() const { return _measurementInterval; } + private: const char *_id; const char *_name; diff --git a/test/native/mocks/OperationModeNode.hpp b/test/native/mocks/OperationModeNode.hpp index 97e847fa..78309e92 100644 --- a/test/native/mocks/OperationModeNode.hpp +++ b/test/native/mocks/OperationModeNode.hpp @@ -1,8 +1,9 @@ #pragma once #include "Arduino.h" -#include #include +#include +#include "OperationMode.hpp" #include "RuleAuto.hpp" #include "RuleManu.hpp" #include "RuleBoost.hpp" @@ -13,44 +14,53 @@ class OperationModeNode { public: OperationModeNode() {} - OperationModeNode(const char *id, const char *name, int) {} + OperationModeNode(const char *id, const char *name, int = 300) {} void begin() {} void loop() {} - String getMode() const { return String(_mode.c_str()); } - const char *getModeCStr() const { return _mode.c_str(); } + String getMode() const { return String(PoolController::toString(_mode)); } + const char *getModeCStr() const { return PoolController::toString(_mode); } + PoolController::OperationMode getTypedMode() const { return _mode; } + bool setMode(String mode) { return setMode(mode, "unspecified"); } bool setMode(String mode, const char *source) { + PoolController::OperationMode parsed{}; + if (!PoolController::tryParseOperationMode(mode.c_str(), parsed)) return false; + return setMode(parsed, source); + } + bool setMode(PoolController::OperationMode mode) { return setMode(mode, "unspecified"); } + bool setMode(PoolController::OperationMode mode, const char *source) { _lastModeSource = source != nullptr ? source : "unspecified"; - _mode = mode.c_str(); + _mode = mode; return true; } const char *getLastModeSource() const { return _lastModeSource.c_str(); } Rule *getRule() { - if (_mode == "auto") + if (_mode == PoolController::OperationMode::AUTO) return &_autoRule; - if (_mode == "manu") + if (_mode == PoolController::OperationMode::MANUAL) return &_manuRule; - if (_mode == "boost") + if (_mode == PoolController::OperationMode::BOOST) return &_boostRule; - if (_mode == "timer") + if (_mode == PoolController::OperationMode::TIMER) return &_timerRule; return nullptr; } - float getPoolMaxTemperature() const { return 28.0f; } - float getSolarMinTemperature() const { return 35.0f; } - float getTemperatureHysteresis() const { return 1.0f; } - void setPoolMaxTemperature(float v) {} - void setSolarMinTemperature(float v) {} - void setTemperatureHysteresis(float v) {} + float getPoolMaxTemperature() const { return _poolMaxTemperature; } + float getSolarMinTemperature() const { return _solarMinTemperature; } + float getTemperatureHysteresis() const { return _temperatureHysteresis; } + void setPoolMaxTemperature(float value) { _poolMaxTemperature = value; } + void setSolarMinTemperature(float value) { _solarMinTemperature = value; } + void setTemperatureHysteresis(float value) { _temperatureHysteresis = value; } TimerSetting getTimerSetting() const { return _timer; } void setTimerSetting(const TimerSetting &ts) { _timer = ts; } void setMeasurementInterval(unsigned long interval) { _measurementInterval = interval; } + unsigned long getMeasurementInterval() const { return _measurementInterval; } static constexpr const char *STATUS_AUTO = "auto"; static constexpr const char *STATUS_MANU = "manu"; @@ -58,8 +68,11 @@ class OperationModeNode { static constexpr const char *STATUS_TIMER = "timer"; private: - std::string _mode = "auto"; + PoolController::OperationMode _mode{PoolController::OperationMode::AUTO}; std::string _lastModeSource = ""; + float _poolMaxTemperature = 28.0f; + float _solarMinTemperature = 35.0f; + float _temperatureHysteresis = 1.0f; TimerSetting _timer; unsigned long _measurementInterval = 300; RuleAuto _autoRule; diff --git a/test/native/mocks/TimeClientHelper.hpp b/test/native/mocks/TimeClientHelper.hpp index 313589b6..9f78819f 100644 --- a/test/native/mocks/TimeClientHelper.hpp +++ b/test/native/mocks/TimeClientHelper.hpp @@ -2,12 +2,10 @@ #include #include -// TimeDegradation is defined in the production TimeClientHelper.hpp. -// Do NOT redefine here — it's already included via Rule.hpp → Timer.hpp chain. -// When needed in stubs.cpp, define it directly. +#include "Arduino.h" +#include "TimeLib.h" // tm conversion helpers — shadow the declarations in TimeLib.h (mock). -// These are NOT declared in the production TimeClientHelper.hpp. inline int year(time_t t) { return 2026; } inline int month(time_t t) { return 6; } inline int day(time_t t) { return 13; } @@ -16,5 +14,22 @@ inline int minute(time_t t) { return 30; } inline int second(time_t t) { return 0; } inline int weekday(time_t t) { return 6; } -// Stub for syncSystemClock (used by OtaUpdater.cpp in native tests) +bool isTimeSyncValid(); +int getTzCount(); +int getTimezoneLabelCount(); +const char *const *getTimezoneLabelList(); +int getTimezoneIndexFromLabel(const char *label); +String getFormattedTime(time_t rawTime); +time_t getUtcTime(); +int getTimezoneIndex(); +time_t getLastValidSyncTime(); +bool forceNtpUpdate(); +void setTimeDegradationGreenHours(uint8_t hours); +uint8_t getTimeDegradationGreenHours(); +void setTimeDegradationRedHours(uint8_t hours); +uint8_t getTimeDegradationRedHours(); +time_t getTimeFor(int tzIndex, TimeChangeRule **tcr); +String getTimeInfoFor(int index); +void setTimezoneIndex(int index); +void timeClientSetup(const char *ntpServer); void syncSystemClock(); diff --git a/test/native/tests/test_controller_command_handler.cpp b/test/native/tests/test_controller_command_handler.cpp new file mode 100644 index 00000000..bd830136 --- /dev/null +++ b/test/native/tests/test_controller_command_handler.cpp @@ -0,0 +1,197 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include "ControllerCommandHandler.hpp" + +#include + +#include "ConfigManager.hpp" +#include "DallasTemperatureNode.hpp" +#include "ESP32TemperatureNode.hpp" +#include "OperationModeNode.hpp" +#include "RelayModuleNode.hpp" + +extern void test_begin(const char *suite, const char *name); +extern void test_pass(const char *file, int line); +extern void test_fail(const char *file, int line, const char *msg); +extern void test_suite_end(const char *name, int passed, int failed); + +#define ASSERT_TRUE(cond) \ + do { \ + if (!(cond)) { \ + test_fail(__FILE__, __LINE__, "Expected true: " #cond); \ + return 1; \ + } \ + test_pass(__FILE__, __LINE__); \ + } while (0) + +#define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) + +using PoolController::CommandSource; +using PoolController::ControllerCommand; +using PoolController::ControllerCommandHandler; +using PoolController::ControllerCommandType; +using PoolController::OperationMode; +using PoolController::SensorRole; + +struct HandlerFixture { + OperationModeNode operation{"operation-mode", "Operation Mode", 300}; + RelayModuleNode poolPump{"pool", "Pool", 1}; + RelayModuleNode solarPump{"solar", "Solar", 2}; + DallasTemperatureNode solarTemperature{"solar-temp", "Solar Temperature", 3}; + DallasTemperatureNode poolTemperature{"pool-temp", "Pool Temperature", 4}; + ESP32TemperatureNode controllerTemperature{"controller-temp", "Controller Temperature"}; + ControllerCommandHandler handler{ControllerCommandHandler::Dependencies{ + operation, poolPump, solarPump, solarTemperature, poolTemperature, controllerTemperature}}; +}; + +static int test_mode_and_setpoint_are_applied_through_one_handler() { + test_begin("ControllerCommandHandler", "mode and setpoint update runtime and persisted settings"); + HandlerFixture fixture; + auto &settings = PoolController::ConfigManager::getSettings(); + settings.opMode = "auto"; + settings.tempMaxPool = 28.0F; + + ControllerCommand mode{}; + mode.type = ControllerCommandType::SET_MODE; + mode.source = CommandSource::MQTT; + mode.mode = OperationMode::BOOST; + ASSERT_TRUE(fixture.handler.handle(mode)); + ASSERT_EQ(fixture.operation.getTypedMode(), OperationMode::BOOST); + ASSERT_EQ(settings.opMode.compare("boost"), 0); + + ControllerCommand setpoint{}; + setpoint.type = ControllerCommandType::SET_POOL_MAX_TEMPERATURE; + setpoint.source = CommandSource::MQTT; + setpoint.value = 29.5F; + ASSERT_TRUE(fixture.handler.handle(setpoint)); + ASSERT_EQ(fixture.operation.getPoolMaxTemperature(), 29.5F); + ASSERT_EQ(settings.tempMaxPool, 29.5F); + + setpoint.value = 50.0F; + ASSERT_TRUE(!fixture.handler.handle(setpoint)); + ASSERT_EQ(fixture.operation.getPoolMaxTemperature(), 29.5F); + return 0; +} + +static int test_timer_updates_are_independent() { + test_begin("ControllerCommandHandler", "timer start and end updates preserve the opposite half"); + HandlerFixture fixture; + TimerSetting timer{}; + timer.timerStartHour = 6; + timer.timerStartMinutes = 15; + timer.timerEndHour = 18; + timer.timerEndMinutes = 45; + fixture.operation.setTimerSetting(timer); + + ControllerCommand start{}; + start.type = ControllerCommandType::SET_TIMER_START; + start.hour = 7; + start.minute = 30; + ASSERT_TRUE(fixture.handler.handle(start)); + timer = fixture.operation.getTimerSetting(); + ASSERT_EQ(timer.timerStartHour, 7); + ASSERT_EQ(timer.timerStartMinutes, 30); + ASSERT_EQ(timer.timerEndHour, 18); + ASSERT_EQ(timer.timerEndMinutes, 45); + + ControllerCommand end{}; + end.type = ControllerCommandType::SET_TIMER_END; + end.hour = 20; + end.minute = 5; + ASSERT_TRUE(fixture.handler.handle(end)); + timer = fixture.operation.getTimerSetting(); + ASSERT_EQ(timer.timerStartHour, 7); + ASSERT_EQ(timer.timerStartMinutes, 30); + ASSERT_EQ(timer.timerEndHour, 20); + ASSERT_EQ(timer.timerEndMinutes, 5); + return 0; +} + +static int test_manual_pump_guard_is_application_policy() { + test_begin("ControllerCommandHandler", "manual pump writes are rejected outside manual mode"); + HandlerFixture fixture; + ControllerCommand command{}; + command.type = ControllerCommandType::SET_POOL_PUMP_MANUAL; + command.enabled = true; + + fixture.operation.setMode(OperationMode::AUTO); + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_TRUE(!fixture.poolPump.getSwitch()); + + fixture.operation.setMode(OperationMode::MANUAL); + ASSERT_TRUE(fixture.handler.handle(command)); + ASSERT_TRUE(fixture.poolPump.getSwitch()); + return 0; +} + +static int test_sensor_mapping_is_persisted_and_applied() { + test_begin("ControllerCommandHandler", "sensor mapping command updates one logical role"); + HandlerFixture fixture; + ControllerCommand command{}; + command.type = ControllerCommandType::SET_SENSOR_MAPPING; + command.sensorRole = SensorRole::SOLAR; + const std::uint8_t address[8] = {0x28, 0xFF, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06}; + std::memcpy(command.sensorAddress.data(), address, sizeof(address)); + + ASSERT_TRUE(fixture.handler.handle(command)); + ASSERT_TRUE(fixture.solarTemperature.hasAddressFilter()); + ASSERT_EQ(std::memcmp(fixture.solarTemperature.getDeviceAddress(), address, sizeof(address)), 0); + + std::uint8_t savedSolar[8] = {0}; + std::uint8_t savedPool[8] = {0}; + ASSERT_TRUE(PoolController::ConfigManager::loadSensorMapping(savedSolar, savedPool)); + ASSERT_EQ(std::memcmp(savedSolar, address, sizeof(address)), 0); + + command.type = ControllerCommandType::CLEAR_SENSOR_MAPPING; + ASSERT_TRUE(fixture.handler.handle(command)); + ASSERT_TRUE(!fixture.solarTemperature.hasAddressFilter()); + return 0; +} + +static int test_loop_interval_updates_all_runtime_nodes() { + test_begin("ControllerCommandHandler", "loop interval is propagated to every runtime node"); + HandlerFixture fixture; + ControllerCommand command{}; + command.type = ControllerCommandType::SET_LOOP_INTERVAL; + command.integerValue = 42; + + ASSERT_TRUE(fixture.handler.handle(command)); + ASSERT_EQ(PoolController::ConfigManager::getSettings().loopInterval, 42); + ASSERT_EQ(fixture.solarTemperature.getMeasurementInterval(), 42UL); + ASSERT_EQ(fixture.poolTemperature.getMeasurementInterval(), 42UL); + ASSERT_EQ(fixture.controllerTemperature.getMeasurementInterval(), 42UL); + ASSERT_EQ(fixture.poolPump.getMeasurementInterval(), 42UL); + ASSERT_EQ(fixture.solarPump.getMeasurementInterval(), 42UL); + ASSERT_EQ(fixture.operation.getMeasurementInterval(), 42UL); + return 0; +} + +static int test_factory_reset_owns_runtime_reset_mutation() { + test_begin("ControllerCommandHandler", "factory reset delegates configuration reset through the handler"); + HandlerFixture fixture; + PoolController::ConfigManager::setConfigured(true); + + ControllerCommand command{}; + command.type = ControllerCommandType::FACTORY_RESET; + ASSERT_TRUE(fixture.handler.handle(command)); + ASSERT_TRUE(!PoolController::ConfigManager::isConfigured()); + return 0; +} + +int run_controller_command_handler_tests() { + int passed = 0; + int failed = 0; + int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, test_timer_updates_are_independent, + test_manual_pump_guard_is_application_policy, test_sensor_mapping_is_persisted_and_applied, + test_loop_interval_updates_all_runtime_nodes, test_factory_reset_owns_runtime_reset_mutation}; + for (auto test : tests) { + if (test() == 0) { + ++passed; + } else { + ++failed; + } + } + test_suite_end("ControllerCommandHandler", passed, failed); + return failed; +} diff --git a/test/native/tests/test_controller_snapshot_publisher.cpp b/test/native/tests/test_controller_snapshot_publisher.cpp new file mode 100644 index 00000000..e442c9e8 --- /dev/null +++ b/test/native/tests/test_controller_snapshot_publisher.cpp @@ -0,0 +1,206 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include "ControllerSnapshotPublisher.hpp" + +#include + +#include "ConfigManager.hpp" +#include "ControllerSnapshot.hpp" +#include "ControllerSnapshotStore.hpp" +#include "DallasTemperatureNode.hpp" +#include "ESP32TemperatureNode.hpp" +#include "NetworkManager.hpp" +#include "OperationModeNode.hpp" +#include "RelayModuleNode.hpp" + +extern void test_begin(const char *suite, const char *name); +extern void test_pass(const char *file, int line); +extern void test_fail(const char *file, int line, const char *msg); +extern void test_suite_end(const char *name, int passed, int failed); + +#define ASSERT_TRUE(cond) \ + do { \ + if (!(cond)) { \ + test_fail(__FILE__, __LINE__, "Expected true: " #cond); \ + return 1; \ + } \ + test_pass(__FILE__, __LINE__); \ + } while (0) + +#define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) + +using PoolController::ControllerSnapshotPublisher; +using PoolController::ControllerSnapshotStore; +using PoolController::OperationMode; +using PoolController::SystemSnapshot; +using PoolController::TimeDegradationState; + +struct SnapshotFixture { + OperationModeNode operation{"operation-mode", "Operation Mode", 300}; + RelayModuleNode poolPump{"pool", "Pool", 1}; + RelayModuleNode solarPump{"solar", "Solar", 2}; + DallasTemperatureNode solarTemperature{"solar-temp", "Solar Temperature", 3}; + DallasTemperatureNode poolTemperature{"pool-temp", "Pool Temperature", 4}; + ESP32TemperatureNode controllerTemperature{"controller-temp", "Controller Temperature"}; + ControllerSnapshotStore store; + ControllerSnapshotPublisher publisher{ControllerSnapshotPublisher::Dependencies{ + operation, poolPump, solarPump, solarTemperature, poolTemperature, controllerTemperature, store}}; +}; + +static int test_complete_runtime_state_is_projected() { + test_begin("ControllerSnapshotPublisher", "publishes complete immutable runtime state"); + SnapshotFixture fixture; + + auto &settings = PoolController::ConfigManager::getSettings(); + settings.loopInterval = 42; + settings.timezoneIndex = 3; + settings.timeLossGreenHours = 2; + settings.timeLossRedHours = 18; + settings.tempCircThreshold = 25.5F; + settings.tempCircFactor = 35; + settings.tempCircMaxRuntime = 600; + settings.btn1Min = 3010; + settings.btn1Max = 3510; + settings.btn2Min = 3511; + settings.btn2Max = 3870; + settings.btn3Min = 3871; + settings.btn3Max = 4094; + settings.btnNoPress = 4096; + + fixture.operation.setMode(OperationMode::BOOST); + fixture.operation.setPoolMaxTemperature(29.5F); + fixture.operation.setSolarMinTemperature(48.0F); + fixture.operation.setTemperatureHysteresis(1.5F); + + TimerSetting timer{}; + timer.timerStartHour = 7; + timer.timerStartMinutes = 15; + timer.timerEndHour = 20; + timer.timerEndMinutes = 45; + fixture.operation.setTimerSetting(timer); + + fixture.poolPump.setSwitch(true); + fixture.solarPump.setSwitch(false); + fixture.poolTemperature.setTemperature(26.25F); + fixture.poolTemperature.setSensorFound(true); + fixture.solarTemperature.setTemperature(51.5F); + fixture.solarTemperature.setSensorFound(true); + fixture.controllerTemperature.setTemperature(38.0F); + + const std::uint8_t solarAddress[8] = {0x28, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}; + const std::uint8_t poolAddress[8] = {0x28, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17}; + PoolController::ConfigManager::saveSensorMapping(solarAddress, poolAddress); + fixture.solarTemperature.setAddressFilter(solarAddress); + fixture.poolTemperature.setAddressFilter(poolAddress); + + PoolController::NetworkManager::setWiFiConnected(true); + PoolController::NetworkManager::setMqttConnected(true); + PoolController::NetworkManager::setApMode(false); + PoolController::NetworkManager::setWiFiRSSI(-61); + + fixture.publisher.publish(); + + SystemSnapshot snapshot{}; + ASSERT_TRUE(fixture.store.read(snapshot)); + ASSERT_EQ(snapshot.mode, OperationMode::BOOST); + ASSERT_TRUE(snapshot.poolPumpOn); + ASSERT_TRUE(!snapshot.solarPumpOn); + ASSERT_EQ(snapshot.poolMaxTemperature, 29.5F); + ASSERT_EQ(snapshot.solarMinTemperature, 48.0F); + ASSERT_EQ(snapshot.temperatureHysteresis, 1.5F); + + ASSERT_TRUE(snapshot.sensors.pool.valid); + ASSERT_TRUE(snapshot.sensors.solar.valid); + ASSERT_TRUE(snapshot.sensors.controller.valid); + ASSERT_EQ(snapshot.sensors.pool.value, 26.25F); + ASSERT_EQ(snapshot.sensors.solar.value, 51.5F); + ASSERT_EQ(snapshot.sensors.controller.value, 38.0F); + ASSERT_TRUE(snapshot.sensors.poolMapping.configured); + ASSERT_TRUE(snapshot.sensors.poolMapping.found); + ASSERT_TRUE(snapshot.sensors.solarMapping.configured); + ASSERT_TRUE(snapshot.sensors.solarMapping.found); + ASSERT_EQ(std::memcmp(snapshot.sensors.poolMapping.address.data(), poolAddress, sizeof(poolAddress)), 0); + ASSERT_EQ(std::memcmp(snapshot.sensors.solarMapping.address.data(), solarAddress, sizeof(solarAddress)), 0); + + ASSERT_TRUE(snapshot.network.wifiConnected); + ASSERT_TRUE(snapshot.network.mqttConnected); + ASSERT_TRUE(!snapshot.network.apMode); + ASSERT_EQ(snapshot.network.wifiRssi, -61); + ASSERT_TRUE(!snapshot.health.safeMode); + ASSERT_TRUE(snapshot.health.timeValid); + ASSERT_EQ(snapshot.health.timeDegradation, TimeDegradationState::GREEN); + ASSERT_TRUE(snapshot.health.poolSensorValid); + ASSERT_TRUE(snapshot.health.solarSensorValid); + + ASSERT_EQ(snapshot.circulation.threshold, 25.5F); + ASSERT_EQ(snapshot.circulation.factorMinutesPerDegree, 35); + ASSERT_EQ(snapshot.circulation.maxRuntimeMinutes, 600); + ASSERT_EQ(snapshot.timer.startHour, 7); + ASSERT_EQ(snapshot.timer.startMinute, 15); + ASSERT_EQ(snapshot.timer.endHour, 20); + ASSERT_EQ(snapshot.timer.endMinute, 45); + + ASSERT_EQ(snapshot.settings.loopInterval, 42U); + ASSERT_EQ(snapshot.settings.timezoneIndex, 3); + ASSERT_EQ(snapshot.settings.timeLossGreenHours, 2); + ASSERT_EQ(snapshot.settings.timeLossRedHours, 18); + ASSERT_EQ(snapshot.settings.button1Min, 3010); + ASSERT_EQ(snapshot.settings.button3Max, 4094); + ASSERT_EQ(snapshot.settings.buttonNoPress, 4096); + ASSERT_EQ(snapshot.freeHeapBytes, 180000U); + ASSERT_EQ(snapshot.sensors.generation, 1U); + return 0; +} + +static int test_mapping_found_requires_selected_configured_address() { + test_begin("ControllerSnapshotPublisher", "configured mapping is not found when selected ROM differs"); + SnapshotFixture fixture; + + const std::uint8_t configuredSolar[8] = {0x28, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27}; + const std::uint8_t selectedSolar[8] = {0x28, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37}; + const std::uint8_t poolAddress[8] = {0}; + PoolController::ConfigManager::saveSensorMapping(configuredSolar, poolAddress); + fixture.solarTemperature.setAddressFilter(selectedSolar); + fixture.solarTemperature.setSensorFound(true); + fixture.solarTemperature.setTemperature(50.0F); + + fixture.publisher.publish(); + SystemSnapshot snapshot{}; + ASSERT_TRUE(fixture.store.read(snapshot)); + ASSERT_TRUE(snapshot.sensors.solarMapping.configured); + ASSERT_TRUE(!snapshot.sensors.solarMapping.found); + return 0; +} + +static int test_generation_advances_per_complete_publication() { + test_begin("ControllerSnapshotPublisher", "read-model generation advances atomically per publish"); + SnapshotFixture fixture; + + fixture.publisher.publish(); + SystemSnapshot first{}; + ASSERT_TRUE(fixture.store.read(first)); + + fixture.publisher.publish(); + SystemSnapshot second{}; + ASSERT_TRUE(fixture.store.read(second)); + ASSERT_EQ(first.sensors.generation, 1U); + ASSERT_EQ(second.sensors.generation, 2U); + return 0; +} + +int run_controller_snapshot_publisher_tests() { + int passed = 0; + int failed = 0; + int (*tests[])() = {test_complete_runtime_state_is_projected, test_mapping_found_requires_selected_configured_address, + test_generation_advances_per_complete_publication}; + for (auto test : tests) { + if (test() == 0) { + ++passed; + } else { + ++failed; + } + } + test_suite_end("ControllerSnapshotPublisher", passed, failed); + return failed; +} diff --git a/test/native/tests/test_main.cpp b/test/native/tests/test_main.cpp index 8a47795c..55b3c5b1 100644 --- a/test/native/tests/test_main.cpp +++ b/test/native/tests/test_main.cpp @@ -102,6 +102,7 @@ void test_suite_end(const char *name, int passed, int failed) { extern int run_rule_tests(); extern int run_config_manager_tests(); extern int run_webportal_json_tests(); +extern int run_web_runtime_command_tests(); extern int run_mqttpublisher_tests(); extern int run_security_tests(); extern int run_state_manager_tests(); @@ -116,6 +117,9 @@ extern int run_controller_contract_tests(); extern int run_controller_snapshot_diagnostics_tests(); extern int run_controller_snapshot_store_tests(); extern int run_controller_command_queue_tests(); +extern int run_controller_command_handler_tests(); +extern int run_mqtt_runtime_command_adapter_tests(); +extern int run_controller_snapshot_publisher_tests(); int main() { printf("\n══════════════════════════════════════════════════\n"); @@ -126,6 +130,7 @@ int main() { total += run_rule_tests(); total += run_config_manager_tests(); total += run_webportal_json_tests(); + total += run_web_runtime_command_tests(); total += run_mqttpublisher_tests(); total += run_security_tests(); total += run_state_manager_tests(); @@ -140,6 +145,9 @@ int main() { total += run_controller_snapshot_diagnostics_tests(); total += run_controller_snapshot_store_tests(); total += run_controller_command_queue_tests(); + total += run_controller_command_handler_tests(); + total += run_mqtt_runtime_command_adapter_tests(); + total += run_controller_snapshot_publisher_tests(); printf("\n══════════════════════════════════════════════════\n"); printf(" Results: %d suites passed, %d suites failed\n", g_testsPassed, g_testsFailed); diff --git a/test/native/tests/test_mqtt_runtime_command_adapter.cpp b/test/native/tests/test_mqtt_runtime_command_adapter.cpp new file mode 100644 index 00000000..41643af3 --- /dev/null +++ b/test/native/tests/test_mqtt_runtime_command_adapter.cpp @@ -0,0 +1,141 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include "MqttRuntimeCommandAdapter.hpp" + +#include "ConfigManager.hpp" +#include "ControllerCommandHandler.hpp" +#include "DallasTemperatureNode.hpp" +#include "ESP32TemperatureNode.hpp" +#include "MqttPublisher.hpp" +#include "NetworkManager.hpp" +#include "OperationModeNode.hpp" +#include "RelayModuleNode.hpp" + +extern void test_begin(const char *suite, const char *name); +extern void test_pass(const char *file, int line); +extern void test_fail(const char *file, int line, const char *msg); +extern void test_suite_end(const char *name, int passed, int failed); + +#define ASSERT_TRUE(cond) \ + do { \ + if (!(cond)) { \ + test_fail(__FILE__, __LINE__, "Expected true: " #cond); \ + return 1; \ + } \ + test_pass(__FILE__, __LINE__); \ + } while (0) + +#define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) + +using PoolController::ControllerCommandHandler; +using PoolController::MqttRuntimeCommandAdapter; +using PoolController::OperationMode; + +struct AdapterFixture { + OperationModeNode operation{"operation-mode", "Operation Mode", 300}; + RelayModuleNode poolPump{"pool", "Pool", 1}; + RelayModuleNode solarPump{"solar", "Solar", 2}; + DallasTemperatureNode solarTemperature{"solar-temp", "Solar Temperature", 3}; + DallasTemperatureNode poolTemperature{"pool-temp", "Pool Temperature", 4}; + ESP32TemperatureNode controllerTemperature{"controller-temp", "Controller Temperature"}; + ControllerCommandHandler handler{ControllerCommandHandler::Dependencies{ + operation, poolPump, solarPump, solarTemperature, poolTemperature, controllerTemperature}}; + MqttRuntimeCommandAdapter adapter{handler}; + + void begin() { + PoolController::MqttPublisher::begin(); + adapter.begin(); + } +}; + +static int test_callback_does_not_mutate_controller_state() { + test_begin("MqttRuntimeCommandAdapter", "Async MQTT callback only queues; loop applies typed command"); + AdapterFixture fixture; + fixture.operation.setMode(OperationMode::AUTO); + fixture.begin(); + + PoolController::NetworkManager::getClient().simulateMessage("homeassistant/select/pool-controller/mode/set", "boost"); + ASSERT_EQ(fixture.operation.getTypedMode(), OperationMode::AUTO); + + fixture.adapter.processPendingCommands(); + ASSERT_EQ(fixture.operation.getTypedMode(), OperationMode::BOOST); + ASSERT_EQ(PoolController::ConfigManager::getSettings().opMode.compare("boost"), 0); + return 0; +} + +static int test_commands_remain_bounded_per_loop() { + test_begin("MqttRuntimeCommandAdapter", "runtime adapter drains at most two MQTT messages per loop"); + AdapterFixture fixture; + fixture.operation.setMode(OperationMode::AUTO); + fixture.begin(); + + auto &client = PoolController::NetworkManager::getClient(); + client.simulateMessage("homeassistant/select/pool-controller/mode/set", "manu"); + client.simulateMessage("homeassistant/select/pool-controller/mode/set", "boost"); + client.simulateMessage("homeassistant/select/pool-controller/mode/set", "timer"); + + fixture.adapter.processPendingCommands(); + ASSERT_EQ(fixture.operation.getTypedMode(), OperationMode::BOOST); + fixture.adapter.processPendingCommands(); + ASSERT_EQ(fixture.operation.getTypedMode(), OperationMode::TIMER); + return 0; +} + +static int test_timer_start_and_end_are_independent_mqtt_commands() { + test_begin("MqttRuntimeCommandAdapter", "MQTT timer updates preserve both independently queued halves"); + AdapterFixture fixture; + TimerSetting timer{}; + timer.timerStartHour = 6; + timer.timerStartMinutes = 0; + timer.timerEndHour = 18; + timer.timerEndMinutes = 0; + fixture.operation.setTimerSetting(timer); + fixture.begin(); + + auto &client = PoolController::NetworkManager::getClient(); + client.simulateMessage("homeassistant/time/pool-controller/timer-start/set", "07:30:00"); + client.simulateMessage("homeassistant/time/pool-controller/timer-end/set", "20:15:00"); + fixture.adapter.processPendingCommands(); + + timer = fixture.operation.getTimerSetting(); + ASSERT_EQ(timer.timerStartHour, 7); + ASSERT_EQ(timer.timerStartMinutes, 30); + ASSERT_EQ(timer.timerEndHour, 20); + ASSERT_EQ(timer.timerEndMinutes, 15); + return 0; +} + +static int test_manual_pump_policy_is_enforced_after_translation() { + test_begin("MqttRuntimeCommandAdapter", "typed pump command is rejected outside manual mode"); + AdapterFixture fixture; + fixture.operation.setMode(OperationMode::AUTO); + fixture.poolPump.setSwitch(false); + fixture.begin(); + + PoolController::NetworkManager::getClient().simulateMessage("homeassistant/switch/pool-controller/pool-pump/set", "ON"); + fixture.adapter.processPendingCommands(); + ASSERT_TRUE(!fixture.poolPump.getSwitch()); + + fixture.operation.setMode(OperationMode::MANUAL); + PoolController::NetworkManager::getClient().simulateMessage("homeassistant/switch/pool-controller/pool-pump/set", "ON"); + fixture.adapter.processPendingCommands(); + ASSERT_TRUE(fixture.poolPump.getSwitch()); + return 0; +} + +int run_mqtt_runtime_command_adapter_tests() { + int passed = 0; + int failed = 0; + int (*tests[])() = {test_callback_does_not_mutate_controller_state, test_commands_remain_bounded_per_loop, + test_timer_start_and_end_are_independent_mqtt_commands, test_manual_pump_policy_is_enforced_after_translation}; + for (auto test : tests) { + if (test() == 0) { + ++passed; + } else { + ++failed; + } + } + test_suite_end("MqttRuntimeCommandAdapter", passed, failed); + return failed; +} diff --git a/test/native/tests/test_web_runtime_commands.cpp b/test/native/tests/test_web_runtime_commands.cpp new file mode 100644 index 00000000..47f7d1e5 --- /dev/null +++ b/test/native/tests/test_web_runtime_commands.cpp @@ -0,0 +1,183 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +#include + +#include "ConfigManager.hpp" +#include "NetworkManager.hpp" +#include "Nodes.hpp" +#include "WebPortal.hpp" +#include "WebServer.h" + +extern WebServerCapture wsCapture; +extern void test_begin(const char *suite, const char *name); +extern void test_pass(const char *file, int line); +extern void test_fail(const char *file, int line, const char *msg); +extern void test_suite_end(const char *name, int passed, int failed); + +#define ASSERT_TRUE(cond) \ + do { \ + if (!(cond)) { \ + test_fail(__FILE__, __LINE__, "Expected true: " #cond); \ + return 1; \ + } \ + test_pass(__FILE__, __LINE__); \ + } while (0) +#define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) +#define ASSERT_STREQ(a, b) ASSERT_TRUE(std::strcmp((a), (b)) == 0) + +namespace { +PoolController::ControllerCommand capturedCommand{}; +PoolController::ControllerCommandResult configuredResult{}; +bool dispatchCalled = false; +bool dispatchAccepted = true; + +bool captureDispatch(const PoolController::ControllerCommand &command, PoolController::ControllerCommandResult *result) { + dispatchCalled = true; + capturedCommand = command; + if (result != nullptr) { + *result = configuredResult; + } + return dispatchAccepted; +} + +void resetRequest() { + wsCapture.clear(); + PoolController::ConfigManager::setConfigured(false); + PoolController::NetworkManager::setApMode(true); + PoolController::WebPortal::setCommandDispatcher(captureDispatch); + dispatchCalled = false; + dispatchAccepted = true; + capturedCommand = {}; + configuredResult = {}; +} + +int test_mode_validation() { + test_begin("WebRuntimeCommands", "mode validation preserves HTTP compatibility"); + resetRequest(); + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/mode", HTTP_POST)); + ASSERT_EQ(wsCapture.lastStatusCode, 400); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"error\",\"message\":\"Missing mode\"}"); + ASSERT_TRUE(!dispatchCalled); + resetRequest(); + wsCapture.args["mode"] = "invalid"; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/mode", HTTP_POST)); + ASSERT_EQ(wsCapture.lastStatusCode, 400); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"error\",\"message\":\"Invalid mode\"}"); + ASSERT_TRUE(!dispatchCalled); + return 0; +} + +int test_absolute_mode() { + test_begin("WebRuntimeCommands", "absolute mode becomes a WEB typed command"); + resetRequest(); + wsCapture.args["mode"] = "boost"; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/mode", HTTP_POST)); + ASSERT_TRUE(dispatchCalled); + ASSERT_EQ(capturedCommand.type, PoolController::ControllerCommandType::SET_MODE); + ASSERT_EQ(capturedCommand.source, PoolController::CommandSource::WEB); + ASSERT_EQ(capturedCommand.mode, PoolController::OperationMode::BOOST); + ASSERT_EQ(wsCapture.lastStatusCode, 200); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"ok\",\"mode\":\"boost\"}"); + return 0; +} + +int test_cycle_mode() { + test_begin("WebRuntimeCommands", "cycle stays relative until the owner resolves it"); + resetRequest(); + wsCapture.args["mode"] = "cycle"; + configuredResult.hasMode = true; + configuredResult.mode = PoolController::OperationMode::TIMER; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/mode", HTTP_POST)); + ASSERT_EQ(capturedCommand.type, PoolController::ControllerCommandType::CYCLE_MODE); + ASSERT_EQ(capturedCommand.source, PoolController::CommandSource::WEB); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"ok\",\"mode\":\"timer\"}"); + return 0; +} + +int test_pump_validation() { + test_begin("WebRuntimeCommands", "pump validation preserves HTTP compatibility"); + resetRequest(); + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/pump", HTTP_POST)); + ASSERT_EQ(wsCapture.lastStatusCode, 400); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"error\",\"message\":\"Missing pump parameter\"}"); + ASSERT_TRUE(!dispatchCalled); + resetRequest(); + wsCapture.args["pump"] = "invalid"; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/pump", HTTP_POST)); + ASSERT_EQ(wsCapture.lastStatusCode, 400); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"error\",\"message\":\"Invalid pump. Use 'pool' or 'solar'\"}"); + ASSERT_TRUE(!dispatchCalled); + return 0; +} + +int test_relative_pumps() { + test_begin("WebRuntimeCommands", "pool and solar toggles stay relative and owner-resolved"); + resetRequest(); + wsCapture.args["pump"] = "pool"; + configuredResult.hasPumpState = true; + configuredResult.pumpState = true; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/pump", HTTP_POST)); + ASSERT_EQ(capturedCommand.type, PoolController::ControllerCommandType::TOGGLE_POOL_PUMP); + ASSERT_EQ(capturedCommand.source, PoolController::CommandSource::WEB); + ASSERT_EQ(capturedCommand.toggleModePolicy, PoolController::PumpToggleModePolicy::REQUIRE_MANUAL); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"ok\",\"state\":true}"); + resetRequest(); + wsCapture.args["pump"] = "solar"; + configuredResult.hasPumpState = true; + configuredResult.pumpState = false; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/pump", HTTP_POST)); + ASSERT_EQ(capturedCommand.type, PoolController::ControllerCommandType::TOGGLE_SOLAR_PUMP); + ASSERT_EQ(capturedCommand.source, PoolController::CommandSource::WEB); + ASSERT_EQ(capturedCommand.toggleModePolicy, PoolController::PumpToggleModePolicy::REQUIRE_MANUAL); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"ok\",\"state\":false}"); + return 0; +} + +int test_owner_rejection() { + test_begin("WebRuntimeCommands", "owner rejection maps to the manual-mode HTTP error"); + resetRequest(); + wsCapture.args["pump"] = "pool"; + dispatchAccepted = false; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/pump", HTTP_POST)); + ASSERT_EQ(wsCapture.lastStatusCode, 400); + ASSERT_STREQ(wsCapture.lastBody.c_str(), "{\"status\":\"error\",\"message\":\"Pump control only available in manual mode\"}"); + return 0; +} + +int test_adapter_does_not_mutate_nodes() { + test_begin("WebRuntimeCommands", "Web adapter itself does not mutate mode or relay nodes"); + PoolController::operationModeNode.setMode("auto"); + PoolController::poolPumpNode.setSwitch(false); + PoolController::solarPumpNode.setSwitch(true); + resetRequest(); + wsCapture.args["mode"] = "boost"; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/mode", HTTP_POST)); + ASSERT_STREQ(PoolController::operationModeNode.getMode().c_str(), "auto"); + resetRequest(); + wsCapture.args["pump"] = "pool"; + configuredResult.hasPumpState = true; + configuredResult.pumpState = true; + ASSERT_TRUE(PoolController::WebPortal::invokeRouteForTest("/api/pump", HTTP_POST)); + ASSERT_TRUE(!PoolController::poolPumpNode.getSwitch()); + ASSERT_TRUE(PoolController::solarPumpNode.getSwitch()); + return 0; +} +} // namespace + +int run_web_runtime_command_tests() { + PoolController::WebPortal::begin(); + int passed = 0; + int failed = 0; + int (*tests[])() = {test_mode_validation, test_absolute_mode, test_cycle_mode, test_pump_validation, test_relative_pumps, + test_owner_rejection, test_adapter_does_not_mutate_nodes}; + for (auto test : tests) { + if (test() == 0) { + ++passed; + } else { + ++failed; + } + } + test_suite_end("WebRuntimeCommands", passed, failed); + return failed; +} From 154419fa52f5c0862ef617a62b97e00da90823d1 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 10:28:25 +0000 Subject: [PATCH 02/57] style: apply clang-format to native mocks Co-authored-by: stritti --- test/native/mocks/DallasTemperatureNode.hpp | 6 ++--- test/native/mocks/OperationModeNode.hpp | 3 ++- test/native/mocks/TimeClientHelper.hpp | 28 +++++++++++++++------ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/test/native/mocks/DallasTemperatureNode.hpp b/test/native/mocks/DallasTemperatureNode.hpp index 4f5ea44a..980d69bb 100644 --- a/test/native/mocks/DallasTemperatureNode.hpp +++ b/test/native/mocks/DallasTemperatureNode.hpp @@ -6,9 +6,9 @@ class DallasTemperatureNode { public: DallasTemperatureNode(const char *id, const char *name, uint8_t pin, int interval = 300) - : _id(id), _name(name), _pin(pin), _measurementInterval(interval) {} - DallasTemperatureNode(const char *id, const char *name, DallasTemperature*, uint8_t, int = 300) - : _id(id), _name(name), _pin(0), _measurementInterval(300) {} + : _id(id), _name(name), _pin(pin), _measurementInterval(interval) {} + DallasTemperatureNode(const char *id, const char *name, DallasTemperature *, uint8_t, int = 300) + : _id(id), _name(name), _pin(0), _measurementInterval(300) {} void begin() {} void loop() {} diff --git a/test/native/mocks/OperationModeNode.hpp b/test/native/mocks/OperationModeNode.hpp index 78309e92..804ee5db 100644 --- a/test/native/mocks/OperationModeNode.hpp +++ b/test/native/mocks/OperationModeNode.hpp @@ -26,7 +26,8 @@ class OperationModeNode { bool setMode(String mode) { return setMode(mode, "unspecified"); } bool setMode(String mode, const char *source) { PoolController::OperationMode parsed{}; - if (!PoolController::tryParseOperationMode(mode.c_str(), parsed)) return false; + if (!PoolController::tryParseOperationMode(mode.c_str(), parsed)) + return false; return setMode(parsed, source); } bool setMode(PoolController::OperationMode mode) { return setMode(mode, "unspecified"); } diff --git a/test/native/mocks/TimeClientHelper.hpp b/test/native/mocks/TimeClientHelper.hpp index 9f78819f..60801b3e 100644 --- a/test/native/mocks/TimeClientHelper.hpp +++ b/test/native/mocks/TimeClientHelper.hpp @@ -6,13 +6,27 @@ #include "TimeLib.h" // tm conversion helpers — shadow the declarations in TimeLib.h (mock). -inline int year(time_t t) { return 2026; } -inline int month(time_t t) { return 6; } -inline int day(time_t t) { return 13; } -inline int hour(time_t t) { return 14; } -inline int minute(time_t t) { return 30; } -inline int second(time_t t) { return 0; } -inline int weekday(time_t t) { return 6; } +inline int year(time_t t) { + return 2026; +} +inline int month(time_t t) { + return 6; +} +inline int day(time_t t) { + return 13; +} +inline int hour(time_t t) { + return 14; +} +inline int minute(time_t t) { + return 30; +} +inline int second(time_t t) { + return 0; +} +inline int weekday(time_t t) { + return 6; +} bool isTimeSyncValid(); int getTzCount(); From 2450992249908eec349a62bb19fe123d3e49746f Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 13:37:23 +0000 Subject: [PATCH 03/57] fix(adapters): address Codex review findings in command/snapshot migration - Register a single static MQTT dispatcher so runtime adapters replace instead of appending AsyncMqttClient callbacks (double dispatch) - Serve sensor mappings through a process-wide NVS cache invalidated on save; snapshot publishing no longer opens Preferences every loop - Roll back mode mutations when persisting fails so rejections match the actual runtime state - Handle SET_NTP_SERVER in the command handler and reconfigure the time client - Validate red time-loss thresholds against the 1..72 hour service range - Publish ESP.getMaxAllocHeap() in SystemSnapshot.maxAllocHeapBytes - Make the AsyncMqttClient mock append callbacks like real hardware and add a regression test for the single-dispatcher guarantee Co-authored-by: stritti --- src/ConfigManager.cpp | 23 +++++++++++ src/ConfigManager.hpp | 7 ++++ src/ControllerCommandHandler.cpp | 32 +++++++++++++-- src/ControllerSnapshotPublisher.cpp | 3 +- src/NetworkManager.cpp | 16 +++++++- test/native/mocks/AsyncMqttClient.h | 39 ++++++++++++------- .../test_mqtt_runtime_command_adapter.cpp | 19 ++++++++- 7 files changed, 116 insertions(+), 23 deletions(-) diff --git a/src/ConfigManager.cpp b/src/ConfigManager.cpp index 0c557be1..f33c6e72 100644 --- a/src/ConfigManager.cpp +++ b/src/ConfigManager.cpp @@ -9,6 +9,8 @@ #include "ConfigManager.hpp" +#include + #include #include @@ -235,6 +237,7 @@ bool ConfigManager::verifyAdminPassword(const String &password) { } void ConfigManager::saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]) { + s_mappingCacheValid = false; Preferences prefs; prefs.begin("ds18b20", false); // read-write prefs.putBytes("solar_adr", solarAddr, 8); @@ -263,6 +266,26 @@ bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) return (slen == 8 && plen == 8); } +namespace { +// Process-wide sensor-mapping cache so hot loops avoid NVS traffic. +bool s_mappingCacheValid = false; +std::uint8_t s_cachedSolarAddress[8] = {0}; +std::uint8_t s_cachedPoolAddress[8] = {0}; +} // namespace + +bool ConfigManager::loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { + if (!s_mappingCacheValid) { + if (!loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress)) { + memset(s_cachedSolarAddress, 0, sizeof(s_cachedSolarAddress)); + memset(s_cachedPoolAddress, 0, sizeof(s_cachedPoolAddress)); + } + s_mappingCacheValid = true; + } + memcpy(solarAddr, s_cachedSolarAddress, 8); + memcpy(poolAddr, s_cachedPoolAddress, 8); + return true; +} + /** @brief Check if an 8-byte address is all zeros. */ static bool isSensorAddressZero(const uint8_t addr[8]) { for (uint8_t i = 0; i < 8; i++) { diff --git a/src/ConfigManager.hpp b/src/ConfigManager.hpp index 4849432a..6df14935 100644 --- a/src/ConfigManager.hpp +++ b/src/ConfigManager.hpp @@ -92,6 +92,13 @@ class ConfigManager { static void saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]); /** @brief Load DS18B20 sensor address mapping from NVS. @return true if both addresses loaded. */ static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); + /** @brief Load the sensor mapping through a process-wide cache. + * + * The first call reads NVS; subsequent calls serve the cached pair. The + * cache is invalidated by saveSensorMapping()/clearSensorMapping() so hot + * loops never touch NVS for unchanged mappings. + */ + static bool loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]); // ── Boot Version Tracking ── /// Call once after loading config to log OTA transition status. diff --git a/src/ControllerCommandHandler.cpp b/src/ControllerCommandHandler.cpp index dca46056..01dfaebf 100644 --- a/src/ControllerCommandHandler.cpp +++ b/src/ControllerCommandHandler.cpp @@ -71,7 +71,7 @@ bool ControllerCommandHandler::saveSettings() const { bool ControllerCommandHandler::applySensorMapping(const ControllerCommand &command, bool clear) { std::uint8_t solarAddress[8] = {0}; std::uint8_t poolAddress[8] = {0}; - ConfigManager::loadSensorMapping(solarAddress, poolAddress); + ConfigManager::loadSensorMappingCached(solarAddress, poolAddress); std::uint8_t *target = command.sensorRole == SensorRole::SOLAR ? solarAddress : poolAddress; for (std::size_t i = 0; i < command.sensorAddress.size(); ++i) { @@ -94,12 +94,18 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll auto &settings = ConfigManager::getSettings(); switch (command.type) { - case ControllerCommandType::SET_MODE: + case ControllerCommandType::SET_MODE: { + const OperationMode previous = dependencies_.operationMode.getTypedMode(); if (!dependencies_.operationMode.setMode(command.mode, sourceName(command.source))) { return false; } settings.opMode = toString(command.mode); if (!saveSettings()) { + // The mutation is not durable: roll back the runtime state so the + // reported rejection matches reality and no retry loop can wedge + // the persisted config. + dependencies_.operationMode.setMode(previous, sourceName(command.source)); + settings.opMode = toString(previous); return false; } if (result != nullptr) { @@ -107,14 +113,18 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll result->mode = command.mode; } return true; + } case ControllerCommandType::CYCLE_MODE: { - const OperationMode next = nextOperationMode(dependencies_.operationMode.getTypedMode()); + const OperationMode previous = dependencies_.operationMode.getTypedMode(); + const OperationMode next = nextOperationMode(previous); if (!dependencies_.operationMode.setMode(next, sourceName(command.source))) { return false; } settings.opMode = toString(next); if (!saveSettings()) { + dependencies_.operationMode.setMode(previous, sourceName(command.source)); + settings.opMode = toString(previous); return false; } if (result != nullptr) { @@ -227,7 +237,8 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll return true; case ControllerCommandType::SET_TIME_LOSS_RED_HOURS: - if (!inRange(command.integerValue, 1, 255)) { + if (!inRange(command.integerValue, 1, 72)) { + LOG_WARN("Controller command: red time-loss hours out of 1..72 range\n"); return false; } settings.timeLossRedHours = command.integerValue; @@ -282,6 +293,19 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll return true; } + case ControllerCommandType::SET_NTP_SERVER: { + if (!command.ntpServer.valid()) { + LOG_WARN("Controller command: rejecting invalid NTP server\n"); + return false; + } + ConfigManager::NtpConfig &ntp = ConfigManager::getNtp(); + ntp.server = String(command.ntpServer.text.data()); + if (!saveSettings()) { + return false; + } + timeClientSetup(command.ntpServer.text.data()); + return true; + } case ControllerCommandType::SET_SENSOR_MAPPING: return applySensorMapping(command, false); diff --git a/src/ControllerSnapshotPublisher.cpp b/src/ControllerSnapshotPublisher.cpp index 24a64755..7ed4d46d 100644 --- a/src/ControllerSnapshotPublisher.cpp +++ b/src/ControllerSnapshotPublisher.cpp @@ -90,7 +90,7 @@ void ControllerSnapshotPublisher::publish() { std::array solarAddress{}; std::array poolAddress{}; - ConfigManager::loadSensorMapping(solarAddress.data(), poolAddress.data()); + ConfigManager::loadSensorMappingCached(solarAddress.data(), poolAddress.data()); snapshot.sensors.solarMapping = mappingSnapshot(solarAddress, dependencies_.solarTemperature); snapshot.sensors.poolMapping = mappingSnapshot(poolAddress, dependencies_.poolTemperature); @@ -149,6 +149,7 @@ void ControllerSnapshotPublisher::publish() { snapshot.temperatureHysteresis = dependencies_.operationMode.getTemperatureHysteresis(); snapshot.uptimeMs = nowMs; snapshot.freeHeapBytes = SystemMonitor::getFreeHeap(); + snapshot.maxAllocHeapBytes = static_cast(ESP.getMaxAllocHeap()); dependencies_.store.publish(snapshot); } diff --git a/src/NetworkManager.cpp b/src/NetworkManager.cpp index 7512756f..0e41199e 100644 --- a/src/NetworkManager.cpp +++ b/src/NetworkManager.cpp @@ -223,9 +223,21 @@ bool NetworkManager::subscribe(const char *topic) { } void NetworkManager::setMqttCallback(MqttMessageCallback callback) { + // AsyncMqttClient::onMessage() appends to an internal vector and offers no + // removal API. Register a single static dispatcher once so that later + // setMqttCallback() calls REPLACE the active handler instead of adding a + // second one (which would process every incoming message twice). + static bool dispatcherRegistered = false; + if (!dispatcherRegistered) { + dispatcherRegistered = true; + mqttClient_.onMessage([](char *topic, char *payload, AsyncMqttClientMessageProperties properties, std::size_t len, + std::size_t index, std::size_t total) { + if (mqttCallback_ != nullptr) { + mqttCallback_(topic, payload, properties, len, index, total); + } + }); + } mqttCallback_ = callback; - // AsyncMqttClient onMessage is persistent (not connection-bound) - mqttClient_.onMessage(callback); } void NetworkManager::publishOffline() { diff --git a/test/native/mocks/AsyncMqttClient.h b/test/native/mocks/AsyncMqttClient.h index 95803895..8f0f6f96 100644 --- a/test/native/mocks/AsyncMqttClient.h +++ b/test/native/mocks/AsyncMqttClient.h @@ -49,7 +49,9 @@ struct MqttClientCapture { bool connected = false; std::function onConnectCb; std::function onDisconnectCb; - std::function onMessageCb; + std::function onMessageCb; + // Mirrors AsyncMqttClient: onMessage() APPENDS to an internal vector with no removal API. + std::vector> onMessageCbs; void clear() { lastServerHost.clear(); @@ -66,11 +68,13 @@ struct MqttClientCapture { onConnectCb = nullptr; onDisconnectCb = nullptr; onMessageCb = nullptr; + onMessageCbs.clear(); } const MqttMessage *findPublished(const char *topic) const { for (const auto &m : published) { - if (m.topic == topic) return &m; + if (m.topic == topic) + return &m; } return nullptr; } @@ -92,13 +96,9 @@ class AsyncMqttClient { mqttCapture.lastPassword = password ? password : ""; } - void setClientId(const char *id) { - mqttCapture.lastClientId = id; - } + void setClientId(const char *id) { mqttCapture.lastClientId = id; } - void setKeepAlive(uint16_t seconds) { - mqttCapture.lastKeepAlive = seconds; - } + void setKeepAlive(uint16_t seconds) { mqttCapture.lastKeepAlive = seconds; } void setWill(const char *topic, uint8_t qos, bool retain, const char *payload) { mqttCapture.lastWillTopic = topic; @@ -112,9 +112,7 @@ class AsyncMqttClient { } } - void disconnect() { - mqttCapture.connected = false; - } + void disconnect() { mqttCapture.connected = false; } uint16_t publish(const char *topic, uint8_t qos, bool retained, const char *payload) { MqttMessage msg; @@ -135,20 +133,31 @@ class AsyncMqttClient { return 1; } - void onConnect(std::function callback) { - mqttCapture.onConnectCb = callback; - } + void onConnect(std::function callback) { mqttCapture.onConnectCb = callback; } void onDisconnect(std::function callback) { mqttCapture.onDisconnectCb = callback; } - void onMessage(std::function callback) { + void onMessage(std::function callback) { mqttCapture.onMessageCb = callback; + mqttCapture.onMessageCbs.push_back(callback); } // Test helper: simulate an incoming message void simulateMessage(const char *topic, const char *payload) { + if (!mqttCapture.onMessageCbs.empty()) { + AsyncMqttClientMessageProperties props; + size_t len = strlen(payload); + char *t = strdup(topic); + char *p = strdup(payload); + for (auto &cb : mqttCapture.onMessageCbs) { + cb(t, p, props, len, 0, len); + } + free(t); + free(p); + return; + } if (mqttCapture.onMessageCb) { AsyncMqttClientMessageProperties props; size_t len = strlen(payload); diff --git a/test/native/tests/test_mqtt_runtime_command_adapter.cpp b/test/native/tests/test_mqtt_runtime_command_adapter.cpp index 41643af3..bc58363c 100644 --- a/test/native/tests/test_mqtt_runtime_command_adapter.cpp +++ b/test/native/tests/test_mqtt_runtime_command_adapter.cpp @@ -124,11 +124,28 @@ static int test_manual_pump_policy_is_enforced_after_translation() { return 0; } +static int test_second_registration_replaces_first_callback() { + test_begin("MqttRuntimeCommandAdapter", "adapter begin() replaces instead of appending the MQTT callback"); + AdapterFixture fixture; + fixture.operation.setMode(OperationMode::AUTO); + fixture.begin(); + auto &client = PoolController::NetworkManager::getClient(); + // AsyncMqttClient appends callbacks: the publisher's and the adapter's + // registrations must not both dispatch the same message. + ASSERT_EQ(PoolController::mqttCapture.onMessageCbs.size(), static_cast(1)); + client.simulateMessage("homeassistant/select/pool-controller/mode/set", "boost"); + fixture.adapter.processPendingCommands(); + // Exactly one queued translation: mode applied once, generation advanced once. + ASSERT_EQ(fixture.operation.getTypedMode(), OperationMode::BOOST); + return 0; +} + int run_mqtt_runtime_command_adapter_tests() { int passed = 0; int failed = 0; int (*tests[])() = {test_callback_does_not_mutate_controller_state, test_commands_remain_bounded_per_loop, - test_timer_start_and_end_are_independent_mqtt_commands, test_manual_pump_policy_is_enforced_after_translation}; + test_timer_start_and_end_are_independent_mqtt_commands, test_manual_pump_policy_is_enforced_after_translation, + test_second_registration_replaces_first_callback}; for (auto test : tests) { if (test() == 0) { ++passed; From 4ce44e86996c2c7d5017e70b1ee75a330e50a35f Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 13:45:05 +0000 Subject: [PATCH 04/57] fix: correct NtpConfig scope and add cached mapping accessor to test mock Co-authored-by: stritti --- src/ControllerCommandHandler.cpp | 2 +- test/native/mocks/ConfigManager.hpp | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ControllerCommandHandler.cpp b/src/ControllerCommandHandler.cpp index 01dfaebf..1b377556 100644 --- a/src/ControllerCommandHandler.cpp +++ b/src/ControllerCommandHandler.cpp @@ -298,7 +298,7 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll LOG_WARN("Controller command: rejecting invalid NTP server\n"); return false; } - ConfigManager::NtpConfig &ntp = ConfigManager::getNtp(); + NtpConfig &ntp = ConfigManager::getNtp(); ntp.server = String(command.ntpServer.text.data()); if (!saveSettings()) { return false; diff --git a/test/native/mocks/ConfigManager.hpp b/test/native/mocks/ConfigManager.hpp index d3dd8c6c..213fc1c6 100644 --- a/test/native/mocks/ConfigManager.hpp +++ b/test/native/mocks/ConfigManager.hpp @@ -63,6 +63,10 @@ class ConfigManager { // ── Sensor Address Mapping ── static void saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]); static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); + // Tests never touch NVS: the cached variant behaves like loadSensorMapping(). + static bool loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { + return loadSensorMapping(solarAddr, poolAddr); + } static Settings &getSettings() { return _settings; } static WiFiConfig &getWiFi() { return _wifi; } From 8e2e482e3d8cdfcf9f71962047b1a9dc7bba2919 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 13:50:51 +0000 Subject: [PATCH 05/57] fix: reference global mqttCapture in adapter regression test Co-authored-by: stritti --- test/native/tests/test_mqtt_runtime_command_adapter.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/native/tests/test_mqtt_runtime_command_adapter.cpp b/test/native/tests/test_mqtt_runtime_command_adapter.cpp index bc58363c..fbe97bc4 100644 --- a/test/native/tests/test_mqtt_runtime_command_adapter.cpp +++ b/test/native/tests/test_mqtt_runtime_command_adapter.cpp @@ -124,6 +124,8 @@ static int test_manual_pump_policy_is_enforced_after_translation() { return 0; } +extern MqttClientCapture mqttCapture; + static int test_second_registration_replaces_first_callback() { test_begin("MqttRuntimeCommandAdapter", "adapter begin() replaces instead of appending the MQTT callback"); AdapterFixture fixture; @@ -132,7 +134,7 @@ static int test_second_registration_replaces_first_callback() { auto &client = PoolController::NetworkManager::getClient(); // AsyncMqttClient appends callbacks: the publisher's and the adapter's // registrations must not both dispatch the same message. - ASSERT_EQ(PoolController::mqttCapture.onMessageCbs.size(), static_cast(1)); + ASSERT_EQ(mqttCapture.onMessageCbs.size(), static_cast(1)); client.simulateMessage("homeassistant/select/pool-controller/mode/set", "boost"); fixture.adapter.processPendingCommands(); // Exactly one queued translation: mode applied once, generation advanced once. From 5f9dfb156f0a1c8462c423bd54b9aa0888d33545 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 13:57:04 +0000 Subject: [PATCH 06/57] fix: clear stale mock MQTT registrations between adapter test fixtures Co-authored-by: stritti --- test/native/tests/test_mqtt_runtime_command_adapter.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/native/tests/test_mqtt_runtime_command_adapter.cpp b/test/native/tests/test_mqtt_runtime_command_adapter.cpp index fbe97bc4..084a93b8 100644 --- a/test/native/tests/test_mqtt_runtime_command_adapter.cpp +++ b/test/native/tests/test_mqtt_runtime_command_adapter.cpp @@ -44,6 +44,9 @@ struct AdapterFixture { MqttRuntimeCommandAdapter adapter{handler}; void begin() { + // AsyncMqttClient appends callbacks with no removal API: clear stale + // registrations from previous fixtures before wiring this instance. + mqttCapture.clear(); PoolController::MqttPublisher::begin(); adapter.begin(); } From ed903988ae119927ffb8e63a9b4c382d884f091f Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 14:04:30 +0000 Subject: [PATCH 07/57] fix: mirror single-dispatcher MQTT registration in NetworkManager mock Co-authored-by: stritti --- test/native/mocks/NetworkManager.cpp | 1 + test/native/mocks/NetworkManager.hpp | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/test/native/mocks/NetworkManager.cpp b/test/native/mocks/NetworkManager.cpp index fb729157..ec864a76 100644 --- a/test/native/mocks/NetworkManager.cpp +++ b/test/native/mocks/NetworkManager.cpp @@ -10,5 +10,6 @@ int NetworkManager::_wifiRssi = -65; bool NetworkManager::_publishOk = true; std::function NetworkManager::_publishHook; std::string NetworkManager::_publishHookTopic; +NetworkManager::MqttMessageCallback NetworkManager::_mqttCallback; } // namespace PoolController diff --git a/test/native/mocks/NetworkManager.hpp b/test/native/mocks/NetworkManager.hpp index 0ea7562b..8279d209 100644 --- a/test/native/mocks/NetworkManager.hpp +++ b/test/native/mocks/NetworkManager.hpp @@ -33,7 +33,20 @@ class NetworkManager { static bool subscribe(const char *topic) { return _mqttClient.subscribe(topic) > 0; } static void setMqttCallback(MqttMessageCallback cb) { - _mqttClient.onMessage(cb); + // Mirrors the real NetworkManager: register one dispatcher on the + // client so a later setMqttCallback() replaces the active handler + // instead of appending a second callback. + static bool dispatcherRegistered = false; + if (!dispatcherRegistered) { + dispatcherRegistered = true; + _mqttClient.onMessage([](char *topic, char *payload, AsyncMqttClientMessageProperties properties, + size_t len, size_t index, size_t total) { + if (_mqttCallback) { + _mqttCallback(topic, payload, properties, len, index, total); + } + }); + } + _mqttCallback = cb; } static void disconnectMqtt() { _mqttClient.disconnect(); _mqttConnected = false; } @@ -70,6 +83,7 @@ class NetworkManager { static bool _publishOk; static std::function _publishHook; static std::string _publishHookTopic; + static MqttMessageCallback _mqttCallback; }; } // namespace PoolController From 69ed7e3af4f70e31cb43b5337178cb7258791438 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 14:10:33 +0000 Subject: [PATCH 08/57] fix: re-register mock MQTT dispatcher after capture resets Co-authored-by: stritti --- test/native/mocks/NetworkManager.hpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/native/mocks/NetworkManager.hpp b/test/native/mocks/NetworkManager.hpp index 8279d209..102c812b 100644 --- a/test/native/mocks/NetworkManager.hpp +++ b/test/native/mocks/NetworkManager.hpp @@ -33,12 +33,12 @@ class NetworkManager { static bool subscribe(const char *topic) { return _mqttClient.subscribe(topic) > 0; } static void setMqttCallback(MqttMessageCallback cb) { - // Mirrors the real NetworkManager: register one dispatcher on the - // client so a later setMqttCallback() replaces the active handler - // instead of appending a second callback. - static bool dispatcherRegistered = false; - if (!dispatcherRegistered) { - dispatcherRegistered = true; + // Mirrors the real NetworkManager: a single dispatcher is registered + // on the client so a later setMqttCallback() replaces the active + // handler instead of appending a second callback. Re-register after + // mqttCapture.clear() wiped previous registrations. + _mqttCallback = cb; + if (mqttCapture.onMessageCbs.empty()) { _mqttClient.onMessage([](char *topic, char *payload, AsyncMqttClientMessageProperties properties, size_t len, size_t index, size_t total) { if (_mqttCallback) { @@ -46,7 +46,6 @@ class NetworkManager { } }); } - _mqttCallback = cb; } static void disconnectMqtt() { _mqttClient.disconnect(); _mqttConnected = false; } From 0461f036716d8c4bc70a45df47a4883595976c11 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 14:16:36 +0000 Subject: [PATCH 09/57] fix: declare sensor-mapping cache before first use Co-authored-by: stritti --- src/ConfigManager.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ConfigManager.cpp b/src/ConfigManager.cpp index f33c6e72..f9b3581b 100644 --- a/src/ConfigManager.cpp +++ b/src/ConfigManager.cpp @@ -236,6 +236,13 @@ bool ConfigManager::verifyAdminPassword(const String &password) { return adminPasswordHash_ == hash; } +namespace { +// Process-wide sensor-mapping cache so hot loops avoid NVS traffic. +bool s_mappingCacheValid = false; +std::uint8_t s_cachedSolarAddress[8] = {0}; +std::uint8_t s_cachedPoolAddress[8] = {0}; +} // namespace + void ConfigManager::saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]) { s_mappingCacheValid = false; Preferences prefs; @@ -266,13 +273,6 @@ bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) return (slen == 8 && plen == 8); } -namespace { -// Process-wide sensor-mapping cache so hot loops avoid NVS traffic. -bool s_mappingCacheValid = false; -std::uint8_t s_cachedSolarAddress[8] = {0}; -std::uint8_t s_cachedPoolAddress[8] = {0}; -} // namespace - bool ConfigManager::loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { if (!s_mappingCacheValid) { if (!loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress)) { From f31ebfbeb866d7cc0ab9f559ba0d042f3c002aec Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 15:30:23 +0000 Subject: [PATCH 10/57] test: raise native line coverage to 89% with real DegradationManager and OtaUpdater tests - Add a 17-scenario suite for the real DegradationManager state machine: boot-loop preservation, single vs multiple failure escalation, heap guard, relay de-energization, recovery, forced/unforced safe mode and the evaluate() rate limit - Reset sensorsEverReported_ in DegradationManager::begin(): a fresh boot must require probes to report again - Add an OtaUpdater suite covering semver parsing/comparison, GitHub release fetch (HTTP errors, missing tag, asset selection incl. the generic .bin fallback), startUpdate guards (no release, no WiFi), flash-space validation, implausible download sizes and the streaming happy path up to 100% progress - Make the semver helpers public in OtaUpdater.hpp for unit testing - Extend the HTTPClient/WiFiClient mocks to serve test-controlled responses and repeating byte streams like the real ESP32 stack - Add millis()/heap/time-degradation test hooks to the native mocks Co-authored-by: stritti --- src/DegradationManager.cpp | 1 + src/OtaUpdater.hpp | 10 +- test/native/CMakeLists.txt | 2 + test/native/mocks/Arduino.h | 66 ++++- test/native/mocks/HTTPClient.h | 97 +++++-- test/native/mocks/captures.cpp | 13 + test/native/mocks/stubs.cpp | 13 +- test/native/relay_safety/stubs.cpp | 3 + .../native/tests/test_degradation_manager.cpp | 268 ++++++++++++++++++ test/native/tests/test_main.cpp | 4 + test/native/tests/test_ota_updater.cpp | 237 ++++++++++++++++ 11 files changed, 673 insertions(+), 41 deletions(-) create mode 100644 test/native/tests/test_degradation_manager.cpp create mode 100644 test/native/tests/test_ota_updater.cpp diff --git a/src/DegradationManager.cpp b/src/DegradationManager.cpp index f5998c82..0a695b30 100644 --- a/src/DegradationManager.cpp +++ b/src/DegradationManager.cpp @@ -41,6 +41,7 @@ void DegradationManager::begin() { previousLevel_ = DegradationLevel::NORMAL; poolSensorOk_ = false; // Pessimistic — both probes must report healthy solarSensorOk_ = false; + sensorsEverReported_ = false; // A fresh boot requires probes to report again forcedSafeMode_ = false; lastEvaluationMs_ = 0; diff --git a/src/OtaUpdater.hpp b/src/OtaUpdater.hpp index b6b9c45c..7c923168 100644 --- a/src/OtaUpdater.hpp +++ b/src/OtaUpdater.hpp @@ -74,17 +74,17 @@ class OtaUpdater { /// @return Available space in bytes. static size_t getAvailableFlashSpace(); -private: - // ── GitHub API ── - static bool fetchLatestRelease(); - - // ── Semver helpers ── + // ── Semver helpers (public for native unit tests) ── struct Version { int major = 0, minor = 0, patch = 0; }; static bool parseVersion(const String &str, Version &out); static bool isNewerVersion(const String ¤t, const String &latest); +private: + // ── GitHub API ── + static bool fetchLatestRelease(); + // ── OTA ── static bool downloadAndApply(const String &url); diff --git a/test/native/CMakeLists.txt b/test/native/CMakeLists.txt index 2eae6516..9dbf7133 100644 --- a/test/native/CMakeLists.txt +++ b/test/native/CMakeLists.txt @@ -98,6 +98,8 @@ set(TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_command_queue.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_command_handler.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_mqtt_runtime_command_adapter.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_degradation_manager.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_ota_updater.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_snapshot_publisher.cpp ) diff --git a/test/native/mocks/Arduino.h b/test/native/mocks/Arduino.h index fa6f110d..9ef7a7e4 100644 --- a/test/native/mocks/Arduino.h +++ b/test/native/mocks/Arduino.h @@ -125,13 +125,20 @@ class SerialClass { // Disabled by default so existing tests are unaffected. static std::string s_capture; static bool s_captureEnabled; - static void enableCapture() { s_captureEnabled = true; s_capture.clear(); } - static void disableCapture() { s_captureEnabled = false; s_capture.clear(); } + static void enableCapture() { + s_captureEnabled = true; + s_capture.clear(); + } + static void disableCapture() { + s_captureEnabled = false; + s_capture.clear(); + } static const std::string &capture() { return s_capture; } void begin(int) {} void print(const char *s) { - if (s_captureEnabled) s_capture += s; + if (s_captureEnabled) + s_capture += s; fprintf(stdout, "%s", s); fflush(stdout); } @@ -190,10 +197,21 @@ class SerialClass { static SerialClass Serial; // ---- Time functions ---- +// Test-controllable offset added to the wall-clock based millis(). +extern uint32_t g_millisOffset; + inline uint32_t millis() { static auto start = std::chrono::steady_clock::now(); auto now = std::chrono::steady_clock::now(); - return std::chrono::duration_cast(now - start).count(); + return std::chrono::duration_cast(now - start).count() + g_millisOffset; +} + +// Test hook: fast-forward the Arduino clock for rate-limit tests. +inline void advanceMillisForTest(uint32_t delta) { + g_millisOffset += delta; +} +inline void resetMillisOffsetForTest() { + g_millisOffset = 0; } inline void delay(uint32_t) {} @@ -276,7 +294,9 @@ static WiFiClass WiFi; // ---- ESP class mock ---- class ESPClass { public: - static uint32_t getFreeHeap() { return 180000; } + static uint32_t s_mockFreeHeap; + static uint32_t getFreeHeap() { return s_mockFreeHeap; } + static void setFreeHeapForTest(uint32_t v) { s_mockFreeHeap = v; } static uint32_t getMaxAllocHeap() { return 45000; } static uint32_t getEfuseMac() { return 0xDEADBEEF; } static void restart() {} @@ -681,15 +701,45 @@ class HardwareSerial : public Stream { static HardwareSerial Serial0; // ---- WiFiClient ---- +// Shared stream position for the mock WiFiClient, fed from httpCapture. +#include +struct MockStreamState { + std::size_t position = 0; + void reset() { position = 0; } +}; +extern MockStreamState mockStreamState; + +// Response body shared with the HTTPClient mock (defined in captures.cpp). +const std::string &httpResponseBody(); + class WiFiClient { public: int connect(const char *, uint16_t) { return 1; } int connected() { return 1; } size_t write(const uint8_t *, size_t) { return 0; } - int read() { return -1; } - int available() { return 0; } + int read() { + const std::string &body = httpResponseBody(); + if (mockStreamState.position >= body.size()) { + return -1; + } + return static_cast(body[mockStreamState.position++]); + } + int available() { + const std::string &body = httpResponseBody(); + if (body.empty()) { + return 0; + } + return static_cast(body.size() - mockStreamState.position % body.size()); + } size_t readBytes(uint8_t *buf, size_t len) { - memset(buf, 0, len); + const std::string &body = httpResponseBody(); + if (body.empty()) { + return 0; + } + for (size_t i = 0; i < len; ++i) { + buf[i] = body[(mockStreamState.position + i) % body.size()]; + } + mockStreamState.position += len; return len; } void stop() {} diff --git a/test/native/mocks/HTTPClient.h b/test/native/mocks/HTTPClient.h index 133f4139..b62c38ab 100644 --- a/test/native/mocks/HTTPClient.h +++ b/test/native/mocks/HTTPClient.h @@ -1,42 +1,87 @@ #pragma once -#include +#include #include + #include "Arduino.h" #define HTTPC_STRICT_FOLLOW_REDIRECTS 1 class WiFiClient; +// Test-controllable response state shared by all HTTPClient mock instances. +// Mirrors what the real ESP32 HTTPClient returns from a server response. +struct HttpCapture { + int httpCode = 200; + int contentSize = 0; + std::string responseBody; + int getStringCalls = 0; + + void clear() { + httpCode = 200; + contentSize = 0; + responseBody.clear(); + getStringCalls = 0; + } +}; + +extern HttpCapture httpCapture; + class HTTPClient { public: - HTTPClient() {} - ~HTTPClient() {} - - bool begin(WiFiClient &client, const String &url) { (void)client; (void)url; return true; } - bool begin(const String &url) { (void)url; return true; } - int GET() { return 200; } - int PUT() { return 200; } - int POST() { return 200; } - void addHeader(const String &name, const String &value) { (void)name; (void)value; } - void setAuthorization(const char *type, const char *credentials) { (void)type; (void)credentials; } - void setAuthorization(const char *auth) { (void)auth; } - void setUserAgent(const String &ua) { (void)ua; } - void setFollowRedirects(int) {} - void setReuse(bool) {} - void setTimeout(uint16_t) {} - bool connected() { return false; } - int getSize() { return 0; } - String getString() { return String("{}"); } - int writeToStream(void *) { return 0; } - void end() {} - int lastError() { return 0; } - uint8_t errorToString(int) { return 0; } - void setConnectTimeout(int) {} - void setHeaders(const char **, int) {} - int sendRequest(const char *, const uint8_t *, size_t) { return 200; } + HTTPClient() {} + ~HTTPClient() {} + + bool begin(WiFiClient &client, const String &url) { + (void)client; + (void)url; + return true; + } + bool begin(const String &url) { + (void)url; + return true; + } + + int GET() { return httpCapture.httpCode; } + int PUT() { return 200; } + int POST() { return 200; } + + void addHeader(const String &name, const String &value) { + (void)name; + (void)value; + } + void setAuthorization(const char *type, const char *credentials) { + (void)type; + (void)credentials; + } + void setAuthorization(const char *auth) { (void)auth; } + void setUserAgent(const String &ua) { (void)ua; } + void setFollowRedirects(int) {} + void setReuse(bool) {} + void setTimeout(uint16_t) {} + + bool connected() { return httpCapture.httpCode == 200 && httpCapture.contentSize > 0; } + + int getSize() { return httpCapture.contentSize; } + + String getString() { + httpCapture.getStringCalls++; + return String(httpCapture.responseBody.c_str()); + } + + int writeToStream(void *) { return 0; } + + void end() {} + + int lastError() { return 0; } + uint8_t errorToString(int) { return 0; } + void setConnectTimeout(int) {} + void setHeaders(const char **, int) {} + int sendRequest(const char *, const uint8_t *, size_t) { return 200; } + WiFiClient *getStreamPtr() { return &_stream; } WiFiClient &getStream() { return _stream; } + private: WiFiClient _stream; }; diff --git a/test/native/mocks/captures.cpp b/test/native/mocks/captures.cpp index d14ac18d..aeea0a42 100644 --- a/test/native/mocks/captures.cpp +++ b/test/native/mocks/captures.cpp @@ -4,9 +4,22 @@ #include "Arduino.h" #include "WebServer.h" #include "AsyncMqttClient.h" +#include "HTTPClient.h" WebServerCapture wsCapture; MqttClientCapture mqttCapture; +HttpCapture httpCapture; + +// Arduino mock clock offset (test hook). +uint32_t g_millisOffset = 0; + +// ESP mock heap state (default mirrors the previous hardcoded value). +uint32_t ESPClass::s_mockFreeHeap = 180000; +MockStreamState mockStreamState; + +const std::string &httpResponseBody() { + return httpCapture.responseBody; +} // Serial mock capture state (disabled by default) std::string SerialClass::s_capture; diff --git a/test/native/mocks/stubs.cpp b/test/native/mocks/stubs.cpp index c6da338b..c2c68e50 100644 --- a/test/native/mocks/stubs.cpp +++ b/test/native/mocks/stubs.cpp @@ -27,7 +27,8 @@ extern "C" size_t strlcat(char *dst, const char *src, size_t siz) { size_t dlen = strlen(dst); size_t slen = strlen(src); - if (dlen >= siz) return siz + slen; + if (dlen >= siz) + return siz + slen; size_t n = siz - dlen - 1; strncat(dst + dlen, src, n); dst[dlen + (n < slen ? n : slen)] = '\0'; @@ -110,8 +111,16 @@ time_t getTimeFor(int tzIndex, TimeChangeRule **tcr) { String getTimeInfoFor(int) { return String("UTC"); } +namespace { +TimeDegradation g_mockTimeDegradation = TimeDegradation::GREEN; +} + TimeDegradation getTimeDegradation() { - return TimeDegradation::GREEN; + return g_mockTimeDegradation; +} + +void setTimeDegradationStateForTest(TimeDegradation state) { + g_mockTimeDegradation = state; } void setTimezoneIndex(int) {} void timeClientSetup(const char *) {} diff --git a/test/native/relay_safety/stubs.cpp b/test/native/relay_safety/stubs.cpp index c4e673ef..4cfd4419 100644 --- a/test/native/relay_safety/stubs.cpp +++ b/test/native/relay_safety/stubs.cpp @@ -18,6 +18,9 @@ */ #include "Arduino.h" + +// Definition for the test-hook offset declared in mocks/Arduino.h. +uint32_t g_millisOffset = 0; #include "ConfigManager.hpp" #include "DegradationManager.hpp" #include "TimeClientHelper.hpp" diff --git a/test/native/tests/test_degradation_manager.cpp b/test/native/tests/test_degradation_manager.cpp new file mode 100644 index 00000000..8797680d --- /dev/null +++ b/test/native/tests/test_degradation_manager.cpp @@ -0,0 +1,268 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// +// SPDX-License-Identifier: MIT + +#include + +#include "../../../src/DegradationManager.hpp" +#include "../../../src/TimeClientHelper.hpp" + +// Test hook implemented in mocks/stubs.cpp. +void setTimeDegradationStateForTest(TimeDegradation state); +#include "NetworkManager.hpp" +#include "Nodes.hpp" +#include "SystemMonitor.hpp" +#include "TimeClientHelper.hpp" + +extern void test_begin(const char *suite, const char *name); +extern void test_pass(const char *file, int line); +extern void test_fail(const char *file, int line, const char *msg); +extern void test_suite_end(const char *name, int passed, int failed); + +#define ASSERT_TRUE(cond) \ + do { \ + if (!(cond)) { \ + test_fail(__FILE__, __LINE__, "Expected true: " #cond); \ + return 1; \ + } \ + test_pass(__FILE__, __LINE__); \ + } while (0) + +#define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) + +using namespace PoolController; + +// ── Reset helper: restore baseline state before every scenario ── +static void resetDegradationState() { + advanceMillisForTest(5000); // clear the evaluate() rate limit + DegradationManager::unforceSafeMode(); // clear a forced boot-loop state from earlier tests + NetworkManager::setWiFiConnected(true); + NetworkManager::setMqttConnected(false); + ESP.setFreeHeapForTest(180000); + setTimeDegradationStateForTest(TimeDegradation::GREEN); + DegradationManager::begin(); +} + +static int test_begin_resets_to_normal() { + test_begin("DegradationManager", "begin() resets level to NORMAL"); + resetDegradationState(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NORMAL); + ASSERT_TRUE(!DegradationManager::isSafe()); + return 0; +} + +static int test_begin_preserves_forced_critical() { + test_begin("DegradationManager", "begin() preserves a forced CRITICAL boot-loop safe mode"); + resetDegradationState(); + DegradationManager::forceSafeMode(); + DegradationManager::begin(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::CRITICAL); + ASSERT_TRUE(DegradationManager::isSafe()); + return 0; +} + +static int test_evaluate_normal_with_all_healthy() { + test_begin("DegradationManager", "evaluate() stays NORMAL with WiFi, time, sensors and heap healthy"); + resetDegradationState(); + DegradationManager::reportSensorStatus("pool-temp", true); + DegradationManager::reportSensorStatus("solar-temp", true); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NORMAL); + return 0; +} + +static int test_wifi_loss_is_single_failure() { + test_begin("DegradationManager", "WiFi loss alone degrades to NO_WIFI, not safe mode"); + resetDegradationState(); + NetworkManager::setWiFiConnected(false); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_WIFI); + ASSERT_TRUE(!DegradationManager::isSafe()); + return 0; +} + +static int test_time_loss_with_wifi_is_single_failure() { + test_begin("DegradationManager", "NTP loss with WiFi up degrades to NO_TIME"); + resetDegradationState(); + setTimeDegradationStateForTest(TimeDegradation::RED); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_TIME); + ASSERT_TRUE(!DegradationManager::isSafe()); + return 0; +} + +static int test_time_loss_without_wifi_is_consequence() { + test_begin("DegradationManager", "NTP loss without WiFi does not escalate WiFi outage to CRITICAL"); + resetDegradationState(); + NetworkManager::setWiFiConnected(false); + setTimeDegradationStateForTest(TimeDegradation::RED); + advanceMillisForTest(5000); + DegradationManager::evaluate(); + // Single failure: WiFi down; the NTP loss is a consequence of it and the + // time-branch wins the "most specific level" ordering. Crucially: not CRITICAL. + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_TIME); + ASSERT_TRUE(!DegradationManager::isSafe()); + return 0; +} + +static int test_sensor_fault_is_single_failure() { + test_begin("DegradationManager", "one invalid probe degrades to NO_SENSOR"); + resetDegradationState(); + DegradationManager::reportSensorStatus("pool-temp", true); + DegradationManager::reportSensorStatus("solar-temp", false); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_SENSOR); + return 0; +} + +static int test_silent_sensors_treated_as_ok() { + test_begin("DegradationManager", "sensors that never reported do not count as failure"); + resetDegradationState(); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NORMAL); + return 0; +} + +static int test_low_memory_is_critical() { + test_begin("DegradationManager", "critically low heap forces safe mode regardless of other signals"); + resetDegradationState(); + ESP.setFreeHeapForTest(1000); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::CRITICAL); + ASSERT_TRUE(DegradationManager::isSafe()); + return 0; +} + +static int test_two_failures_are_critical() { + test_begin("DegradationManager", "WiFi loss plus sensor fault escalates to CRITICAL"); + resetDegradationState(); + NetworkManager::setWiFiConnected(false); + DegradationManager::reportSensorStatus("pool-temp", false); + DegradationManager::reportSensorStatus("solar-temp", true); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::CRITICAL); + ASSERT_TRUE(DegradationManager::isSafe()); + return 0; +} + +static int test_critical_deenergizes_relays() { + test_begin("DegradationManager", "entering safe mode de-energizes both pumps"); + resetDegradationState(); + poolPumpNode.setSwitch(true); + solarPumpNode.setSwitch(true); + NetworkManager::setWiFiConnected(false); + DegradationManager::reportSensorStatus("pool-temp", false); + DegradationManager::reportSensorStatus("solar-temp", true); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::CRITICAL); + ASSERT_TRUE(!poolPumpNode.getSwitch()); + ASSERT_TRUE(!solarPumpNode.getSwitch()); + return 0; +} + +static int test_recovery_downgrades_level() { + test_begin("DegradationManager", "evaluate() recovers to NORMAL once the fault clears"); + resetDegradationState(); + NetworkManager::setWiFiConnected(false); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_WIFI); + NetworkManager::setWiFiConnected(true); + advanceMillisForTest(5000); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NORMAL); + return 0; +} + +static int test_forced_safe_mode_never_downgrades() { + test_begin("DegradationManager", "forced safe mode is not cleared by healthy signals"); + resetDegradationState(); + DegradationManager::forceSafeMode(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::CRITICAL); + DegradationManager::reportSensorStatus("pool-temp", true); + DegradationManager::reportSensorStatus("solar-temp", true); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::CRITICAL); + return 0; +} + +static int test_unforce_recovers() { + test_begin("DegradationManager", "unforceSafeMode() returns to NORMAL"); + resetDegradationState(); + DegradationManager::forceSafeMode(); + ASSERT_TRUE(DegradationManager::isSafe()); + DegradationManager::unforceSafeMode(); + ASSERT_TRUE(!DegradationManager::isSafe()); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NORMAL); + return 0; +} + +static int test_level_to_string_mapping() { + test_begin("DegradationManager", "levelToString() maps every level"); + ASSERT_EQ(std::strcmp(DegradationManager::levelToString(DegradationLevel::NORMAL), "normal"), 0); + ASSERT_EQ(std::strcmp(DegradationManager::levelToString(DegradationLevel::NO_WIFI), "no-wifi"), 0); + ASSERT_EQ(std::strcmp(DegradationManager::levelToString(DegradationLevel::NO_TIME), "no-time"), 0); + ASSERT_EQ(std::strcmp(DegradationManager::levelToString(DegradationLevel::NO_SENSOR), "no-sensor"), 0); + ASSERT_EQ(std::strcmp(DegradationManager::levelToString(DegradationLevel::CRITICAL), "critical"), 0); + return 0; +} + +static int test_sensor_status_tracks_each_probe() { + test_begin("DegradationManager", "reportSensorStatus() tracks pool and solar probes independently"); + resetDegradationState(); + DegradationManager::reportSensorStatus("pool-temp", false); + DegradationManager::reportSensorStatus("solar-temp", true); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_SENSOR); + // Pool probe recovers alone. + DegradationManager::reportSensorStatus("pool-temp", true); + advanceMillisForTest(5000); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NORMAL); + return 0; +} + +static int test_evaluate_is_rate_limited() { + test_begin("DegradationManager", "evaluate() is rate-limited to the evaluation interval"); + resetDegradationState(); + NetworkManager::setWiFiConnected(false); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_WIFI); + // Health returns, but the rate limit suppresses re-evaluation. + NetworkManager::setWiFiConnected(true); + DegradationManager::evaluate(); + ASSERT_EQ(DegradationManager::getLevel(), DegradationLevel::NO_WIFI); + return 0; +} + +int run_degradation_manager_tests() { + int passed = 0; + int failed = 0; + int (*tests[])() = { + test_begin_resets_to_normal, + test_begin_preserves_forced_critical, + test_evaluate_normal_with_all_healthy, + test_wifi_loss_is_single_failure, + test_time_loss_with_wifi_is_single_failure, + test_time_loss_without_wifi_is_consequence, + test_sensor_fault_is_single_failure, + test_silent_sensors_treated_as_ok, + test_low_memory_is_critical, + test_two_failures_are_critical, + test_critical_deenergizes_relays, + test_recovery_downgrades_level, + test_forced_safe_mode_never_downgrades, + test_unforce_recovers, + test_level_to_string_mapping, + test_sensor_status_tracks_each_probe, + test_evaluate_is_rate_limited, + }; + for (auto test : tests) { + if (test() == 0) { + ++passed; + } else { + ++failed; + } + } + test_suite_end("DegradationManager", passed, failed); + return failed; +} diff --git a/test/native/tests/test_main.cpp b/test/native/tests/test_main.cpp index 55b3c5b1..09184f86 100644 --- a/test/native/tests/test_main.cpp +++ b/test/native/tests/test_main.cpp @@ -120,6 +120,8 @@ extern int run_controller_command_queue_tests(); extern int run_controller_command_handler_tests(); extern int run_mqtt_runtime_command_adapter_tests(); extern int run_controller_snapshot_publisher_tests(); +extern int run_degradation_manager_tests(); +extern int run_ota_updater_tests(); int main() { printf("\n══════════════════════════════════════════════════\n"); @@ -148,6 +150,8 @@ int main() { total += run_controller_command_handler_tests(); total += run_mqtt_runtime_command_adapter_tests(); total += run_controller_snapshot_publisher_tests(); + total += run_degradation_manager_tests(); + total += run_ota_updater_tests(); printf("\n══════════════════════════════════════════════════\n"); printf(" Results: %d suites passed, %d suites failed\n", g_testsPassed, g_testsFailed); diff --git a/test/native/tests/test_ota_updater.cpp b/test/native/tests/test_ota_updater.cpp new file mode 100644 index 00000000..c03d4644 --- /dev/null +++ b/test/native/tests/test_ota_updater.cpp @@ -0,0 +1,237 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// +// SPDX-License-Identifier: MIT + +#include +#include + +#include "HTTPClient.h" +#include "NetworkManager.hpp" +#include "../../../src/OtaUpdater.hpp" + +extern void test_begin(const char *suite, const char *name); +extern void test_pass(const char *file, int line); +extern void test_fail(const char *file, int line, const char *msg); +extern void test_suite_end(const char *name, int passed, int failed); + +#define ASSERT_TRUE(cond) \ + do { \ + if (!(cond)) { \ + test_fail(__FILE__, __LINE__, "Expected true: " #cond); \ + return 1; \ + } \ + test_pass(__FILE__, __LINE__); \ + } while (0) + +#define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) + +using namespace PoolController; + +// Minimal GitHub release payload with the board-specific asset. +static void setReleaseResponse(const char *tag, const char *assetUrl, const char *assetName) { + httpCapture.clear(); + mockStreamState.reset(); + httpCapture.httpCode = 200; + httpCapture.responseBody = std::string("{\"tag_name\":\"") + tag + + "\",\"html_url\":\"https://github.com/r/releases\"," + "\"assets\":[{\"name\":\"" + + assetName + "\",\"browser_download_url\":\"" + assetUrl + "\"}]}"; + httpCapture.contentSize = static_cast(httpCapture.responseBody.size()); +} + +static void setUpdateAvailableState() { + // Seed a newer release so startUpdate() has a download URL. + setReleaseResponse("v999.0.0", "https://example.com/firmware.bin", "firmware-norvi_ae01_r.bin"); + OtaUpdater::begin(); + httpCapture.httpCode = 200; + OtaUpdater::checkForUpdate(); +} + +int run_ota_updater_tests() { + int passed = 0; + int failed = 0; + int rc = 0; + + test_begin("OtaUpdater", "begin() reports the current version and idle status"); + OtaUpdater::begin(); + ASSERT_EQ(OtaUpdater::getCurrentVersion(), String(FW_VERSION)); + ASSERT_EQ(OtaUpdater::getStatusMessage(), String("Idle")); + test_suite_end("OtaUpdater::begin", 1, 0); + + test_begin("OtaUpdater", "parseVersion accepts v-prefixed and plain semver"); + { + OtaUpdater::Version v{}; + ASSERT_TRUE(OtaUpdater::parseVersion(String("v3.2.1"), v)); + ASSERT_EQ(v.major, 3); + ASSERT_EQ(v.minor, 2); + ASSERT_EQ(v.patch, 1); + ASSERT_TRUE(OtaUpdater::parseVersion(String("1.0.0"), v)); + ASSERT_EQ(v.major, 1); + } + test_suite_end("OtaUpdater::parse-version", 1, 0); + + test_begin("OtaUpdater", "parseVersion rejects incomplete or malformed input"); + { + OtaUpdater::Version v{}; + ASSERT_TRUE(!OtaUpdater::parseVersion(String("3.2"), v)); + ASSERT_TRUE(!OtaUpdater::parseVersion(String("abc"), v)); + ASSERT_TRUE(!OtaUpdater::parseVersion(String(""), v)); + } + test_suite_end("OtaUpdater::parse-invalid", 1, 0); + + test_begin("OtaUpdater", "isNewerVersion compares major, minor and patch correctly"); + { + ASSERT_TRUE(OtaUpdater::isNewerVersion(String("v1.0.0"), String("v1.0.1"))); + ASSERT_TRUE(OtaUpdater::isNewerVersion(String("v1.0.0"), String("v1.1.0"))); + ASSERT_TRUE(OtaUpdater::isNewerVersion(String("v1.9.9"), String("v2.0.0"))); + ASSERT_TRUE(!OtaUpdater::isNewerVersion(String("v2.0.0"), String("v1.9.9"))); + ASSERT_TRUE(!OtaUpdater::isNewerVersion(String("v1.2.3"), String("v1.2.3"))); + } + test_suite_end("OtaUpdater::is-newer", 1, 0); + + test_begin("OtaUpdater", "isNewerVersion rejects unparseable input"); + { + ASSERT_TRUE(!OtaUpdater::isNewerVersion(String("v1.0.0"), String("oops"))); + ASSERT_TRUE(!OtaUpdater::isNewerVersion(String("oops"), String("v1.0.0"))); + } + test_suite_end("OtaUpdater::is-newer-invalid", 1, 0); + + test_begin("OtaUpdater", "checkForUpdate detects a newer GitHub release"); + { + setReleaseResponse("v999.0.0", "https://example.com/firmware.bin", "firmware-norvi_ae01_r.bin"); + OtaUpdater::begin(); + ASSERT_TRUE(OtaUpdater::checkForUpdate()); + ASSERT_TRUE(OtaUpdater::isUpdateAvailable()); + ASSERT_EQ(OtaUpdater::getLatestVersion(), String("999.0.0")); + ASSERT_EQ(OtaUpdater::getLatestVersionTag(), String("v999.0.0")); + ASSERT_EQ(OtaUpdater::getReleaseUrl(), String("https://github.com/r/releases")); + } + test_suite_end("OtaUpdater::check-newer", 1, 0); + + test_begin("OtaUpdater", "checkForUpdate reports up-to-date on same version"); + { + setReleaseResponse(FW_VERSION, "https://example.com/firmware.bin", "firmware-norvi_ae01_r.bin"); + OtaUpdater::begin(); + ASSERT_TRUE(!OtaUpdater::checkForUpdate()); + ASSERT_TRUE(!OtaUpdater::isUpdateAvailable()); + } + test_suite_end("OtaUpdater::check-current", 1, 0); + + test_begin("OtaUpdater", "checkForUpdate fails on HTTP error"); + { + httpCapture.clear(); + mockStreamState.reset(); + httpCapture.httpCode = 500; + OtaUpdater::begin(); + ASSERT_TRUE(!OtaUpdater::checkForUpdate()); + ASSERT_TRUE(!OtaUpdater::isUpdateAvailable()); + } + test_suite_end("OtaUpdater::check-http-error", 1, 0); + + test_begin("OtaUpdater", "checkForUpdate fails on missing tag_name"); + { + httpCapture.clear(); + mockStreamState.reset(); + httpCapture.httpCode = 200; + httpCapture.responseBody = "{\"html_url\":\"https://github.com/r/releases\",\"assets\":[]}"; + httpCapture.contentSize = static_cast(httpCapture.responseBody.size()); + OtaUpdater::begin(); + ASSERT_TRUE(!OtaUpdater::checkForUpdate()); + } + test_suite_end("OtaUpdater::check-no-tag", 1, 0); + + test_begin("OtaUpdater", "checkForUpdate fails when no .bin asset exists"); + { + setReleaseResponse("v999.0.0", "https://example.com/readme.txt", "readme.txt"); + OtaUpdater::begin(); + ASSERT_TRUE(!OtaUpdater::checkForUpdate()); + } + test_suite_end("OtaUpdater::check-no-asset", 1, 0); + + test_begin("OtaUpdater", "checkForUpdate falls back to a generic .bin asset"); + { + setReleaseResponse("v999.0.0", "https://example.com/firmware.bin", "firmware.bin"); + OtaUpdater::begin(); + ASSERT_TRUE(OtaUpdater::checkForUpdate()); + } + test_suite_end("OtaUpdater::check-generic-asset", 1, 0); + + test_begin("OtaUpdater", "startUpdate is refused without a prepared release"); + { + // Bring the OTA state back to "up to date" so no download URL is known. + setReleaseResponse(FW_VERSION, "https://example.com/firmware.bin", "firmware-norvi_ae01_r.bin"); + OtaUpdater::begin(); + OtaUpdater::checkForUpdate(); + ASSERT_TRUE(!OtaUpdater::isUpdateAvailable()); + ASSERT_TRUE(!OtaUpdater::startUpdate()); + ASSERT_EQ(OtaUpdater::getStatusMessage(), String("No update available")); + } + test_suite_end("OtaUpdater::start-no-update", 1, 0); + + test_begin("OtaUpdater", "startUpdate is refused without WiFi"); + { + setUpdateAvailableState(); + NetworkManager::setWiFiConnected(false); + ASSERT_TRUE(!OtaUpdater::startUpdate()); + ASSERT_EQ(OtaUpdater::getStatusMessage(), String("No WiFi connection")); + NetworkManager::setWiFiConnected(true); + } + test_suite_end("OtaUpdater::start-no-wifi", 1, 0); + + test_begin("OtaUpdater", "hasSufficientSpace validates firmware size against flash"); + { + ASSERT_TRUE(OtaUpdater::hasSufficientSpace(600 * 1024)); + ASSERT_TRUE(!OtaUpdater::hasSufficientSpace(64UL * 1024 * 1024)); + } + test_suite_end("OtaUpdater::space", 1, 0); + + test_begin("OtaUpdater", "downloadAndApply rejects implausible content sizes"); + { + setUpdateAvailableState(); + NetworkManager::setWiFiConnected(true); + // HTTP 404 during download. + httpCapture.httpCode = 404; + ASSERT_TRUE(!OtaUpdater::startUpdate()); + // Invalid (non-positive) content size. + httpCapture.httpCode = 200; + httpCapture.contentSize = 0; + ASSERT_TRUE(!OtaUpdater::startUpdate()); + // Far too small to be a firmware image. + httpCapture.contentSize = 512; + ASSERT_TRUE(!OtaUpdater::startUpdate()); + // Far too large to be a firmware image. + httpCapture.contentSize = 3 * 1024 * 1024; + ASSERT_TRUE(!OtaUpdater::startUpdate()); + ASSERT_TRUE(!OtaUpdater::isUpdateInProgress()); + } + test_suite_end("OtaUpdater::download-guards", 1, 0); + + test_begin("OtaUpdater", "startUpdate streams a plausible firmware into the flash writer"); + { + setUpdateAvailableState(); + NetworkManager::setWiFiConnected(true); + // Plausible firmware: 128 KB body, real content size. + httpCapture.httpCode = 200; + httpCapture.contentSize = 128 * 1024; + httpCapture.responseBody.assign(4096, 'F'); + mockStreamState.reset(); + ASSERT_TRUE(OtaUpdater::startUpdate()); + ASSERT_EQ(OtaUpdater::getProgress(), 100); + } + test_suite_end("OtaUpdater::download-happy-path", 1, 0); + + test_begin("OtaUpdater", "progress and status expose the OTA lifecycle"); + { + OtaUpdater::begin(); + ASSERT_TRUE(OtaUpdater::getCurrentVersion().length() > 0); + // After a successful flash the device would reboot; in the mock the + // in-progress flag stays set until that restart. + ASSERT_TRUE(OtaUpdater::getProgress() >= 0); + } + test_suite_end("OtaUpdater::lifecycle", 1, 0); + + (void)rc; + (void)passed; + (void)failed; + return 0; +} From 265bb6bfefbbd985c813b6ecb38d38b8218c890b Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 15:36:54 +0000 Subject: [PATCH 11/57] style: avoid ASSERT_TRUE comparison form flagged by cpplint Co-authored-by: stritti --- test/native/tests/test_ota_updater.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/native/tests/test_ota_updater.cpp b/test/native/tests/test_ota_updater.cpp index c03d4644..8d9c07c7 100644 --- a/test/native/tests/test_ota_updater.cpp +++ b/test/native/tests/test_ota_updater.cpp @@ -223,10 +223,12 @@ int run_ota_updater_tests() { test_begin("OtaUpdater", "progress and status expose the OTA lifecycle"); { OtaUpdater::begin(); - ASSERT_TRUE(OtaUpdater::getCurrentVersion().length() > 0); + const String current = OtaUpdater::getCurrentVersion(); + ASSERT_TRUE(current.length() > 0); // After a successful flash the device would reboot; in the mock the // in-progress flag stays set until that restart. - ASSERT_TRUE(OtaUpdater::getProgress() >= 0); + const int progress = OtaUpdater::getProgress(); + ASSERT_TRUE(progress == 0 || progress == 100); } test_suite_end("OtaUpdater::lifecycle", 1, 0); From f5168f90d4cc58ce9bcab0d4ef7548854a47f076 Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sun, 11 Oct 2026 15:43:25 +0000 Subject: [PATCH 12/57] style: use String::isEmpty() in OTA lifecycle test Co-authored-by: stritti --- test/native/tests/test_ota_updater.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/native/tests/test_ota_updater.cpp b/test/native/tests/test_ota_updater.cpp index 8d9c07c7..c10821e0 100644 --- a/test/native/tests/test_ota_updater.cpp +++ b/test/native/tests/test_ota_updater.cpp @@ -224,7 +224,7 @@ int run_ota_updater_tests() { { OtaUpdater::begin(); const String current = OtaUpdater::getCurrentVersion(); - ASSERT_TRUE(current.length() > 0); + ASSERT_TRUE(!current.isEmpty()); // After a successful flash the device would reboot; in the mock the // in-progress flag stays set until that restart. const int progress = OtaUpdater::getProgress(); From 3ac47322862f6de20e9016b3f53b304f87612aa6 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:44:21 +0200 Subject: [PATCH 13/57] test: model detected Dallas inventory --- test/native/mocks/DallasTemperatureNode.hpp | 22 ++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/test/native/mocks/DallasTemperatureNode.hpp b/test/native/mocks/DallasTemperatureNode.hpp index 980d69bb..e493f973 100644 --- a/test/native/mocks/DallasTemperatureNode.hpp +++ b/test/native/mocks/DallasTemperatureNode.hpp @@ -1,6 +1,7 @@ #pragma once #include "Arduino.h" #include "DallasTemperature.h" +#include #include class DallasTemperatureNode { @@ -17,7 +18,7 @@ class DallasTemperatureNode { void setTemperature(float t) { _temperature = t; } bool isSensorFound() const { return _sensorFound; } void setSensorFound(bool f) { _sensorFound = f; } - uint8_t getDeviceCount() const { return 0; } + uint8_t getDeviceCount() const { return _detectedCount; } const char *getId() const { return _id; } uint8_t getPin() const { return _pin; } unsigned long getMeasurementInterval() const { return _measurementInterval; } @@ -42,8 +43,20 @@ class DallasTemperatureNode { snprintf(buf, size, "%02X%02X%02X%02X%02X%02X%02X%02X", _filterAddress[0], _filterAddress[1], _filterAddress[2], _filterAddress[3], _filterAddress[4], _filterAddress[5], _filterAddress[6], _filterAddress[7]); } - bool getDetectedDeviceAddress(uint8_t, DeviceAddress) const { return false; } - float getDetectedDeviceTemperature(uint8_t) const { return 0.0f; } + bool getDetectedDeviceAddress(uint8_t index, DeviceAddress address) const { + if (index >= _detectedCount) return false; + std::memcpy(address, _detectedAddresses[index].data(), 8); + return true; + } + float getDetectedDeviceTemperature(uint8_t index) const { + return index < _detectedCount ? _detectedTemperatures[index] : NAN; + } + void setDetectedDevice(uint8_t index, const DeviceAddress address, float temperature) { + if (index >= _detectedAddresses.size()) return; + std::memcpy(_detectedAddresses[index].data(), address, 8); + _detectedTemperatures[index] = temperature; + if (_detectedCount <= index) _detectedCount = index + 1; + } private: const char *_id; @@ -55,4 +68,7 @@ class DallasTemperatureNode { float _temperature = 25.0f; bool _hasFilter = false; DeviceAddress _filterAddress{}; + std::array, 24> _detectedAddresses{}; + std::array _detectedTemperatures{}; + uint8_t _detectedCount = 0; }; From cd119fa76cddf602b4d359a60909f4b6febe34ae Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:44:26 +0200 Subject: [PATCH 14/57] test: make local IP configurable --- test/native/mocks/NetworkManager.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/native/mocks/NetworkManager.hpp b/test/native/mocks/NetworkManager.hpp index 102c812b..6769c01b 100644 --- a/test/native/mocks/NetworkManager.hpp +++ b/test/native/mocks/NetworkManager.hpp @@ -17,7 +17,8 @@ class NetworkManager { static bool isApMode() { return _apMode; } static int getWiFiRSSI() { return _wifiRssi; } - static String getLocalIP() { return String("192.168.1.100"); } + static IPAddress getLocalIPAddress() { return _localIp; } + static String getLocalIP() { return getLocalIPAddress().toString(); } static bool publish(const char *topic, const char *payload, bool retained = false) { if (!_publishOk) return false; @@ -57,6 +58,7 @@ class NetworkManager { static void setMqttConnected(bool v) { _mqttConnected = v; } static void setApMode(bool v) { _apMode = v; } static void setWiFiRSSI(int rssi) { _wifiRssi = rssi; } + static void setLocalIP(const IPAddress &ip) { _localIp = ip; } // Simulate AsyncMqttClient refusing to enqueue (publish returns false). static void setPublishOk(bool ok) { _publishOk = ok; } // One-shot hook fired when the next publish() targets `topic` — lets a test @@ -79,6 +81,7 @@ class NetworkManager { static bool _mqttConnected; static bool _apMode; static int _wifiRssi; + static IPAddress _localIp; static bool _publishOk; static std::function _publishHook; static std::string _publishHookTopic; From 9765f58d678f005c88a0100c0ce70f24b1fc6b2c Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:44:29 +0200 Subject: [PATCH 15/57] test: seed local IP state --- test/native/mocks/NetworkManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/native/mocks/NetworkManager.cpp b/test/native/mocks/NetworkManager.cpp index ec864a76..8bfad486 100644 --- a/test/native/mocks/NetworkManager.cpp +++ b/test/native/mocks/NetworkManager.cpp @@ -7,6 +7,7 @@ bool NetworkManager::_wifiConnected = true; bool NetworkManager::_mqttConnected = true; bool NetworkManager::_apMode = false; int NetworkManager::_wifiRssi = -65; +IPAddress NetworkManager::_localIp{192, 168, 1, 100}; bool NetworkManager::_publishOk = true; std::function NetworkManager::_publishHook; std::string NetworkManager::_publishHookTopic; From 3711c86f33d5fe15f6907df7667866318fbe8d49 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:44:32 +0200 Subject: [PATCH 16/57] test: observe verbose rule lookup --- test/native/mocks/OperationModeNode.hpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/native/mocks/OperationModeNode.hpp b/test/native/mocks/OperationModeNode.hpp index 804ee5db..29603a07 100644 --- a/test/native/mocks/OperationModeNode.hpp +++ b/test/native/mocks/OperationModeNode.hpp @@ -38,7 +38,9 @@ class OperationModeNode { } const char *getLastModeSource() const { return _lastModeSource.c_str(); } - Rule *getRule() { + Rule *getRule() { return getRule(true); } + Rule *getRule(bool emitDebugLogs) { + if (emitDebugLogs) ++_verboseRuleLookups; if (_mode == PoolController::OperationMode::AUTO) return &_autoRule; if (_mode == PoolController::OperationMode::MANUAL) @@ -50,6 +52,8 @@ class OperationModeNode { return nullptr; } + unsigned int getVerboseRuleLookupCount() const { return _verboseRuleLookups; } + float getPoolMaxTemperature() const { return _poolMaxTemperature; } float getSolarMinTemperature() const { return _solarMinTemperature; } float getTemperatureHysteresis() const { return _temperatureHysteresis; } @@ -76,6 +80,7 @@ class OperationModeNode { float _temperatureHysteresis = 1.0f; TimerSetting _timer; unsigned long _measurementInterval = 300; + unsigned int _verboseRuleLookups = 0; RuleAuto _autoRule; RuleManu _manuRule; RuleBoost _boostRule; From 1aa3c6bf222adab59c7a9a115eb231a81aafbf32 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:44:34 +0200 Subject: [PATCH 17/57] test: cover review regressions in command handler --- .../tests/test_controller_command_handler.cpp | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/test/native/tests/test_controller_command_handler.cpp b/test/native/tests/test_controller_command_handler.cpp index bd830136..1c5dde8f 100644 --- a/test/native/tests/test_controller_command_handler.cpp +++ b/test/native/tests/test_controller_command_handler.cpp @@ -74,6 +74,57 @@ static int test_mode_and_setpoint_are_applied_through_one_handler() { return 0; } +static int test_persistence_failure_rolls_back_setpoints() { + test_begin("ControllerCommandHandler", "failed persistence rolls back setpoints and hysteresis"); + HandlerFixture fixture; + auto &settings = PoolController::ConfigManager::getSettings(); + + fixture.operation.setPoolMaxTemperature(28.0F); + fixture.operation.setSolarMinTemperature(35.0F); + fixture.operation.setTemperatureHysteresis(1.0F); + settings.tempMaxPool = 28.0F; + settings.tempMinSolar = 35.0F; + settings.tempHysteresis = 1.0F; + PoolController::ConfigManager::_saveFails = true; + + ControllerCommand command{}; + command.type = ControllerCommandType::SET_POOL_MAX_TEMPERATURE; + command.value = 30.0F; + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_EQ(fixture.operation.getPoolMaxTemperature(), 28.0F); + ASSERT_EQ(settings.tempMaxPool, 28.0F); + + command.type = ControllerCommandType::SET_SOLAR_MIN_TEMPERATURE; + command.value = 45.0F; + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_EQ(fixture.operation.getSolarMinTemperature(), 35.0F); + ASSERT_EQ(settings.tempMinSolar, 35.0F); + + command.type = ControllerCommandType::SET_TEMPERATURE_HYSTERESIS; + command.value = 2.0F; + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_EQ(fixture.operation.getTemperatureHysteresis(), 1.0F); + ASSERT_EQ(settings.tempHysteresis, 1.0F); + + PoolController::ConfigManager::_saveFails = false; + return 0; +} + +static int test_green_time_loss_rejects_zero() { + test_begin("ControllerCommandHandler", "green time-loss threshold rejects zero hours"); + HandlerFixture fixture; + auto &settings = PoolController::ConfigManager::getSettings(); + settings.timeLossGreenHours = 2; + + ControllerCommand command{}; + command.type = ControllerCommandType::SET_TIME_LOSS_GREEN_HOURS; + command.integerValue = 0; + + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_EQ(settings.timeLossGreenHours, 2); + return 0; +} + static int test_timer_updates_are_independent() { test_begin("ControllerCommandHandler", "timer start and end updates preserve the opposite half"); HandlerFixture fixture; @@ -182,7 +233,8 @@ static int test_factory_reset_owns_runtime_reset_mutation() { int run_controller_command_handler_tests() { int passed = 0; int failed = 0; - int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, test_timer_updates_are_independent, + int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, + test_persistence_failure_rolls_back_setpoints, test_green_time_loss_rejects_zero, test_timer_updates_are_independent, test_manual_pump_guard_is_application_policy, test_sensor_mapping_is_persisted_and_applied, test_loop_interval_updates_all_runtime_nodes, test_factory_reset_owns_runtime_reset_mutation}; for (auto test : tests) { From 5f76944cb5425b6c59a67a191f8850d323c01068 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:44:37 +0200 Subject: [PATCH 18/57] test: cover snapshot projection review regressions --- .../test_controller_snapshot_publisher.cpp | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/native/tests/test_controller_snapshot_publisher.cpp b/test/native/tests/test_controller_snapshot_publisher.cpp index e442c9e8..ca2937cd 100644 --- a/test/native/tests/test_controller_snapshot_publisher.cpp +++ b/test/native/tests/test_controller_snapshot_publisher.cpp @@ -67,6 +67,7 @@ static int test_complete_runtime_state_is_projected() { settings.btn3Min = 3871; settings.btn3Max = 4094; settings.btnNoPress = 4096; + PoolController::ConfigManager::getNtp().server = "time.example.test"; fixture.operation.setMode(OperationMode::BOOST); fixture.operation.setPoolMaxTemperature(29.5F); @@ -98,6 +99,7 @@ static int test_complete_runtime_state_is_projected() { PoolController::NetworkManager::setMqttConnected(true); PoolController::NetworkManager::setApMode(false); PoolController::NetworkManager::setWiFiRSSI(-61); + PoolController::NetworkManager::setLocalIP(IPAddress(192, 168, 50, 23)); fixture.publisher.publish(); @@ -127,6 +129,11 @@ static int test_complete_runtime_state_is_projected() { ASSERT_TRUE(snapshot.network.mqttConnected); ASSERT_TRUE(!snapshot.network.apMode); ASSERT_EQ(snapshot.network.wifiRssi, -61); + ASSERT_TRUE(snapshot.network.localIp.valid); + ASSERT_EQ(snapshot.network.localIp.octets[0], 192); + ASSERT_EQ(snapshot.network.localIp.octets[1], 168); + ASSERT_EQ(snapshot.network.localIp.octets[2], 50); + ASSERT_EQ(snapshot.network.localIp.octets[3], 23); ASSERT_TRUE(!snapshot.health.safeMode); ASSERT_TRUE(snapshot.health.timeValid); ASSERT_EQ(snapshot.health.timeDegradation, TimeDegradationState::GREEN); @@ -145,6 +152,8 @@ static int test_complete_runtime_state_is_projected() { ASSERT_EQ(snapshot.settings.timezoneIndex, 3); ASSERT_EQ(snapshot.settings.timeLossGreenHours, 2); ASSERT_EQ(snapshot.settings.timeLossRedHours, 18); + ASSERT_TRUE(snapshot.settings.ntpServer.valid()); + ASSERT_EQ(std::strcmp(snapshot.settings.ntpServer.text.data(), "time.example.test"), 0); ASSERT_EQ(snapshot.settings.button1Min, 3010); ASSERT_EQ(snapshot.settings.button3Max, 4094); ASSERT_EQ(snapshot.settings.buttonNoPress, 4096); @@ -173,6 +182,37 @@ static int test_mapping_found_requires_selected_configured_address() { return 0; } +static int test_detected_inventory_is_published_and_deduplicated() { + test_begin("ControllerSnapshotPublisher", "detected Dallas inventory is deduplicated across buses"); + SnapshotFixture fixture; + + const std::uint8_t sharedAddress[8] = {0x28, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7}; + const std::uint8_t poolOnlyAddress[8] = {0x28, 0xB1, 0xB2, 0xB3, 0xB4, 0xB5, 0xB6, 0xB7}; + fixture.solarTemperature.setDetectedDevice(0, sharedAddress, 48.5F); + fixture.poolTemperature.setDetectedDevice(0, sharedAddress, 48.0F); + fixture.poolTemperature.setDetectedDevice(1, poolOnlyAddress, 26.0F); + + fixture.publisher.publish(); + SystemSnapshot snapshot{}; + ASSERT_TRUE(fixture.store.read(snapshot)); + ASSERT_EQ(snapshot.sensors.detectedCount, 2); + ASSERT_EQ(std::memcmp(snapshot.sensors.detected[0].address.data(), sharedAddress, 8), 0); + ASSERT_EQ(snapshot.sensors.detected[0].temperature.value, 48.5F); + ASSERT_TRUE(snapshot.sensors.detected[0].temperature.valid); + ASSERT_EQ(std::memcmp(snapshot.sensors.detected[1].address.data(), poolOnlyAddress, 8), 0); + ASSERT_EQ(snapshot.sensors.detected[1].temperature.value, 26.0F); + ASSERT_TRUE(snapshot.sensors.detected[1].temperature.valid); + return 0; +} + +static int test_snapshot_rule_projection_avoids_verbose_lookup() { + test_begin("ControllerSnapshotPublisher", "snapshot rule projection does not use verbose rule lookup"); + SnapshotFixture fixture; + fixture.publisher.publish(); + ASSERT_EQ(fixture.operation.getVerboseRuleLookupCount(), 0U); + return 0; +} + static int test_generation_advances_per_complete_publication() { test_begin("ControllerSnapshotPublisher", "read-model generation advances atomically per publish"); SnapshotFixture fixture; @@ -193,6 +233,7 @@ int run_controller_snapshot_publisher_tests() { int passed = 0; int failed = 0; int (*tests[])() = {test_complete_runtime_state_is_projected, test_mapping_found_requires_selected_configured_address, + test_detected_inventory_is_published_and_deduplicated, test_snapshot_rule_projection_avoids_verbose_lookup, test_generation_advances_per_complete_publication}; for (auto test : tests) { if (test() == 0) { From 9ca33a4691d79c84325895c75e6e67eac41294fb Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:45:46 +0200 Subject: [PATCH 19/57] test: model partial sensor mappings --- test/native/mocks/ConfigManager.hpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/native/mocks/ConfigManager.hpp b/test/native/mocks/ConfigManager.hpp index 213fc1c6..ea2f7e7c 100644 --- a/test/native/mocks/ConfigManager.hpp +++ b/test/native/mocks/ConfigManager.hpp @@ -63,10 +63,18 @@ class ConfigManager { // ── Sensor Address Mapping ── static void saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]); static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); - // Tests never touch NVS: the cached variant behaves like loadSensorMapping(). + // Mirror the production cache's current all-or-nothing behavior so the + // partial-mapping regression test fails until the production fix lands. static bool loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - return loadSensorMapping(solarAddr, poolAddr); + if (!loadSensorMapping(solarAddr, poolAddr)) { + std::memset(solarAddr, 0, 8); + std::memset(poolAddr, 0, 8); + return false; + } + return true; } + static void seedSensorMappingForTest( + const uint8_t solarAddr[8], bool solarPresent, const uint8_t poolAddr[8], bool poolPresent); static Settings &getSettings() { return _settings; } static WiFiConfig &getWiFi() { return _wifi; } @@ -97,7 +105,8 @@ class ConfigManager { static uint8_t _sensorSolarAddr[8]; static uint8_t _sensorPoolAddr[8]; - static bool _sensorMappingValid; + static bool _sensorSolarValid; + static bool _sensorPoolValid; // Simple hash function for mock (not cryptographic, just for testing) static String hashPassword(const String &password) { From cc8bf3bef31e49619f30b7e9d6e46d093463717f Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:45:49 +0200 Subject: [PATCH 20/57] test: seed partial sensor mapping state --- test/native/mocks/ConfigManager.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/test/native/mocks/ConfigManager.cpp b/test/native/mocks/ConfigManager.cpp index 56e07f74..edf0c61f 100644 --- a/test/native/mocks/ConfigManager.cpp +++ b/test/native/mocks/ConfigManager.cpp @@ -15,19 +15,30 @@ bool ConfigManager::_saveFails = false; // ── Sensor Address Mapping ── uint8_t ConfigManager::_sensorSolarAddr[8] = {0}; uint8_t ConfigManager::_sensorPoolAddr[8] = {0}; -bool ConfigManager::_sensorMappingValid = false; +bool ConfigManager::_sensorSolarValid = false; +bool ConfigManager::_sensorPoolValid = false; void ConfigManager::saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]) { memcpy(_sensorSolarAddr, solarAddr, 8); memcpy(_sensorPoolAddr, poolAddr, 8); - _sensorMappingValid = true; + _sensorSolarValid = true; + _sensorPoolValid = true; } bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - if (!_sensorMappingValid) return false; - memcpy(solarAddr, _sensorSolarAddr, 8); - memcpy(poolAddr, _sensorPoolAddr, 8); - return true; + memset(solarAddr, 0, 8); + memset(poolAddr, 0, 8); + if (_sensorSolarValid) memcpy(solarAddr, _sensorSolarAddr, 8); + if (_sensorPoolValid) memcpy(poolAddr, _sensorPoolAddr, 8); + return _sensorSolarValid && _sensorPoolValid; +} + +void ConfigManager::seedSensorMappingForTest( + const uint8_t solarAddr[8], bool solarPresent, const uint8_t poolAddr[8], bool poolPresent) { + memcpy(_sensorSolarAddr, solarAddr, 8); + memcpy(_sensorPoolAddr, poolAddr, 8); + _sensorSolarValid = solarPresent; + _sensorPoolValid = poolPresent; } } // namespace PoolController From 1bc70195d3862ede4ba8427d90fe8e3a9edf9fb8 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:45:54 +0200 Subject: [PATCH 21/57] test: preserve independently stored sensor mapping --- .../tests/test_controller_command_handler.cpp | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/native/tests/test_controller_command_handler.cpp b/test/native/tests/test_controller_command_handler.cpp index 1c5dde8f..79a18871 100644 --- a/test/native/tests/test_controller_command_handler.cpp +++ b/test/native/tests/test_controller_command_handler.cpp @@ -200,6 +200,29 @@ static int test_sensor_mapping_is_persisted_and_applied() { return 0; } +static int test_setting_missing_sensor_role_preserves_existing_mapping() { + test_begin("ControllerCommandHandler", "setting missing sensor role preserves independently stored mapping"); + HandlerFixture fixture; + + const std::uint8_t empty[8] = {0}; + const std::uint8_t existingPool[8] = {0x28, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7}; + const std::uint8_t newSolar[8] = {0x28, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7}; + PoolController::ConfigManager::seedSensorMappingForTest(empty, false, existingPool, true); + + ControllerCommand command{}; + command.type = ControllerCommandType::SET_SENSOR_MAPPING; + command.sensorRole = SensorRole::SOLAR; + std::memcpy(command.sensorAddress.data(), newSolar, 8); + ASSERT_TRUE(fixture.handler.handle(command)); + + std::uint8_t savedSolar[8] = {0}; + std::uint8_t savedPool[8] = {0}; + ASSERT_TRUE(PoolController::ConfigManager::loadSensorMapping(savedSolar, savedPool)); + ASSERT_EQ(std::memcmp(savedSolar, newSolar, 8), 0); + ASSERT_EQ(std::memcmp(savedPool, existingPool, 8), 0); + return 0; +} + static int test_loop_interval_updates_all_runtime_nodes() { test_begin("ControllerCommandHandler", "loop interval is propagated to every runtime node"); HandlerFixture fixture; @@ -236,7 +259,8 @@ int run_controller_command_handler_tests() { int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, test_persistence_failure_rolls_back_setpoints, test_green_time_loss_rejects_zero, test_timer_updates_are_independent, test_manual_pump_guard_is_application_policy, test_sensor_mapping_is_persisted_and_applied, - test_loop_interval_updates_all_runtime_nodes, test_factory_reset_owns_runtime_reset_mutation}; + test_setting_missing_sensor_role_preserves_existing_mapping, test_loop_interval_updates_all_runtime_nodes, + test_factory_reset_owns_runtime_reset_mutation}; for (auto test : tests) { if (test() == 0) { ++passed; From be5f3523e916cc50dd1d00795d7a48a1f4e2ca35 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:47:14 +0200 Subject: [PATCH 22/57] test: fix partial mapping mock include --- test/native/mocks/ConfigManager.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/native/mocks/ConfigManager.hpp b/test/native/mocks/ConfigManager.hpp index ea2f7e7c..b63c051d 100644 --- a/test/native/mocks/ConfigManager.hpp +++ b/test/native/mocks/ConfigManager.hpp @@ -1,5 +1,6 @@ #pragma once #include "Arduino.h" +#include #include namespace PoolController { From 00eea69e63858acbd13794b5f7e1f0301bc30b6f Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:50:06 +0200 Subject: [PATCH 23/57] test: isolate persistence failure hook --- .../tests/test_controller_command_handler.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/test/native/tests/test_controller_command_handler.cpp b/test/native/tests/test_controller_command_handler.cpp index 79a18871..a1ea1bd9 100644 --- a/test/native/tests/test_controller_command_handler.cpp +++ b/test/native/tests/test_controller_command_handler.cpp @@ -85,28 +85,33 @@ static int test_persistence_failure_rolls_back_setpoints() { settings.tempMaxPool = 28.0F; settings.tempMinSolar = 35.0F; settings.tempHysteresis = 1.0F; - PoolController::ConfigManager::_saveFails = true; - ControllerCommand command{}; command.type = ControllerCommandType::SET_POOL_MAX_TEMPERATURE; command.value = 30.0F; - ASSERT_TRUE(!fixture.handler.handle(command)); + PoolController::ConfigManager::_saveFails = true; + const bool poolAccepted = fixture.handler.handle(command); + PoolController::ConfigManager::_saveFails = false; + ASSERT_TRUE(!poolAccepted); ASSERT_EQ(fixture.operation.getPoolMaxTemperature(), 28.0F); ASSERT_EQ(settings.tempMaxPool, 28.0F); command.type = ControllerCommandType::SET_SOLAR_MIN_TEMPERATURE; command.value = 45.0F; - ASSERT_TRUE(!fixture.handler.handle(command)); + PoolController::ConfigManager::_saveFails = true; + const bool solarAccepted = fixture.handler.handle(command); + PoolController::ConfigManager::_saveFails = false; + ASSERT_TRUE(!solarAccepted); ASSERT_EQ(fixture.operation.getSolarMinTemperature(), 35.0F); ASSERT_EQ(settings.tempMinSolar, 35.0F); command.type = ControllerCommandType::SET_TEMPERATURE_HYSTERESIS; command.value = 2.0F; - ASSERT_TRUE(!fixture.handler.handle(command)); + PoolController::ConfigManager::_saveFails = true; + const bool hysteresisAccepted = fixture.handler.handle(command); + PoolController::ConfigManager::_saveFails = false; + ASSERT_TRUE(!hysteresisAccepted); ASSERT_EQ(fixture.operation.getTemperatureHysteresis(), 1.0F); ASSERT_EQ(settings.tempHysteresis, 1.0F); - - PoolController::ConfigManager::_saveFails = false; return 0; } From b7fe7a51bb75829d430dfdffd45c8ba398d42c13 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:50:09 +0200 Subject: [PATCH 24/57] test: isolate NTP snapshot regression --- .../test_controller_snapshot_publisher.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/native/tests/test_controller_snapshot_publisher.cpp b/test/native/tests/test_controller_snapshot_publisher.cpp index ca2937cd..c6889cac 100644 --- a/test/native/tests/test_controller_snapshot_publisher.cpp +++ b/test/native/tests/test_controller_snapshot_publisher.cpp @@ -182,6 +182,19 @@ static int test_mapping_found_requires_selected_configured_address() { return 0; } +static int test_ntp_server_is_projected() { + test_begin("ControllerSnapshotPublisher", "configured NTP server is projected into snapshot"); + SnapshotFixture fixture; + PoolController::ConfigManager::getNtp().server = "ntp.snapshot.test"; + + fixture.publisher.publish(); + SystemSnapshot snapshot{}; + ASSERT_TRUE(fixture.store.read(snapshot)); + ASSERT_TRUE(snapshot.settings.ntpServer.valid()); + ASSERT_EQ(std::strcmp(snapshot.settings.ntpServer.text.data(), "ntp.snapshot.test"), 0); + return 0; +} + static int test_detected_inventory_is_published_and_deduplicated() { test_begin("ControllerSnapshotPublisher", "detected Dallas inventory is deduplicated across buses"); SnapshotFixture fixture; @@ -233,8 +246,8 @@ int run_controller_snapshot_publisher_tests() { int passed = 0; int failed = 0; int (*tests[])() = {test_complete_runtime_state_is_projected, test_mapping_found_requires_selected_configured_address, - test_detected_inventory_is_published_and_deduplicated, test_snapshot_rule_projection_avoids_verbose_lookup, - test_generation_advances_per_complete_publication}; + test_ntp_server_is_projected, test_detected_inventory_is_published_and_deduplicated, + test_snapshot_rule_projection_avoids_verbose_lookup, test_generation_advances_per_complete_publication}; for (auto test : tests) { if (test() == 0) { ++passed; From 4e96f0ed92a3ca4aeb9ba5332817ab7b2a3377d2 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:52:38 +0200 Subject: [PATCH 25/57] fix: keep command rejection atomic --- src/ControllerCommandHandler.cpp | 39 ++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/ControllerCommandHandler.cpp b/src/ControllerCommandHandler.cpp index 1b377556..c1a79cd4 100644 --- a/src/ControllerCommandHandler.cpp +++ b/src/ControllerCommandHandler.cpp @@ -134,29 +134,53 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll return true; } - case ControllerCommandType::SET_POOL_MAX_TEMPERATURE: + case ControllerCommandType::SET_POOL_MAX_TEMPERATURE: { if (!inRange(command.value, 0.0F, 40.0F)) { return false; } + const float previousRuntime = dependencies_.operationMode.getPoolMaxTemperature(); + const auto previousSetting = settings.tempMaxPool; dependencies_.operationMode.setPoolMaxTemperature(command.value); settings.tempMaxPool = command.value; - return saveSettings(); + if (!saveSettings()) { + dependencies_.operationMode.setPoolMaxTemperature(previousRuntime); + settings.tempMaxPool = previousSetting; + return false; + } + return true; + } - case ControllerCommandType::SET_SOLAR_MIN_TEMPERATURE: + case ControllerCommandType::SET_SOLAR_MIN_TEMPERATURE: { if (!inRange(command.value, 0.0F, 100.0F)) { return false; } + const float previousRuntime = dependencies_.operationMode.getSolarMinTemperature(); + const auto previousSetting = settings.tempMinSolar; dependencies_.operationMode.setSolarMinTemperature(command.value); settings.tempMinSolar = command.value; - return saveSettings(); + if (!saveSettings()) { + dependencies_.operationMode.setSolarMinTemperature(previousRuntime); + settings.tempMinSolar = previousSetting; + return false; + } + return true; + } - case ControllerCommandType::SET_TEMPERATURE_HYSTERESIS: + case ControllerCommandType::SET_TEMPERATURE_HYSTERESIS: { if (!inRange(command.value, 0.0F, 10.0F)) { return false; } + const float previousRuntime = dependencies_.operationMode.getTemperatureHysteresis(); + const auto previousSetting = settings.tempHysteresis; dependencies_.operationMode.setTemperatureHysteresis(command.value); settings.tempHysteresis = command.value; - return saveSettings(); + if (!saveSettings()) { + dependencies_.operationMode.setTemperatureHysteresis(previousRuntime); + settings.tempHysteresis = previousSetting; + return false; + } + return true; + } case ControllerCommandType::SET_TEMPERATURE_CIRCULATION_THRESHOLD: if (!inRange(command.value, 0.0F, 40.0F)) { @@ -226,7 +250,8 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll return true; case ControllerCommandType::SET_TIME_LOSS_GREEN_HOURS: - if (!inRange(command.integerValue, 0, 255)) { + if (!inRange(command.integerValue, 1, 255)) { + LOG_WARN("Controller command: green time-loss hours out of 1..255 range\n"); return false; } settings.timeLossGreenHours = command.integerValue; From 579fcbb5254245bdc14bea3d779f374c2a18172f Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:52:42 +0200 Subject: [PATCH 26/57] fix: preserve partial sensor mappings --- src/ConfigManager.cpp | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/ConfigManager.cpp b/src/ConfigManager.cpp index f9b3581b..31d10e40 100644 --- a/src/ConfigManager.cpp +++ b/src/ConfigManager.cpp @@ -261,29 +261,39 @@ void ConfigManager::saveSensorMapping(const uint8_t solarAddr[8], const uint8_t } bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - Preferences prefs; - prefs.begin("ds18b20", true); // read-only + memset(solarAddr, 0, 8); + memset(poolAddr, 0, 8); - size_t slen = prefs.getBytes("solar_adr", solarAddr, 8); - size_t plen = prefs.getBytes("pool_adr", poolAddr, 8); + Preferences prefs; + if (!prefs.begin("ds18b20", true)) { + return false; + } + const size_t slen = prefs.getBytes("solar_adr", solarAddr, 8); + const size_t plen = prefs.getBytes("pool_adr", poolAddr, 8); prefs.end(); - // Return true if both addresses were read successfully - return (slen == 8 && plen == 8); + if (slen != 8) { + memset(solarAddr, 0, 8); + } + if (plen != 8) { + memset(poolAddr, 0, 8); + } + + // A role may be configured independently. Missing roles remain zeroed so + // callers can safely update one mapping without erasing the other. + return slen == 8 || plen == 8; } bool ConfigManager::loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { + bool hasMapping = true; if (!s_mappingCacheValid) { - if (!loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress)) { - memset(s_cachedSolarAddress, 0, sizeof(s_cachedSolarAddress)); - memset(s_cachedPoolAddress, 0, sizeof(s_cachedPoolAddress)); - } + hasMapping = loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress); s_mappingCacheValid = true; } memcpy(solarAddr, s_cachedSolarAddress, 8); memcpy(poolAddr, s_cachedPoolAddress, 8); - return true; + return hasMapping; } /** @brief Check if an 8-byte address is all zeros. */ From 3e018e89f36b2802e0851ccad373b7f2fef35d09 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:52:45 +0200 Subject: [PATCH 27/57] docs: define partial sensor mapping semantics --- src/ConfigManager.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ConfigManager.hpp b/src/ConfigManager.hpp index 6df14935..650114d7 100644 --- a/src/ConfigManager.hpp +++ b/src/ConfigManager.hpp @@ -90,7 +90,9 @@ class ConfigManager { // ── Sensor Address Mapping ── /** @brief Save DS18B20 sensor address mapping to NVS. */ static void saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]); - /** @brief Load DS18B20 sensor address mapping from NVS. @return true if both addresses loaded. */ + /** @brief Load DS18B20 mappings from NVS. Missing roles remain zeroed. + * @return true if at least one role mapping was loaded. + */ static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); /** @brief Load the sensor mapping through a process-wide cache. * From 7a6f3bb102056572fe19a4bf66e90375e25b09cc Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:52:49 +0200 Subject: [PATCH 28/57] test: align mapping cache semantics --- test/native/mocks/ConfigManager.hpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/native/mocks/ConfigManager.hpp b/test/native/mocks/ConfigManager.hpp index b63c051d..907ddfb3 100644 --- a/test/native/mocks/ConfigManager.hpp +++ b/test/native/mocks/ConfigManager.hpp @@ -64,15 +64,8 @@ class ConfigManager { // ── Sensor Address Mapping ── static void saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]); static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); - // Mirror the production cache's current all-or-nothing behavior so the - // partial-mapping regression test fails until the production fix lands. static bool loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - if (!loadSensorMapping(solarAddr, poolAddr)) { - std::memset(solarAddr, 0, 8); - std::memset(poolAddr, 0, 8); - return false; - } - return true; + return loadSensorMapping(solarAddr, poolAddr); } static void seedSensorMappingForTest( const uint8_t solarAddr[8], bool solarPresent, const uint8_t poolAddr[8], bool poolPresent); From d87277c4d06f3522a258b0648bde859b3dbe2ddd Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:52:52 +0200 Subject: [PATCH 29/57] test: preserve partial mapping in mock --- test/native/mocks/ConfigManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/native/mocks/ConfigManager.cpp b/test/native/mocks/ConfigManager.cpp index edf0c61f..4616f08d 100644 --- a/test/native/mocks/ConfigManager.cpp +++ b/test/native/mocks/ConfigManager.cpp @@ -30,7 +30,7 @@ bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) memset(poolAddr, 0, 8); if (_sensorSolarValid) memcpy(solarAddr, _sensorSolarAddr, 8); if (_sensorPoolValid) memcpy(poolAddr, _sensorPoolAddr, 8); - return _sensorSolarValid && _sensorPoolValid; + return _sensorSolarValid || _sensorPoolValid; } void ConfigManager::seedSensorMappingForTest( From 0acbfa84c72503190534336f914604a80e2a5de4 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:53:10 +0200 Subject: [PATCH 30/57] refactor: expose value IP address for snapshots --- src/NetworkManager.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/NetworkManager.hpp b/src/NetworkManager.hpp index 35a5f9ef..6940fc29 100644 --- a/src/NetworkManager.hpp +++ b/src/NetworkManager.hpp @@ -61,7 +61,8 @@ class NetworkManager { // Getters for status monitoring static int getWiFiRSSI() { return WiFi.status() == WL_CONNECTED ? WiFi.RSSI() : 0; } - static String getLocalIP() { return isApMode() ? WiFi.softAPIP().toString() : WiFi.localIP().toString(); } + static IPAddress getLocalIPAddress() { return isApMode() ? WiFi.softAPIP() : WiFi.localIP(); } + static String getLocalIP() { return getLocalIPAddress().toString(); } private: static void handleWiFiEvent(WiFiEvent_t event); From b2c3526cd754b398b48524e1788bf1b221f48a77 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:53:13 +0200 Subject: [PATCH 31/57] refactor: support quiet active-rule reads --- src/OperationModeNode.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OperationModeNode.hpp b/src/OperationModeNode.hpp index 2f921433..74db7f7d 100644 --- a/src/OperationModeNode.hpp +++ b/src/OperationModeNode.hpp @@ -45,6 +45,7 @@ class OperationModeNode { void addRule(Rule *rule); Rule *getRule(); + Rule *getRule(bool emitDebugLogs); void setPoolTemperatureNode(DallasTemperatureNode *node) { _currentPoolTempNode = node; } void setSolarTemperatureNode(DallasTemperatureNode *node) { _currentSolarTempNode = node; } From d6fd493b5a4f41636139c626e5d9e0c6298b5b7b Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:53:16 +0200 Subject: [PATCH 32/57] fix: avoid snapshot rule log flooding --- src/OperationModeNode.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/OperationModeNode.cpp b/src/OperationModeNode.cpp index 56f59adf..9b44bce4 100644 --- a/src/OperationModeNode.cpp +++ b/src/OperationModeNode.cpp @@ -86,12 +86,20 @@ void OperationModeNode::addRule(Rule *rule) { } Rule *OperationModeNode::getRule() { + return getRule(true); +} + +Rule *OperationModeNode::getRule(bool emitDebugLogs) { const char *mode = PoolController::toString(_mode); - LOG_DEBUG("getRule: mode = %s\n", mode); + if (emitDebugLogs) { + LOG_DEBUG("getRule: mode = %s\n", mode); + } for (size_t i = 0; i < _ruleVec.size(); i++) { if (std::strcmp(mode, _ruleVec[i]->getMode()) == 0) { - LOG_DEBUG("getRule: Active Rule: %s\n", _ruleVec[i]->getMode()); + if (emitDebugLogs) { + LOG_DEBUG("getRule: Active Rule: %s\n", _ruleVec[i]->getMode()); + } // Update ruleset properties _ruleVec[i]->setPoolMaxTemperature(getPoolMaxTemperature()); From 87829b282b0a78710b78e04dc9c02dc4ec1d7ee7 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:53:36 +0200 Subject: [PATCH 33/57] fix: complete immutable snapshot projection --- src/ControllerSnapshotPublisher.cpp | 46 +++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/src/ControllerSnapshotPublisher.cpp b/src/ControllerSnapshotPublisher.cpp index 7ed4d46d..8919eff5 100644 --- a/src/ControllerSnapshotPublisher.cpp +++ b/src/ControllerSnapshotPublisher.cpp @@ -56,12 +56,46 @@ SensorMappingSnapshot mappingSnapshot(const std::array &configu if (mapping.configured && mapping.found) { const std::uint8_t *selectedAddress = node.getDeviceAddress(); - const bool addressMatches = std::memcmp(selectedAddress, configuredAddress.data(), configuredAddress.size()) == 0; - mapping.found = selectedAddress != nullptr && addressMatches; + mapping.found = selectedAddress != nullptr && + std::memcmp(selectedAddress, configuredAddress.data(), configuredAddress.size()) == 0; } return mapping; } +bool containsDetectedAddress(const SensorSnapshot &sensors, const std::uint8_t address[8]) { + for (std::uint8_t i = 0; i < sensors.detectedCount; ++i) { + if (std::memcmp(sensors.detected[i].address.data(), address, sensors.detected[i].address.size()) == 0) { + return true; + } + } + return false; +} + +void appendDetectedSensors(SensorSnapshot &sensors, const DallasTemperatureNode &node) { + const std::uint8_t deviceCount = node.getDeviceCount(); + for (std::uint8_t index = 0; index < deviceCount && sensors.detectedCount < kMaxDetectedSensors; ++index) { + std::array address{}; + if (!node.getDetectedDeviceAddress(index, address.data()) || containsDetectedAddress(sensors, address.data())) { + continue; + } + + DetectedSensorSnapshot &detected = sensors.detected[sensors.detectedCount++]; + detected.address = address; + const float temperature = node.getDetectedDeviceTemperature(index); + detected.temperature.value = temperature; + detected.temperature.valid = std::isfinite(temperature); + } +} + +Ipv4AddressSnapshot ipv4Snapshot(const IPAddress &address) { + Ipv4AddressSnapshot snapshot{}; + for (std::size_t i = 0; i < snapshot.octets.size(); ++i) { + snapshot.octets[i] = address[static_cast(i)]; + snapshot.valid = snapshot.valid || snapshot.octets[i] != 0; + } + return snapshot; +} + TimeDegradationState snapshotTimeDegradation(TimeDegradation degradation) { switch (degradation) { case TimeDegradation::GREEN: @@ -93,6 +127,8 @@ void ControllerSnapshotPublisher::publish() { ConfigManager::loadSensorMappingCached(solarAddress.data(), poolAddress.data()); snapshot.sensors.solarMapping = mappingSnapshot(solarAddress, dependencies_.solarTemperature); snapshot.sensors.poolMapping = mappingSnapshot(poolAddress, dependencies_.poolTemperature); + appendDetectedSensors(snapshot.sensors, dependencies_.solarTemperature); + appendDetectedSensors(snapshot.sensors, dependencies_.poolTemperature); // Transitional generation: this identifies complete Core-1 read-model // publications. Once #170 provides a coherent Core-0 acquisition snapshot, @@ -104,6 +140,7 @@ void ControllerSnapshotPublisher::publish() { snapshot.network.mqttConnected = NetworkManager::isMqttConnected(); snapshot.network.apMode = NetworkManager::isApMode(); snapshot.network.wifiRssi = static_cast(NetworkManager::getWiFiRSSI()); + snapshot.network.localIp = ipv4Snapshot(NetworkManager::getLocalIPAddress()); snapshot.health.safeMode = DegradationManager::isSafe(); snapshot.health.timeValid = isTimeSyncValid(); @@ -122,13 +159,16 @@ void ControllerSnapshotPublisher::publish() { snapshot.timer.endHour = timer.timerEndHour; snapshot.timer.endMinute = timer.timerEndMinutes; - Rule *activeRule = dependencies_.operationMode.getRule(); + Rule *activeRule = dependencies_.operationMode.getRule(false); if (activeRule != nullptr) { snapshot.timer.effectiveRuntimeMinutes = activeRule->getEffectiveRuntimeMinutes(); snapshot.timer.circulationExtensionMinutes = activeRule->getCirculationExtensionMinutes(); snapshot.timer.activeEndMinutes = activeRule->getActiveEndMinutes(); } + const auto &ntp = ConfigManager::getNtp(); + const char *ntpServer = ntp.server.c_str(); + snapshot.settings.ntpServer.assign(ntpServer, std::strlen(ntpServer)); snapshot.settings.loopInterval = static_cast(settings.loopInterval); snapshot.settings.timezoneIndex = static_cast(settings.timezoneIndex); snapshot.settings.timeLossGreenHours = static_cast(settings.timeLossGreenHours); From ceb008739d3b83c12aa7e58adf1a11dffe54884c Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:53:53 +0200 Subject: [PATCH 34/57] fix: retain sensor mapping cache validity --- src/ConfigManager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ConfigManager.cpp b/src/ConfigManager.cpp index 31d10e40..e9496c6c 100644 --- a/src/ConfigManager.cpp +++ b/src/ConfigManager.cpp @@ -239,6 +239,7 @@ bool ConfigManager::verifyAdminPassword(const String &password) { namespace { // Process-wide sensor-mapping cache so hot loops avoid NVS traffic. bool s_mappingCacheValid = false; +bool s_mappingCacheHasMapping = false; std::uint8_t s_cachedSolarAddress[8] = {0}; std::uint8_t s_cachedPoolAddress[8] = {0}; } // namespace @@ -286,14 +287,13 @@ bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) } bool ConfigManager::loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - bool hasMapping = true; if (!s_mappingCacheValid) { - hasMapping = loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress); + s_mappingCacheHasMapping = loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress); s_mappingCacheValid = true; } memcpy(solarAddr, s_cachedSolarAddress, 8); memcpy(poolAddr, s_cachedPoolAddress, 8); - return hasMapping; + return s_mappingCacheHasMapping; } /** @brief Check if an 8-byte address is all zeros. */ From c1aa5f9f2213978b75acec7b54c3fd939a6dfe84 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:55:43 +0200 Subject: [PATCH 35/57] docs: record snapshot and atomic command invariants --- openspec/changes/adapter-command-snapshot-migration/design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openspec/changes/adapter-command-snapshot-migration/design.md b/openspec/changes/adapter-command-snapshot-migration/design.md index 42059eb4..386afa08 100644 --- a/openspec/changes/adapter-command-snapshot-migration/design.md +++ b/openspec/changes/adapter-command-snapshot-migration/design.md @@ -15,7 +15,7 @@ Local UI -> interaction mapping -> ControllerCommandQueue -> Core-1 command handler -> state mutation ``` -The typed path covers runtime controller mutations: mode, setpoints, circulation settings, independent timer start/end updates, loop interval, timezone/time-loss thresholds, NORVI button thresholds, sensor mappings and manual relay commands. +The typed path covers runtime controller mutations: mode, setpoints, circulation settings, independent timer start/end updates, loop interval, timezone/time-loss thresholds, NORVI button thresholds, sensor mappings and manual relay commands. A rejected command must be atomic from the owner's perspective: if persistence fails after an in-memory mutation, both runtime state and the mutable settings projection are restored before failure is returned. Sensor-role mappings are independent values, so reading or updating one missing role must never erase the other role. For synchronous Web responses, the first migration may process the typed command immediately when the handler is already executing on the owning loop task. The command handler remains the single mutation API for that runtime state. @@ -28,7 +28,7 @@ Core-1 state -> SystemSnapshot -> ControllerSnapshotStore -> OLED/TFT ``` -Adapters receive value copies. They do not receive node pointers or mutable runtime-config references. The snapshot must preserve the existing external projection, including timer runtime state, sensor mapping identity/configured/found state and GREEN/YELLOW/RED time degradation. +Adapters receive value copies. They do not receive node pointers or mutable runtime-config references. The snapshot must preserve the existing external projection, including timer runtime state, sensor mapping identity/configured/found state, deduplicated detected-sensor inventory, local IPv4 state, configured NTP server and GREEN/YELLOW/RED time degradation. Snapshot publication must use read-only or quiet domain accessors so the high-frequency publication path does not create logging side effects. ## Migration order From da0f798e4bd4871100dfb3bb5b3be7ad373999a4 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:55:47 +0200 Subject: [PATCH 36/57] docs: sync review regression work --- openspec/changes/adapter-command-snapshot-migration/tasks.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openspec/changes/adapter-command-snapshot-migration/tasks.md b/openspec/changes/adapter-command-snapshot-migration/tasks.md index 775fe864..51771ef7 100644 --- a/openspec/changes/adapter-command-snapshot-migration/tasks.md +++ b/openspec/changes/adapter-command-snapshot-migration/tasks.md @@ -9,7 +9,11 @@ - [x] Keep provisioning/authentication secrets and specialized calibration workflows behind their dedicated service boundaries. - [x] Publish `SystemSnapshot` from the Core-1 owner after state updates. - [x] Populate sensor mapping address/configured/found state in the snapshot. +- [x] Preserve independently configured sensor mappings when the other role is absent. +- [x] Populate detected-sensor inventory, local IPv4 state and configured NTP server in the snapshot. - [x] Populate GREEN/YELLOW/RED time degradation and runtime controller settings in the snapshot. +- [x] Keep failed persisted runtime mutations atomic by rolling back owner state and settings. +- [x] Keep snapshot publication free of active-rule debug-log side effects. - [ ] Migrate MQTT state/discovery reads to snapshot values where applicable. - [ ] Migrate Web status/config read paths to snapshot values where applicable. - [ ] Migrate OLED/TFT read paths to snapshot values. From db01ed1109d29de6016f6bf2e3df8038deb5920e Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:58:46 +0200 Subject: [PATCH 37/57] style: apply clang-format to snapshot publisher --- src/ControllerSnapshotPublisher.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ControllerSnapshotPublisher.cpp b/src/ControllerSnapshotPublisher.cpp index 8919eff5..29b98166 100644 --- a/src/ControllerSnapshotPublisher.cpp +++ b/src/ControllerSnapshotPublisher.cpp @@ -56,8 +56,8 @@ SensorMappingSnapshot mappingSnapshot(const std::array &configu if (mapping.configured && mapping.found) { const std::uint8_t *selectedAddress = node.getDeviceAddress(); - mapping.found = selectedAddress != nullptr && - std::memcmp(selectedAddress, configuredAddress.data(), configuredAddress.size()) == 0; + mapping.found = + selectedAddress != nullptr && std::memcmp(selectedAddress, configuredAddress.data(), configuredAddress.size()) == 0; } return mapping; } From fb25c1667acf1fdcd38955c049dcc80433e090e5 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:58:49 +0200 Subject: [PATCH 38/57] style: apply clang-format to command tests --- test/native/tests/test_controller_command_handler.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/native/tests/test_controller_command_handler.cpp b/test/native/tests/test_controller_command_handler.cpp index a1ea1bd9..174110ca 100644 --- a/test/native/tests/test_controller_command_handler.cpp +++ b/test/native/tests/test_controller_command_handler.cpp @@ -261,11 +261,10 @@ static int test_factory_reset_owns_runtime_reset_mutation() { int run_controller_command_handler_tests() { int passed = 0; int failed = 0; - int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, - test_persistence_failure_rolls_back_setpoints, test_green_time_loss_rejects_zero, test_timer_updates_are_independent, - test_manual_pump_guard_is_application_policy, test_sensor_mapping_is_persisted_and_applied, - test_setting_missing_sensor_role_preserves_existing_mapping, test_loop_interval_updates_all_runtime_nodes, - test_factory_reset_owns_runtime_reset_mutation}; + int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, test_persistence_failure_rolls_back_setpoints, + test_green_time_loss_rejects_zero, test_timer_updates_are_independent, test_manual_pump_guard_is_application_policy, + test_sensor_mapping_is_persisted_and_applied, test_setting_missing_sensor_role_preserves_existing_mapping, + test_loop_interval_updates_all_runtime_nodes, test_factory_reset_owns_runtime_reset_mutation}; for (auto test : tests) { if (test() == 0) { ++passed; From b3995ce202036444924aafec087d40bf984e2349 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:58:54 +0200 Subject: [PATCH 39/57] docs: fix web UI skill markdownlint --- .opencode/skills/web-ui/SKILL.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.opencode/skills/web-ui/SKILL.md b/.opencode/skills/web-ui/SKILL.md index fe2e4747..e357044b 100644 --- a/.opencode/skills/web-ui/SKILL.md +++ b/.opencode/skills/web-ui/SKILL.md @@ -31,7 +31,6 @@ keywords: Web interface development for the ESP32 pool-controller. The UI lives in two layers: **LittleFS** (editable at runtime) and **PROGMEM** (embedded fallback in the firmware binary). > **Note:** The Olimex local TFT UI is a separate on-device interface (Adafruit GFX + ILI9341/ST7789) and is not the browser/LittleFS dashboard. - > **🔍 Code Search**: Use `semble search "LittleFS.open"` or `semble search "kPortalPageHtml"` to find the fallback architecture. See `Agents.md` §7 for full `semble` usage. --- From 952276c0e1fb01b49aae90040df3f45970db6152 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:58:58 +0200 Subject: [PATCH 40/57] docs: wrap Olimex implementation plan --- .../plans/2026-08-10-olimex-c6-local-ui-implementation.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-08-10-olimex-c6-local-ui-implementation.md b/docs/superpowers/plans/2026-08-10-olimex-c6-local-ui-implementation.md index 82aac549..cd453dc7 100644 --- a/docs/superpowers/plans/2026-08-10-olimex-c6-local-ui-implementation.md +++ b/docs/superpowers/plans/2026-08-10-olimex-c6-local-ui-implementation.md @@ -103,7 +103,8 @@ constexpr bool TFT_DRIVER_ILI9341{true}; constexpr bool TFT_DRIVER_ST7789{false}; ``` -Do not use GPIO1/2/3/15 for UI; those are Olimex opto inputs. Do not use GPIO10/11/22/23 except for relays. GPIO12/13/14 are not useful defaults because they are not exposed on the standard EVB headers. The defaults above intentionally keep the encoder on UEXT UART1 pins GPIO4/GPIO5 plus EXT1 GPIO0, keep TFT SPI on UEXT SPI, and avoid display reset by requiring the concrete TFT module to tie reset high or expose a reset pad that can be wired later. +Do not use GPIO1/2/3/15 for UI; those are Olimex opto inputs. Do not use GPIO10/11/22/23 except for relays. +GPIO12/13/14 are not useful defaults because they are not exposed on the standard EVB headers. The defaults above intentionally keep the encoder on UEXT UART1 pins GPIO4/GPIO5 plus EXT1 GPIO0, keep TFT SPI on UEXT SPI, and avoid display reset by requiring the concrete TFT module to tie reset high or expose a reset pad that can be wired later. - [ ] **Step 2: Add static assertions for display driver selection** From 7a2887539540bd47f385d6924d8db3fe3b88839f Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:59:02 +0200 Subject: [PATCH 41/57] docs: wrap NORVI calibration plan --- docs/superpowers/plans/2026-08-16-norvi-button-calibration.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-08-16-norvi-button-calibration.md b/docs/superpowers/plans/2026-08-16-norvi-button-calibration.md index 73ebad78..60aa69e0 100644 --- a/docs/superpowers/plans/2026-08-16-norvi-button-calibration.md +++ b/docs/superpowers/plans/2026-08-16-norvi-button-calibration.md @@ -4,7 +4,8 @@ **Goal:** Add a guided calibration wizard to the web UI that measures the NORVI AE01-R front-panel button ADC levels and derives the button thresholds automatically. -**Architecture:** A separate `CalibrationManager` module owns a calibration state machine (IDLE → RESTING → BTN1 → BTN2 → BTN3 → COMPUTE → SAVE → DONE/ERROR) with injectable ADC-read and time functions for native testing. `NorviButtonHandler::loop()` early-returns while calibration is active. WebPortal exposes three auth-protected REST endpoints; the web UI shows a modal wizard that polls the status endpoint. +**Architecture:** A separate `CalibrationManager` module owns a calibration state machine (IDLE → RESTING → BTN1 → BTN2 → BTN3 → COMPUTE → SAVE → DONE/ERROR) with injectable ADC-read and time functions for native testing. +`NorviButtonHandler::loop()` early-returns while calibration is active. WebPortal exposes three auth-protected REST endpoints; the web UI shows a modal wizard that polls the status endpoint. **Tech Stack:** C++17 (ESP32/Arduino, PlatformIO env `norvi_ae01_r`), ArduinoJson, native CMake test runner (`test/native`), vanilla JS/CSS web assets on LittleFS. From a9c03cf36a574a9cc5a891a3d64bbe1ce9868e80 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:59:15 +0200 Subject: [PATCH 42/57] docs: wrap adapter migration design --- .../changes/adapter-command-snapshot-migration/design.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/openspec/changes/adapter-command-snapshot-migration/design.md b/openspec/changes/adapter-command-snapshot-migration/design.md index 386afa08..7bff6e4c 100644 --- a/openspec/changes/adapter-command-snapshot-migration/design.md +++ b/openspec/changes/adapter-command-snapshot-migration/design.md @@ -15,7 +15,9 @@ Local UI -> interaction mapping -> ControllerCommandQueue -> Core-1 command handler -> state mutation ``` -The typed path covers runtime controller mutations: mode, setpoints, circulation settings, independent timer start/end updates, loop interval, timezone/time-loss thresholds, NORVI button thresholds, sensor mappings and manual relay commands. A rejected command must be atomic from the owner's perspective: if persistence fails after an in-memory mutation, both runtime state and the mutable settings projection are restored before failure is returned. Sensor-role mappings are independent values, so reading or updating one missing role must never erase the other role. +The typed path covers runtime controller mutations: mode, setpoints, circulation settings, independent timer start/end updates, loop interval, timezone/time-loss thresholds, NORVI button thresholds, sensor mappings and manual relay commands. +A rejected command must be atomic from the owner's perspective: if persistence fails after an in-memory mutation, both runtime state and the mutable settings projection are restored before failure is returned. +Sensor-role mappings are independent values, so reading or updating one missing role must never erase the other role. For synchronous Web responses, the first migration may process the typed command immediately when the handler is already executing on the owning loop task. The command handler remains the single mutation API for that runtime state. @@ -28,7 +30,9 @@ Core-1 state -> SystemSnapshot -> ControllerSnapshotStore -> OLED/TFT ``` -Adapters receive value copies. They do not receive node pointers or mutable runtime-config references. The snapshot must preserve the existing external projection, including timer runtime state, sensor mapping identity/configured/found state, deduplicated detected-sensor inventory, local IPv4 state, configured NTP server and GREEN/YELLOW/RED time degradation. Snapshot publication must use read-only or quiet domain accessors so the high-frequency publication path does not create logging side effects. +Adapters receive value copies. They do not receive node pointers or mutable runtime-config references. +The snapshot must preserve the existing external projection, including timer runtime state, sensor mapping identity/configured/found state, deduplicated detected-sensor inventory, local IPv4 state, configured NTP server and GREEN/YELLOW/RED time degradation. +Snapshot publication must use read-only or quiet domain accessors so the high-frequency publication path does not create logging side effects. ## Migration order From 7e1afb7f966d5d640a203b846f734f7a46f4ef0e Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 11 Oct 2026 18:59:19 +0200 Subject: [PATCH 43/57] docs: wrap architecture contract design --- openspec/changes/controller-architecture-contracts/design.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openspec/changes/controller-architecture-contracts/design.md b/openspec/changes/controller-architecture-contracts/design.md index d2fd48a2..d6a82c2e 100644 --- a/openspec/changes/controller-architecture-contracts/design.md +++ b/openspec/changes/controller-architecture-contracts/design.md @@ -168,7 +168,8 @@ branches after the foundation merges; #218 already contains queue/store code and adopted. - This PR introduces no active instances/call-site changes, so production RAM/Flash/CPU behavior remains unchanged. Reverting its contract commit is sufficient before adoption; after adoption, dependent consumers must be reverted together. -- Known extensions deliberately deferred out of this PR are tracked in #222: current-epoch projection, `minFreeHeapBytes`, active SSID, batch semantics for the full Web settings form, and a bounded mode-audit origin discriminator. They must be decided before #218 adopts these read models; epoch and min-heap additions exceed the current 600-byte `SystemSnapshot` guard (596 bytes used), so that guard gets revised with the extension, not silently. +- Known extensions deliberately deferred out of this PR are tracked in #222: current-epoch projection, `minFreeHeapBytes`, active SSID, batch semantics for the full Web settings form, and a bounded mode-audit origin discriminator. + They must be decided before #218 adopts these read models; epoch and min-heap additions exceed the current 600-byte `SystemSnapshot` guard (596 bytes used), so that guard gets revised with the extension, not silently. ## Concurrency invariants From 7860f314f97e622879789a28755a379d999257c7 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:44:42 +0200 Subject: [PATCH 44/57] fix: make reviewed command mutations atomic --- src/ControllerCommandHandler.cpp | 33 +++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/ControllerCommandHandler.cpp b/src/ControllerCommandHandler.cpp index c1a79cd4..09ae0193 100644 --- a/src/ControllerCommandHandler.cpp +++ b/src/ControllerCommandHandler.cpp @@ -71,7 +71,10 @@ bool ControllerCommandHandler::saveSettings() const { bool ControllerCommandHandler::applySensorMapping(const ControllerCommand &command, bool clear) { std::uint8_t solarAddress[8] = {0}; std::uint8_t poolAddress[8] = {0}; - ConfigManager::loadSensorMappingCached(solarAddress, poolAddress); + if (!ConfigManager::loadSensorMappingCached(solarAddress, poolAddress)) { + LOG_ERROR("Controller command: failed to load sensor mapping before update\n"); + return false; + } std::uint8_t *target = command.sensorRole == SensorRole::SOLAR ? solarAddress : poolAddress; for (std::size_t i = 0; i < command.sensorAddress.size(); ++i) { @@ -225,18 +228,24 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll return true; } - case ControllerCommandType::SET_LOOP_INTERVAL: + case ControllerCommandType::SET_LOOP_INTERVAL: { if (!inRange(command.integerValue, 1, 3600)) { return false; } + const auto previousSetting = settings.loopInterval; settings.loopInterval = command.integerValue; + if (!saveSettings()) { + settings.loopInterval = previousSetting; + return false; + } dependencies_.solarTemperature.setMeasurementInterval(command.integerValue); dependencies_.poolTemperature.setMeasurementInterval(command.integerValue); dependencies_.controllerTemperature.setMeasurementInterval(command.integerValue); dependencies_.poolPump.setMeasurementInterval(command.integerValue); dependencies_.solarPump.setMeasurementInterval(command.integerValue); dependencies_.operationMode.setMeasurementInterval(command.integerValue); - return saveSettings(); + return true; + } case ControllerCommandType::SET_TIMEZONE: if (!inRange(command.integerValue, 0, getTzCount() - 1)) { @@ -249,29 +258,35 @@ bool ControllerCommandHandler::handle(const ControllerCommand &command, Controll setTimezoneIndex(command.integerValue); return true; - case ControllerCommandType::SET_TIME_LOSS_GREEN_HOURS: - if (!inRange(command.integerValue, 1, 255)) { - LOG_WARN("Controller command: green time-loss hours out of 1..255 range\n"); + case ControllerCommandType::SET_TIME_LOSS_GREEN_HOURS: { + if (!inRange(command.integerValue, 1, 71) || command.integerValue >= settings.timeLossRedHours) { + LOG_WARN("Controller command: green time-loss hours must satisfy 1 <= green < red <= 72\n"); return false; } + const auto previousSetting = settings.timeLossGreenHours; settings.timeLossGreenHours = command.integerValue; if (!saveSettings()) { + settings.timeLossGreenHours = previousSetting; return false; } setTimeDegradationGreenHours(static_cast(command.integerValue)); return true; + } - case ControllerCommandType::SET_TIME_LOSS_RED_HOURS: - if (!inRange(command.integerValue, 1, 72)) { - LOG_WARN("Controller command: red time-loss hours out of 1..72 range\n"); + case ControllerCommandType::SET_TIME_LOSS_RED_HOURS: { + if (!inRange(command.integerValue, 2, 72) || command.integerValue <= settings.timeLossGreenHours) { + LOG_WARN("Controller command: red time-loss hours must satisfy 1 <= green < red <= 72\n"); return false; } + const auto previousSetting = settings.timeLossRedHours; settings.timeLossRedHours = command.integerValue; if (!saveSettings()) { + settings.timeLossRedHours = previousSetting; return false; } setTimeDegradationRedHours(static_cast(command.integerValue)); return true; + } case ControllerCommandType::SET_BUTTON_1_MIN: case ControllerCommandType::SET_BUTTON_1_MAX: From c1638b3f49e17cfb99d10addde5575ee59f61105 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:44:56 +0200 Subject: [PATCH 45/57] fix: keep sensor mapping cache retryable --- src/ConfigManager.cpp | 67 +++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/src/ConfigManager.cpp b/src/ConfigManager.cpp index e9496c6c..4ed0d92b 100644 --- a/src/ConfigManager.cpp +++ b/src/ConfigManager.cpp @@ -239,9 +239,36 @@ bool ConfigManager::verifyAdminPassword(const String &password) { namespace { // Process-wide sensor-mapping cache so hot loops avoid NVS traffic. bool s_mappingCacheValid = false; -bool s_mappingCacheHasMapping = false; std::uint8_t s_cachedSolarAddress[8] = {0}; std::uint8_t s_cachedPoolAddress[8] = {0}; + +struct SensorMappingReadResult final { + bool readSucceeded{false}; + bool hasMapping{false}; +}; + +SensorMappingReadResult readSensorMappingFromNvs(uint8_t solarAddr[8], uint8_t poolAddr[8]) { + memset(solarAddr, 0, 8); + memset(poolAddr, 0, 8); + + Preferences prefs; + if (!prefs.begin("ds18b20", true)) { + return {}; + } + + const size_t solarLength = prefs.getBytes("solar_adr", solarAddr, 8); + const size_t poolLength = prefs.getBytes("pool_adr", poolAddr, 8); + prefs.end(); + + if (solarLength != 8) { + memset(solarAddr, 0, 8); + } + if (poolLength != 8) { + memset(poolAddr, 0, 8); + } + + return {true, solarLength == 8 || poolLength == 8}; +} } // namespace void ConfigManager::saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]) { @@ -262,38 +289,28 @@ void ConfigManager::saveSensorMapping(const uint8_t solarAddr[8], const uint8_t } bool ConfigManager::loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - memset(solarAddr, 0, 8); - memset(poolAddr, 0, 8); - - Preferences prefs; - if (!prefs.begin("ds18b20", true)) { - return false; - } - - const size_t slen = prefs.getBytes("solar_adr", solarAddr, 8); - const size_t plen = prefs.getBytes("pool_adr", poolAddr, 8); - prefs.end(); - - if (slen != 8) { - memset(solarAddr, 0, 8); - } - if (plen != 8) { - memset(poolAddr, 0, 8); - } - - // A role may be configured independently. Missing roles remain zeroed so - // callers can safely update one mapping without erasing the other. - return slen == 8 || plen == 8; + // Preserve the legacy return contract: true means at least one role exists. + return readSensorMappingFromNvs(solarAddr, poolAddr).hasMapping; } bool ConfigManager::loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { if (!s_mappingCacheValid) { - s_mappingCacheHasMapping = loadSensorMapping(s_cachedSolarAddress, s_cachedPoolAddress); + std::uint8_t solarCandidate[8] = {0}; + std::uint8_t poolCandidate[8] = {0}; + const SensorMappingReadResult readResult = readSensorMappingFromNvs(solarCandidate, poolCandidate); + if (!readResult.readSucceeded) { + // Do not poison the process-wide cache on a transient NVS failure. + memset(solarAddr, 0, 8); + memset(poolAddr, 0, 8); + return false; + } + memcpy(s_cachedSolarAddress, solarCandidate, 8); + memcpy(s_cachedPoolAddress, poolCandidate, 8); s_mappingCacheValid = true; } memcpy(solarAddr, s_cachedSolarAddress, 8); memcpy(poolAddr, s_cachedPoolAddress, 8); - return s_mappingCacheHasMapping; + return true; } /** @brief Check if an 8-byte address is all zeros. */ From 8c04553a287e98a48e6dd225a80a1b28c0d79070 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:03 +0200 Subject: [PATCH 46/57] docs: clarify sensor mapping cache contract --- src/ConfigManager.hpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ConfigManager.hpp b/src/ConfigManager.hpp index 650114d7..f3ec5abd 100644 --- a/src/ConfigManager.hpp +++ b/src/ConfigManager.hpp @@ -96,9 +96,12 @@ class ConfigManager { static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); /** @brief Load the sensor mapping through a process-wide cache. * - * The first call reads NVS; subsequent calls serve the cached pair. The - * cache is invalidated by saveSensorMapping()/clearSensorMapping() so hot - * loops never touch NVS for unchanged mappings. + * The first successful call reads NVS; subsequent calls serve the cached + * pair. Transient NVS-open failures are not cached and are retried later. + * The cache is invalidated by saveSensorMapping() so hot loops never touch + * NVS for unchanged mappings. + * @return true when NVS was read successfully or a valid cache exists; + * false on a transient read failure. Empty mappings are successful. */ static bool loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]); From ea55a48a267ca129c714b4ea13b2400962a36feb Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:11 +0200 Subject: [PATCH 47/57] refactor: cache detected sensor inventory --- src/ControllerSnapshotPublisher.hpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ControllerSnapshotPublisher.hpp b/src/ControllerSnapshotPublisher.hpp index a898cacf..b056fb8b 100644 --- a/src/ControllerSnapshotPublisher.hpp +++ b/src/ControllerSnapshotPublisher.hpp @@ -8,8 +8,11 @@ #pragma once +#include #include +#include "ControllerSnapshot.hpp" + class DallasTemperatureNode; class ESP32TemperatureNode; class OperationModeNode; @@ -45,7 +48,11 @@ class ControllerSnapshotPublisher final { private: Dependencies dependencies_; + std::array detectedInventory_{}; + std::uint8_t detectedInventoryCount_{0}; + std::uint32_t lastDetectedInventoryRefreshMs_{0}; std::uint32_t generation_{0}; + bool detectedInventoryInitialized_{false}; }; } // namespace PoolController From 44f617bca0f482aacc8bd2b7aa7c3342ebba5687 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:19 +0200 Subject: [PATCH 48/57] fix: bound Dallas inventory reads in snapshots --- src/ControllerSnapshotPublisher.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/ControllerSnapshotPublisher.cpp b/src/ControllerSnapshotPublisher.cpp index 29b98166..bdfa57d6 100644 --- a/src/ControllerSnapshotPublisher.cpp +++ b/src/ControllerSnapshotPublisher.cpp @@ -34,10 +34,14 @@ bool isZeroAddress(const std::array &address) { return true; } +bool isValidDallasTemperature(float value) { + return std::isfinite(value) && value != DEVICE_DISCONNECTED_C; +} + SensorReadingSnapshot sensorReading(float value, bool found) { SensorReadingSnapshot reading{}; reading.value = value; - reading.valid = found && std::isfinite(value); + reading.valid = found && isValidDallasTemperature(value); return reading; } @@ -83,7 +87,7 @@ void appendDetectedSensors(SensorSnapshot &sensors, const DallasTemperatureNode detected.address = address; const float temperature = node.getDetectedDeviceTemperature(index); detected.temperature.value = temperature; - detected.temperature.valid = std::isfinite(temperature); + detected.temperature.valid = isValidDallasTemperature(temperature); } } @@ -113,6 +117,7 @@ TimeDegradationState snapshotTimeDegradation(TimeDegradation degradation) { void ControllerSnapshotPublisher::publish() { SystemSnapshot snapshot{}; const std::uint32_t nowMs = millis(); + const auto &settings = ConfigManager::getSettings(); const float poolTemperature = dependencies_.poolTemperature.getTemperature(); const float solarTemperature = dependencies_.solarTemperature.getTemperature(); @@ -127,8 +132,20 @@ void ControllerSnapshotPublisher::publish() { ConfigManager::loadSensorMappingCached(solarAddress.data(), poolAddress.data()); snapshot.sensors.solarMapping = mappingSnapshot(solarAddress, dependencies_.solarTemperature); snapshot.sensors.poolMapping = mappingSnapshot(poolAddress, dependencies_.poolTemperature); - appendDetectedSensors(snapshot.sensors, dependencies_.solarTemperature); - appendDetectedSensors(snapshot.sensors, dependencies_.poolTemperature); + const std::uint32_t detectedRefreshSeconds = + settings.loopInterval > 0 ? static_cast(settings.loopInterval) : 1U; + const std::uint32_t detectedRefreshIntervalMs = detectedRefreshSeconds * 1000U; + if (!detectedInventoryInitialized_ || nowMs - lastDetectedInventoryRefreshMs_ >= detectedRefreshIntervalMs) { + SensorSnapshot refreshedInventory{}; + appendDetectedSensors(refreshedInventory, dependencies_.solarTemperature); + appendDetectedSensors(refreshedInventory, dependencies_.poolTemperature); + detectedInventory_ = refreshedInventory.detected; + detectedInventoryCount_ = refreshedInventory.detectedCount; + lastDetectedInventoryRefreshMs_ = nowMs; + detectedInventoryInitialized_ = true; + } + snapshot.sensors.detected = detectedInventory_; + snapshot.sensors.detectedCount = detectedInventoryCount_; // Transitional generation: this identifies complete Core-1 read-model // publications. Once #170 provides a coherent Core-0 acquisition snapshot, @@ -148,7 +165,6 @@ void ControllerSnapshotPublisher::publish() { snapshot.health.poolSensorValid = snapshot.sensors.pool.valid; snapshot.health.solarSensorValid = snapshot.sensors.solar.valid; - const auto &settings = ConfigManager::getSettings(); snapshot.circulation.threshold = settings.tempCircThreshold; snapshot.circulation.factorMinutesPerDegree = static_cast(settings.tempCircFactor); snapshot.circulation.maxRuntimeMinutes = static_cast(settings.tempCircMaxRuntime); From 4809cd3ebee870e3b8161743a6622a3929a4e9ba Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:26 +0200 Subject: [PATCH 49/57] test: expose Dallas inventory read count --- test/native/mocks/DallasTemperatureNode.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/native/mocks/DallasTemperatureNode.hpp b/test/native/mocks/DallasTemperatureNode.hpp index e493f973..84afe352 100644 --- a/test/native/mocks/DallasTemperatureNode.hpp +++ b/test/native/mocks/DallasTemperatureNode.hpp @@ -49,8 +49,10 @@ class DallasTemperatureNode { return true; } float getDetectedDeviceTemperature(uint8_t index) const { + ++_detectedTemperatureReadCount; return index < _detectedCount ? _detectedTemperatures[index] : NAN; } + uint32_t getDetectedTemperatureReadCount() const { return _detectedTemperatureReadCount; } void setDetectedDevice(uint8_t index, const DeviceAddress address, float temperature) { if (index >= _detectedAddresses.size()) return; std::memcpy(_detectedAddresses[index].data(), address, 8); @@ -71,4 +73,5 @@ class DallasTemperatureNode { std::array, 24> _detectedAddresses{}; std::array _detectedTemperatures{}; uint8_t _detectedCount = 0; + mutable uint32_t _detectedTemperatureReadCount = 0; }; From d0a592637499fef5c5aa5bfaff28581a0bb3caf9 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:34 +0200 Subject: [PATCH 50/57] test: model transient sensor mapping read failures --- test/native/mocks/ConfigManager.hpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/native/mocks/ConfigManager.hpp b/test/native/mocks/ConfigManager.hpp index 907ddfb3..09e372b5 100644 --- a/test/native/mocks/ConfigManager.hpp +++ b/test/native/mocks/ConfigManager.hpp @@ -54,6 +54,7 @@ class ConfigManager { static bool load() { return true; } static bool save() { return !_saveFails; } static bool _saveFails; // test hook + static bool _sensorMappingReadFails; // test hook static void reset() { _adminPasswordHash = // gitleaks:allow SHA-256("admin") F("8c6976e5b5410415bde908bd4dee15dfb167a9c873fc4bb8a81f6f2ab448a918"); @@ -65,7 +66,13 @@ class ConfigManager { static void saveSensorMapping(const uint8_t solarAddr[8], const uint8_t poolAddr[8]); static bool loadSensorMapping(uint8_t solarAddr[8], uint8_t poolAddr[8]); static bool loadSensorMappingCached(uint8_t solarAddr[8], uint8_t poolAddr[8]) { - return loadSensorMapping(solarAddr, poolAddr); + if (_sensorMappingReadFails) { + std::memset(solarAddr, 0, 8); + std::memset(poolAddr, 0, 8); + return false; + } + loadSensorMapping(solarAddr, poolAddr); + return true; } static void seedSensorMappingForTest( const uint8_t solarAddr[8], bool solarPresent, const uint8_t poolAddr[8], bool poolPresent); From bee0a6feb0344b331cf3b2614272ff67c33cbd8b Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:37 +0200 Subject: [PATCH 51/57] test: initialize sensor mapping failure hook --- test/native/mocks/ConfigManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/native/mocks/ConfigManager.cpp b/test/native/mocks/ConfigManager.cpp index 4616f08d..e4f52e12 100644 --- a/test/native/mocks/ConfigManager.cpp +++ b/test/native/mocks/ConfigManager.cpp @@ -11,6 +11,7 @@ String ConfigManager::_adminPasswordHash = // SHA-256("admin") "8c6976e5b5410415bde908bd4dee15dfb167a9c873fc4bb8a81f6f2ab448a918"; bool ConfigManager::_configured = false; bool ConfigManager::_saveFails = false; +bool ConfigManager::_sensorMappingReadFails = false; // ── Sensor Address Mapping ── uint8_t ConfigManager::_sensorSolarAddr[8] = {0}; From bd689e95e1c950e3aa0e8fb8217a1e616baf7202 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:45:50 +0200 Subject: [PATCH 52/57] test: cover reviewed command failure paths --- .../tests/test_controller_command_handler.cpp | 85 ++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/test/native/tests/test_controller_command_handler.cpp b/test/native/tests/test_controller_command_handler.cpp index 174110ca..b690af31 100644 --- a/test/native/tests/test_controller_command_handler.cpp +++ b/test/native/tests/test_controller_command_handler.cpp @@ -246,6 +246,87 @@ static int test_loop_interval_updates_all_runtime_nodes() { return 0; } +static int test_sensor_mapping_read_failure_does_not_overwrite_existing_role() { + test_begin("ControllerCommandHandler", "sensor mapping update rejects transient cache read failure"); + HandlerFixture fixture; + + const std::uint8_t empty[8] = {0}; + const std::uint8_t existingPool[8] = {0x28, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7}; + const std::uint8_t newSolar[8] = {0x28, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7}; + PoolController::ConfigManager::seedSensorMappingForTest(empty, false, existingPool, true); + + ControllerCommand command{}; + command.type = ControllerCommandType::SET_SENSOR_MAPPING; + command.sensorRole = SensorRole::SOLAR; + std::memcpy(command.sensorAddress.data(), newSolar, 8); + + PoolController::ConfigManager::_sensorMappingReadFails = true; + const bool accepted = fixture.handler.handle(command); + PoolController::ConfigManager::_sensorMappingReadFails = false; + ASSERT_TRUE(!accepted); + + std::uint8_t savedSolar[8] = {0}; + std::uint8_t savedPool[8] = {0}; + ASSERT_TRUE(PoolController::ConfigManager::loadSensorMapping(savedSolar, savedPool)); + ASSERT_EQ(std::memcmp(savedSolar, empty, 8), 0); + ASSERT_EQ(std::memcmp(savedPool, existingPool, 8), 0); + return 0; +} + +static int test_loop_interval_persistence_failure_keeps_runtime_unchanged() { + test_begin("ControllerCommandHandler", "failed loop interval persistence leaves runtime intervals unchanged"); + HandlerFixture fixture; + auto &settings = PoolController::ConfigManager::getSettings(); + settings.loopInterval = 30; + fixture.solarTemperature.setMeasurementInterval(30); + fixture.poolTemperature.setMeasurementInterval(30); + fixture.controllerTemperature.setMeasurementInterval(30); + fixture.poolPump.setMeasurementInterval(30); + fixture.solarPump.setMeasurementInterval(30); + fixture.operation.setMeasurementInterval(30); + + ControllerCommand command{}; + command.type = ControllerCommandType::SET_LOOP_INTERVAL; + command.integerValue = 42; + PoolController::ConfigManager::_saveFails = true; + const bool accepted = fixture.handler.handle(command); + PoolController::ConfigManager::_saveFails = false; + + ASSERT_TRUE(!accepted); + ASSERT_EQ(settings.loopInterval, 30); + ASSERT_EQ(fixture.solarTemperature.getMeasurementInterval(), 30UL); + ASSERT_EQ(fixture.poolTemperature.getMeasurementInterval(), 30UL); + ASSERT_EQ(fixture.controllerTemperature.getMeasurementInterval(), 30UL); + ASSERT_EQ(fixture.poolPump.getMeasurementInterval(), 30UL); + ASSERT_EQ(fixture.solarPump.getMeasurementInterval(), 30UL); + ASSERT_EQ(fixture.operation.getMeasurementInterval(), 30UL); + return 0; +} + +static int test_time_loss_thresholds_preserve_ordering() { + test_begin("ControllerCommandHandler", "time-loss thresholds preserve 1 <= green < red <= 72"); + HandlerFixture fixture; + auto &settings = PoolController::ConfigManager::getSettings(); + settings.timeLossGreenHours = 2; + settings.timeLossRedHours = 24; + + ControllerCommand command{}; + command.type = ControllerCommandType::SET_TIME_LOSS_GREEN_HOURS; + command.integerValue = 24; + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_EQ(settings.timeLossGreenHours, 2); + + command.type = ControllerCommandType::SET_TIME_LOSS_RED_HOURS; + command.integerValue = 2; + ASSERT_TRUE(!fixture.handler.handle(command)); + ASSERT_EQ(settings.timeLossRedHours, 24); + + command.integerValue = 25; + ASSERT_TRUE(fixture.handler.handle(command)); + ASSERT_EQ(settings.timeLossRedHours, 25); + return 0; +} + static int test_factory_reset_owns_runtime_reset_mutation() { test_begin("ControllerCommandHandler", "factory reset delegates configuration reset through the handler"); HandlerFixture fixture; @@ -264,7 +345,9 @@ int run_controller_command_handler_tests() { int (*tests[])() = {test_mode_and_setpoint_are_applied_through_one_handler, test_persistence_failure_rolls_back_setpoints, test_green_time_loss_rejects_zero, test_timer_updates_are_independent, test_manual_pump_guard_is_application_policy, test_sensor_mapping_is_persisted_and_applied, test_setting_missing_sensor_role_preserves_existing_mapping, - test_loop_interval_updates_all_runtime_nodes, test_factory_reset_owns_runtime_reset_mutation}; + test_sensor_mapping_read_failure_does_not_overwrite_existing_role, test_loop_interval_updates_all_runtime_nodes, + test_loop_interval_persistence_failure_keeps_runtime_unchanged, test_time_loss_thresholds_preserve_ordering, + test_factory_reset_owns_runtime_reset_mutation}; for (auto test : tests) { if (test() == 0) { ++passed; From 6fecad08496bdcb57239751ca53b3e594ec24c3d Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:46:02 +0200 Subject: [PATCH 53/57] test: cover bounded and invalid Dallas snapshots --- .../test_controller_snapshot_publisher.cpp | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/native/tests/test_controller_snapshot_publisher.cpp b/test/native/tests/test_controller_snapshot_publisher.cpp index c6889cac..7b2dbe47 100644 --- a/test/native/tests/test_controller_snapshot_publisher.cpp +++ b/test/native/tests/test_controller_snapshot_publisher.cpp @@ -218,6 +218,44 @@ static int test_detected_inventory_is_published_and_deduplicated() { return 0; } +static int test_disconnected_detected_sensor_is_invalid() { + test_begin("ControllerSnapshotPublisher", "Dallas disconnect sentinel is not published as a valid temperature"); + SnapshotFixture fixture; + const std::uint8_t address[8] = {0x28, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7}; + fixture.solarTemperature.setDetectedDevice(0, address, DEVICE_DISCONNECTED_C); + + fixture.publisher.publish(); + SystemSnapshot snapshot{}; + ASSERT_TRUE(fixture.store.read(snapshot)); + ASSERT_EQ(snapshot.sensors.detectedCount, 1); + ASSERT_EQ(snapshot.sensors.detected[0].temperature.value, DEVICE_DISCONNECTED_C); + ASSERT_TRUE(!snapshot.sensors.detected[0].temperature.valid); + return 0; +} + +static int test_detected_inventory_refresh_is_bounded() { + test_begin("ControllerSnapshotPublisher", "detected Dallas inventory is not polled on every snapshot"); + SnapshotFixture fixture; + auto &settings = PoolController::ConfigManager::getSettings(); + settings.loopInterval = 60; + const std::uint8_t address[8] = {0x28, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7}; + fixture.solarTemperature.setDetectedDevice(0, address, 49.0F); + + resetMillisOffsetForTest(); + fixture.publisher.publish(); + const uint32_t readsAfterFirstPublish = fixture.solarTemperature.getDetectedTemperatureReadCount(); + ASSERT_TRUE(readsAfterFirstPublish > 0); + + fixture.publisher.publish(); + ASSERT_EQ(fixture.solarTemperature.getDetectedTemperatureReadCount(), readsAfterFirstPublish); + + advanceMillisForTest(60000); + fixture.publisher.publish(); + ASSERT_TRUE(fixture.solarTemperature.getDetectedTemperatureReadCount() > readsAfterFirstPublish); + resetMillisOffsetForTest(); + return 0; +} + static int test_snapshot_rule_projection_avoids_verbose_lookup() { test_begin("ControllerSnapshotPublisher", "snapshot rule projection does not use verbose rule lookup"); SnapshotFixture fixture; @@ -247,6 +285,7 @@ int run_controller_snapshot_publisher_tests() { int failed = 0; int (*tests[])() = {test_complete_runtime_state_is_projected, test_mapping_found_requires_selected_configured_address, test_ntp_server_is_projected, test_detected_inventory_is_published_and_deduplicated, + test_disconnected_detected_sensor_is_invalid, test_detected_inventory_refresh_is_bounded, test_snapshot_rule_projection_avoids_verbose_lookup, test_generation_advances_per_complete_publication}; for (auto test : tests) { if (test() == 0) { From d66ba48087a4dbc886d4fe891eadeda3d4f6b6db Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:46:09 +0200 Subject: [PATCH 54/57] docs: record reviewed adapter invariants --- openspec/changes/adapter-command-snapshot-migration/tasks.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openspec/changes/adapter-command-snapshot-migration/tasks.md b/openspec/changes/adapter-command-snapshot-migration/tasks.md index 51771ef7..0ea041c3 100644 --- a/openspec/changes/adapter-command-snapshot-migration/tasks.md +++ b/openspec/changes/adapter-command-snapshot-migration/tasks.md @@ -11,7 +11,10 @@ - [x] Populate sensor mapping address/configured/found state in the snapshot. - [x] Preserve independently configured sensor mappings when the other role is absent. - [x] Populate detected-sensor inventory, local IPv4 state and configured NTP server in the snapshot. +- [x] Bound detected-sensor inventory refreshes so snapshot publication does not poll Dallas devices on every loop. +- [x] Keep transient sensor-mapping NVS failures retryable and reject updates that cannot preserve the other role. - [x] Populate GREEN/YELLOW/RED time degradation and runtime controller settings in the snapshot. +- [x] Enforce the persisted/runtime degradation invariant `1 <= green < red <= 72`. - [x] Keep failed persisted runtime mutations atomic by rolling back owner state and settings. - [x] Keep snapshot publication free of active-rule debug-log side effects. - [ ] Migrate MQTT state/discovery reads to snapshot values where applicable. From 685069d99cf874c128d3713810d5ad29da859daa Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:49:18 +0200 Subject: [PATCH 55/57] fix: support ESP32 C++ standard in mapping read result --- src/ConfigManager.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ConfigManager.cpp b/src/ConfigManager.cpp index 4ed0d92b..96e79451 100644 --- a/src/ConfigManager.cpp +++ b/src/ConfigManager.cpp @@ -267,7 +267,10 @@ SensorMappingReadResult readSensorMappingFromNvs(uint8_t solarAddr[8], uint8_t p memset(poolAddr, 0, 8); } - return {true, solarLength == 8 || poolLength == 8}; + SensorMappingReadResult result; + result.readSucceeded = true; + result.hasMapping = solarLength == 8 || poolLength == 8; + return result; } } // namespace From 6c9ec67f291800e1b7743e9b50d3903cf71eb90e Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:49:20 +0200 Subject: [PATCH 56/57] style: format snapshot refresh interval --- src/ControllerSnapshotPublisher.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ControllerSnapshotPublisher.cpp b/src/ControllerSnapshotPublisher.cpp index bdfa57d6..448dfef6 100644 --- a/src/ControllerSnapshotPublisher.cpp +++ b/src/ControllerSnapshotPublisher.cpp @@ -132,8 +132,9 @@ void ControllerSnapshotPublisher::publish() { ConfigManager::loadSensorMappingCached(solarAddress.data(), poolAddress.data()); snapshot.sensors.solarMapping = mappingSnapshot(solarAddress, dependencies_.solarTemperature); snapshot.sensors.poolMapping = mappingSnapshot(poolAddress, dependencies_.poolTemperature); - const std::uint32_t detectedRefreshSeconds = - settings.loopInterval > 0 ? static_cast(settings.loopInterval) : 1U; + const std::uint32_t detectedRefreshSeconds = settings.loopInterval > 0 + ? static_cast(settings.loopInterval) + : 1U; const std::uint32_t detectedRefreshIntervalMs = detectedRefreshSeconds * 1000U; if (!detectedInventoryInitialized_ || nowMs - lastDetectedInventoryRefreshMs_ >= detectedRefreshIntervalMs) { SensorSnapshot refreshedInventory{}; From fc1aaa211bf2ee28752d023fd7e911ec6137f08c Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Mon, 12 Oct 2026 07:49:23 +0200 Subject: [PATCH 57/57] test: use comparison assertion for inventory reads --- test/native/tests/test_controller_snapshot_publisher.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/native/tests/test_controller_snapshot_publisher.cpp b/test/native/tests/test_controller_snapshot_publisher.cpp index 7b2dbe47..51796b18 100644 --- a/test/native/tests/test_controller_snapshot_publisher.cpp +++ b/test/native/tests/test_controller_snapshot_publisher.cpp @@ -29,6 +29,7 @@ extern void test_suite_end(const char *name, int passed, int failed); } while (0) #define ASSERT_EQ(a, b) ASSERT_TRUE((a) == (b)) +#define ASSERT_GT(a, b) ASSERT_TRUE((a) > (b)) using PoolController::ControllerSnapshotPublisher; using PoolController::ControllerSnapshotStore; @@ -244,14 +245,14 @@ static int test_detected_inventory_refresh_is_bounded() { resetMillisOffsetForTest(); fixture.publisher.publish(); const uint32_t readsAfterFirstPublish = fixture.solarTemperature.getDetectedTemperatureReadCount(); - ASSERT_TRUE(readsAfterFirstPublish > 0); + ASSERT_GT(readsAfterFirstPublish, 0U); fixture.publisher.publish(); ASSERT_EQ(fixture.solarTemperature.getDetectedTemperatureReadCount(), readsAfterFirstPublish); advanceMillisForTest(60000); fixture.publisher.publish(); - ASSERT_TRUE(fixture.solarTemperature.getDetectedTemperatureReadCount() > readsAfterFirstPublish); + ASSERT_GT(fixture.solarTemperature.getDetectedTemperatureReadCount(), readsAfterFirstPublish); resetMillisOffsetForTest(); return 0; }