Refresh Rippr snapshot and bundle with full project documentation
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>
This commit is contained in:
288
rippr-src/docs/ARCHITECTURE.md
Normal file
288
rippr-src/docs/ARCHITECTURE.md
Normal file
@@ -0,0 +1,288 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user