From 78bbd96f45c9b1cbab60d3dfa7fde8f6e5237803 Mon Sep 17 00:00:00 2001 From: Alex Petrochenko Date: Mon, 18 May 2026 15:53:39 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20C1=20C2=20H1=20H2=20H3=20=E2=80=94=20sta?= =?UTF-8?q?bility=20fixes=20for=20long-running=20operation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1 (critical): Add 15s watchdog for WEATHER_REQUESTING state — resets to IDLE if TCP connection hangs silently, preventing permanent weather death C2 (critical): Fix millis() rollover in all retry timers — replace unsafe `millis() >= nextRetryTime` with subtraction-safe `(millis() - nextRetryTime) < 0x80000000UL` in RetryConfig and WiFiRetryConfig; fix boot guard with static flag instead of raw millis() comparison H1 (high): Fix DST last-Sunday formula — was using incorrect year-only heuristic; now derives weekday of the 31st from current day's tm_wday: `weekdayOf31 = (tm_wday + (31 - day)) % 7`. Verified: March 2026 = 29th ✓ H2 (high): Replace String+= with snprintf+sendContent in handleAPITime, handleAPIStatus, handleAPIDebug, handleAPIWeather — eliminates permanent heap fragmentation from JS polling every second H3 (high): Add volatile to weatherState and ntpState — shared between ESPAsyncTCP callbacks and main loop; prevents stale register-cached reads RAM: 37,268 bytes (46%) — reduced from 37,560 due to String elimination Co-Authored-By: Claude Sonnet 4.6 --- firmware/weather_clock/config.h | 6 +- firmware/weather_clock/globals.h | 7 +- firmware/weather_clock/ntp_client.cpp | 7 +- firmware/weather_clock/weather.cpp | 1 + firmware/weather_clock/weather_clock.ino | 20 +++- firmware/weather_clock/web_server.cpp | 128 ++++++++++++++--------- 6 files changed, 108 insertions(+), 61 deletions(-) diff --git a/firmware/weather_clock/config.h b/firmware/weather_clock/config.h index e82b06b..8abfbc6 100644 --- a/firmware/weather_clock/config.h +++ b/firmware/weather_clock/config.h @@ -70,7 +70,8 @@ struct RetryConfig { } bool isRetryTime() { - return nextRetryTime > 0 && millis() >= nextRetryTime; + // Subtraction-safe: works correctly across millis() rollover at ~49.7 days + return nextRetryTime > 0 && (millis() - nextRetryTime) < 0x80000000UL; } void reset() { @@ -101,7 +102,8 @@ struct WiFiRetryConfig { } bool isRetryTime() { - return nextRetryTime > 0 && millis() >= nextRetryTime; + // Subtraction-safe: works correctly across millis() rollover at ~49.7 days + return nextRetryTime > 0 && (millis() - nextRetryTime) < 0x80000000UL; } void reset() { diff --git a/firmware/weather_clock/globals.h b/firmware/weather_clock/globals.h index a960124..79a3efd 100644 --- a/firmware/weather_clock/globals.h +++ b/firmware/weather_clock/globals.h @@ -29,9 +29,9 @@ extern NTPClient timeClient; extern ESP8266WebServer server; extern ESP8266HTTPUpdateServer httpUpdater; -// State machines -extern WeatherState weatherState; -extern NTPState ntpState; +// State machines — volatile: written from ESPAsyncTCP callbacks, read in main loop +extern volatile WeatherState weatherState; +extern volatile NTPState ntpState; extern WiFiConnectionState wifiConnState; // Retry configurations @@ -71,6 +71,7 @@ extern SunTimes sunTimes; extern uint8_t displayMode; extern unsigned long lastModeSwitch; extern unsigned long lastWeatherUpdate; +extern unsigned long weatherRequestStart; // Dissolve transition state extern bool inTransition; diff --git a/firmware/weather_clock/ntp_client.cpp b/firmware/weather_clock/ntp_client.cpp index 63d7bc7..6bf0b42 100644 --- a/firmware/weather_clock/ntp_client.cpp +++ b/firmware/weather_clock/ntp_client.cpp @@ -26,7 +26,9 @@ bool ICACHE_FLASH_ATTR isDST(unsigned long epochTime) { // March: DST starts last Sunday at 01:00 UTC if (month == 3) { - int lastSunday = 31 - ((5 + timeinfo->tm_year) % 7); + // Compute weekday of the 31st from current day's weekday (tm_wday: 0=Sun) + int weekdayOf31 = (timeinfo->tm_wday + (31 - day)) % 7; + int lastSunday = 31 - weekdayOf31; if (day < lastSunday) return false; if (day > lastSunday) return true; if (hour < 1) return false; @@ -35,7 +37,8 @@ bool ICACHE_FLASH_ATTR isDST(unsigned long epochTime) { // October: DST ends last Sunday at 01:00 UTC if (month == 10) { - int lastSunday = 31 - ((1 + timeinfo->tm_year) % 7); + int weekdayOf31 = (timeinfo->tm_wday + (31 - day)) % 7; + int lastSunday = 31 - weekdayOf31; if (day < lastSunday) return true; if (day > lastSunday) return false; if (hour < 1) return true; diff --git a/firmware/weather_clock/weather.cpp b/firmware/weather_clock/weather.cpp index db3c3f4..08f91dd 100644 --- a/firmware/weather_clock/weather.cpp +++ b/firmware/weather_clock/weather.cpp @@ -156,6 +156,7 @@ void ICACHE_FLASH_ATTR fetchWeatherAsync() { weatherRequest.setTimeout(10); // 10 seconds weatherRequest.send(); weatherState = WEATHER_REQUESTING; + weatherRequestStart = millis(); Serial.println("Weather request sent (non-blocking)"); } else { weatherState = WEATHER_FAILED; diff --git a/firmware/weather_clock/weather_clock.ino b/firmware/weather_clock/weather_clock.ino index c71b670..ae85555 100644 --- a/firmware/weather_clock/weather_clock.ino +++ b/firmware/weather_clock/weather_clock.ino @@ -50,9 +50,9 @@ NTPClient timeClient(ntpUDP, "pool.ntp.org", 0, 60000); ESP8266WebServer server(80); ESP8266HTTPUpdateServer httpUpdater; -// State machines -WeatherState weatherState = WEATHER_IDLE; -NTPState ntpState = NTP_IDLE; +// State machines — volatile: written from ESPAsyncTCP callbacks, read in main loop +volatile WeatherState weatherState = WEATHER_IDLE; +volatile NTPState ntpState = NTP_IDLE; WiFiConnectionState wifiConnState = WIFI_CONN_IDLE; // Retry configurations @@ -92,6 +92,7 @@ SunTimes sunTimes; uint8_t displayMode = 0; unsigned long lastModeSwitch = 0; unsigned long lastWeatherUpdate = 0; +unsigned long weatherRequestStart = 0; // Tracks when WEATHER_REQUESTING began (TCP hang watchdog) // Dissolve transition state bool inTransition = false; @@ -311,14 +312,23 @@ void loop() { } } + // Watchdog: reset if WEATHER_REQUESTING stuck >15s (TCP hang / half-open connection) + if (weatherState == WEATHER_REQUESTING && (millis() - weatherRequestStart) > 15000UL) { + Serial.println("Weather request timeout (TCP hang) — resetting state"); + weatherState = WEATHER_IDLE; + weatherRetry.scheduleRetry(); + } + // Check for weather retry if (weatherRetry.isRetryTime() && weatherState == WEATHER_IDLE) { Serial.println("Weather retry time reached, attempting retry..."); fetchWeatherAsync(); } - // Update weather periodically - if (config.weather_enabled && millis() > 10000) { + // Update weather periodically (static flag avoids millis() > 10000 rollover trap) + static bool weatherBootReady = false; + if (!weatherBootReady && millis() > 10000UL) weatherBootReady = true; + if (config.weather_enabled && weatherBootReady) { unsigned long weatherInterval = config.weather_interval * 1000UL; if (millis() - lastWeatherUpdate > weatherInterval || lastWeatherUpdate == 0) { if (timeClient.isTimeSet() && weatherState == WEATHER_IDLE && !weatherRetry.isRetryTime()) { diff --git a/firmware/weather_clock/web_server.cpp b/firmware/weather_clock/web_server.cpp index 065ee7c..fb7c020 100644 --- a/firmware/weather_clock/web_server.cpp +++ b/firmware/weather_clock/web_server.cpp @@ -337,68 +337,98 @@ void ICACHE_FLASH_ATTR handleConfigSave() { } void ICACHE_FLASH_ATTR handleAPITime() { - String json = "{"; - json += "\"time\":\"" + timeClient.getFormattedTime() + "\","; - json += "\"hours\":" + String(timeClient.getHours()) + ","; - json += "\"minutes\":" + String(timeClient.getMinutes()) + ","; - json += "\"seconds\":" + String(timeClient.getSeconds()) + ","; - json += "\"epoch\":" + String(timeClient.getEpochTime()); - json += "}"; + char buf[256]; - server.send(200, "application/json", json); + server.setContentLength(CONTENT_LENGTH_UNKNOWN); + server.send(200, "application/json", ""); + + snprintf_P(buf, sizeof(buf), + PSTR("{\"time\":\"%s\",\"hours\":%d,\"minutes\":%d,\"seconds\":%d,\"epoch\":%lu}"), + timeClient.getFormattedTime().c_str(), + timeClient.getHours(), + timeClient.getMinutes(), + timeClient.getSeconds(), + timeClient.getEpochTime()); + server.sendContent(buf); + + server.sendContent(""); } void ICACHE_FLASH_ATTR handleAPIStatus() { - String json = "{"; - json += "\"wifi\":{"; - json += "\"ssid\":\"" + String(WiFi.SSID()) + "\","; - json += "\"ip\":\"" + WiFi.localIP().toString() + "\","; - json += "\"rssi\":" + String(WiFi.RSSI()) + ","; - json += "\"hostname\":\"" + String(config.hostname) + "\""; - json += "},"; - json += "\"time\":{"; - json += "\"current\":\"" + timeClient.getFormattedTime() + "\","; - json += "\"timezone_offset\":" + String(config.timezone_offset) + ","; - json += "\"ntp_synced\":" + String(timeClient.isTimeSet() ? "true" : "false"); - json += "},"; - json += "\"system\":{"; - json += "\"uptime\":" + String(millis() / 1000) + ","; - json += "\"free_heap\":" + String(ESP.getFreeHeap()) + ","; - json += "\"chip_id\":\"" + String(ESP.getChipId(), HEX) + "\""; - json += "}"; - json += "}"; + char buf[256]; - server.send(200, "application/json", json); + server.setContentLength(CONTENT_LENGTH_UNKNOWN); + server.send(200, "application/json", ""); + + snprintf_P(buf, sizeof(buf), + PSTR("{\"wifi\":{\"ssid\":\"%s\",\"ip\":\"%s\",\"rssi\":%d,\"hostname\":\"%s\"},"), + WiFi.SSID().c_str(), + WiFi.localIP().toString().c_str(), + WiFi.RSSI(), + config.hostname); + server.sendContent(buf); + + snprintf_P(buf, sizeof(buf), + PSTR("\"time\":{\"current\":\"%s\",\"timezone_offset\":%ld,\"ntp_synced\":%s},"), + timeClient.getFormattedTime().c_str(), + config.timezone_offset, + timeClient.isTimeSet() ? "true" : "false"); + server.sendContent(buf); + + snprintf_P(buf, sizeof(buf), + PSTR("\"system\":{\"uptime\":%lu,\"free_heap\":%u,\"chip_id\":\"%x\"}}"), + millis() / 1000, + ESP.getFreeHeap(), + ESP.getChipId()); + server.sendContent(buf); + + server.sendContent(""); } void ICACHE_FLASH_ATTR handleAPIDebug() { - String json = "{"; - json += "\"internet_connected\":" + String(internetConnected ? "true" : "false") + ","; - json += "\"ntp_attempts\":" + String(ntpAttempts) + ","; - json += "\"ntp_successes\":" + String(ntpSuccesses) + ","; - json += "\"last_error\":\"" + lastError + "\","; - json += "\"gateway\":\"" + WiFi.gatewayIP().toString() + "\","; - json += "\"dns\":\"" + WiFi.dnsIP().toString() + "\""; - json += "}"; + char buf[256]; - server.send(200, "application/json", json); + server.setContentLength(CONTENT_LENGTH_UNKNOWN); + server.send(200, "application/json", ""); + + snprintf_P(buf, sizeof(buf), + PSTR("{\"internet_connected\":%s,\"ntp_attempts\":%d,\"ntp_successes\":%d,\"last_error\":\"%s\",\"gateway\":\"%s\",\"dns\":\"%s\"}"), + internetConnected ? "true" : "false", + ntpAttempts, + ntpSuccesses, + lastError.c_str(), + WiFi.gatewayIP().toString().c_str(), + WiFi.dnsIP().toString().c_str()); + server.sendContent(buf); + + server.sendContent(""); } void ICACHE_FLASH_ATTR handleAPIWeather() { - String json = "{"; - json += "\"enabled\":" + String(config.weather_enabled ? "true" : "false") + ","; - json += "\"valid\":" + String(weather.valid ? "true" : "false") + ","; - json += "\"temperature\":" + String(weather.temperature, 1) + ","; - json += "\"weathercode\":" + String(weather.weathercode) + ","; - json += "\"windspeed\":" + String(weather.windspeed, 1) + ","; - json += "\"last_update\":" + String(weather.lastUpdate) + ","; - json += "\"sunrise\":\"" + String(sunTimes.sunrise) + "\","; - json += "\"sunset\":\"" + String(sunTimes.sunset) + "\","; - json += "\"sunrise_minutes\":" + String(sunTimes.sunriseMinutes) + ","; - json += "\"sunset_minutes\":" + String(sunTimes.sunsetMinutes); - json += "}"; + char buf[256]; - server.send(200, "application/json", json); + server.setContentLength(CONTENT_LENGTH_UNKNOWN); + server.send(200, "application/json", ""); + + snprintf_P(buf, sizeof(buf), + PSTR("{\"enabled\":%s,\"valid\":%s,\"temperature\":%.1f,\"weathercode\":%d,\"windspeed\":%.1f,\"last_update\":%lu,"), + config.weather_enabled ? "true" : "false", + weather.valid ? "true" : "false", + weather.temperature, + weather.weathercode, + weather.windspeed, + weather.lastUpdate); + server.sendContent(buf); + + snprintf_P(buf, sizeof(buf), + PSTR("\"sunrise\":\"%s\",\"sunset\":\"%s\",\"sunrise_minutes\":%d,\"sunset_minutes\":%d}"), + sunTimes.sunrise, + sunTimes.sunset, + sunTimes.sunriseMinutes, + sunTimes.sunsetMinutes); + server.sendContent(buf); + + server.sendContent(""); } void ICACHE_FLASH_ATTR handleAPIConfigExport() {