Speed pipeline: smoothing, stop clamping, distance, moving average (#17) #101

Merged
robert merged 1 commit from area/speed-pipeline into main 2026-09-04 20:08:51 +02:00
Owner

Closes #17.

Extends #69's SpeedPipeline (companion/core, core.location package) rather than forking it: a new GpsFix type + onGpsFix() entry point sits in front of the existing wheel/GPS arbitration and StopDetector, adding everything #17 asks for that #69 deliberately left out ("'stopped' is defined in #69, not here").

Accuracy gate. Fixes with accuracyMeters > 30 m (or NaN, i.e. Location.hasAccuracy() == false) are rejected outright — before smoothing, before touching the position baseline. 30 m sits above typical canopy-degraded fixes (real-world GPS accuracy is roughly 3-15 m under open sky, 10-30 m under tree cover, 30-100+ m in genuine urban canyon/multipath), so ordinary riding under trees is not thrown away, while fixes bad enough to plausibly corrupt distance are. This is a tuned engineering judgement call, documented as such on SpeedPipeline.MAX_ACCEPTABLE_ACCURACY_METERS — not a platform constant, and the number to revisit once a real GPX-replay harness (#23) exists.

Has-speed vs. distance/dt fallback. Location.getSpeed() used directly when the platform supplied one; a great-circle (haversine) position delta over elapsed time otherwise — both paths implemented and covered by tests, including the no-speed-field rollover case.

~3 s smoothing. A plain 3-sample simple moving average (SMA) at the 1 Hz cadence NFR-B2 assumes — chosen over an exponential/low-pass filter because GPS fixes arrive at a known, steady rate (no irregular spacing for a decay constant to handle better than a fixed window would) and a fixed window is trivially testable (sum of N known values / N) versus reasoning about an EMA's tail. Justification is on SpeedPipeline.GPS_SMOOTHING_WINDOW_SIZE.

Real position-based distance. onGpsFix accumulates haversine deltas between consecutive accepted fixes rather than speed × dt — exact regardless of whether a given fix carried a platform speed. (onGpsSpeedSample, the pre-existing scalar-only entry point, keeps its speed × dt approximation since it genuinely never receives a position — unchanged, still covered by #69's own tests.)

Rejected fixes are never silent. SpeedPipeline.rejectedFixCount/.lastRejectedFix surface every accuracy rejection; RideService logs each one via Log.w. Wiring this into an actual GPS-quality UI (the watch status strip already has a placeholder icon per #59/PR #87) is later scope, per the issue's own wording.

Display clamp. SpeedPipelineSample.speedMetersPerSecond now reports exactly 0.0 once StopDetector confirms STOPPED, instead of whatever residual jitter produced that state. The 0.8 m/s number itself still lives in exactly one place — StopDetector.STOP_THRESHOLD_MPS (#69/D35) — this only decides what a caller sees once that single decision has already landed on STOPPED, per the issue's "state explicitly whether the display clamp and stop threshold are the same number, and why" follow-up.

Android-SDK-bound half

  • companion/location/src/main/kotlin/de/butzei/pedalpebble/location/LocationFixMapping.kt: a small LocationFix -> GpsFix adapter, following the mirrored-type pattern this codebase already uses for RideSetupState/RidePermissions rather than growing a dependency from :companion:core onto :companion:location.
  • RideService (companion/ride) now constructs a SpeedPipeline and feeds every LocationFix through onGpsFix, logging (not silently dropping) any rejected fix. No wheel sensor is wired into this service yet (#19's remaining scope) — the KDoc added to RideService flags the "null here currently only means accuracy-rejected" assumption that breaks once #19 lands. The notification text is left as the existing fix-count display; reflecting real distance/speed there is a UI decision this issue doesn't make unilaterally.

Deviation from the literal acceptance criteria — reported, not silently overridden

The issue's acceptance criteria say FusedLocationProviderClient at 1 s/high accuracy. AndroidLocationSource (companion/location, built in #15/PR #97, merged before this issue's remaining scope was picked up) already deliberately uses android.location.LocationManager's modern LocationRequest.Builder API instead — a documented decision (see that file's own KDoc), not an oversight, made possible because minSdk is already 31 (D19), exactly where that builder API starts. It already requests 1 Hz / QUALITY_HIGH_ACCURACY. Adding play-services-location now would contradict that already-shipped design for no behavioural gain. No play-services-location dependency was added — verified via git diff --stat on gradle/libs.versions.toml and every build.gradle.kts (empty).

Tests

  • companion/core/src/test/kotlin/de/butzei/pedalpebble/core/location/SpeedPipelineTest.kt adds real JVM/Kotest coverage: the accuracy gate (including the NaN/hasAccuracy()==false edge case caught while wiring LocationFixMapping), a fix's own platform speed taking precedence over a position-derived one, the distance/dt fallback converging through the smoothing window to the expected steady-state speed, wheel-live fixes dropping without counting as rejections, and the display clamp. Run via ./gradlew :companion:core:test — all passing (167 → 173 tests).
  • Real Android build verified via ./gradlew :companion:assembleDebug (real SDK + JDK 21) — passes. No GPS hardware in this sandbox, so the live location callback itself is compiled, not exercised — same honesty caveat as every other sensor/location PR tonight.

https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt

Closes #17. Extends #69's `SpeedPipeline` (`companion/core`, `core.location` package) rather than forking it: a new `GpsFix` type + `onGpsFix()` entry point sits in front of the existing wheel/GPS arbitration and `StopDetector`, adding everything #17 asks for that #69 deliberately left out ("'stopped' is defined in #69, not here"). **Accuracy gate.** Fixes with `accuracyMeters > 30 m` (or `NaN`, i.e. `Location.hasAccuracy() == false`) are rejected outright — before smoothing, before touching the position baseline. 30 m sits above typical canopy-degraded fixes (real-world GPS accuracy is roughly 3-15 m under open sky, 10-30 m under tree cover, 30-100+ m in genuine urban canyon/multipath), so ordinary riding under trees is not thrown away, while fixes bad enough to plausibly corrupt distance are. This is a tuned engineering judgement call, documented as such on `SpeedPipeline.MAX_ACCEPTABLE_ACCURACY_METERS` — not a platform constant, and the number to revisit once a real GPX-replay harness (#23) exists. **Has-speed vs. distance/dt fallback.** `Location.getSpeed()` used directly when the platform supplied one; a great-circle (haversine) position delta over elapsed time otherwise — both paths implemented and covered by tests, including the no-speed-field rollover case. **~3 s smoothing.** A plain 3-sample simple moving average (SMA) at the 1 Hz cadence NFR-B2 assumes — chosen over an exponential/low-pass filter because GPS fixes arrive at a known, steady rate (no irregular spacing for a decay constant to handle better than a fixed window would) and a fixed window is trivially testable (sum of N known values / N) versus reasoning about an EMA's tail. Justification is on `SpeedPipeline.GPS_SMOOTHING_WINDOW_SIZE`. **Real position-based distance.** `onGpsFix` accumulates haversine deltas between consecutive *accepted* fixes rather than `speed × dt` — exact regardless of whether a given fix carried a platform speed. (`onGpsSpeedSample`, the pre-existing scalar-only entry point, keeps its `speed × dt` approximation since it genuinely never receives a position — unchanged, still covered by #69's own tests.) **Rejected fixes are never silent.** `SpeedPipeline.rejectedFixCount`/`.lastRejectedFix` surface every accuracy rejection; `RideService` logs each one via `Log.w`. Wiring this into an actual GPS-quality UI (the watch status strip already has a placeholder icon per #59/PR #87) is later scope, per the issue's own wording. **Display clamp.** `SpeedPipelineSample.speedMetersPerSecond` now reports exactly `0.0` once `StopDetector` confirms `STOPPED`, instead of whatever residual jitter produced that state. The 0.8 m/s number itself still lives in exactly one place — `StopDetector.STOP_THRESHOLD_MPS` (#69/D35) — this only decides what a caller *sees* once that single decision has already landed on STOPPED, per the issue's "state explicitly whether the display clamp and stop threshold are the same number, and why" follow-up. ## Android-SDK-bound half - `companion/location/src/main/kotlin/de/butzei/pedalpebble/location/LocationFixMapping.kt`: a small `LocationFix -> GpsFix` adapter, following the mirrored-type pattern this codebase already uses for `RideSetupState`/`RidePermissions` rather than growing a dependency from `:companion:core` onto `:companion:location`. - `RideService` (`companion/ride`) now constructs a `SpeedPipeline` and feeds every `LocationFix` through `onGpsFix`, logging (not silently dropping) any rejected fix. No wheel sensor is wired into this service yet (#19's remaining scope) — the KDoc added to `RideService` flags the "`null` here currently only means accuracy-rejected" assumption that breaks once #19 lands. The notification text is left as the existing fix-count display; reflecting real distance/speed there is a UI decision this issue doesn't make unilaterally. ## Deviation from the literal acceptance criteria — reported, not silently overridden The issue's acceptance criteria say `FusedLocationProviderClient` at 1 s/high accuracy. `AndroidLocationSource` (`companion/location`, built in #15/PR #97, merged before this issue's remaining scope was picked up) already deliberately uses `android.location.LocationManager`'s modern `LocationRequest.Builder` API instead — a documented decision (see that file's own KDoc), not an oversight, made possible because `minSdk` is already 31 (D19), exactly where that builder API starts. It already requests 1 Hz / `QUALITY_HIGH_ACCURACY`. Adding `play-services-location` now would contradict that already-shipped design for no behavioural gain. **No `play-services-location` dependency was added** — verified via `git diff --stat` on `gradle/libs.versions.toml` and every `build.gradle.kts` (empty). ## Tests - `companion/core/src/test/kotlin/de/butzei/pedalpebble/core/location/SpeedPipelineTest.kt` adds real JVM/Kotest coverage: the accuracy gate (including the `NaN`/`hasAccuracy()==false` edge case caught while wiring `LocationFixMapping`), a fix's own platform speed taking precedence over a position-derived one, the distance/dt fallback converging through the smoothing window to the expected steady-state speed, wheel-live fixes dropping without counting as rejections, and the display clamp. Run via `./gradlew :companion:core:test` — all passing (167 → 173 tests). - Real Android build verified via `./gradlew :companion:assembleDebug` (real SDK + JDK 21) — passes. No GPS hardware in this sandbox, so the live location callback itself is compiled, not exercised — same honesty caveat as every other sensor/location PR tonight. https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt
Speed pipeline: accuracy gate, smoothing, distance, GPS fix wiring (#17)
Some checks failed
dev-artifact / build-pbw (push) Failing after 0s
dev-artifact / build-apk (push) Failing after 0s
dev-artifact / publish (push) Has been skipped
fast-lane / host-c-tests (push) Failing after 0s
fast-lane / jvm-tests (push) Failing after 0s
fast-lane / pebble-build (push) Failing after 0s
fast-lane / lint-and-secrets (push) Failing after 0s
fast-lane / meta-declares-required-jobs (push) Failing after 0s
fast-lane / host-c-tests (pull_request) Failing after 0s
fast-lane / jvm-tests (pull_request) Failing after 0s
fast-lane / pebble-build (pull_request) Failing after 0s
fast-lane / lint-and-secrets (pull_request) Failing after 0s
fast-lane / meta-declares-required-jobs (pull_request) Failing after 0s
a4ff2923f7
Extends #69's SpeedPipeline (companion/core, core.location package) rather than
forking it: a new GpsFix type + onGpsFix() entry point sits in front of the
existing wheel/GPS arbitration and StopDetector, adding everything #17 asks for
that #69 deliberately did not build.

- Accuracy gate: fixes with accuracyMeters > 30 m (or NaN, i.e.
  Location.hasAccuracy() == false) are rejected outright, before smoothing,
  before touching the position baseline. 30 m sits above typical canopy-
  degraded fixes (10-30 m) so normal riding under trees is not thrown away,
  while catching the multipath/urban-canyon fixes (30-100+ m) bad enough to
  corrupt distance.
- Has-speed vs. distance/dt fallback: Location.getSpeed() used directly when
  the platform supplied one; a great-circle (haversine) position delta over
  elapsed time otherwise.
- ~3 s smoothing: a plain 3-sample simple moving average at the 1 Hz cadence
  NFR-B2 assumes, chosen over an EMA because GPS fixes arrive at a known,
  steady rate (no irregular spacing for a decay constant to handle better) and
  a fixed window is simpler to test and reason about.
- Real position-based distance: onGpsFix accumulates haversine deltas between
  consecutive *accepted* fixes, not speed x dt — exact regardless of whether a
  given fix carried a platform speed.
- Rejected fixes are never silent: SpeedPipeline.rejectedFixCount and
  .lastRejectedFix surface every accuracy rejection; wiring that into an
  actual GPS-quality UI (watch status strip already has a placeholder icon
  per #59) is later scope, per the issue's own wording.
- Display clamp: SpeedPipelineSample.speedMetersPerSecond now reports exactly
  0.0 once StopDetector confirms STOPPED, rather than whatever residual
  jitter produced that state — the 0.8 m/s number itself still lives in
  exactly one place, StopDetector.STOP_THRESHOLD_MPS (#69/D35); this only
  decides what a caller sees once that single decision has landed.

Android-SDK-bound half:
- companion/location/.../LocationFixMapping.kt: a two-line LocationFix ->
  GpsFix adapter, keeping the mirrored-type pattern this codebase already
  uses for RideSetupState/RidePermissions rather than growing a dependency
  from :companion:core onto :companion:location.
- RideService (companion/ride) now constructs a SpeedPipeline and feeds every
  LocationFix through onGpsFix, logging (not silently dropping) any rejected
  fix. No wheel sensor is wired into this service yet (#19's remaining scope),
  and the notification text is left as the existing fix-count display -
  reflecting real distance/speed there is a UI decision this issue does not
  make unilaterally.

Deviation from the acceptance criteria, reported rather than silently
overridden: AndroidLocationSource (companion/location, built in #15/PR #97)
already deliberately uses android.location.LocationManager's modern
LocationRequest.Builder API instead of FusedLocationProviderClient/Play
Services - a documented decision, not an oversight, made possible because
minSdk is already 31 (D19), exactly where that builder API starts. Adding
play-services-location now would contradict that already-shipped design for
no behavioural gain (1 Hz/high-accuracy is already what AndroidLocationSource
requests). No play-services-location dependency was added; verified via
`git diff --stat` on gradle/libs.versions.toml and every build.gradle.kts
(empty).

Tests: companion/core/src/test/.../SpeedPipelineTest.kt adds coverage for the
accuracy gate (including the NaN/hasAccuracy()==false edge case), the
has-speed path taking precedence over position, the distance/dt fallback
converging through the smoothing window to the expected steady-state speed,
wheel-live drops not counting as rejections, and the display clamp - run via
`./gradlew :companion:core:test` (real JVM/Kotest, all passing). Real Android
build verified via `./gradlew :companion:assembleDebug` (real SDK/JDK 21, no
GPS hardware in this sandbox so the live location callback is compiled, not
exercised - same honesty caveat as every other sensor/location PR tonight).

Claude-Session: https://claude.ai/code/session_01DAoXbRmJUf2uxNYBfdAXPt
robert merged commit cbea9e266e into main 2026-09-04 20:08:51 +02:00
Sign in to join this conversation.
No description provided.