From a123ddbbad554f16289afd23f48c08dd830f7dd9 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 5 Jul 2026 22:07:14 +0200 Subject: [PATCH 1/2] =?UTF-8?q?refactor:=20structural=20Clean=20Code=20imp?= =?UTF-8?q?rovements=20=E2=80=94=20setup=20decomposition,=20dispatch=20tab?= =?UTF-8?q?le,=20OTA=20split,=20utility=20extraction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Decompose PoolMonitorContext::setup() (133 lines) into focused phases: printBootBanner, initSystem, updateBootAndUptimeStats, handleBootLoop, runNetworkCycle, runOfflineCycle - Replace handleMqttMessage if-else chain with data-driven dispatch table (TopicHandler array + range-for loop) - Split OtaUpdater::downloadAndApply() (100 lines) into openFirmwareDownload() + streamToUpdate() helper functions - Extract equalsIgnoreCaseAscii and parseHomeAssistantBoolState into dedicated MqttUtils.hpp/.cpp (separation of concerns) - Replace while(1) hard-lock in initSystem() with proper NVS cleanup + ESP.restart() - Make kOtaBufferSize public for external access - Zero functional changes — verified by build + static analysis --- src/PoolMonitor/MqttUtils.cpp | 50 +++++ src/PoolMonitor/MqttUtils.hpp | 28 +++ src/PoolMonitor/OtaUpdater.cpp | 78 ++++--- src/PoolMonitor/OtaUpdater.hpp | 5 +- src/PoolMonitor/PoolMonitorContext.cpp | 273 ++++++++++++------------- src/PoolMonitor/PoolMonitorContext.hpp | 47 ++--- 6 files changed, 274 insertions(+), 207 deletions(-) create mode 100644 src/PoolMonitor/MqttUtils.cpp create mode 100644 src/PoolMonitor/MqttUtils.hpp diff --git a/src/PoolMonitor/MqttUtils.cpp b/src/PoolMonitor/MqttUtils.cpp new file mode 100644 index 0000000..3e189a4 --- /dev/null +++ b/src/PoolMonitor/MqttUtils.cpp @@ -0,0 +1,50 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// +// SPDX-License-Identifier: MIT + +/** + * @file MqttUtils.cpp + * @brief Implementation of MQTT payload parsing helpers. + */ + +#include "MqttUtils.hpp" + +#include +#include + +namespace PoolMonitor { + +bool equalsIgnoreCaseAscii(const char* lhs, const char* rhs) { + if (lhs == nullptr || rhs == nullptr) { + return false; + } + while (*lhs != '\0' && *rhs != '\0') { + if (std::tolower(static_cast(*lhs)) != + std::tolower(static_cast(*rhs))) { + return false; + } + lhs++; + rhs++; + } + return *lhs == *rhs; +} + +bool parseHomeAssistantBoolState(const char* value) { + if (value == nullptr) { + return false; + } + if (equalsIgnoreCaseAscii(value, "true") + || equalsIgnoreCaseAscii(value, "on") + || equalsIgnoreCaseAscii(value, "1")) { + return true; + } + if (equalsIgnoreCaseAscii(value, "false") + || equalsIgnoreCaseAscii(value, "off") + || equalsIgnoreCaseAscii(value, "0")) { + return false; + } + printf("⚠️\tUnexpected boolean MQTT payload: %s (defaulting to false)\n", value); + return false; +} + +} // namespace PoolMonitor diff --git a/src/PoolMonitor/MqttUtils.hpp b/src/PoolMonitor/MqttUtils.hpp new file mode 100644 index 0000000..b3f0a9f --- /dev/null +++ b/src/PoolMonitor/MqttUtils.hpp @@ -0,0 +1,28 @@ +// Copyright (c) 2018-2026 Smart Swimming Pool, Stephan Strittmatter +// +// SPDX-License-Identifier: MIT + +/** + * @file MqttUtils.hpp + * @brief Utility helpers for MQTT payload parsing in the Pool Monitor. + */ + +#pragma once + +namespace PoolMonitor { + +/** + * @brief Case-insensitive ASCII string comparison. + * @return true when both strings are equal (ignoring ASCII case). + */ +bool equalsIgnoreCaseAscii(const char* lhs, const char* rhs); + +/** + * @brief Parse boolean MQTT state payloads used by Home Assistant topics. + * + * Accepts "true"/"false" (case-insensitive), "on"/"off", "1"/"0". + * Logs a warning for unexpected values and defaults to false. + */ +bool parseHomeAssistantBoolState(const char* value); + +} // namespace PoolMonitor diff --git a/src/PoolMonitor/OtaUpdater.cpp b/src/PoolMonitor/OtaUpdater.cpp index 6187be5..ac4d9c2 100644 --- a/src/PoolMonitor/OtaUpdater.cpp +++ b/src/PoolMonitor/OtaUpdater.cpp @@ -341,14 +341,22 @@ bool OtaUpdater::isNewerVersion(const String ¤t, const String &latest) { return false; } -// ── OTA Download + Flash ── +// ── OTA: download response into Update stream ── -bool OtaUpdater::downloadAndApply(const String &url) { +/** + * @brief Perform HTTP GET on the firmware URL and validate the response. + * @param[in] url Firmware download URL. + * @param[out] http HTTPClient to reuse (already connected). + * @param[out] totalSize Expected download size (from Content-Length). + * @param[out] stream WiFiClient stream pointer for reading the body. + * @return true if the response is valid (HTTP 200, non-empty content). + */ +static bool openFirmwareDownload(const String &url, HTTPClient &http, + int &totalSize, WiFiClient *&stream) { WiFiClientSecure client; client.setCACert(kGitHubRootCA); client.setTimeout(10000); - HTTPClient http; http.begin(client, url); http.setUserAgent("PoolMonitor/1.0"); http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); @@ -356,39 +364,43 @@ bool OtaUpdater::downloadAndApply(const String &url) { int httpCode = http.GET(); if (httpCode != 200) { Serial.printf("🛑\tOTA: Download returned HTTP %d\n", httpCode); - http.end(); return false; } - int totalSize = http.getSize(); + totalSize = http.getSize(); if (totalSize <= 0) { Serial.println("🛑\tOTA: Invalid content size"); - http.end(); return false; } - Serial.printf("⬇️\tOTA: Download size: %d bytes\n", totalSize); + stream = http.getStreamPtr(); + return true; +} + +/** + * @brief Stream firmware data from WiFiClient into the Update object. + * @param stream WiFiClient with firmware body ready to read. + * @param totalSize Expected total download size. + * @param[out] progress Optional progress pointer (0-100 percentage, can be nullptr). + * @return Number of bytes successfully written to Update, or 0 on failure. + */ +static int streamToUpdate(WiFiClient *stream, int totalSize, int *progress) { if (!Update.begin(totalSize)) { Serial.printf("🛑\tOTA: Update.begin() failed: %s\n", Update.errorString()); - http.end(); - return false; + return 0; } - // Stream download in chunks with stall protection - WiFiClient *stream = http.getStreamPtr(); - uint8_t buffer[kOtaBufferSize]; + uint8_t buffer[OtaUpdater::kOtaBufferSize]; int totalRead = 0; unsigned long lastProgressMs = millis(); - while (http.connected() && totalRead < totalSize) { - // Abort if no progress for kDownloadTimeoutMs (stalled connection) - if (millis() - lastProgressMs > kDownloadTimeoutMs) { + while (stream->connected() && totalRead < totalSize) { + if (millis() - lastProgressMs > OtaUpdater::kDownloadTimeoutMs) { Serial.printf("🛑\tOTA: Download stalled for %lu ms, aborting (%d/%d)\n", - kDownloadTimeoutMs, totalRead, totalSize); + OtaUpdater::kDownloadTimeoutMs, totalRead, totalSize); Update.end(false); - http.end(); - return false; + return 0; } size_t available = stream->available(); @@ -407,21 +419,36 @@ bool OtaUpdater::downloadAndApply(const String &url) { if (written != read) { Serial.printf("🛑\tOTA: Write error at byte %d: %s\n", totalRead, Update.errorString()); Update.end(false); - http.end(); - return false; + return 0; } totalRead += read; lastProgressMs = millis(); - progress_ = (totalRead * 100) / totalSize; - Serial.printf("⬇️\tOTA: %d%% (%d/%d)\n", progress_, totalRead, totalSize); + if (progress) { + *progress = (totalRead * 100) / totalSize; + } + Serial.printf("⬇️\tOTA: %d%% (%d/%d)\n", + progress ? *progress : 0, totalRead, totalSize); + } + + return totalRead; +} + +bool OtaUpdater::downloadAndApply(const String &url) { + HTTPClient http; + int totalSize = 0; + WiFiClient *stream = nullptr; + + if (!openFirmwareDownload(url, http, totalSize, stream)) { + http.end(); + return false; } + int totalRead = streamToUpdate(stream, totalSize, &progress_); http.end(); if (totalRead != totalSize) { Serial.printf("🛑\tOTA: Incomplete download (%d / %d)\n", totalRead, totalSize); - Update.end(false); return false; } @@ -432,9 +459,6 @@ bool OtaUpdater::downloadAndApply(const String &url) { Serial.println("✅\tOTA: Update successful! Rebooting..."); - // Close Preferences so NVS writes from this wake cycle are finalized - // before restart — matches the AGENTS.md rule to always call - // preferences.end() before ESP.restart() or deep sleep. if (prefs_ != nullptr) { prefs_->end(); } @@ -442,7 +466,7 @@ bool OtaUpdater::downloadAndApply(const String &url) { Serial.flush(); delay(1000); ESP.restart(); - return true; // Never actually reached + return true; } } // namespace PoolMonitor diff --git a/src/PoolMonitor/OtaUpdater.hpp b/src/PoolMonitor/OtaUpdater.hpp index 64a0712..81518b1 100644 --- a/src/PoolMonitor/OtaUpdater.hpp +++ b/src/PoolMonitor/OtaUpdater.hpp @@ -88,6 +88,9 @@ class OtaUpdater { /// Max milliseconds without progress before aborting a stalled download. static constexpr unsigned long kDownloadTimeoutMs = 30000UL; + /// Buffer size for OTA download chunks. + static constexpr int kOtaBufferSize = 4096; + private: // ── GitHub API ── static bool fetchLatestRelease(); @@ -111,8 +114,6 @@ class OtaUpdater { static bool updateAvailable_; static bool updateInProgress_; static int progress_; - - static constexpr int kOtaBufferSize = 4096; }; } // namespace PoolMonitor diff --git a/src/PoolMonitor/PoolMonitorContext.cpp b/src/PoolMonitor/PoolMonitorContext.cpp index 03e9312..bc1fdb5 100644 --- a/src/PoolMonitor/PoolMonitorContext.cpp +++ b/src/PoolMonitor/PoolMonitorContext.cpp @@ -22,6 +22,7 @@ #include "DisplayManager.hpp" #include "OtaUpdater.hpp" #include "TimeClientHelper.hpp" +#include "MqttUtils.hpp" #include "../Version.h" namespace PoolMonitor { @@ -59,51 +60,61 @@ PoolMonitorContext::~PoolMonitorContext() { } auto PoolMonitorContext::setup() -> void { + printBootBanner(); + initSystem(); + + unsigned long totalUptime = updateBootAndUptimeStats(); + PoolMonitor::beginTimeClient(); + + handleBootLoop(totalUptime); // deep sleeps inside if boot loop, returns otherwise + + if (isNetworkCycle()) { + runNetworkCycle(totalUptime); + } else { + runOfflineCycle(totalUptime); + } +} + +auto PoolMonitorContext::printBootBanner() -> void { Serial.println(F(" ------------------------------------- ")); Serial.println(F("| Pool Monitor |")); Serial.println(F("| www.smart-swimmingpool.com |")); Serial.println(F(" ------------------------------------- ")); Serial.printf("📦\tFW Version: %s\n", FW_VERSION); Serial.printf("📦\tGitHub Repo: %s\n", GITHUB_REPO); +} - // Initialize system monitor and check for boot loops +auto PoolMonitorContext::initSystem() -> void { SystemMonitor::begin(); bootLoopDetected_ = SystemMonitor::detectBootLoop(); - // Initialize preferences preferences_ = new Preferences(); if (!preferences_->begin("pool-monitor", false)) { Serial.println("🛑\tFailed to open preferences"); - while (1) { - delay(1000); - } + preferences_->end(); + delete preferences_; + preferences_ = nullptr; + ESP.restart(); } - // Register with SystemMonitor for safe NVS shutdown before restart SystemMonitor::setPreferences(preferences_); - - // Initialize OTA updater OtaUpdater::begin(*preferences_); +} - // Track boot count - unsigned int boot_count = preferences_->getUInt("boot_count", 0); - Serial.printf("Current boot count: %u\n", ++boot_count); - preferences_->putUInt("boot_count", boot_count); +auto PoolMonitorContext::updateBootAndUptimeStats() -> unsigned long { + unsigned int bootCount = preferences_->getUInt("boot_count", 0); + Serial.printf("Current boot count: %u\n", ++bootCount); + preferences_->putUInt("boot_count", bootCount); - // Track cumulative uptime across sleep cycles - unsigned long total_uptime = preferences_->getULong("total_uptime", 0); - // Use actual sleep duration from last cycle (supports night-mode 4h sleeps) + unsigned long totalUptime = preferences_->getULong("total_uptime", 0); uint32_t lastSleepDuration = preferences_->getUInt("last_sleep_sec", TIME_TO_SLEEP_SECONDS); - total_uptime += lastSleepDuration; + totalUptime += lastSleepDuration; preferences_->remove("last_sleep_sec"); - preferences_->putULong("total_uptime", total_uptime); + preferences_->putULong("total_uptime", totalUptime); Serial.printf("Total uptime: %lu seconds (%.1f hours, last sleep: %u s)\n", - total_uptime, total_uptime / 3600.0, lastSleepDuration); + totalUptime, totalUptime / 3600.0, lastSleepDuration); - // ── Power-save: WiFi/MQTT only every (SKIP_WIFI_WAKE_CYCLES + 1) wake-ups ── uint32_t cyclesWithoutWiFi = preferences_->getUInt("no_wifi_count", 0); - - // Force network on first boot (no MQTT config exists yet → portal needed) bool hasConfig = (preferences_->getString("mqtt_server", "").length() > 0); bool doNetwork = !hasConfig || (cyclesWithoutWiFi >= SKIP_WIFI_WAKE_CYCLES); @@ -117,80 +128,71 @@ auto PoolMonitorContext::setup() -> void { doNetwork ? 0 : cyclesWithoutWiFi + 1, SKIP_WIFI_WAKE_CYCLES); - // Initialize NTP time client - PoolMonitor::beginTimeClient(); + return totalUptime; +} - // ── Boot-loop safe mode ── - if (bootLoopDetected_) { - Serial.println("⚠️\tBoot loop detected! Entering safe mode — skipping network, using cached data"); - // Reset counter so next wake tries normal boot instead of staying safe forever - SystemMonitor::clearBootLoopCounter(); - initializeDisplay(); - loadState(); - reconstructTime(total_uptime); - DisplayManager::updateDisplay(poolTemp_, solarTemp_, poolPumpOn_, solarPumpOn_, - poolMode_.c_str(), lastUpdate_.c_str()); - prepareForSleep(); // deep sleep, does not return - } +auto PoolMonitorContext::handleBootLoop(unsigned long totalUptime) -> void { + if (!bootLoopDetected_) return; - // ── On network cycles: full display init + network + MQTT + display update ── - // ── On no-network cycles: E-Ink retains its image, skip all display ops ── - if (doNetwork) { - initializeDisplay(); - initializeNetwork(); - initializeMqtt(); + Serial.println("⚠️\tBoot loop detected! Entering safe mode — skipping network, using cached data"); + SystemMonitor::clearBootLoopCounter(); + initializeDisplay(); + loadState(); + reconstructTime(totalUptime); + DisplayManager::updateDisplay(poolTemp_, solarTemp_, poolPumpOn_, solarPumpOn_, + poolMode_.c_str(), lastUpdate_.c_str()); + prepareForSleep(); +} - // Boot successful — reached after WiFi + MQTT without crash - SystemMonitor::clearBootLoopCounter(); +auto PoolMonitorContext::isNetworkCycle() -> bool { + return preferences_ == nullptr || preferences_->getUInt("no_wifi_count", 0) == 0; +} - // Load fresh data (MQTT callbacks may have updated NVS values) - loadState(); +auto PoolMonitorContext::runNetworkCycle(unsigned long totalUptime) -> void { + initializeDisplay(); + initializeNetwork(); + initializeMqtt(); - // NTP sync - if (isNtpSyncNeeded()) { - Serial.println("⏰\tNTP sync needed - updating time from server"); - lastUpdate_ = PoolMonitor::getCurrentTime(); - preferences_->putString("last_update", lastUpdate_); - preferences_->putULong("last_ntp_sync", total_uptime); + SystemMonitor::clearBootLoopCounter(); + loadState(); - unsigned long synced_epoch = PoolMonitor::timeClient.getEpochTime(); - preferences_->putULong("last_epoch", synced_epoch); + if (isNtpSyncNeeded()) { + Serial.println("⏰\tNTP sync needed - updating time from server"); + lastUpdate_ = PoolMonitor::getCurrentTime(); + preferences_->putString("last_update", lastUpdate_); + preferences_->putULong("last_ntp_sync", totalUptime); - Serial.printf("⏰\tNTP synced successfully at %s (next sync in ~%d seconds)\n", - lastUpdate_.c_str(), NTP_SYNC_INTERVAL_SECONDS); - } else { - reconstructTime(total_uptime); - } + unsigned long syncedEpoch = PoolMonitor::timeClient.getEpochTime(); + preferences_->putULong("last_epoch", syncedEpoch); - // Safety net: process any retained MQTT messages that arrived after polling - NetworkManager::loop(); - // Reload state in case late messages updated preferences - loadState(); + Serial.printf("⏰\tNTP synced successfully at %s (next sync in ~%d seconds)\n", + lastUpdate_.c_str(), NTP_SYNC_INTERVAL_SECONDS); + } else { + reconstructTime(totalUptime); + } - // Always update display on network cycles (data was potentially refreshed) - Serial.println("🖥️\tUpdating display"); - DisplayManager::updateDisplay(poolTemp_, solarTemp_, poolPumpOn_, solarPumpOn_, - poolMode_.c_str(), lastUpdate_.c_str()); + NetworkManager::loop(); + loadState(); - // Remember displayed values for next comparison - preferences_->putFloat("last_display_pool", poolTemp_); - preferences_->putFloat("last_display_solar", solarTemp_); - preferences_->putBool("last_display_ppump", poolPumpOn_); - preferences_->putBool("last_display_spump", solarPumpOn_); - preferences_->putString("last_display_mode", poolMode_); + Serial.println("🖥️\tUpdating display"); + DisplayManager::updateDisplay(poolTemp_, solarTemp_, poolPumpOn_, solarPumpOn_, + poolMode_.c_str(), lastUpdate_.c_str()); - } else { - // No-network cycle: E-Ink retains image, skip all display operations - // Only reconstruct time and load cached state for bookkeeping - loadState(); - reconstructTime(total_uptime); + preferences_->putFloat("last_display_pool", poolTemp_); + preferences_->putFloat("last_display_solar", solarTemp_); + preferences_->putBool("last_display_ppump", poolPumpOn_); + preferences_->putBool("last_display_spump", solarPumpOn_); + preferences_->putString("last_display_mode", poolMode_); +} - // Load MQTT settings from preferences (for status display next cycle) - mqtt_server = preferences_->getString("mqtt_server", ""); - mqtt_server_port = preferences_->getUInt("mqtt_port", 1883); +auto PoolMonitorContext::runOfflineCycle(unsigned long totalUptime) -> void { + loadState(); + reconstructTime(totalUptime); - Serial.println("🖥️\tNo network — E-Ink retains image, display skipped (saving power)"); - } + mqtt_server = preferences_->getString("mqtt_server", ""); + mqtt_server_port = preferences_->getUInt("mqtt_port", 1883); + + Serial.println("🖥️\tNo network — E-Ink retains image, display skipped (saving power)"); } auto PoolMonitorContext::loop() -> void { @@ -559,43 +561,45 @@ auto PoolMonitorContext::saveState() -> void { Serial.println("💾\tState saved"); } -// Case-insensitive ASCII string comparison -static bool equalsIgnoreCaseAscii(const char* lhs, const char* rhs) { - if (lhs == nullptr || rhs == nullptr) { - return false; - } - while (*lhs != '\0' && *rhs != '\0') { - if (std::tolower(static_cast(*lhs)) != - std::tolower(static_cast(*rhs))) { - return false; - } - lhs++; - rhs++; - } - return *lhs == *rhs; -} - -// Parse boolean MQTT state payloads used by Home Assistant topics -static bool parseHomeAssistantBoolState(const char* value) { - if (value == nullptr) { - return false; - } - if (equalsIgnoreCaseAscii(value, "true") - || equalsIgnoreCaseAscii(value, "on") - || equalsIgnoreCaseAscii(value, "1")) { - return true; - } - if (equalsIgnoreCaseAscii(value, "false") - || equalsIgnoreCaseAscii(value, "off") - || equalsIgnoreCaseAscii(value, "0")) { - return false; - } - Serial.printf("⚠️\tUnexpected boolean MQTT payload: %s (defaulting to false)\n", value); - return false; -} +// ── MQTT dispatch table ── + +using MqttHandler = void (*)(const char* payload); + +struct TopicHandler { + const char* topic; + const char* label; + MqttHandler handler; +}; + +static const TopicHandler kTopicHandlers[] = { + { kHaTopicPoolTemp, "Pool temperature", + [](const char* payload) { + poolTemp_ = String(payload).toFloat(); + preferences_->putFloat("pool_temp", poolTemp_); + } }, + { kHaTopicSolarTemp, "Solar temperature", + [](const char* payload) { + solarTemp_ = String(payload).toFloat(); + preferences_->putFloat("solar_temp", solarTemp_); + } }, + { kHaTopicPoolPump, "Pool pump", + [](const char* payload) { + poolPumpOn_ = parseHomeAssistantBoolState(payload); + preferences_->putBool("pump_pool", poolPumpOn_); + } }, + { kHaTopicSolarPump, "Solar pump", + [](const char* payload) { + solarPumpOn_ = parseHomeAssistantBoolState(payload); + preferences_->putBool("pump_solar", solarPumpOn_); + } }, + { kHaTopicMode, "Operation Mode", + [](const char* payload) { + poolMode_ = String(payload); + preferences_->putString("pool_mode", poolMode_); + } }, +}; void PoolMonitorContext::handleMqttMessage(char* topic, byte* payload, unsigned int length) { - // Stack-allocated buffer instead of heap allocation char payloadCopy[MQTT_PAYLOAD_BUFFER_SIZE]; size_t payloadLength = length; if (payloadLength >= sizeof(payloadCopy)) { @@ -606,34 +610,15 @@ void PoolMonitorContext::handleMqttMessage(char* topic, byte* payload, unsigned memcpy(payloadCopy, payload, payloadLength); payloadCopy[payloadLength] = '\0'; - String payloadString = String(payloadCopy); - - // Match Home Assistant state topics directly - if (strcmp(topic, kHaTopicPoolTemp) == 0) { - Serial.println("\tPool temperature: " + payloadString); - poolTemp_ = payloadString.toFloat(); - preferences_->putFloat("pool_temp", poolTemp_); - - } else if (strcmp(topic, kHaTopicSolarTemp) == 0) { - Serial.println("\tSolar temperature: " + payloadString); - solarTemp_ = payloadString.toFloat(); - preferences_->putFloat("solar_temp", solarTemp_); - - } else if (strcmp(topic, kHaTopicPoolPump) == 0) { - Serial.println("\tPool pump: " + payloadString); - poolPumpOn_ = parseHomeAssistantBoolState(payloadCopy); - preferences_->putBool("pump_pool", poolPumpOn_); - - } else if (strcmp(topic, kHaTopicSolarPump) == 0) { - Serial.println("\tSolar pump: " + payloadString); - solarPumpOn_ = parseHomeAssistantBoolState(payloadCopy); - preferences_->putBool("pump_solar", solarPumpOn_); - - } else if (strcmp(topic, kHaTopicMode) == 0) { - Serial.println("\tOperation Mode: " + payloadString); - poolMode_ = payloadString; - preferences_->putString("pool_mode", poolMode_); + for (const auto& entry : kTopicHandlers) { + if (strcmp(topic, entry.topic) == 0) { + Serial.println(String("\t") + entry.label + ": " + String(payloadCopy)); + entry.handler(payloadCopy); + return; + } } + + Serial.printf("⚠️\tUnknown MQTT topic: %s\n", topic); } auto PoolMonitorContext::isMqttConnected() -> bool { diff --git a/src/PoolMonitor/PoolMonitorContext.hpp b/src/PoolMonitor/PoolMonitorContext.hpp index aa623b7..ceacfff 100644 --- a/src/PoolMonitor/PoolMonitorContext.hpp +++ b/src/PoolMonitor/PoolMonitorContext.hpp @@ -71,55 +71,34 @@ struct PoolMonitorContext final { static auto isNtpSyncNeeded() -> bool; private: - /** - * @brief Initialize display and show initial screen. - */ - auto initializeDisplay() -> void; + // ── Setup lifecycle (phases of setup()) ── - /** - * @brief Initialize network connections. - */ + static auto printBootBanner() -> void; + auto initSystem() -> void; + auto updateBootAndUptimeStats() -> unsigned long; + auto handleBootLoop(unsigned long totalUptime) -> void; + auto isNetworkCycle() -> bool; + auto runNetworkCycle(unsigned long totalUptime) -> void; + auto runOfflineCycle(unsigned long totalUptime) -> void; + + // ── Subsystem helpers ── + + auto initializeDisplay() -> void; auto initializeNetwork() -> void; + auto initializeMqtt() -> void; - /** - * @brief Show setup screen with QR code for WiFi configuration. - */ static auto showSetupScreen() -> void; - - /** - * @brief Show screen when WiFi connection is successful. - */ static auto showWiFiConnectedScreen() -> void; - - /** - * @brief Show screen when WiFi connection fails. - */ static auto showWiFiConnectionFailedScreen() -> void; - /** - * @brief Initialize MQTT subscriptions. - */ - auto initializeMqtt() -> void; - - /** - * @brief Load saved state from Preferences. - */ auto loadState() -> void; - - /** - * @brief Save current state to Preferences. - */ auto saveState() -> void; - /** - * @brief Handle incoming MQTT messages. - */ static auto handleMqttMessage(char* topic, byte* payload, unsigned int length) -> void; bool bootLoopDetected_ = false; bool stateLoaded_ = false; - // MQTT settings (static for access from callbacks) static String mqtt_server; static uint16_t mqtt_server_port; }; From c6c3bc4eab7e3e3d28f15c69b2f64a12b6c84e49 Mon Sep 17 00:00:00 2001 From: Stephan Strittmatter Date: Sun, 5 Jul 2026 22:19:51 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20code=20review=20findings=20=E2=80=94?= =?UTF-8?q?=20dangling=20reference,=20printf,=20trailing=20return=20type?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move WiFiClientSecure client to downloadAndApply() scope so it outlives HTTPClient usage (fix dangling reference bug) - Replace printf() with Serial.printf() in MqttUtils (Arduino context) - Normalize isNtpSyncNeeded() to trailing return type for consistency --- src/PoolMonitor/MqttUtils.cpp | 2 +- src/PoolMonitor/OtaUpdater.cpp | 14 +++++++++----- src/PoolMonitor/PoolMonitorContext.cpp | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/PoolMonitor/MqttUtils.cpp b/src/PoolMonitor/MqttUtils.cpp index 3e189a4..3e415ad 100644 --- a/src/PoolMonitor/MqttUtils.cpp +++ b/src/PoolMonitor/MqttUtils.cpp @@ -43,7 +43,7 @@ bool parseHomeAssistantBoolState(const char* value) { || equalsIgnoreCaseAscii(value, "0")) { return false; } - printf("⚠️\tUnexpected boolean MQTT payload: %s (defaulting to false)\n", value); + Serial.printf("⚠️\tUnexpected boolean MQTT payload: %s (defaulting to false)\n", value); return false; } diff --git a/src/PoolMonitor/OtaUpdater.cpp b/src/PoolMonitor/OtaUpdater.cpp index ac4d9c2..c61dcb8 100644 --- a/src/PoolMonitor/OtaUpdater.cpp +++ b/src/PoolMonitor/OtaUpdater.cpp @@ -352,11 +352,8 @@ bool OtaUpdater::isNewerVersion(const String ¤t, const String &latest) { * @return true if the response is valid (HTTP 200, non-empty content). */ static bool openFirmwareDownload(const String &url, HTTPClient &http, + WiFiClientSecure &client, int &totalSize, WiFiClient *&stream) { - WiFiClientSecure client; - client.setCACert(kGitHubRootCA); - client.setTimeout(10000); - http.begin(client, url); http.setUserAgent("PoolMonitor/1.0"); http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); @@ -435,11 +432,18 @@ static int streamToUpdate(WiFiClient *stream, int totalSize, int *progress) { } bool OtaUpdater::downloadAndApply(const String &url) { + WiFiClientSecure client; + client.setCACert(kGitHubRootCA); + client.setTimeout(10000); + HTTPClient http; + http.setUserAgent("PoolMonitor/1.0"); + http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS); + int totalSize = 0; WiFiClient *stream = nullptr; - if (!openFirmwareDownload(url, http, totalSize, stream)) { + if (!openFirmwareDownload(url, http, client, totalSize, stream)) { http.end(); return false; } diff --git a/src/PoolMonitor/PoolMonitorContext.cpp b/src/PoolMonitor/PoolMonitorContext.cpp index bc1fdb5..1485435 100644 --- a/src/PoolMonitor/PoolMonitorContext.cpp +++ b/src/PoolMonitor/PoolMonitorContext.cpp @@ -629,7 +629,7 @@ auto PoolMonitorContext::isWiFiConnected() -> bool { return NetworkManager::isWiFiConnected(); } -bool PoolMonitorContext::isNtpSyncNeeded() { +auto PoolMonitorContext::isNtpSyncNeeded() -> bool { unsigned long last_ntp_sync = preferences_->getULong("last_ntp_sync", 0); unsigned long total_uptime = preferences_->getULong("total_uptime", 0);