diff --git a/firmware/esp32-csi-node/main/csi_collector.c b/firmware/esp32-csi-node/main/csi_collector.c index 7a13e5b7..903fcbb9 100644 --- a/firmware/esp32-csi-node/main/csi_collector.c +++ b/firmware/esp32-csi-node/main/csi_collector.c @@ -25,13 +25,13 @@ /* ADR-060: Access the global NVS config for MAC filter and channel override. */ extern nvs_config_t g_nvs_config; -/* Defensive fix (#232, #375, #385, #386, #390): capture node_id at init-time - * into a module-local static. Using the global g_nvs_config.node_id directly - * at every callback is vulnerable to any memory corruption that clobbers the - * struct (which users have reported reverting node_id to the Kconfig default - * of 1). The local copy is set once at csi_collector_init() and then used - * exclusively by csi_serialize_frame(). */ +/* Defensive fix (#232, #375, #385, #386, #390): capture node_id into a + * module-local static BEFORE wifi_init_sta() runs, because WiFi driver init + * can corrupt g_nvs_config.node_id (confirmed on device 80:b5:4e:c1:be:b8). + * main.c calls csi_collector_set_node_id() immediately after nvs_config_load(), + * and csi_serialize_frame() uses s_node_id exclusively. */ static uint8_t s_node_id = 1; +static bool s_node_id_early_set = false; /* ADR-057: Build-time guard — fail early if CSI is not enabled in sdkconfig. * Without this, the firmware compiles but crashes at runtime with: @@ -222,14 +222,31 @@ static void wifi_promiscuous_cb(void *buf, wifi_promiscuous_pkt_type_t type) (void)type; } +void csi_collector_set_node_id(uint8_t node_id) +{ + s_node_id = node_id; + s_node_id_early_set = true; + ESP_LOGI(TAG, "Early capture node_id=%u (before WiFi init, #232/#390)", + (unsigned)node_id); +} + void csi_collector_init(void) { - /* Capture node_id into module-local static at init time. After this point - * csi_serialize_frame() uses s_node_id exclusively, isolating the UDP - * frame node_id field from any memory corruption of g_nvs_config. */ - s_node_id = g_nvs_config.node_id; - ESP_LOGI(TAG, "Captured node_id=%u at init (defensive copy for #232/#375/#385/#390)", - (unsigned)s_node_id); + if (!s_node_id_early_set) { + /* Fallback: no early capture — use current g_nvs_config (may be clobbered). */ + s_node_id = g_nvs_config.node_id; + ESP_LOGW(TAG, "Late capture node_id=%u (no early set_node_id call)", + (unsigned)s_node_id); + } else if (g_nvs_config.node_id != s_node_id) { + /* Canary: early capture disagrees with current g_nvs_config — corruption + * happened between nvs_config_load() and here (likely wifi_init_sta). */ + ESP_LOGW(TAG, "node_id clobber CONFIRMED: early=%u g_nvs_config=%u " + "(WiFi init likely corrupted struct, using early value)", + (unsigned)s_node_id, (unsigned)g_nvs_config.node_id); + } else { + ESP_LOGI(TAG, "node_id=%u verified (early capture matches g_nvs_config)", + (unsigned)s_node_id); + } /* ADR-060: Determine the CSI channel. * Priority: 1) NVS override (--channel), 2) connected AP channel, 3) Kconfig default. */ @@ -290,16 +307,6 @@ void csi_collector_init(void) ESP_LOGI(TAG, "CSI collection initialized (node_id=%u, channel=%u)", (unsigned)s_node_id, (unsigned)csi_channel); - - /* Clobber-detection canary: if g_nvs_config.node_id no longer matches the - * value we captured, something corrupted the struct between nvs_config_load - * and here. This is the historic #232/#375 symptom. */ - if (g_nvs_config.node_id != s_node_id) { - ESP_LOGW(TAG, "node_id clobber detected: captured=%u but g_nvs_config=%u " - "(frames will use captured value %u). Please report to #390.", - (unsigned)s_node_id, (unsigned)g_nvs_config.node_id, - (unsigned)s_node_id); - } } /* Accessor for other modules that need the authoritative runtime node_id. */ diff --git a/firmware/esp32-csi-node/main/csi_collector.h b/firmware/esp32-csi-node/main/csi_collector.h index 6033ab4c..dfb3f20e 100644 --- a/firmware/esp32-csi-node/main/csi_collector.h +++ b/firmware/esp32-csi-node/main/csi_collector.h @@ -30,14 +30,24 @@ void csi_collector_init(void); /** - * Get the runtime node_id captured at csi_collector_init(). + * Capture node_id BEFORE wifi_init_sta() or any other heavy init. * - * This is a defensive copy of g_nvs_config.node_id taken at init time. Other - * modules (edge_processing, wasm_runtime, display_ui) should prefer this - * accessor over reading g_nvs_config.node_id directly, because the global - * struct can be clobbered by memory corruption (see #232, #375, #385, #390). + * Must be called from app_main() immediately after nvs_config_load(). + * WiFi driver initialization can corrupt g_nvs_config.node_id (confirmed + * on device 80:b5:4e:c1:be:b8, NVS=3 but post-WiFi reads as 1). + * This early capture shields s_node_id from that corruption window. * - * @return Node ID (0-255) as loaded from NVS or Kconfig default at boot. + * @param node_id Value from g_nvs_config.node_id, read right after NVS load. + */ +void csi_collector_set_node_id(uint8_t node_id); + +/** + * Get the runtime node_id (early capture if available, otherwise init-time). + * + * Other modules (edge_processing, wasm_runtime, display_ui) should prefer + * this accessor over reading g_nvs_config.node_id directly. + * + * @return Node ID (0-255) as loaded from NVS at boot. */ uint8_t csi_collector_get_node_id(void); diff --git a/firmware/esp32-csi-node/main/main.c b/firmware/esp32-csi-node/main/main.c index 9deef344..b80b0f83 100644 --- a/firmware/esp32-csi-node/main/main.c +++ b/firmware/esp32-csi-node/main/main.c @@ -140,6 +140,11 @@ void app_main(void) /* Load runtime config (NVS overrides Kconfig defaults) */ nvs_config_load(&g_nvs_config); + /* Capture node_id IMMEDIATELY — before wifi_init_sta() can corrupt + * g_nvs_config. See #232/#375/#390: WiFi driver init clobbers the struct + * on some devices, reverting node_id to the Kconfig default of 1. */ + csi_collector_set_node_id(g_nvs_config.node_id); + const esp_app_desc_t *app_desc = esp_app_get_description(); ESP_LOGI(TAG, "ESP32-S3 CSI Node (ADR-018) — v%s — Node ID: %d", app_desc->version, g_nvs_config.node_id);