From 6d4c5daa0b823bedca9ccdc211da2e1df51b0621 Mon Sep 17 00:00:00 2001 From: root Date: Sat, 19 Sep 2026 16:32:45 +0000 Subject: [PATCH] fix(telemetry): address GNSS freshness review findings --- docs/CODE_MAP.md | 2 +- .../performance_timer_service.dart | 14 +- lib/models/telemetry_sample.dart | 7 +- test/canonical_dashboard_responsive_test.dart | 2 + test/pr4_freshness_regression_test.dart | 158 ++++++++++++++++++ 5 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 test/pr4_freshness_regression_test.dart diff --git a/docs/CODE_MAP.md b/docs/CODE_MAP.md index e6d2600..c1c72fc 100644 --- a/docs/CODE_MAP.md +++ b/docs/CODE_MAP.md @@ -37,7 +37,7 @@ 6. **UI Display (`lib/features/dashboard/widgets/speed_readout.dart`)** - Rebuilding the entire dashboard at 20Hz would cause severe UI lag. - `ResponsiveDashboardLayout` isolates updates using the `SpeedReadout` leaf widget. - - `SpeedReadout` watches `telemetryStreamProvider.select((s) => s.value?.speed ?? 0.0)`. + - `SpeedReadout` selects freshness-aware `sample.freshSpeed` from `telemetryStreamProvider`; unavailable/stale speed renders as `--`. - Clamps the value to `>= 0.0`, converts to an integer string (`toStringAsFixed(0)`), and renders. ## C. Discoveries (Current implementation speeds) diff --git a/lib/features/performance/performance_timer_service.dart b/lib/features/performance/performance_timer_service.dart index 8c889a5..21ca331 100644 --- a/lib/features/performance/performance_timer_service.dart +++ b/lib/features/performance/performance_timer_service.dart @@ -77,17 +77,22 @@ class PerformanceTimerService extends StateNotifier { void _onTelemetryUpdate(TelemetrySample sample) { if (sample.speed == null) return; - // Reject corrupted or impossible samples - if (sample.quality != null && sample.quality!.isRejected) return; - // Reject stale GPS samples from mutating performance runs or launching if (!sample.isSpeedFresh) { + // A recovered moving fix cannot establish when the launch occurred. + _lastSample = null; + if (state.state == PerformanceState.ready) { + state = state.copyWith(state: PerformanceState.idle); + } if (state.state == PerformanceState.timing) { state = state.copyWith(state: PerformanceState.aborted); } return; } + // Reject corrupted or impossible samples + if (sample.quality != null && sample.quality!.isRejected) return; + final speedMph = sample.speed!; if (state.state == PerformanceState.ready || state.state == PerformanceState.idle) { @@ -95,7 +100,8 @@ class PerformanceTimerService extends StateNotifier { if (state.state != PerformanceState.ready) { state = state.copyWith(state: PerformanceState.ready); } - } else if (speedMph >= _speedThresholdMph) { + } else if (speedMph >= _speedThresholdMph && + state.state == PerformanceState.ready) { // Start timing immediately when motorcycle launches past 1.0 MPH // Interpolate the exact start time between the last frame (stopped) and this frame _startTime = _interpolateTime( diff --git a/lib/models/telemetry_sample.dart b/lib/models/telemetry_sample.dart index dc72719..0bac587 100644 --- a/lib/models/telemetry_sample.dart +++ b/lib/models/telemetry_sample.dart @@ -106,7 +106,7 @@ class TelemetrySample { /// fresh according to the canonical evidence staleness bound (4000 ms). /// /// Returns false when: - /// - [latitude] or [longitude] is null (no GPS fix available) + /// - [latitude] or [longitude] is absent, non-finite, or out of range /// - [gpsAgeMs] exceeds [RecordingHealthContract.evidenceStaleAfterMs] (4000 ms) /// - [gpsTimestamp] age exceeds [RecordingHealthContract.evidenceStaleAfterMs] relative to [timestamp] /// @@ -115,6 +115,11 @@ class TelemetrySample { /// - Intermediate 10 Hz / 20 Hz sample repetitions within the 4000 ms bound /// - Synthetic test samples where GPS metadata was not explicitly attached bool get isGpsFresh { + if (latitude == null || longitude == null || + !latitude!.isFinite || !longitude!.isFinite || + latitude!.abs() > 90 || longitude!.abs() > 180) { + return false; + } if (gpsAgeMs != null) { return gpsAgeMs! >= 0 && gpsAgeMs! <= RecordingHealthContract.evidenceStaleAfterMs; diff --git a/test/canonical_dashboard_responsive_test.dart b/test/canonical_dashboard_responsive_test.dart index 2326b2c..23c731f 100644 --- a/test/canonical_dashboard_responsive_test.dart +++ b/test/canonical_dashboard_responsive_test.dart @@ -226,6 +226,8 @@ void main() { leanAngle: 24.5, longitudinalG: 0.42, lateralG: 0.35, + latitude: 37.7749, + longitude: -122.4194, gpsAccuracy: 9.0, ), ); diff --git a/test/pr4_freshness_regression_test.dart b/test/pr4_freshness_regression_test.dart new file mode 100644 index 0000000..b855380 --- /dev/null +++ b/test/pr4_freshness_regression_test.dart @@ -0,0 +1,158 @@ +import 'dart:async'; + +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:motorcycle_dash/database/database.dart' hide TelemetrySample; +import 'package:motorcycle_dash/features/performance/performance_timer_service.dart'; +import 'package:motorcycle_dash/models/telemetry_sample.dart'; + +void main() { + final base = DateTime.utc(2026, 9, 19); + + test( + 'normal READY launch retains interpolation fallback and null-speed handling', + () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + final controller = StreamController(sync: true); + final timer = PerformanceTimerService(db, controller.stream); + addTearDown(() async { + timer.dispose(); + await controller.close(); + await db.close(); + }); + + timer.setTarget(PerformanceTarget.sixty); + for (final age in [null, 0, 4001]) { + controller.add(TelemetrySample(timestamp: base, gpsAgeMs: age)); + expect(timer.state.state, PerformanceState.ready); + } + // No previous speed sample: launch time falls back to this timestamp. + controller.add(TelemetrySample(timestamp: base, speed: 5, gpsAgeMs: 0)); + expect(timer.state.state, PerformanceState.timing); + for (final age in [null, 0, 4001]) { + controller.add( + TelemetrySample( + timestamp: base.add(const Duration(milliseconds: 100)), + gpsAgeMs: age, + ), + ); + expect(timer.state.state, PerformanceState.timing); + } + controller.add( + TelemetrySample( + timestamp: base.add(const Duration(seconds: 3)), + speed: 60, + gpsAgeMs: 0, + ), + ); + expect(timer.state.state, PerformanceState.completed); + final runs = await db.select(db.performanceRuns).get(); + expect(runs, hasLength(1)); + expect(runs.single.timeMs, 3000); + }, + ); + + test('stale READY requires new stationary evidence before launch', () async { + final db = AppDatabase.forTesting(NativeDatabase.memory()); + final controller = StreamController(sync: true); + final timer = PerformanceTimerService(db, controller.stream); + addTearDown(() async { + timer.dispose(); + await controller.close(); + await db.close(); + }); + void emit(int ms, double speed, int age) { + controller.add( + TelemetrySample( + timestamp: base.add(Duration(milliseconds: ms)), + speed: speed, + gpsAgeMs: age, + ), + ); + } + + timer.setTarget(PerformanceTarget.sixty); + emit(0, 0, 0); + expect(timer.state.state, PerformanceState.ready); + emit(4000, 0, 4000); + expect(timer.state.state, PerformanceState.ready); + emit(4001, 0, 4001); + expect(timer.state.state, PerformanceState.idle); + emit(5000, 30, 5000); + emit(6000, 40, 0); + expect(timer.state.state, PerformanceState.idle); + emit(9000, 65, 0); + expect(timer.state.state, PerformanceState.idle); + expect(await db.select(db.performanceRuns).get(), isEmpty); + + emit(10000, 0, 0); + expect(timer.state.state, PerformanceState.ready); + emit(10200, 5, 0); + expect(timer.state.state, PerformanceState.timing); + emit(13000, 65, 0); + expect(timer.state.state, PerformanceState.completed); + final runs = await db.select(db.performanceRuns).get(); + expect(runs, hasLength(1)); + expect(runs.single.timeMs, closeTo(2727, 1)); + }); + + for (final coordinates in <(double?, double?)>[ + (null, null), + (null, 0), + (0, null), + (double.nan, 0), + (0, double.nan), + (double.infinity, 0), + (0, double.negativeInfinity), + (90.1, 0), + (-90.1, 0), + (0, 180.1), + (0, -180.1), + ]) { + for (final metadata in ['absent', 'age', 'timestamp']) { + test('invalid coordinates $coordinates with $metadata are not fresh', () { + expect( + TelemetrySample( + timestamp: base, + latitude: coordinates.$1, + longitude: coordinates.$2, + gpsAgeMs: metadata == 'age' ? 0 : null, + gpsTimestamp: metadata == 'timestamp' ? base : null, + ).isGpsFresh, + isFalse, + ); + }); + } + } + + for (final age in [3999, 4000, 4001]) { + for (final useAge in [true, false]) { + test('GPS boundary $age with explicit age $useAge', () { + expect( + TelemetrySample( + timestamp: base, + latitude: 0, + longitude: 0, + gpsAgeMs: useAge ? age : null, + gpsTimestamp: useAge + ? null + : base.subtract(Duration(milliseconds: age)), + ).isGpsFresh, + age <= 4000, + ); + }); + } + } + test('valid coordinate limits retain metadata-free compatibility', () { + for (final coordinates in [(90.0, 180.0), (-90.0, -180.0), (0.0, 0.0)]) { + expect( + TelemetrySample( + timestamp: base, + latitude: coordinates.$1, + longitude: coordinates.$2, + ).isGpsFresh, + isTrue, + ); + } + }); +}