From 944b7bea7794efd4cd708f8738da07e0c249612c Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 30 Aug 2026 20:56:25 -0400 Subject: [PATCH] fix(firmware): retry WiFi forever; add an uplink watchdog A node that lost WiFi could give up permanently. The disconnect handler counted retries in s_retry_num, which is only ever cleared on a successful IP_EVENT_STA_GOT_IP, so it accumulated across the node's entire uptime. After ten cumulative disconnects it set WIFI_FAIL_BIT and never called esp_wifi_connect() again. No backoff reset, no periodic retry, no reboot. Only a power cycle cleared it. MEASURED 2026-08-30: the AP serving the fleet took a firmware update and rebooted at ~03:38. All three wall-mounted nodes stopped delivering within three minutes of each other (nodes 0 and 1 at 03:37:03, to the second) and none recovered for 5.5-7.5 hours, until each was physically unplugged. CSI capture ran the whole time -- node 0's sync sequence advanced 16600 -> 46340 at a steady ~30/s while it was invisible to the server -- because capture is driver-level and needs no IP. The boards looked alive on a bench and dead to the aggregator. Retry is now unbounded with 500 ms -> 30 s exponential backoff, scheduled on an esp_timer one-shot so esp_wifi_connect() is never reached through a delay inside the event handler. Two things that were conflated are now separate: WIFI_BOOT_WAIT_ATTEMPTS only releases app_main from its startup wait so the rest of the node can boot; nothing stops the node retrying. The uplink watchdog covers the failures we have not diagnosed: no successful send for CONFIG_UPLINK_WATCHDOG_TIMEOUT_S restarts the node. The signal is a successful sendto(), which on UDP means the stack accepted the datagram, NOT that the server received it -- so stopping the aggregator cannot reboot the fleet. It arms on the first successful send rather than at boot, so a node powered up while the AP is down does not reboot-loop; the unbounded reconnect owns that case. The timeout is 1200 s, sized from a 112-minute soak rather than a guess: 30 observed silences across two nodes, median 225 s, one 510 s outlier that looked like two cycles running together. An earlier 120 s would have rebooted healthy nodes every few minutes, and even 600 s left only 90 s of margin over an event already seen once. Do not lower it below ~900 s while the ~225 s stalls are unexplained; a needless reboot re-runs ESP-NOW leader election and discards mesh time-sync state. Also guards thermal.c's apply_tx_power behind CONFIG_THERMAL_THROTTLE so a monitor-only build (the default, and what is on the wall) stops warning about an unused function. Kconfig gains CSI_GATE_MESH_ALIGNED here alongside the watchdog options; its implementation lands in a later commit and the option is inert until then. Co-Authored-By: claude-flow (cherry picked from commit bb6d954de68fd4d542e191fd5e82c753f67a9874) --- .../esp32-csi-node/main/Kconfig.projbuild | 62 +++++++++ firmware/esp32-csi-node/main/main.c | 121 ++++++++++++++++-- firmware/esp32-csi-node/main/stream_sender.c | 16 +++ firmware/esp32-csi-node/main/stream_sender.h | 12 ++ firmware/esp32-csi-node/main/thermal.c | 36 +++++- 5 files changed, 234 insertions(+), 13 deletions(-) diff --git a/firmware/esp32-csi-node/main/Kconfig.projbuild b/firmware/esp32-csi-node/main/Kconfig.projbuild index a3aaa7e15..9a51c4fd2 100644 --- a/firmware/esp32-csi-node/main/Kconfig.projbuild +++ b/firmware/esp32-csi-node/main/Kconfig.projbuild @@ -518,6 +518,7 @@ menu "Onboard LED (ADR-183)" (reaches yellow with less motion). endmenu + menu "Thermal monitoring" config THERMAL_MONITOR @@ -589,3 +590,64 @@ config THERMAL_HYSTERESIS_C heat is a reason to act now. endmenu + +menu "Network supervision" + +config UPLINK_WATCHDOG + bool "Restart the node if the CSI uplink stops for too long" + default y + help + Bounds how long a node can sit powered, capturing, and unable to reach + the network. + + MEASURED 2026-08-30: an AP-side event at 03:37 EDT disconnected all three + wall-mounted nodes within three minutes of each other, and every one of + them stayed dead for 5.5-7.5 hours until it was physically unplugged. The + specific cause of that outage (a reconnect counter that gave up + permanently) is fixed in main.c, but this exists so the NEXT unknown + wedge costs minutes instead of a night of data. + + The watchdog counts a successful sendto(), which on UDP means the network + stack accepted the datagram -- not that the aggregator received it. That + is deliberate: stopping the server does NOT reboot the fleet, because + sendto keeps succeeding into a socket whose peer is gone. It fires only + when the node's own network path is broken. + + It arms on the first successful send rather than at boot, so a node + powered on while the AP is down does not reboot-loop; the unbounded + reconnect handles that case instead. + +config UPLINK_WATCHDOG_TIMEOUT_S + int "Restart after this many seconds with no successful send" + depends on UPLINK_WATCHDOG + range 30 3600 + default 1200 + help + Must sit above the worst SELF-HEALING gap, or the watchdog reboots nodes + that were about to recover on their own -- which is worse than the fault + it guards against, because every reboot re-runs ESP-NOW leader election + and discards the mesh time-sync state. + + This was 120 s on the reasoning that the ENOMEM backoff caps at 2 s so a + healthy node is never quiet for long. A 58-minute soak on 2026-08-30 + falsified that: nodes 0 and 2 each went silent repeatedly with a median + gap of 210 s and a maximum of 225 s, and recovered every time without + intervention. 120 s would have rebooted them every few minutes forever. + (The cause of those 210 s gaps is still unknown -- it is not the ENOMEM + backoff, which cannot produce a gap that long.) + + Sized from the completed 112-minute soak, not the partial one: 30 + silences across two nodes, median 225 s, p90 225 s, and a single 510 s + outlier that looks like two cycles running together. A third such merge + would reach ~735 s, so 600 s was too tight -- it left only 90 s of margin + over an event already observed once. + + 1200 s is 2.35x the observed maximum while still bounding a true wedge to + twenty minutes rather than the 7.5 hours measured the same day. Revisit + it if the ~225 s flapping is ever explained and fixed; until then do not + lower it below ~900 s. + + If nodes do restart unexpectedly, check the reset reason in the sync + packet's health field before assuming the watchdog is responsible. + +endmenu diff --git a/firmware/esp32-csi-node/main/main.c b/firmware/esp32-csi-node/main/main.c index 5771dc0e2..660899708 100644 --- a/firmware/esp32-csi-node/main/main.c +++ b/firmware/esp32-csi-node/main/main.c @@ -60,7 +60,52 @@ nvs_config_t g_nvs_config; static EventGroupHandle_t s_wifi_event_group; static int s_retry_num = 0; -#define MAX_RETRY 10 + +/* WiFi reconnection. + * + * This used to be `#define MAX_RETRY 10` with the disconnect handler giving up + * permanently once s_retry_num reached it: it set WIFI_FAIL_BIT and never + * called esp_wifi_connect() again. s_retry_num is only cleared on a successful + * IP_EVENT_STA_GOT_IP, so it accumulated over the node's entire uptime -- ten + * disconnects spread across days left the node permanently off the network + * with nothing but a power cycle to clear it. + * + * MEASURED 2026-08-30: a single AP-side event at 03:37:03 EDT disconnected all + * three wall-mounted nodes within three minutes of each other. Every one of + * them latched off and stayed off for 5.5-7.5 hours, until it was physically + * unplugged. CSI capture kept running throughout (it is driver-level and needs + * no IP), so the boards looked alive on the bench and dead to the server. + * + * Retry is now unbounded with exponential backoff. Two things that were + * conflated are now separate: WIFI_BOOT_WAIT_ATTEMPTS only releases app_main + * from its startup wait so the rest of the node can boot; it does not stop the + * node trying to reconnect. Nothing stops the node trying to reconnect. */ +#define WIFI_BOOT_WAIT_ATTEMPTS 10 +#define WIFI_RETRY_BASE_MS 500 +#define WIFI_RETRY_MAX_MS 30000 + +static esp_timer_handle_t s_reconnect_timer; + +/* 500 ms doubling to a 30 s ceiling. Backoff matters because the failure this + * guards against is an AP that is down or rebooting: hammering + * esp_wifi_connect() at full rate for the minutes an AP takes to come back + * wastes power and floods the log without reconnecting any sooner. */ +static uint32_t wifi_retry_delay_ms(int attempt) +{ + int shift = attempt - 1; + if (shift < 0) shift = 0; + if (shift > 6) shift = 6; + uint32_t d = (uint32_t)WIFI_RETRY_BASE_MS << shift; + return d > WIFI_RETRY_MAX_MS ? WIFI_RETRY_MAX_MS : d; +} + +/* Runs on the esp_timer task, not the event loop task -- esp_wifi_connect() + * must not be reached through a delay inside the event handler itself. */ +static void wifi_reconnect_cb(void *arg) +{ + (void)arg; + esp_wifi_connect(); +} static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_id, void *event_data) @@ -70,13 +115,22 @@ static void event_handler(void *arg, esp_event_base_t event_base, } else if (event_base == WIFI_EVENT && event_id == WIFI_EVENT_STA_DISCONNECTED) { wifi_event_sta_disconnected_t *disc = (wifi_event_sta_disconnected_t *)event_data; ESP_LOGW(TAG, "WiFi disconnected, reason=%d rssi=%d", disc->reason, disc->rssi); - if (s_retry_num < MAX_RETRY) { - esp_wifi_connect(); - s_retry_num++; - ESP_LOGI(TAG, "Retrying WiFi connection (%d/%d)", s_retry_num, MAX_RETRY); - } else { + s_retry_num++; + /* Release the boot wait once, so a node that comes up while the AP is + * down still starts CSI capture and the mesh instead of blocking in + * app_main. Retrying continues regardless. */ + if (s_retry_num == WIFI_BOOT_WAIT_ATTEMPTS) { xEventGroupSetBits(s_wifi_event_group, WIFI_FAIL_BIT); } + uint32_t delay_ms = wifi_retry_delay_ms(s_retry_num); + if (s_reconnect_timer != NULL) { + esp_timer_stop(s_reconnect_timer); /* may not be running; harmless */ + esp_timer_start_once(s_reconnect_timer, (uint64_t)delay_ms * 1000); + } else { + esp_wifi_connect(); /* timer unavailable: retry now */ + } + ESP_LOGI(TAG, "Reconnecting in %lu ms (attempt %d)", + (unsigned long)delay_ms, s_retry_num); } else if (event_base == IP_EVENT && event_id == IP_EVENT_STA_GOT_IP) { ip_event_got_ip_t *event = (ip_event_got_ip_t *)event_data; ESP_LOGI(TAG, "Got IP: " IPSTR, IP2STR(&event->ip_info.ip)); @@ -85,6 +139,41 @@ static void event_handler(void *arg, esp_event_base_t event_base, } } +#ifdef CONFIG_UPLINK_WATCHDOG +/* Uplink supervision. + * + * The reconnect fix above addresses the one wedge whose mechanism we have + * proven. This catches the ones we have not: any state in which the node is + * powered, capturing, and unable to put a packet on the wire. It bounds such a + * state to CONFIG_UPLINK_WATCHDOG_TIMEOUT_S instead of the 7.5 hours measured + * on 2026-08-30. + * + * The signal is a successful sendto(), which on UDP means the stack accepted + * the datagram -- NOT that the server received it. That distinction is what + * makes this safe: stopping the aggregator does not reboot the fleet, because + * sendto keeps succeeding into a socket whose peer is gone. It fires only when + * the node's own network path is broken. + * + * It arms on the first successful send rather than at boot, so a node that has + * never reached the network does not reboot-loop while the AP is down; the + * unbounded reconnect above owns that case. */ +static void uplink_watchdog_tick(void) +{ + int64_t last = stream_sender_last_success_us(); + if (last == 0) { + return; /* never delivered -- not armed yet */ + } + int64_t idle_s = (esp_timer_get_time() - last) / 1000000; + if (idle_s < CONFIG_UPLINK_WATCHDOG_TIMEOUT_S) { + return; + } + ESP_LOGE(TAG, "uplink watchdog: no successful send for %lld s " + "(limit %d s) -- restarting", + (long long)idle_s, CONFIG_UPLINK_WATCHDOG_TIMEOUT_S); + esp_restart(); +} +#endif /* CONFIG_UPLINK_WATCHDOG */ + static void wifi_init_sta(void) { s_wifi_event_group = xEventGroupCreate(); @@ -136,6 +225,17 @@ static void wifi_init_sta(void) } #endif + const esp_timer_create_args_t reconnect_args = { + .callback = &wifi_reconnect_cb, + .name = "wifi_reconnect", + }; + esp_err_t rc_ret = esp_timer_create(&reconnect_args, &s_reconnect_timer); + if (rc_ret != ESP_OK) { + ESP_LOGW(TAG, "reconnect timer create failed: %s (falling back to " + "immediate retry)", esp_err_to_name(rc_ret)); + s_reconnect_timer = NULL; + } + ESP_ERROR_CHECK(esp_wifi_start()); ESP_LOGI(TAG, "WiFi STA initialized, connecting to SSID: %s", g_nvs_config.wifi_ssid); @@ -148,7 +248,9 @@ static void wifi_init_sta(void) if (bits & WIFI_CONNECTED_BIT) { ESP_LOGI(TAG, "Connected to WiFi"); } else if (bits & WIFI_FAIL_BIT) { - ESP_LOGE(TAG, "Failed to connect to WiFi after %d retries", MAX_RETRY); + ESP_LOGW(TAG, "Not connected after %d attempts -- continuing boot; " + "reconnection keeps retrying in the background", + WIFI_BOOT_WAIT_ATTEMPTS); } } @@ -541,8 +643,11 @@ void app_main(void) (swarm_ret == ESP_OK) ? g_nvs_config.seed_url : "off", (adapt_ret == ESP_OK) ? "on" : "off"); - /* Main loop — keep alive */ + /* Main loop — keep alive, and supervise the uplink. */ while (1) { vTaskDelay(pdMS_TO_TICKS(10000)); +#ifdef CONFIG_UPLINK_WATCHDOG + uplink_watchdog_tick(); +#endif } } diff --git a/firmware/esp32-csi-node/main/stream_sender.c b/firmware/esp32-csi-node/main/stream_sender.c index ecfc6ec9f..b766aff52 100644 --- a/firmware/esp32-csi-node/main/stream_sender.c +++ b/firmware/esp32-csi-node/main/stream_sender.c @@ -37,6 +37,12 @@ static uint32_t s_enomem_suppressed = 0; * succeeds. */ static uint32_t s_enomem_streak = 0; +/* Timestamp of the last sendto() the stack accepted, on either path; 0 until + * the node has ever delivered anything. Read by the uplink watchdog in main.c + * -- see the rationale there for why "accepted by the stack" is deliberately + * not "received by the server". */ +static int64_t s_last_success_us = 0; + static int sender_init_internal(const char *ip, uint16_t port) { s_sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); @@ -118,6 +124,7 @@ int stream_sender_send(const uint8_t *data, size_t len) /* A send got through — buffer pressure cleared; reset the backoff streak. */ s_enomem_streak = 0; + s_last_success_us = esp_timer_get_time(); return sent; } @@ -146,9 +153,18 @@ int stream_sender_send_priority(const uint8_t *data, size_t len) } return -1; } + /* Counts for the watchdog: the mesh sync packet rides this path at ~1 Hz, + * and its getting through proves the node's network path is intact even + * when the bulk CSI stream is being suppressed by the ENOMEM backoff. */ + s_last_success_us = esp_timer_get_time(); return sent; } +int64_t stream_sender_last_success_us(void) +{ + return s_last_success_us; +} + void stream_sender_deinit(void) { if (s_sock >= 0) { diff --git a/firmware/esp32-csi-node/main/stream_sender.h b/firmware/esp32-csi-node/main/stream_sender.h index 198bbee4e..8dedca431 100644 --- a/firmware/esp32-csi-node/main/stream_sender.h +++ b/firmware/esp32-csi-node/main/stream_sender.h @@ -50,6 +50,18 @@ int stream_sender_send(const uint8_t *data, size_t len); */ int stream_sender_send_priority(const uint8_t *data, size_t len); +/** + * esp_timer timestamp (microseconds) of the last sendto() the network stack + * accepted, on either the bulk or the priority path. Returns 0 if the node has + * not successfully sent anything since boot. + * + * This reports that the datagram was accepted for transmission, NOT that the + * aggregator received it -- UDP gives no such signal. Used by the uplink + * watchdog to detect a node that is powered and capturing but cannot reach the + * network. + */ +int64_t stream_sender_last_success_us(void); + /** * Close the UDP sender socket. */ diff --git a/firmware/esp32-csi-node/main/thermal.c b/firmware/esp32-csi-node/main/thermal.c index 20e67ff99..9759a7f15 100644 --- a/firmware/esp32-csi-node/main/thermal.c +++ b/firmware/esp32-csi-node/main/thermal.c @@ -55,6 +55,10 @@ static thermal_state_t s_state = THERMAL_OK; static int8_t s_txp = TXP_FULL; +#ifdef CONFIG_THERMAL_THROTTLE +/* Only referenced from the throttling block in thermal_tick(). Guarded so a + * monitor-only build (the default, and what is on the wall) does not warn + * about an unused function. */ static void apply_tx_power(int8_t q) { if (q == s_txp) { @@ -70,15 +74,37 @@ static void apply_tx_power(int8_t q) q, esp_err_to_name(r)); } } +#endif /* CONFIG_THERMAL_THROTTLE */ esp_err_t thermal_init(void) { - temperature_sensor_config_t cfg = - TEMPERATURE_SENSOR_CONFIG_DEFAULT(-10, 110); - esp_err_t r = temperature_sensor_install(&cfg, &s_sensor); + /* The requested span must fit ENTIRELY inside one of the sensor's + * predefined measurement ranges -- the driver walks a table and takes the + * first that contains it, so a span crossing two boundaries is rejected + * with ESP_ERR_INVALID_ARG rather than clamped. (-10,110) looks reasonable + * and is invalid for exactly that reason; it failed on real hardware. + * + * (20,100) is the right first choice here: the warn/throttle/critical + * thresholds all sit comfortably inside it, and indoor ambient is + * essentially never below 20 C. The wider fallbacks exist so an unheated + * room, or a different chip revision with a different table, degrades to a + * usable range instead of leaving the node unmonitored. */ + static const struct { int lo, hi; } spans[] = { + {20, 100}, {-10, 80}, {-30, 50}, {50, 125}, + }; + esp_err_t r = ESP_ERR_INVALID_ARG; + for (size_t i = 0; i < sizeof(spans) / sizeof(spans[0]); i++) { + temperature_sensor_config_t cfg = + TEMPERATURE_SENSOR_CONFIG_DEFAULT(spans[i].lo, spans[i].hi); + r = temperature_sensor_install(&cfg, &s_sensor); + if (r == ESP_OK) { + ESP_LOGI(TAG, "sensor range %d..%d C", spans[i].lo, spans[i].hi); + break; + } + } if (r != ESP_OK) { - ESP_LOGW(TAG, "sensor install failed: %s (continuing unmonitored)", - esp_err_to_name(r)); + ESP_LOGW(TAG, "sensor install failed on every range: %s " + "(continuing unmonitored)", esp_err_to_name(r)); return r; } r = temperature_sensor_enable(s_sensor);