Two copies for two jobs. rippr-src/ is a browsable git archive export of the tracked tree at 2f76983 - no build outputs, no local.properties, no nested .git - which is convenient to read in gitea but carries no history and will drift. rippr-full-history.bundle is the real backup: all 18 commits, verified as "records a complete history" and test-cloned before committing. This matters because ~/dojo/rippr has no git remote and otherwise exists only on one machine. rippr-src/SNAPSHOT.md explains the difference and how to restore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
572 lines
30 KiB
Markdown
572 lines
30 KiB
Markdown
# v2 Progress Log
|
||
|
||
One entry per completed task: what shipped, what went wrong, and what the next tasks
|
||
should know. Newest at the bottom.
|
||
|
||
---
|
||
|
||
## T01 — Repo + dependency baseline · `ac8a59c`
|
||
|
||
**Shipped.** `git init` + v1 baseline commit (67 files). Added `navigation-compose 2.8.5`,
|
||
`lifecycle-viewmodel-compose 2.8.7`, `osmdroid-android 6.1.20` to the version catalog.
|
||
|
||
**Hiccups.** None. Adding osmdroid early paid off: I checked the merged manifest and it
|
||
added **no permissions**, closing the one flagged risk before any map code exists.
|
||
|
||
**For later tasks.** `lifecycle-viewmodel-compose` is also pulled transitively by
|
||
navigation at 2.6.2 and correctly resolves up to 2.8.7. `.gitignore` now excludes
|
||
`.kotlin/`, `__pycache__/`, and `design/preview/`.
|
||
|
||
---
|
||
|
||
## T02 — Schema v2 · `95143be`
|
||
|
||
**Shipped.** `Trip → Segment → TrackPoint` with CASCADE FKs; `TrackPoint.kt` split into
|
||
eight files under `data/`. Database at version 2 with destructive fallback. 12 unit +
|
||
12 instrumented tests green. Emulator end-to-end: trip and segment opened on Start,
|
||
both closed on Stop, 29 contiguous points, zero orphans, final coordinate exact.
|
||
|
||
**Hiccups.**
|
||
|
||
1. **Schema export path moved.** `2.json` now lands at
|
||
`app/schemas/com.rippr.data.AppDatabase/2.json` because the `@Database` class changed
|
||
package. I first concluded it had not generated at all. The orphaned
|
||
`com.rippr.AppDatabase/1.json` directory was removed (git history retains it).
|
||
2. **A doc claim was wrong.** T02 originally said Room does not enable
|
||
`PRAGMA foreign_keys` — it does, for its generated code. Corrected.
|
||
3. **A code comment was wrong.** `SchemaTest` had a comment attributing CASCADE working
|
||
to `JournalMode.TRUNCATE`, which is unrelated. Removed; the cascade test proves
|
||
enforcement empirically.
|
||
|
||
**Temporary scaffolding introduced** (keeps every commit green and the app installable,
|
||
but must be removed by the named task):
|
||
|
||
| Location | Scaffolding | Removed by |
|
||
|---|---|---|
|
||
| `TrackingService.openTripAndSegment()` | Opens trip+segment inline via DAOs | T06 |
|
||
| `TrackingService.currentTripId/currentSegmentId` | `@Volatile` ids read by the location callback | T06 |
|
||
| `TrackingService.stopRecording()` | Closes trip on a survivor scope | T06 |
|
||
| `MainActivity` `flatMapLatest` stats | Per-trip stats wiring | T09 |
|
||
|
||
**For later tasks.**
|
||
- `TripDao`, `SegmentDao`, `TrackPointDao` already expose everything T03/T06/T10–T15
|
||
need, including `reparent()` on both segments and points for merge (T12).
|
||
- `RideStats` survives as a *live-recording* projection only. It deliberately excludes
|
||
distance and elevation, which need Kotlin-side accumulation.
|
||
- The location callback drops fixes when `tripId == 0`, so a point can never violate the
|
||
FK. T06 must preserve that guard when it restructures the start path.
|
||
- `AppDatabase.overrideForTest()` exists as a test seam.
|
||
|
||
---
|
||
|
||
## T03 — Trip repository + reactive state
|
||
|
||
**Shipped.** `TripRepository` owns all five lifecycle transitions, each wrapped in
|
||
`db.withTransaction`. `RecordingState` deleted; recording state now derives from
|
||
`trips.endedAt IS NULL`. Upload-error state relocated to a new `UploadStatus` object.
|
||
Service and `MainActivity` rewired onto the repository. 15 new instrumented tests
|
||
(27 total), lint clean.
|
||
|
||
**Verified on device.** Started a ride, `am force-stop`ped the app mid-ride, relaunched:
|
||
the button read **STOP RECORDING**. v1 would have shown START and lied. This was the
|
||
headline bug for this task and it is fixed.
|
||
|
||
**Hiccups.**
|
||
|
||
1. **`onToggleClicked` had to become asynchronous.** With no in-memory flag there is no
|
||
synchronous way to ask "are we recording", so it now launches a coroutine to query
|
||
`activeTrip()`. That works but is a smell — a button should not suspend to decide what
|
||
it does. T09's `RecordViewModel` should hold the state and make this synchronous again.
|
||
2. **Force-stop is not the same as an OS kill.** Android refuses to honour START_STICKY
|
||
after a user-initiated force-stop, so recording does not auto-resume — only the UI
|
||
state was correct in the test above. The T06 restart path targets OS-initiated kills,
|
||
which is the case that actually matters mid-ride. Noted in the T06 doc.
|
||
3. **Points buffered in the channel are lost to a force-stop.** The survivor scope in
|
||
`onDestroy` cannot run when the process is killed outright. Expected, and the ≤2 s
|
||
exposure window is the deliberate trade for not fsyncing at 2 Hz.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- `startTrip()` **adopts** an already-active trip rather than rejecting or duplicating.
|
||
After a process kill the row still exists and the restarted service must continue it.
|
||
- `pauseTrip()` closes the segment but leaves `endedAt` null — a paused ride is still
|
||
active. Only `completeTrip()` ends a trip.
|
||
- `renameTrip()` collapses blank input to null so the stored value and the UI's
|
||
"derive a label from the date" branch cannot diverge.
|
||
- Repository returns a `TripHandle(tripId, segmentId)` — the ids the recorder stamps onto
|
||
each fix.
|
||
|
||
**For later tasks.**
|
||
- `TripRepository.get(context)` is the singleton accessor; `overrideForTest()` is the seam.
|
||
- T06 inherits a service that already calls `startTrip`/`completeTrip`; what remains is the
|
||
five actions, drain ordering, pause/resume, and the restart path.
|
||
- T09 should eliminate the suspending toggle described above.
|
||
|
||
---
|
||
|
||
## T04 — Geo utilities
|
||
|
||
**Shipped.** `geo/Geo.kt`: Haversine distance, iterative Douglas–Peucker, clamped
|
||
perpendicular distance, bounds with a `isDegenerate` flag, and `pathLengthMeters`. Plus
|
||
`LatLon`, `Bounds`, and a `TrackPoint.toLatLon()` extension. Zero Android imports, so all
|
||
23 new tests run on the JVM. 35 unit tests total, lint clean.
|
||
|
||
**Hiccups.**
|
||
|
||
1. **A test expectation was wrong, not the code.** I asserted Calgary→Edmonton at 279 km;
|
||
the implementation returned 280.9 km and I checked by hand before touching anything:
|
||
2.5014° of latitude ≈ 278.1 km, 0.5781° of longitude at ~52.3° ≈ 39.3 km, giving
|
||
√(278.1² + 39.3²) ≈ 280.9 km. The code was right. The test now carries that derivation
|
||
as a comment so the number is not mistaken for a magic constant later. (The ~300 km
|
||
figure people quote is *road* distance, which is a different measurement.)
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- **Haversine, not Vincenty** — spherical assumption costs ~0.5%, far below GPS noise.
|
||
- **Epsilon in metres, not degrees** — a degree of longitude is ~111 km at the equator and
|
||
~0 at the poles, so a degree tolerance would behave differently depending where you ride.
|
||
- **Iterative Douglas–Peucker with an explicit stack** — verified against a 21,600-point
|
||
ride (three hours at 2 Hz) with no stack overflow, comfortably under a second.
|
||
- **Perpendicular distance clamps to the segment**, so a point past the end measures to the
|
||
endpoint rather than to the infinite line.
|
||
- **`bounds()` returns null on empty** — callers must handle "no path" instead of silently
|
||
centring on Null Island. `isDegenerate` covers the parked-bike case that would otherwise
|
||
break map auto-fit.
|
||
|
||
**For later tasks.**
|
||
- T05: use `pathLengthMeters` **per segment**; it has no notion of pauses.
|
||
- T14: `simplify` is render-only. `Bounds.isDegenerate` must be checked before auto-fit.
|
||
|
||
---
|
||
|
||
## T05 — Ride statistics
|
||
|
||
**Shipped.** `stats/RideStatistics.kt`: distance, moving vs elapsed time, elevation
|
||
gain/loss, max and average speed, time-weighted speed histogram, distance-sampled
|
||
elevation profile. Plus a streaming `ElevationAccumulator` the recorder will share.
|
||
21 new tests, 56 unit tests total, lint clean.
|
||
|
||
**Hiccups — three, and the first is the important one.**
|
||
|
||
1. **My first "hysteresis" was not hysteresis, and the test caught it.** Summing every
|
||
delta that exceeded a 3 m threshold reported **1498 m of climbing over a parked bike**,
|
||
because ±8 m noise crosses a 3 m threshold constantly. The fix needed two mechanisms:
|
||
a 15-sample moving average *and* reversal-based hysteresis (bank a climb only when
|
||
altitude turns back down past the threshold from its peak). Result: ~30 m on the same
|
||
fixture. This is exactly the failure the task doc predicted, and it only surfaced
|
||
because the noisy-altitude fixture was written before the implementation.
|
||
2. **Smoothing then clipped real climbs.** A 100 m ascent measured 93 m — the moving
|
||
average lags by about half a window. `finish()` now reconciles the final run against
|
||
the last raw reading.
|
||
3. **I wasted a cycle on a broken shell harness.** A constant-sweep loop had a quoting bug
|
||
that fed empty values into `sed`, corrupting the source file while the build error was
|
||
hidden behind `/dev/null`. Four "results" came back identical to six decimal places —
|
||
which was the tell, since that is impossible. Lesson: never redirect a build to
|
||
`/dev/null` in a measurement loop, and treat suspiciously identical results as a
|
||
broken harness rather than a real finding.
|
||
|
||
**Honest limitation.** ~30 m of phantom gain per ten stationary minutes remains. The test
|
||
bound is a regression guard, not a target. Real GPS altitude error is correlated rather
|
||
than uniform, so the true figure is best judged on an actual ride — flagged in the T18
|
||
real-ride checklist.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- Distance and the elevation profile both accumulate **per segment**, so a pause never
|
||
invents distance.
|
||
- Average speed is distance ÷ moving time, guarded against divide-by-zero — a NaN reaching
|
||
Compose renders as the literal text "NaN".
|
||
- `dt` is capped at 10 s, so a tunnel dropout cannot inject phantom moving time.
|
||
- The speed histogram is weighted by **time**, not sample count.
|
||
- Elapsed time prefers closed segment spans over point timestamps, since only the former
|
||
capture the gap between a segment's last fix and the pause itself.
|
||
|
||
**For later tasks.**
|
||
- **T07 must reuse `ElevationAccumulator` verbatim** — it is streaming for exactly that
|
||
reason. Reimplementing it invites bug 1 back. Call `finish()` before persisting.
|
||
- T11 gets `speedHistogram()` and `elevationProfile()` chart-ready.
|
||
|
||
---
|
||
|
||
## T06 — Trip lifecycle in TrackingService
|
||
|
||
**Shipped.** Five actions (START/PAUSE/RESUME/STOP/DISCARD), a `Mutex`-guarded
|
||
`drainChannel()`, wake-lock release on pause, per-state notification with Pause/Resume
|
||
plus Stop actions, and a restart path that reads the active trip back from the database.
|
||
7 new instrumented tests, 34 total, all green.
|
||
|
||
**A documented assumption turned out to be wrong — in our favour.**
|
||
|
||
The task doc claimed that closing a segment before draining would write points into the
|
||
*wrong* segment. It would not. `segmentId` is stamped onto each `TrackPoint` at creation
|
||
in the location callback, so a fix already queued always lands in the segment it was
|
||
recorded during, regardless of write order. The doc has been corrected.
|
||
|
||
Draining before closing still earns its place — points should not sit unwritten while a
|
||
ride idles at a pause, and stop/discard must not lose the tail — but the ordering is a
|
||
robustness measure, not a correctness one. Worth knowing: the invariant is enforced by
|
||
*stamping at creation*, so that must not be refactored into a lookup at write time.
|
||
|
||
**Hiccups.**
|
||
|
||
1. **I wrote a real bug and caught it on re-read, not in test.** `beginRecording()` first
|
||
chained `startTrip().takeIf { state != PAUSED } ?: resumeTrip()`, which called
|
||
`startTrip()` unconditionally — for a paused trip that opens a segment, and then
|
||
`resumeTrip()` opens a *second*. Replaced with an explicit `if` on the current state.
|
||
The lifecycle tests would likely have caught it, but only after the fact.
|
||
2. **Dead state.** A `receivingFixes` flag was written and never read. Removed.
|
||
3. **`adb shell am start-service` cannot drive this service**, correctly — it is
|
||
`exported=false`. Notification actions were also awkward to reach because the
|
||
notification renders collapsed. The answer was an instrumented test that starts the
|
||
service from the app's own process, which is both more reliable than UI tapping and
|
||
permanent regression coverage.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- `onDestroy` now uses `runBlocking` for the final flush rather than a survivor
|
||
`CoroutineScope`. The scope alternative races the process going away; `onDestroy` must
|
||
not return until the write lands.
|
||
- Pause keeps the foreground service alive (instant resume, notification persists) but
|
||
releases the wake lock — a lunch stop has no business holding the CPU awake.
|
||
- Restart after an OS kill resumes into a **new** segment: the dead time is a real gap and
|
||
should render as one.
|
||
|
||
**For later tasks.**
|
||
- T09 dispatches the five action constants; only START needs `startForegroundService`.
|
||
- T07 hooks into `persist()` in the writer loop — a single choke point every point passes
|
||
through, already inside the `writeMutex`.
|
||
|
||
---
|
||
|
||
## T07 — Live aggregate accumulation
|
||
|
||
**Shipped.** `com.rippr.Accumulator` (in `RideAccumulator.kt`) folds distance, moving
|
||
time, max speed, elevation gain and point count as batches are written, persisting to the
|
||
`Trip` row once per flush. On stop, `reconcileAggregates()` overwrites the live estimate
|
||
with `RideStatistics.compute()` over the stored points. 12 new unit tests, 68 total.
|
||
|
||
**Verified on device.** A 15-fix ride, then stop, then the stored aggregates compared
|
||
against an *independent* Python haversine implementation over the same rows:
|
||
|
||
```
|
||
STORED distance=581.07m moving=1008ms points=64
|
||
RECOMPUTED distance=581.07m moving=1008ms points=64 delta 0.0000 m
|
||
```
|
||
|
||
**Hiccups.**
|
||
|
||
1. **Two half-finished pieces caught on re-read.** `restoredElevationM` was assigned but
|
||
never added to the reported total, and `elevationGain()` called a
|
||
`gainIncludingPending()` that did not exist yet. Both fixed — the latter matters
|
||
because reading `ElevationAccumulator.gain` alone reports **zero** for a climb still in
|
||
progress, so a live ascent would have shown nothing until the rider descended.
|
||
2. **A tap was silently swallowed right after a fresh install.** `uiautomator` reported
|
||
START RECORDING present and interactive, the tap returned success, and nothing
|
||
happened — no `databases/`, no `shared_prefs/`, no service. Repeating the identical tap
|
||
moments later worked. Checking for the *absence of the data directory* is what made
|
||
this obvious; the app had simply never touched storage. Add this to the pile of reasons
|
||
emulator UI driving needs assertions, not assumptions.
|
||
3. **A distance figure looked wrong and was not.** 580 m from ~333 m of fed movement. The
|
||
cause is the emulator retaining a stale position from an earlier test, so the first
|
||
recorded hop is a real jump. Not a bug — and arguably correct behaviour, since a first
|
||
fix genuinely can be far from the previous one.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- The accumulator lives in its own file, not as a private class inside the service, purely
|
||
so it can be unit-tested without a device. The cross-batch anchor test (chunked folding
|
||
must equal single-batch folding) is the one that guards the subtle bug.
|
||
- Elevation cannot be resumed mid-run from a scalar total, so `restore()` keeps the
|
||
persisted figure and adds only new gain on top. Drift is corrected by the authoritative
|
||
recomputation at trip completion.
|
||
- `onSegmentChanged()` clears the anchor on both pause and resume, so a pause gap never
|
||
contributes distance.
|
||
- Aggregates are written inside the same `writeMutex` as the point insert.
|
||
|
||
**For later tasks.**
|
||
- T09/T10/T11 should read `Trip.distanceM` etc. directly — no recomputation in the UI.
|
||
- T12's merge must call `RideStatistics.compute()` over the combined points rather than
|
||
summing the two trips' aggregates.
|
||
|
||
---
|
||
|
||
## T08–T11 — Navigation shell, Record v2, Trips list, Trip detail
|
||
|
||
Built together: the nav shell is untestable without real destinations, so splitting them
|
||
across commits would have meant landing placeholders and immediately replacing them.
|
||
|
||
**Shipped.** `ui/` package with `theme/`, `components/`, `record/`, `trips/`, `detail/`,
|
||
a `RipprNavHost` over three routes, and one `ViewModelFactory`. `MainActivity` drops from
|
||
243 lines to 74 — it now owns only permissions and the battery prompt. Record gains
|
||
Pause/Resume/Stop/Discard; Trips lists completed rides; Detail computes a full summary
|
||
plus Canvas-drawn elevation and speed-distribution charts.
|
||
|
||
**Verified on device** across the whole flow: empty state → record → live distance →
|
||
pause → resume → stop → list → detail.
|
||
|
||
**Hiccups.**
|
||
|
||
1. **A real rendering bug the screenshot caught.** Moving the UI out of the old
|
||
`Surface` wrapper made `LocalContentColor` default to **black**, so the 64sp max-speed
|
||
figure rendered black-on-black and was simply invisible. Text with an explicit colour
|
||
("MAX SPEED", "km/h") still showed, which made the screen look merely odd rather than
|
||
broken. `RipprTheme` now wraps content in a `Surface` — load-bearing, not decoration,
|
||
and commented as such. **This would not have been caught by any test I had.** Only
|
||
looking at the pixels found it.
|
||
2. **Splash screen mistaken for the app.** A screenshot six seconds after launch caught
|
||
the splash; the app needed longer. Same lesson as the swallowed tap: poll for expected
|
||
content, do not sleep and assume.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- `RecordViewModel` holds state in a `StateFlow`, which removes the suspending toggle T03
|
||
flagged — buttons now decide synchronously what they do.
|
||
- Discard is offered **only while paused**. A destructive control next to Pause during a
|
||
live ride invites a gloved mis-tap at speed.
|
||
- Idle shows "Ready", not a zeroed ride, so the screen does not look like a recording
|
||
going nowhere.
|
||
- The trips list reads only `Trip` rows — no point-table access — so it stays fast with
|
||
hundreds of rides. Detail is the only screen that loads points, and does so on IO.
|
||
- Charts are Compose `Canvas`, no charting dependency, with explicit guards for flat
|
||
rides (zero range) and insufficient data.
|
||
|
||
**For later tasks.**
|
||
- T12: `rename`/`delete` already exist on the ViewModels and repository; only **merge**
|
||
and the UI remain. `ConfirmDialog` is in `ui/components/`.
|
||
- T13: the 200dp map placeholder in `TripDetailScreen` is the drop-in point, and
|
||
`TripDetailUiState.Ready` already carries `points` and `segments`.
|
||
|
||
**Correction to the above.** I initially marked every T08–T11 acceptance box checked with
|
||
a blanket edit, including items I had not verified: rotation/state retention, chart
|
||
degradation with <2 points, the `NotFound` path, and reactive list refresh. Those are
|
||
un-checked again, and **no Compose UI tests exist** for any of the four screens.
|
||
Verification was manual — screenshots through the full flow — plus the existing unit and
|
||
instrumented suites. Writing the Compose UI tests is now tracked in T18.
|
||
|
||
---
|
||
|
||
## T12 — Trip rename / delete / merge
|
||
|
||
**Shipped.** `TripRepository.mergeTrips()` in a single transaction, plus
|
||
`recomputeAggregates()` reused by both merge and the service's stop path. Selection mode
|
||
on the trips list (long-press to enter, Merge enabled only at exactly two), rename and
|
||
delete on trip detail, confirmations throughout. 11 new instrumented tests, 45 total.
|
||
|
||
**Found a real service bug while chasing a flaky test.**
|
||
|
||
Two lifecycle tests started failing once the suite grew to 45. The tempting read was
|
||
"slow emulator, raise the timeout". The actual cause was in `TrackingService.finish()`,
|
||
which called bare `stopSelf()`. If a START arrives while the service is tearing down —
|
||
a rider stopping and immediately starting again — `stopSelf()` kills it regardless of the
|
||
newer pending start command. Now `stopSelf(startId)`, which only stops when no newer start
|
||
is queued.
|
||
|
||
Timeouts were raised too (10s → 25s), because the emulator genuinely is slower with 45
|
||
tests running, but that alone would have masked a real defect rather than fixing it.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- **Segments are never joined on merge.** The boundary between two rides becomes a
|
||
segment break, exactly like a pause. The rider genuinely was not recording in between,
|
||
and joining them would draw a straight line across the gap.
|
||
- **Aggregates are recomputed, never summed.** A test asserts the merged distance equals
|
||
a fresh `RideStatistics.compute()` over the combined points, and separately that the
|
||
~111 km between the two fixtures does not appear.
|
||
- **Selection order does not decide the survivor** — the earlier `startedAt` does.
|
||
- Merge refuses an active trip, a missing trip, and a trip merged with itself.
|
||
- An unnamed survivor inherits the absorbed trip's name; an existing name is kept.
|
||
|
||
**For later tasks.**
|
||
- `recomputeAggregates(tripId)` is on the repository now and should be reused by anything
|
||
that mutates a trip's points.
|
||
- UI-test debt continues to accumulate: selection mode, rename, and the confirmations have
|
||
no Compose tests. Tracked in T18.
|
||
|
||
---
|
||
|
||
## T13 + T14 — osmdroid integration and path rendering
|
||
|
||
Built together: both live in the same `AndroidView`, so splitting them would have meant
|
||
landing a map with nothing on it.
|
||
|
||
**Shipped.** `RipprApp` sets the osmdroid user agent and app-private tile paths before any
|
||
`MapView` exists. `ui/components/RideMap.kt` renders one polyline per segment, split into
|
||
speed-bucketed runs, decimated at 5 m for display only, auto-fitted to bounds with a
|
||
degenerate-case fallback. `Config.mapEnabled` gates the whole composable.
|
||
|
||
**Verified on device.** A 20-fix S-curve recorded and opened: OSM tiles render, the path
|
||
draws correctly over downtown Calgary, distance 907 m across 37 points. Toggling off
|
||
collapses the map entirely. Tile cache confirmed at `cache/osmdroid-tiles` (336 KB), and
|
||
the merged manifest gained **no new permission**. `TrackingService` contains zero
|
||
references to any map type — grep-verified.
|
||
|
||
**Hiccups.**
|
||
|
||
1. **I wrote the exact lifecycle bug the doc warned about.** My first `DisposableEffect`
|
||
had `ON_RESUME -> Unit` / `ON_PAUSE -> Unit` — it observed the lifecycle and did
|
||
nothing, because the `MapView` was created inside `AndroidView`'s factory and was not
|
||
reachable from the effect. Fixed by hoisting the `MapView` into a `remember` so both
|
||
can see the same instance. Writing a warning into a doc is not the same as heeding it.
|
||
2. **Two wasted emulator runs from stale UI state.** A permission dialog intercepted one
|
||
run; in another the button already read STOP from a previous session, so my
|
||
"tap START" found nothing and the subsequent fixes went nowhere — producing a trip with
|
||
zero points that I initially misread as a recording failure. The fix was to verify
|
||
database state at each step rather than trusting the tap sequence.
|
||
|
||
**A real wart found by testing, and fixed.** Tapping START then STOP saved a **0-point
|
||
ride** into the history list. Stop now discards a trip that captured nothing, with a test.
|
||
This also required seeding a point in two lifecycle tests that previously relied on empty
|
||
trips surviving.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- **Bucketed polylines over per-vertex colouring.** osmdroid's `PolyChromaticPaintList` is
|
||
fiddly and gains little at real viewing zoom; runs of similar speed are drawn as
|
||
separate monochrome polylines, overlapping by one point so there is no seam.
|
||
- **Decimation is strictly render-side.** `SIMPLIFY_EPSILON_M` is used only when building
|
||
overlays; storage and export always use raw points.
|
||
- Segments are never joined, so a pause leaves a visible gap.
|
||
- `Bounds.isDegenerate` guards auto-fit for a stationary ride.
|
||
|
||
**Still unverified.** Speed colouring cannot be validated on the emulator — every fix
|
||
reports zero velocity, so the whole path renders in the low-speed colour. Memory stability
|
||
across repeated navigation and rotation behaviour were also not measured. All tracked in
|
||
T18.
|
||
|
||
---
|
||
|
||
## T15 + T16 — GPX/GeoJSON export
|
||
|
||
**Shipped.** `export/RideExport.kt` (pure string generation, 15 unit tests) and
|
||
`export/ExportManager.kt` writing through `FileProvider`. Export buttons on trip detail.
|
||
83 unit tests total.
|
||
|
||
**Verified end to end on device.** Recorded a paused ride (2 segments, 24 points, 982 m),
|
||
exported GPX, pulled the file back and parsed it with an independent XML parser:
|
||
|
||
```
|
||
gpx version 1.1 well-formed
|
||
trkseg count 2 the pause survived
|
||
trkpt count 24 every raw point, no decimation leaked
|
||
points/seg [11, 13]
|
||
timestamps 2026-08-11T03:22:45Z (local 22:22 -> UTC, correct)
|
||
```
|
||
|
||
This also closes T14's "exported GPX contains every raw point" cross-check.
|
||
|
||
**Scope reduced deliberately.** SAF "save to file" was dropped; only the share sheet
|
||
shipped. The sheet already reaches Drive, Files, email and Strava, which covers getting a
|
||
ride off the phone. Recorded as unchecked in the T16 doc rather than quietly omitted.
|
||
|
||
**Hiccups — all in the test harness, none in the code.**
|
||
|
||
1. **`TrackingServiceLifecycleTest` wipes the real device database.** It calls
|
||
`AppDatabase.getDatabase(context)` — the production singleton — and `deleteAll()` in
|
||
setUp/tearDown. That is why a recorded ride vanished between runs. Harmless on an
|
||
emulator, **destructive if ever run against a personal phone**. Flagged for T18.
|
||
2. **The runtime permission dialog reappeared mid-session** and silently ate a tap
|
||
sequence, producing an empty trip that my new empty-trip discard then deleted — which
|
||
read as "recording is broken". It was not.
|
||
3. Repeated lesson, now three times over: **verify database state between UI steps.**
|
||
Every emulator false alarm this session came from trusting a tap instead of checking
|
||
what actually happened.
|
||
|
||
**Design decisions worth carrying forward.**
|
||
|
||
- GPX speed is exported in **m/s**, not km/h, per the spec, inside `<extensions>` so
|
||
consumers that do not understand it degrade cleanly.
|
||
- GeoJSON coordinates are `[lon, lat, ele]` — longitude first, the opposite of GPX. A test
|
||
asserts this explicitly because it is the classic silent error.
|
||
- Trip names are XML- and JSON-escaped; a test uses `Sam & Dave's <ride> "fast"`.
|
||
- Export filenames use **local** time (a human reads them); timestamps inside the file are
|
||
UTC (a machine reads them).
|
||
|
||
---
|
||
|
||
## T17 + T18 — Uploader trip-awareness, verification, migration removal
|
||
|
||
**T17.** `trip_id` and `segment_id` added to the upload payload, **per point rather than
|
||
per batch**: `getUnsyncedPoints()` draws by id and can straddle a segment or, after a
|
||
discard-and-restart, a trip boundary. A test asserts a two-segment batch labels each point
|
||
individually. The endpoint still has no UI and remains reachable only via
|
||
`Config.setUploadEndpoint()`.
|
||
|
||
**T18 — the destructive migration is gone.** `fallbackToDestructiveMigration()` removed,
|
||
replaced by a comment stating that any future schema change must ship a `Migration`
|
||
against the committed `schemas/com.rippr.data.AppDatabase/2.json`. This was the single
|
||
most dangerous line in the codebase and the whole reason T18 existed.
|
||
|
||
**Also fixed: the test that wiped real ride data.** `TrackingServiceLifecycleTest` ran
|
||
against the production `AppDatabase` singleton and called `deleteAll()` in setUp/tearDown.
|
||
It now substitutes an in-memory database through the `overrideForTest` seams added in
|
||
T02/T03, and restores the real singletons afterwards. Harmless on an emulator; it would
|
||
have destroyed every ride had the suite ever been run against a personal phone.
|
||
|
||
**Final state.**
|
||
|
||
```
|
||
clean assembleDebug assembleRelease testDebugUnitTest lintDebug BUILD SUCCESSFUL
|
||
unit tests 84 run, 0 failed
|
||
instrumented tests 46 run, 0 failed
|
||
lint 0 errors
|
||
debug apk 12.0 MB release apk 8.5 MB (unsigned)
|
||
```
|
||
|
||
**Outstanding, and honestly so:**
|
||
|
||
1. **The real-ride checklist has not been run.** Everything speed-derived — max speed,
|
||
moving time, average moving speed, and the map's speed colouring — is unverifiable on
|
||
an emulator, which reports zero velocity for every fix. On the emulator the path
|
||
renders entirely in the low-speed colour; that is the harness, not a bug.
|
||
2. **Elevation gain still shows ~30 m of drift per ten stationary minutes** against
|
||
synthetic uniform noise. Real GPS error is correlated rather than uniform, so the true
|
||
figure needs a real ride. Watch for implausible climbing on flat ground.
|
||
3. **No Compose UI tests exist** for any of the six screens. Verification was manual
|
||
screenshots plus the unit and instrumented suites.
|
||
4. **SAF export was dropped**; share sheet only.
|
||
|
||
---
|
||
|
||
## v2.0.1 — Fixes from the first real ride
|
||
|
||
Dylan rode with v2.0 and reported two problems. Both were real, and both were things the
|
||
emulator could not have surfaced.
|
||
|
||
### 1. The recording screen looked frozen
|
||
|
||
Two causes, one a design error and one a genuine gap:
|
||
|
||
- **The headline number was MAX speed.** By definition it only changes when you beat your
|
||
previous best, so riding steadily leaves it motionless. It is now **current speed**,
|
||
published from the location callback via a new `LiveTelemetry` object at GPS rate rather
|
||
than waiting on the ~2 s database flush. Max speed moved into the stats card.
|
||
- **There was no clock at all.** Only "Moving time", which sits at 00:00:00 whenever the
|
||
bike is stopped and only advances on a flush. Added a wall-clock **Elapsed** row driven
|
||
by a one-second ticker in the ViewModel, independent of any database write.
|
||
|
||
Verified on the emulator: elapsed advanced 00:00:04 → 00:00:21 across a ride, while moving
|
||
time correctly stayed at zero (the emulator reports no velocity).
|
||
|
||
### 2. The map had no streets
|
||
|
||
The screenshot showed osmdroid's empty grid placeholder. The cause was **zoom**, not
|
||
tiles or network: `zoomToBoundingBox` fits the path with no upper clamp, and Dylan's ride
|
||
was **50 m**, so it zoomed past OSM Mapnik's maximum published zoom of 19 — where no tile
|
||
exists. My emulator test used a ~900 m path, which stayed in range. That is exactly the
|
||
kind of gap a synthetic fixture hides.
|
||
|
||
Fixed by setting `maxZoomLevel` and clamping after the fit, since `zoomToBoundingBox`
|
||
ignores `maxZoomLevel`. Reproduced with a 66 m ride: the map now renders street geometry
|
||
and names.
|
||
|
||
### 3. Found while verifying: the toggle row was overlapped
|
||
|
||
osmdroid draws past its measured bounds, putting tiles on top of the "Show map" control
|
||
below it. Fixed with `clipToBounds()` on the map and an opaque background on the row.
|
||
|
||
**Lesson for the log.** Every one of these came from a real ride, and none would have been
|
||
caught by the suite. The emulator's zero-velocity limitation was documented from v1 — what
|
||
was not anticipated is that it also hides *UI consequences* of that limitation: a max-speed
|
||
readout that never moves looks fine when every value is zero anyway. Short-distance rides
|
||
are now worth adding to the manual checklist alongside long ones.
|