test: make firmware logic bugs catchable without hardware (Workstream F)
The firmware has never been flashed, and a real bug already reached the repo because of it: RD03E_FRAME_LEN was 5 for a 6-byte frame, so the footer check collided with the distance high byte and EVERY distance reading was garbage — always `lo | 0x5500`, about 218 metres, regardless of what the sensor saw. That was pure logic with no hardware dependency. It should have been catchable on a laptop, and there was simply no way to run the code. Extracted the hardware-free logic out of the three drivers — rd03e_parse, bmp280_compensate, mems_level — as moves rather than rewrites, carrying the explanatory comments along with the code they explain. The drivers now own only their bus I/O and call into the pure units, so nothing changes for the real device. `./run_tests.sh` builds them with gcc -Wall -Wextra -Werror plus a dependency-free assert harness: 175 checks, 0 failed, from a clean tree. Proven to catch the actual bug rather than assumed to: reintroducing FRAME_LEN 5 fails four checks, including one that reads "a simple-report frame is 6 bytes, not 5", plus the truncated-frame and 5-byte-window cases. Restored, green again. This does NOT make the firmware verified, and the README says so plainly — it is called a narrow exception and scoped to pure logic. Wiring, timing, real register behaviour and the reconstructed RD-03E frame format all still need the physical board. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -39,6 +39,21 @@ specific, itemized breakdown — this mirrors the same convention
|
||||
`frontend/src/lib/sdr.ts`'s `HARDWARE PASS REQUIRED` header comment uses
|
||||
elsewhere in this repo.
|
||||
|
||||
**One narrow exception, added deliberately.** The pure logic — frame
|
||||
parsing, byte order, compensation maths, level maths — has been extracted
|
||||
into ESP-IDF-free units under `main/` and is now covered by a host test
|
||||
suite you can run anywhere gcc exists:
|
||||
|
||||
```bash
|
||||
./run_tests.sh # from the repo root, or test/run_tests.sh from here
|
||||
```
|
||||
|
||||
That suite is *machine-verified*, not reasoned about. It is also strictly
|
||||
about logic: it never touches a bus, a pin, or ESP-IDF, and it cannot tell
|
||||
you whether the protocols it implements match the real modules. Read the
|
||||
first subsection of [What's verified vs. not](#whats-verified-vs-not) for
|
||||
exactly what it does and does not establish.
|
||||
|
||||
## Directory layout
|
||||
|
||||
```
|
||||
@@ -46,6 +61,10 @@ firmware/esp32p4-sensor-node/
|
||||
├── CMakeLists.txt top-level ESP-IDF project file
|
||||
├── sdkconfig.defaults seed config (idf.py generates the real sdkconfig)
|
||||
├── README.md this file
|
||||
├── test/ host test harness (gcc only, no ESP-IDF)
|
||||
│ ├── run_tests.sh build + run; non-zero exit on failure
|
||||
│ ├── Makefile same build, for `make check`
|
||||
│ └── test_*.c plain-assert tests, zero dependencies
|
||||
├── components/
|
||||
│ ├── README.md
|
||||
│ └── rtlsdr_experimental/ Workstream J — opt-in USB-host RTL-SDR module
|
||||
@@ -59,11 +78,20 @@ firmware/esp32p4-sensor-node/
|
||||
├── telemetry_client.{h,c} HTTP POST task -> /api/device/telemetry
|
||||
├── sensor_driver.h the sensor_driver_t registry interface
|
||||
├── sensor_registry.{h,c} the concrete list of compiled-in drivers
|
||||
├── bmp280.{h,c} temperature/pressure over I2C
|
||||
├── rd03e.{h,c} presence/distance/gesture over UART
|
||||
└── mems_mic.{h,c} EVP-style audio RMS level over I2S
|
||||
├── bmp280.{h,c} temperature/pressure over I2C (I2C traffic)
|
||||
├── bmp280_compensate.{h,c} PURE: calib/ADC decode + Bosch compensation
|
||||
├── rd03e.{h,c} presence/distance/gesture over UART (UART I/O)
|
||||
├── rd03e_parse.{h,c} PURE: the 6-byte frame scanner
|
||||
├── mems_mic.{h,c} EVP-style audio level over I2S (I2S traffic)
|
||||
└── mems_level.{h,c} PURE: RMS -> dBFS maths
|
||||
```
|
||||
|
||||
The units marked PURE include only `<stdint.h>`/`<stddef.h>`/`<math.h>` —
|
||||
no ESP-IDF, no FreeRTOS, no logging — so `test/` can compile them with
|
||||
plain gcc. The drivers alongside them own the bus I/O and call in. Any new
|
||||
decision that is pure arithmetic or byte handling belongs in a PURE unit,
|
||||
where it can be tested before it reaches a board.
|
||||
|
||||
## Build instructions
|
||||
|
||||
Requires an ESP-IDF install (v5.3 or newer — ESP32-P4 target support landed
|
||||
@@ -113,9 +141,66 @@ BLE provisioning) in this build — see the spec's scope boundary.
|
||||
|
||||
## What's verified vs. not
|
||||
|
||||
**Structurally verified** (reasoned through carefully against ESP-IDF's
|
||||
documented API surface and each sensor's public protocol docs; internally
|
||||
consistent; no known syntax errors or obviously-wrong API usage):
|
||||
### Machine-verified on a host: the pure logic (`test/`)
|
||||
|
||||
Run it with `./run_tests.sh` (from the repo root, or `test/run_tests.sh`
|
||||
here). It needs **gcc and nothing else** — no ESP-IDF, no toolchain, no
|
||||
board, no network. It compiles with `-Wall -Wextra -Werror` and exits
|
||||
non-zero on any failure. Current status: **175 checks, 0 failures.**
|
||||
|
||||
This exists because a real bug shipped in this firmware and sat there
|
||||
undetected: `RD03E_FRAME_LEN` was `5` for a 6-byte frame, so the footer
|
||||
check compared the *distance high byte* against `0x55` instead of the
|
||||
second footer byte. Frames only "validated" when the high byte happened to
|
||||
be `0x55`, and every distance reading came back as `lo | 0x5500` — roughly
|
||||
218 metres, always. That was pure arithmetic with zero hardware dependency
|
||||
and it should have been catchable on a laptop. The three pure units below
|
||||
were split out of their drivers precisely so that class of bug now is.
|
||||
|
||||
The ESP-IDF-free units, and what the tests actually prove about each:
|
||||
|
||||
- `main/rd03e_parse.c` — the frame scanner. Proven: a well-formed frame
|
||||
yields the exact expected gesture and distance; `0x2C 0x01` is 300 cm,
|
||||
little-endian (the shipped bug produced 21804 cm here); `RD03E_FRAME_LEN`
|
||||
really is 6 and two back-to-back frames occupy exactly 12 bytes without
|
||||
desynchronising; two frames in one buffer report the **newest**; a wrong
|
||||
byte in *either* footer position is rejected; a truncated trailing frame
|
||||
is ignored and never read past; garbage (including a stray `0xAA`) before
|
||||
a valid frame is skipped; a `0xAA` that is really a payload byte does not
|
||||
fool the scanner; the full 16-bit distance range decodes with the correct
|
||||
byte order; NULL/short/empty inputs return "no frame" rather than
|
||||
crashing.
|
||||
- `main/bmp280_compensate.c` — calibration/ADC decoding plus the Bosch
|
||||
§3.11.3 compensation maths. Proven: all twelve calibration coefficients
|
||||
decode little-endian with signedness preserved; the 20-bit ADC words
|
||||
decode with pressure first, temperature second, and the XLSB's low nibble
|
||||
discarded; the datasheet's own worked reference values (adc_T=519888,
|
||||
adc_P=415148 with the published calibration set) come out at ~25.08 °C
|
||||
and ~100653 Pa; temperature rises with raw ADC; pressure falls
|
||||
monotonically across an ADC sweep and stays in a physically plausible
|
||||
band; the `var1 == 0` guard returns exactly `0.0` rather than `inf`/`NaN`
|
||||
when the calibration block is all zeros (i.e. a silently-failed I2C read).
|
||||
- `main/mems_level.c` — RMS → dBFS. Proven: a full-scale block reads
|
||||
~0 dBFS; silence reads the −120 floor and is never `-inf` or `NaN` (which
|
||||
would poison the JSON the backend receives); sub-LSB noise in the padding
|
||||
bits stays at the floor; halving amplitude costs ~6 dB; negative samples
|
||||
carry the same energy as positive; the level rises monotonically with
|
||||
amplitude and stays inside [−120, 0]; empty/NULL blocks do not divide by
|
||||
zero.
|
||||
|
||||
**What this does NOT prove — and the distinction matters.** These tests
|
||||
verify the firmware's logic against the *protocol and datasheet as this
|
||||
repo understands them*. They cannot verify that understanding. If the
|
||||
RD-03E's real frame format differs from the reconstruction below, every
|
||||
test still passes and every reading is still wrong. Nothing here touches a
|
||||
UART, an I2C bus, an I2S clock, a GPIO, or ESP-IDF itself. See the
|
||||
hardware list further down — it is unchanged by these tests.
|
||||
|
||||
### Structurally verified only
|
||||
|
||||
(Reasoned through carefully against ESP-IDF's documented API surface and
|
||||
each sensor's public protocol docs; internally consistent; no known syntax
|
||||
errors or obviously-wrong API usage. Not compiled, not run.)
|
||||
|
||||
- Project skeleton (`CMakeLists.txt` × 2, `sdkconfig.defaults`,
|
||||
`idf_component_register` call) follows ESP-IDF's standard project layout.
|
||||
@@ -126,24 +211,26 @@ consistent; no known syntax errors or obviously-wrong API usage):
|
||||
- HTTP client (`telemetry_client.c`) builds the exact JSON shape the spec's
|
||||
contract defines and POSTs it via `esp_http_client` with
|
||||
`Authorization: Bearer <token>` and `Content-Type: application/json`.
|
||||
- BMP280 driver (`bmp280.c`): register map and the double-precision
|
||||
compensation formulas are transcribed from Bosch's public BMP280
|
||||
- BMP280 driver (`bmp280.c` + `bmp280_compensate.c`): register map and the
|
||||
double-precision compensation formulas are transcribed from Bosch's public BMP280
|
||||
datasheet (rev 1.23, §3.11.1–3.11.3) — this is well-trodden, publicly
|
||||
documented territory, and the formulas are checkable line-by-line against
|
||||
the datasheet. Uses ESP-IDF's newer `driver/i2c_master.h` API (the
|
||||
current idiomatic choice; the older `driver/i2c.h` is being phased out).
|
||||
Temperature + pressure only — the part in use (a GY-BMP280 breakout) has
|
||||
no humidity sensor, unlike its BME280 sibling.
|
||||
- RD-03E driver (`rd03e.c`): the 5-byte "simple report" UART frame
|
||||
(`0xAA` header, gesture byte, little-endian distance, `0x55 0x55`
|
||||
footer) is reconstructed from a third-party bring-up write-up, not
|
||||
Ai-Thinker's own datasheet (not available while writing this) — **the
|
||||
single least-certain piece of code in this entire firmware.** The
|
||||
- RD-03E driver (`rd03e.c` + `rd03e_parse.c`): the **6-byte** "simple
|
||||
report" UART frame (`0xAA` header, gesture byte, little-endian distance
|
||||
low/high, `0x55 0x55` footer) is reconstructed from a third-party
|
||||
bring-up write-up, not Ai-Thinker's own datasheet (not available while
|
||||
writing this) — **the single least-certain piece of this entire
|
||||
firmware.** The parser itself is now host-tested (above); the *format it
|
||||
parses* is still a reconstruction, and that is the risk that remains. The
|
||||
gesture byte's exact value-to-meaning mapping is unconfirmed, so the
|
||||
driver reports it as a raw code in `metadata` rather than guessing at a
|
||||
translated label. Cross-confirmed from multiple sources: 256000 baud,
|
||||
8N1 UART framing.
|
||||
- I2S MEMS microphone driver (`mems_mic.c`): uses ESP-IDF's current
|
||||
- I2S MEMS microphone driver (`mems_mic.c` + `mems_level.c`): uses ESP-IDF's current
|
||||
`driver/i2s_std.h` API (standard/Philips mode, mono, 32-bit slot). The
|
||||
24-bit-in-32-bit-slot right-shift and dBFS reference level are the
|
||||
commonly-documented values for the INMP441 family this module's pinout
|
||||
@@ -156,25 +243,41 @@ consistent; no known syntax errors or obviously-wrong API usage):
|
||||
|
||||
**NOT verified — requires real hardware bring-up:**
|
||||
|
||||
None of the following is touched by `run_tests.sh`. The host tests cover
|
||||
arithmetic and byte handling; everything in this list is about wiring,
|
||||
timing, and what the silicon actually does.
|
||||
|
||||
- `idf.py build` has never actually been run in this environment (no
|
||||
ESP-IDF toolchain installed here) — there could be a typo, a missing
|
||||
include, or an API signature mismatch against whatever exact ESP-IDF
|
||||
version you build with that only a real compile will surface.
|
||||
version you build with that only a real compile will surface. The host
|
||||
harness deliberately does **not** compile `rd03e.c` / `bmp280.c` /
|
||||
`mems_mic.c` (they need ESP-IDF headers), so it cannot catch this.
|
||||
- The firmware has still **never been flashed to a board.** Nothing below
|
||||
has been observed; it has only been reasoned about.
|
||||
- I2C timing/electricals: pull-up resistor values, bus speed headroom,
|
||||
cable length — none of this has been bench-tested.
|
||||
- BMP280 compensation formula correctness in practice: the math is
|
||||
transcribed carefully, but "matches the datasheet" and "produces a
|
||||
plausible number when this exact C runs on this exact silicon" are
|
||||
different claims until someone compares a real reading to a reference
|
||||
thermometer/barometer.
|
||||
- RD-03E frame parsing, as above — this one especially, since the frame
|
||||
format itself (not just the implementation) is reconstructed from a
|
||||
third-party source rather than an official datasheet. Verify against a
|
||||
- BMP280 readings in practice: the maths now reproduces the datasheet's own
|
||||
worked reference values on a host, but "matches the datasheet's reference
|
||||
numbers" and "a real BMP280 on this bus returns the register contents we
|
||||
assume, and the result matches a reference thermometer/barometer" are
|
||||
different claims. Also unverified: the forced-mode `ctrl_meas` value, the
|
||||
status-polling loop, the reset delay, and the burst-read register
|
||||
addresses — all of that is I2C traffic the host tests never execute.
|
||||
- The RD-03E **frame format itself** — not the parser, which is now
|
||||
host-tested, but the reconstruction it implements. This is the gap the
|
||||
tests cannot close: they assert the parser matches the format in
|
||||
`rd03e_parse.h`, and that format came from a third-party write-up rather
|
||||
than an official datasheet. If it is wrong, the tests pass and the
|
||||
readings are garbage. Verify against a
|
||||
logic analyzer capture before trusting field values, and treat the
|
||||
gesture code's meaning as genuinely unknown until cross-checked.
|
||||
- I2S mic timing/levels: the BCLK/WS timing relationship, whether the
|
||||
- I2S mic timing/levels: the BCLK/WS timing relationship, and whether the
|
||||
24-bit-in-32-bit-slot shift is exactly right for this specific module
|
||||
revision, and whether the dBFS numbers land in a sane, usable range —
|
||||
revision. The host tests prove the shift-and-RMS arithmetic is
|
||||
self-consistent; they say nothing about whether shifting by 8 is the
|
||||
correct alignment for the real bitstream, or whether the dBFS numbers
|
||||
land in a sane, usable range once a real mic is feeding them —
|
||||
none of this has been bench-tested. Confirm by talking near the mic and
|
||||
checking the reported level actually rises before trusting it unattended.
|
||||
- Wi-Fi reconnect behavior under real-world conditions (router reboot,
|
||||
|
||||
Reference in New Issue
Block a user