From 28b9c18ee6f2babc3d30712e0fa0a1147e5d3545 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:36:42 +0000 Subject: [PATCH 1/8] plan 0016: abandon a drag run whose GPS fix is lost for good MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D1. Every drag_timer abort (fix gap, mid-run standstill, prove-out) edges on a fix passed to onFix(), and the glue only feeds it while gpsData.fix. A fix that dropped mid-pass and never came back left the run LAUNCHED forever; in manual mode the tree sat in kRunning (where the Select-hold exit is deliberately suppressed) on a pinned screen that ate every button — a wedge with no way out but the reboot combo. The root cause is that the physics has no notion of wall time, so the fix lives there: DragTimer::checkFixLoss(nowMs, lastFixMs). The glue records millis() whenever it feeds a fix and calls the watchdog every loop; a staged or launched timer with no fix for kFixLossAbortMs (= the in-stream kFixGapAbortMs) drops to ARMED and forgets its previous fix so a returning fix starts a fresh stream rather than resuming a stale run. The tree's existing "runActive fell without a run" rule then shows RUN ABORTED, which accepts the exit hold. Keying on the last fix FED also covers a receiver that stops streaming with its fix flag latched. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/BirdsEye.ino | 11 ++++++ BirdsEye/drag_timer.cpp | 8 ++++ BirdsEye/drag_timer.h | 19 ++++++++++ BirdsEye/gps_functions.ino | 1 + CHANGELOG.md | 7 ++++ CLAUDE.md | 9 +++-- docs/plans/0016-manual-drag-tree.md | 23 +++++++++++ tests/drag_timer_test.cpp | 59 +++++++++++++++++++++++++++++ tests/drag_tree_test.cpp | 24 ++++++++++++ 9 files changed, 158 insertions(+), 3 deletions(-) diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index 9d23a6c..afdaa0d 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -176,6 +176,11 @@ drag_timer::DragTimer* dragTimer = nullptr; int dragLastRunCount = 0; // run-complete edge for lap history capture int dragDistanceIdx = -1; // index into drag_timer's distance table bool dragWasStaged = false; // ARMED->STAGED edge for the idle-grace re-arm +// millis() of the last fix fed to dragTimer->onFix() — the fix-loss +// watchdog's reference (review D1). Every physics rule edges on a fix, +// so without a wall-clock check a fix lost mid-pass left the run live +// forever (and the manual tree's pinned screen with no exit). +uint32_t dragLastFixMillis = 0; // Manual drag mode (plan 0016): the christmas-tree staging sequence. // dragManualMode is latched by startDragSession and cleared ONLY by @@ -529,6 +534,11 @@ void checkForNewLapData() { // re-stages every couple of minutes, so an active queue never idles // out, while a genuinely parked car still ends after the grace. if (dragTimer != nullptr) { + // Fix-loss watchdog first, so the staged/run edges below and the + // manual tree (stepped later this loop) all see the abort. + if (dragTimer->checkFixLoss((uint32_t)millis(), dragLastFixMillis)) { + debugln(F("Drag: fix lost — staged/in-flight run abandoned")); + } const bool stagedNow = dragTimer->staged(); if (stagedNow && !dragWasStaged) { raceSessionStartedAt = millis(); @@ -1769,6 +1779,7 @@ void startDragSession(int distanceIdx, bool manualStaging) { dragDistanceIdx = dragTimer->targetIdx(); // clamped by the unit dragLastRunCount = 0; dragWasStaged = false; + dragLastFixMillis = (uint32_t)millis(); dragManualMode = manualStaging; dragLastRtMs = 0; dragGreenEpochMs = 0; diff --git a/BirdsEye/drag_timer.cpp b/BirdsEye/drag_timer.cpp index 904a686..a5b17c4 100644 --- a/BirdsEye/drag_timer.cpp +++ b/BirdsEye/drag_timer.cpp @@ -264,6 +264,14 @@ bool DragTimer::onFix(double lat, double lng, float speedMph, return completed; } +bool DragTimer::checkFixLoss(uint32_t nowMs, uint32_t lastFixMs) { + if (phase_ == Phase::kArmed) return false; + if ((uint32_t)(nowMs - lastFixMs) < kFixLossAbortMs) return false; + resetToArmed(); + havePrev_ = false; // the next fix is the first of a fresh stream + return true; +} + unsigned long DragTimer::currentEtMs(uint64_t nowGpsMs) const { if (phase_ != Phase::kLaunched) return 0; const double et = (double)nowGpsMs - runStartMs_; diff --git a/BirdsEye/drag_timer.h b/BirdsEye/drag_timer.h index d750363..12f66d5 100644 --- a/BirdsEye/drag_timer.h +++ b/BirdsEye/drag_timer.h @@ -97,6 +97,16 @@ constexpr float kRestageFt = 10.0f; // to the re-latch (the mean follows the parked car's drifting fix). constexpr int kAnchorMeanWindow = 32; +// Wall-clock fix-loss watchdog (review fix D1). Every rule above edges +// on a fix passed to onFix(), so a fix that drops mid-pass and never +// comes back left the run LAUNCHED forever — and on the manual tree's +// pinned screen that was a wedge with no exit. The glue therefore +// reports host time + the time of the last fix it fed, and a staged or +// launched timer with no fix for this long is abandoned exactly like +// the in-stream fix-gap abort (same threshold, same reasoning: nothing +// measured across the gap is trustworthy). +constexpr uint32_t kFixLossAbortMs = kFixGapAbortMs; + enum class Phase : uint8_t { kArmed, // waiting for a standstill (also post-run / post-abort) kStaged, // stopped, anchor latched, watching for the rollout @@ -133,6 +143,15 @@ class DragTimer { // reaction time is this minus the green-light epoch. uint64_t runStartEpochMs() const { return (uint64_t)(runStartMs_ + 0.5); } + // Fix-loss watchdog, called every loop by the glue with host + // millis() and the millis() of the last fix it fed to onFix(). Once + // kFixLossAbortMs pass without a fix, a STAGED or LAUNCHED timer + // drops back to ARMED and forgets its previous fix, so a returning + // fix starts a fresh stream instead of resuming a stale run. Returns + // true when it aborted. Both args are wrap-safe uint32 millis; the + // unit still never reads a clock itself. + bool checkFixLoss(uint32_t nowMs, uint32_t lastFixMs); + int runs() const { return runs_; } // Live ET while a run is on, 0 otherwise. nowGpsMs lets the display diff --git a/BirdsEye/gps_functions.ino b/BirdsEye/gps_functions.ino index 6701182..cdcc989 100644 --- a/BirdsEye/gps_functions.ino +++ b/BirdsEye/gps_functions.ino @@ -496,6 +496,7 @@ void GPS_LOOP() { dragTimer->onFix(gpsData.latitudeDegrees, gpsData.longitudeDegrees, (float)(gpsData.speed * 1.15078), // knots -> mph getGpsUnixTimestampMillis()); + dragLastFixMillis = (uint32_t)millis(); // fix-loss watchdog reference } #ifdef SD_CARD_LOGGING_ENABLED diff --git a/CHANGELOG.md b/CHANGELOG.md index d8cd041..e684478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 ## [Unreleased] ### Fixed +- **Drag mode review fixes** (plans 0015/0016): + - A GPS fix lost mid-pass and never regained no longer wedges manual + drag mode. The run stayed "live" forever — the staging screen stuck + on a ticking ET with every button ignored. A staged or in-flight run + with no fix for 2 s is now abandoned (RUN ABORTED on the manual + screen, from which Select-hold exits), and a returning fix starts + fresh instead of resuming the stale run. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index 513d644..bf74619 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -891,8 +891,11 @@ loop() ~250 Hz fake a launch), launch rollout-style (11.25 in displacement + ≥2 mph, ET start interpolated between the straddling 25 Hz fixes), accumulate chord distance to the target, finish with interpolated ET + trap speed - and a 0-60 split (0 if never reached). Mid-run standstill (3 s) or a - ≥2 s fix gap abandons the run **silently** — which is also how a + and a 0-60 split (0 if never reached). Mid-run standstill (3 s), a + ≥2 s fix gap, or 2 s of wall clock with no fix at all (the + `checkFixLoss` watchdog the glue calls every loop with `millis()` — + every other rule edges on a fix, so a fix lost for good would + otherwise leave the run live forever) abandons the run **silently** — which is also how a queue-creep phantom launch self-cancels — then the timer re-arms on the next standstill, so a whole day of passes is one DOVEX session (`race_mode=DRAG`, course `DRAG 1/4 MILE` etc., laps line = run ETs; @@ -2057,7 +2060,7 @@ the one loaded). Sector lines stay optional — zero, one, or two. | Drag tree cadence / pre-stage hold | 500 ms per yellow / staged +2 s before the tree starts | `drag_tree.h` | | Drag tree failed-launch / exit hold | green +5 s still → failed / Select held 2 s → end session | `drag_tree.h` | | Drag tree flash half-period | 500 ms (3 Hz OLED aliases anything faster) | `drag_tree.h` | -| Drag stage / launch / abort | ≤1 mph held 1 s / ≥2 mph + rollout / ≤2 mph held 3 s or ≥2 s fix gap (silent) | `drag_timer.h` | +| Drag stage / launch / abort | ≤1 mph held 1 s / ≥2 mph + rollout / ≤2 mph held 3 s, ≥2 s fix gap, or 2 s wall-clock with no fix (`checkFixLoss`) (silent) | `drag_timer.h` | | Drag prove-out | launch must reach 15 mph within 5 s of ET start, else silently abandoned | `drag_timer.h` | | Drag time base | Unix epoch ms (`getGpsUnixTimestampMillis()`) — never time-of-day ms (wraps at UTC midnight) | `gps_functions.ino` | | Drag distances | 660 / 1000 / 1320 / 2640 / 5280 ft (picker order) | `drag_timer.cpp` | diff --git a/docs/plans/0016-manual-drag-tree.md b/docs/plans/0016-manual-drag-tree.md index 745bc40..a966eb6 100644 --- a/docs/plans/0016-manual-drag-tree.md +++ b/docs/plans/0016-manual-drag-tree.md @@ -136,3 +136,26 @@ on GPS_SPEED exactly as before. - No jump-start detection during PRE-STAGE (movement there just re-stages silently — the foul window is the yellows, like a real tree between pre-stage and green). + +## Review fixes (2026-09) + +Findings from the pre-release review of plans 0015 + 0016, one commit +each, each with a regression test in the pure unit that owns the rule. + +- **D1 — fix lost mid-pass wedged the pinned screen.** `kRunning` skips + the Select-hold exit (so a pass can't be ended by a stray thumb), and + every physics abort edges on a fix passed to `onFix()` — which the glue + only calls with a fix. A fix that dropped at speed and never came back + left `runActive` true forever: tree stuck in `kRunning`, pinned page + eating every button, live ET still ticking. The root cause is the + physics having no notion of wall time, so the fix is there, not in the + tree: `DragTimer::checkFixLoss(nowMs, lastFixMs)` — the glue passes + `millis()` and the `millis()` of the last fix it fed, and a staged or + launched timer with no fix for `kFixLossAbortMs` (= the in-stream + `kFixGapAbortMs`, 2 s, same reasoning) drops to ARMED **and forgets its + previous fix**, so a returning fix starts a fresh stream instead of + resuming a stale run. The tree's existing "`runActive` fell without a + run" rule then surfaces RUN ABORTED, whose screen accepts the exit + hold. Keying on the last fix *fed* (not `gpsData.fix`) also covers a + receiver that stops streaming with its fix flag latched true. Applies + to automatic mode too (a live ET frozen on a dead fix). diff --git a/tests/drag_timer_test.cpp b/tests/drag_timer_test.cpp index a161089..0dc11d9 100644 --- a/tests/drag_timer_test.cpp +++ b/tests/drag_timer_test.cpp @@ -264,6 +264,65 @@ TEST_CASE("fix gap over threshold aborts the run") { CHECK(s.t.runs() == 0); } +TEST_CASE("fix lost mid-run with no fix ever returning aborts on the watchdog") { + // Review D1: every rule edges on onFix(), so a fix that drops at speed + // and never returns used to leave the run LAUNCHED forever (on the + // manual tree's pinned screen, a wedge with no exit). + Strip s(0); + s.standstill(0.0, 2000); + const double v = 60.0; + for (double x = 0.0; x < 200.0; x += v * 0.04) { + s.fix(x, v * kFtPerSecToMph); + } + REQUIRE(s.t.runActive()); + + const uint32_t lastFixMillis = 50000; // host millis of the last fix fed + CHECK_FALSE(s.t.checkFixLoss(lastFixMillis + drag_timer::kFixLossAbortMs - 1, + lastFixMillis)); + CHECK(s.t.runActive()); + CHECK(s.t.checkFixLoss(lastFixMillis + drag_timer::kFixLossAbortMs, + lastFixMillis)); + CHECK_FALSE(s.t.runActive()); + CHECK(s.t.phase() == Phase::kArmed); + CHECK(s.t.runs() == 0); + // Idempotent once armed. + CHECK_FALSE(s.t.checkFixLoss(lastFixMillis + 60000, lastFixMillis)); +} + +TEST_CASE("fix-loss watchdog is millis-wrap safe") { + Strip s(0); + s.standstill(0.0, 2000); + REQUIRE(s.t.phase() == Phase::kStaged); + const uint32_t last = 0xFFFFFF00u; + CHECK_FALSE(s.t.checkFixLoss(last + 100u, last)); // wrapped, 100 ms + CHECK(s.t.checkFixLoss(last + drag_timer::kFixLossAbortMs, last)); + CHECK(s.t.phase() == Phase::kArmed); +} + +TEST_CASE("a fix returning after the watchdog starts a fresh stream, not a stale run") { + Strip s(0); + s.standstill(0.0, 2000); + const double v = 60.0; + for (double x = 0.0; x < 200.0; x += v * 0.04) { + s.fix(x, v * kFtPerSecToMph); + } + REQUIRE(s.t.runActive()); + REQUIRE(s.t.checkFixLoss(10000 + drag_timer::kFixLossAbortMs, 10000)); + // The fix comes back well down the strip still at speed: nothing may + // resume, and crossing the target must not complete a run. + s.now += 500; + CHECK_FALSE(s.t.runActive()); + for (double x = 400.0; x < 900.0; x += v * 0.04) { + CHECK_FALSE(s.fix(x, v * kFtPerSecToMph)); + } + CHECK(s.t.runs() == 0); + // And the timer is healthy: stop, stage, run. + s.standstill(900.0, 2000); + CHECK(s.t.phase() == Phase::kStaged); + CHECK(s.launchConstAccel(900.0, 30.0, 15000)); + CHECK(s.t.runs() == 1); +} + TEST_CASE("fix gap under threshold accumulates the chord and continues") { Strip s(0); s.standstill(0.0, 2000); diff --git a/tests/drag_tree_test.cpp b/tests/drag_tree_test.cpp index ac38e7b..fce655f 100644 --- a/tests/drag_tree_test.cpp +++ b/tests/drag_tree_test.cpp @@ -279,6 +279,30 @@ TEST_CASE("mid-run physics abort surfaces as RUN ABORTED") { CHECK(r.stage() == Stage::kWaitStop); } +TEST_CASE("fix lost mid-run: the physics watchdog abort frees the pinned screen") { + // Review D1 end to end on the tree's terms: the GPS fix drops at + // speed and never returns. The glue's fix-loss watchdog + // (DragTimer::checkFixLoss) drops runActive; the tree must surface + // RUN ABORTED and the exit hold must then work — before the fix, the + // tree sat in kRunning forever with the exit hold suppressed. + Rig r; + r.toGreen(); + r.speed = 60.0f; + r.runActive = true; + r.now += 100; + r.step(); + REQUIRE(r.stage() == Stage::kRunning); + r.fix = false; + r.tick(drag_timer::kFixLossAbortMs); // physics still "running"... + CHECK(r.stage() == Stage::kRunning); + r.runActive = false; // ...until the watchdog fires + r.now += 20; + r.step(); + REQUIRE(r.stage() == Stage::kAborted); + r.selectHeld = true; + CHECK(r.tick(drag_tree::kExitHoldMs + 40)); +} + // --------------------------------------------------------------------------- // Exit hold // --------------------------------------------------------------------------- From df8b37a00f4bf45a5774a081a3781afbf95db751 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:37:47 +0000 Subject: [PATCH 2/8] plan 0016: gate drag timing on the GPS time lock, via one predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D2. dragStagingLoop() fed the tree in.fix = gpsData.fix, and GPS_LOOP fed dragTimer->onFix() on a bare fix too, while the LED search pip and the staging screen's WAITING FOR GPS line both required fix && timeValid. So the tree could run its countdown while the screen said it was waiting — and, worse, before the lock the receiver reports a placeholder date, while drag timing is Unix epoch ms: the staged anchor, the green stamp and the ET start all sat on a clock that jumps when the lock lands (a backwards step silently aborts the run, a forwards one is a garbage reaction time). gpsFixAndTimeLocked() is now the single predicate for the physics feed (automatic mode included), the tree input, the staging screen and the LED search pip, so they cannot disagree again. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/BirdsEye.ino | 18 +++++++++++++++++- BirdsEye/display_pages.ino | 2 +- BirdsEye/drag_tree.h | 2 +- BirdsEye/gps_functions.ino | 6 ++++-- BirdsEye/neopixel.ino | 2 +- BirdsEye/sim/sim_prototypes.h | 1 + CHANGELOG.md | 6 ++++++ CLAUDE.md | 3 +++ docs/plans/0016-manual-drag-tree.md | 13 +++++++++++++ 9 files changed, 47 insertions(+), 6 deletions(-) diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index afdaa0d..b6c746d 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -1444,6 +1444,22 @@ bool dragModeIsActive() { return dragTimer != nullptr; } +/** + * @brief THE "GPS is usable for drag timing" predicate: a position fix + * AND the receiver's full UTC time lock (validDate + validTime + + * fullyResolved — the log-file-creation gate). Before the lock the + * receiver reports a placeholder date, and drag timing runs on Unix + * EPOCH ms, so a placeholder-dated fix would timestamp the staged + * anchor, the green light and the ET start on a clock that jumps by + * years the moment the lock lands (review D2: a silently aborted run, + * or a garbage reaction time). The physics feed, the manual tree's + * input, the staging screen's WAITING FOR GPS line and the LED search + * pip all read this one function so they can never disagree. + */ +bool gpsFixAndTimeLocked() { + return gpsData.fix && gpsData.timeValid; +} + // Drag-mode display accessors (null-safe): the trap/0-60 stats have no // lap-timer analog, so they don't ride the activeTimer*() surface — // display_pages reads these directly, like the sprint pages read @@ -1808,7 +1824,7 @@ void dragStagingLoop() { drag_tree::Inputs in; in.nowMs = millis(); - in.fix = gpsData.fix; + in.fix = gpsFixAndTimeLocked(); // same gate as the physics feed in.speedMph = gps_speed_mph; in.timerStaged = dragTimer->staged(); in.runActive = dragTimer->runActive(); diff --git a/BirdsEye/display_pages.ino b/BirdsEye/display_pages.ino index 79e6e19..16b89c5 100644 --- a/BirdsEye/display_pages.ino +++ b/BirdsEye/display_pages.ino @@ -258,7 +258,7 @@ void displayPage_drag_staging() { display.println(dragDistanceLabel()); display.println(); display.setTextSize(2); - if (!gpsData.fix || !gpsData.timeValid) { + if (!gpsFixAndTimeLocked()) { display.println(F(" WAITING")); display.println(F(" FOR GPS")); } else if (gps_speed_mph >= drag_timer::kLaunchMinMph) { diff --git a/BirdsEye/drag_tree.h b/BirdsEye/drag_tree.h index 678a70c..e64522b 100644 --- a/BirdsEye/drag_tree.h +++ b/BirdsEye/drag_tree.h @@ -84,7 +84,7 @@ enum class Stage : uint8_t { // Snapshot built fresh by the sketch each loop iteration. struct Inputs { uint32_t nowMs = 0; // millis() - bool fix = false; // gpsData.fix + bool fix = false; // gpsFixAndTimeLocked(): fix AND UTC lock float speedMph = 0.0f; // gps_speed_mph (meaningful only with fix) bool timerStaged = false; // dragTimer->staged() bool runActive = false; // dragTimer->runActive() diff --git a/BirdsEye/gps_functions.ino b/BirdsEye/gps_functions.ino index cdcc989..3de63a8 100644 --- a/BirdsEye/gps_functions.ino +++ b/BirdsEye/gps_functions.ino @@ -486,13 +486,15 @@ void GPS_LOOP() { sprintTimer->updateCurrentTime(getGpsTimeInMilliseconds()); sprintTimer->loop(gpsData.latitudeDegrees, gpsData.longitudeDegrees, gpsData.altitude, gpsData.speed); - } else if (gpsData.fix && dragTimer != nullptr) { + } else if (gpsFixAndTimeLocked() && dragTimer != nullptr) { // Drag mode (plan 0015): the run state machine lives in the // host-tested drag_timer unit; run completion is captured on the // run-count edge in checkForNewLapData() like sprint. EPOCH ms, // not getGpsTimeInMilliseconds() — that clock wraps to zero at // UTC midnight (evening sessions, US time zones) and a wrap - // aborts whatever run is in flight. + // aborts whatever run is in flight. Gated on the full UTC time + // lock, not just a fix: before it the epoch comes from the + // receiver's placeholder date and jumps when the lock lands. dragTimer->onFix(gpsData.latitudeDegrees, gpsData.longitudeDegrees, (float)(gpsData.speed * 1.15078), // knots -> mph getGpsUnixTimestampMillis()); diff --git a/BirdsEye/neopixel.ino b/BirdsEye/neopixel.ino index fdfb2e2..52f00c5 100644 --- a/BirdsEye/neopixel.ino +++ b/BirdsEye/neopixel.ino @@ -387,7 +387,7 @@ void NEOPIXEL_LOOP() { led_frame::Rgb stripPx[led_frame::kStripCount]; bool stripOff = raceEngineStopped(); if (!stripOff) { - if (!gpsData.fix || !gpsData.timeValid) { + if (!gpsFixAndTimeLocked()) { // Wins over the staging tree too: physics can't stage without // a fix (the tree sits in kWaitStop) and the familiar green // pip is the correct "GPS searching" signal; the pinned diff --git a/BirdsEye/sim/sim_prototypes.h b/BirdsEye/sim/sim_prototypes.h index 24cf65d..f200766 100644 --- a/BirdsEye/sim/sim_prototypes.h +++ b/BirdsEye/sim/sim_prototypes.h @@ -64,6 +64,7 @@ bool raceEngineStopped(); SprintTimer* getActiveTimerSprint(); bool sprintModeIsActive(); bool dragModeIsActive(); +bool gpsFixAndTimeLocked(); bool dragIsStaged(); const char* dragDistanceLabel(); float dragLastTrapMph(); diff --git a/CHANGELOG.md b/CHANGELOG.md index e684478..bfb5df2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,12 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 with no fix for 2 s is now abandoned (RUN ABORTED on the manual screen, from which Select-hold exits), and a returning fix starts fresh instead of resuming the stale run. + - Drag mode waits for the GPS time lock, not just a position fix, + before staging, running the christmas tree or timing a run. Before + the lock the receiver's placeholder date made the drag clock jump + when the lock landed — silently killing a run or producing a bogus + reaction time. The staging screen, the LED search pip and the timer + now share one "fix + time lock" check. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index bf74619..20afae6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -899,6 +899,9 @@ loop() ~250 Hz queue-creep phantom launch self-cancels — then the timer re-arms on the next standstill, so a whole day of passes is one DOVEX session (`race_mode=DRAG`, course `DRAG 1/4 MILE` etc., laps line = run ETs; + the timer is fed only while `gpsFixAndTimeLocked()` — fix AND UTC + lock, the one predicate the tree, staging screen and LED search pip + share, because drag time is epoch ms and the pre-lock date jumps; trap/0-60 deliberately NOT in the header — the 25 Hz rows carry speed). Run capture rides `checkForNewLapData()`'s run-count edge; each completed run AND each fresh STAGED latch re-arms the auto-idle grace diff --git a/docs/plans/0016-manual-drag-tree.md b/docs/plans/0016-manual-drag-tree.md index a966eb6..527d15d 100644 --- a/docs/plans/0016-manual-drag-tree.md +++ b/docs/plans/0016-manual-drag-tree.md @@ -159,3 +159,16 @@ each, each with a regression test in the pure unit that owns the rule. hold. Keying on the last fix *fed* (not `gpsData.fix`) also covers a receiver that stops streaming with its fix flag latched true. Applies to automatic mode too (a live ET frozen on a dead fix). +- **D2 — the tree counted down before the GPS time lock.** The tree's + `in.fix` was bare `gpsData.fix`, while the LED search pip and the + staging screen's WAITING FOR GPS line used `fix && timeValid`, and the + physics was fed on a bare fix too. Before the lock the receiver + reports a placeholder date, and drag timing is Unix **epoch** ms — so + the anchor, the green-light stamp and the ET start sat on a clock that + jumps by years the moment the lock lands (a backwards step silently + aborts the run; a forwards one is a garbage RT). One predicate, + `gpsFixAndTimeLocked()`, now gates the physics feed (automatic mode + too), the tree input, the staging screen and the LED search pip. No + pure-unit test: the predicate is the one-line conjunction, and the + point of the fix is that four call sites share it; the sim always + injects a resolved time, so its goldens are unchanged. From f7ecab4079361726f7c0a43af6ee716e68109adc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:38:58 +0000 Subject: [PATCH 3/8] plan 0015: re-arming the idle grace also clears a running idle timer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D3. The drag stage latch, the drag run and the sprint run re-armed the auto-idle grace by rewriting raceSessionStartedAt, but checkAutoIdle() returned early during the grace without touching an idle timer that had already started. Creep below 5 mph long enough to start the timer, then re-stage: the stale timer kept its old start, and the session ended the moment the new grace expired rather than a full hold later — exactly the staging-queue case the re-arm exists for. The grace start and the idle timer now live together in idle_policy::Clock. rearmGrace() is the only way to restart the grace and always clears the timer; advance() holds the grace/reset/hold sequence the sketch used to inline, so the whole thing is host-tested (including the creep -> re-stage regression, camera yield and millis wrap). The sketch keeps only millis() and the end-of-session effects. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/BirdsEye.ino | 45 +++++----------- BirdsEye/idle_policy.cpp | 25 +++++++++ BirdsEye/idle_policy.h | 31 +++++++++++ CHANGELOG.md | 5 ++ CLAUDE.md | 6 ++- docs/plans/0015-drag-mode.md | 18 +++++++ tests/idle_policy_test.cpp | 99 ++++++++++++++++++++++++++++++++++++ 7 files changed, 195 insertions(+), 34 deletions(-) diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index b6c746d..4cd0577 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -192,10 +192,11 @@ unsigned long dragLastRtMs = 0; // reaction time of the last manual run uint64_t dragGreenEpochMs = 0; // green-light instant, Unix epoch ms int dragPendingDistanceIdx = 0; // carried from the distance picker to // the mode page -unsigned long idleStartTime = 0; -bool idleTimerRunning = false; bool raceActive = false; -unsigned long raceSessionStartedAt = 0; // For auto-idle grace period after RPM wake +// Auto-idle grace window + idle timer as ONE struct (idle_policy::Clock, +// review D3): re-arming the grace must also clear a running idle timer, +// so idle_policy::rearmGrace() is the only way to restart it. +idle_policy::Clock idleClock; // How the active session started (RACE_ENTRY_NONE between sessions). Decides // the session-end rule and the camera driver: manual/speed sessions have no // engine signal, so they record from session start and end on the 5 min @@ -541,14 +542,14 @@ void checkForNewLapData() { } const bool stagedNow = dragTimer->staged(); if (stagedNow && !dragWasStaged) { - raceSessionStartedAt = millis(); + idle_policy::rearmGrace(idleClock, (uint32_t)millis()); } dragWasStaged = stagedNow; int runs = dragTimer->runs(); if (runs > dragLastRunCount) { dragLastRunCount = runs; - raceSessionStartedAt = millis(); + idle_policy::rearmGrace(idleClock, (uint32_t)millis()); if (lapHistoryCount < lapHistoryMaxLaps) { lastLap = dragTimer->lastEtMs(); lapHistory[lapHistoryCount] = lastLap; @@ -568,7 +569,7 @@ void checkForNewLapData() { int runs = sprintTimer->getRuns(); if (runs > sprintLastRunCount) { sprintLastRunCount = runs; - raceSessionStartedAt = millis(); + idle_policy::rearmGrace(idleClock, (uint32_t)millis()); if (lapHistoryCount < lapHistoryMaxLaps) { lastLap = sprintTimer->getLastRunTime(); lapHistory[lapHistoryCount] = lastLap; @@ -2022,7 +2023,7 @@ void trackDetectionLoop() { void startRaceSession(RaceEntryCause cause) { raceActive = true; enableLogging = true; - raceSessionStartedAt = millis(); + idle_policy::rearmGrace(idleClock, (uint32_t)millis()); raceEntryCause = cause; // Create a minimal CourseManager if none exists yet (no track detected) createLapAnythingCourseManager(); @@ -2086,8 +2087,7 @@ void endRaceSession() { detectedTrackIndex = -1; raceActive = false; raceEntryCause = RACE_ENTRY_NONE; - idleTimerRunning = false; - idleStartTime = 0; + idleClock = idle_policy::Clock{}; // Reset lap history lapHistoryCount = 0; @@ -2157,29 +2157,10 @@ void checkAutoIdle() { const idle_policy::Decision d = idle_policy::evaluate(pin); // Camera owns the end of a tach session while recording (see idle_policy - // for the rule and the GPS-lock-hold exception). - if (d.yieldToCamera) return; - - // Grace period: don't auto-idle within first 3 minutes of a session. - // After RPM wake the car is often stationary (warming up, waiting for - // track session) and GPS needs time to reacquire. Without this, the - // idle timer kills the session before the driver even moves. (Sprint - // runs re-arm it via checkForNewLapData().) - if (millis() - raceSessionStartedAt < 180000UL) return; - - if (d.resetTimer) { - idleTimerRunning = false; - idleStartTime = 0; - return; - } - - if (!idleTimerRunning) { - idleTimerRunning = true; - idleStartTime = millis(); - return; - } - - if (millis() - idleStartTime >= d.holdMs) { + // for the rule and the GPS-lock-hold exception); the 3 min grace (re- + // armed by sprint/drag runs and drag stage latches, together with the + // idle timer) and the hold itself are idle_policy::advance(). + if (idle_policy::advance(idleClock, d, (uint32_t)millis())) { if (d.stopCameraOnEnd) { debugln(F("Auto-idle: 5min at <5mph — ending session + camera")); // This ender owns the camera for manual/speed sessions: sessionDemand diff --git a/BirdsEye/idle_policy.cpp b/BirdsEye/idle_policy.cpp index 437bd9c..db96429 100644 --- a/BirdsEye/idle_policy.cpp +++ b/BirdsEye/idle_policy.cpp @@ -43,4 +43,29 @@ Decision evaluate(const Inputs& in) { return d; } +void rearmGrace(Clock& c, uint32_t nowMs) { + c.graceStartMs = nowMs; + c.idleRunning = false; + c.idleStartMs = 0; +} + +bool advance(Clock& c, const Decision& d, uint32_t nowMs) { + // Camera owns the end: leave the timer exactly as it is. + if (d.yieldToCamera) return false; + + if ((uint32_t)(nowMs - c.graceStartMs) < kSessionGraceMs) return false; + + if (d.resetTimer) { + c.idleRunning = false; + c.idleStartMs = 0; + return false; + } + if (!c.idleRunning) { + c.idleRunning = true; + c.idleStartMs = nowMs; + return false; + } + return (uint32_t)(nowMs - c.idleStartMs) >= d.holdMs; +} + } // namespace idle_policy diff --git a/BirdsEye/idle_policy.h b/BirdsEye/idle_policy.h index acfb80d..cdf922b 100644 --- a/BirdsEye/idle_policy.h +++ b/BirdsEye/idle_policy.h @@ -59,4 +59,35 @@ struct Decision { // resetTimer / "let the timer run toward holdMs" applies. Decision evaluate(const Inputs& in); +// ---- The idle clock: grace window + idle timer (review fix D3) ---- +// +// Grace: no auto-idle in the first kSessionGraceMs of a session (after an +// RPM wake the car is often stationary while GPS reacquires), and the +// grace RE-ARMS on activity that proves the session is alive: a +// completed sprint/drag run, a fresh drag STAGED latch. +// +// The two halves live in ONE struct because they must move together. +// The sketch used to re-arm by rewriting raceSessionStartedAt alone, +// while the grace check returned early without touching an idle timer +// that had already started. A creep under the idle speed that started +// the timer, then a re-stage, left the stale timer running under the +// new grace — so the session ended the instant the new grace expired +// instead of a full hold later. rearmGrace() is now the only way to +// restart the grace, and it always clears the timer with it. +constexpr uint32_t kSessionGraceMs = 180000; // 3 min + +struct Clock { + uint32_t graceStartMs = 0; + bool idleRunning = false; + uint32_t idleStartMs = 0; +}; + +// Session start, or activity that proves the session alive: restart the +// grace window AND clear any idle timer already running. +void rearmGrace(Clock& c, uint32_t nowMs); + +// Advance the clock one iteration against evaluate()'s decision. Returns +// true exactly when the session should end now. Wrap-safe uint32 millis. +bool advance(Clock& c, const Decision& d, uint32_t nowMs); + } // namespace idle_policy diff --git a/CHANGELOG.md b/CHANGELOG.md index bfb5df2..fcd550c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 when the lock landed — silently killing a run or producing a bogus reaction time. The staging screen, the LED search pip and the timer now share one "fix + time lock" check. + - A drag re-stage or completed run (and a completed sprint run) now + gives the session a full idle allowance again. Re-arming the 3-minute + grace did not stop an idle timer that had already started, so slow + queue creep followed by a re-stage could end the session the moment + the new grace ran out. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index 20afae6..cb2b31c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -128,7 +128,7 @@ desktop toolchain. This is where logic worth unit-testing lives. | File | Purpose | |---|---| | `haversine.{h,cpp}` | Great-circle distance in miles (track proximity) | -| `idle_policy.{h,cpp}` | Auto-idle session-end decision table (tach 60 s/2 mph vs manual/speed 5 min/5 mph, camera-yield + GPS-lock-hold exception, sprint engine-aware reset) + the promotion of SPEED/MANUAL sessions to TACH rules once the engine fires | +| `idle_policy.{h,cpp}` | Auto-idle session-end decision table (tach 60 s/2 mph vs manual/speed 5 min/5 mph, camera-yield + GPS-lock-hold exception, sprint engine-aware reset) + the promotion of SPEED/MANUAL sessions to TACH rules once the engine fires + the idle `Clock` (3 min grace + idle timer as one struct; `rearmGrace()` — session start, sprint/drag runs, drag stage latches — always clears a running timer too) | | `gps_stats.{h,cpp}` | GPS pipeline drop accounting: expected-vs-received PVT window math (exact fractional carry, 1-frame jitter slack, capped credit, rate-switch suppression) feeding the debug-page `Drops` counter | | `gps_time.{h,cpp}` | Leap-year/Unix-epoch math, `u64ToDecimalString` | | `gps_validation.{h,cpp}` | PVT sample sanity gate + dtostrf-output check | @@ -857,7 +857,9 @@ loop() ~250 Hz never yields to the camera — it is the only ender. **Sprint mode is engine-aware**: idle counts only while the tach reads 0 too (between-run queue waits keep the engine running), and every completed run re-arms - the 3-minute grace period. + the 3-minute grace period (`idle_policy::rearmGrace`, which clears any + idle timer already running — re-arming the grace alone left a stale + timer that ended the session as the new grace expired). - **Sprint mode (plan 0002)**: tracks under `/TRACKS/SPRINT/` make the session point-to-point. `trackDetectionLoop()` finds the nearest manifest entry PER KIND; with both kinds in range the `race_mode` diff --git a/docs/plans/0015-drag-mode.md b/docs/plans/0015-drag-mode.md index 0161221..1562292 100644 --- a/docs/plans/0015-drag-mode.md +++ b/docs/plans/0015-drag-mode.md @@ -172,3 +172,21 @@ still ends ~8 min after its last movement. The engine-aware sprint reset in - No trap-zone speed averaging (see Accuracy). - No per-distance best history across sessions — the DOVEX files are the record; the webapp is the place to compare days. + +## Review fixes (2026-09) + +Findings from the pre-release review of plans 0015 + 0016, one commit +each, each with a regression test in the pure unit that owns the rule. + +- **D3 — re-arming the idle grace left a running idle clock behind.** + The drag stage latch, the drag run and the sprint run all re-armed the + auto-idle grace by rewriting `raceSessionStartedAt`, but + `checkAutoIdle()` returned early during the grace without touching an + idle timer that had *already started*. Creep below the idle speed long + enough to start the timer, then re-stage: the stale timer kept its old + start, so the session ended the instant the new grace expired instead + of a full hold later. The grace and the timer now live together in + `idle_policy::Clock`; `rearmGrace()` is the only way to restart the + grace and always clears the timer, and `advance()` holds the whole + grace/reset/hold sequence the sketch used to inline — host-tested, + including the creep→re-stage regression and millis wrap. diff --git a/tests/idle_policy_test.cpp b/tests/idle_policy_test.cpp index 7877d73..8517fe5 100644 --- a/tests/idle_policy_test.cpp +++ b/tests/idle_policy_test.cpp @@ -116,3 +116,102 @@ TEST_CASE("idle_policy - sprint with engine off falls through to the speed rule" in.speedMph = 0.5f; CHECK(evaluate(in).resetTimer == false); // timer runs toward the end } + +// --------------------------------------------------------------------------- +// The idle clock: grace + timer (review D3) +// --------------------------------------------------------------------------- + +namespace { + +// Drive the clock at 100 ms ticks with a fixed decision; returns the ms +// offset (from `from`) at which advance() first said "end", or 0. +uint32_t runUntilEnd(Clock& c, const Decision& d, uint32_t from, + uint32_t forMs) { + for (uint32_t t = 0; t <= forMs; t += 100) { + if (advance(c, d, from + t)) return t; + } + return 0; +} + +} // namespace + +TEST_CASE("idle_policy - clock: no idle end inside the grace window") { + Clock c; + rearmGrace(c, 1000); + const Decision d = evaluate(base()); // tach, stopped: idle + CHECK(runUntilEnd(c, d, 1000, kSessionGraceMs - 100) == 0); + CHECK_FALSE(c.idleRunning); +} + +TEST_CASE("idle_policy - clock: idle holds the full hold after the grace") { + Clock c; + rearmGrace(c, 0); + const Decision d = evaluate(base()); + const uint32_t end = runUntilEnd(c, d, 0, kSessionGraceMs + 2 * kTachIdleHoldMs); + // First post-grace tick starts the timer; the end is one hold later. + CHECK(end == kSessionGraceMs + kTachIdleHoldMs); +} + +TEST_CASE("idle_policy - clock: activity resets a running timer") { + Clock c; + rearmGrace(c, 0); + Inputs in = base(); + const Decision idle = evaluate(in); + in.speedMph = 30.0f; + const Decision moving = evaluate(in); + uint32_t t = kSessionGraceMs; + CHECK_FALSE(advance(c, idle, t)); + REQUIRE(c.idleRunning); + CHECK_FALSE(advance(c, moving, t + 1000)); + CHECK_FALSE(c.idleRunning); +} + +TEST_CASE("idle_policy - clock: re-arming the grace clears an idle timer already running") { + // Regression (review D3): drag queue creep under the idle speed + // starts the timer after the grace; the car then re-stages, which + // re-arms the grace. The stale timer used to keep running under the + // new grace, so the session ended the moment the NEW grace expired. + // It must instead get a full grace AND a full hold. + Inputs in = base(); + in.speedRuleSession = true; // manual drag session: 5 min / 5 mph + const Decision idle = evaluate(in); + + Clock c; + rearmGrace(c, 0); + uint32_t t = kSessionGraceMs; + CHECK_FALSE(advance(c, idle, t)); // idle timer starts + REQUIRE(c.idleRunning); + + t += kSpeedIdleHoldMs - 10000; // 10 s short of ending... + CHECK_FALSE(advance(c, idle, t)); + rearmGrace(c, t); // ...a fresh stage latch + CHECK_FALSE(c.idleRunning); + + const uint32_t end = runUntilEnd(c, idle, t, kSessionGraceMs + 2 * kSpeedIdleHoldMs); + CHECK(end == kSessionGraceMs + kSpeedIdleHoldMs); +} + +TEST_CASE("idle_policy - clock: camera yield leaves the timer untouched") { + Clock c; + rearmGrace(c, 0); + const Decision idle = evaluate(base()); + uint32_t t = kSessionGraceMs; + CHECK_FALSE(advance(c, idle, t)); + REQUIRE(c.idleRunning); + Inputs in = base(); + in.cameraRecording = true; + const Decision yield = evaluate(in); + REQUIRE(yield.yieldToCamera); + CHECK_FALSE(advance(c, yield, t + kTachIdleHoldMs * 2)); + CHECK(c.idleRunning); + CHECK(c.idleStartMs == t); +} + +TEST_CASE("idle_policy - clock: millis wrap inside the grace and the hold") { + Clock c; + const uint32_t start = 0xFFFFFFFFu - 1000u; + rearmGrace(c, start); + const Decision d = evaluate(base()); + CHECK(runUntilEnd(c, d, start, kSessionGraceMs + 2 * kTachIdleHoldMs) == + kSessionGraceMs + kTachIdleHoldMs); +} From 3007687a31b0500b01efac6c03eaa5e438f69de5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:41:25 +0000 Subject: [PATCH 4/8] plan 0016: stamp the green light at "now", not the last fix's time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D4. dragStagingLoop() stamped the green edge with getGpsUnixTimestampMillis(), which is the time of the LAST PVT — the green lands anywhere up to a nav period (40 ms at 25 Hz) after it. The run start it is subtracted from is interpolated to fix-time accuracy, so every reaction time read 0-40 ms high with sampling-phase jitter. onPVTReceived() now records millis() at arrival (gpsPvtArrivalMillis) and the green is stamped gps_time::epochNowMs(lastEpoch, arrival, millis()); the subtraction moved to drag_tree::reactionTimeMs(). Both are pure and host-tested, including millis wrap and the old bias. The receiver's own output latency remains (not observable without PPS) and is documented as a small constant residual. The sim golden drag_staging_results changes: the same scripted pass now reads RT 0.66 instead of 0.68. Regenerated from --print output (only that one line differs). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/BirdsEye.ino | 19 ++++++++++++----- BirdsEye/drag_tree.cpp | 6 ++++++ BirdsEye/drag_tree.h | 9 ++++++++ BirdsEye/gps_functions.ino | 1 + BirdsEye/gps_time.cpp | 6 ++++++ BirdsEye/gps_time.h | 12 +++++++++++ BirdsEye/sim/golden/golden_hashes.txt | 2 +- CHANGELOG.md | 4 ++++ CLAUDE.md | 7 +++++-- docs/plans/0016-manual-drag-tree.md | 14 +++++++++++++ tests/drag_tree_test.cpp | 30 +++++++++++++++++++++++++++ tests/gps_time_test.cpp | 15 ++++++++++++++ 12 files changed, 117 insertions(+), 8 deletions(-) diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index 4cd0577..50042ac 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -97,6 +97,7 @@ #include "dovex_header.h" #include "drag_timer.h" #include "drag_tree.h" +#include "gps_time.h" // epochNowMs — the green-light stamp (drag tree RT) #include "gps_functions.h" #include "gps_status_page.h" #include "haversine.h" @@ -473,6 +474,10 @@ volatile bool gpsDataFresh = false; // Set by PVT callback, cleared by GPS_LOOP // Compare this against a remembered value instead. (gpsFrameCounter is no // help: it zeroes every second for the frame-rate maths.) volatile uint32_t gpsPvtSequence = 0; +// millis() when the last PVT arrived — with that PVT's epoch it gives +// "now" on the epoch clock between fixes (gps_time::epochNowMs). Used to +// stamp the drag tree's green light (review D4). +volatile uint32_t gpsPvtArrivalMillis = 0; // GPS nav-rate target: the rate GPS_RECONFIGURE() (and every wake/recovery // path that calls it) re-asserts. Boot starts in status mode (5 Hz + @@ -1841,14 +1846,18 @@ void dragStagingLoop() { if (fx.greenEdge) { // EPOCH ms, the same clock as runStartEpochMs() — RT is the - // difference of the two, so they must never mix time bases. - dragGreenEpochMs = getGpsUnixTimestampMillis(); + // difference of the two, so they must never mix time bases. And + // "now" on that clock, not the last fix's time: the green lands up + // to a nav period after the last PVT, and stamping it with the + // fix's time read every RT 0-40 ms high with jitter (review D4). + dragGreenEpochMs = gps_time::epochNowMs(getGpsUnixTimestampMillis(), + gpsPvtArrivalMillis, + (uint32_t)millis()); dragLastRtMs = 0; } if (fx.runStartEdge) { - const uint64_t s = dragTimer->runStartEpochMs(); - dragLastRtMs = - (s > dragGreenEpochMs) ? (unsigned long)(s - dragGreenEpochMs) : 0; + dragLastRtMs = drag_tree::reactionTimeMs(dragTimer->runStartEpochMs(), + dragGreenEpochMs); } if (fx.consumedButton) { resetButtons(); // the press must not also drive displayLoop() diff --git a/BirdsEye/drag_tree.cpp b/BirdsEye/drag_tree.cpp index f0c0f11..0366621 100644 --- a/BirdsEye/drag_tree.cpp +++ b/BirdsEye/drag_tree.cpp @@ -194,4 +194,10 @@ char countdownDigit(Stage st) { bool stripActive(Stage st) { return st != Stage::kRunning; } +unsigned long reactionTimeMs(uint64_t runStartEpochMs, uint64_t greenEpochMs) { + if (runStartEpochMs == 0 || greenEpochMs == 0) return 0; + if (runStartEpochMs <= greenEpochMs) return 0; + return (unsigned long)(runStartEpochMs - greenEpochMs); +} + } // namespace drag_tree diff --git a/BirdsEye/drag_tree.h b/BirdsEye/drag_tree.h index e64522b..a747fff 100644 --- a/BirdsEye/drag_tree.h +++ b/BirdsEye/drag_tree.h @@ -136,6 +136,15 @@ char countdownDigit(Stage st); // display's 3 Hz refresh. bool flashPhase(uint32_t nowMs); +// Reaction time from two Unix-epoch-ms instants: the physics' +// interpolated rollout crossing (DragTimer::runStartEpochMs) minus the +// green-light stamp. 0 when either is missing or the run started before +// green (cannot happen with the launch gate closed, but a clamp beats +// an unsigned wrap to ~49 days on the results screen). The green stamp +// must be taken with gps_time::epochNowMs — "now", not the last fix's +// time — or RT reads high by up to a nav period (review D4). +unsigned long reactionTimeMs(uint64_t runStartEpochMs, uint64_t greenEpochMs); + // False only for kRunning: the strip returns to the normal race compose // (RPM/speed scale) while the pass is being driven. bool stripActive(Stage st); diff --git a/BirdsEye/gps_functions.ino b/BirdsEye/gps_functions.ino index 3de63a8..7db9b94 100644 --- a/BirdsEye/gps_functions.ino +++ b/BirdsEye/gps_functions.ino @@ -221,6 +221,7 @@ void onPVTReceived(UBX_NAV_PVT_data_t *pvt) { gpsDataFresh = true; gpsPvtSequence++; // monotonic; for consumers that run after GPS_LOOP() + gpsPvtArrivalMillis = millis(); // epoch 'now' between fixes (gps_time::epochNowMs) gpsFrameCounter++; } diff --git a/BirdsEye/gps_time.cpp b/BirdsEye/gps_time.cpp index 21cd9dc..da2f5e7 100644 --- a/BirdsEye/gps_time.cpp +++ b/BirdsEye/gps_time.cpp @@ -77,4 +77,10 @@ size_t u64ToDecimalString(uint64_t val, char* buf, size_t buf_size) { return digits; } +uint64_t epochNowMs(uint64_t lastPvtEpochMs, uint32_t pvtArrivalMillis, + uint32_t nowMillis) { + if (lastPvtEpochMs == 0) return 0; + return lastPvtEpochMs + (uint32_t)(nowMillis - pvtArrivalMillis); +} + } // namespace gps_time diff --git a/BirdsEye/gps_time.h b/BirdsEye/gps_time.h index e9b0823..32d71bf 100644 --- a/BirdsEye/gps_time.h +++ b/BirdsEye/gps_time.h @@ -31,6 +31,18 @@ uint64_t unixTimestampMillis(uint16_t year, uint8_t month, uint8_t day, uint8_t hour, uint8_t minute, uint8_t sec, uint16_t ms); +// "Now" on the Unix-epoch-ms clock, between PVT fixes: the epoch of the +// last PVT plus the host time elapsed since it ARRIVED (both host times +// wrap-safe uint32 millis). getGpsUnixTimestampMillis() alone is the +// last fix's time — up to a whole nav period (40 ms at 25 Hz) stale and +// jittering with the sampling phase, which is fine for a log row and +// wrong for stamping an instant like the drag tree's green light +// (review D4). 0 while no PVT epoch exists. The receiver's own output +// latency (fix time -> callback) is NOT recoverable without a PPS line, +// so the result still trails true time by that small constant. +uint64_t epochNowMs(uint64_t lastPvtEpochMs, uint32_t pvtArrivalMillis, + uint32_t nowMillis); + // Convert an unsigned 64-bit value to a decimal ASCII string. // Writes at most 21 chars (20 digits for UINT64_MAX + null terminator). // Returns the number of digits written, NOT counting the null terminator. diff --git a/BirdsEye/sim/golden/golden_hashes.txt b/BirdsEye/sim/golden/golden_hashes.txt index 08185ff..94e9f25 100644 --- a/BirdsEye/sim/golden/golden_hashes.txt +++ b/BirdsEye/sim/golden/golden_hashes.txt @@ -18,6 +18,6 @@ course_line_point_a_done -14 646ca520 main_menu_after_create -1 14716beb main_menu_parked_on_line -1 14716beb drag_staging_running -19 28d73474 -drag_staging_results -19 f02a745d +drag_staging_results -19 c57c486d pair_camera_unpaired -6 5d1129cc camera_serial_entry -7 56588408 diff --git a/CHANGELOG.md b/CHANGELOG.md index fcd550c..1ec6086 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 grace did not stop an idle timer that had already started, so slow queue creep followed by a re-stage could end the session the moment the new grace ran out. + - Manual drag reaction time no longer reads 0–40 ms high. The green + light was stamped with the last GPS fix's time instead of "now", so + RT carried up to a whole GPS update period of error, varying run to + run. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index cb2b31c..69419f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,7 +130,7 @@ desktop toolchain. This is where logic worth unit-testing lives. | `haversine.{h,cpp}` | Great-circle distance in miles (track proximity) | | `idle_policy.{h,cpp}` | Auto-idle session-end decision table (tach 60 s/2 mph vs manual/speed 5 min/5 mph, camera-yield + GPS-lock-hold exception, sprint engine-aware reset) + the promotion of SPEED/MANUAL sessions to TACH rules once the engine fires + the idle `Clock` (3 min grace + idle timer as one struct; `rearmGrace()` — session start, sprint/drag runs, drag stage latches — always clears a running timer too) | | `gps_stats.{h,cpp}` | GPS pipeline drop accounting: expected-vs-received PVT window math (exact fractional carry, 1-frame jitter slack, capped credit, rate-switch suppression) feeding the debug-page `Drops` counter | -| `gps_time.{h,cpp}` | Leap-year/Unix-epoch math, `u64ToDecimalString` | +| `gps_time.{h,cpp}` | Leap-year/Unix-epoch math, `u64ToDecimalString`, `epochNowMs` (epoch "now" between fixes = last PVT epoch + millis since its arrival) | | `gps_validation.{h,cpp}` | PVT sample sanity gate + dtostrf-output check | | `dovex_header.{h,cpp}` | DOVEX 1 KB header `format()` / `parse()` | | `filename_validator.{h,cpp}` | FAT-safe / traversal-proof check for BLE filenames | @@ -926,7 +926,10 @@ loop() ~250 Hz FAILED TO LAUNCH; a mid-run physics abort surfaces as RUN ABORTED — all three flash the strip red and wait for a button. **RT** = the interpolated rollout crossing (`runStartEpochMs()`) minus the green - epoch, both Unix epoch ms — display-only (results screen), not in the + epoch (`drag_tree::reactionTimeMs`), both Unix epoch ms — the green + stamped as "now" via `gps_time::epochNowMs` (last PVT epoch + millis + since that PVT arrived, `gpsPvtArrivalMillis`), never the last fix's + time, which read RT up to a nav period high — display-only (results screen), not in the DOVEX header. The display is **pinned** to `PAGE_DRAG_STAGING` for the whole manual session (gpsLockHold construction); presses are consumed by `dragStagingLoop()` (the `gpsStatusPageLoop()` slot: diff --git a/docs/plans/0016-manual-drag-tree.md b/docs/plans/0016-manual-drag-tree.md index 527d15d..9a3b46d 100644 --- a/docs/plans/0016-manual-drag-tree.md +++ b/docs/plans/0016-manual-drag-tree.md @@ -172,3 +172,17 @@ each, each with a regression test in the pure unit that owns the rule. pure-unit test: the predicate is the one-line conjunction, and the point of the fix is that four call sites share it; the sim always injects a resolved time, so its goldens are unchanged. +- **D4 — reaction time read high with jitter.** The green edge was + stamped `getGpsUnixTimestampMillis()`, which is the *last fix's* time: + the green lands anywhere up to a nav period (40 ms at 25 Hz) after it, + so every RT read 0–40 ms high with sampling-phase jitter, while the run + start it is subtracted from is interpolated to fix-time accuracy. The + PVT callback now records `millis()` at arrival + (`gpsPvtArrivalMillis`), the green is stamped + `gps_time::epochNowMs(lastEpoch, arrivalMillis, millis())`, and the + subtraction is `drag_tree::reactionTimeMs()` — both pure and + host-tested. Residual, stated rather than hidden: the receiver's own + output latency (fix time → callback) is a small constant the firmware + cannot see without a PPS line, so RT still trails by that. The sim's + `drag_staging_results` golden moved (RT 0.68 → 0.66 on the same + scripted pass) and was regenerated. diff --git a/tests/drag_tree_test.cpp b/tests/drag_tree_test.cpp index fce655f..f5aa240 100644 --- a/tests/drag_tree_test.cpp +++ b/tests/drag_tree_test.cpp @@ -4,6 +4,7 @@ #include "doctest.h" #include "drag_tree.h" +#include "gps_time.h" #include "led_frame.h" using drag_tree::Effects; @@ -424,3 +425,32 @@ TEST_CASE("countdownDigit and stripActive maps") { CHECK(drag_tree::stripActive(Stage::kRedLight)); CHECK_FALSE(drag_tree::stripActive(Stage::kRunning)); } + +// --------------------------------------------------------------------------- +// Reaction time (review D4) +// --------------------------------------------------------------------------- + +TEST_CASE("reactionTimeMs: run start minus green, clamped") { + CHECK(drag_tree::reactionTimeMs(1785077000500ull, 1785077000000ull) == 500u); + CHECK(drag_tree::reactionTimeMs(1785077000000ull, 1785077000000ull) == 0u); + CHECK(drag_tree::reactionTimeMs(1785077000000ull, 1785077000500ull) == 0u); + CHECK(drag_tree::reactionTimeMs(0, 1785077000000ull) == 0u); + CHECK(drag_tree::reactionTimeMs(1785077000500ull, 0) == 0u); +} + +TEST_CASE("RT: green stamped at 'now' on the epoch clock, not the last fix") { + // The last PVT (epoch E) arrived at host millis 10000; the tree goes + // green 35 ms later, between fixes. The driver's rollout crossing is + // interpolated at E + 535 by the physics (fix-time accurate). True RT + // is 500 ms. Stamping green with the last fix's epoch (the pre-fix + // glue) read 535 — high by the sampling phase, 0-40 ms of jitter at + // 25 Hz. + const uint64_t e = 1785077000000ull; + const uint32_t arrival = 10000; + const uint32_t greenMillis = arrival + 35; + const uint64_t runStart = e + 535; + + const uint64_t green = gps_time::epochNowMs(e, arrival, greenMillis); + CHECK(drag_tree::reactionTimeMs(runStart, green) == 500u); + CHECK(drag_tree::reactionTimeMs(runStart, e) == 535u); // the old bias +} diff --git a/tests/gps_time_test.cpp b/tests/gps_time_test.cpp index d269569..7d8d4a9 100644 --- a/tests/gps_time_test.cpp +++ b/tests/gps_time_test.cpp @@ -196,3 +196,18 @@ TEST_CASE("u64ToDecimalString - zero-size buffer returns 0 without writing") { CHECK(u64ToDecimalString(42, &sentinel, 0) == 0u); CHECK(sentinel == 'Z'); // untouched } + +TEST_CASE("epochNowMs - extrapolates the last PVT epoch by host time since arrival") { + const uint64_t e = 1785077000000ull; + CHECK(epochNowMs(e, 5000, 5000) == e); + CHECK(epochNowMs(e, 5000, 5037) == e + 37); +} + +TEST_CASE("epochNowMs - millis wrap between arrival and now") { + const uint64_t e = 1785077000000ull; + CHECK(epochNowMs(e, 0xFFFFFFF0u, 0x10u) == e + 0x20); +} + +TEST_CASE("epochNowMs - no PVT epoch yet stays 0") { + CHECK(epochNowMs(0, 5000, 9000) == 0u); +} From d94f9904538cbbe4d87c3f03c2f26c52d0e5ab9d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:42:03 +0000 Subject: [PATCH 5/8] plan 0016: the drag Select-hold exit stops a paired camera MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D5. The manual tree's exit effect called endRaceSession() directly. endRaceSession() deliberately never touches the camera (a tach session's recording must outlive a stationary grid idle), so every ender that owns the camera notifies it first — and the LOGGING STOP confirm did, while the drag exit did not: a paired Insta360 kept recording after the user explicitly ended the session. Both user-initiated enders now go through endRaceSessionByUser() (CAMERA_NOTIFY_SESSION_END, then endRaceSession), so the pair cannot drift apart again. Pure glue with no decision to extract; covered by the sim build/walk (camera surface stubbed there). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/BirdsEye.ino | 25 ++++++++++++++++++++----- BirdsEye/display_ui.ino | 3 +-- BirdsEye/sim/sim_prototypes.h | 1 + CHANGELOG.md | 3 +++ CLAUDE.md | 4 +++- docs/plans/0016-manual-drag-tree.md | 8 ++++++++ 6 files changed, 36 insertions(+), 8 deletions(-) diff --git a/BirdsEye/BirdsEye.ino b/BirdsEye/BirdsEye.ino index 50042ac..e6b3612 100644 --- a/BirdsEye/BirdsEye.ino +++ b/BirdsEye/BirdsEye.ino @@ -1863,7 +1863,9 @@ void dragStagingLoop() { resetButtons(); // the press must not also drive displayLoop() } if (fx.exitSession) { - endRaceSession(); // clears dragManualMode -> releases the pin + // User-initiated: stops a paired camera too, exactly like the + // LOGGING STOP confirm. Clears dragManualMode -> releases the pin. + endRaceSessionByUser(); switchToDisplayPage(PAGE_MAIN_MENU); resetButtons(); } @@ -2038,17 +2040,30 @@ void startRaceSession(RaceEntryCause cause) { createLapAnythingCourseManager(); } +/** + * @brief The user's explicit "I'm done": stop the camera recording + * immediately (bypassing its stationary+engine-off hold), then end the + * session. The ONE path for every user-initiated ender — the LOGGING + * STOP confirm and the manual drag Select-hold exit. Two hand-written + * copies drifted once: the drag exit called endRaceSession() alone and + * left a paired camera recording in the staging lane (review D5). + */ +void endRaceSessionByUser() { + CAMERA_NOTIFY_SESSION_END(); + endRaceSession(); +} + /** * @brief End the current race session: write DOVEX header, close file, - * clean up CourseManager, reset state. Used by both checkAutoIdle() - * and LOGGING_STOP_CONFIRM in display_ui.ino. + * clean up CourseManager, reset state. Used by checkAutoIdle() and, via + * endRaceSessionByUser(), by the user-initiated enders. */ void endRaceSession() { // Deliberately NO camera notification here: for TACH sessions the // camera must keep recording through a stationary grid idle — its own // stationary-AND-engine-off rule decides the recording stop. The - // camera is stopped explicitly where the ender owns it: the manual - // stop confirm (display_ui.ino), the manual/speed-session idle timer + // camera is stopped explicitly where the ender owns it: the user's + // own enders (endRaceSessionByUser()), the manual/speed-session idle timer // (checkAutoIdle() calls CAMERA_NOTIFY_SESSION_END() itself before // this), and shutdown entry (CAMERA_SLEEP() in enterShutdown()). diff --git a/BirdsEye/display_ui.ino b/BirdsEye/display_ui.ino index 35fcd86..88d8159 100644 --- a/BirdsEye/display_ui.ino +++ b/BirdsEye/display_ui.ino @@ -604,8 +604,7 @@ void handleMenuPageSelection() { // recording immediately too (bypasses its stationary+engine-off // hold). Auto-idle deliberately does NOT do this — see the comment // in endRaceSession(). - CAMERA_NOTIFY_SESSION_END(); - endRaceSession(); + endRaceSessionByUser(); switchToDisplayPage(PAGE_MAIN_MENU); } debug(F("Stop Logging?: ")); diff --git a/BirdsEye/sim/sim_prototypes.h b/BirdsEye/sim/sim_prototypes.h index f200766..9c280ed 100644 --- a/BirdsEye/sim/sim_prototypes.h +++ b/BirdsEye/sim/sim_prototypes.h @@ -84,6 +84,7 @@ unsigned long dragLastReactionMs(); void trackDetectionLoop(); void startRaceSession(RaceEntryCause cause); void endRaceSession(); +void endRaceSessionByUser(); void createLapAnythingCourseManager(); void checkAutoIdle(); void autoRaceModeCheck(); diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ec6086..ce055f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 light was stamped with the last GPS fix's time instead of "now", so RT carried up to a whole GPS update period of error, varying run to run. + - Leaving manual drag mode with the Select hold now stops a paired + Insta360 recording, the same as Stop Logging does. It used to keep + recording after the session ended. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index 69419f9..7b2ecb9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -934,7 +934,9 @@ loop() ~250 Hz the whole manual session (gpsLockHold construction); presses are consumed by `dragStagingLoop()` (the `gpsStatusPageLoop()` slot: after `readButtons()`, `resetButtons()` on consumption). Exit = hold - Select 2 s in any non-running state (`kExitHoldMs`; a held side + Select 2 s in any non-running state — through `endRaceSessionByUser()`, + the one user-initiated ender shared with the LOGGING STOP confirm + (camera notify, then end) (`kExitHoldMs`; a held side button disarms it so the reboot combo wins; the pin's only gate is `dragManualMode`, cleared in `endRaceSession()`, so every session ender releases it). The LED tree renders strip-only via a new arm in diff --git a/docs/plans/0016-manual-drag-tree.md b/docs/plans/0016-manual-drag-tree.md index 9a3b46d..cc80b68 100644 --- a/docs/plans/0016-manual-drag-tree.md +++ b/docs/plans/0016-manual-drag-tree.md @@ -186,3 +186,11 @@ each, each with a regression test in the pure unit that owns the rule. cannot see without a PPS line, so RT still trails by that. The sim's `drag_staging_results` golden moved (RT 0.68 → 0.66 on the same scripted pass) and was regenerated. +- **D5 — the Select-hold exit left a paired camera recording.** The + tree's exit effect called `endRaceSession()` alone, which deliberately + never touches the camera (a tach session's camera outlives a grid + idle); the LOGGING STOP confirm — the other user-initiated ender — + notifies the camera first. Both now go through one + `endRaceSessionByUser()` (camera notify, then end), so the two can't + drift again. Sketch glue with no decision in it, so no pure test; the + sim builds and walks the exit path (its camera surface is a stub). From 1664e76a6b80c5672baef59c6c4437a5f34f716d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:42:59 +0000 Subject: [PATCH 6/8] plan 0015: no LED pace pip in automatic drag either MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D6. NEOPIXEL_LOOP()'s paceValid became true once one run existed and the next was live, and drag's pace accessor is hard-wired 0.0 (there is no reference to pace against). From run 2 on, an automatic pass therefore got the pace pip parked in its deadband — one dim centre pixel — instead of the RPM/speed scale run 1 had. Plan 0016 had patched this for manual drag only (!dragManualActive()). The gate is now led_modes::paceValid(PaceGate), pure and host-tested, with hasPaceReference = !dragModeIsActive() — covering both drag modes by construction rather than by a per-mode exception. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/led_modes.cpp | 7 ++++++ BirdsEye/led_modes.h | 18 +++++++++++++ BirdsEye/neopixel.ino | 20 +++++++-------- CHANGELOG.md | 3 +++ CLAUDE.md | 11 +++++--- docs/plans/0015-drag-mode.md | 10 +++++++- docs/plans/0016-manual-drag-tree.md | 4 ++- tests/led_modes_test.cpp | 39 +++++++++++++++++++++++++++++ 8 files changed, 96 insertions(+), 16 deletions(-) diff --git a/BirdsEye/led_modes.cpp b/BirdsEye/led_modes.cpp index 8bf77ef..aa27c8b 100644 --- a/BirdsEye/led_modes.cpp +++ b/BirdsEye/led_modes.cpp @@ -127,4 +127,11 @@ void renderSearchPip(uint32_t tMs, Rgb out[kStripCount]) { out[pos] = led_frame::kGreen; } +bool paceValid(const PaceGate& g) { + if (!g.hasPaceReference) return false; + if (!g.raceStarted || g.laps < 1) return false; + if (g.runMode && !g.runActive) return false; + return true; +} + } // namespace led_modes diff --git a/BirdsEye/led_modes.h b/BirdsEye/led_modes.h index 9d05b3d..37dc72d 100644 --- a/BirdsEye/led_modes.h +++ b/BirdsEye/led_modes.h @@ -55,6 +55,24 @@ PacePip pacePip(float paceMsPerM); // Render the full 9-px strip: dim white centerline + the pip. void renderPace(float paceMsPerM, led_frame::Rgb out[led_frame::kStripCount]); +// Whether the strip should show the pace pip at all (else it falls +// through to the RPM/speed scale). Needs a started race with a completed +// lap/run to pace against, a run in progress in the run-based modes +// (sprint, drag — between runs there is nothing to pace), AND a mode +// that actually HAS a pace reference. Drag has none (its pace accessor +// is hard-wired 0.0), so every drag run — automatic or manual — gets +// the scale; before this was a pure function, only manual drag was +// excluded and automatic runs 2+ showed nothing but the dim centerline +// (review D6). +struct PaceGate { + bool raceStarted = false; + int laps = 0; // completed laps / runs + bool runMode = false; // sprint or drag + bool runActive = false; // meaningful only in runMode + bool hasPaceReference = true; // false in drag mode +}; +bool paceValid(const PaceGate& g); + // ---- Generic scale (RPM now, temps later) ------------------------------ struct ScaleSpec { diff --git a/BirdsEye/neopixel.ino b/BirdsEye/neopixel.ino index 52f00c5..5575926 100644 --- a/BirdsEye/neopixel.ino +++ b/BirdsEye/neopixel.ino @@ -402,16 +402,16 @@ void NEOPIXEL_LOOP() { // RPM/speed arms below take over for the pass. drag_tree::renderStrip(dragTreeStage(), now, stripPx); } else { - const bool paceValid = - activeTimerRaceStarted() && activeTimerLaps() >= 1 && - !((sprintModeIsActive() || dragModeIsActive()) && - !activeTimerRunActive()) && - // Manual drag has no reference to pace against (the pace - // accessor is hardwired 0.0) — suppress the centered pip - // so runs 2+ get the RPM/speed scale like run 1. Constant - // false for auto drag/sprint/circuit, so nothing changes. - !dragManualActive(); - if (paceValid) { + led_modes::PaceGate pg; + pg.raceStarted = activeTimerRaceStarted(); + pg.laps = activeTimerLaps(); + pg.runMode = sprintModeIsActive() || dragModeIsActive(); + pg.runActive = activeTimerRunActive(); + // Drag has no reference to pace against (the pace accessor is + // hard-wired 0.0) — automatic AND manual, so every run gets + // the RPM/speed scale instead of a lone centerline (review D6). + pg.hasPaceReference = !dragModeIsActive(); + if (led_modes::paceValid(pg)) { led_modes::renderPace(activeTimerPaceDifference(), stripPx); } else if (raceEntryCause == RACE_ENTRY_TACH) { const led_modes::ScaleSpec rpmSpec = { diff --git a/CHANGELOG.md b/CHANGELOG.md index ce055f0..94956cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,9 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 - Leaving manual drag mode with the Select hold now stops a paired Insta360 recording, the same as Stop Logging does. It used to keep recording after the session ended. + - Automatic drag runs after the first show the RPM/speed LED bar again + instead of a single dim centre pixel (a pace display with nothing to + pace against). - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index 7b2ecb9..a5353e7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -142,7 +142,7 @@ desktop toolchain. This is where logic worth unit-testing lives. | `loop_profile.{h,cpp}` | Main-loop CPU accounting: per-section tick accumulation, saturating (never wrapping — a uint32 of DWT ticks is only ~67 s), and the once-a-second rollup into shares of wall time, loop rate, mean and worst iteration, plus measured idle (`SLP`). **Two clocks on purpose**: durations in TICKS with `ticksPerUs` supplied at rollup (so a sub-microsecond section is not quantised to zero), but the WINDOW closed on `millis()` — DWT counts cycles and stops when the core halts, and using it as a wall clock inflated the very first hardware reading. Board-portable by construction — the nRF5340 comparison needs the same instrument | | `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, speed with no red band at all), the `StatusAction` threshold/hysteresis/flash table, and `flashOn()` — the ONE definition of flash phase, shared with `led_status` | +| `led_modes.{h,cpp}` | Strip modes + status actions: pace pip math (ms/m, slower = left/red) + `paceValid()` (when the pip shows at all), generic `ScaleSpec` left-fill (RPM red past halfway, speed with no red band at all), the `StatusAction` threshold/hysteresis/flash table, and `flashOn()` — the ONE definition of flash phase, shared with `led_status` | | `led_status.{h,cpp}` | The eight assignable status-LED modes (subsystem 16): the mode enum + strict name parser that the `led_status_left`/`led_status_right` settings store, the GPS and camera readiness ladders, and `evalMode()` — which delegates every threshold mode to `led_modes::evalStatus` rather than re-implementing hysteresis. `Inputs.eggSupported` is why an `egt` LED is dark on a stock build instead of a permanent solid blue | | `led_animations.{h,cpp}` | Boot + purple-sector animations as pure functions of `(tMs, seed)` — hash-based sparkles, no rand()/millis(), golden-testable | | `sector_purple.{h,cpp}` | The lap/sector CLOSE-EDGE monitor (the name predates half its job): open-time best snapshots + a derived S3 defeat the library's lap-line `updateBestSectors()` race, and the same trick one level up defeats it for `getBestLapTime()`. Emits which sector or lap just closed, its verdict **against the last recorded one** (not the best — that only ever answers purple or red), and the two purple flags. No purple on lap 1 | @@ -912,7 +912,8 @@ loop() ~250 Hz rotation pages — the Current Lap page shows live ET / last ET + `trap`/`0-60` subtext / `*staged*`; the Pace page becomes the live 0-60 readout; the Best Lap page adds the best run's trap/0-60; the LED pace - pip is suppressed between runs like sprint. + pip never shows in drag (no pace reference — every run, automatic or + manual, gets the RPM/speed scale; `led_modes::paceValid`). - **Manual drag mode (plan 0016)**: the Manual row runs the same physics behind a **christmas tree**. The host-tested `drag_tree` unit is the ONE sequencer driving both outputs: LED strip `----w----` (staged @@ -1599,8 +1600,10 @@ hardware needs no power switch. Wake = chip reset = fresh `setup()`. 2. **No GPS lock** (`!(gpsData.fix && gpsData.timeValid)` — the log-file-creation gate) → green **search pip** bouncing end-to-end (`renderSearchPip`, 1.6 s round-trip triangle wave). - 3. **Pace valid** (`activeTimerRaceStarted() && laps >= 1 && !(sprint - && between-runs)`, mirroring the OLED pace page) → the **pace + 3. **Pace valid** (`led_modes::paceValid`, host-tested: + `activeTimerRaceStarted() && laps >= 1 && !(sprint/drag && + between-runs)` and never in drag, which has no pace reference; + mirroring the OLED pace page) → the **pace pip**: `activeTimerPaceDifference()` is **ms per meter**, positive = slower; full deflection ±1.0 ms/m (`kPaceFullScaleMsPerM`, 0.25/pixel), ±0.125 deadband = dim-white centerline only. Slower = diff --git a/docs/plans/0015-drag-mode.md b/docs/plans/0015-drag-mode.md index 1562292..d5ae7cb 100644 --- a/docs/plans/0015-drag-mode.md +++ b/docs/plans/0015-drag-mode.md @@ -161,7 +161,8 @@ still ends ~8 min after its last movement. The engine-aware sprint reset in `*waiting*` between runs; the Best Lap page adds a best-run `trap / 0-60` subtext; the Pace page shows the live 0–60 status during a run instead of a meaningless +0.00 pace. The LED pace pip is suppressed - between runs exactly like sprint. + between runs exactly like sprint (and, since review fix D6, during runs + too — drag has no pace reference). - No settings persistence for the distance choice — the picker is two presses, and a stale remembered distance is worse than none. @@ -190,3 +191,10 @@ each, each with a regression test in the pure unit that owns the rule. grace and always clears the timer, and `advance()` holds the whole grace/reset/hold sequence the sketch used to inline — host-tested, including the creep→re-stage regression and millis wrap. +- **D6 — automatic runs 2+ showed only the dim centerline.** The strip's + `paceValid` turned true once a run existed and a new one was live, + and drag's pace accessor is hard-wired 0.0 — so from the second run on + the pass got the pace pip parked in its deadband (one dim pixel) + instead of the RPM/speed scale. Plan 0016 had excluded manual drag + only. The gate is now `led_modes::paceValid(PaceGate)`, pure and + host-tested, with `hasPaceReference = !dragModeIsActive()`. diff --git a/docs/plans/0016-manual-drag-tree.md b/docs/plans/0016-manual-drag-tree.md index cc80b68..0f91c89 100644 --- a/docs/plans/0016-manual-drag-tree.md +++ b/docs/plans/0016-manual-drag-tree.md @@ -119,7 +119,9 @@ their priority above the whole branch. During the run (`kRunning`) the arm goes inactive and the normal RPM/speed scale takes over; `paceValid` gains `!dragManualActive()` so manual runs 2+ get the scale instead of a meaningless centered 0.0-pace pip (constant false for every -other mode — auto unchanged). +other mode — auto unchanged). *Superseded by review fix D6 (plan 0015): +automatic drag had the same problem, so the exclusion is now all of drag +mode, in the host-tested `led_modes::paceValid`.* ## Menu diff --git a/tests/led_modes_test.cpp b/tests/led_modes_test.cpp index fdc09d0..f812542 100644 --- a/tests/led_modes_test.cpp +++ b/tests/led_modes_test.cpp @@ -379,3 +379,42 @@ TEST_CASE("evalStatus treats a new Source tag like any other non-kNone") { CHECK(s1.active == s2.active); } } + +// --------------------------------------------------------------------------- +// Pace gate (review D6) +// --------------------------------------------------------------------------- + +TEST_CASE("paceValid: circuit paces once a lap exists") { + led_modes::PaceGate g; + g.raceStarted = true; + g.laps = 0; + CHECK_FALSE(led_modes::paceValid(g)); + g.laps = 1; + CHECK(led_modes::paceValid(g)); + g.raceStarted = false; + CHECK_FALSE(led_modes::paceValid(g)); +} + +TEST_CASE("paceValid: sprint paces only during a run") { + led_modes::PaceGate g; + g.raceStarted = true; + g.laps = 2; + g.runMode = true; + g.runActive = false; + CHECK_FALSE(led_modes::paceValid(g)); + g.runActive = true; + CHECK(led_modes::paceValid(g)); +} + +TEST_CASE("paceValid: drag never paces, automatic runs 2+ included") { + // Regression: automatic drag run 2 (laps >= 1, run active) used to + // show the pace pip for a pace delta hard-wired to 0 — just the dim + // centerline — because only MANUAL drag was excluded. + led_modes::PaceGate g; + g.raceStarted = true; + g.laps = 1; + g.runMode = true; + g.runActive = true; + g.hasPaceReference = false; + CHECK_FALSE(led_modes::paceValid(g)); +} From 8e17e1c8f2a3703d12826a59167c96ed047a50cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:45:29 +0000 Subject: [PATCH 7/8] plan 0015: don't record the return-road drive as a drag run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D7. In automatic mode a car stopped in the shutdown area stages, and driving the return road back at 20-30 mph then clears the rollout, the 15 mph prove-out and the target distance — a slow bogus run in lapHistory and the DOVEX laps line. A completed run is now discarded (rejectedRuns() counts it) only when BOTH hold: its launch->finish chord points more than 90 deg from the last recorded run's (a strip runs one way, the return road the other), AND its trap speed is below kCruiseTrapRatio (1.2) x its average speed. The ratio is the physics of a pass — from a standstill at full effort trap/average is 2.0 at constant acceleration, 1.5 at constant power, and above ~1.3 even for a car that tops out early — while a drive that settles into a cruise sits near 1.0-1.15. Requiring both keeps it conservative: the first run of a session, every same-direction run and every full-effort run in either direction are kept. A trap floor or ET ceiling was rejected because karts and brisk return-road drives overlap on both. Rationale recorded in plan 0015. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/drag_timer.cpp | 30 +++++++++- BirdsEye/drag_timer.h | 32 ++++++++++ CHANGELOG.md | 5 ++ CLAUDE.md | 7 ++- docs/plans/0015-drag-mode.md | 23 ++++++- tests/drag_timer_test.cpp | 113 +++++++++++++++++++++++++++++++++++ 6 files changed, 207 insertions(+), 3 deletions(-) diff --git a/BirdsEye/drag_timer.cpp b/BirdsEye/drag_timer.cpp index a5b17c4..b57af91 100644 --- a/BirdsEye/drag_timer.cpp +++ b/BirdsEye/drag_timer.cpp @@ -2,6 +2,8 @@ #include "haversine.h" +#include + namespace drag_timer { namespace { @@ -24,6 +26,9 @@ const char* const kDovexNames[kDistanceCount] = { }; double lerp(double a, double b, double f) { return a + (b - a) * f; } + +constexpr double kPi = 3.14159265358979323846; +constexpr double kMphPerFtPerSec = 3600.0 / 5280.0; } // namespace float targetFeet(int idx) { @@ -52,6 +57,8 @@ void DragTimer::setTarget(int idx) { targetIdx_ = idx; targetFt_ = kTargetsFt[idx]; havePrev_ = false; + haveRunDir_ = false; + rejectedRuns_ = 0; runs_ = 0; lastEtMs_ = 0; lastTrapMph_ = 0.0f; @@ -162,6 +169,8 @@ bool DragTimer::onFix(double lat, double lng, float speedMph, runStartMs_ = (double)gpsTimeMs; } phase_ = Phase::kLaunched; + launchLat_ = anchorLat_; + launchLng_ = anchorLng_; runDistFt_ = (float)(d - kRolloutFt); // overshoot past rollout provenOut_ = speedMph >= kProveOutMph; sixtyCrossed_ = false; @@ -223,9 +232,28 @@ bool DragTimer::onFix(double lat, double lng, float speedMph, const double f = (targetFt_ - distBefore) / stepFt; const double tFin = lerp((double)prevTimeMs_, (double)gpsTimeMs, f); const float trap = (float)lerp(prevSpeedMph_, speedMph, f); + const double etMs = tFin - runStartMs_; + + // Return-road gate (D7): heading back AND cruising -> not a pass. + const double cosLat = cos(launchLat_ * kPi / 180.0); + const double dirN = lat - launchLat_; + const double dirE = (lng - launchLng_) * cosLat; + const bool headingBack = + haveRunDir_ && (dirN * runDirN_ + dirE * runDirE_) < 0.0; + const double avgMph = + etMs > 0.0 ? targetFt_ / (etMs / 1000.0) * kMphPerFtPerSec : 0.0; + const bool cruising = trap < kCruiseTrapRatio * avgMph; + if (headingBack && cruising) { + rejectedRuns_++; + resetToArmed(); + break; + } + haveRunDir_ = true; + runDirN_ = dirN; + runDirE_ = dirE; runs_++; - lastEtMs_ = (unsigned long)(tFin - runStartMs_ + 0.5); + lastEtMs_ = (unsigned long)(etMs + 0.5); lastTrapMph_ = trap; last0to60Ms_ = run0to60Ms_; if (bestEtMs_ == 0 || lastEtMs_ < bestEtMs_) { diff --git a/BirdsEye/drag_timer.h b/BirdsEye/drag_timer.h index 12f66d5..db4cd6e 100644 --- a/BirdsEye/drag_timer.h +++ b/BirdsEye/drag_timer.h @@ -92,6 +92,26 @@ constexpr uint32_t kProveOutMs = 5000; // to launch; only a real slow reposition trips it. constexpr float kRestageFt = 10.0f; +// Return-road rejection (review fix D7). A car stopped in the shutdown +// area stages, then drives back down the return road at 20-30 mph: it +// clears the rollout, the prove-out and the target distance, so without +// this gate it records a slow bogus "run". Two properties separate that +// drive from any pass, and a completed run is discarded only when BOTH +// hold — so a real pass is never lost on one signal alone: +// 1. It heads BACK: its launch->finish chord points more than 90 deg +// away from the last recorded run's (all passes on a strip run the +// same way; the return road parallels it the other way). +// 2. It is a CRUISE, not an acceleration: trap speed below +// kCruiseTrapRatio x the run's average speed. Any pass from a +// standstill at full effort has trap >= ~1.3x average even when +// the car tops out early (constant acceleration gives 2.0, constant +// power 1.5); a drive that settles at a cruising speed in the first +// few seconds sits near 1.0-1.15. +// Conservative by construction: the first run of a session (no heading +// to compare against), every run in the strip's direction, and any +// genuine acceleration in either direction are all kept. +constexpr float kCruiseTrapRatio = 1.2f; + // Anchor running-mean window (fix count). Averaging shrinks the anchor // noise well below a single fix's jitter; the cap keeps it responsive // to the re-latch (the mean follows the parked car's drifting fix). @@ -154,6 +174,9 @@ class DragTimer { int runs() const { return runs_; } + // Completed runs discarded by the return-road gate (diagnostic). + int rejectedRuns() const { return rejectedRuns_; } + // Live ET while a run is on, 0 otherwise. nowGpsMs lets the display // tick between fixes. unsigned long currentEtMs(uint64_t nowGpsMs) const; @@ -211,7 +234,16 @@ class DragTimer { bool slowTracking_ = false; uint64_t slowSinceMs_ = 0; + // Launch point of the live run (the anchor at launch) and the + // launch->finish direction of the last RECORDED run, for the + // return-road gate. Local flat-earth north/east components; only the + // sign of their dot product is used. + double launchLat_ = 0.0, launchLng_ = 0.0; + bool haveRunDir_ = false; + double runDirN_ = 0.0, runDirE_ = 0.0; + // Records. + int rejectedRuns_ = 0; int runs_ = 0; unsigned long lastEtMs_ = 0; float lastTrapMph_ = 0.0f; diff --git a/CHANGELOG.md b/CHANGELOG.md index 94956cd..2cbde0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,11 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 - Automatic drag runs after the first show the RPM/speed LED bar again instead of a single dim centre pixel (a pace display with nothing to pace against). + - Automatic drag mode no longer records the drive back down the return + road as a slow run. A finished "run" is discarded only when it both + heads the opposite way to the last recorded run and was driven at a + cruise rather than a full-effort acceleration, so real passes — + including slow ones — still count. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index a5353e7..97e60be 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -905,7 +905,11 @@ loop() ~250 Hz lock, the one predicate the tree, staging screen and LED search pip share, because drag time is epoch ms and the pre-lock date jumps; trap/0-60 deliberately NOT in the header — the 25 Hz rows carry speed). - Run capture rides `checkForNewLapData()`'s run-count edge; each + A finished run is discarded as a **return-road drive** only when it + heads >90° from the last recorded run AND traps below + `kCruiseTrapRatio` (1.2) × its average speed (a cruise, not a pass) — + both, so a real pass is never lost on one signal (`rejectedRuns()` + counts them). Run capture rides `checkForNewLapData()`'s run-count edge; each completed run AND each fresh STAGED latch re-arms the auto-idle grace (an active staging queue never idles out; manual 5 min/5 mph rules otherwise apply, with the usual tach promotion). Display: no new @@ -2075,6 +2079,7 @@ the one loaded). Sector lines stay optional — zero, one, or two. | Drag tree flash half-period | 500 ms (3 Hz OLED aliases anything faster) | `drag_tree.h` | | Drag stage / launch / abort | ≤1 mph held 1 s / ≥2 mph + rollout / ≤2 mph held 3 s, ≥2 s fix gap, or 2 s wall-clock with no fix (`checkFixLoss`) (silent) | `drag_timer.h` | | Drag prove-out | launch must reach 15 mph within 5 s of ET start, else silently abandoned | `drag_timer.h` | +| Drag return-road gate | discard a finished run heading >90° from the last recorded one AND trapping < 1.2 × its average speed (`kCruiseTrapRatio`) | `drag_timer.h` | | Drag time base | Unix epoch ms (`getGpsUnixTimestampMillis()`) — never time-of-day ms (wraps at UTC midnight) | `gps_functions.ino` | | Drag distances | 660 / 1000 / 1320 / 2640 / 5280 ft (picker order) | `drag_timer.cpp` | | Track JSON coordinate precision | 8 decimals (~1.1 mm) | `track_json.h` | diff --git a/docs/plans/0015-drag-mode.md b/docs/plans/0015-drag-mode.md index d5ae7cb..f3c0176 100644 --- a/docs/plans/0015-drag-mode.md +++ b/docs/plans/0015-drag-mode.md @@ -72,7 +72,8 @@ as a timestamp gap. speed; 0 if never reached (short cars on the 1/8). Finish: cumulative distance crosses the target → ET and trap both interpolated between the straddling fixes; run recorded (count, last/best ET, best-run trap and - 0–60 snapshot); back to ARMED. There is no FINISHED phase — "re-arm" + 0–60 snapshot) unless the return-road gate (review D7, below) discards + it; back to ARMED. There is no FINISHED phase — "re-arm" IS "wait for standstill", which is ARMED. Two aborts, both silent: ≤ 2 mph held 3 s before the target (this also self-cancels queue-creep phantom launches), and the **prove-out gate** — a launch that fails to @@ -198,3 +199,23 @@ each, each with a regression test in the pure unit that owns the rule. instead of the RPM/speed scale. Plan 0016 had excluded manual drag only. The gate is now `led_modes::paceValid(PaceGate)`, pure and host-tested, with `hasPaceReference = !dragModeIsActive()`. +- **D7 — return-road drives recorded as runs.** A car stopped in the + shutdown area stages; driving the return road back at 20–30 mph then + clears the rollout, the 15 mph prove-out and the target distance, and + lands a slow bogus run in the lap history and the DOVEX laps line. + Chosen gate: a completed run is discarded (counted in `rejectedRuns()`, + never recorded) only when **both** (a) its launch→finish chord points + more than 90° from the last *recorded* run's — all passes on a strip + run one way, the return road the other — **and** (b) its trap speed + is below `kCruiseTrapRatio` (1.2) × its average speed. (b) is the + physics of a pass: from a standstill at full effort trap/average is + 2.0 at constant acceleration and 1.5 at constant power, and stays + above ~1.3 even for a car that tops out early on the distance it + picked; a drive that settles into a cruise within a few seconds sits + near 1.0–1.15. Rejected alternatives: a trap-speed floor (a 206 kart + traps ~50 mph and a return road can be driven at 30 — no floor + separates them), an ET ceiling per distance (same overlap), and either + signal alone (heading alone kills two-way top-speed passes; the ratio + alone kills a slow-topping vehicle cruising out the back half of a + long distance). Deliberately conservative: the first run of a session + (no heading to compare) and every same-direction run are always kept. diff --git a/tests/drag_timer_test.cpp b/tests/drag_timer_test.cpp index 0dc11d9..d6004d7 100644 --- a/tests/drag_timer_test.cpp +++ b/tests/drag_timer_test.cpp @@ -436,6 +436,119 @@ TEST_CASE("a slow but real pass proves out and records") { // Launch gate (manual staging tree, plan 0016) // --------------------------------------------------------------------------- +// --------------------------------------------------------------------------- +// Return-road gate (review D7) +// --------------------------------------------------------------------------- + +namespace { + +// Stage at `x0`, then drive BACK down the strip (decreasing x): brisk +// 3 s acceleration to `cruiseMph`, then hold it. Returns completion. +bool returnRoadDrive(Strip& s, double x0, double cruiseMph, uint64_t maxMs) { + const double vMax = cruiseMph / kFtPerSecToMph; // ft/s + const double a = vMax / 3.0; + const uint64_t start = s.now; + while (s.now - start < maxMs) { + const double tS = (double)(s.now - start) / 1000.0; + double x, v; + if (tS < 3.0) { + x = 0.5 * a * tS * tS; + v = a * tS; + } else { + x = 0.5 * a * 9.0 + vMax * (tS - 3.0); + v = vMax; + } + if (s.fix(x0 - x, v * kFtPerSecToMph)) return true; + } + return false; +} + +} // namespace + +TEST_CASE("return-road drive after a run is not recorded") { + // The car finishes a real pass, stops in the shutdown area, stages + // there, then drives the return road back at 25 mph. It clears the + // rollout, the 15 mph prove-out and the 660 ft target — a bogus ~20 s + // "run" before the gate. + Strip s(0); + s.standstill(0.0, 2000); + REQUIRE(s.launchConstAccel(0.0, 20.0, 15000)); + REQUIRE(s.t.runs() == 1); + const unsigned long realEt = s.t.lastEtMs(); + + // Coast on past the stripe and park in the shutdown area. + s.standstill(900.0, 2000); + REQUIRE(s.t.phase() == Phase::kStaged); + CHECK_FALSE(returnRoadDrive(s, 900.0, 25.0, 40000)); + CHECK(s.t.runs() == 1); + CHECK(s.t.rejectedRuns() == 1); + CHECK(s.t.lastEtMs() == realEt); + CHECK(s.t.bestEtMs() == realEt); + CHECK_FALSE(s.t.runActive()); + + // Back in the lanes, the next real pass still records. + s.standstill(0.0, 2000); + REQUIRE(s.t.phase() == Phase::kStaged); + CHECK(s.launchConstAccel(0.0, 20.0, 15000)); + CHECK(s.t.runs() == 2); +} + +TEST_CASE("a slow cruise-profile pass in the strip's direction still records") { + // Conservative: the heading test alone never discards a run going the + // same way as the last one, however gently it was driven. + Strip s(0); + s.standstill(0.0, 2000); + REQUIRE(s.launchConstAccel(0.0, 20.0, 15000)); + s.standstill(-900.0, 2000); // back behind the start line, staged + REQUIRE(s.t.phase() == Phase::kStaged); + // Same direction as run 1 (increasing x), cruise profile. + const double vMax = 25.0 / kFtPerSecToMph; + const double a = vMax / 3.0; + const uint64_t start = s.now; + bool done = false; + while (!done && s.now - start < 40000) { + const double tS = (double)(s.now - start) / 1000.0; + const double x = tS < 3.0 ? 0.5 * a * tS * tS + : 0.5 * a * 9.0 + vMax * (tS - 3.0); + const double v = tS < 3.0 ? a * tS : vMax; + done = s.fix(-900.0 + x, v * kFtPerSecToMph); + } + CHECK(done); + CHECK(s.t.runs() == 2); + CHECK(s.t.rejectedRuns() == 0); +} + +TEST_CASE("a full-effort pass in the opposite direction still records") { + // Two-way passes (e.g. wind-averaged top-speed runs) accelerate the + // whole way — trap well above average — so heading alone never drops + // them. + Strip s(0); + s.standstill(0.0, 2000); + REQUIRE(s.launchConstAccel(0.0, 20.0, 15000)); + s.standstill(1500.0, 2000); + REQUIRE(s.t.phase() == Phase::kStaged); + const uint64_t start = s.now; + bool done = false; + while (!done && s.now - start < 15000) { + const double tS = (double)(s.now - start) / 1000.0; + done = s.fix(1500.0 - 0.5 * 20.0 * tS * tS, 20.0 * tS * kFtPerSecToMph); + } + CHECK(done); + CHECK(s.t.runs() == 2); + CHECK(s.t.rejectedRuns() == 0); +} + +TEST_CASE("the first run of a session is never discarded by the gate") { + // No previous heading to compare against: a cruise-profile first run + // records (it can only be judged against a later one's direction). + Strip s(0); + s.standstill(900.0, 2000); + REQUIRE(s.t.phase() == Phase::kStaged); + CHECK(returnRoadDrive(s, 900.0, 25.0, 40000)); + CHECK(s.t.runs() == 1); + CHECK(s.t.rejectedRuns() == 0); +} + TEST_CASE("launch disabled: rollout at speed re-arms instead of running") { Strip s(0); s.t.setLaunchEnabled(false); From 572834d91da78b4f15d08acdb9ccee57cd353a82 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 04:47:03 +0000 Subject: [PATCH 8/8] plan 0015: creeping through the rollout re-stages instead of timing late MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding D8. Staged, a car creeping at 1-2 mph (above the 1 mph staging threshold, below the 2 mph launch speed) could cover the whole 11.25 in rollout without launching; when it then passed 2 mph the launch edge found the rollout already behind it (dPrev >= kRolloutFt), the interpolation clamped f to 0, and the ET started at the previous fix with the crept ground credited to the run's distance — a short ET. A STAGED car moving above the staging threshold that passes the rollout without launch speed now drops to ARMED. If it stops, it re-stages at the new spot a second later and the next launch times exactly (the new test also shows the old code mistimed that case through the anchor mean's lag); if it rolls straight into a launch there was no standing start and nothing is recorded. The rule is gated on Doppler speed, so standstill position jitter past the rollout radius still cannot un-stage a parked car. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SdhRyfZ4eYuooXi9uy28aw --- BirdsEye/drag_timer.cpp | 18 ++++++++++++++-- BirdsEye/drag_timer.h | 4 +++- CHANGELOG.md | 4 ++++ CLAUDE.md | 4 +++- docs/plans/0015-drag-mode.md | 20 ++++++++++++++++- tests/drag_timer_test.cpp | 42 ++++++++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 5 deletions(-) diff --git a/BirdsEye/drag_timer.cpp b/BirdsEye/drag_timer.cpp index b57af91..22c086c 100644 --- a/BirdsEye/drag_timer.cpp +++ b/BirdsEye/drag_timer.cpp @@ -176,9 +176,23 @@ bool DragTimer::onFix(double lat, double lng, float speedMph, sixtyCrossed_ = false; run0to60Ms_ = 0; slowTracking_ = false; + } else if (d >= kRolloutFt && speedMph > kStagedMaxMph) { + // Creep through the rollout below launch speed (review D8). The + // car has left its staged spot without launching, so there is + // no standstill left to time from: letting it carry on meant a + // later push past 2 mph "launched" with the rollout ALREADY + // behind it — the interpolation clamped to the previous fix, so + // the clock started late and the timed distance included ground + // crept before it. Re-stage instead: a car that stops is staged + // again at its new spot a second later; one that rolls straight + // into a launch has no standing start and records nothing. Real + // speed (Doppler, > the staging threshold) is required, so + // standstill position jitter past the rollout radius still + // cannot un-stage a parked car. + resetToArmed(); } else if (d >= kRestageFt) { - // Moved to a new spot without ever reaching launch speed — a - // slow reposition (staging-lane creep). Re-stage from scratch. + // Drifted far from the anchor with no speed behind it — a + // reposition the speed test above missed. Re-stage from scratch. resetToArmed(); } break; diff --git a/BirdsEye/drag_timer.h b/BirdsEye/drag_timer.h index db4cd6e..423baff 100644 --- a/BirdsEye/drag_timer.h +++ b/BirdsEye/drag_timer.h @@ -89,7 +89,9 @@ constexpr uint32_t kProveOutMs = 5000; // (speed stayed under kLaunchMinMph) has moved to a new spot — drop // back to ARMED and re-stage there. Deliberately much larger than the // rollout so standstill jitter can never un-stage a car that is about -// to launch; only a real slow reposition trips it. +// to launch. A car CREEPING (speed above kStagedMaxMph) re-stages much +// sooner — the moment it passes the rollout (review D8) — because a +// launch from there would start the clock late. constexpr float kRestageFt = 10.0f; // Return-road rejection (review fix D7). A car stopped in the shutdown diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cbde0a..6e17ab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,10 @@ and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2 heads the opposite way to the last recorded run and was driven at a cruise rather than a full-effort acceleration, so real passes — including slow ones — still count. + - Creeping forward slowly after staging no longer produces a short + drag ET. Rolling past the start point below launch speed now + re-stages the car; before, the clock started late and counted the + crept distance toward the run. - **A soft reboot no longer runs the next boot under a watchdog it can't see.** The nRF52 hardware WDT survives `NVIC_SystemReset()` — only a pin, brown-out, power-on or System OFF reset clears it — so every diff --git a/CLAUDE.md b/CLAUDE.md index 97e60be..2030e6e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -891,7 +891,9 @@ loop() ~250 Hz stage at a standstill (≤1 mph held 1 s; the anchor is a **re-latching running mean** of standstill fixes so GPS drift in a staging lane can't fake a launch), launch rollout-style (11.25 in displacement + ≥2 mph, - ET start interpolated between the straddling 25 Hz fixes), accumulate + ET start interpolated between the straddling 25 Hz fixes; creeping + through the rollout below 2 mph re-stages instead, so the clock can + never start with the rollout already behind the car), accumulate chord distance to the target, finish with interpolated ET + trap speed and a 0-60 split (0 if never reached). Mid-run standstill (3 s), a ≥2 s fix gap, or 2 s of wall clock with no fix at all (the diff --git a/docs/plans/0015-drag-mode.md b/docs/plans/0015-drag-mode.md index f3c0176..688c853 100644 --- a/docs/plans/0015-drag-mode.md +++ b/docs/plans/0015-drag-mode.md @@ -61,7 +61,8 @@ as a timestamp gap. re-latching anchor the only false-launch source left is real motion (queue creep), and that self-cancels through the abort rule without recording anything. Launch = displacement from anchor ≥ rollout **AND** - speed ≥ 2 mph (jitter or dead-slow creep never launches); the ET start is + speed ≥ 2 mph (jitter or dead-slow creep never launches; creep that + passes the rollout below 2 mph re-stages — review D8); the ET start is linearly interpolated on the displacement curve between the two straddling fixes, and the run's distance is seeded with the overshoot past rollout. - **LAUNCHED**: per fix, add the chord distance (drag runs are straight — @@ -219,3 +220,20 @@ each, each with a regression test in the pure unit that owns the rule. alone kills a slow-topping vehicle cruising out the back half of a long distance). Deliberately conservative: the first run of a session (no heading to compare) and every same-direction run are always kept. +- **D8 — a slow creep before launch started the clock late.** Staged, + a car creeping at 1–2 mph (above the 1 mph staging threshold, below + the 2 mph launch speed) could cover the whole 11.25 in rollout without + launching; the moment it then passed 2 mph the launch edge found the + rollout already behind it (`dPrev ≥ kRolloutFt`), the interpolation + clamped to the previous fix, and the run was timed from late with the + crept ground credited to its distance. Now a STAGED car moving above + the staging threshold that passes the rollout without launch speed + **re-stages** (drops to ARMED). If it stops, it is staged again at the + new spot a second later and the next launch times exactly; if it rolls + straight into a launch it had no standing start and records nothing — + the strip equivalent of rolling through the beams. Gated on Doppler + speed, so standstill position jitter past the rollout radius still + cannot un-stage a parked car (the 10 ft `kRestageFt` drift rule is + unchanged). Tests: creep→launch records nothing; creep→stop→launch + times to the analytic ET (the old code missed it by the anchor mean's + lag). diff --git a/tests/drag_timer_test.cpp b/tests/drag_timer_test.cpp index d6004d7..2b1d7d0 100644 --- a/tests/drag_timer_test.cpp +++ b/tests/drag_timer_test.cpp @@ -126,6 +126,48 @@ TEST_CASE("slow reposition re-stages at the new spot") { CHECK(s.t.runs() == 0); } +TEST_CASE("creep through the rollout below launch speed re-stages") { + // Review D8: creeping at 1-2 mph past the rollout, then pushing past + // 2 mph, used to "launch" with the rollout already behind the car — + // the interpolation clamped to the previous fix, starting the clock + // late and crediting crept ground to the run. + Strip s(0); + s.standstill(0.0, 2000); + REQUIRE(s.t.phase() == Phase::kStaged); + const double v = 2.2; // ft/s = 1.5 mph: above staged, below launch + double x = 0.0; + while (x < drag_timer::kRolloutFt - 0.1) { + s.fix(x, v * kFtPerSecToMph); + x += v * 0.04; + } + CHECK(s.t.phase() == Phase::kStaged); // inside the rollout: still staged + for (int i = 0; i < 10; i++) { + s.fix(x, v * kFtPerSecToMph); + x += v * 0.04; + } + CHECK(s.t.phase() == Phase::kArmed); // crept through it: re-stage + + // Rolling straight into a launch from the creep: no standing start, + // nothing timed. + CHECK_FALSE(s.launchConstAccel(x, 20.0, 15000)); + CHECK(s.t.runs() == 0); +} + +TEST_CASE("creep, stop, then launch times from the new spot exactly") { + Strip s(0); + s.standstill(0.0, 2000); + const double v = 2.2; // 1.5 mph creep for 3 ft + for (double x = 0.0; x < 3.0; x += v * 0.04) s.fix(x, v * kFtPerSecToMph); + s.standstill(3.0, 1500); + REQUIRE(s.t.phase() == Phase::kStaged); + + const double a = 14.7; + REQUIRE(s.launchConstAccel(3.0, a, 60000)); + const double tRoll = sqrt(2.0 * drag_timer::kRolloutFt / a); + const double tFin = sqrt(2.0 * (660.0 + drag_timer::kRolloutFt) / a); + CHECK(fabs((double)s.t.lastEtMs() - (tFin - tRoll) * 1000.0) <= kDtMs); +} + // --------------------------------------------------------------------------- // The run: rollout, ET, trap, 0-60 // ---------------------------------------------------------------------------