Unit tests: snapping on out-and-back routes, off-route hysteresis #39

Open
opened 2026-08-31 17:14:50 +02:00 by robert · 2 comments
robert commented 2026-08-31 17:14:50 +02:00 (Migrated from git.butzei.de)

Goal

The navigation logic most likely to fail in a way that is confusing on the road.

Acceptance criteria

  • Snapping tested on an out-and-back route: must not jump to the return leg
  • Snapping tested on a lollipop route and on a route that crosses itself
  • Re-acquisition after a GPS gap tested
  • Off-route enter and exit hysteresis tested at the thresholds
  • Rejoining further along the route tested
  • Distance-to-turn and ETA tested against hand-computed values

Files

  • companion/src/test/
## Goal The navigation logic most likely to fail in a way that is confusing on the road. ## Acceptance criteria - [ ] Snapping tested on an out-and-back route: must not jump to the return leg - [ ] Snapping tested on a lollipop route and on a route that crosses itself - [ ] Re-acquisition after a GPS gap tested - [ ] Off-route enter and exit hysteresis tested at the thresholds - [ ] Rejoining further along the route tested - [ ] Distance-to-turn and ETA tested against hand-computed values ## Files - `companion/src/test/`
Owner

Audited this issue's 6 acceptance criteria against #31's RouteSnapper (merged tonight as PR #114) and opened PR #116 with the real, missing test scope. Not closing this issue — real scope remains (see below), leaving that call to Robert.

Already covered by RouteSnapperTest (#31), verified non-superficial (tight tolerances):

  1. Out-and-back not jumping to the return leg — covered.
  2. Re-acquisition after a GPS gap — covered (tight tolerance on resulting distance, not just "found something").
  3. Rejoining further along the route — covered (same test also demonstrates the widened-window reacquisition).

No near-duplicate tests added for these three.

Genuinely missing, now added in PR #116:
2. Lollipop and self-crossing routes. This surfaced a real bug in RouteSnapper, not just an untested case: on a route that crosses itself, ordinary GPS noise (3 m, well inside normal accuracy) near the crossing could make the windowed search jump the cursor irrecoverably to a much later, chronologically-wrong visit of the same physical point. Reproduced concretely on an 8 m half-size figure-eight; fixed with a two-phase search in RouteSnapper.snap() that is verified to change nothing on any previously-covered path (full existing test suite + real-komoot fixture pass unchanged) while closing the gap. A lollipop fixture is also added and confirmed to already track correctly without needing the fix.

Out of this issue's honest scope, not fabricated, dependency edges added instead:
4. Off-route enter/exit hysteresis at the thresholds — belongs to #32, which is separate and still open. RouteSnapper deliberately only reports a raw offset for #32 to build hysteresis on; there is no OffRouteDetector.kt in the repo yet. #39 now depends on #32.
6. Distance-to-turn and ETA against hand-computed values — belongs to #33 ("Next turn, then-turn, remaining distance and ETA"), also separate and open, targeting a NavEngine.kt that doesn't exist yet. The pieces it would combine (Cue.distanceAlongRouteMeters, RouteSnap.distanceAlongRouteMeters) both already exist independently, but nothing combines them yet, and no ETA logic exists anywhere. #39 now depends on #33.

PR: #116. Real ./gradlew :companion:core:test --rerun-tasks and koverVerify both green.

Audited this issue's 6 acceptance criteria against #31's `RouteSnapper` (merged tonight as PR #114) and opened PR #116 with the real, missing test scope. Not closing this issue — real scope remains (see below), leaving that call to Robert. **Already covered by `RouteSnapperTest` (#31), verified non-superficial (tight tolerances):** 1. Out-and-back not jumping to the return leg — covered. 3. Re-acquisition after a GPS gap — covered (tight tolerance on resulting distance, not just "found something"). 5. Rejoining further along the route — covered (same test also demonstrates the widened-window reacquisition). No near-duplicate tests added for these three. **Genuinely missing, now added in PR #116:** 2. Lollipop and self-crossing routes. This surfaced a **real bug** in `RouteSnapper`, not just an untested case: on a route that crosses itself, ordinary GPS noise (3 m, well inside normal accuracy) near the crossing could make the windowed search jump the cursor irrecoverably to a much later, chronologically-wrong visit of the same physical point. Reproduced concretely on an 8 m half-size figure-eight; fixed with a two-phase search in `RouteSnapper.snap()` that is verified to change nothing on any previously-covered path (full existing test suite + real-komoot fixture pass unchanged) while closing the gap. A lollipop fixture is also added and confirmed to already track correctly without needing the fix. **Out of this issue's honest scope, not fabricated, dependency edges added instead:** 4. Off-route enter/exit hysteresis at the thresholds — belongs to #32, which is separate and still open. `RouteSnapper` deliberately only reports a raw offset for #32 to build hysteresis on; there is no `OffRouteDetector.kt` in the repo yet. **#39 now depends on #32.** 6. Distance-to-turn and ETA against hand-computed values — belongs to #33 ("Next turn, then-turn, remaining distance and ETA"), also separate and open, targeting a `NavEngine.kt` that doesn't exist yet. The pieces it would combine (`Cue.distanceAlongRouteMeters`, `RouteSnap.distanceAlongRouteMeters`) both already exist independently, but nothing combines them yet, and no ETA logic exists anywhere. **#39 now depends on #33.** PR: #116. Real `./gradlew :companion:core:test --rerun-tasks` and `koverVerify` both green.
Owner

PR #116 merged. Independently re-verified the claimed bug fix before merging: reverted just RouteSnapper.kt's two-phase-search change (keeping the new tests), re-ran ./gradlew :companion:core:test --tests "*RouteSnapperTest*" — the new figure-eight test genuinely fails against the pre-fix code, confirming this is a real bug catch, not a test written to match already-correct behaviour. Restored the fix, full :companion:core:test suite green again.

Leaving this issue open as Sentinel recommended — real scope remains on #32 and #33, now correctly tracked as dependencies. Will pick those up as separate issues in due course.

PR #116 merged. Independently re-verified the claimed bug fix before merging: reverted just `RouteSnapper.kt`'s two-phase-search change (keeping the new tests), re-ran `./gradlew :companion:core:test --tests "*RouteSnapperTest*"` — the new figure-eight test genuinely fails against the pre-fix code, confirming this is a real bug catch, not a test written to match already-correct behaviour. Restored the fix, full `:companion:core:test` suite green again. Leaving this issue open as Sentinel recommended — real scope remains on #32 and #33, now correctly tracked as dependencies. Will pick those up as separate issues in due course.
Sign in to join this conversation.
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
robert/PedalPebble#39
No description provided.