Speed-source arbitration and GPS quality reporting (#19) #102

Merged

robert merged 1 commit from area/speed-source-arbitration into main 2026-09-04 20:24:11 +02:00

Conversation 0 Commits 1 Files changed 7 +593 -26

robert commented 2026-09-04 20:23:16 +02:00

Owner

Copy link

Closes #19.

Scope

Most of the wheel-vs-GPS arbitration was already built by #69/#17 (SpeedPipeline). This issue's
remaining scope, per its acceptance criteria: reconcile the 3s-vs-5s wheel-live-window discrepancy,
derive GPS_QUALITY (new logic), wire SPEED_SOURCE/GPS_QUALITY to the generated Proto.Key
constants, close a real distance-discontinuity gap on source handover, and give "no source at all"
an honest reporting state instead of a frozen number.

The 3s-vs-5s wheel-live-window discrepancy

SpeedPipeline.WHEEL_LIVE_WINDOW_NANOS was shipped as 5s by #69/PR #100. This issue's own
acceptance criterion says 3s. Checked docs/DECISIONS.md D4 (the original speed-sourcing
decision, predating #69): it already says, verbatim, "wheel sensor if it has reported within the
last 3 s, GPS otherwise." So this wasn't a case of picking between two arbitrary numbers — D4
had already settled on 3s, and #69's 5s was a default picked without D4 in view (its own KDoc
justified 5s only in the abstract, "generous relative to a sensor's normal notify rate," against no
specific requirement). Resolved: changed the constant to 3s, matching D4, updated
SpeedPipelineTest's timings, and amended docs/DECISIONS.md D35's own "2026-09-04 update" note
with the correction and the reasoning (docs/DECISIONS.md, D35 amendment).

SPEED_SOURCE / GPS_QUALITY wire keys

Both already existed from #7 (shared/message_keys.json, generated into
companion/pebble/.../Proto.kt's Proto.Key.SPEED_SOURCE / Proto.Key.GPS_QUALITY), and the
numbering already matched this issue exactly — message_keys.json's own note on SPEED_SOURCE
already reads "0 = none, 1 = GPS, 2 = wheel sensor," the same numbering this issue's acceptance
criterion asks for. No reconciliation needed, unlike #7's own RideCmd numbering fix. New:
companion/pebble/src/main/kotlin/de/butzei/pedalpebble/pebble/SpeedWireEncoding.kt — a small,
pure encoder (following ProtocolHandshake.kt's existing "pure logic near Proto.kt, no
PebbleKit/Android dependency" pattern in this module) turning a SpeedReport + a GPS_QUALITY int
into the Map<Int, Int> of Proto.Key -> value a future AppMessage dictionary-builder (#4/Spike
A) can merge in directly — nothing sends an AppMessage yet, PebbleTransport is still #4's
placeholder.

GPS_QUALITY 0-3 derivation

New: companion/core/.../location/GpsQuality.kt, deriveGpsQuality(accuracyMeters, fixAgeSeconds): Int. Grades accuracy and age independently into 4 tiers each and reports the worse of the two
(not an average — a fresh-but-inaccurate fix and a stale-but-accurate fix are both real problems,
and averaging would hide which one is wrong):

• Accuracy tiers (10m / 20m / 30m boundaries) are anchored to MAX_ACCEPTABLE_ACCURACY_METERS's
  own already-documented real-world bands (roughly 3-15m open sky, 10-30m tree cover, 30m+ rejected
  outright) — tier 3 sits inside open-sky, tier 2 mid-canopy, tier 1 covers the rest of what's
  actually accepted, tier 0 is what the accuracy gate already rejects.
• Age tiers (2s / 5s / 10s boundaries) are anchored to NFR-B2's 1Hz cadence and the new
  SIGNAL_TIMEOUT_NANOS (see below) — tier 3 tolerates one missed beat, tier 2 a handful of
  consecutive misses, tier 1 runs right up to the no-signal boundary, tier 0 is what's already past it.

SpeedPipeline.currentGpsQuality(atNanos) wires this to the real pipeline state (the most recently
accepted fix's accuracy/age; 0 if no fix has ever been accepted). Deliberately reads the position
baseline directly rather than "whichever source is preferred" — GPS quality keeps reporting
correctly even while the wheel is the active source, feeding off the same baseline the
discontinuity guard below keeps fresh.

The distance-discontinuity guard (the real bug this issue asked me to look for)

Found a genuine bug, not just a missing test: onGpsFix checked the accuracy gate before the
wheel-live check. So a GPS fix that failed accuracy while the wheel was live was rejected outright
and never refreshed the position baseline (lastAcceptedGpsFix) — even though GPS wasn't the active
source and the fix was only ever going to matter as a future baseline. A sustained run of such fixes
(a bridge or urban canyon, with a wheel sensor also paired) left the baseline stuck at whatever fix
preceded the bad patch. The moment the wheel then fell quiet and GPS accuracy recovered, the very
next accepted fix computed its distance against that stale baseline — a spike bounded only by how
far the rider had actually ridden during the bad patch, not by anything the pipeline checked.

Fix: check wheel-liveness first; while the wheel is live, refresh the baseline from every
incoming fix regardless of its own accuracy (never counted as a rejection either way — dropped for
arbitration, not for quality). Bounds the worst case to a single fix's own accuracy error instead of
an unbounded, ever-growing jump.

Test proving it (SpeedPipelineTest.kt, "onGpsFix does not spike distance when GPS accuracy was
poor throughout a long wheel-live stretch"): 9 seconds of wheel-live riding with GPS producing
50m-accuracy fixes throughout (tracking the same real 5 m/s progression), then the wheel falls
silent and a good fix arrives 5m further along. Asserts movingDistanceMeters is ~50m (45m wheel +
5m final delta) — the regression this guards against would report ~95m (the stale 45m-old baseline
producing a 50m delta in one interval instead of 5m).

No-signal reporting

New: SpeedReport sealed interface (Live(sample) / NoSignal) and
SpeedPipeline.currentReport(atNanos). Previously, a caller polling after both sources went quiet
would just keep seeing whatever SpeedPipelineSample was last produced, forever — the exact "stale
number" this issue's acceptance criterion forbids, and the phone-side mirror of docs/DECISIONS.md
D44's watch-side rule ("unavailability must be signalled explicitly, never inferred from silence").
currentReport is keyed off advance()'s own shared bookkeeping (any source, not source-specific)
against a new SIGNAL_TIMEOUT_NANOS = 10s — deliberately not reused from
WHEEL_LIVE_WINDOW_NANOS (3s), since the two answer different questions: 3s governs which source
wins when both could be live (tight, because GPS should hand back over promptly); 10s governs "is
there a signal at all," which has to tolerate a run of ordinary missed/rejected 1Hz GPS fixes (a
bridge, tall buildings) without flapping the display, while still surfacing a genuine loss (a
tunnel, an unpaired wheel sensor with no GPS fix either) within an order of magnitude a rider would
call "the display noticed." Full reasoning on the constant's own KDoc.

Files

• companion/core/src/main/kotlin/de/butzei/pedalpebble/core/location/SpeedPipeline.kt — window fix,
  discontinuity-guard reorder, SpeedReport/currentReport/currentGpsQuality.
• companion/core/src/main/kotlin/de/butzei/pedalpebble/core/location/GpsQuality.kt — new, pure
  deriveGpsQuality.
• companion/pebble/src/main/kotlin/de/butzei/pedalpebble/pebble/SpeedWireEncoding.kt — new, wire
  encoding against Proto.Key.
• Why no separate SpeedSourceArbiter.kt (the issue's literal suggested filename): the arbitration
  itself was already fully built and correct in SpeedPipeline before this issue's scope was picked
  up — a wrapper class re-deciding an already-decided source would be exactly the "two definitions"
  problem D35 exists to prevent, one level up. What was actually still owned (no-signal query,
  GPS-quality derivation, the discontinuity bug, wire encoding) landed where each piece's own
  dependencies put it instead; reasoning is on SpeedWireEncoding.kt's own KDoc.
• docs/DECISIONS.md D35 amended (2026-09-04, #19) with the window correction and this PR's summary.
• Tests: GpsQualityTest.kt (new), SpeedWireEncodingTest.kt (new), 7 new cases in
  SpeedPipelineTest.kt (discontinuity regression, currentReport/SpeedReport x3,
  currentGpsQuality wiring x3).

Verification

• ./gradlew :companion:core:test — real run, all green (SpeedPipelineTest 19 tests, GpsQualityTest
  6 tests, plus the rest of the module unaffected).
• ./gradlew :companion:pebble:test — real run, all green (ProtocolHandshakeTest 6,
  SpeedWireEncodingTest 6).
• ./gradlew :companion:assembleDebug — real Android SDK + JDK 21 build, succeeds.

https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

Closes #19. ## Scope Most of the wheel-vs-GPS arbitration was already built by #69/#17 (`SpeedPipeline`). This issue's remaining scope, per its acceptance criteria: reconcile the 3s-vs-5s wheel-live-window discrepancy, derive `GPS_QUALITY` (new logic), wire `SPEED_SOURCE`/`GPS_QUALITY` to the generated `Proto.Key` constants, close a real distance-discontinuity gap on source handover, and give "no source at all" an honest reporting state instead of a frozen number. ## The 3s-vs-5s wheel-live-window discrepancy `SpeedPipeline.WHEEL_LIVE_WINDOW_NANOS` was shipped as **5s** by #69/PR #100. This issue's own acceptance criterion says **3s**. Checked `docs/DECISIONS.md` D4 (the *original* speed-sourcing decision, predating #69): it already says, verbatim, "wheel sensor if it has reported within the last **3 s**, GPS otherwise." So this wasn't a case of picking between two arbitrary numbers — D4 had already settled on 3s, and #69's 5s was a default picked without D4 in view (its own KDoc justified 5s only in the abstract, "generous relative to a sensor's normal notify rate," against no specific requirement). **Resolved: changed the constant to 3s**, matching D4, updated `SpeedPipelineTest`'s timings, and amended `docs/DECISIONS.md` D35's own "2026-09-04 update" note with the correction and the reasoning (`docs/DECISIONS.md`, D35 amendment). ## SPEED_SOURCE / GPS_QUALITY wire keys Both already existed from #7 (`shared/message_keys.json`, generated into `companion/pebble/.../Proto.kt`'s `Proto.Key.SPEED_SOURCE` / `Proto.Key.GPS_QUALITY`), and **the numbering already matched this issue exactly** — `message_keys.json`'s own note on `SPEED_SOURCE` already reads "0 = none, 1 = GPS, 2 = wheel sensor," the same numbering this issue's acceptance criterion asks for. No reconciliation needed, unlike #7's own `RideCmd` numbering fix. New: `companion/pebble/src/main/kotlin/de/butzei/pedalpebble/pebble/SpeedWireEncoding.kt` — a small, pure encoder (following `ProtocolHandshake.kt`'s existing "pure logic near `Proto.kt`, no PebbleKit/Android dependency" pattern in this module) turning a `SpeedReport` + a `GPS_QUALITY` int into the `Map<Int, Int>` of `Proto.Key` -> value a future `AppMessage` dictionary-builder (#4/Spike A) can merge in directly — nothing sends an `AppMessage` yet, `PebbleTransport` is still #4's placeholder. ## GPS_QUALITY 0-3 derivation New: `companion/core/.../location/GpsQuality.kt`, `deriveGpsQuality(accuracyMeters, fixAgeSeconds): Int`. Grades accuracy and age independently into 4 tiers each and reports the **worse of the two** (not an average — a fresh-but-inaccurate fix and a stale-but-accurate fix are both real problems, and averaging would hide which one is wrong): - **Accuracy tiers** (10m / 20m / 30m boundaries) are anchored to `MAX_ACCEPTABLE_ACCURACY_METERS`'s own already-documented real-world bands (roughly 3-15m open sky, 10-30m tree cover, 30m+ rejected outright) — tier 3 sits inside open-sky, tier 2 mid-canopy, tier 1 covers the rest of what's actually accepted, tier 0 is what the accuracy gate already rejects. - **Age tiers** (2s / 5s / 10s boundaries) are anchored to NFR-B2's 1Hz cadence and the new `SIGNAL_TIMEOUT_NANOS` (see below) — tier 3 tolerates one missed beat, tier 2 a handful of consecutive misses, tier 1 runs right up to the no-signal boundary, tier 0 is what's already past it. `SpeedPipeline.currentGpsQuality(atNanos)` wires this to the real pipeline state (the most recently *accepted* fix's accuracy/age; 0 if no fix has ever been accepted). Deliberately reads the position baseline directly rather than "whichever source is preferred" — GPS quality keeps reporting correctly even while the wheel is the active source, feeding off the same baseline the discontinuity guard below keeps fresh. ## The distance-discontinuity guard (the real bug this issue asked me to look for) Found a genuine bug, not just a missing test: `onGpsFix` checked the accuracy gate **before** the wheel-live check. So a GPS fix that failed accuracy *while the wheel was live* was rejected outright and never refreshed the position baseline (`lastAcceptedGpsFix`) — even though GPS wasn't the active source and the fix was only ever going to matter as a future baseline. A sustained run of such fixes (a bridge or urban canyon, with a wheel sensor also paired) left the baseline stuck at whatever fix preceded the bad patch. The moment the wheel then fell quiet and GPS accuracy recovered, the very next accepted fix computed its distance against that stale baseline — a spike bounded only by how far the rider had actually ridden during the bad patch, not by anything the pipeline checked. **Fix:** check wheel-liveness first; while the wheel is live, refresh the baseline from *every* incoming fix regardless of its own accuracy (never counted as a rejection either way — dropped for arbitration, not for quality). Bounds the worst case to a single fix's own accuracy error instead of an unbounded, ever-growing jump. **Test proving it** (`SpeedPipelineTest.kt`, "onGpsFix does not spike distance when GPS accuracy was poor throughout a long wheel-live stretch"): 9 seconds of wheel-live riding with GPS producing 50m-accuracy fixes throughout (tracking the same real 5 m/s progression), then the wheel falls silent and a good fix arrives 5m further along. Asserts `movingDistanceMeters` is ~50m (45m wheel + 5m final delta) — the regression this guards against would report ~95m (the stale 45m-old baseline producing a 50m delta in one interval instead of 5m). ## No-signal reporting New: `SpeedReport` sealed interface (`Live(sample)` / `NoSignal`) and `SpeedPipeline.currentReport(atNanos)`. Previously, a caller polling after both sources went quiet would just keep seeing whatever `SpeedPipelineSample` was last produced, forever — the exact "stale number" this issue's acceptance criterion forbids, and the phone-side mirror of `docs/DECISIONS.md` D44's watch-side rule ("unavailability must be signalled explicitly, never inferred from silence"). `currentReport` is keyed off `advance()`'s own shared bookkeeping (any source, not source-specific) against a new `SIGNAL_TIMEOUT_NANOS` = **10s** — deliberately *not* reused from `WHEEL_LIVE_WINDOW_NANOS` (3s), since the two answer different questions: 3s governs which source *wins* when both could be live (tight, because GPS should hand back over promptly); 10s governs "is there a signal at all," which has to tolerate a run of ordinary missed/rejected 1Hz GPS fixes (a bridge, tall buildings) without flapping the display, while still surfacing a genuine loss (a tunnel, an unpaired wheel sensor with no GPS fix either) within an order of magnitude a rider would call "the display noticed." Full reasoning on the constant's own KDoc. ## Files - `companion/core/src/main/kotlin/de/butzei/pedalpebble/core/location/SpeedPipeline.kt` — window fix, discontinuity-guard reorder, `SpeedReport`/`currentReport`/`currentGpsQuality`. - `companion/core/src/main/kotlin/de/butzei/pedalpebble/core/location/GpsQuality.kt` — new, pure `deriveGpsQuality`. - `companion/pebble/src/main/kotlin/de/butzei/pedalpebble/pebble/SpeedWireEncoding.kt` — new, wire encoding against `Proto.Key`. - Why no separate `SpeedSourceArbiter.kt` (the issue's literal suggested filename): the arbitration itself was already fully built and correct in `SpeedPipeline` before this issue's scope was picked up — a wrapper class re-deciding an already-decided source would be exactly the "two definitions" problem D35 exists to prevent, one level up. What was actually still owned (no-signal query, GPS-quality derivation, the discontinuity bug, wire encoding) landed where each piece's own dependencies put it instead; reasoning is on `SpeedWireEncoding.kt`'s own KDoc. - `docs/DECISIONS.md` D35 amended (2026-09-04, #19) with the window correction and this PR's summary. - Tests: `GpsQualityTest.kt` (new), `SpeedWireEncodingTest.kt` (new), 7 new cases in `SpeedPipelineTest.kt` (discontinuity regression, `currentReport`/`SpeedReport` x3, `currentGpsQuality` wiring x3). ## Verification - `./gradlew :companion:core:test` — real run, all green (SpeedPipelineTest 19 tests, GpsQualityTest 6 tests, plus the rest of the module unaffected). - `./gradlew :companion:pebble:test` — real run, all green (ProtocolHandshakeTest 6, SpeedWireEncodingTest 6). - `./gradlew :companion:assembleDebug` — real Android SDK + JDK 21 build, succeeds. https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

robert added 1 commit 2026-09-04 20:23:16 +02:00

Speed-source arbitration and GPS quality reporting (#19) ... 996fdb2d3d

Closes #19.

- Reconciled the wheel-live window: SpeedPipeline.WHEEL_LIVE_WINDOW_NANOS was 5s (#69/PR #100), but
  D4 (predating #69) and this issue's own acceptance criterion both say 3s. Changed the constant to
  3s, corrected SpeedPipelineTest's timings, and amended DECISIONS.md D35.
- Fixed a real distance-discontinuity bug in SpeedPipeline.onGpsFix: the accuracy gate ran before
  the wheel-live check, so a poor-accuracy fix arriving while the wheel was live was rejected
  outright instead of refreshing the position baseline. A sustained run of such fixes left the
  baseline stale, producing a distance spike once a good fix and a quiet wheel coincided again.
  Reordered so the wheel-live branch refreshes the baseline unconditionally; regression test added.
- Added SpeedReport (Live/NoSignal) and SpeedPipeline.currentReport()/currentGpsQuality() so a caller
  polling after both sources go quiet gets an explicit no-signal state instead of a frozen sample.
- Added deriveGpsQuality(accuracyMeters, fixAgeSeconds): a pure 0-3 derivation, worse-of-two-tiers,
  anchored to MAX_ACCEPTABLE_ACCURACY_METERS' accuracy bands and the new SIGNAL_TIMEOUT_NANOS (10s).
- Added companion/pebble/SpeedWireEncoding.kt: SPEED_SOURCE/GPS_QUALITY wire encoding against the
  generated Proto.Key constants. SPEED_SOURCE (0/1/2) and GPS_QUALITY (0-3) already had wire keys
  and matching numbering from #7 -- no reconciliation needed there.
- New tests: GpsQualityTest, SpeedWireEncodingTest, plus 7 new SpeedPipelineTest cases. All real,
  ./gradlew :companion:core:test and :companion:pebble:test pass; ./gradlew :companion:assembleDebug
  succeeds.

Claude-Session: https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

robert merged commit 2ec483b5a7 into main 2026-09-04 20:24:11 +02:00

robert referenced this pull request from a commit 2026-09-04 20:24:12 +02:00

Merge pull request 'Speed-source arbitration and GPS quality reporting (#19)' (#102) from area/speed-source-arbitration into main

robert referenced this pull request 2026-09-04 20:24:24 +02:00

Speed-source arbitration and GPS quality reporting #19

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:companion

Android companion app

  

area:docs

Documentation

  

area:shared

Message contract shared by both sides

  

area:tooling

Build tooling, SDK, dev scripts

  

area:watchapp

Pebble C watchapp

  

blocker

Blocks a large part of the project until resolved

  

kind:chore

Setup, scaffolding, maintenance

  

kind:feature

New functionality

  

kind:spike

Time-boxed investigation to de-risk a decision

  

kind:test

Tests and fixtures

No labels

area:companion

area:docs

area:shared

area:tooling

area:watchapp

blocker

kind:chore

kind:feature

kind:spike

kind:test

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

robert/PedalPebble!102

No description provided.