fix: C1 C2 H1 H2 H3 — stability fixes for long-running operation

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 <noreply@anthropic.com>
2026-05-18 15:53:39 +01:00
parent eeec0155c3
commit 78bbd96f45
6 changed files with 108 additions and 61 deletions
@@ -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() {
@@ -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;
@@ -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;
@@ -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;
@@ -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()) {
@@ -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() {