Re-exported at 46a0726, which adds README.md plus docs/ARCHITECTURE, DEVELOPMENT, TESTING, v1 history including the original brief, and a v3 backlog. 112 files, and the bundle now carries 19 commits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
289 lines
12 KiB
Markdown
289 lines
12 KiB
Markdown
# Architecture
|
|
|
|
The decisions that shaped Rippr, and — more usefully — *why*. Several exist because
|
|
something specific went wrong; those are marked.
|
|
|
|
---
|
|
|
|
## The recording pipeline
|
|
|
|
```
|
|
onLocationResult (main looper)
|
|
│ stamp tripId + segmentId, sanitize speed, reject bad-accuracy fixes
|
|
▼
|
|
Channel(UNLIMITED) ──► single writer coroutine (Dispatchers.IO)
|
|
│ batches of 25, flushed every ~2s, under writeMutex
|
|
├─► insertPoints()
|
|
└─► Accumulator.fold() → updateAggregates() on the Trip row
|
|
```
|
|
|
|
### Why an unbounded Channel
|
|
|
|
The location callback must never block. Writing to SQLite from inside `onLocationResult`
|
|
would couple GPS delivery to disk latency, and a slow flash write would drop fixes. The
|
|
channel decouples them: the callback's `trySend` never suspends, and back pressure stalls
|
|
the *writer*, not the sensor.
|
|
|
|
**Do not replace this with a direct write.** It is the single most load-bearing decision in
|
|
the recording path.
|
|
|
|
### Why points are stamped at creation
|
|
|
|
Each `TrackPoint` carries its `tripId` and `segmentId` from the moment it is built in the
|
|
callback, not looked up at write time.
|
|
|
|
This makes pausing safe: a fix already queued when the rider pauses lands in the segment it
|
|
was actually recorded during, whatever order the writes happen in.
|
|
|
|
> **Correction worth remembering.** The v2 plan originally claimed that closing a segment
|
|
> before draining the channel would misfile points. That was wrong — stamping at creation
|
|
> already prevents it. Draining before closing is still done, but as robustness (not
|
|
> leaving points unwritten while a ride idles), not correctness. If anyone ever refactors
|
|
> stamping into a write-time lookup, this guarantee disappears.
|
|
|
|
### Why batched writes
|
|
|
|
A 2 Hz stream would otherwise mean two transactions per second for hours. Batching to 25
|
|
points or 2 seconds — whichever comes first — cuts that to one write per two seconds.
|
|
|
|
### Why WAL journal mode
|
|
|
|
A ride is unrecoverable if writes are lost to a crash, but `fsync` on every insert at 2 Hz
|
|
burns battery. WAL with `NORMAL` sync survives app crashes; only an OS-level crash can lose
|
|
the last few points. That is the accepted trade.
|
|
|
|
### Why the wake lock
|
|
|
|
A foreground service alone does not guarantee CPU time with the screen off on all OEMs. A
|
|
`PARTIAL_WAKE_LOCK` keeps the writer coroutine running. It is **released on pause** — a
|
|
lunch stop has no business holding the CPU awake — and re-acquired on resume.
|
|
|
|
---
|
|
|
|
## Data model
|
|
|
|
```
|
|
Trip 1───* Segment 1───* TrackPoint
|
|
```
|
|
|
|
```kotlin
|
|
Trip(id, startedAt, endedAt?, name?, state,
|
|
distanceM, movingMillis, maxSpeedKmh, elevationGainM, pointCount)
|
|
Segment(id, tripId, startedAt, endedAt?)
|
|
TrackPoint(id, tripId, segmentId, timestamp, lat, lon, speedKmh, altitudeM,
|
|
accuracyM, bearingDeg, synced)
|
|
```
|
|
|
|
### Why Segment exists
|
|
|
|
Three of the four things v1 lacked — reset, pause, and trips — were the same missing
|
|
concept. Segments are what make pause *correct*: they represent a pause-free stretch of
|
|
recording, so every consumer naturally leaves a gap where the rider stopped.
|
|
|
|
They propagate into three places, and all three would be wrong without them:
|
|
- **Distance** accumulates only within a segment
|
|
- **Map polylines** are drawn one per segment, never joined
|
|
- **GPX** emits one `<trkseg>` per segment, which is exactly how GPX represents a gap
|
|
|
|
### Why both `endedAt` and `state`
|
|
|
|
`endedAt == null` distinguishes active from finished, but cannot distinguish RECORDING from
|
|
PAUSED. The service needs that difference to decide what to do when the OS restarts it
|
|
mid-ride.
|
|
|
|
### Why aggregates are denormalised onto Trip
|
|
|
|
**minSdk 26 means SQLite 3.18, which has no window functions** — no `LAG`, no `OVER`. There
|
|
is simply no SQL expression for "distance from the previous point". So consecutive-point
|
|
maths happens in Kotlin, in the writer loop that already touches every point, and the
|
|
result is persisted on the Trip row.
|
|
|
|
This also keeps the trips list fast: it reads only `Trip` rows, never the point table, so
|
|
it stays responsive with hundreds of rides.
|
|
|
|
### Ordering is by `(segmentId, id)`, never `timestamp`
|
|
|
|
`timestamp` comes from `Location.time`, which is GPS-derived and can jump. The autoincrement
|
|
`id` is genuinely monotonic in write order.
|
|
|
|
### Migrations are mandatory
|
|
|
|
`fallbackToDestructiveMigration()` was used during v2 development, when the only data was
|
|
throwaway test rides, and **removed in T18**. Any schema change from here must ship a
|
|
`Migration` against `app/schemas/com.rippr.data.AppDatabase/2.json`. Reinstating the
|
|
fallback would silently delete every stored ride on the next version bump.
|
|
|
|
---
|
|
|
|
## State ownership
|
|
|
|
Recording state is derived from the database, never held in memory.
|
|
|
|
v1 kept an in-memory flag. It lied after process death: `START_STICKY` restarts the service
|
|
with a null intent, and the flag came back `false` while a ride was genuinely underway.
|
|
This was verified fixed by force-stopping the app mid-ride — the button correctly still
|
|
read STOP.
|
|
|
|
`TripRepository` owns every transition, each in a `withTransaction`, and **each is
|
|
idempotent**:
|
|
|
|
| Transition | Behaviour on an unexpected state |
|
|
|---|---|
|
|
| `startTrip` | **Adopts** an already-active trip rather than creating a second |
|
|
| `pauseTrip` | No-op if already paused |
|
|
| `resumeTrip` | No-op if already recording — never opens a duplicate segment |
|
|
| `completeTrip` / `discardTrip` | No-op with no active trip |
|
|
|
|
Idempotency is not defensive padding: the OS can restart the service at any moment, and
|
|
these are the states it can restart into.
|
|
|
|
### Ephemeral state is separate and deliberate
|
|
|
|
Two objects hold genuinely transient state that *should* be lost on restart:
|
|
`UploadStatus` (last upload error) and `LiveTelemetry` (current speed, published from the
|
|
GPS callback at sensor rate so the speedo does not wait on the 2-second flush).
|
|
|
|
---
|
|
|
|
## Statistics
|
|
|
|
Computed twice, deliberately, by the same code:
|
|
|
|
- **Live**, folded per batch into `Accumulator`, so the screen shows moving numbers
|
|
- **Authoritatively**, via `RideStatistics.compute()` over stored points when a trip
|
|
completes, overwriting the live estimate
|
|
|
|
A mid-ride process kill loses the in-memory accumulator, so the live figure can drift.
|
|
Recomputing at completion with the *same functions* keeps the two consistent by
|
|
construction rather than by discipline.
|
|
|
|
### The cross-batch anchor
|
|
|
|
`Accumulator` keeps the last point of the *previous* batch. Without it, distance restarts
|
|
at every flush boundary and under-reports by roughly one hop per batch — a few percent,
|
|
invisible until compared against an odometer. A unit test asserts that chunked folding
|
|
equals single-batch folding.
|
|
|
|
### Elevation gain needs two mechanisms
|
|
|
|
This is the subtlest maths in the codebase, and it was measured, not assumed.
|
|
|
|
A naive "sum every delta above a 3 m threshold" reported **1498 m of climbing over a parked
|
|
bike** with ±8 m altitude noise, because noise crosses any small threshold constantly.
|
|
|
|
The shipped version combines:
|
|
1. A **15-sample moving average**, cutting noise by roughly √window
|
|
2. **Reversal hysteresis** — a climb banks only once altitude turns back *down* past the
|
|
threshold from its peak
|
|
|
|
That brings the same fixture to ~30 m. Smoothing then clipped real terrain (a 100 m climb
|
|
measured 93 m, since a moving average lags by half a window), so `finish()` reconciles the
|
|
final run against the last raw reading.
|
|
|
|
**~30 m of drift per ten stationary minutes remains.** The regression test guards against
|
|
returning to 1498 m; it is not a claim of accuracy.
|
|
|
|
### Other definitions worth knowing
|
|
|
|
- **Average speed is distance ÷ moving time**, not the mean of speed samples. The mean
|
|
over-weights time spent stopped.
|
|
- **Sample gaps are capped at 10 s.** Without it, a two-minute tunnel counts as two minutes
|
|
of moving time at the last known speed.
|
|
- **The speed histogram is weighted by time**, not sample count.
|
|
- **Division guards everywhere** — a NaN reaching Compose renders as the literal text "NaN".
|
|
|
|
---
|
|
|
|
## UI
|
|
|
|
`MainActivity` (74 lines) owns only runtime permissions and the battery-optimisation
|
|
prompt. Everything else is `ui/{theme,components,record,trips,detail}` with one ViewModel
|
|
per screen behind a single factory. No DI framework — three screens do not earn Hilt.
|
|
|
|
### Two hard-won UI lessons
|
|
|
|
**`Surface` is load-bearing, not decoration.** Moving the UI out of the old `Surface`
|
|
wrapper made `LocalContentColor` default to black, rendering the 64sp speed figure
|
|
invisible against the near-black ground. Text with an explicit colour still showed, so the
|
|
screen looked merely odd rather than broken. `RipprTheme` now wraps content in a `Surface`.
|
|
**No test would have caught this** — only looking at a screenshot did.
|
|
|
|
**Show current speed, not max.** The first real ride reported the screen as frozen. A max
|
|
figure only moves when you beat it, so riding steadily leaves it motionless. The headline
|
|
is now current speed, with max demoted to the stats card, and a wall-clock elapsed timer
|
|
ticks every second independent of any database write.
|
|
|
|
### Control layout
|
|
|
|
| State | Primary | Secondary |
|
|
|---|---|---|
|
|
| Idle | START RECORDING | — |
|
|
| Recording | PAUSE | STOP |
|
|
| Paused | RESUME | STOP · DISCARD |
|
|
|
|
**Discard appears only while paused.** A destructive control next to Pause during a live
|
|
ride invites a gloved mis-tap at speed. Buttons are 72dp/56dp because they get pressed with
|
|
gloves on.
|
|
|
|
---
|
|
|
|
## Map
|
|
|
|
osmdroid, chosen over MapLibre and Google Maps because it needs no API key, no billing
|
|
account, and no GCP project, and it caches tiles for offline use — which matters on
|
|
mountain rides.
|
|
|
|
Three things that will break it if disturbed:
|
|
|
|
1. **The user agent must be set before any `MapView` is constructed.** osmdroid's default is
|
|
rejected by OSM's tile servers with a 403, and the failure presents as an empty map
|
|
rather than an error. Set in `RipprApp.onCreate`.
|
|
2. **`onDetach()` is not optional.** Without it, tile handles and the downloader thread
|
|
outlive the composable and the leak compounds. The `MapView` is hoisted into a
|
|
`remember` so the lifecycle observer can reach the same instance the `AndroidView`
|
|
shows — an earlier version observed the lifecycle and did nothing, because it could not
|
|
see the view.
|
|
3. **Zoom must be clamped.** `zoomToBoundingBox` ignores `maxZoomLevel`. A 50 m ride zooms
|
|
past OSM Mapnik's maximum published zoom of 19, where no tile exists, and the map renders
|
|
as an empty grid. This shipped in v2.0 and was found on the first real ride.
|
|
|
|
Rendering uses **bucketed polylines** — runs of similar speed drawn as separate monochrome
|
|
lines, overlapping by one point — rather than osmdroid's per-vertex `PolyChromaticPaintList`,
|
|
which is fiddly and gains little at real viewing zoom.
|
|
|
|
**Decimation is render-only.** `Geo.simplify` is applied when building overlays and nowhere
|
|
else. Storage and export always use raw points; the export tests assert exact counts
|
|
specifically to catch a leak.
|
|
|
|
---
|
|
|
|
## Export
|
|
|
|
Pure string generation with no Android dependency, so the format logic is unit-tested on
|
|
the JVM.
|
|
|
|
- **GPX 1.1**, one `<trkseg>` per segment, speed in `<extensions>` in **m/s** per the spec
|
|
- **GeoJSON**, coordinates `[lon, lat, ele]` — **longitude first**, the opposite of GPX and
|
|
the classic silent error; a test asserts it explicitly
|
|
- **Timestamps inside files are UTC**; **filenames use local time**, because a human reads
|
|
the filename and a machine reads the contents
|
|
- Names are XML- and JSON-escaped — a ride called `Sam & Dave's <ride>` would otherwise
|
|
produce a malformed file, and the failure is invisible until an import rejects it
|
|
|
|
Delivery is via `FileProvider` and the share sheet. A raw `file://` URI throws
|
|
`FileUriExposedException` on Android 7+, and the receiving app needs
|
|
`FLAG_GRANT_READ_URI_PERMISSION` or it reports a SecurityException that looks like a bug in
|
|
*that* app.
|
|
|
|
---
|
|
|
|
## Upload
|
|
|
|
`TelemetryUploader` is deliberately subordinate to recording: every failure is swallowed and
|
|
retried, and nothing in it can stop the location pipeline. Points carry `synced = 0` until
|
|
the server acknowledges them, so a dead endpoint costs only a growing backlog.
|
|
|
|
`trip_id` and `segment_id` are stamped **per point, not per batch**, because
|
|
`getUnsyncedPoints()` draws by id and a batch can straddle a segment or — after a
|
|
discard-and-restart — a trip boundary.
|