refactor: structural Clean Code — setup lifecycle, dispatch table, OTA split, MqttUtils - #38
Merged
Merged
Conversation
…ispatch table, OTA split, utility extraction - 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
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Super-linter summary
All files and directories linted successfully For more information, see the GitHub Actions workflow run Powered by Super-linter |
…rn type - 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Structural Clean Code Refactoring
Dieser PR setzt die umfangreicheren strukturellen Verbesserungen aus dem Clean-Code-Review um. Fokus: Funktionen klein halten, if-else-Ketten eliminieren, Verantwortlichkeiten trennen.
1. PoolMonitorContext::setup() aufgeteilt (~133 Zeilen → 6 Phasen)
Problem:
setup()war eine monolithische 133-Zeilen-Funktion, die Boot-Banner, System-Init, Uptime-Tracking, Netzwerk-Entscheidung, Boot-Loop-Handling, NTP-Sync und Display-Update in einer einzigen Methode vermischte.Lösung: Aufgeteilt in sechs fokussierte Methoden mit klaren Namen:
printBootBanner()— FW-Logo und VersioninitSystem()— SystemMonitor, Preferences, OTAupdateBootAndUptimeStats()— boot_count, total_uptime, Netzwerk-EntscheidunghandleBootLoop(totalUptime)— Safe Mode bei Boot-Loop (Deep Sleep)runNetworkCycle(totalUptime)— Display + WiFi + MQTT + NTP + Display-UpdaterunOfflineCycle(totalUptime)— Zeit-Rekonstruktion, kein NetzwerkDie
setup()-Methode selbst ist jetzt nur noch 10 Zeilen und zeigt den Ablauf auf einen Blick.2. if-else-Kette in handleMqttMessage → Dispatch Table
Problem: 5-fach verschachteltes
if/else iffür MQTT-Topic-Dispatch.Lösung: Datengetriebene Dispatch-Tabelle mit
TopicHandler-Array. Jeder Eintrag enthält Topic, Label und Handler-Funktion. Die Schleife matcht und dispatched in 3 Zeilen.Erweiterbarkeit: Neues Topic = neuer Eintrag im Array, kein neuer else-Zweig.
3. OTA downloadAndApply aufgeteilt (100 Zeilen → 2 Helfer)
Problem:
downloadAndApply()war 100 Zeilen lang und vermischte HTTP-Download, Streaming und Update-Finalisierung.Lösung: Drei klar getrennte Phasen:
openFirmwareDownload()— HTTP-Request, Validierung, Stream-ÖffnungstreamToUpdate(stream, size, &progress)— Streaming in Update-Objekt mit Stall-ErkennungdownloadAndApply()— Orchestrierungs-Funktion (16 Zeilen)4. Utility-Funktionen extrahiert (MqttUtils)
Problem:
equalsIgnoreCaseAscii()undparseHomeAssistantBoolState()waren als statische Funktionen in PoolMonitorContext.cpp vergraben.Lösung: Neue Datei
MqttUtils.hpp/.cpp— wiederverwendbar, testbar, getrennte Verantwortung.5. Preferences-Fehler: Hard Lock → Clean Restart
Problem: Bei fehlschlagendem
preferences_->begin()trat einwhile(1) { delay(1000); }auf — hartes Lockup ohne Diagnose.Lösung: Cleanup (end + delete) +
ESP.restart()— das Gerät startet neu statt für immer zu hängen.Checkliste