diff --git a/docs/AI_OTA_NOTES.md b/docs/AI_OTA_NOTES.md index f31b451..5d0401a 100644 --- a/docs/AI_OTA_NOTES.md +++ b/docs/AI_OTA_NOTES.md @@ -75,7 +75,9 @@ The single LittleFS partition has no independent rollback slot. Do not weaken si ## Reboot Routing -ElegantOTA auto-reboot stays disabled with `ElegantOTA.setAutoReboot(false)`. Successful ElegantOTA and pull OTA paths call `remoteQueueResetAt()` so the main loop owns the reset and normal teardown instead of a task or callback calling `ESP.restart()` directly. +ElegantOTA auto-reboot stays disabled with `ElegantOTA.setAutoReboot(false)`. Successful ElegantOTA and pull OTA paths call `remoteQueueOtaResetAt()` so their reset remains distinguishable from ordinary remote resets during an active update. Other scheduled main-loop resets use `remoteQueueResetAt()`; neither path should call `ESP.restart()` directly. + +ElegantOTA start and main-loop remote action dispatch share `otaDispatchMutex`. Hold it across the complete extracted action batch so `onOTAStart()` cannot publish active OTA until in-flight hardware work finishes. If OTA starts first, restore extracted actions without overwriting newer pending members of the display, low-power, soft-sleep, or timer replacement groups. The rollback path withdraws WiFi and mDNS with `stopWifi()` before `esp_ota_mark_app_invalid_rollback_and_reboot()`. Do not bypass this routing: a direct reboot can leave the service advertised until resolver caches expire. diff --git a/include/parameter.h b/include/parameter.h index 7d8a748..a4774b4 100644 --- a/include/parameter.h +++ b/include/parameter.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "calibration_validation.h" Preferences settingsPreferences; @@ -66,10 +67,13 @@ const uint32_t WSP_SET_SAMPLES = 1u << 10; const uint32_t WSP_WIFI_UPDATE = 1u << 11; const uint32_t WSP_RESET = 1u << 12; const uint32_t WSP_BLE_GYRO = 1u << 13; +const uint32_t WSP_OTA_RESET = 1u << 14; portMUX_TYPE wsPendingMux = portMUX_INITIALIZER_UNLOCKED; volatile uint32_t wsPendingMask = 0; volatile uint8_t pendingSamplesInUse = 0; volatile unsigned long pendingResetAt = 0; +volatile unsigned long pendingOtaResetAt = 0; +std::mutex otaDispatchMutex; const uint8_t OTA_DISPLAY_NONE = 0; const uint8_t OTA_DISPLAY_PROGRESS = 1; @@ -86,6 +90,13 @@ inline void remoteQueueResetAt(unsigned long resetAt) { portEXIT_CRITICAL(&wsPendingMux); } +inline void remoteQueueOtaResetAt(unsigned long resetAt) { + portENTER_CRITICAL(&wsPendingMux); + pendingOtaResetAt = resetAt; + wsPendingMask |= WSP_OTA_RESET; + portEXIT_CRITICAL(&wsPendingMux); +} + int i_onWrite_counter = 0; volatile unsigned long t_heartBeat = 0; volatile unsigned long t_firstConnect = 0; diff --git a/include/pull_ota.h b/include/pull_ota.h index 5970a69..afe068d 100644 --- a/include/pull_ota.h +++ b/include/pull_ota.h @@ -1285,7 +1285,7 @@ bool pullOtaInstall( } pullOtaDraw("Firmware done", "Restarting", "Web UI next"); delay(1500); - remoteQueueResetAt(millis()); + remoteQueueOtaResetAt(millis()); return true; } @@ -1376,7 +1376,7 @@ bool pullOtaResumePendingLittleFs() { } pullOtaDraw("Update done", "Restarting"); delay(1500); - remoteQueueResetAt(millis()); + remoteQueueOtaResetAt(millis()); return true; } diff --git a/include/websocket.h b/include/websocket.h index c946254..d903a86 100644 --- a/include/websocket.h +++ b/include/websocket.h @@ -141,21 +141,66 @@ inline void wsReplacePending(uint32_t setBits, uint32_t clearBits) { void processWsPendingCmds() { portENTER_CRITICAL(&wsPendingMux); - uint32_t mask = wsPendingMask; + uint32_t mask = b_ota ? (wsPendingMask & WSP_OTA_RESET) : wsPendingMask; uint8_t samplesInUse = pendingSamplesInUse; unsigned long resetAt = pendingResetAt; - wsPendingMask = 0; + unsigned long otaResetAt = pendingOtaResetAt; + wsPendingMask &= ~mask; if (mask & WSP_RESET) { pendingResetAt = 0; } + if (mask & WSP_OTA_RESET) { + pendingOtaResetAt = 0; + } portEXIT_CRITICAL(&wsPendingMux); if (mask == 0) return; + std::lock_guard otaDispatchLock(otaDispatchMutex); + portENTER_CRITICAL(&wsPendingMux); + if (b_ota) { + uint32_t deferredMask = mask & ~WSP_OTA_RESET; + const uint32_t replacementGroups[] = { + WSP_DISPLAY_ON | WSP_DISPLAY_OFF, + WSP_LOWPWR_ON | WSP_LOWPWR_OFF, + WSP_SLEEP_ON | WSP_SLEEP_OFF, + WSP_TIMER_START | WSP_TIMER_STOP | WSP_TIMER_ZERO, + }; + for (const uint32_t group : replacementGroups) { + if (wsPendingMask & group) { + deferredMask &= ~group; + } + } + if ((deferredMask & WSP_RESET) && !(wsPendingMask & WSP_RESET)) { + pendingResetAt = resetAt; + } + wsPendingMask |= deferredMask; + mask &= WSP_OTA_RESET; + } + portEXIT_CRITICAL(&wsPendingMux); + if (mask == 0) return; + + if (mask & WSP_OTA_RESET) { + if (otaResetAt != 0 && (long)(millis() - otaResetAt) < 0) { + portENTER_CRITICAL(&wsPendingMux); + if (!(wsPendingMask & WSP_OTA_RESET)) { + pendingOtaResetAt = otaResetAt; + } + wsPendingMask |= WSP_OTA_RESET; + portEXIT_CRITICAL(&wsPendingMux); + mask &= ~WSP_OTA_RESET; + if (mask == 0) return; + } else { + reset(); + return; + } + } if (mask & WSP_RESET) { if (resetAt != 0 && (long)(millis() - resetAt) < 0) { portENTER_CRITICAL(&wsPendingMux); + if (!(wsPendingMask & WSP_RESET)) { + pendingResetAt = resetAt; + } wsPendingMask |= WSP_RESET; - pendingResetAt = resetAt; portEXIT_CRITICAL(&wsPendingMux); mask &= ~WSP_RESET; if (mask == 0) return; @@ -207,6 +252,10 @@ void processWsPendingCmds() { wifiUpdate(); #endif } + if (b_ota) { + remoteQueuePending(mask & WSP_POWER_OFF); + return; + } if (mask & WSP_POWER_OFF) { b_powerOff = true; } diff --git a/include/wifi_ota.h b/include/wifi_ota.h index 348779d..4262b96 100644 --- a/include/wifi_ota.h +++ b/include/wifi_ota.h @@ -66,7 +66,10 @@ void processOtaDisplayUpdate() { void onOTAStart() { Serial.println("OTA update started!"); + std::lock_guard otaDispatchLock(otaDispatchMutex); + portENTER_CRITICAL(&wsPendingMux); b_ota = true; + portEXIT_CRITICAL(&wsPendingMux); } void onOTAProgress(size_t current, size_t final) { @@ -85,7 +88,7 @@ void onOTAEnd(bool success) { if (success) { Serial.println("OTA update finished successfully!"); queueOtaDisplay(OTA_DISPLAY_SUCCESS); - remoteQueueResetAt(millis() + OTA_RESTART_DELAY_MS); + remoteQueueOtaResetAt(millis() + OTA_RESTART_DELAY_MS); } else { Serial.println("There was an error during OTA update!"); queueOtaDisplay(OTA_DISPLAY_FAILURE); diff --git a/tools/test_ota_reboot_routing_contract.py b/tools/test_ota_reboot_routing_contract.py index 8642c35..221955b 100644 --- a/tools/test_ota_reboot_routing_contract.py +++ b/tools/test_ota_reboot_routing_contract.py @@ -7,6 +7,29 @@ ROLLBACK_HEADER = ROOT / "include" / "ota_rollback.h" WIFI_SETUP_SOURCE = ROOT / "src" / "wifi_setup.cpp" HDS_SOURCE = ROOT / "src" / "hds.ino" +WEBSOCKET_HEADER = ROOT / "include" / "websocket.h" +PARAMETER_HEADER = ROOT / "include" / "parameter.h" +BLE_HEADER = ROOT / "include" / "ble.h" + +WSP_DISPLAY_ON = 1 << 0 +WSP_DISPLAY_OFF = 1 << 1 +WSP_LOWPWR_ON = 1 << 2 +WSP_LOWPWR_OFF = 1 << 3 +WSP_SLEEP_ON = 1 << 4 +WSP_SLEEP_OFF = 1 << 5 +WSP_POWER_OFF = 1 << 6 +WSP_TIMER_START = 1 << 7 +WSP_TIMER_STOP = 1 << 8 +WSP_TIMER_ZERO = 1 << 9 +WSP_RESET = 1 << 12 +WSP_OTA_RESET = 1 << 14 + +REPLACEMENT_GROUPS = ( + WSP_DISPLAY_ON | WSP_DISPLAY_OFF, + WSP_LOWPWR_ON | WSP_LOWPWR_OFF, + WSP_SLEEP_ON | WSP_SLEEP_OFF, + WSP_TIMER_START | WSP_TIMER_STOP | WSP_TIMER_ZERO, +) def assert_contains(path, text): @@ -54,8 +77,62 @@ def assert_pending_sleep_ota_interleaving(): raise AssertionError("OTA wake leaves applied soft sleep or powered-off rails") +def assert_ordered(path, snippets): + contents = path.read_text(encoding="utf-8") + cursor = 0 + for snippet in snippets: + index = contents.find(snippet, cursor) + if index < 0: + raise AssertionError(f"{path.name} missing ordered snippet: {snippet}") + cursor = index + len(snippet) + + +def assert_interleavings(): + pending = WSP_RESET | WSP_TIMER_START + extracted = pending & WSP_OTA_RESET + pending &= ~extracted + if extracted != 0 or pending != WSP_RESET | WSP_TIMER_START: + raise AssertionError("ordinary remote reset escaped OTA deferral") + + extracted = WSP_DISPLAY_OFF | WSP_TIMER_START | WSP_OTA_RESET + pending = 0 + deferred = extracted & ~WSP_OTA_RESET + for group in REPLACEMENT_GROUPS: + if pending & group: + deferred &= ~group + pending |= deferred + extracted &= WSP_OTA_RESET + if pending != WSP_DISPLAY_OFF | WSP_TIMER_START or extracted != WSP_OTA_RESET: + raise AssertionError("OTA start race did not restore extracted remote actions") + + conflicts = ( + (WSP_DISPLAY_OFF, WSP_DISPLAY_ON), + (WSP_LOWPWR_ON, WSP_LOWPWR_OFF), + (WSP_SLEEP_ON, WSP_SLEEP_OFF), + (WSP_TIMER_START, WSP_TIMER_ZERO), + ) + for older, newer in conflicts: + deferred = older + pending = newer + for group in REPLACEMENT_GROUPS: + if pending & group: + deferred &= ~group + pending |= deferred + if pending != newer: + raise AssertionError("restored command overrode newer replacement intent") + + extracted = WSP_POWER_OFF + pending = 0 + ota_active = True + if ota_active: + pending |= extracted & WSP_POWER_OFF + if pending != WSP_POWER_OFF: + raise AssertionError("pull OTA start lost an extracted power action") + + def main(): assert_pending_sleep_ota_interleaving() + assert_interleavings() hds = HDS_SOURCE.read_text(encoding="utf-8") loop = hds[hds.index("void loop() {"):hds.index("void chargingOLED(")] if "if (b_ota || bleHasLiveClient()" not in loop: @@ -72,17 +149,29 @@ def main(): # restart must be queued through the main-loop reset mechanism instead. assert_contains(WIFI_OTA_HEADER, "ElegantOTA.setAutoReboot(false)") assert_not_contains(WIFI_OTA_HEADER, "ElegantOTA.setAutoReboot(true)") - assert_contains(WIFI_OTA_HEADER, "remoteQueueResetAt(millis() + OTA_RESTART_DELAY_MS)") + assert_contains(WIFI_OTA_HEADER, "remoteQueueOtaResetAt(millis() + OTA_RESTART_DELAY_MS)") + assert_not_contains(WIFI_OTA_HEADER, "remoteQueueResetAt(millis() + OTA_RESTART_DELAY_MS)") + assert_ordered( + WIFI_OTA_HEADER, + [ + "void onOTAStart()", + "std::lock_guard otaDispatchLock(otaDispatchMutex);", + "portENTER_CRITICAL(&wsPendingMux);", + "b_ota = true;", + "portEXIT_CRITICAL(&wsPendingMux);", + ], + ) # Pull OTA's success paths must queue the restart rather than calling # ESP.restart() directly from the Pull OTA task or the setup() task. - assert_contains(PULL_OTA_HEADER, "remoteQueueResetAt(millis())") + assert_contains(PULL_OTA_HEADER, "remoteQueueOtaResetAt(millis())") pull_ota_contents = PULL_OTA_HEADER.read_text(encoding="utf-8") - if pull_ota_contents.count("remoteQueueResetAt(millis())") != 2: + if pull_ota_contents.count("remoteQueueOtaResetAt(millis())") != 2: raise AssertionError( "pull_ota.h expected exactly two queued restarts " "(pullOtaInstall and pullOtaResumePendingLittleFs)" ) + assert_not_contains(PULL_OTA_HEADER, "remoteQueueResetAt(millis())") assert_not_contains(PULL_OTA_HEADER, "ESP.restart();\n return true;") # hdsOtaRollback() withdraws mDNS/WiFi before the rollback reboot, since @@ -101,6 +190,51 @@ def main(): "WiFi.config(INADDR_NONE, INADDR_NONE, INADDR_NONE, INADDR_NONE);", ) + assert_contains(PARAMETER_HEADER, "const uint32_t WSP_OTA_RESET = 1u << 14;") + assert_contains(PARAMETER_HEADER, "volatile unsigned long pendingOtaResetAt = 0;") + assert_contains(PARAMETER_HEADER, "inline void remoteQueueOtaResetAt(unsigned long resetAt)") + assert_contains(PARAMETER_HEADER, "std::mutex otaDispatchMutex;") + assert_contains(BLE_HEADER, "remoteQueuePending(WSP_RESET);") + + websocket = WEBSOCKET_HEADER.read_text(encoding="utf-8") + dispatcher_start = websocket.index("void processWsPendingCmds() {") + dispatcher = websocket[dispatcher_start:websocket.index("#if HDS_FEATURE_WEBSOCKET", dispatcher_start)] + assert_ordered( + WEBSOCKET_HEADER, + [ + "uint32_t mask = b_ota ? (wsPendingMask & WSP_OTA_RESET) : wsPendingMask;", + "wsPendingMask &= ~mask;", + "if (mask & WSP_OTA_RESET)", + ], + ) + dispatch_lock = "std::lock_guard otaDispatchLock(otaDispatchMutex);" + race_check = dispatcher.index("if (b_ota) {") + if dispatcher.index(dispatch_lock) > race_check: + raise AssertionError("OTA lifecycle lock starts after the OTA recheck") + pre_race = dispatcher[dispatcher.index("portEXIT_CRITICAL(&wsPendingMux);"):race_check] + for action in ["reset();", "u8g2.", "wakeScaleFromSoftSleep", "stopWatch.", "wifiUpdate();", "b_powerOff = true;"]: + if action in pre_race: + raise AssertionError(f"pending action dispatches before OTA recheck: {action}") + if "uint32_t deferredMask = mask & ~WSP_OTA_RESET;" not in dispatcher: + raise AssertionError("OTA start race does not restore extracted remote actions") + for group in [ + "WSP_DISPLAY_ON | WSP_DISPLAY_OFF", + "WSP_LOWPWR_ON | WSP_LOWPWR_OFF", + "WSP_SLEEP_ON | WSP_SLEEP_OFF", + "WSP_TIMER_START | WSP_TIMER_STOP | WSP_TIMER_ZERO", + ]: + if group not in dispatcher: + raise AssertionError(f"restoration does not preserve newer replacement group: {group}") + assert_ordered( + WEBSOCKET_HEADER, + [ + "wifiUpdate();", + "if (b_ota) {", + "remoteQueuePending(mask & WSP_POWER_OFF);", + "if (mask & WSP_POWER_OFF)", + ], + ) + print("OTA reboot routing contract tests passed")