diff --git a/firmware/esp32-csi-node/main/edge_processing.h b/firmware/esp32-csi-node/main/edge_processing.h index 0fe280b0a6..dafdb0a7cb 100644 --- a/firmware/esp32-csi-node/main/edge_processing.h +++ b/firmware/esp32-csi-node/main/edge_processing.h @@ -23,6 +23,11 @@ #include #include #include "esp_err.h" +/* Required for CONFIG_SOC_WIFI_HE_SUPPORT, which EDGE_MAX_SUBCARRIERS below + * switches on. Without it the macro is undefined, and C evaluates an undefined + * identifier in #if as 0 — silently selecting the pre-HE size on an HE part + * with no warning. */ +#include "sdkconfig.h" /* ---- Magic numbers ---- */ #define EDGE_VITALS_MAGIC 0xC5110002 /**< Vitals packet magic. */ @@ -42,8 +47,8 @@ * HE-capable AP delivers HE20 frames with 256 bins (iq_len = 512 bytes). * * `process_frame()` guards with `n_subcarriers > EDGE_MAX_SUBCARRIERS -> return`, - * so on C6 every frame was rejected and the whole edge pipeline — vitals, - * presence, fall detection, per-slot counting — silently did nothing. The Edge + * so on C6 every frame was rejected and the whole edge pipeline — vitals, + * presence, fall detection, per-slot counting — silently did nothing. The Edge * DSP task started, logged its banner, and never processed a frame. Confirmed * on hardware: no edge_proc log past init and no vitals packet ever reaching * the sink. diff --git a/firmware/esp32-csi-node/test/Makefile b/firmware/esp32-csi-node/test/Makefile index 12a7098c59..5ef78fbb19 100644 --- a/firmware/esp32-csi-node/test/Makefile +++ b/firmware/esp32-csi-node/test/Makefile @@ -56,11 +56,12 @@ FUZZ_JOBS ?= 1 test_vitals run_vitals test_mmwave_detect run_mmwave_detect host_tests \ test_thermal run_thermal test_csi_sanitize run_csi_sanitize \ test_c6_antenna run_c6_antenna test_serial_onboarding run_serial_onboarding \ - run_delivery_contract test_ota_health run_ota_health + run_delivery_contract test_ota_health run_ota_health \ + test_edge_grid run_edge_grid all: fuzz_serialize fuzz_edge fuzz_nvs test_adr110 test_vitals test_mmwave_detect \ test_thermal test_csi_sanitize test_c6_antenna test_serial_onboarding \ - test_ota_health + test_ota_health test_edge_grid # --- XIAO ESP32-C6 RF switch selection --- # Host-side truth table for the GPIO14 antenna selector. GPIO3 is always driven @@ -134,6 +135,22 @@ test_serial_onboarding: test_serial_onboarding_protocol.c $(MAIN_DIR)/serial_onb run_serial_onboarding: test_serial_onboarding ./test_serial_onboarding +# --- Edge subcarrier grid (HE20 truncation regression) --- +# Host-side. Pins EDGE_MAX_SUBCARRIERS against ../main/edge_processing.h so the +# test and the firmware cannot disagree about how wide a CSI frame may be. +# +# Built TWICE on purpose. The constant is target-conditional -- 256 on HE-capable +# parts (C6/C5), 128 pre-HE (S3) -- so a single compilation can only ever prove +# one branch. The HE build is the regression that failed before the fix; the +# pre-HE build guards against fixing C6 by spending .bss on every S3. +test_edge_grid: test_edge_subcarrier_grid.c $(MAIN_DIR)/edge_processing.h + cc -std=c99 -Wall -Wextra -Istubs_he -Istubs -I$(MAIN_DIR) -DEXPECT_HE=1 -o test_edge_grid_he $< + cc -std=c99 -Wall -Wextra -Istubs -I$(MAIN_DIR) -o test_edge_grid_pre_he $< + +run_edge_grid: test_edge_grid + ./test_edge_grid_he + ./test_edge_grid_pre_he + # --- OTA first-boot health check (ADR-379) --- # Host-side. Exercises ota_health_step() from ../main/ota_health.h directly -- # the decision the device runs to confirm or roll back an OTA'd image. @@ -147,8 +164,9 @@ run_delivery_contract: python3 test_delivery_contract.py host_tests: run_adr110 run_vitals run_mmwave_detect run_thermal run_csi_sanitize \ - run_c6_antenna run_serial_onboarding run_delivery_contract run_ota_health - @echo "Host tests passed (ADR-110 + CSI sanitation + vitals + mmwave + thermal + C6 antenna + USB onboarding + delivery contract + OTA health)" + run_c6_antenna run_serial_onboarding run_delivery_contract run_ota_health \ + run_edge_grid + @echo "Host tests passed (ADR-110 + CSI sanitation + vitals + mmwave + thermal + C6 antenna + USB onboarding + delivery contract + OTA health + edge grid)" # --- Serialize fuzzer --- # Tests csi_serialize_frame() with random wifi_csi_info_t inputs. @@ -186,5 +204,5 @@ run_all: run_serialize run_edge run_nvs clean: rm -f fuzz_serialize fuzz_edge fuzz_nvs test_adr110 test_vitals test_mmwave_detect \ test_thermal test_csi_sanitize test_c6_antenna test_serial_onboarding \ - test_ota_health + test_ota_health test_edge_grid_he test_edge_grid_pre_he rm -rf corpus_serialize/ corpus_edge/ corpus_nvs/ diff --git a/firmware/esp32-csi-node/test/fuzz_edge_enqueue.c b/firmware/esp32-csi-node/test/fuzz_edge_enqueue.c index 52fb937bcf..83cf7098e5 100644 --- a/firmware/esp32-csi-node/test/fuzz_edge_enqueue.c +++ b/firmware/esp32-csi-node/test/fuzz_edge_enqueue.c @@ -27,10 +27,21 @@ #include #include -/* ---- Reproduce the ring buffer from edge_processing.h ---- */ +/* ---- Reproduce the ring buffer from edge_processing.h ---- + * + * EDGE_MAX_SUBCARRIERS deliberately does NOT appear here. This target + * exercises the SPSC ring (ring_push/ring_pop), which is bounded by + * EDGE_MAX_IQ_BYTES and never consults the subcarrier grid. It previously + * carried a private `#define EDGE_MAX_SUBCARRIERS 128` that nothing read -- + * dead, and free to drift from the real constant without any test failing. + * That is precisely how the HE20 truncation bug survived: the grid is + * target-conditional (128 pre-HE, 256 on C6/C5) and a stale private copy + * would have masked it. + * + * The grid is pinned against the real header in test_edge_subcarrier_grid.c. + */ #define EDGE_RING_SLOTS 16 #define EDGE_MAX_IQ_BYTES 1024 -#define EDGE_MAX_SUBCARRIERS 128 typedef struct { uint8_t iq_data[EDGE_MAX_IQ_BYTES]; diff --git a/firmware/esp32-csi-node/test/stubs_he/sdkconfig.h b/firmware/esp32-csi-node/test/stubs_he/sdkconfig.h new file mode 100644 index 0000000000..647235e419 --- /dev/null +++ b/firmware/esp32-csi-node/test/stubs_he/sdkconfig.h @@ -0,0 +1,21 @@ +/* Stub: sdkconfig.h for an HE-capable target (ESP32-C6/C5). + * + * Exists so the subcarrier-grid test can prove that edge_processing.h pulls in + * sdkconfig.h ITSELF, rather than relying on some other header having done it + * first. The macro is deliberately defined ONLY here and never on the compiler + * command line: if the header stops including sdkconfig.h, C evaluates the + * undefined identifier in `#if` as 0, silently selects the 128-bin pre-HE grid + * on a 256-bin part, and the whole edge pipeline goes quiet with no warning. + * Passing -DCONFIG_SOC_WIFI_HE_SUPPORT=1 instead would mask exactly that. + * + * Placed on the include path ahead of stubs/ so it shadows the base stub. + */ +#ifndef SDKCONFIG_H_STUB +#define SDKCONFIG_H_STUB + +#include "esp_stubs.h" + +/* The SoC capability macro IDF defines for HE-capable parts. */ +#define CONFIG_SOC_WIFI_HE_SUPPORT 1 + +#endif diff --git a/firmware/esp32-csi-node/test/test_edge_subcarrier_grid.c b/firmware/esp32-csi-node/test/test_edge_subcarrier_grid.c new file mode 100644 index 0000000000..3e2e2ac65a --- /dev/null +++ b/firmware/esp32-csi-node/test/test_edge_subcarrier_grid.c @@ -0,0 +1,149 @@ +/** + * @file test_edge_subcarrier_grid.c + * @brief Pins EDGE_MAX_SUBCARRIERS to the radio's actual CSI grid. + * + * Regression test for the HE20 truncation bug: `EDGE_MAX_SUBCARRIERS` was a + * flat 128, an ESP32-S3 assumption. A C6/C5 associated to an HE-capable AP + * delivers HE20 frames with 256 bins, and `process_frame()` rejects anything + * larger than the constant: + * + * if (n_subcarriers == 0 || n_subcarriers > EDGE_MAX_SUBCARRIERS) return; + * + * so on those parts every frame was dropped and the entire edge pipeline -- + * vitals, presence, fall detection -- silently did nothing while the task + * logged a healthy banner. + * + * WHAT THIS TEST DELIBERATELY DOES NOT DO: it does not re-implement the guard. + * `fuzz_edge_enqueue.c` carried its own private copy of `EDGE_MAX_SUBCARRIERS` + * that could drift from the real one without anything failing, which is how a + * constant this load-bearing went wrong unnoticed. Re-stating the predicate + * here would recreate exactly that hazard. Instead this pulls the REAL constant + * from ../main/edge_processing.h and asserts the properties the guard derives + * from it, so the test and the firmware cannot disagree. + * + * Built twice, because the constant is target-conditional and one compilation + * can only ever prove one branch: + * + * test_edge_grid_he -Istubs_he (defines the macro) expects 256 + * test_edge_grid_pre_he -Istubs (macro absent) expects 128 + * + * The HE macro is supplied by a stub `sdkconfig.h` and NEVER by -D on the + * command line. That is the point: `edge_processing.h` must include + * sdkconfig.h itself. If it does not, C evaluates the undefined identifier in + * `#if` as 0, silently picks the 128-bin grid on a 256-bin part, and the edge + * pipeline goes quiet exactly as it did before this fix -- with no warning and + * no failing build. Defining the macro on the command line would compile the + * right branch regardless and mask that dependency completely. + */ + +#include +#include + +#include "edge_processing.h" + +static int g_failures = 0; + +#define CHECK(cond, ...) \ + do { \ + if (!(cond)) { \ + printf(" FAIL: " __VA_ARGS__); \ + printf("\n (%s, line %d)\n", #cond, __LINE__); \ + g_failures++; \ + } \ + } while (0) + +/** Bins an HE20 frame carries on an HE-capable ESP32 (C6/C5). */ +#define HE20_SUBCARRIERS 256 + +/** Bins a pre-HE part (S3 etc) reports at most. */ +#define PRE_HE_SUBCARRIERS 128 + +/** Bytes per subcarrier on the wire: one int8 I and one int8 Q. */ +#define BYTES_PER_SUBCARRIER 2 + +/* EXPECT_HE is a TEST-ONLY marker set by the Makefile, deliberately spelled + * differently from the IDF macro. It records what the build INTENDS; the + * assertions below then check what the header actually CONCLUDED. Deciding the + * expectation with `#if CONFIG_SOC_WIFI_HE_SUPPORT` would be circular: when the + * header fails to pull in sdkconfig.h, the macro is invisible to the test too, + * so it would quietly assert the pre-HE case and pass while the bug was live. + * That exact mistake was made writing this test and caught by running it + * against the broken header first. */ +#ifndef EXPECT_HE +#define EXPECT_HE 0 +#endif + +int main(void) +{ +#if EXPECT_HE + const char *build = "HE-capable (C6/C5)"; + const unsigned expect = HE20_SUBCARRIERS; +#else + const char *build = "pre-HE (S3)"; + const unsigned expect = PRE_HE_SUBCARRIERS; +#endif + + printf("edge subcarrier grid: %s build\n", build); + printf(" EDGE_MAX_SUBCARRIERS = %u (expect %u)\n", + (unsigned)EDGE_MAX_SUBCARRIERS, expect); + +#if EXPECT_HE + /* 0. The header must reach the SoC capability macro on its own. sdkconfig.h + * defines it; if edge_processing.h does not include sdkconfig.h, C reads + * the undefined identifier in `#if` as 0 and silently selects the pre-HE + * grid on a 256-bin part -- no error, no warning, no failing build, and + * a dead edge pipeline. This is the assertion that catches a missing + * `#include "sdkconfig.h"`. */ +#ifndef CONFIG_SOC_WIFI_HE_SUPPORT + CHECK(0, "CONFIG_SOC_WIFI_HE_SUPPORT is not visible after including " + "edge_processing.h -- the header is not including sdkconfig.h, so " + "the subcarrier grid silently falls back to the pre-HE size"); +#endif +#endif + + /* 1. The constant matches the radio this build targets. This is the + * assertion that would have failed before the fix on a C6. */ + CHECK((unsigned)EDGE_MAX_SUBCARRIERS == expect, + "EDGE_MAX_SUBCARRIERS is %u, expected %u for a %s build", + (unsigned)EDGE_MAX_SUBCARRIERS, expect, build); + +#if EXPECT_HE + /* 2. An HE20 frame survives the guard rather than being discarded. The + * guard rejects `n_subcarriers > EDGE_MAX_SUBCARRIERS`, so accepting a + * 256-bin frame is exactly `256 <= EDGE_MAX_SUBCARRIERS`. */ + CHECK(HE20_SUBCARRIERS <= EDGE_MAX_SUBCARRIERS, + "a %u-bin HE20 frame is rejected by the guard on an HE part", + (unsigned)HE20_SUBCARRIERS); +#else + /* 2'. Pre-HE parts keep the smaller grid. Sizing these buffers for 256 on + * an S3 would spend ~3.5 KB of .bss the part can never fill. */ + CHECK((unsigned)EDGE_MAX_SUBCARRIERS == PRE_HE_SUBCARRIERS, + "pre-HE grid changed from %u; that is a .bss regression on S3", + (unsigned)PRE_HE_SUBCARRIERS); +#endif + + /* 3. A full-width frame still fits one ring slot. Raising the subcarrier + * grid without the I/Q budget to carry it would move the truncation + * from process_frame() into ring_push()'s memcpy clamp, which is + * quieter still -- the frame would arrive, be silently shortened, and + * be processed as though complete. */ + const unsigned widest_iq_bytes = + (unsigned)EDGE_MAX_SUBCARRIERS * BYTES_PER_SUBCARRIER; + printf(" widest frame = %u bytes of I/Q (EDGE_MAX_IQ_BYTES = %u)\n", + widest_iq_bytes, (unsigned)EDGE_MAX_IQ_BYTES); + CHECK(widest_iq_bytes <= (unsigned)EDGE_MAX_IQ_BYTES, + "a full-width frame needs %u B but a ring slot holds %u B", + widest_iq_bytes, (unsigned)EDGE_MAX_IQ_BYTES); + + /* 4. Top-K selection has to fit inside the grid it selects from. */ + CHECK((unsigned)EDGE_TOP_K <= (unsigned)EDGE_MAX_SUBCARRIERS, + "EDGE_TOP_K (%u) exceeds the subcarrier grid (%u)", + (unsigned)EDGE_TOP_K, (unsigned)EDGE_MAX_SUBCARRIERS); + + if (g_failures == 0) { + printf(" PASS (%s)\n", build); + return 0; + } + printf(" %d check(s) FAILED\n", g_failures); + return 1; +}