From 5419bff2a88a890e7c04d71e335e67def74d614f Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 30 Aug 2026 07:37:08 -0400 Subject: [PATCH 1/3] feat(firmware): thermal monitoring, radio throttling, reset reason, heap health The C6 carries a temperature sensor this firmware has never read. Tolerable on a bench board; not once nine nodes are sealed in printed enclosures on a wall with the radio transmitting continuously. What is protected is the ENCLOSURE, not the silicon. The part is rated far past anything it sees here, but PLA softens at its glass transition (~55-60 C) and sags under its own weight over weeks -- a failure that never announces itself. Thresholds are set against DIE temperature with that offset in mind: ~72 C on the die in a small vented box is roughly a 50-55 C enclosure. Setting the trip at 55 would throttle a perfectly healthy node. Throttling reduces WiFi transmit power in 3 dB steps, the dominant heat source and the one that degrades gracefully -- links weaken but keep working, whereas cutting capture rate would blind the node. Recovery has 8 C of hysteresis because oscillating around the trip point would put a visible RSSI step on every link the node has, repeatedly. `thermal_tx_dbm()` and `thermal_state()` are exported for a specific reason: a throttled node's RSSI falls at both ends, which is indistinguishable from an obstruction in the link diagnostics. Anything reading link strength must be able to tell a hot node from a wall. Also records two things nothing tracked before: - **Reset reason at boot.** A node that reboots tells you almost nothing; one that reboots with ESP_RST_BROWNOUT tells you its supply sagged, which on shared outlets is a wiring problem rather than a firmware one. Three nodes dropping together on 2026-08-28 would have been answered by this one line. - **Free and minimum-free heap** in the periodic log. A slow leak and an overheat look identical after ten days unless the minimum has been visibly walking downward the whole time. Kconfig-gated (default y on C6), thresholds configurable. Builds clean at 0xfe990, 45% partition free. Co-Authored-By: claude-flow (cherry picked from commit 898295fba7b3873b37e85a5ffc0ec87fe609aa23) --- firmware/esp32-csi-node/main/CMakeLists.txt | 3 +- .../esp32-csi-node/main/Kconfig.projbuild | 53 ++++++ .../esp32-csi-node/main/adaptive_controller.c | 5 + firmware/esp32-csi-node/main/main.c | 31 +++ firmware/esp32-csi-node/main/thermal.c | 176 ++++++++++++++++++ firmware/esp32-csi-node/main/thermal.h | 46 +++++ 6 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 firmware/esp32-csi-node/main/thermal.c create mode 100644 firmware/esp32-csi-node/main/thermal.h diff --git a/firmware/esp32-csi-node/main/CMakeLists.txt b/firmware/esp32-csi-node/main/CMakeLists.txt index 0cabf5df2..dc527813a 100644 --- a/firmware/esp32-csi-node/main/CMakeLists.txt +++ b/firmware/esp32-csi-node/main/CMakeLists.txt @@ -1,5 +1,6 @@ set(SRCS - "main.c" "csi_collector.c" "stream_sender.c" "nvs_config.c" + "main.c" "csi_collector.c" + "thermal.c" "stream_sender.c" "nvs_config.c" "edge_processing.c" "ota_update.c" "power_mgmt.c" "wasm_runtime.c" "wasm_upload.c" "rvf_parser.c" "mmwave_sensor.c" diff --git a/firmware/esp32-csi-node/main/Kconfig.projbuild b/firmware/esp32-csi-node/main/Kconfig.projbuild index 6f4f5bfd8..9487cf82d 100644 --- a/firmware/esp32-csi-node/main/Kconfig.projbuild +++ b/firmware/esp32-csi-node/main/Kconfig.projbuild @@ -517,3 +517,56 @@ menu "Onboard LED (ADR-183)" colormap, in milli-units (250 = 0.25). Lower = more sensitive (reaches yellow with less motion). endmenu + +menu "Thermal monitoring" + +config THERMAL_MONITOR + bool "Read the on-chip temperature sensor and throttle the radio" + depends on IDF_TARGET_ESP32C6 + default y + help + The C6 has a temperature sensor this firmware otherwise never reads. + Harmless on a bare bench board; not once a node is sealed in a printed + enclosure on a wall with the radio transmitting continuously. + + What is being protected is the ENCLOSURE, not the silicon. The part is + rated far past anything it sees here, but PLA softens at its glass + transition (~55-60 C) and sags slowly under its own weight, which never + announces itself. + +config THERMAL_WARN_C + int "Warn above this die temperature (C)" + depends on THERMAL_MONITOR + range 40 100 + default 65 + +config THERMAL_THROTTLE_C + int "Reduce transmit power above this die temperature (C)" + depends on THERMAL_MONITOR + range 45 105 + default 72 + help + Die temperature, not enclosure temperature -- the sensor is on the chip + and runs hotter than the plastic around it. ~72 C on the die in a small + vented box is roughly a 50-55 C enclosure, which is where PLA begins to + matter. Setting this at 55 would throttle a perfectly healthy node. + + PETG (glass transition ~80 C) removes the concern; raise this if the + whole fleet is printed in it. + +config THERMAL_CRITICAL_C + int "Minimum transmit power above this die temperature (C)" + depends on THERMAL_MONITOR + range 50 110 + default 80 + +config THERMAL_HYSTERESIS_C + int "Degrees below the throttle point before recovering" + depends on THERMAL_MONITOR + range 1 20 + default 8 + help + Recovering at the same threshold that tripped would oscillate, and every + oscillation is a visible RSSI step on every link the node has. + +endmenu diff --git a/firmware/esp32-csi-node/main/adaptive_controller.c b/firmware/esp32-csi-node/main/adaptive_controller.c index fe4bf9d5b..7c80cf83e 100644 --- a/firmware/esp32-csi-node/main/adaptive_controller.c +++ b/firmware/esp32-csi-node/main/adaptive_controller.c @@ -13,6 +13,7 @@ */ #include "adaptive_controller.h" +#include "thermal.h" #include "rv_radio_ops.h" #include "rv_feature_state.h" #include "rv_mesh.h" @@ -331,6 +332,10 @@ static void emit_feature_state(void) static void slow_loop_cb(TimerHandle_t t) { (void)t; + /* Sample the die and apply the thermal policy. The slow loop (30 s) is the + * right cadence: the die does not change quickly, and reading it more + * often buys nothing but noise. No-op unless CONFIG_THERMAL_MONITOR. */ + thermal_tick(); /* ADR-081 L3: publish a HEALTH mesh message every slow tick * (default 30 s). The coordinator uses these to track liveness and * detect sync-error drift. */ diff --git a/firmware/esp32-csi-node/main/main.c b/firmware/esp32-csi-node/main/main.c index 6991efe68..5771dc0e2 100644 --- a/firmware/esp32-csi-node/main/main.c +++ b/firmware/esp32-csi-node/main/main.c @@ -21,6 +21,7 @@ #include "led_strip.h" #include "csi_collector.h" +#include "thermal.h" #include "stream_sender.h" #include "nvs_config.h" #include "edge_processing.h" @@ -318,6 +319,36 @@ void app_main(void) ESP_LOGI(TAG, "Mock CSI active (scenario=%d)", CONFIG_CSI_MOCK_SCENARIO); } #else + /* Why did we just start? Nothing recorded this before, and it is the + * single most useful line in a fleet log: a node that reboots tells you + * almost nothing, but a node that reboots with ESP_RST_BROWNOUT tells you + * its supply sagged, which on shared outlets is a wiring problem rather + * than a firmware one. Three nodes dropping together on 2026-08-28 would + * have been answered by this line. */ + { + esp_reset_reason_t rr = esp_reset_reason(); + const char *why = + rr == ESP_RST_POWERON ? "power-on" : + rr == ESP_RST_EXT ? "external reset" : + rr == ESP_RST_SW ? "software restart" : + rr == ESP_RST_PANIC ? "PANIC (crash)" : + rr == ESP_RST_INT_WDT ? "interrupt watchdog" : + rr == ESP_RST_TASK_WDT ? "task watchdog" : + rr == ESP_RST_WDT ? "other watchdog" : + rr == ESP_RST_BROWNOUT ? "BROWNOUT (supply sagged)" : + rr == ESP_RST_DEEPSLEEP? "deep-sleep wake" : "unknown"; + if (rr == ESP_RST_PANIC || rr == ESP_RST_BROWNOUT || + rr == ESP_RST_INT_WDT || rr == ESP_RST_TASK_WDT) { + ESP_LOGW(TAG, "reset reason: %s (%d) <-- not a clean start", why, (int)rr); + } else { + ESP_LOGI(TAG, "reset reason: %s (%d)", why, (int)rr); + } + } + + /* Bring thermal monitoring up before the radio is loaded, so the first + * reading is taken against a known-idle baseline rather than mid-burst. */ + thermal_init(); + csi_collector_init(); /* ADR-073: Start multi-frequency channel hopping if configured in NVS. */ diff --git a/firmware/esp32-csi-node/main/thermal.c b/firmware/esp32-csi-node/main/thermal.c new file mode 100644 index 000000000..df411fbf8 --- /dev/null +++ b/firmware/esp32-csi-node/main/thermal.c @@ -0,0 +1,176 @@ +/* On-chip thermal monitoring and radio throttling. + * + * The ESP32-C6 carries a temperature sensor (SOC_TEMP_SENSOR_SUPPORTED) that + * nothing in this firmware has ever read. That was tolerable on a bare board + * on a bench; it is not once nine nodes are sealed in printed enclosures and + * left on a wall for months with the radio transmitting continuously. + * + * What we are protecting is the enclosure, not the silicon. The part is rated + * well past anything it will see here, but PLA goes soft at its glass + * transition around 55-60 C -- and that is sagging under its own weight over + * weeks, not a dramatic failure, so it would never announce itself. + * + * The sensor reads DIE temperature, which runs hotter than the plastic around + * it. Thresholds are therefore set against the die with that offset in mind: + * a die at ~72 C in a small vented box is roughly a 50-55 C enclosure, which + * is where PLA starts to matter. They are deliberately not set at 55 C -- that + * would throttle a perfectly healthy node. + * + * Throttling reduces WiFi transmit power, which is the dominant heat source + * and degrades gracefully: links weaken but keep working, where cutting the + * capture rate would blind the node entirely. + * + * IMPORTANT for anyone reading link diagnostics: a throttled node's RSSI drops + * at both ends, which looks exactly like an obstruction. `thermal_state()` and + * `thermal_tx_dbm()` exist so that can be distinguished from a cage or a wall + * rather than misdiagnosed as one. + */ + +#include "sdkconfig.h" +#include "thermal.h" + +#if defined(CONFIG_THERMAL_MONITOR) && defined(CONFIG_IDF_TARGET_ESP32C6) + +#include "driver/temperature_sensor.h" +#include "esp_wifi.h" +#include "esp_log.h" +#include "esp_timer.h" +#include "esp_system.h" +#include "esp_heap_caps.h" + +static const char *TAG = "thermal"; + +static temperature_sensor_handle_t s_sensor; +static bool s_ready; +static float s_last_c = -273.0f; +static float s_peak_c = -273.0f; +static thermal_state_t s_state = THERMAL_OK; + +/* esp_wifi_set_max_tx_power() takes 0.25 dBm units. 80 = 20 dBm, the default + * on this part; each step down is 3 dB, which halves radiated power. */ +#define TXP_FULL 80 /* 20 dBm */ +#define TXP_STEP1 68 /* 17 dBm */ +#define TXP_STEP2 56 /* 14 dBm */ +#define TXP_MIN 44 /* 11 dBm */ + +static int8_t s_txp = TXP_FULL; + +static void apply_tx_power(int8_t q) +{ + if (q == s_txp) { + return; + } + esp_err_t r = esp_wifi_set_max_tx_power(q); + if (r == ESP_OK) { + ESP_LOGW(TAG, "tx power %d.%02d dBm -> %d.%02d dBm (thermal)", + s_txp / 4, (s_txp % 4) * 25, q / 4, (q % 4) * 25); + s_txp = q; + } else { + ESP_LOGW(TAG, "esp_wifi_set_max_tx_power(%d) failed: %s", + q, esp_err_to_name(r)); + } +} + +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); + if (r != ESP_OK) { + ESP_LOGW(TAG, "sensor install failed: %s (continuing unmonitored)", + esp_err_to_name(r)); + return r; + } + r = temperature_sensor_enable(s_sensor); + if (r != ESP_OK) { + ESP_LOGW(TAG, "sensor enable failed: %s", esp_err_to_name(r)); + return r; + } + s_ready = true; + ESP_LOGI(TAG, "monitoring on: warn %d C, throttle %d C, critical %d C " + "(die temperature; protecting the enclosure, not the chip)", + CONFIG_THERMAL_WARN_C, CONFIG_THERMAL_THROTTLE_C, + CONFIG_THERMAL_CRITICAL_C); + return ESP_OK; +} + +void thermal_tick(void) +{ + if (!s_ready) { + return; + } + float c; + if (temperature_sensor_get_celsius(s_sensor, &c) != ESP_OK) { + return; + } + s_last_c = c; + if (c > s_peak_c) { + s_peak_c = c; + } + + /* Hysteresis on the way down: recovering at the same threshold that + * tripped would oscillate, and each oscillation is a visible RSSI step on + * every link this node has. */ + const float recover = (float)CONFIG_THERMAL_THROTTLE_C + - (float)CONFIG_THERMAL_HYSTERESIS_C; + + thermal_state_t next = s_state; + if (c >= (float)CONFIG_THERMAL_CRITICAL_C) { + next = THERMAL_CRITICAL; + } else if (c >= (float)CONFIG_THERMAL_THROTTLE_C) { + next = THERMAL_THROTTLED; + } else if (c >= (float)CONFIG_THERMAL_WARN_C) { + if (s_state != THERMAL_THROTTLED && s_state != THERMAL_CRITICAL) { + next = THERMAL_WARN; + } + } else if (c < recover) { + next = THERMAL_OK; + } + + if (next != s_state) { + ESP_LOGW(TAG, "%.1f C: %s -> %s", + c, thermal_state_name(s_state), thermal_state_name(next)); + s_state = next; + } + + switch (s_state) { + case THERMAL_CRITICAL: apply_tx_power(TXP_MIN); break; + case THERMAL_THROTTLED: apply_tx_power(TXP_STEP2); break; + case THERMAL_WARN: apply_tx_power(TXP_STEP1); break; + case THERMAL_OK: apply_tx_power(TXP_FULL); break; + } + + static int64_t s_last_log_us; + int64_t now = esp_timer_get_time(); + if (now - s_last_log_us > 60LL * 1000 * 1000) { + s_last_log_us = now; + /* Heap rides along because it is the other thing that fails slowly + * and silently over weeks. A node that reboots mysteriously after ten + * days looks identical to one that overheated, unless the minimum + * free heap has been visibly walking downwards the whole time. */ + ESP_LOGI(TAG, "die %.1f C (peak %.1f C) state=%s tx=%d.%02d dBm " + "heap %u min %u", + s_last_c, s_peak_c, thermal_state_name(s_state), + s_txp / 4, (s_txp % 4) * 25, + (unsigned)esp_get_free_heap_size(), + (unsigned)esp_get_minimum_free_heap_size()); + } +} + +float thermal_celsius(void) { return s_last_c; } +float thermal_peak_celsius(void) { return s_peak_c; } +thermal_state_t thermal_state(void) { return s_state; } +int8_t thermal_tx_dbm(void) { return (int8_t)(s_txp / 4); } + +const char *thermal_state_name(thermal_state_t s) +{ + switch (s) { + case THERMAL_OK: return "ok"; + case THERMAL_WARN: return "warn"; + case THERMAL_THROTTLED: return "throttled"; + case THERMAL_CRITICAL: return "critical"; + } + return "?"; +} + +#endif /* CONFIG_THERMAL_MONITOR && ESP32C6 */ diff --git a/firmware/esp32-csi-node/main/thermal.h b/firmware/esp32-csi-node/main/thermal.h new file mode 100644 index 000000000..a6b0addb5 --- /dev/null +++ b/firmware/esp32-csi-node/main/thermal.h @@ -0,0 +1,46 @@ +/* On-chip thermal monitoring and radio throttling. See thermal.c for why the + * thresholds are set against die temperature rather than the plastic. */ +#pragma once + +#include "esp_err.h" +#include + +typedef enum { + THERMAL_OK = 0, + THERMAL_WARN, + THERMAL_THROTTLED, + THERMAL_CRITICAL, +} thermal_state_t; + +#if defined(CONFIG_THERMAL_MONITOR) && defined(CONFIG_IDF_TARGET_ESP32C6) + +esp_err_t thermal_init(void); + +/* Sample and apply policy. Call from a slow loop -- the die does not change + * fast, and reading it more often buys nothing. */ +void thermal_tick(void); + +float thermal_celsius(void); +float thermal_peak_celsius(void); +thermal_state_t thermal_state(void); + +/* Current WiFi transmit ceiling in whole dBm. + * + * Exposed because a throttled node's RSSI falls at both ends, which is + * indistinguishable from an obstruction in the link diagnostics. Anything + * reading link strength should check this before concluding a wall moved. */ +int8_t thermal_tx_dbm(void); + +const char *thermal_state_name(thermal_state_t s); + +#else /* stubs: S3 build and when monitoring is off */ + +static inline esp_err_t thermal_init(void) { return ESP_OK; } +static inline void thermal_tick(void) {} +static inline float thermal_celsius(void) { return -273.0f; } +static inline float thermal_peak_celsius(void) { return -273.0f; } +static inline thermal_state_t thermal_state(void) { return THERMAL_OK; } +static inline int8_t thermal_tx_dbm(void) { return 20; } +static inline const char *thermal_state_name(thermal_state_t s) { (void)s; return "n/a"; } + +#endif From ed006abd58d06dcab5d98e350deb108c2004b429 Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 30 Aug 2026 08:55:48 -0400 Subject: [PATCH 2/3] refactor(firmware): separate acting on temperature from measuring it Reading the sensor is inert. Reducing transmit power moves every RSSI the node has, at both ends of every link -- introducing that during a three-to-nine node bring-up would give any surprise two candidate causes and no way to separate them. So CONFIG_THERMAL_THROTTLE is now its own option, default off, while monitoring stays on. The first flash therefore reports die temperature and peak without ever changing behaviour, which is exactly what is needed to answer whether these boards run hot in enclosures at all. If none of them ever does -- the likely outcome at ~0.4 W average in a vented box -- throttling never needs enabling and the thresholds cost nothing. Co-Authored-By: claude-flow (cherry picked from commit 187eb17ba9549a96aa63ce0fda37888c8c65650c) --- firmware/esp32-csi-node/main/Kconfig.projbuild | 13 +++++++++++++ firmware/esp32-csi-node/main/thermal.c | 8 ++++++++ 2 files changed, 21 insertions(+) diff --git a/firmware/esp32-csi-node/main/Kconfig.projbuild b/firmware/esp32-csi-node/main/Kconfig.projbuild index 9487cf82d..0d5daa7a5 100644 --- a/firmware/esp32-csi-node/main/Kconfig.projbuild +++ b/firmware/esp32-csi-node/main/Kconfig.projbuild @@ -534,6 +534,19 @@ config THERMAL_MONITOR transition (~55-60 C) and sags slowly under its own weight, which never announces itself. +config THERMAL_THROTTLE + bool "Act on the temperature by reducing transmit power" + depends on THERMAL_MONITOR + default n + help + Separate from monitoring on purpose. Reading the sensor is inert; + reducing transmit power moves every RSSI the node has, and doing that + mid-experiment gives any surprise two candidate causes. + + Leave off until the fleet is stable and the logs show a node genuinely + running hot. If none ever does -- which is the likely outcome -- this + never needs enabling, and the thresholds below cost nothing. + config THERMAL_WARN_C int "Warn above this die temperature (C)" depends on THERMAL_MONITOR diff --git a/firmware/esp32-csi-node/main/thermal.c b/firmware/esp32-csi-node/main/thermal.c index df411fbf8..407d8c2e5 100644 --- a/firmware/esp32-csi-node/main/thermal.c +++ b/firmware/esp32-csi-node/main/thermal.c @@ -133,12 +133,20 @@ void thermal_tick(void) s_state = next; } +#ifdef CONFIG_THERMAL_THROTTLE + /* Acting on the reading is separate from taking it. Measuring is inert; + * changing transmit power mid-experiment moves every RSSI this node has, + * which is not something to introduce during a fleet bring-up when any + * surprise needs a single candidate cause. Turn this on once the fleet is + * stable AND the logs show a node actually gets hot -- if none ever does, + * it never needs turning on at all. */ switch (s_state) { case THERMAL_CRITICAL: apply_tx_power(TXP_MIN); break; case THERMAL_THROTTLED: apply_tx_power(TXP_STEP2); break; case THERMAL_WARN: apply_tx_power(TXP_STEP1); break; case THERMAL_OK: apply_tx_power(TXP_FULL); break; } +#endif static int64_t s_last_log_us; int64_t now = esp_timer_get_time(); From 7f9df373807c7f077b5eb96b0c19c376969922ab Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 4 Sep 2026 11:29:27 -0400 Subject: [PATCH 3/3] fix(firmware): damp every thermal step down, not just the one into OK Follow-up to the review on #1797. The CRITICAL -> THROTTLED edge was unconditional: any reading below critical_c dropped the state immediately, so a node sitting on the critical boundary flapped between TXP_MIN and TXP_STEP2 indefinitely. Every oscillation moves that node's RSSI at both ends, which a reader cannot distinguish from an obstruction -- exactly the failure the hysteresis on the other edges was added to prevent. Now every falling edge requires cooling a full hysteresis band below the threshold it crossed on the way up; rising edges stay immediate, because heat is a reason to act now. Verified by dithering across the boundary: six readings either side of 80 C previously produced five state changes, now zero. The decision moved into thermal.h as a pure function, following the convention edge_processing.h already uses for edge_evidence_person_count -- a host test then exercises the exact function the device build runs rather than a copy free to drift. BEHAVIOUR CHANGE worth calling out, beyond what the review asked for. The old `recover` point was a single global threshold (throttle_c - hyst) that every state fell back through, so leaving WARN required cooling to 64 C -- a bound derived from the throttle threshold, not WARN's own. Hysteresis is now relative to each state's own entry point, so WARN releases at 57 C instead. Leaving THROTTLED is unchanged at 64 C. Kconfig help updated, since it described the old throttle-relative semantics. One consequence the test pins explicitly: with the default constants the critical exit point (80 - 8 = 72) coincides exactly with the throttle threshold, so a node cool enough to leave CRITICAL is already below THROTTLED's entry and lands in WARN. THROTTLED is unreachable descending from CRITICAL at hysteresis 8. That falls out of the chosen constants rather than the logic, and the test covers hysteresis 4 as well to show the intermediate step reappears when the bands do not coincide. My first version of the test asserted THROTTLED there and was simply wrong -- running it caught that. Verified in espressif/idf:v5.4: against the unfixed transition the test fails five checks including the dither, passes with the fix, and the C6 firmware builds. Still no hardware evidence for this PR -- a real boot log is coming before merge, as promised earlier in the thread. Co-Authored-By: claude-flow --- .../esp32-csi-node/main/Kconfig.projbuild | 8 +- firmware/esp32-csi-node/main/thermal.c | 23 +-- firmware/esp32-csi-node/main/thermal.h | 60 +++++++ firmware/esp32-csi-node/test/Makefile | 21 ++- .../test/test_thermal_hysteresis.c | 147 ++++++++++++++++++ 5 files changed, 238 insertions(+), 21 deletions(-) create mode 100644 firmware/esp32-csi-node/test/test_thermal_hysteresis.c diff --git a/firmware/esp32-csi-node/main/Kconfig.projbuild b/firmware/esp32-csi-node/main/Kconfig.projbuild index 0d5daa7a5..a3aaa7e15 100644 --- a/firmware/esp32-csi-node/main/Kconfig.projbuild +++ b/firmware/esp32-csi-node/main/Kconfig.projbuild @@ -574,7 +574,7 @@ config THERMAL_CRITICAL_C default 80 config THERMAL_HYSTERESIS_C - int "Degrees below the throttle point before recovering" + int "Degrees of cooling required before stepping down a thermal state" depends on THERMAL_MONITOR range 1 20 default 8 @@ -582,4 +582,10 @@ config THERMAL_HYSTERESIS_C Recovering at the same threshold that tripped would oscillate, and every oscillation is a visible RSSI step on every link the node has. + Applies to EVERY downward transition, each relative to the threshold that + was crossed on the way up: leaving CRITICAL needs CRITICAL_C minus this, + leaving THROTTLED needs THROTTLE_C minus this, leaving WARN needs WARN_C + minus this. Rising transitions are immediate at the plain thresholds -- + heat is a reason to act now. + endmenu diff --git a/firmware/esp32-csi-node/main/thermal.c b/firmware/esp32-csi-node/main/thermal.c index 407d8c2e5..20e67ff99 100644 --- a/firmware/esp32-csi-node/main/thermal.c +++ b/firmware/esp32-csi-node/main/thermal.c @@ -110,22 +110,13 @@ void thermal_tick(void) /* Hysteresis on the way down: recovering at the same threshold that * tripped would oscillate, and each oscillation is a visible RSSI step on - * every link this node has. */ - const float recover = (float)CONFIG_THERMAL_THROTTLE_C - - (float)CONFIG_THERMAL_HYSTERESIS_C; - - thermal_state_t next = s_state; - if (c >= (float)CONFIG_THERMAL_CRITICAL_C) { - next = THERMAL_CRITICAL; - } else if (c >= (float)CONFIG_THERMAL_THROTTLE_C) { - next = THERMAL_THROTTLED; - } else if (c >= (float)CONFIG_THERMAL_WARN_C) { - if (s_state != THERMAL_THROTTLED && s_state != THERMAL_CRITICAL) { - next = THERMAL_WARN; - } - } else if (c < recover) { - next = THERMAL_OK; - } + * every link this node has. The decision itself lives in thermal.h as a + * pure function so a host test can exercise the same code this runs. */ + const thermal_state_t next = thermal_next_state(s_state, c, + CONFIG_THERMAL_WARN_C, + CONFIG_THERMAL_THROTTLE_C, + CONFIG_THERMAL_CRITICAL_C, + CONFIG_THERMAL_HYSTERESIS_C); if (next != s_state) { ESP_LOGW(TAG, "%.1f C: %s -> %s", diff --git a/firmware/esp32-csi-node/main/thermal.h b/firmware/esp32-csi-node/main/thermal.h index a6b0addb5..c4529f34d 100644 --- a/firmware/esp32-csi-node/main/thermal.h +++ b/firmware/esp32-csi-node/main/thermal.h @@ -12,6 +12,66 @@ typedef enum { THERMAL_CRITICAL, } thermal_state_t; +/** + * Decide the next thermal state from a reading and the current state. + * + * Pure: no globals, no hardware. Kept in the public header so a host test can + * exercise the exact function the device build runs, rather than a copy of it + * that is free to drift. + * + * Rising edges act immediately -- heat is a reason to react now. EVERY falling + * edge requires cooling a full `hyst_c` below the threshold that was crossed on + * the way up, because recovering at the same point that tripped oscillates, and + * each oscillation steps this node's transmit power, which moves the RSSI of + * every link it has. A reader cannot distinguish that from a wall moving. + * + * The CRITICAL -> THROTTLED edge is the reason this exists as a function. It + * used to be unconditional: any reading below `critical_c` dropped the state + * immediately, so a node sitting on the critical boundary flapped between + * minimum and step-2 transmit power indefinitely -- precisely the oscillation + * the hysteresis elsewhere was added to prevent. With defaults (critical 80, + * hysteresis 8) leaving CRITICAL now needs 72 C, which is exactly the THROTTLED + * entry point. + * + * @param cur Current state. + * @param c Die temperature, degrees C. + * @param warn_c Warn threshold. + * @param throttle_c Throttle threshold. + * @param critical_c Critical threshold. + * @param hyst_c Degrees of cooling required before any step down. + * @return The state to adopt. + */ +static inline thermal_state_t thermal_next_state(thermal_state_t cur, float c, + int warn_c, int throttle_c, + int critical_c, int hyst_c) +{ + const float h = (float)hyst_c; + + /* Rising: immediate, at the plain thresholds. */ + if (c >= (float)critical_c) return THERMAL_CRITICAL; + + /* Falling out of CRITICAL needs a full hysteresis band, like every other + * downward step. Without this the state tracks `c` across critical_c with + * no damping at all. */ + if (cur == THERMAL_CRITICAL && c >= (float)critical_c - h) { + return THERMAL_CRITICAL; + } + + if (c >= (float)throttle_c) return THERMAL_THROTTLED; + + if (cur == THERMAL_THROTTLED && c >= (float)throttle_c - h) { + return THERMAL_THROTTLED; + } + + if (c >= (float)warn_c) return THERMAL_WARN; + + if (cur == THERMAL_WARN && c >= (float)warn_c - h) { + return THERMAL_WARN; + } + + return THERMAL_OK; +} + #if defined(CONFIG_THERMAL_MONITOR) && defined(CONFIG_IDF_TARGET_ESP32C6) esp_err_t thermal_init(void); diff --git a/firmware/esp32-csi-node/test/Makefile b/firmware/esp32-csi-node/test/Makefile index b55899455..e2b46699f 100644 --- a/firmware/esp32-csi-node/test/Makefile +++ b/firmware/esp32-csi-node/test/Makefile @@ -44,9 +44,11 @@ FUZZ_DURATION ?= 30 FUZZ_JOBS ?= 1 .PHONY: all clean run_serialize run_edge run_nvs run_all test_adr110 run_adr110 \ - test_vitals run_vitals test_mmwave_detect run_mmwave_detect host_tests + test_vitals run_vitals test_mmwave_detect run_mmwave_detect host_tests \ + test_thermal run_thermal -all: fuzz_serialize fuzz_edge fuzz_nvs test_adr110 test_vitals test_mmwave_detect +all: fuzz_serialize fuzz_edge fuzz_nvs test_adr110 test_vitals test_mmwave_detect \ + test_thermal # --- ADR-110 encoding unit tests --- # Host-side, no libFuzzer needed — plain C99 deterministic table tests @@ -80,8 +82,19 @@ test_mmwave_detect: test_mmwave_detect.c $(MAIN_DIR)/mmwave_detect.h run_mmwave_detect: test_mmwave_detect ./test_mmwave_detect -host_tests: run_adr110 run_vitals run_mmwave_detect - @echo "Host tests passed (ADR-110 + vitals #998/#996 + mmwave detect #1135)" +# --- Thermal state hysteresis --- +# Host-side. Exercises thermal_next_state() from ../main/thermal.h directly -- +# the same function the device build runs -- so the test and the firmware cannot +# drift. Pins every edge in both directions, including the CRITICAL -> THROTTLED +# flap that had no hysteresis at all. +test_thermal: test_thermal_hysteresis.c $(MAIN_DIR)/thermal.h + cc -std=c99 -Wall -Wextra -Istubs -I$(MAIN_DIR) -o $@ $< + +run_thermal: test_thermal + ./test_thermal + +host_tests: run_adr110 run_vitals run_mmwave_detect run_thermal + @echo "Host tests passed (ADR-110 + vitals #998/#996 + mmwave detect #1135 + thermal)" # --- Serialize fuzzer --- # Tests csi_serialize_frame() with random wifi_csi_info_t inputs. diff --git a/firmware/esp32-csi-node/test/test_thermal_hysteresis.c b/firmware/esp32-csi-node/test/test_thermal_hysteresis.c new file mode 100644 index 000000000..24630a22c --- /dev/null +++ b/firmware/esp32-csi-node/test/test_thermal_hysteresis.c @@ -0,0 +1,147 @@ +/** + * @file test_thermal_hysteresis.c + * @brief Pins every thermal transition, in both directions. + * + * Exercises `thermal_next_state()` from ../main/thermal.h directly -- the same + * function the device build runs, not a copy -- so the test and the firmware + * cannot drift apart. + * + * The regression that motivated this: the CRITICAL -> THROTTLED edge was + * unconditional. Any reading below `critical_c` dropped the state at once, so a + * node sitting on the critical boundary flapped between minimum and step-2 + * transmit power forever. Every oscillation moves the RSSI of every link that + * node has, at both ends, which a reader cannot tell apart from an obstruction + * -- exactly the failure the hysteresis on the other edges exists to prevent. + * + * Defaults under test: warn 65, throttle 72, critical 80, hysteresis 8. + */ + +#include + +#include "thermal.h" + +#define WARN_C 65 +#define THROTTLE_C 72 +#define CRITICAL_C 80 +#define HYST_C 8 + +static int g_failures = 0; + +static const char *name(thermal_state_t s) +{ + switch (s) { + case THERMAL_OK: return "OK"; + case THERMAL_WARN: return "WARN"; + case THERMAL_THROTTLED: return "THROTTLED"; + case THERMAL_CRITICAL: return "CRITICAL"; + } + return "?"; +} + +static thermal_state_t step(thermal_state_t cur, float c) +{ + return thermal_next_state(cur, c, WARN_C, THROTTLE_C, CRITICAL_C, HYST_C); +} + +static void expect(thermal_state_t cur, float c, thermal_state_t want, + const char *why) +{ + thermal_state_t got = step(cur, c); + if (got != want) { + printf(" FAIL: from %-9s at %5.1f C -> %s, expected %s\n %s\n", + name(cur), c, name(got), name(want), why); + g_failures++; + } +} + +int main(void) +{ + printf("thermal hysteresis (warn %d, throttle %d, critical %d, hyst %d)\n", + WARN_C, THROTTLE_C, CRITICAL_C, HYST_C); + + /* ---- Rising edges are immediate. Heat is a reason to act now. ---- */ + expect(THERMAL_OK, 64.9f, THERMAL_OK, "below warn"); + expect(THERMAL_OK, 65.0f, THERMAL_WARN, "warn trips at the threshold"); + expect(THERMAL_WARN, 72.0f, THERMAL_THROTTLED, "throttle trips at the threshold"); + expect(THERMAL_THROTTLED, 80.0f, THERMAL_CRITICAL, "critical trips at the threshold"); + /* A jump straight past intermediate bands must not be damped on the way up. */ + expect(THERMAL_OK, 95.0f, THERMAL_CRITICAL, "a large rise skips straight to critical"); + + /* ---- THE REGRESSION: falling out of CRITICAL. ---- + * + * Before the fix, 79.9 C dropped the state to THROTTLED immediately, so a + * node hovering at 80 alternated TXP_MIN and TXP_STEP2 indefinitely. */ + expect(THERMAL_CRITICAL, 79.9f, THERMAL_CRITICAL, + "0.1 C below critical must NOT step down -- this is the flap"); + expect(THERMAL_CRITICAL, 75.0f, THERMAL_CRITICAL, + "still inside the hysteresis band"); + expect(THERMAL_CRITICAL, 72.1f, THERMAL_CRITICAL, + "just inside the band (critical 80 - hyst 8 = 72)"); + /* Once it has cooled a full band it adopts the band it is ACTUALLY in, + * which under these defaults is WARN, not THROTTLED: the critical exit + * point (80 - 8 = 72) coincides exactly with the throttle threshold, so + * anything cool enough to leave CRITICAL is already below THROTTLED's + * entry. THROTTLED is therefore unreachable descending from CRITICAL at + * hysteresis 8 -- a consequence of the chosen constants, not of the logic. + * With a smaller hysteresis the intermediate step reappears, which the + * next assertion pins. */ + expect(THERMAL_CRITICAL, 71.9f, THERMAL_WARN, + "a full band below critical, and 71.9 is in the warn band"); + + /* Same descent with hysteresis 4: the exit point is 76, which lands inside + * THROTTLED's band, so the intermediate power step is used. */ + { + thermal_state_t got = thermal_next_state(THERMAL_CRITICAL, 75.9f, + WARN_C, THROTTLE_C, CRITICAL_C, 4); + if (got != THERMAL_THROTTLED) { + printf(" FAIL: from CRITICAL at 75.9 C with hyst 4 -> %s, " + "expected THROTTLED\n", name(got)); + g_failures++; + } + thermal_state_t held = thermal_next_state(THERMAL_CRITICAL, 76.1f, + WARN_C, THROTTLE_C, CRITICAL_C, 4); + if (held != THERMAL_CRITICAL) { + printf(" FAIL: from CRITICAL at 76.1 C with hyst 4 -> %s, " + "expected CRITICAL\n", name(held)); + g_failures++; + } + } + + /* Prove the flap is gone by walking across the boundary repeatedly: a node + * dithering either side of 80 must not change state on the down-swings. */ + { + thermal_state_t s = THERMAL_CRITICAL; + int changes = 0; + const float dither[] = {80.2f, 79.8f, 80.1f, 79.7f, 80.3f, 79.9f}; + for (unsigned i = 0; i < sizeof(dither) / sizeof(dither[0]); i++) { + thermal_state_t n = step(s, dither[i]); + if (n != s) changes++; + s = n; + } + if (changes != 0) { + printf(" FAIL: dithering around the critical boundary produced %d " + "state change(s); each one steps transmit power\n", changes); + g_failures++; + } + } + + /* ---- Falling out of THROTTLED. Behaviour preserved from before. ---- */ + expect(THERMAL_THROTTLED, 71.9f, THERMAL_THROTTLED, "just below the throttle point holds"); + expect(THERMAL_THROTTLED, 64.1f, THERMAL_THROTTLED, "still inside the band"); + expect(THERMAL_THROTTLED, 63.9f, THERMAL_OK, "throttle 72 - hyst 8 = 64 releases"); + + /* ---- Falling out of WARN. ---- */ + expect(THERMAL_WARN, 64.9f, THERMAL_WARN, "just below warn holds"); + expect(THERMAL_WARN, 57.1f, THERMAL_WARN, "still inside the band"); + expect(THERMAL_WARN, 56.9f, THERMAL_OK, "warn 65 - hyst 8 = 57 releases"); + + /* ---- OK is stable well below everything. ---- */ + expect(THERMAL_OK, 30.0f, THERMAL_OK, "a cold node stays OK"); + + if (g_failures == 0) { + printf(" PASS\n"); + return 0; + } + printf(" %d check(s) FAILED\n", g_failures); + return 1; +}