From e1f8a99c0e9ff4c1ef1dba521ccf8abcaf99968b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 19:24:34 +0000 Subject: [PATCH 1/3] =?UTF-8?q?chore:=20cut=20v4.1.0=20=E2=80=94=20version?= =?UTF-8?q?=20stamp,=20CHANGELOG=20cut,=20channel-accurate=20release=20not?= =?UTF-8?q?es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FIRMWARE_VERSION 4.0.0 -> 4.1.0 (matches the v4.1.0 tag to come). MINOR is right: new settings and device behaviour, nothing removed and no format break — track files, the DOVEX layout, log filenames and the BLE command protocol are byte-for-byte what 4.0.0 wrote. CHANGELOG: [Unreleased] had accreted the same shape as last cut — two separate "### Changed" headings and a trailing "### Added" after "### Fixed" from successive merges. Consolidated into one Added/Changed/Fixed set under [4.1.0] - 2026-08-22. All 20 entries and every body line moved VERBATIM (verified by diffing the sorted line multiset before and after); only the five entries below gained a channel marker. The marker is the substantive change here. Most of this release's headline work — the whole NeoPixel subsystem — is behind BIRDSEYE_ENABLE_NEOPIXEL, which is off in the published images, so a reader of these notes would otherwise go looking for LEDs that a release build never lights. Each affected entry is now marked *(beta channel)*, and the section intro says plainly which half of the release is live for every user. Two plan-0007 behaviours are NOT gated and were being described as if they were: the tach page's corrected *OVER REV* header (display_pages.ino reads settingOverrevLimit outside any #if) and the pace page's STOPPED state (raceEngineStopped(), same) both ship active — those entries now say *(every build)* on that half and *(beta channel)* on the LED half. Link refs: [Unreleased] was still comparing from v3.1.0, stale since the 4.0.0 cut, and [4.0.0] never got a ref at all. Both fixed, [4.1.0] added. DovesLapTimer pins are deliberately untouched. compile-sketch.yml warns that promoting the channel means bumping them, but the library's BETA branch is source-identical to the v4.3.0 tag master already pins (the only difference between the two refs is five CI workflow files), so there is nothing to bump. Verified: 507 host test cases / 325,581 assertions pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U6kd8xwnUbXyd2Ce6m8ydX --- BirdsEye/project.h | 4 +-- CHANGELOG.md | 75 ++++++++++++++++++++++++++++++---------------- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/BirdsEye/project.h b/BirdsEye/project.h index 9d8ec19..907646e 100644 --- a/BirdsEye/project.h +++ b/BirdsEye/project.h @@ -31,10 +31,10 @@ #ifdef FIRMWARE_VERSION_OVERRIDE #define FIRMWARE_VERSION _BE_TOSTRING(FIRMWARE_VERSION_OVERRIDE) #else - // The 4.0.0 release cut (matches the v4.0.0 tag). The webapp still keys + // The 4.1.0 release cut (matches the v4.1.0 tag). The webapp still keys // the track JSON budget off this — 8 KB at or above 3.2.0 — so never // stamp a build below that line again. - #define FIRMWARE_VERSION "4.0.0" + #define FIRMWARE_VERSION "4.1.0" #endif /////////////////////////////////////////// diff --git a/CHANGELOG.md b/CHANGELOG.md index 4544d7a..5f5b407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,30 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 ## [Unreleased] -Slated to release as **4.1.0** (minor — new NeoPixel subsystem on the -beta channel, plus the fixes below). +Nothing yet. + +## [4.1.0] - 2026-08-22 + +MINOR — new settings and device behaviour, all backwards compatible. Track +files, the DOVEX log format, the log filenames and the BLE command protocol +are byte-for-byte unchanged from 4.0.0, so 4.0.0 logs, tracks and companion +apps keep working. + +**Which of this is live in the release firmware.** The NeoPixel strip +(subsystem 16) and the SensorEgg EGT POC (subsystem 14) sit behind the +`BIRDSEYE_ENABLE_NEOPIXEL` / `BIRDSEYE_ENABLE_SENSOREGG` build flags, and +those flags are **off** in the published `BirdsEye-sense` / +`BirdsEye-nonsense` images — a flag-on build performs a one-way +NFC-pads-to-GPIO conversion on hardware that mostly has no LEDs wired, so it +is not something to push to the whole fleet. Entries below marked +*(beta channel)* therefore ship as code in this release but are only active +in a beta build. Everything else is live for every user: the Bluetooth +download speed-up, the RPM filter rework, the `tach_filter` and pickup-health +diagnostics, the tach page's corrected `*OVER REV*` header, the pace page's +`STOPPED` state, the four new Back/Cancel menu rows, and the transfer-mode +exit fixes. The new LED settings keys are written to `/SETTINGS.json` on +every build (so a card moved into a beta unit is already configured); only a +flag-on build reads them. ### Added - **NeoPixel LED strip subsystem** (beta channel only, @@ -41,12 +63,14 @@ beta channel, plus the fixes below). every filename stay exactly as they were, and timezone presentation remains the viewing app's job. - **Overrev alert** (plan 0007): new `overrev_limit` setting (default 0 - = disabled) — past it the whole LED chain flashes red until RPM falls - back below the normal `rev_limit`. The rev limit warns the engine is - at its ceiling; the overrev limit says it's broken. -- **Temp1 alert threshold setting** (`temp1_alert_c`, default 650 °C): - the right status LED is now a tri-state — flashing red at/above the - limit, off when good, solid blue when there is no probe signal. + = disabled) — past it the whole LED chain flashes red *(beta channel)* + and the tach page's `*OVER REV*` header trips *(every build)*, both + until RPM falls back below the normal `rev_limit`. The rev limit warns + the engine is at its ceiling; the overrev limit says it's broken. +- **Temp1 alert threshold setting** *(beta channel)* (`temp1_alert_c`, + default 650 °C): the right status LED is now a tri-state — flashing red + at/above the limit, off when good, solid blue when there is no probe + signal. - **`tach_filter` setting** (default `smooth`, plan 0009): picks the RPM estimator, so the tach can be A/B'd against a live engine at the track instead of argued about from a plotted log. `smooth` is the new filter @@ -60,9 +84,14 @@ beta channel, plus the fixes below). thrown away this power-cycle. A count that climbs with RPM is ignition ringing or missed sparks reaching the ISR, i.e. the pickup rather than the filter. -- **GPS-search pip**: in race mode without a full GPS lock the strip - shows a green pixel bouncing end-to-end instead of the RPM scale, so - a not-yet-timing session is visibly "searching". +- **GPS-search pip** *(beta channel)*: in race mode without a full GPS + lock the strip shows a green pixel bouncing end-to-end instead of the + RPM scale, so a not-yet-timing session is visibly "searching". +- **The browser-sim harness can fake a GPS fix.** A "GPS fix" toggle (plus + an mph field) streams a deterministic synthetic 25 Hz fix parked on the + bundled OKC track's start line, so fix-gated flows — most usefully the + on-device course creator, including its 3 s point-averaging hold — can be + exercised in the simulator without loading a log file. ### Changed - **Settings file and JSON document buffers raised 512 -> 1024 bytes** @@ -100,16 +129,15 @@ beta channel, plus the fixes below). a flat ~20 RPM (worst case under 300) from 1500 to 14 000 RPM. The cost is ~90 ms more lag on a 5500 RPM/s pull. Set `tach_filter` to `legacy` to get the old behaviour back. -- **Engine dies mid-session** (tach-proven sessions): the LED bar goes - dark and the pace page shows `STOPPED` instead of a still-counting - pace — status LEDs (temp alert) stay live. Clears on restart. +- **Engine dies mid-session** (tach-proven sessions): the pace page shows + `STOPPED` instead of a still-counting pace *(every build)*, and the LED + bar goes dark while the status LEDs (temp alert) stay live + *(beta channel)*. Clears on restart. - **Tach page `*OVER REV*` header** now means actual overrev: it shows only when `overrev_limit` is enabled and RPM reaches it (was a hardcoded 9999 RPM). The `rev_limit` warning stays on the LED only. -- **Temp status LED flashes red** (was orange), matching the rev - flasher's alert language. - -### Changed +- **Temp status LED flashes red** *(beta channel)* (was orange), matching + the rev flasher's alert language. - **Bluetooth downloads are faster, and now say why when they are not** (plan 0008). A 3.3 MB session downloading at 28.8 KB/s on an iPad prompted a look at the whole transfer path. Three things were capping @@ -179,13 +207,6 @@ beta channel, plus the fixes below). (16 columns after the `Temp1`/`Junction1`/`Temp2` additions) injected nothing. It now accepts 13+ and reads the stable first 13. -### Added -- **The browser-sim harness can fake a GPS fix.** A "GPS fix" toggle (plus - an mph field) streams a deterministic synthetic 25 Hz fix parked on the - bundled OKC track's start line, so fix-gated flows — most usefully the - on-device course creator, including its 3 s point-averaging hold — can be - exercised in the simulator without loading a log file. - ## [4.0.0] - 2026-08-10 ### Added @@ -1419,7 +1440,9 @@ Initial tagged release. Core capabilities: - 8+ OLED display pages, Bluetooth LE file download / settings / track sync, and a low-power sleep mode. -[Unreleased]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v3.1.0...HEAD +[Unreleased]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v4.1.0...HEAD +[4.1.0]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v4.0.0...v4.1.0 +[4.0.0]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v3.1.0...v4.0.0 [3.1.0]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v3.0.2...v3.1.0 [3.0.2]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v3.0.1...v3.0.2 [3.0.1]: https://github.com/TheAngryRaven/DovesDataLogger/compare/v3.0.0...v3.0.1 From 938e45dd02f1736b643a5b117d4b76c80ffa1cde Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:58:17 +0000 Subject: [PATCH 2/3] =?UTF-8?q?feat:=20ship=20the=20NeoPixel=20strip=20in?= =?UTF-8?q?=20every=20build=20=E2=80=94=20BIRDSEYE=5FENABLE=5FNEOPIXEL=20d?= =?UTF-8?q?efaults=20to=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LED strip stops being a beta-only build and becomes a core feature in 4.1.0: a stock logger drives a strip the moment someone wires one, with no special firmware. The flag's #ifndef default in project.h goes 0 -> 1, which is the whole mechanism — every workflow already installs the Adafruit NeoPixel library, release.yml passes no feature flags at all, and compile-sketch.yml's non-BETA arm passes an empty FEATURE_FLAGS, so all three channels pick the new default up on their own. beta.yml's explicit -DBIRDSEYE_ENABLE_NEOPIXEL=1 is now redundant but harmless, and left alone. WHAT THIS COSTS, stated plainly because no later firmware can undo it: the subsystem's first boot programs UICR->NFCPINS to convert the two NFC pads to GPIO and self-resets once so the latch takes effect. That is a ONE-WAY change — reversing it needs a full chip erase and a bootloader reflash over USB — and with the flag on by default it is charged to EVERY device that installs 4.1.0, wired for LEDs or not, since the firmware cannot tell the difference. The trade was made deliberately: this hardware never uses NFC, the pads are otherwise idle, and gating a headline feature behind a separate build had kept it out of everyone's hands. Users who want the pads for something else must not install 4.1.0, so the CHANGELOG leads its 4.1.0 section with an upgrade note saying exactly that, including the extra reboot to expect. No other translation unit changes: grep confirms `#if BIRDSEYE_ENABLE_NEOPIXEL` appears nowhere outside neopixel.ino, so nothing else compiles differently and the simulator (which excludes that file) is untouched — 6/6 sim ctest still green with the flag flipped, goldens and both lap oracles included. Docs follow the decision rather than trailing it: the CHANGELOG's channel note is replaced by the upgrade warning and the per-entry "(beta channel)" markers come off the LED entries (they are not beta any more); CLAUDE.md subsystem 16, its flags list, constants table, File Map intro and workflows row; the ARCHITECTURE LED bullet; CONTRIBUTING's flag table (now noting this is the one flag defaulting to 1); and compile-sketch.yml's channel comment, where the difference between the two arms is now just SensorEgg and the library ref. Two in-code comments still calling the strip beta-only are corrected too. Verified: 507 host test cases / 325,581 assertions and 6/6 sim ctest. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U6kd8xwnUbXyd2Ce6m8ydX --- .github/workflows/compile-sketch.yml | 5 +- ARCHITECTURE.md | 6 ++- BirdsEye/BirdsEye.ino | 6 ++- BirdsEye/project.h | 36 +++++++++----- BirdsEye/settings.ino | 6 +-- CHANGELOG.md | 72 +++++++++++++++------------- CLAUDE.md | 38 +++++++++------ CONTRIBUTING.md | 2 +- 8 files changed, 102 insertions(+), 69 deletions(-) diff --git a/.github/workflows/compile-sketch.yml b/.github/workflows/compile-sketch.yml index dbd9e92..5e07000 100644 --- a/.github/workflows/compile-sketch.yml +++ b/.github/workflows/compile-sketch.yml @@ -37,7 +37,10 @@ jobs: # SensorEgg POC and the NeoPixel strip exactly as beta.yml does, so # the flag-on build is compile-checked on the PR rather than first # failing on the publish workflow. Everything else builds the - # master/release defaults (off). + # project.h defaults — which since 4.1.0 means the NeoPixel strip is + # ON (it is a core feature now; the flag defaults to 1) and only the + # SensorEgg POC is off. So the difference between the two arms is + # now just SensorEgg and the DovesLapTimer ref. FEATURE_FLAGS: ${{ (github.base_ref == 'BETA' || github.head_ref == 'BETA' || github.ref_name == 'BETA') && '-DBIRDSEYE_ENABLE_SENSOREGG=1 -DBIRDSEYE_ENABLE_NEOPIXEL=1' || '' }} # Build both XIAO nRF52840 variants. The Sense board has the onboard # LSM6DS3 IMU; the plain board does not (accelerometer logging degrades diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 05f16b3..ccdc70c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -142,8 +142,10 @@ to the matching `*_LOOP()`. `led_brightness` by day, `led_brightness_night` after dark (see *Local time* below). The strip's 5 V boost converter has its EN pin driven low in sleep, so System OFF really - powers the LEDs down. Gated on `BIRDSEYE_ENABLE_NEOPIXEL`: on in - beta, off (fully compiled out, no UICR write) in master/release. + powers the LEDs down. `BIRDSEYE_ENABLE_NEOPIXEL` is on in **every** + channel as of 4.1.0, which is what makes the strip a core feature — at + the price of a one-way, fleet-wide UICR NFC→GPIO conversion on the + first boot after updating. - **Course creator** (`course_creator` + `track_json` pure units, glued into the menu/pages/SD modules) — authors a track course on the device by walking to each cone and holding for a 3 s GPS average. Autocross diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index 3f13b6f..cc6bfec 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -1078,8 +1078,10 @@ void setup() { // beta channel): the one-time UICR NFC->GPIO write needs direct NVMC // access, which is illegal once the SoftDevice is up. Also before // wdtSetup() so the one-time self-reset can't race the watchdog. Needs - // SETTINGS_SETUP (led_brightness) — a no-op unless - // BIRDSEYE_ENABLE_NEOPIXEL is set (beta channel only). + // SETTINGS_SETUP (led_brightness). Since 4.1.0 this runs on EVERY + // channel — BIRDSEYE_ENABLE_NEOPIXEL defaults to 1 — so the first boot + // of any 4.1.0+ image is the one that spends the NFC pads and resets + // once. See project.h. NEOPIXEL_SETUP(); // Camera auto-record: load the persisted Insta360 serial + init the FSM diff --git a/BirdsEye/project.h b/BirdsEye/project.h index 907646e..3a82384 100644 --- a/BirdsEye/project.h +++ b/BirdsEye/project.h @@ -84,21 +84,31 @@ // ---- NeoPixel strip (11 px: 2 status + 9-px pace/RPM strip) ---- // -// 0 (default — master and release): the whole subsystem is compiled out. -// The module's entry points become no-ops and, critically, the firmware -// NEVER writes UICR->NFCPINS and never drives pins 30/31 (P0.09/P0.10, -// the NFC pads) — a flag-off build leaves the pads exactly as it found -// them. +// 1 (default — master, beta and release all ship this as of 4.1.0): the +// strip is a CORE feature, present in every image so a logger works the +// moment someone wires LEDs to it. On first boot NEOPIXEL_SETUP() +// converts the NFC pads to GPIO by programming UICR->NFCPINS and +// self-resets once so the pin latch takes effect. After that: pin 30 = +// boost converter EN, pin 31 = WS2812 data. See plan 0006 and neopixel.h. // -// 1 (the beta channel passes -DBIRDSEYE_ENABLE_NEOPIXEL=1): on first -// boot NEOPIXEL_SETUP() converts the NFC pads to GPIO by programming -// UICR->NFCPINS (a ONE-WAY change — undoing it needs a full chip erase, -// i.e. a bootloader reflash; accepted, NFC is never used on this -// hardware) and self-resets once so the pin latch takes effect. After -// that: pin 30 = boost converter EN, pin 31 = WS2812 data. See plan -// 0006 and neopixel.h. +// KNOW WHAT THIS COSTS, because it is charged to every unit in the field, +// not just the ones with LEDs on them. The UICR write is ONE-WAY — undoing +// it needs a full chip erase, i.e. a bootloader reflash over USB — so the +// first boot after updating to 4.1.0 permanently spends the NFC pads and +// reboots itself once, on every device, wired for LEDs or not. That was the +// deliberate 4.1.0 decision (NFC is not used on this hardware and the pads +// are otherwise idle); it is recorded here rather than in a commit message +// because nothing about a later build can undo it. +// +// 0 (no shipped channel sets this; -DBIRDSEYE_ENABLE_NEOPIXEL=0 forces it): +// the subsystem is compiled out — no Adafruit_NeoPixel dependency, and on a +// board that has NOT already been converted the firmware never writes UICR +// and never drives pins 30/31. On one that HAS (it ran a flag-on build +// before), the stubs still hold the boost EN pin low, because a floating EN +// leaves the 5 V rail up through System OFF — see the #else block in +// neopixel.ino. #ifndef BIRDSEYE_ENABLE_NEOPIXEL - #define BIRDSEYE_ENABLE_NEOPIXEL 0 + #define BIRDSEYE_ENABLE_NEOPIXEL 1 #endif /////////////////////////////////////////// diff --git a/BirdsEye/settings.ino b/BirdsEye/settings.ino index 876378a..02ec442 100644 --- a/BirdsEye/settings.ino +++ b/BirdsEye/settings.ino @@ -139,9 +139,9 @@ static void ensureDefaultSettings() { // starts the rotation at the speed page — end users never see the // diagnostic counters; "show" restores them for development/tuning. { "debug_pages", "hide" }, - // NeoPixel strip (plan 0006). Written on every channel so - // SETTINGS.json has a uniform shape (SensorEgg DOVEX-column - // precedent); only a BIRDSEYE_ENABLE_NEOPIXEL build reads them. + // NeoPixel strip (plan 0006). Read on every channel since 4.1.0 — + // BIRDSEYE_ENABLE_NEOPIXEL defaults to 1, so these are live + // settings on a stock logger, not beta-only bookkeeping. { "led_brightness", "64" }, // global cap 0-255; 0 = LEDs disabled { "rev_limit", "15000" }, // true RPM: LED scale ceiling + rev flasher + OVER REV header // Plan 0007: the PROBLEM limit (0 = disabled) — whole LED chain diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f5b407..6c8a712 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,25 +21,36 @@ files, the DOVEX log format, the log filenames and the BLE command protocol are byte-for-byte unchanged from 4.0.0, so 4.0.0 logs, tracks and companion apps keep working. -**Which of this is live in the release firmware.** The NeoPixel strip -(subsystem 16) and the SensorEgg EGT POC (subsystem 14) sit behind the -`BIRDSEYE_ENABLE_NEOPIXEL` / `BIRDSEYE_ENABLE_SENSOREGG` build flags, and -those flags are **off** in the published `BirdsEye-sense` / -`BirdsEye-nonsense` images — a flag-on build performs a one-way -NFC-pads-to-GPIO conversion on hardware that mostly has no LEDs wired, so it -is not something to push to the whole fleet. Entries below marked -*(beta channel)* therefore ship as code in this release but are only active -in a beta build. Everything else is live for every user: the Bluetooth -download speed-up, the RPM filter rework, the `tach_filter` and pickup-health -diagnostics, the tach page's corrected `*OVER REV*` header, the pace page's -`STOPPED` state, the four new Back/Cancel menu rows, and the transfer-mode -exit fixes. The new LED settings keys are written to `/SETTINGS.json` on -every build (so a card moved into a beta unit is already configured); only a -flag-on build reads them. +> ### Read this before updating +> +> **This release permanently converts the two NFC pads to GPIO, on every +> device, and reboots once while doing it.** +> +> The LED strip stops being a beta-only experiment in 4.1.0 and ships in the +> normal firmware, so a logger lights up the moment someone wires a strip to +> it — no special build. Making those two pads usable as GPIO means writing +> the chip's UICR, and **that write cannot be undone by any later firmware**; +> reversing it needs a full chip erase and a bootloader reflash over USB. +> Your logger does the write the first time it boots after updating, then +> resets itself once so the change takes effect. That single extra reboot +> during the update is expected — nothing is wrong. +> +> This happens whether or not you have LEDs attached, because the firmware +> cannot know. The trade was made deliberately: this hardware does not use +> NFC for anything, the pads are otherwise idle, and requiring a separate +> build to use a headline feature had kept it out of everyone's hands. If you +> have some future use for the NFC pads on your device, **do not install +> 4.1.0.** +> +> With no strip wired, nothing else changes — the pins simply sit there. + +The SensorEgg wireless-EGT proof of concept remains beta-only +(`BIRDSEYE_ENABLE_SENSOREGG`, off in the published images), but it gained +nothing this release — so every entry below is live for every user. ### Added -- **NeoPixel LED strip subsystem** (beta channel only, - `BIRDSEYE_ENABLE_NEOPIXEL`, plan 0006): 11 WS2812 pixels on the NFC +- **NeoPixel LED strip subsystem** (plan 0006) — **now in every build**, + see the upgrade note above: 11 WS2812 pixels on the NFC pads converted to GPIO — 2 status indicators + a 9-px strip with a centerline. A global brightness cap (`led_brightness` setting, 0 disables the LEDs entirely) that no LED can ever exceed; a pace pip @@ -52,8 +63,7 @@ flag-on build reads them. sleep truly powers the LEDs off. **First boot of a flag-on build performs a one-way NFC-pads-to-GPIO conversion (UICR write) and resets once.** -- **LED day/night brightness + a device timezone** (beta channel, plan - 0010): new `utc_offset_min` setting (minutes east of UTC, default 0) +- **LED day/night brightness + a device timezone** (plan 0010): new `utc_offset_min` setting (minutes east of UTC, default 0) gives the device a local wall clock, and the LED strip swaps to `led_brightness_night` (default 16) between `led_night_start_hour` (19) and `led_day_start_hour` (7) — local hours, so 7am is the @@ -63,12 +73,12 @@ flag-on build reads them. every filename stay exactly as they were, and timezone presentation remains the viewing app's job. - **Overrev alert** (plan 0007): new `overrev_limit` setting (default 0 - = disabled) — past it the whole LED chain flashes red *(beta channel)* - and the tach page's `*OVER REV*` header trips *(every build)*, both - until RPM falls back below the normal `rev_limit`. The rev limit warns - the engine is at its ceiling; the overrev limit says it's broken. -- **Temp1 alert threshold setting** *(beta channel)* (`temp1_alert_c`, - default 650 °C): the right status LED is now a tri-state — flashing red + = disabled) — past it the whole LED chain flashes red and the tach + page's `*OVER REV*` header trips, both until RPM falls back below the + normal `rev_limit`. The rev limit warns the engine is at its ceiling; + the overrev limit says it's broken. +- **Temp1 alert threshold setting** (`temp1_alert_c`, default 650 °C): + the right status LED is now a tri-state — flashing red at/above the limit, off when good, solid blue when there is no probe signal. - **`tach_filter` setting** (default `smooth`, plan 0009): picks the RPM @@ -84,8 +94,7 @@ flag-on build reads them. thrown away this power-cycle. A count that climbs with RPM is ignition ringing or missed sparks reaching the ISR, i.e. the pickup rather than the filter. -- **GPS-search pip** *(beta channel)*: in race mode without a full GPS - lock the strip shows a green pixel bouncing end-to-end instead of the +- **GPS-search pip**: in race mode without a full GPS lock the strip shows a green pixel bouncing end-to-end instead of the RPM scale, so a not-yet-timing session is visibly "searching". - **The browser-sim harness can fake a GPS fix.** A "GPS fix" toggle (plus an mph field) streams a deterministic synthetic 25 Hz fix parked on the @@ -130,14 +139,13 @@ flag-on build reads them. is ~90 ms more lag on a 5500 RPM/s pull. Set `tach_filter` to `legacy` to get the old behaviour back. - **Engine dies mid-session** (tach-proven sessions): the pace page shows - `STOPPED` instead of a still-counting pace *(every build)*, and the LED - bar goes dark while the status LEDs (temp alert) stay live - *(beta channel)*. Clears on restart. + `STOPPED` instead of a still-counting pace, and the LED bar goes dark + while the status LEDs (temp alert) stay live. Clears on restart. - **Tach page `*OVER REV*` header** now means actual overrev: it shows only when `overrev_limit` is enabled and RPM reaches it (was a hardcoded 9999 RPM). The `rev_limit` warning stays on the LED only. -- **Temp status LED flashes red** *(beta channel)* (was orange), matching - the rev flasher's alert language. +- **Temp status LED flashes red** (was orange), matching the rev + flasher's alert language. - **Bluetooth downloads are faster, and now say why when they are not** (plan 0008). A 3.3 MB session downloading at 28.8 KB/s on an iPad prompted a look at the whole transfer path. Three things were capping diff --git a/CLAUDE.md b/CLAUDE.md index b070ef7..4588b86 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,7 @@ Core capabilities: DovesSensorEgg thermocouple pod's advertising broadcasts (`PW-ADV` v1 and v2), logs `Temp1`/`Junction1`/`Temp2` DOVEX columns + Temp1/Temp2 race pages (subsystem 14) -- **NeoPixel strip (beta)**: 11 WS2812 pixels on the NFC pads converted +- **NeoPixel strip**: 11 WS2812 pixels on the NFC pads converted to GPIO — 2 status alert LEDs + a 9-px pace-pip / RPM-scale strip with a global brightness cap, boot animation, and a purple session-best sector celebration (subsystem 16) @@ -176,7 +176,7 @@ handoff spec. | Path | Contents | |---|---| -| `.github/workflows/` | CI: compile-sketch (+ flash-size gate), arduino-lint, unit-tests, clang-tidy, coverage, sim-build (native sim TU + 60 s boot soak + determinism + goldens + lap oracles + two-session carryover, plus a wasm job: emsdk 3.1.61 build + node smoke + `birdseye-sim-wasm` artifact), release (dual-board build + GitHub Release + prod OTA manifest to `gh-pages`), beta (dual-board build on `BETA`-branch push → latest-only `beta/` OTA channel on `gh-pages`, no Release). Per-channel build config: `BETA` builds track DovesLapTimer's `BETA` branch and pass `-DBIRDSEYE_ENABLE_SENSOREGG=1`; master/release pin `v4.3.0` and build the all-flags-off defaults | +| `.github/workflows/` | CI: compile-sketch (+ flash-size gate), arduino-lint, unit-tests, clang-tidy, coverage, sim-build (native sim TU + 60 s boot soak + determinism + goldens + lap oracles + two-session carryover, plus a wasm job: emsdk 3.1.61 build + node smoke + `birdseye-sim-wasm` artifact), release (dual-board build + GitHub Release + prod OTA manifest to `gh-pages`), beta (dual-board build on `BETA`-branch push → latest-only `beta/` OTA channel on `gh-pages`, no Release). Per-channel build config: `BETA` builds track DovesLapTimer's `BETA` branch and pass `-DBIRDSEYE_ENABLE_SENSOREGG=1`; master/release pin `v4.3.0` and build the `project.h` defaults — which since 4.1.0 means NeoPixel ON, SensorEgg off | | `tests/` | Host doctest harness (CMake) for the pure-logic units | | `docs/plans/` | Numbered design records (`NNNN-slug.md`, see its README) — the rationale behind each chunk of work; plan-executing commits cite the number. Same convention as DovesDataViewer | | `CHANGELOG.md` | Keep-a-Changelog history; release workflow ties to version tags | @@ -1312,12 +1312,17 @@ hardware needs no power switch. Wake = chip reset = fresh `setup()`. ### 16. NeoPixel Strip (`neopixel.{h,ino}`, `led_frame/led_modes/led_animations/sector_purple.{h,cpp}`) -- **BUILD FLAG — `BIRDSEYE_ENABLE_NEOPIXEL` (`project.h`)**: `0` - (master/release default) compiles the whole subsystem down to no-op - `NEOPIXEL_*` entry points — no UICR write, no pin driving, no - Adafruit NeoPixel dependency in the image. `1` (beta channel, passed - by `beta.yml` + BETA-targeted `compile-sketch.yml` runs) is - everything below. +- **BUILD FLAG — `BIRDSEYE_ENABLE_NEOPIXEL` (`project.h`)**: `1` on + **every** channel as of 4.1.0 — master, beta and release — so the strip + is a core feature rather than a special build. Everything below is in + every image. The cost is charged fleet-wide and cannot be taken back: + the first boot after updating performs the ONE-WAY UICR NFC→GPIO + conversion and self-resets once, on every device, LEDs wired or not. + `0` (no shipped channel sets it) compiles the subsystem out — no + Adafruit NeoPixel dependency, and no UICR write on a board that has not + already been converted; on one that HAS, the `#else` stubs still hold + boost EN low, because a floating EN leaves the 5 V rail up through + System OFF. - **Hardware**: 11 WS2812 pixels fed by an Adafruit 5 V boost converter. Pixels 0 and 10 are status indicators; pixels 1–9 are the strip with pixel 5 the centerline. Pin 30 (P0.09/NFC1) drives the boost EN @@ -1699,7 +1704,7 @@ the one loaded). Sector lines stay optional — zero, one, or two. | SensorEgg scanner self-heal | 30 s no packet → stop+start kick | `sensoregg_protocol.h` | | SensorEgg RSSI floor | −90 dBm | `sensoregg_protocol.h` | | SensorEgg pairing MAC | `SENSOREGG_MAC` (all-zeros = any egg) | `sensoregg.h` | -| NeoPixel strip flag | `BIRDSEYE_ENABLE_NEOPIXEL`, default 0; 1 on the beta channel | `project.h` | +| NeoPixel strip flag | `BIRDSEYE_ENABLE_NEOPIXEL`, default **1** on every channel since 4.1.0 | `project.h` | | NeoPixel pins | 30 = boost EN, 31 = data (NFC pads, post-UICR) | `neopixel.h` | | NeoPixel layout | 11 px: status 0 + strip 1–9 (center px 5) + status 10 | `led_frame.h` | | LED frame rate | 30 Hz (`NPX_FRAME_INTERVAL_MS` 33) | `neopixel.ino` | @@ -1781,8 +1786,9 @@ This device operates in ignition-noise environments. Three layers of defense: `compiler.cpp.extra_flags` property — a second `--build-property` for one key replaces the first). Local setup: CONTRIBUTING.md "Local build flags". -- **Feature flags** (`project.h`, all default `0`, all tested with `#if` - so an explicit `-DFLAG=0` wins): +- **Feature flags** (`project.h`, tested with `#if` so an explicit + `-DFLAG=0` wins; `BIRDSEYE_ENABLE_NEOPIXEL` defaults to `1`, the rest + to `0`): - `BIRDSEYE_ENABLE_ONBOARD_CHARGING` — off in **every** channel. See subsystem 10: HICHG hold + the USB charging UX. The hardware now has an external charging circuit. @@ -1790,10 +1796,12 @@ This device operates in ignition-noise environments. Three layers of defense: (`beta.yml`, plus `compile-sketch.yml` for PRs targeting `BETA` so the flag-on build is compile-checked before it reaches the publish workflow). See subsystem 14. - - `BIRDSEYE_ENABLE_NEOPIXEL` — off in master/release, **on in beta** - (same two workflows as SENSOREGG). First flag-on boot performs the - ONE-WAY UICR NFC→GPIO conversion and self-resets once. See - subsystem 16. + - `BIRDSEYE_ENABLE_NEOPIXEL` — **on everywhere since 4.1.0** (the + `project.h` default; no workflow needs to pass it). The first boot of + any 4.1.0+ image performs the ONE-WAY UICR NFC→GPIO conversion and + self-resets once, on every device. This is the one flag whose default + is 1 — see subsystem 16 and the upgrade note at the top of + CHANGELOG.md's 4.1.0 section. When adding a flag: give it a `#ifndef` default in `project.h`, decide its per-channel value in the workflows, and document it here + in CONTRIBUTING.md's flag table. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 374b285..2bacb00 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,7 +80,7 @@ so an explicit `-DFLAG=0` still turns the feature off. |---|---|---|---| | `BIRDSEYE_ENABLE_ONBOARD_CHARGING` | `0` | nothing — off in every channel | Holds the BQ25100 HICHG pin high for ~100 mA fast charge and runs the USB charging UX (VBUS wake shortcuts to the charge screen; the main menu drops into the charging loop after `USB_MENU_CHARGE_IDLE_MS`). Off, the firmware leaves HICHG alone and an external charging circuit owns the battery. The VBUS park at shutdown happens either way — see ARCHITECTURE.md. | | `BIRDSEYE_ENABLE_SENSOREGG` | `0` | `beta.yml`, and `compile-sketch.yml` for PRs targeting `BETA` | Compiles in the wireless-EGT POC: passive BLE scanner, Temp1 race page, and BLE core up at boot. Off, the accessors return NaN, so `Temp1`/`Junction1` still log as `nan` and the log format is unchanged. | -| `BIRDSEYE_ENABLE_NEOPIXEL` | `0` | `beta.yml`, and `compile-sketch.yml` for PRs targeting `BETA` | Compiles in the NeoPixel strip subsystem (plan 0006): on first boot it programs `UICR->NFCPINS` to convert the NFC pads to GPIO (**one-way** — undoing it needs a full chip erase) and self-resets once, then drives pin 30 as the 5 V boost EN and pin 31 as WS2812 data. Off, the firmware never touches UICR or those pins. Needs the `Adafruit NeoPixel` library. | +| `BIRDSEYE_ENABLE_NEOPIXEL` | **`1`** | every channel (the `project.h` default — no workflow passes it) | Compiles in the NeoPixel strip subsystem (plan 0006): on first boot it programs `UICR->NFCPINS` to convert the NFC pads to GPIO (**one-way** — undoing it needs a full chip erase) and self-resets once, then drives pin 30 as the 5 V boost EN and pin 31 as WS2812 data. **The only flag defaulting to 1**, since 4.1.0 — that conversion is therefore charged to every device in the field, LEDs wired or not. Forcing `=0` compiles it out; on a board already converted the stubs still hold boost EN low. Needs the `Adafruit NeoPixel` library. | ### arduino-cli The exact invocation CI uses is in From 9ff7c17cf1940ddd8efd592ffb1dd09cbb0a2073 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:35:40 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20release-review=20findings=20for=204.?= =?UTF-8?q?1.0=20=E2=80=94=20BLE=20settings=20list,=20LED=20rail,=20strict?= =?UTF-8?q?=20setting=20parse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six defects found reviewing the BETA -> master promotion diff. Each restores behaviour the code already claimed; none adds a feature. BLE `SLIST` was broken on every 4.1.0 device (release build, not flag-gated). /SETTINGS.json is parsed in TWO places, and only one was raised by plan 0010: settings.ino went 512 -> 1024 because the nine new keys take the default file from 329 to 538 bytes, while bluetooth.ino's SLIST handler kept a 512-byte buffer and a <512> document. It read 511 bytes of 538, deserializeJson() returned IncompleteInput, and the handler answered SERR:PARSE without emitting a single SVAL: line — the companion app's settings screen came up empty. SGET/SSET were unaffected (they route through settings.ino). Both parsers are now sized by SETTINGS_JSON_CAPACITY in settings.h, so they cannot drift again; the SLIST pair is static rather than stack, matching sd_functions.ino's "keeps JSON_BUFFER_SIZE off the stack" idiom now that it is 2 KB. A beta-converted board updating to a release build never powered its LED rail down. The flag-off stubs did nothing at all, by design — correct for virgin hardware, wrong for a board carrying the one-way UICR NFC->GPIO conversion an earlier beta build performed. There P0.09 (boost EN) was left in reset state (input, disconnected) for the whole session and through System OFF, where the driven level is the only thing holding the rail down (same retention as the "blue conn LED stays on after sleep" report). A floating EN reads as enabled, so "off" kept the 5 V rail and 11 idle WS2812s alive on a device with no power switch. NEOPIXEL_SETUP/NEOPIXEL_SLEEP now drive EN low, but ONLY when UICR->NFCPINS shows the pads are already GPIO — an unconverted board is still never touched, so the flag's promise holds exactly. Numeric settings were parsed with atoi(), which answers 0 for "" and for "garbage". For rev_limit and temp1_alert_c that is harmless (0 is below their floors), but for every key plans 0006/0010 added, 0 is IN range — and for led_brightness it means "LEDs off, never raise the 5 V rail". So a blank or corrupt value silently killed the strip instead of "clamping back to the compiled-in default per the house idiom" as its own comment claimed, and looked exactly like dead hardware. New host-tested setting_parse unit rejects anything that is not a complete integer, so the existing range check keeps the default; a deliberate 0 still works. evalStatus() strobed when handed clearBelow above threshold. rev_limit and overrev_limit clamp independently, so overrev_limit <= rev_limit * 0.97 put the overrev action's release point above its own trip point: the latch set on one frame and cleared on the next, flickering all 11 pixels at the 30 Hz frame rate instead of flashing at 100 ms. Fixed in the pure unit rather than at the call site so it covers the settings-driven assignability planned for phase 2, with a regression test. Pace page "STOPPED" was clipped: 8 chars at text size 3 is a 144 px advance on a 128 px panel. Seven chars at x=1 fits in 126 and centres. local_time.cpp was the only new pure unit missing from clang-tidy.yml, and it links into the release image (BirdsEye.ino calls isValidOffsetMinutes() unconditionally). Added, along with setting_parse.cpp. CLAUDE.md corrections found in the same pass: the NeoPixel library is not "linked in but inert" on a flag-off build, it is not compiled at all; the LED settings keys are written AND parsed on every channel, only their use is compiled out; the BLE manual-exit reboot was dated to a 4.0.1 release that does not exist; and subsystem 10's teardown order predated NEOPIXEL_SLEEP/WAKE. Subsystem 8 now documents that two parsers of the settings file exist. Verified: 515 host test cases / 325,646 assertions (up from 507/325,581 — 7 new setting_parse cases, 1 new led_modes case), and 6/6 sim ctest including the golden pixel fixtures and both lap oracles. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U6kd8xwnUbXyd2Ce6m8ydX --- .github/workflows/clang-tidy.yml | 2 + BirdsEye/BirdsEye.ino | 50 +++++++++++------ BirdsEye/bluetooth.ino | 16 +++++- BirdsEye/display_pages.ino | 7 ++- BirdsEye/led_modes.cpp | 12 +++- BirdsEye/neopixel.ino | 44 +++++++++++++-- BirdsEye/setting_parse.cpp | 56 +++++++++++++++++++ BirdsEye/setting_parse.h | 46 +++++++++++++++ BirdsEye/settings.h | 27 +++++++++ BirdsEye/settings.ino | 4 +- BirdsEye/sim/CMakeLists.txt | 1 + CHANGELOG.md | 36 ++++++++++++ CLAUDE.md | 31 +++++++++-- tests/CMakeLists.txt | 2 + tests/led_modes_test.cpp | 41 ++++++++++++++ tests/setting_parse_test.cpp | 96 ++++++++++++++++++++++++++++++++ 16 files changed, 437 insertions(+), 34 deletions(-) create mode 100644 BirdsEye/setting_parse.cpp create mode 100644 BirdsEye/setting_parse.h create mode 100644 tests/setting_parse_test.cpp diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 3c5e68a..24a693b 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -46,4 +46,6 @@ jobs: BirdsEye/led_modes.cpp \ BirdsEye/led_animations.cpp \ BirdsEye/sector_purple.cpp \ + BirdsEye/local_time.cpp \ + BirdsEye/setting_parse.cpp \ BirdsEye/ble_stream.cpp diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index cc6bfec..e0064c0 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -99,6 +99,7 @@ #include "haversine.h" #include "idle_policy.h" #include "local_time.h" +#include "setting_parse.h" #include "neopixel.h" #include "replay.h" #include "sat_bars.h" @@ -1018,25 +1019,38 @@ void setup() { } // NeoPixel strip (plan 0006). Both clamp back to the compiled-in // default on a missing or nonsense value, per the house idiom. - if (getSetting("led_brightness", buf, sizeof(buf))) { - const int b = atoi(buf); + // + // That idiom needs setting_parse::parseIntSetting, not atoi(): atoi + // answers 0 for "" and for "garbage", and for every setting below + // EXCEPT rev_limit and temp1_alert_c, 0 is inside the accepted range. + // led_brightness 0 disables the LEDs and never raises the 5 V boost + // rail, so a blank value read as a deliberate "off" and looked exactly + // like dead hardware. parseIntSetting rejects a non-integer outright, + // the range check then fails, and the compiled-in default stands. + int parsedSetting = 0; + if (getSetting("led_brightness", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int b = parsedSetting; if (b >= 0 && b <= 255) settingLedBrightness = (uint8_t)b; } - if (getSetting("rev_limit", buf, sizeof(buf))) { - const int r = atoi(buf); + if (getSetting("rev_limit", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int r = parsedSetting; // Floor keeps a garbled value from parking the scale at zero; // ceiling matches the tach filter's ~20k true-RPM limit. if (r >= 1000 && r <= 20000) settingRevLimit = r; } - if (getSetting("overrev_limit", buf, sizeof(buf))) { - const int r = atoi(buf); + if (getSetting("overrev_limit", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int r = parsedSetting; // 0 (the default) disables the whole-chain overrev flash; any // other value clamps to the same band as rev_limit. if (r == 0) settingOverrevLimit = 0; else if (r >= 1000 && r <= 20000) settingOverrevLimit = r; } - if (getSetting("temp1_alert_c", buf, sizeof(buf))) { - const int t = atoi(buf); + if (getSetting("temp1_alert_c", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int t = parsedSetting; // Celsius. Floor above any plausible ambient so a garbled value // can't latch the alert at power-on; ceiling past any real EGT. if (t >= 50 && t <= 1200) settingTemp1AlertC = t; @@ -1044,20 +1058,24 @@ void setup() { // Local time (plan 0010). The band is the pure unit's, not a literal // here, so ±14 h has one home. Out of band keeps the 0 default — // i.e. UTC — which is exactly the pre-0010 behaviour. - if (getSetting("utc_offset_min", buf, sizeof(buf))) { - const int o = atoi(buf); + if (getSetting("utc_offset_min", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int o = parsedSetting; if (local_time::isValidOffsetMinutes(o)) settingUtcOffsetMin = (int16_t)o; } - if (getSetting("led_brightness_night", buf, sizeof(buf))) { - const int b = atoi(buf); + if (getSetting("led_brightness_night", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int b = parsedSetting; if (b >= 0 && b <= 255) settingLedBrightnessNight = (uint8_t)b; } - if (getSetting("led_day_start_hour", buf, sizeof(buf))) { - const int h = atoi(buf); + if (getSetting("led_day_start_hour", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int h = parsedSetting; if (h >= 0 && h <= 23) settingLedDayStartHour = (uint8_t)h; } - if (getSetting("led_night_start_hour", buf, sizeof(buf))) { - const int h = atoi(buf); + if (getSetting("led_night_start_hour", buf, sizeof(buf)) && + setting_parse::parseIntSetting(buf, &parsedSetting)) { + const int h = parsedSetting; if (h >= 0 && h <= 23) settingLedNightStartHour = (uint8_t)h; } crossingThresholdMeters = settingLapDetectionDistance; diff --git a/BirdsEye/bluetooth.ino b/BirdsEye/bluetooth.ino index c5638b5..6af4d9d 100644 --- a/BirdsEye/bluetooth.ino +++ b/BirdsEye/bluetooth.ino @@ -8,6 +8,11 @@ #include "camera_ble.h" #include "filename_validator.h" #include "firmware_ota.h" +// For SETTINGS_JSON_CAPACITY and getSetting/setSetting. This module used +// them via Arduino's concatenation of BirdsEye.ino's includes; naming the +// dependency follows camera_ble.ino and keeps the SLIST buffer sizes tied +// to the settings module that owns them. +#include "settings.h" // Target connection interval in 1.25 ms units: 12 = 15 ms, the fastest an // Apple central is permitted to accept from an accessory. See bleTuneLink(). @@ -1040,7 +1045,13 @@ void processSettingsCommand() { return; } - char fileBuf[512]; + // The SECOND parser of /SETTINGS.json (settings.ino has the other). + // Both are sized by SETTINGS_JSON_CAPACITY so they can never drift + // again — see the comment on it in settings.h for what happened when + // they did. static, not stack: 2 KB in one frame is more than the loop + // task's budget wants, and it matches the house idiom in + // sd_functions.ino ("keeps JSON_BUFFER_SIZE off the stack"). + static char fileBuf[SETTINGS_JSON_CAPACITY]; int bytesRead = settingsFile.read(fileBuf, sizeof(fileBuf) - 1); settingsFile.close(); releaseSDAccess(SD_ACCESS_TRACK_PARSE); @@ -1056,7 +1067,8 @@ void processSettingsCommand() { } fileBuf[bytesRead] = '\0'; - StaticJsonDocument<512> doc; + static StaticJsonDocument doc; + doc.clear(); DeserializationError err = deserializeJson(doc, fileBuf); if (err != DeserializationError::Ok) { debug(F("BLE: SLIST - JSON parse error: ")); diff --git a/BirdsEye/display_pages.ino b/BirdsEye/display_pages.ino index 4107f54..cc23ebd 100644 --- a/BirdsEye/display_pages.ino +++ b/BirdsEye/display_pages.ino @@ -741,9 +741,12 @@ void displayPage_gps_pace() { display.setTextColor(DISPLAY_TEXT_WHITE); const int lineHeight = 21; if (engineStopped) { - display.setCursor(0, lineHeight); + // 7 chars, NOT 8 with a leading space: size 3 is an 18 px advance, so + // " STOPPED" needs 144 px on a 128 px panel and the trailing D was + // clipped on every render. 7 x 18 = 126 fits, and x=1 centres it. + display.setCursor(1, lineHeight); display.setTextSize(3); - display.print(F(" STOPPED")); + display.print(F("STOPPED")); } else if (sprintModeIsActive() && !activeTimerRunActive()) { // Sprint mode, between runs — no live pace to compare (see lap page). display.setCursor(0, lineHeight); diff --git a/BirdsEye/led_modes.cpp b/BirdsEye/led_modes.cpp index 2188f94..ed2190b 100644 --- a/BirdsEye/led_modes.cpp +++ b/BirdsEye/led_modes.cpp @@ -78,9 +78,19 @@ led_frame::Rgb evalStatus(const StatusAction& a, StatusState& s, float value, s.active = false; return a.invalidColor; } + // A caller can hand us clearBelow ABOVE threshold — overrev_limit and + // rev_limit clamp independently, so overrev_limit <= rev_limit * + // kRevClearFrac makes the overrev action's release point sit above its + // own trip point. Left alone, a value in that inverted band sets the + // latch on one frame and clears it on the next: a 15 Hz strobe of the + // whole chain instead of the intended 100 ms flash. A release point + // above the trip point is never meaningful, so collapse it — the action + // degrades to a plain threshold with no hysteresis, which is right. + const float clearBelow = + a.clearBelow > a.threshold ? a.threshold : a.clearBelow; if (!s.active && value >= a.threshold) { s.active = true; - } else if (s.active && value < a.clearBelow) { + } else if (s.active && value < clearBelow) { s.active = false; } if (!s.active) { diff --git a/BirdsEye/neopixel.ino b/BirdsEye/neopixel.ino index 8f366a8..876ea4c 100644 --- a/BirdsEye/neopixel.ino +++ b/BirdsEye/neopixel.ino @@ -337,15 +337,49 @@ void NEOPIXEL_WAKE() { #else // !BIRDSEYE_ENABLE_NEOPIXEL /////////////////////////////////////////// -// SUBSYSTEM COMPILED OUT (the master/release default — see project.h) +// SUBSYSTEM COMPILED OUT // -// No UICR write, no pin driving, no Adafruit_NeoPixel dependency in -// the image. The pads stay exactly as the chip shipped. +// Since 4.1.0 no shipped channel takes this branch — the flag defaults +// to 1 (see project.h). It is reached only by a build that forces +// -DBIRDSEYE_ENABLE_NEOPIXEL=0, which is why the already-converted case +// below matters more than it looks: the boards most likely to run such a +// build are ones that already ran a flag-on one. +// +// No UICR write, no Adafruit_NeoPixel dependency in the image. On a +// board that has never run a flag-on build the pads stay exactly as the +// chip shipped, and this file drives nothing at all. +// +// ONE exception, and it is a power bug if you remove it: a board that +// HAS run a flag-on build carries the one-way UICR NFC->GPIO conversion +// forever, and a later flag-off image (a beta unit updating to a prod +// release) inherits it. With stubs that truly do nothing, P0.09 — the +// boost converter's EN — is left in its reset state (input, disconnected) +// for the whole session AND through System OFF, where a driven-LOW level +// is the only thing that holds the rail down (the same retention that +// caused the "blue conn LED stays on after sleep" report, subsystem 10). +// EN floating on the Adafruit boost module reads as enabled, so "off" +// keeps the 5 V rail and 11 idle WS2812s alive on a device with no power +// switch — a flat pack in a day or two. +// +// So: drive EN low, but ONLY when the conversion has already happened. +// PROTECT clear (0) means the pads are already GPIO. On an unconverted +// board the bit is set, this is skipped, and the promise above holds +// exactly — we never touch a pad the user didn't opt into. /////////////////////////////////////////// -void NEOPIXEL_SETUP() {} +static void npxHoldConvertedBoostOff() { + if ((NRF_UICR->NFCPINS & UICR_NFCPINS_PROTECT_Msk) != 0) { + return; // pads still NFC — never touched by a flag-off build + } + pinMode(NEOPIXEL_PIN_BOOST_EN, OUTPUT); + digitalWrite(NEOPIXEL_PIN_BOOST_EN, LOW); +} + +void NEOPIXEL_SETUP() { npxHoldConvertedBoostOff(); } void NEOPIXEL_LOOP() {} -void NEOPIXEL_SLEEP() {} +// Re-assert before System OFF: the level is retained there, and that is +// the case that costs a battery rather than a few mA of run current. +void NEOPIXEL_SLEEP() { npxHoldConvertedBoostOff(); } void NEOPIXEL_WAKE() {} void neopixelNotifyPurpleSector() {} diff --git a/BirdsEye/setting_parse.cpp b/BirdsEye/setting_parse.cpp new file mode 100644 index 0000000..ea15cea --- /dev/null +++ b/BirdsEye/setting_parse.cpp @@ -0,0 +1,56 @@ +#include "setting_parse.h" + +#include + +namespace setting_parse { +namespace { + +bool isSpace(char c) { + return c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '\v' || + c == '\f'; +} + +} // namespace + +bool parseIntSetting(const char* s, int* out) { + if (s == nullptr || out == nullptr) { + return false; + } + const char* p = s; + while (isSpace(*p)) { + p++; + } + bool negative = false; + if (*p == '+' || *p == '-') { + negative = (*p == '-'); + p++; + } + if (*p < '0' || *p > '9') { + return false; // no digits at all — "" / " " / "abc" / "-" / "+" + } + // Accumulate in long long so overflow is detected rather than wrapped. + // A settings file is hand-editable, so "99999999999" must be rejected, + // not folded into some in-range value. + long long acc = 0; + while (*p >= '0' && *p <= '9') { + acc = acc * 10 + (*p - '0'); + if (acc > 4294967296LL) { + return false; // far past any int; stop before acc itself overflows + } + p++; + } + while (isSpace(*p)) { + p++; + } + if (*p != '\0') { + return false; // trailing junk — "12abc", "1.5", "0x10" + } + const long long value = negative ? -acc : acc; + if (value < (long long)INT_MIN || value > (long long)INT_MAX) { + return false; + } + *out = (int)value; + return true; +} + +} // namespace setting_parse diff --git a/BirdsEye/setting_parse.h b/BirdsEye/setting_parse.h new file mode 100644 index 0000000..12f9579 --- /dev/null +++ b/BirdsEye/setting_parse.h @@ -0,0 +1,46 @@ +#pragma once + +/////////////////////////////////////////// +// SETTING VALUE PARSING (pure unit) +// +// Strict numeric parsing for /SETTINGS.json values, extracted because +// atoi() is the wrong tool here and the wrongness is invisible. +// +// atoi("") and atoi("garbage") both answer 0, with no way to tell either +// from a real "0". Every numeric setting in BirdsEye.ino is read as +// +// if (getSetting(key, buf, n)) { const int v = atoi(buf); +// if (inRange(v)) applyIt(v); } +// +// which is safe only while 0 is OUT of the accepted range — true for +// rev_limit (floor 1000) and temp1_alert_c (floor 50), and FALSE for +// every setting plan 0006/0010 added. For led_brightness, 0 is a real and +// destructive value: it disables the LEDs and the 5 V boost rail never +// comes up. So a blank or corrupt led_brightness did not "clamp back to +// the compiled-in default per the house idiom" as its comment claims — it +// silently killed the strip, and looked exactly like dead hardware. +// Same shape for led_brightness_night (0 = blank strip) and the two +// day/night hours (0 = midnight, a legitimate value). +// +// parseIntSetting() answers false for anything that is not a complete +// integer, so the caller's existing range check keeps the compiled-in +// default — which is what those comments always meant. +/////////////////////////////////////////// + +namespace setting_parse { + +/** + * @brief Parse a settings value as a complete decimal integer. + * + * Accepts optional leading whitespace and an optional +/- sign, then one + * or more digits, then optional trailing whitespace, and nothing else. + * Rejects "" / " " / "abc" / "12abc" / "1.5" / "0x10" — anything atoi() + * would quietly turn into a number. + * + * @param s NUL-terminated value (may be nullptr). + * @param out Receives the value only when this returns true. + * @return true if the whole string is an integer that fits an int. + */ +bool parseIntSetting(const char* s, int* out); + +} // namespace setting_parse diff --git a/BirdsEye/settings.h b/BirdsEye/settings.h index 4282bed..f21283d 100644 --- a/BirdsEye/settings.h +++ b/BirdsEye/settings.h @@ -9,6 +9,33 @@ #include +/////////////////////////////////////////// +// SETTINGS JSON CAPACITY — the single source of truth. +// +// Both the read buffer and the ArduinoJson document that parse +// /SETTINGS.json must be this size, in EVERY module that parses the file. +// There are two such parsers: settings.ino (getSetting/setSetting) and +// bluetooth.ino's `SLIST` handler, which enumerates the file for the +// companion app. +// +// This constant exists because those two drifted. Plan 0010 raised +// settings.ino's pair 512 -> 1024 when nine new keys took the default file +// from 329 to 538 bytes; bluetooth.ino's copy stayed at 512, so `SLIST` +// read 511 bytes of a 538-byte file, deserializeJson() returned +// IncompleteInput, and the handler answered SERR:PARSE on every device +// instead of listing a single key. Nothing coupled the two numbers, so +// nothing caught it. Now they cannot drift. +// +// TWO separate walls, both sized by this: the READ CAP (a file longer than +// capacity-1 parses as IncompleteInput and every key read fails) and the +// DOCUMENT CAPACITY (22 string pairs need JSON_OBJECT_SIZE(22); a <512> +// document returns NoMemory regardless of how much was read). +// +// Adding settings keys is not free — see the measureJson() guard in +// setSettingInner(), and subsystem 8 in CLAUDE.md. +/////////////////////////////////////////// +#define SETTINGS_JSON_CAPACITY 1024 + // Initialize the settings file (creates defaults on first boot, // adds any missing keys on upgrade). Call once from setup() AFTER // SD_SETUP(). diff --git a/BirdsEye/settings.ino b/BirdsEye/settings.ino index 02ec442..fec6839 100644 --- a/BirdsEye/settings.ino +++ b/BirdsEye/settings.ino @@ -23,8 +23,8 @@ static const char SETTINGS_BAD_PATH[] = "/SETTINGS.json.bad"; // string pairs need JSON_OBJECT_SIZE(22) and a <512> doc returns // NoMemory. Keep the two numbers equal, and see the measureJson() guard // in setSettingInner() before adding more keys. -static char settingsFileBuffer[1024]; -static StaticJsonDocument<1024> settingsJson; +static char settingsFileBuffer[SETTINGS_JSON_CAPACITY]; +static StaticJsonDocument settingsJson; // 12 racing-adjacent words used to build a friendly default device name so // logs dumped from a fleet of devices stay distinguishable. The literals diff --git a/BirdsEye/sim/CMakeLists.txt b/BirdsEye/sim/CMakeLists.txt index 78cbdf3..bf9c5d8 100644 --- a/BirdsEye/sim/CMakeLists.txt +++ b/BirdsEye/sim/CMakeLists.txt @@ -149,6 +149,7 @@ set(SIM_CORE_SOURCES ${BIRDSEYE_DIR}/idle_policy.cpp ${BIRDSEYE_DIR}/lap_format.cpp ${BIRDSEYE_DIR}/local_time.cpp + ${BIRDSEYE_DIR}/setting_parse.cpp ${BIRDSEYE_DIR}/led_animations.cpp ${BIRDSEYE_DIR}/led_frame.cpp ${BIRDSEYE_DIR}/led_modes.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c8a712..3e57c95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,42 @@ nothing this release — so every entry below is live for every user. be silent. ### Fixed +- **The Bluetooth settings list works again.** Asking the device to + enumerate its settings (`SLIST` — what the companion app's settings + screen does) answered a parse error and listed nothing. The nine keys + added this release took `/SETTINGS.json` from 329 to 538 bytes, and while + the settings module's own reader was raised to 1024 bytes to suit, a + *second* copy of that reader inside the Bluetooth code was left at 512 — + so it read 511 bytes of a 538-byte file and gave up on the truncated + result. Both are now sized by one shared constant, so they cannot drift + apart again. Reading and writing individual settings was never affected. +- **A blank or corrupt `led_brightness` no longer looks like dead + hardware.** Every numeric setting was read with a parser that answers + "0" for an empty or non-numeric value, and 0 is a real, meaningful + setting for most of the new keys — for `led_brightness` it means *LEDs + off, and never even power up the 5 V rail*. A garbled value therefore + switched the strip off silently instead of falling back to the default, + as the code always claimed it did. Values are now parsed strictly: + anything that is not a whole number is rejected and the default stands. + A deliberate `0` still works exactly as before. +- **The overrev alert no longer strobes when the two RPM limits are set + close together.** `rev_limit` and `overrev_limit` are validated + independently, so nothing stopped you setting the overrev limit at or + below the rev limit. Doing so inverted the alert's release point, + and the whole LED chain flickered at the frame rate instead of flashing. +- **`STOPPED` on the pace page is no longer cut off.** At the size the page + draws it, the text needed 144 pixels of a 128-pixel screen, so the final + letter was clipped on every render. +- **A build with the LED subsystem compiled out now still powers the LED + rail down.** Such a build does nothing with the LED pins by design — but + on a board whose pads an earlier LED-enabled build had already converted, + "nothing" left the boost converter's enable pin undriven, so the 5 V rail + could stay up through shutdown and drain the battery of a device with no + power switch. It now holds that pin low, but *only* on a board that has + already been converted; hardware that never ran an LED-enabled build is + still never touched. (No shipped 4.1.0 image is built this way — the LED + subsystem is in all of them — so this protects custom builds and anyone + rolling one back.) - **Four menus you could get into but not out of now have a Back/Cancel row.** The transfer menu (Bluetooth / USB), the replay session browser, and the course creator's first two screens (track prompt, course type) diff --git a/CLAUDE.md b/CLAUDE.md index 4588b86..7776bb0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,6 +123,7 @@ desktop toolchain. This is where logic worth unit-testing lives. | `ble_stream.{h,cpp}` | BLE file-transfer read-ahead bookkeeping: chunk size from the negotiated MTU, the compacting refill (`Move`), slice/consume, transfer rate. An off-by-one here corrupts a downloaded session, so the index math is host-tested away from the radio | | `sd_access_policy.{h,cpp}` | SD access arbitration decision table (mode values + grant/deny rules) | | `lap_format.{h,cpp}` | ms → `M:SS.mmm` lap-time rendering (three zero-minutes styles), used by all display pages | +| `setting_parse.{h,cpp}` | Strict integer parsing for `/SETTINGS.json` values. Exists because `atoi()` answers 0 for `""` and `"garbage"`, and 0 is an in-range, **destructive** value for the LED keys (`led_brightness` 0 = strip off + boost rail never raised) — so a blank setting read as a deliberate "off". Rejects anything that is not a complete integer, so the caller's range check keeps the compiled-in default | | `local_time.{h,cpp}` | UTC + a fixed signed minute offset → local wall clock (4-digit year, correct month/year/leap rollover both ways) + the `isNight()` window test. **No DST, and NOTHING logged goes through it** — saved data stays UTC (subsystem 17) | | `led_frame.{h,cpp}` | NeoPixel pixel layout (11 px: 2 status + 9-px strip), `Rgb`/`Frame` PODs, and **`applyCap()` — the single global-brightness choke point** (post-condition: no channel exceeds the cap) | | `led_modes.{h,cpp}` | Strip modes + status actions: pace pip math (ms/m, slower = left/red), generic `ScaleSpec` left-fill (RPM red past halfway; temps later), and the `StatusAction` threshold/hysteresis/flash table — the phase-2 assignability hook | @@ -673,7 +674,7 @@ loop() ~250 Hz cycle, plus `fwReset()` to abort any in-flight OTA and free the staging file + SD access. - *Manual Exit* (`bleExitTransferMode()`): the parked-loop Exit button - runs `BLE_STOP()` then the same 100 ms-delay reboot. Before 4.0.1 a + runs `BLE_STOP()` then the same 100 ms-delay reboot. Before 4.1.0 a manual exit dropped back to the menu without rebooting, so settings written over BLE silently didn't apply until the next power cycle. The SIM stub returns after stopping (no reboot) so the golden menu walk can @@ -711,6 +712,15 @@ loop() ~250 Hz - Separate `StaticJsonDocument<1024>` — does not share the track parser's `JSON_BUFFER_SIZE` buffer. - Total RAM cost: ~2 KB (1024-byte file buffer + 1024-byte JSON document). +- **There are TWO parsers of this file, and both are sized by + `SETTINGS_JSON_CAPACITY` (`settings.h`).** `settings.ino` owns + `getSetting()`/`setSetting()`; `bluetooth.ino`'s `SLIST` handler has its + own buffer + document to enumerate the file for the companion app. They + drifted once — plan 0010 raised settings.ino's pair 512 → 1024 and + missed the BLE copy, so `SLIST` read 511 B of a 538 B file and answered + `SERR:PARSE` on every device while `SGET`/`SSET` still worked. The + shared constant is what makes that impossible now; if you add a third + parser, size it from the same macro. - **The two 1024s must stay equal, and adding keys is not free.** Every read path caps at `sizeof(settingsFileBuffer) - 1`, so a file bigger than the buffer parses as `IncompleteInput` and *every* key read @@ -821,9 +831,18 @@ hardware needs no power switch. Wake = chip reset = fresh `setup()`. System OFF with the conn LED still driven; GPIO state is retained there, hence the "blue light stays on after sleep" field report) → `DISPLAY_SLEEP()` → `GPS_SLEEP()` (u-blox software backup, µA, config - retained while powered; TIMER3 stopped) → IMU power rail off. The - charging-loop soft resume (`softResumeFromCharging()`) restarts the - egg scanner via `SENSOREGG_WAKE()`; BLE/camera stay lazy. + retained while powered; TIMER3 stopped) → IMU power rail off → + `NEOPIXEL_SLEEP()` (blank while 5 V is up, data LOW, then boost EN + LOW — before the charging branch, so the strip is dark on the cable + too). **`NEOPIXEL_SLEEP()` is not a no-op on a flag-OFF build**: it + still drives boost EN low when `UICR->NFCPINS` shows the pads were + already converted by an earlier beta build, because the driven level + is what survives System OFF and a floating EN leaves the rail up (the + same retention as the conn-LED report above). An unconverted board is + never touched. The charging-loop soft resume + (`softResumeFromCharging()`) restarts the egg scanner via + `SENSOREGG_WAKE()` and re-raises the strip via `NEOPIXEL_WAKE()`; + BLE/camera stay lazy. - **System OFF entry** (`shutdownSystemOff()`, no return): wait for the entry combo's buttons to release (a held button = SENSE satisfied = instant wake-reset), **sample the tach line's parked idle level** @@ -1597,7 +1616,7 @@ the one loaded). Sector lines stay optional — zero, one, or two. | `debug_pages` | string | `"hide"` | Race-rotation diagnostic pages (`GPS_DEBUG` + `GPS_STATS`): `hide` = rotation starts at the speed page (end-user default), `show` = diagnostics restored at the front. Anything other than an explicit `show` means hide. Also swaps the tachometer page's subtext line for the tach filter diagnostic (`max:NNNNN S rj:NN`, plan 0009). No-op on the rotation under `ENDURANCE_MODE` (already starts at speed) | | `cylinder_count` | int | `1` | Cylinders the **pickup sees** — a clamp on one plug wire of a twin sees ONE. Only a shared coil / all-cylinder harness sees them all | | `tach_filter` | string | `"smooth"` | RPM estimator (plan 0009). `smooth` = outlier gate + RPM-aware noise models; `legacy` = the pre-0009 filter bit for bit, for A/B against older logs; `raw` = no estimator at all, the `rpm` column is exactly what the pickup delivers. Anything else means `smooth`. Diagnostic knob — the intent is one session each at the track, not a permanent tuning dial | -| `led_brightness` | int | `64` | NeoPixel global brightness cap 0–255 — no LED channel ever exceeds it (`led_frame::applyCap`). `0` disables the LEDs entirely (boost rail never enabled). Read only by `BIRDSEYE_ENABLE_NEOPIXEL` builds; clamp back to 64 on nonsense | +| `led_brightness` | int | `64` | NeoPixel global brightness cap 0–255 — no LED channel ever exceeds it (`led_frame::applyCap`). `0` disables the LEDs entirely (boost rail never enabled). Written AND parsed on every channel (`ensureDefaultSettings()` + the boot block in `BirdsEye.ino`); only its *use* is compiled out with the flag. A non-numeric value keeps the 64 default — via `setting_parse::parseIntSetting`, never `atoi()`, because `atoi("")` is 0 and 0 here means "LEDs off" | | `rev_limit` | int | `15000` | True RPM WARNING limit: RPM-scale ceiling and the left status LED flasher threshold. Clamp 1000–20000 (tach filter's ceiling) | | `overrev_limit` | int | `0` (disabled) | True RPM PROBLEM limit (plan 0007): past it the whole 11-px chain flashes red (outranks the purple celebration) and the tach page shows `*OVER REV*`; latch clears below `rev_limit × 0.97`. 0 = off (no chain flash, no header); else clamp 1000–20000 | | `temp1_alert_c` | int | `650` | Temp1 (EGT) alert threshold in **Celsius** for the right status LED: red flash at/above, clears 20 °C below, solid blue when the probe signal is NaN/stale. Clamp 50–1200 | @@ -1733,7 +1752,7 @@ the one loaded). Sector lines stay optional — zero, one, or two. | SdFat | SD card (FAT16/32) | | DovesLapTimer | Lap/sector timing (external: TheAngryRaven/DovesLapTimer). CI refs: `BETA`-targeted builds track the library's `BETA` branch; master/release builds pin `v4.3.0` (bump deliberately) | | Seeed Arduino LSM6DS3 | Onboard IMU accelerometer/gyro (Sense variant, ±16g) | -| Adafruit NeoPixel | WS2812 strip driver (subsystem 16; linked in but inert unless `BIRDSEYE_ENABLE_NEOPIXEL`) | +| Adafruit NeoPixel | WS2812 strip driver (subsystem 16). **Only compiled/linked when `BIRDSEYE_ENABLE_NEOPIXEL` is set** — the include sits inside the `#if` in `neopixel.ino`, so a master/release image carries none of it | | Bluefruit nRF52 | BLE (built into board package) | | Adafruit TinyUSB | USB Mass Storage (`Adafruit_USBD_MSC`); built into board package | diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3222198..556329b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,7 @@ add_executable(birdseye_tests sd_access_policy_test.cpp lap_format_test.cpp local_time_test.cpp + setting_parse_test.cpp tach_filter_test.cpp camera_fsm_test.cpp idle_policy_test.cpp @@ -50,6 +51,7 @@ add_executable(birdseye_tests ${BIRDSEYE_DIR}/sd_access_policy.cpp ${BIRDSEYE_DIR}/lap_format.cpp ${BIRDSEYE_DIR}/local_time.cpp + ${BIRDSEYE_DIR}/setting_parse.cpp ${BIRDSEYE_DIR}/tach_filter.cpp ${BIRDSEYE_DIR}/camera_fsm.cpp ${BIRDSEYE_DIR}/idle_policy.cpp diff --git a/tests/led_modes_test.cpp b/tests/led_modes_test.cpp index 713a424..f6fd9c1 100644 --- a/tests/led_modes_test.cpp +++ b/tests/led_modes_test.cpp @@ -233,6 +233,47 @@ TEST_CASE("overrev-style action: wide hysteresis holds the latch down to clear") CHECK(!st.active); } +TEST_CASE("inverted hysteresis (clearBelow above threshold) never strobes") { + // rev_limit and overrev_limit clamp independently, so a user can set + // overrev_limit at or below rev_limit * kRevClearFrac. neopixel.ino then + // builds the overrev action with clearBelow ABOVE threshold. Before the + // clamp in evalStatus, a value inside that inverted band set the latch on + // one frame and cleared it on the next — all 11 pixels strobing at the + // 30 Hz frame rate instead of flashing at 100 ms. + // + // Concretely: rev_limit 15000 (clear 14550), overrev_limit 12000. + StatusAction over{Source::kRpm, 12000.0f, 14550.0f, led_frame::kRed, + led_modes::kRevFlashHalfPeriodMs, led_frame::kOff}; + StatusState st; + + // At 13000 RPM: above the 12000 trip, below the bogus 14550 release. + led_modes::evalStatus(over, st, 13000.0f, true, 0); + CHECK(st.active); + // The latch must HOLD across successive frames, not alternate. + for (uint32_t f = 1; f < 20; f++) { + led_modes::evalStatus(over, st, 13000.0f, true, f * 33); + CAPTURE(f); + CHECK(st.active); + } + // It still releases below the (collapsed) trip point. + led_modes::evalStatus(over, st, 11999.0f, true, 0); + CHECK(!st.active); + // And re-arms cleanly. + led_modes::evalStatus(over, st, 12000.0f, true, 0); + CHECK(st.active); + + // A sane configuration is untouched by the clamp: overrev above rev. + StatusAction sane{Source::kRpm, 16000.0f, 14550.0f, led_frame::kRed, + led_modes::kRevFlashHalfPeriodMs, led_frame::kOff}; + StatusState st2; + led_modes::evalStatus(sane, st2, 16100.0f, true, 0); + CHECK(st2.active); + led_modes::evalStatus(sane, st2, 15000.0f, true, 0); + CHECK(st2.active); // wide band still latched + led_modes::evalStatus(sane, st2, 14000.0f, true, 0); + CHECK(!st2.active); +} + TEST_CASE("search pip: one green pixel, bounces to both ends, deterministic") { Rgb out[kStripCount]; auto litIndex = [&](uint32_t t) { diff --git a/tests/setting_parse_test.cpp b/tests/setting_parse_test.cpp new file mode 100644 index 0000000..ba5bd11 --- /dev/null +++ b/tests/setting_parse_test.cpp @@ -0,0 +1,96 @@ +#include "doctest.h" + +#include "setting_parse.h" + +#include + +using setting_parse::parseIntSetting; + +namespace { + +// Helper: parse and return the value, or a sentinel when rejected. +int parsed(const char* s, int sentinel = -999999) { + int v = 0; + return parseIntSetting(s, &v) ? v : sentinel; +} + +} // namespace + +TEST_CASE("parseIntSetting: accepts plain integers") { + CHECK(parsed("0") == 0); + CHECK(parsed("7") == 7); + CHECK(parsed("64") == 64); + CHECK(parsed("255") == 255); + CHECK(parsed("15000") == 15000); + CHECK(parsed("-360") == -360); + CHECK(parsed("+330") == 330); + CHECK(parsed("-0") == 0); +} + +TEST_CASE("parseIntSetting: tolerates surrounding whitespace") { + CHECK(parsed(" 64") == 64); + CHECK(parsed("64 ") == 64); + CHECK(parsed("\t 64 \r\n") == 64); + CHECK(parsed(" -360 ") == -360); +} + +TEST_CASE("parseIntSetting: rejects everything atoi() would silently zero") { + // THE bug this unit exists for: each of these came back as 0 from + // atoi(), and 0 is an in-range, destructive value for led_brightness. + int v = 12345; + CHECK(!parseIntSetting("", &v)); + CHECK(v == 12345); // out is untouched on rejection + CHECK(!parseIntSetting(" ", &v)); + CHECK(!parseIntSetting("abc", &v)); + CHECK(!parseIntSetting("null", &v)); + CHECK(!parseIntSetting("true", &v)); + CHECK(!parseIntSetting("-", &v)); + CHECK(!parseIntSetting("+", &v)); + CHECK(!parseIntSetting(nullptr, &v)); + CHECK(v == 12345); +} + +TEST_CASE("parseIntSetting: rejects partial numbers rather than truncating") { + int v = 0; + CHECK(!parseIntSetting("12abc", &v)); // atoi -> 12 + CHECK(!parseIntSetting("1.5", &v)); // atoi -> 1 + CHECK(!parseIntSetting("0x10", &v)); // atoi -> 0 + CHECK(!parseIntSetting("64,", &v)); // atoi -> 64 + CHECK(!parseIntSetting("6 4", &v)); // atoi -> 6 + CHECK(!parseIntSetting("--5", &v)); +} + +TEST_CASE("parseIntSetting: rejects values that do not fit an int") { + int v = 0; + CHECK(!parseIntSetting("99999999999", &v)); + CHECK(!parseIntSetting("-99999999999", &v)); + CHECK(!parseIntSetting("340282366920938463463374607431768211456", &v)); + // The boundaries themselves still parse. + CHECK(parsed("2147483647") == 2147483647); + CHECK(parsed("-2147483648") == INT_MIN); +} + +TEST_CASE("parseIntSetting: null out pointer is rejected, not dereferenced") { + CHECK(!parseIntSetting("64", nullptr)); +} + +TEST_CASE("the caller contract: a rejected value keeps the compiled-in default") { + // Mirrors BirdsEye.ino's read pattern for led_brightness, which is the + // one where 0 is both in range and destructive (LEDs off, 5 V boost + // rail never enabled). A blank or corrupt value must leave the default + // standing rather than reading as "the user asked for 0". + auto applyBrightness = [](const char* stored, unsigned char current) { + int b = 0; + if (parseIntSetting(stored, &b) && b >= 0 && b <= 255) { + return (unsigned char)b; + } + return current; + }; + + CHECK(applyBrightness("", 64) == 64); // was 0 -> LEDs dead + CHECK(applyBrightness("garbage", 64) == 64); // was 0 -> LEDs dead + CHECK(applyBrightness("128", 64) == 128); // a real change still lands + CHECK(applyBrightness("0", 64) == 0); // a DELIBERATE 0 still works + CHECK(applyBrightness("300", 64) == 64); // out of range -> default + CHECK(applyBrightness("-5", 64) == 64); +}