From 186937efb7cd9e940b844c83ea6fbc49b5f4a22e Mon Sep 17 00:00:00 2001 From: vibe-bot Date: Sat, 10 Oct 2026 21:02:26 +0000 Subject: [PATCH] refactor(application): add coherent controller snapshot store Co-authored-by: stritti --- .../tasks.md | 1 + src/ControllerSnapshotStore.hpp | 71 ++++++++++++ test/native/CMakeLists.txt | 1 + .../tests/test_controller_snapshot_store.cpp | 102 ++++++++++++++++++ test/native/tests/test_main.cpp | 2 + 5 files changed, 177 insertions(+) create mode 100644 src/ControllerSnapshotStore.hpp create mode 100644 test/native/tests/test_controller_snapshot_store.cpp diff --git a/openspec/changes/controller-architecture-contracts/tasks.md b/openspec/changes/controller-architecture-contracts/tasks.md index 25b10740..3d4e209d 100644 --- a/openspec/changes/controller-architecture-contracts/tasks.md +++ b/openspec/changes/controller-architecture-contracts/tasks.md @@ -22,6 +22,7 @@ - [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] 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. - [ ] In #218, validate Web/legacy NTP input without silent truncation and complete owner/adapter regressions. - [ ] In #218/#220, verify both-bus inventory aggregation, deduplication, overflow and rescan replacement on the owning task. diff --git a/src/ControllerSnapshotStore.hpp b/src/ControllerSnapshotStore.hpp new file mode 100644 index 00000000..d36e8617 --- /dev/null +++ b/src/ControllerSnapshotStore.hpp @@ -0,0 +1,71 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +/** + * @file ControllerSnapshotStore.hpp + * @brief Consistent publication of immutable controller read models. + */ + +#pragma once + +#include "ControllerSnapshot.hpp" + +#if defined(ESP32) || defined(ARDUINO_ARCH_ESP32) +#include +#include +#else +#include +#endif + +namespace PoolController { + +/** + * @brief Small synchronized store for complete `SystemSnapshot` values. + * + * The application owner publishes one complete snapshot after updating state. + * Adapters copy the latest snapshot under a short critical section and never + * dereference mutable controller nodes directly. + */ +class ControllerSnapshotStore final { +public: + void publish(const SystemSnapshot &snapshot) { + lock(); + snapshot_ = snapshot; + hasSnapshot_ = true; + unlock(); + } + + bool read(SystemSnapshot &out) const { + lock(); + if (!hasSnapshot_) { + unlock(); + return false; + } + out = snapshot_; + unlock(); + return true; + } + + bool hasSnapshot() const { + lock(); + const bool result = hasSnapshot_; + unlock(); + return result; + } + +private: +#if defined(ESP32) || defined(ARDUINO_ARCH_ESP32) + mutable portMUX_TYPE mux_ = portMUX_INITIALIZER_UNLOCKED; + void lock() const { portENTER_CRITICAL(&mux_); } + void unlock() const { portEXIT_CRITICAL(&mux_); } +#else + mutable std::mutex mutex_; + void lock() const { mutex_.lock(); } + void unlock() const { mutex_.unlock(); } +#endif + + SystemSnapshot snapshot_{}; + bool hasSnapshot_{false}; +}; + +} // namespace PoolController diff --git a/test/native/CMakeLists.txt b/test/native/CMakeLists.txt index c02f444e..35acb612 100644 --- a/test/native/CMakeLists.txt +++ b/test/native/CMakeLists.txt @@ -91,6 +91,7 @@ set(TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_mqtt_command_queue.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_controller_contracts.cpp ${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 ) diff --git a/test/native/tests/test_controller_snapshot_store.cpp b/test/native/tests/test_controller_snapshot_store.cpp new file mode 100644 index 00000000..864d1eb4 --- /dev/null +++ b/test/native/tests/test_controller_snapshot_store.cpp @@ -0,0 +1,102 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// SPDX-License-Identifier: MIT + +/** + * @file test_controller_snapshot_store.cpp + * @brief Tests for coherent publication of controller read models. + */ + +#include "ControllerSnapshotStore.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::ControllerSnapshotStore; +using PoolController::OperationMode; +using PoolController::SystemSnapshot; + +static int test_empty_store() { + test_begin("ControllerSnapshotStore", "read fails until first complete snapshot is published"); + ControllerSnapshotStore store; + SystemSnapshot snapshot{}; + ASSERT_TRUE(!store.hasSnapshot()); + ASSERT_TRUE(!store.read(snapshot)); + return 0; +} + +static int test_complete_snapshot_copy() { + test_begin("ControllerSnapshotStore", "read returns one complete published snapshot"); + ControllerSnapshotStore store; + SystemSnapshot published{}; + published.sensors.generation = 7; + published.sensors.pool.value = 27.25F; + published.sensors.pool.valid = true; + published.mode = OperationMode::BOOST; + published.poolPumpOn = true; + published.solarPumpOn = true; + published.freeHeapBytes = 123456; + + store.publish(published); + + SystemSnapshot read{}; + ASSERT_TRUE(store.hasSnapshot()); + ASSERT_TRUE(store.read(read)); + ASSERT_EQ(read.sensors.generation, 7U); + ASSERT_EQ(read.sensors.pool.value, 27.25F); + ASSERT_TRUE(read.sensors.pool.valid); + ASSERT_EQ(read.mode, OperationMode::BOOST); + ASSERT_TRUE(read.poolPumpOn && read.solarPumpOn); + ASSERT_EQ(read.freeHeapBytes, 123456U); + return 0; +} + +static int test_new_publish_replaces_whole_snapshot() { + test_begin("ControllerSnapshotStore", "new publication atomically replaces the previous value object"); + ControllerSnapshotStore store; + SystemSnapshot first{}; + first.sensors.generation = 1; + first.poolPumpOn = true; + first.solarPumpOn = false; + store.publish(first); + + SystemSnapshot second{}; + second.sensors.generation = 2; + second.poolPumpOn = false; + second.solarPumpOn = true; + store.publish(second); + + SystemSnapshot read{}; + ASSERT_TRUE(store.read(read)); + ASSERT_EQ(read.sensors.generation, 2U); + ASSERT_TRUE(!read.poolPumpOn); + ASSERT_TRUE(read.solarPumpOn); + return 0; +} + +int run_controller_snapshot_store_tests() { + int passed = 0; + int failed = 0; + int (*tests[])() = {test_empty_store, test_complete_snapshot_copy, test_new_publish_replaces_whole_snapshot}; + for (auto test : tests) { + if (test() == 0) { + ++passed; + } else { + ++failed; + } + } + test_suite_end("ControllerSnapshotStore", passed, failed); + return failed; +} diff --git a/test/native/tests/test_main.cpp b/test/native/tests/test_main.cpp index ce3a61b8..8a47795c 100644 --- a/test/native/tests/test_main.cpp +++ b/test/native/tests/test_main.cpp @@ -114,6 +114,7 @@ extern int run_calibration_manager_tests(); extern int run_mqtt_command_queue_tests(); 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(); int main() { @@ -137,6 +138,7 @@ int main() { total += run_mqtt_command_queue_tests(); total += run_controller_contract_tests(); total += run_controller_snapshot_diagnostics_tests(); + total += run_controller_snapshot_store_tests(); total += run_controller_command_queue_tests(); printf("\n══════════════════════════════════════════════════\n");