Add Rippr source snapshot and full-history bundle
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>
This commit is contained in:
76
rippr-src/docs/v2/01-baseline.md
Normal file
76
rippr-src/docs/v2/01-baseline.md
Normal file
@@ -0,0 +1,76 @@
|
||||
# T01 — Repo + dependency baseline
|
||||
|
||||
**Phase** 0 · **Depends on** — · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Put the project under version control and add every dependency v2 needs, verifying the
|
||||
build stays green before a single line of feature code changes. Done when `git log` shows
|
||||
v1 committed, the new libraries resolve, and `assembleDebug testDebugUnitTest lintDebug`
|
||||
all pass.
|
||||
|
||||
## Context
|
||||
|
||||
- `~/dojo/rippr` is **not currently a git repo**. A multi-phase rewrite with no version
|
||||
control and no way to diff or revert is reckless — this is the first thing to fix.
|
||||
- `.gitignore` already exists and covers `build/`, `.gradle/`, `local.properties`,
|
||||
`.idea/`, `.DS_Store`.
|
||||
- `gradle/libs.versions.toml` is the single source of dependency truth. Everything goes
|
||||
through the catalog; no inline coordinates in `app/build.gradle.kts`.
|
||||
- `app/build.gradle.kts` already carries a `constraints` block forcing
|
||||
`androidx.fragment:1.8.5` — play-services drags in 1.1.0 transitively and lint rejects
|
||||
it. Leave that alone.
|
||||
|
||||
## Design
|
||||
|
||||
Three new dependencies, all in the catalog:
|
||||
|
||||
| Library | Why |
|
||||
|---|---|
|
||||
| `org.osmdroid:osmdroid-android` | Map rendering (T13/T14). Apache-2.0, no API key |
|
||||
| `androidx.navigation:navigation-compose` | Three destinations (T08) |
|
||||
| `androidx.lifecycle:lifecycle-viewmodel-compose` | ViewModels (T08) |
|
||||
|
||||
osmdroid is added now rather than at T13 so any resolution or manifest-merge surprise
|
||||
surfaces against a known-green build instead of tangled up with map code.
|
||||
|
||||
`local.properties` stays gitignored — it holds a machine-specific `sdk.dir`.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. `git init` in `~/dojo/rippr`, confirm `.gitignore` covers build outputs, and verify
|
||||
`git status` shows no `build/` or `local.properties` noise.
|
||||
2. Commit v1 as the baseline — source, design generators, vendored Material glyphs, and
|
||||
the v2 docs.
|
||||
3. Add to `[versions]`: `osmdroid`, `navigation`, `lifecycleViewmodel`.
|
||||
4. Add to `[libraries]`: `osmdroid-android`, `androidx-navigation-compose`,
|
||||
`androidx-lifecycle-viewmodel-compose`.
|
||||
5. Wire the three into `app/build.gradle.kts` `dependencies`.
|
||||
6. Build and test.
|
||||
7. Commit the dependency bump separately, so a resolution problem is bisectable.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] `git log` shows at least two commits: v1 baseline, then dependency bump
|
||||
- [ ] `git status` clean — no build artefacts or `local.properties` tracked
|
||||
- [ ] All three libraries resolve
|
||||
- [ ] `./gradlew assembleDebug testDebugUnitTest lintDebug` passes
|
||||
- [ ] 12/12 existing unit tests still green
|
||||
|
||||
## Tests
|
||||
|
||||
No new tests. This task's job is proving the existing suite still passes with the new
|
||||
dependency graph — particularly that osmdroid does not introduce a manifest-merge
|
||||
conflict or a duplicate-class error.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **osmdroid manifest merge.** It historically declared storage permissions. Modern
|
||||
versions use app-private cache, but check the merged manifest afterward:
|
||||
`aapt2 dump xmltree --file AndroidManifest.xml app/build/outputs/apk/debug/app-debug.apk`
|
||||
and confirm no unexpected permission appeared.
|
||||
- **Do not commit the APK.** Release artefacts belong in `~/dojo/samplez`, not here.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Any use of the new libraries. This task only proves they resolve.
|
||||
128
rippr-src/docs/v2/02-schema.md
Normal file
128
rippr-src/docs/v2/02-schema.md
Normal file
@@ -0,0 +1,128 @@
|
||||
# T02 — Schema v2 entities + DAOs
|
||||
|
||||
**Phase** 1 · **Depends on** T01 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Replace the flat single-table schema with `Trip → Segment → TrackPoint`, split the
|
||||
monolithic `TrackPoint.kt` into a `data/` package, and add the queries the v2 screens
|
||||
need. Done when the new schema builds, exports to `app/schemas/com.rippr.data.AppDatabase/2.json`, and the
|
||||
instrumented DAO tests pass.
|
||||
|
||||
## Context
|
||||
|
||||
`app/src/main/java/com/rippr/TrackPoint.kt` (113 lines) currently holds the entity, the
|
||||
`RideStats` projection, the DAO, **and** the `AppDatabase` all in one file. That was fine
|
||||
for one table and is not fine for three.
|
||||
|
||||
Existing details that matter:
|
||||
|
||||
- `AppDatabase` uses `JournalMode.WRITE_AHEAD_LOGGING` — deliberate, keep it. WAL with
|
||||
NORMAL sync survives app crashes without fsyncing every 2 Hz write.
|
||||
- `exportSchema = true`, and `app/schemas/com.rippr.AppDatabase/1.json` exists.
|
||||
- `observeStats()` returns a single-row aggregate with `COALESCE` guards so an empty
|
||||
table does not null-crash the non-null `RideStats` fields. **That COALESCE lesson
|
||||
carries over** — the same trap applies to per-trip aggregates.
|
||||
- The `synced` column drives the upload backlog; it stays.
|
||||
|
||||
## Design
|
||||
|
||||
```kotlin
|
||||
enum class TripState { RECORDING, PAUSED, COMPLETED }
|
||||
|
||||
@Entity(tableName = "trips")
|
||||
data class Trip(
|
||||
@PrimaryKey(autoGenerate = true) val id: Long = 0,
|
||||
val startedAt: Long,
|
||||
val endedAt: Long? = null, // null ⇒ active
|
||||
val name: String? = null, // null ⇒ UI derives a name from startedAt
|
||||
val state: TripState = TripState.RECORDING,
|
||||
val distanceM: Double = 0.0,
|
||||
val movingMillis: Long = 0,
|
||||
val maxSpeedKmh: Float = 0f,
|
||||
val elevationGainM: Double = 0.0,
|
||||
val pointCount: Int = 0,
|
||||
)
|
||||
|
||||
@Entity(
|
||||
tableName = "segments",
|
||||
foreignKeys = [ForeignKey(Trip::class, ["id"], ["tripId"], onDelete = CASCADE)],
|
||||
indices = [Index("tripId")],
|
||||
)
|
||||
data class Segment(
|
||||
@PrimaryKey(autoGenerate = true) val id: Long = 0,
|
||||
val tripId: Long,
|
||||
val startedAt: Long,
|
||||
val endedAt: Long? = null, // null ⇒ open
|
||||
)
|
||||
```
|
||||
|
||||
`TrackPoint` gains `tripId` and `segmentId`, both with CASCADE foreign keys and indices.
|
||||
Deleting a trip therefore removes its segments and points in one statement — which is
|
||||
exactly what Discard (T06) and Delete (T12) need.
|
||||
|
||||
**Why `state` as well as `endedAt`:** `endedAt == null` distinguishes active from
|
||||
finished, but cannot distinguish RECORDING from PAUSED. Both are needed — the service
|
||||
resumes differently depending on which it finds after a process restart.
|
||||
|
||||
**Destructive migration.** v1 data is intentionally discarded, so bump to version 2 with
|
||||
`fallbackToDestructiveMigration()`. No `Migration` object, no `MigrationTestHelper`.
|
||||
|
||||
> ⚠ This is correct **only** while there is no ride data worth keeping. Once v2 is in
|
||||
> daily use, a future schema change would silently wipe real rides. **T18 removes it.**
|
||||
|
||||
### New queries
|
||||
|
||||
| Query | Used by |
|
||||
|---|---|
|
||||
| `observeActiveTrip(): Flow<Trip?>` — `WHERE endedAt IS NULL` | T03, T09 |
|
||||
| `observeCompletedTrips(): Flow<List<Trip>>` — ordered `startedAt DESC` | T10 |
|
||||
| `observeTrip(id): Flow<Trip?>` | T11 |
|
||||
| `pointsForTrip(id): List<TrackPoint>` — ordered `segmentId, id` | T11, T14, T15 |
|
||||
| `segmentsForTrip(id): List<Segment>` | T14, T15 |
|
||||
| `openSegment(tripId): Segment?` | T06 |
|
||||
| `deleteTrip(id)` — CASCADE handles the rest | T06, T12 |
|
||||
|
||||
Ordering by `segmentId, id` rather than `timestamp` matters: `timestamp` is GPS time and
|
||||
can jump, whereas the autoincrement id is genuinely monotonic in write order.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Create `data/` package; move and split the existing file.
|
||||
2. Add `Trip`, `Segment`, `TripState` (+ a Room `TypeConverter` for the enum, or store
|
||||
its `name` as `String`).
|
||||
3. Add `tripId`/`segmentId` to `TrackPoint` with FKs and indices.
|
||||
4. Split DAOs: `TripDao`, `SegmentDao`, `TrackPointDao`.
|
||||
5. Bump `AppDatabase` to `version = 2`, add all three entities, add
|
||||
`fallbackToDestructiveMigration()` with a comment pointing at T18.
|
||||
6. Verify CASCADE actually fires in a test rather than assuming — Room does enable
|
||||
foreign keys for its generated code, but that is worth proving, not trusting.
|
||||
7. Update `TrackPointDaoTest.kt` for the new shape; add trip/segment tests.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] `app/schemas/com.rippr.data.AppDatabase/2.json` generated (path follows the
|
||||
package, which moved to `com.rippr.data`; the orphaned v1 dir was removed)
|
||||
- [x] Deleting a trip cascades to its segments and points
|
||||
- [x] `observeActiveTrip()` emits null on an empty database, not a crash
|
||||
- [x] Per-trip aggregate queries return zeros, not nulls, for a trip with no points
|
||||
- [x] Existing `TelemetryTest` / `TelemetryUploaderTest` untouched and green
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented (`app/src/androidTest/…`):
|
||||
- CASCADE delete removes segments and points
|
||||
- `observeActiveTrip` emits on insert, and emits null once `endedAt` is set
|
||||
- Points ordered by `segmentId, id` across multiple segments
|
||||
- Empty-trip aggregates are zero, not null (the COALESCE trap from v1)
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **`fallbackToDestructiveMigration()` silently wipes.** Correct here, dangerous later.
|
||||
- **Foreign keys.** Room enables them for its generated code; the cascade test
|
||||
confirms this empirically rather than relying on it.
|
||||
- **The `synced` column must survive** the refactor or the upload backlog logic breaks.
|
||||
|
||||
## Out of scope
|
||||
|
||||
The repository layer (T03), any service changes (T06), any aggregate computation (T07).
|
||||
112
rippr-src/docs/v2/03-repository.md
Normal file
112
rippr-src/docs/v2/03-repository.md
Normal file
@@ -0,0 +1,112 @@
|
||||
# T03 — Trip repository + reactive state
|
||||
|
||||
**Phase** 1 · **Depends on** T02 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Own trip lifecycle transitions in one place and make recording state a function of the
|
||||
database rather than an in-memory flag. Done when `RecordingState` is deleted, the UI
|
||||
observes trip state through a Flow, and killing the app process no longer loses track of
|
||||
whether a ride is in progress.
|
||||
|
||||
## State as of T02
|
||||
|
||||
`TripDao`/`SegmentDao` already expose every query this task needs (`observeActive`, `getActive`, `openSegment`, `close`, `setState`, `deleteById`).
|
||||
|
||||
T02 left **temporary scaffolding** that this task and T06 replace:
|
||||
`TrackingService.openTripAndSegment()` opens a trip and segment inline via the DAOs, and
|
||||
`@Volatile currentTripId/currentSegmentId` are read by the location callback. The callback
|
||||
drops fixes while those are 0 so a point can never violate the FK — **preserve that guard**.
|
||||
|
||||
## Context
|
||||
|
||||
v1 keeps state in a process-wide singleton in
|
||||
`app/src/main/java/com/rippr/Telemetry.kt`:
|
||||
|
||||
```kotlin
|
||||
object RecordingState {
|
||||
private val _isRecording = MutableStateFlow(false)
|
||||
…
|
||||
}
|
||||
```
|
||||
|
||||
This was already an improvement over the Activity-local `isRecording` the original spec
|
||||
used, but it still lies after process death: `START_STICKY` restarts the service with a
|
||||
null intent and the flag resets to `false` while a ride is genuinely underway.
|
||||
|
||||
The fix is that the database already knows. A row in `trips` with `endedAt IS NULL` *is*
|
||||
the fact of an in-progress ride, and it survives anything short of uninstall.
|
||||
|
||||
**Keep the rest of `Telemetry.kt` exactly as it is.** `msToKmh`, `sanitizeSpeedKmh`,
|
||||
`isUsableFix`, `formatDuration`, and `encodeBatch` are pure, already unit-tested by
|
||||
`TelemetryTest`, and have no reason to change. Only `RecordingState` goes.
|
||||
|
||||
`RecordingState.lastUploadError` is also used by `TelemetryUploader` and surfaced on the
|
||||
Record screen — that one is genuinely ephemeral UI state and can stay as a small
|
||||
`UploadStatus` object, or move onto the repository. Do not silently drop it.
|
||||
|
||||
## Design
|
||||
|
||||
```kotlin
|
||||
class TripRepository(
|
||||
private val tripDao: TripDao,
|
||||
private val segmentDao: SegmentDao,
|
||||
private val pointDao: TrackPointDao,
|
||||
) {
|
||||
fun observeActiveTrip(): Flow<Trip?>
|
||||
fun observeTrip(id: Long): Flow<Trip?>
|
||||
fun observeCompletedTrips(): Flow<List<Trip>>
|
||||
|
||||
suspend fun startTrip(now: Long): Trip // creates trip + first segment
|
||||
suspend fun pauseTrip(now: Long) // closes open segment, state=PAUSED
|
||||
suspend fun resumeTrip(now: Long) // opens new segment, state=RECORDING
|
||||
suspend fun completeTrip(now: Long) // closes segment, endedAt, COMPLETED
|
||||
suspend fun discardTrip() // deletes active trip (CASCADE)
|
||||
}
|
||||
```
|
||||
|
||||
Every transition is idempotent and safe to call from an unexpected state — the service
|
||||
can be restarted by the OS at any moment, so `resumeTrip` on an already-recording trip
|
||||
must be a no-op rather than opening a duplicate segment.
|
||||
|
||||
**Single instance.** v1 uses a `@Volatile` double-checked singleton for `AppDatabase`;
|
||||
follow the same pattern for the repository rather than introducing a DI framework. The
|
||||
app is small and Hilt would be more ceremony than it earns.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Create `data/TripRepository.kt` with the interface above.
|
||||
2. Implement each transition as a single Room `@Transaction` so a crash mid-transition
|
||||
cannot leave a trip with two open segments.
|
||||
3. Guard each transition on current state; log and no-op on nonsensical transitions.
|
||||
4. Delete `RecordingState` from `Telemetry.kt`.
|
||||
5. Relocate upload-error state so the Record screen keeps its indicator.
|
||||
6. Update `TelemetryUploader` and `TrackingService` references.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] `RecordingState` no longer exists
|
||||
- [x] `observeActiveTrip()` reflects reality after a simulated process death
|
||||
- [x] Double `startTrip()` adopts rather than duplicating
|
||||
- [x] `resumeTrip()` while already RECORDING is a no-op
|
||||
- [x] Upload-error surfacing still works, now via `UploadStatus`
|
||||
- [x] `TelemetryTest` still green (the pure functions are untouched)
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented, against a real in-memory database:
|
||||
- start → pause → resume → complete produces exactly two segments
|
||||
- discard removes the trip and all its points
|
||||
- transitions from wrong states are no-ops
|
||||
- `observeActiveTrip` emits null after complete
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Transitions race the writer loop.** The service writes points continuously; pausing
|
||||
mid-flush must not orphan points into a closed segment. T06 handles ordering — pause
|
||||
drains the channel *before* closing the segment.
|
||||
- **Do not delete the pure Telemetry functions.** They are load-bearing and tested.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Wiring transitions to service actions (T06) and aggregate maths (T07).
|
||||
92
rippr-src/docs/v2/04-geo.md
Normal file
92
rippr-src/docs/v2/04-geo.md
Normal file
@@ -0,0 +1,92 @@
|
||||
# T04 — Geo utilities
|
||||
|
||||
**Phase** 2 · **Depends on** T01 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Pure geographic maths with no Android dependencies: distance between fixes, polyline
|
||||
simplification for rendering, and bounding boxes for map auto-fit. Done when
|
||||
`geo/Geo.kt` exists, imports nothing from `android.*`, and is covered by JVM unit tests
|
||||
that run without a device.
|
||||
|
||||
## Context
|
||||
|
||||
This is the pattern `Telemetry.kt` already establishes and the reason its logic is
|
||||
cheaply testable — the conversion and sanitisation functions there are covered by
|
||||
`TelemetryTest` with no emulator involved. `Geo` follows the same rule.
|
||||
|
||||
Three consumers depend on this task: T05 (statistics), T14 (path rendering), T15
|
||||
(export). It is on nobody's blocking path from the UI side, so it can be built in
|
||||
parallel with the whole data-layer track.
|
||||
|
||||
## Design
|
||||
|
||||
```kotlin
|
||||
object Geo {
|
||||
const val EARTH_RADIUS_M = 6_371_008.8 // IUGG mean radius
|
||||
|
||||
fun haversineMeters(lat1: Double, lon1: Double, lat2: Double, lon2: Double): Double
|
||||
|
||||
/** Douglas–Peucker. Always preserves first and last points. */
|
||||
fun simplify(points: List<LatLon>, epsilonMeters: Double): List<LatLon>
|
||||
|
||||
fun bounds(points: List<LatLon>): Bounds? // null for empty input
|
||||
}
|
||||
|
||||
data class LatLon(val lat: Double, val lon: Double)
|
||||
data class Bounds(val minLat: Double, val minLon: Double,
|
||||
val maxLat: Double, val maxLon: Double)
|
||||
```
|
||||
|
||||
**Haversine, not Vincenty.** Haversine assumes a sphere and is accurate to roughly 0.5%
|
||||
— a few metres per kilometre. For motorcycle ride distances that is far below GPS noise,
|
||||
and Vincenty's iterative solution would be false precision at real cost.
|
||||
|
||||
**Douglas–Peucker with a metre epsilon**, not a degree epsilon. A degree of longitude is
|
||||
~111 km at the equator and ~0 at the poles, so a degree-based tolerance behaves
|
||||
differently depending where you ride. Perpendicular distance is computed with a local
|
||||
equirectangular approximation — valid over the short spans between consecutive fixes and
|
||||
far cheaper than a full geodesic.
|
||||
|
||||
**Recursion depth.** A naive recursive Douglas–Peucker on 20,000+ points can blow the
|
||||
stack in a pathological case. Implement iteratively with an explicit work stack.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Create `geo/Geo.kt` with `LatLon`, `Bounds`, and the three functions.
|
||||
2. Haversine in double precision throughout — float loses metres over a long ride.
|
||||
3. Iterative Douglas–Peucker with an explicit stack.
|
||||
4. `bounds()` returns null on empty rather than a degenerate zero box, so callers must
|
||||
handle "no path" explicitly instead of silently centring on Null Island.
|
||||
5. Add an extension to map `TrackPoint` → `LatLon` so callers stay tidy.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] No `android.*` imports in `geo/`
|
||||
- [x] Haversine matches known reference distances within 0.5%
|
||||
- [x] `simplify()` always returns first and last points unchanged
|
||||
- [x] `simplify()` with epsilon 0 returns the input unchanged
|
||||
- [x] `simplify()` on 25,000 synthetic points completes quickly and does not stack-overflow
|
||||
- [x] `bounds()` returns null for an empty list
|
||||
|
||||
## Tests
|
||||
|
||||
JVM unit tests (`app/src/test/…`), no device needed:
|
||||
- Known pairs: Calgary→Edmonton ≈ 280.9 km (great-circle, not road); a 1° latitude step ≈ 111.2 km; identical points → 0
|
||||
- Antimeridian and equator crossings behave sanely
|
||||
- Douglas–Peucker: a straight line collapses to two points; a zigzag above epsilon is
|
||||
preserved; endpoints always survive
|
||||
- 25,000-point performance and stack-safety check
|
||||
- Bounds across a mixed-sign coordinate set
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Float vs double.** `TrackPoint.latitude/longitude` are already `Double`; keep them
|
||||
that way through every calculation. `speedKmh` is `Float`, which is fine — it is
|
||||
display data, not accumulated.
|
||||
- **Epsilon tuning is a T14 concern**, not this one. Expose it as a parameter and let the
|
||||
render layer choose.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Anything that consumes these functions — statistics (T05), rendering (T14), export (T15).
|
||||
126
rippr-src/docs/v2/05-stats.md
Normal file
126
rippr-src/docs/v2/05-stats.md
Normal file
@@ -0,0 +1,126 @@
|
||||
# T05 — Ride statistics
|
||||
|
||||
**Phase** 2 · **Depends on** T04 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Derive every number the trip detail screen shows from a list of points, as pure
|
||||
functions. Done when distance, moving time, elevation gain, speed histogram, and
|
||||
elevation profile are computed and unit-tested with no device.
|
||||
|
||||
## Context
|
||||
|
||||
v1 shows max speed, point count, and a naive `lastTimestamp - firstTimestamp` duration —
|
||||
computed in SQL by `TrackPointDao.observeStats()`. That query cannot survive into v2 for
|
||||
the derived values, because **minSdk 26 means SQLite 3.18, which has no window
|
||||
functions**, and distance/elevation both need consecutive-row differences.
|
||||
|
||||
So these move to Kotlin. T07 accumulates them live during recording; this task provides
|
||||
the authoritative batch computation used on trip completion and on the detail screen.
|
||||
|
||||
Reuse from `Telemetry.kt`:
|
||||
- `SPEED_NOISE_FLOOR_KMH` (1.5) — the moving/stopped threshold, so "moving" means the
|
||||
same thing here as it does to the recorder
|
||||
- `formatDuration` — already tested, used for display
|
||||
|
||||
## Available from T04
|
||||
|
||||
`com.rippr.geo.Geo` provides `haversineMeters` (both `Double` and `LatLon` overloads),
|
||||
`simplify`, `perpendicularDistanceMeters`, `bounds`, and `pathLengthMeters`. `LatLon`,
|
||||
`Bounds` (with `isDegenerate` for the stationary-ride case) and a `TrackPoint.toLatLon()`
|
||||
extension are there too. All Android-free and unit-tested.
|
||||
|
||||
Use `Geo.pathLengthMeters` per segment rather than summing across the whole ride — it has
|
||||
no notion of pause boundaries and will happily span them.
|
||||
|
||||
## Design
|
||||
|
||||
```kotlin
|
||||
object RideStatistics {
|
||||
fun compute(points: List<TrackPoint>, segments: List<Segment>): RideSummary
|
||||
}
|
||||
|
||||
data class RideSummary(
|
||||
val distanceM: Double,
|
||||
val elapsedMillis: Long, // wall clock, first fix to last
|
||||
val movingMillis: Long, // time above the speed noise floor
|
||||
val maxSpeedKmh: Float,
|
||||
val avgMovingSpeedKmh: Float, // distance / movingMillis, not the mean of samples
|
||||
val elevationGainM: Double,
|
||||
val elevationLossM: Double,
|
||||
val pointCount: Int,
|
||||
)
|
||||
```
|
||||
|
||||
**Distance never crosses a segment boundary.** Points either side of a pause may be
|
||||
kilometres apart; summing across the gap would invent distance the rider never covered.
|
||||
Accumulate per segment and total.
|
||||
|
||||
**Elevation gain needs two mechanisms, not one.** A threshold alone is not enough —
|
||||
measured, not assumed: a parked bike with ±8 m noise reported **1498 m** of climbing with
|
||||
simple thresholding, because noise crosses any small threshold constantly. The shipped
|
||||
implementation combines a 15-sample moving average (cuts noise by ~sqrt(window)) with
|
||||
**reversal** hysteresis (a climb banks only once altitude turns back down by more than
|
||||
3 m from its peak). That brings the same fixture to ~30 m.
|
||||
|
||||
The moving average lags the true altitude by about half a window, which clipped a 100 m
|
||||
climb to 93 m, so `finish()` reconciles the final run against the last raw reading.
|
||||
|
||||
**Average speed is distance ÷ moving time**, not the arithmetic mean of `speedKmh`
|
||||
samples. The mean of samples over-weights the time spent stopped and under-reports the
|
||||
real pace.
|
||||
|
||||
**Speed histogram and elevation profile** are series for T11's charts:
|
||||
|
||||
```kotlin
|
||||
fun speedHistogram(points: List<TrackPoint>, bucketKmh: Int = 10): List<Bucket>
|
||||
fun elevationProfile(points: List<TrackPoint>, maxSamples: Int = 200): List<ElevationSample>
|
||||
```
|
||||
|
||||
The profile is downsampled by distance-along-path, not by index, so a stretch where the
|
||||
bike sat idle at 2 Hz does not dominate the chart.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Create `stats/RideStatistics.kt`.
|
||||
2. `compute()` walks points grouped by `segmentId`, using `Geo.haversineMeters`.
|
||||
3. Moving time accumulates `dt` only where `speedKmh >= SPEED_NOISE_FLOOR_KMH`; cap any
|
||||
single `dt` (say 10 s) so a GPS dropout does not inject phantom moving time.
|
||||
4. Elevation gain/loss with the 3 m hysteresis state machine.
|
||||
5. Histogram and profile helpers.
|
||||
6. Unit tests throughout.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Distance excludes inter-segment gaps
|
||||
- [x] A stationary point cloud with ±8 m altitude noise yields ~0 m elevation gain
|
||||
- [x] Moving time excludes stopped periods
|
||||
- [x] Average speed uses moving time, not elapsed
|
||||
- [x] Empty input returns a zeroed summary, never a crash or NaN
|
||||
- [x] Single-point input returns zero distance, not NaN
|
||||
|
||||
## Tests
|
||||
|
||||
JVM unit tests:
|
||||
- Synthetic straight-line ride: distance matches Haversine within rounding
|
||||
- Two segments separated by a large jump: distance excludes the gap
|
||||
- Stationary noisy-altitude fixture: elevation gain ≈ 0 (the regression test that
|
||||
matters most)
|
||||
- A genuine 100 m climb registers ~100 m
|
||||
- Half-stopped ride: moving time ≈ half of elapsed
|
||||
- Empty and single-point inputs
|
||||
- `avgMovingSpeedKmh` on a known distance and duration
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **NaN propagation.** Division by zero moving time must return 0, not NaN — a NaN
|
||||
reaching Compose renders as literal "NaN" on screen.
|
||||
- **`dt` capping matters.** Without it, a two-minute tunnel dropout counts as two minutes
|
||||
of moving time at the last known speed.
|
||||
- **Keep this in sync with T07.** The live accumulator and this batch computation must
|
||||
agree, or the number changes when a ride finishes. T07 recomputes with *this* code on
|
||||
completion, which keeps them consistent by construction.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Charts and rendering (T11); live accumulation during recording (T07).
|
||||
146
rippr-src/docs/v2/06-service-lifecycle.md
Normal file
146
rippr-src/docs/v2/06-service-lifecycle.md
Normal file
@@ -0,0 +1,146 @@
|
||||
# T06 — Trip lifecycle in TrackingService
|
||||
|
||||
**Phase** 3 · **Depends on** T03 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Teach the foreground service the full ride lifecycle — start, pause, resume, stop,
|
||||
discard — with segments opening and closing correctly and the notification reflecting
|
||||
actual state. Done when a paused ride keeps its notification, stops consuming GPS, and
|
||||
resumes into a *new* segment.
|
||||
|
||||
## Context
|
||||
|
||||
`app/src/main/java/com/rippr/TrackingService.kt` (265 lines) already gets the hard parts
|
||||
right, and **none of this architecture should be disturbed**:
|
||||
|
||||
- Location callbacks `trySend` into an **unbounded `Channel`**; a single writer coroutine
|
||||
drains it in batches of 25 every 2 s. This is why slow disk I/O can never block the GPS
|
||||
thread, and why no fix is dropped under back pressure.
|
||||
- `onDestroy` drains whatever remains on a scope that *outlives* `serviceScope`, so the
|
||||
final points still land. Verified on the emulator: 81 points, ids 1..81 contiguous.
|
||||
- A `PARTIAL_WAKE_LOCK` keeps the writer running with the screen off — a foreground
|
||||
service alone does not guarantee this on all OEMs.
|
||||
- `startForegroundCompat()` runs before anything else, because Android allows roughly
|
||||
five seconds from `startForegroundService()` before killing the process.
|
||||
|
||||
What changes is lifecycle, not plumbing.
|
||||
|
||||
### State after T03
|
||||
|
||||
The repository is already wired in: the service holds `TripRepository`, start calls
|
||||
`trips.startTrip()`, and `stopRecording()` calls `trips.completeTrip()` on a survivor
|
||||
scope. `@Volatile currentTripId`/`currentSegmentId` remain (the location callback runs on
|
||||
another thread) and are now populated from the returned `TripHandle`.
|
||||
|
||||
Still to do here:
|
||||
- The five actions; today there is only start and `ACTION_STOP`
|
||||
- `drainChannel()` and drain-before-close ordering
|
||||
- Pause/resume, wake-lock release on pause, per-state notification
|
||||
- The null-intent restart path (`trips.activeTrip()` already provides what it needs)
|
||||
- The `tripId == 0L` guard in the location callback — **keep it**
|
||||
|
||||
**Note on force-stop.** A user-initiated `am force-stop` is more aggressive than an OS
|
||||
kill: Android will not honour START_STICKY afterwards. The database state stays correct
|
||||
and the UI reads it correctly, but recording does not auto-resume. The restart path here
|
||||
covers OS-initiated kills, which is the case that matters on a long ride.
|
||||
|
||||
## Design
|
||||
|
||||
### Actions
|
||||
|
||||
| Action | Behaviour |
|
||||
|---|---|
|
||||
| `ACTION_START` | `repo.startTrip()`, acquire wake lock, request location updates |
|
||||
| `ACTION_PAUSE` | **Drain channel first**, close segment, `removeLocationUpdates`, release wake lock, keep service alive |
|
||||
| `ACTION_RESUME` | `repo.resumeTrip()` (new segment), re-acquire wake lock, re-request updates |
|
||||
| `ACTION_STOP` | Drain, close segment, `completeTrip()`, recompute aggregates (T07), stop service |
|
||||
| `ACTION_DISCARD` | Drain and discard buffered points, `discardTrip()` (CASCADE), stop service |
|
||||
|
||||
**Correction to the original reasoning.** This doc previously claimed that closing a
|
||||
segment before draining would write points into the *wrong* segment. That is wrong:
|
||||
`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, whatever order the
|
||||
writes happen in.
|
||||
|
||||
Draining before closing still matters, just for different reasons: points should not sit
|
||||
unwritten while a ride idles at a pause, and stop/discard must not lose the tail. The
|
||||
order used is stop-the-source → drain → close.
|
||||
|
||||
### Pause keeps the service, drops the wake lock
|
||||
|
||||
The foreground service stays alive so the notification persists and resuming is instant.
|
||||
But with no location updates arriving there is nothing for the writer to do, so the wake
|
||||
lock is released — a paused ride at a long lunch stop should not hold the CPU awake.
|
||||
|
||||
### Notification states
|
||||
|
||||
| State | Title | Text | Actions |
|
||||
|---|---|---|---|
|
||||
| Recording | Rippr Active | Recording ride telemetry… | Pause, Stop |
|
||||
| Paused | Rippr Paused | Ride paused — not recording | Resume, Stop |
|
||||
|
||||
`NotificationCompat.Builder` is rebuilt and re-notified on transition; the channel and id
|
||||
(101) stay as they are. The small icon stays `R.drawable.ic_stat_rippr`.
|
||||
|
||||
### Restart after process death
|
||||
|
||||
`START_STICKY` redelivers a null intent. On a null intent the service asks the repository
|
||||
for the active trip:
|
||||
|
||||
- **RECORDING** → resume recording into a new segment (the gap is real; the phone was off)
|
||||
- **PAUSED** → restore the paused notification, do not request location
|
||||
- **none** → `stopSelf()`
|
||||
|
||||
This is only possible because T03 moved state into the database.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Replace the single `ACTION_STOP` constant with the five actions.
|
||||
2. Inject `TripRepository`; drop all `RecordingState` references.
|
||||
3. Add `drainChannel()` — a suspend function that empties the channel into the DB and
|
||||
returns once flushed; reuse it from pause, stop, discard, and `onDestroy`.
|
||||
4. Split wake lock acquire/release out of `onStartCommand` so pause/resume can call them.
|
||||
5. Rebuild the notification per state; add `buildNotification(state: TripState)`.
|
||||
6. Handle the null-intent restart path.
|
||||
7. `stopWithTask="false"` in the manifest already survives task swipe — keep it.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Start → pause → resume → stop produces exactly two segments
|
||||
- [x] No point is written into a closed segment
|
||||
- [x] Paused: notification persists, no location updates, wake lock released
|
||||
- [x] Discard deletes the trip and every one of its points
|
||||
- [x] Force-stop then restart with an active RECORDING trip resumes into a new segment
|
||||
- [x] Contiguous point ids across a pause (no gaps, no duplicates)
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented plus emulator end-to-end:
|
||||
- Feed fixes, pause, feed more, resume, feed more, stop → assert two segments with the
|
||||
right point counts either side
|
||||
- Assert no point has a `segmentId` whose segment closed before its timestamp
|
||||
- Discard leaves zero rows
|
||||
|
||||
Emulator harness (proven in v1):
|
||||
```
|
||||
adb shell am start-foreground-service -n com.rippr/.TrackingService # blocked: not exported
|
||||
```
|
||||
Drive through the UI instead — `adb shell input tap`, after waiting for the UI to
|
||||
actually exist. A fixed `sleep` is not enough; the activity took 8.7 s to display on a
|
||||
cold start during v1 testing and a premature tap silently does nothing.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Do not replace the Channel with a direct write.** It is the reason the GPS callback
|
||||
never blocks.
|
||||
- **Drain ordering** as described above; get this wrong and points land in the wrong
|
||||
segment, which only shows up later as a rendering artefact.
|
||||
- **Wake lock double-release** throws. `setReferenceCounted(false)` is already set; still
|
||||
guard with `isHeld`.
|
||||
- **`onDestroy` already spawns a survivor scope** for the final flush — do not remove it
|
||||
while refactoring.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Aggregate maths (T07) and any UI (T09).
|
||||
113
rippr-src/docs/v2/07-aggregates.md
Normal file
113
rippr-src/docs/v2/07-aggregates.md
Normal file
@@ -0,0 +1,113 @@
|
||||
# T07 — Live aggregate accumulation
|
||||
|
||||
**Phase** 3 · **Depends on** T04, T06 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Keep distance, moving time, elevation gain, and max speed current on the active `Trip`
|
||||
row while recording, so the Record screen shows live numbers without rescanning the
|
||||
point table. Done when those values update during a ride and are reconciled exactly on
|
||||
completion.
|
||||
|
||||
## Context
|
||||
|
||||
v1's `observeStats()` recomputes aggregates in SQL on every emission. That worked for
|
||||
`MAX`, `AVG`, and `COUNT` over one table, but v2 needs **distance** and **elevation
|
||||
gain**, which require consecutive-row differences.
|
||||
|
||||
**SQLite 3.18 on minSdk 26 has no window functions** — no `LAG`, no `OVER`. There is no
|
||||
SQL expression for "distance from the previous point". So the maths moves into Kotlin,
|
||||
into the writer loop that is already touching every point as it lands.
|
||||
|
||||
`TrackingService` already batches writes every ~2 s (`FLUSH_INTERVAL_MS = 2000`,
|
||||
`FLUSH_SIZE = 25`), so one extra `UPDATE trips …` per flush is negligible — roughly one
|
||||
write every two seconds against a table with a single active row.
|
||||
|
||||
## Design
|
||||
|
||||
The writer loop keeps the **last point of the previous batch** in memory as the anchor
|
||||
for cross-batch distance. Without it, distance resets at every flush boundary and
|
||||
under-reports by roughly one inter-point hop per 25 points.
|
||||
|
||||
**Reuse `ElevationAccumulator` from T05 verbatim.** It is already streaming (fed one
|
||||
altitude at a time) precisely so the recorder can share it. Do **not** reimplement the
|
||||
hysteresis here — a naive version measured 1498 m of phantom climbing on a parked bike.
|
||||
Remember to call `finish()` before persisting final values.
|
||||
|
||||
Implemented as `com.rippr.Accumulator` in `RideAccumulator.kt` rather than a private
|
||||
class inside the service, so it is unit-testable without a device.
|
||||
|
||||
```kotlin
|
||||
private data class Accumulator(
|
||||
var lastPoint: TrackPoint? = null,
|
||||
var distanceM: Double = 0.0,
|
||||
var movingMillis: Long = 0,
|
||||
var maxSpeedKmh: Float = 0f,
|
||||
var elevationGainM: Double = 0.0,
|
||||
var climbBuffer: Double = 0.0, // hysteresis state, see T05
|
||||
var pointCount: Int = 0,
|
||||
)
|
||||
```
|
||||
|
||||
Rules, matching T05 exactly:
|
||||
- Distance accumulates only **within** a segment. On resume, `lastPoint` resets to null
|
||||
so the pause gap contributes nothing.
|
||||
- Moving time accumulates `dt` only where `speedKmh >= Telemetry.SPEED_NOISE_FLOOR_KMH`,
|
||||
with `dt` capped (10 s) so a tunnel dropout cannot inject phantom movement.
|
||||
- Elevation gain uses the same 3 m hysteresis.
|
||||
|
||||
### Reconciliation on completion
|
||||
|
||||
Incremental accumulation can drift — a process kill mid-ride loses the in-memory
|
||||
accumulator, and the restarted service resumes from zero while the DB row holds a stale
|
||||
partial. So on `ACTION_STOP`, after the final drain, **recompute authoritatively** with
|
||||
`RideStatistics.compute()` over all stored points and overwrite the row.
|
||||
|
||||
That is why T05 and T07 must agree: the live number is an estimate, the finished number
|
||||
is computed by T05's code. Using the same functions for both keeps them consistent by
|
||||
construction rather than by discipline.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Add `Accumulator` to `TrackingService`, owned by the writer coroutine.
|
||||
2. After each batch insert, fold the batch into the accumulator and `UPDATE` the trip row
|
||||
in the same transaction as the point insert — so a crash cannot commit points without
|
||||
their aggregate contribution.
|
||||
3. Reset `lastPoint` on segment change (pause/resume).
|
||||
4. On restart-with-active-trip, seed the accumulator from the persisted trip row and set
|
||||
`lastPoint` to the last stored point of the open segment.
|
||||
5. On stop, recompute with `RideStatistics.compute()` and overwrite.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Distance updates live during recording
|
||||
- [x] Distance does not jump across a pause
|
||||
- [x] Distance is correct across flush boundaries (no per-batch reset)
|
||||
- [x] Stationary noisy-altitude recording yields ~0 m elevation gain
|
||||
- [x] Post-stop values exactly match `RideStatistics.compute()` over the stored points
|
||||
- [x] Restart mid-ride resumes accumulation instead of restarting from zero
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented:
|
||||
- Insert 100 synthetic points across four flushes; assert accumulated distance equals the
|
||||
batch computation over the same points (the cross-batch anchor regression test)
|
||||
- Pause/resume with a 5 km jump between segments; assert the gap is excluded
|
||||
- Simulate restart: seed accumulator from DB, continue, assert continuity
|
||||
|
||||
Emulator: feed a synthetic ride and compare the live displayed distance against the
|
||||
post-stop recomputation — they should differ by nothing.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **The cross-batch anchor is the subtle bug here.** Losing `lastPoint` between flushes
|
||||
silently under-reports distance by a few percent, which is exactly the kind of error
|
||||
nobody notices until they compare against a bike odometer.
|
||||
- **Same transaction as the insert.** Points and their aggregate contribution must commit
|
||||
atomically or a crash leaves them disagreeing.
|
||||
- **Do not let the accumulator's definitions drift from T05.** Share constants; do not
|
||||
copy magic numbers.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Displaying any of this (T09, T11).
|
||||
98
rippr-src/docs/v2/08-nav-shell.md
Normal file
98
rippr-src/docs/v2/08-nav-shell.md
Normal file
@@ -0,0 +1,98 @@
|
||||
# T08 — Navigation shell + ViewModels
|
||||
|
||||
**Phase** 4 · **Depends on** T01 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Three navigable destinations, theme extracted out of the Activity, and state moved into
|
||||
ViewModels. Done when Record, Trips, and Trip Detail are reachable with a working back
|
||||
stack and `MainActivity` no longer owns screen state.
|
||||
|
||||
## Context
|
||||
|
||||
`app/src/main/java/com/rippr/MainActivity.kt` (216 lines) currently does everything:
|
||||
permission launcher, service intents, theme definition, screen composition, and state.
|
||||
That was proportionate for one screen and will not survive three.
|
||||
|
||||
Existing pieces to preserve rather than rewrite:
|
||||
- The permission flow (`RequestMultiplePermissions`, with `POST_NOTIFICATIONS` added only
|
||||
on TIRAMISU+) works and should move largely as-is.
|
||||
- The battery-optimisation exemption prompt is genuinely useful for an app that must
|
||||
survive a multi-hour ride; keep it on the Record screen.
|
||||
- `RipprTheme` is currently a private composable applying `darkColorScheme()`. Dylan
|
||||
called the UI "simple and clean" and explicitly likes it — **do not redesign it**.
|
||||
Extract, do not restyle.
|
||||
- `StatRow` and `BigStat` are reusable; move them into a shared `ui/components/`.
|
||||
|
||||
## Design
|
||||
|
||||
```
|
||||
ui/
|
||||
theme/ Theme.kt, Color.kt
|
||||
components/ StatRow.kt, BigStat.kt, ConfirmDialog.kt
|
||||
record/ RecordScreen.kt, RecordViewModel.kt
|
||||
trips/ TripsScreen.kt, TripsViewModel.kt
|
||||
detail/ TripDetailScreen.kt, TripDetailViewModel.kt
|
||||
RipprNavHost.kt
|
||||
```
|
||||
|
||||
Routes:
|
||||
|
||||
| Route | Screen |
|
||||
|---|---|
|
||||
| `record` | Start destination |
|
||||
| `trips` | Trip list |
|
||||
| `trip/{tripId}` | Detail, `tripId: Long` argument |
|
||||
|
||||
`navigation-compose` rather than hand-rolled state switching: the back stack from detail
|
||||
→ list is worth not writing by hand, and it gives correct behaviour on the system back
|
||||
gesture for free.
|
||||
|
||||
**ViewModels** get the repository through a simple factory — the app already uses a
|
||||
`@Volatile` singleton for `AppDatabase` and T03 adds one for `TripRepository`. Hilt would
|
||||
be more ceremony than a three-screen app earns.
|
||||
|
||||
**Navigation from Record → Trips** goes in a top bar action, keeping the Record screen's
|
||||
centre free for the numeric readout.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Extract `ui/theme/` from `MainActivity`.
|
||||
2. Move `StatRow` / `BigStat` into `ui/components/`, unchanged.
|
||||
3. Create `RipprNavHost` with the three routes.
|
||||
4. Add a ViewModel per screen with a shared factory.
|
||||
5. Reduce `MainActivity` to: permissions, battery exemption, `setContent { RipprTheme { RipprNavHost() } }`.
|
||||
6. Move service-intent dispatch into `RecordViewModel`.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] All three destinations reachable; system back works from detail
|
||||
- [x] `MainActivity` holds no screen state
|
||||
- [x] Visual appearance of the Record screen is unchanged
|
||||
- [x] Permission flow and battery-optimisation prompt still function
|
||||
- [ ] Config change (rotation) does not lose state
|
||||
|
||||
> **Not verified.** The unchecked items above, and the Compose UI tests below, were
|
||||
> not done. Verification for T08–T11 was manual on the emulator (screenshots through
|
||||
> the full flow) plus the existing unit and instrumented suites. Compose UI tests are
|
||||
> outstanding — tracked in T18.
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented Compose UI tests:
|
||||
- Navigate record → trips → detail → back → back
|
||||
- Rotation preserves screen state
|
||||
- Existing suites stay green
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Do not restyle.** The brief was "simple and clean, I like that". This task is
|
||||
structural only.
|
||||
- **`tripId` argument type.** Room ids are `Long`; declare `NavType.LongType` explicitly
|
||||
or it silently arrives as a String.
|
||||
- The v1 UI reads state via `collectAsStateWithLifecycle` already — keep that, it is the
|
||||
correct choice over `collectAsState` for lifecycle-aware collection.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Screen content beyond wiring (T09, T10, T11); any map (T13).
|
||||
114
rippr-src/docs/v2/09-record-screen.md
Normal file
114
rippr-src/docs/v2/09-record-screen.md
Normal file
@@ -0,0 +1,114 @@
|
||||
# T09 — Record screen v2
|
||||
|
||||
**Phase** 4 · **Depends on** T06, T08 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Add Pause/Resume, Stop, and Discard to the recording screen while keeping the clean
|
||||
numeric readout intact, and show per-trip rather than lifetime stats. Done when a ride
|
||||
can be fully controlled from the screen and the numbers reflect the current trip only.
|
||||
|
||||
## Context
|
||||
|
||||
Direct feedback: *"app is simple and clean. I like that but it's missing stuff."* The
|
||||
missing stuff is controls, not decoration. **Resist adding visual weight.**
|
||||
|
||||
v1's screen shows `MAX SPEED` as a large monospace figure, then a card with points
|
||||
captured, avg speed, duration, and pending upload, then one full-width button. That
|
||||
layout stays; it gains controls and loses its lifetime scope.
|
||||
|
||||
The current single button toggles start/stop via `RecordingState.isRecording`. v2 has
|
||||
three states and needs a different control arrangement.
|
||||
|
||||
## Design
|
||||
|
||||
### Control layout by state
|
||||
|
||||
| State | Primary | Secondary |
|
||||
|---|---|---|
|
||||
| Idle | **START RECORDING** (full width) | — |
|
||||
|
||||
Service actions to dispatch: `TrackingService.ACTION_START` / `ACTION_PAUSE` /
|
||||
`ACTION_RESUME` / `ACTION_STOP` / `ACTION_DISCARD`. Only START uses
|
||||
`startForegroundService`; the rest are plain `startService` on the running service.
|
||||
| Recording | **PAUSE** | STOP |
|
||||
| Paused | **RESUME** | STOP · DISCARD |
|
||||
|
||||
Discard appears **only when paused** — deliberately. Offering a destructive action next
|
||||
to Pause during an active ride invites a gloved mis-tap at 100 km/h. Pausing first is a
|
||||
natural speed bump.
|
||||
|
||||
Discard shows a confirmation dialog stating what is lost ("Delete this ride? N points
|
||||
recorded over X km will be permanently deleted."). Stop and Pause need no confirmation.
|
||||
|
||||
### Replacing T02 scaffolding
|
||||
|
||||
`MainActivity` derives stats with `flatMapLatest` over `TripRepository.observeActiveTrip()`
|
||||
into `observeTripStats(tripId)`, and `isRecording` from `observeActiveTrip().map { it != null }`.
|
||||
Upload errors come from the `UploadStatus` object. All of that moves into `RecordViewModel`,
|
||||
and once T07 lands the screen reads persisted aggregates off the `Trip` row rather than
|
||||
recomputing in SQL.
|
||||
|
||||
`onToggleClicked` is now a `lifecycleScope.launch` that queries `trips.activeTrip()` — there
|
||||
is no synchronous recording flag any more. The ViewModel should expose state instead so the
|
||||
screen never has to suspend to decide what a button does.
|
||||
|
||||
### Stats become per-trip
|
||||
|
||||
Driven by `observeActiveTrip()` from T03, showing the live aggregates from T07:
|
||||
|
||||
- Max speed (the big figure — it is the number Dylan cares about)
|
||||
- Distance — **new in v2**, read straight off `Trip.distanceM` (no SQL recomputation)
|
||||
- Duration, moving time
|
||||
- Points captured
|
||||
- Pending upload (only when non-zero, and only if an endpoint is configured)
|
||||
|
||||
When idle, the screen shows a resting state rather than stale numbers from the last ride.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. `RecordViewModel` exposes `RecordUiState` derived from `observeActiveTrip()`.
|
||||
2. Map `TripState` to the control layout above.
|
||||
3. Dispatch service intents for each action.
|
||||
4. Add `ConfirmDialog` for Discard.
|
||||
5. Reuse `Telemetry.formatDuration` for both duration fields.
|
||||
6. Keep the battery-optimisation prompt.
|
||||
7. Add a top-bar action to reach the Trips list.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] All five transitions work from the UI
|
||||
- [x] Discard is unavailable while actively recording
|
||||
- [x] Discard confirms, and states what will be lost
|
||||
- [ ] Stats reset between trips — no bleed from the previous ride
|
||||
- [x] Distance appears and increases during a ride
|
||||
- [x] Idle state is visibly distinct from a zeroed ride
|
||||
- [ ] Screen still reads at a glance; no added clutter
|
||||
|
||||
> **Not verified.** The unchecked items above, and the Compose UI tests below, were
|
||||
> not done. Verification for T08–T11 was manual on the emulator (screenshots through
|
||||
> the full flow) plus the existing unit and instrumented suites. Compose UI tests are
|
||||
> outstanding — tracked in T18.
|
||||
|
||||
## Tests
|
||||
|
||||
Compose UI tests:
|
||||
- Each state renders the right controls
|
||||
- Discard dialog appears, cancel is non-destructive
|
||||
- State survives rotation mid-ride
|
||||
|
||||
Emulator end-to-end: drive the full lifecycle through taps, then verify the database
|
||||
matches what the screen claimed.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Tap targets.** These get used in gloves. Keep the 64–72dp button height v1 uses.
|
||||
- **Wait for the UI before tapping in tests.** v1's cold start took 8.7 s; a fixed sleep
|
||||
produced a silently-missed tap and a false "service didn't start" conclusion. Poll
|
||||
`uiautomator dump` for the button text instead.
|
||||
- **Do not show "Pending upload" when no endpoint is configured** — it reads as an error
|
||||
when it is just an unused feature.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Trips list (T10), any map, live path preview.
|
||||
89
rippr-src/docs/v2/10-trips-list.md
Normal file
89
rippr-src/docs/v2/10-trips-list.md
Normal file
@@ -0,0 +1,89 @@
|
||||
# T10 — Trips list screen
|
||||
|
||||
**Phase** 4 · **Depends on** T02, T08 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
A scrollable list of completed rides, each showing enough to identify it, tapping through
|
||||
to detail. Done when past rides are browsable and the list updates reactively as rides
|
||||
complete.
|
||||
|
||||
## Context
|
||||
|
||||
There is currently no way to see a past ride at all — v1 has one screen showing lifetime
|
||||
totals. This is the first place trips become visible as objects.
|
||||
|
||||
`TripDao.observeCompletedTrips()` from T02 provides the data, ordered `startedAt DESC`.
|
||||
All displayed values are already persisted on the `Trip` row by T07, so **this screen
|
||||
runs no computation** — no loading points, no statistics. That keeps it fast with
|
||||
hundreds of rides.
|
||||
|
||||
## Design
|
||||
|
||||
One row per trip:
|
||||
|
||||
```
|
||||
Sat 9 Aug · 14:32 47.2 km
|
||||
1h 12m moving · max 118 km/h ▸
|
||||
```
|
||||
|
||||
- Name if set, otherwise a date-derived label ("Sat 9 Aug · 14:32")
|
||||
- Distance as the right-aligned prominent figure
|
||||
- Moving time and max speed as the secondary line
|
||||
- Monospace with `tabular-nums` for the figures so columns align down the list
|
||||
|
||||
**Empty state** matters here — a new install has no trips, and an empty screen with no
|
||||
explanation reads as broken. Show a short line pointing at the Record tab.
|
||||
|
||||
**An in-progress trip is excluded** (`endedAt IS NULL`). A half-finished ride in the
|
||||
history list with a growing distance would be confusing; the Record screen owns that.
|
||||
|
||||
Grouping by month is tempting but premature. Revisit when there are enough rides to
|
||||
justify it.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. `TripsViewModel` exposing `observeCompletedTrips()` as state.
|
||||
2. `LazyColumn` of `TripRow`.
|
||||
3. Date formatting via `java.time` with the device locale and zone — **not** hardcoded
|
||||
patterns. Ride timestamps come from `Location.time`, which is UTC epoch millis.
|
||||
4. Reuse `Telemetry.formatDuration`.
|
||||
5. Distance formatting helper — km with one decimal; metres below 1 km.
|
||||
6. Tap navigates to `trip/{tripId}`.
|
||||
7. Empty state.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Completed trips listed newest first
|
||||
- [x] Active trip excluded
|
||||
- [x] Empty state shown on a fresh install
|
||||
- [x] Tap opens the right trip
|
||||
- [ ] List updates when a ride completes, without manual refresh
|
||||
- [x] No point-table reads — the screen uses only `Trip` rows
|
||||
|
||||
> **Not verified.** The unchecked items above, and the Compose UI tests below, were
|
||||
> not done. Verification for T08–T11 was manual on the emulator (screenshots through
|
||||
> the full flow) plus the existing unit and instrumented suites. Compose UI tests are
|
||||
> outstanding — tracked in T18.
|
||||
|
||||
## Tests
|
||||
|
||||
Compose UI tests:
|
||||
- Empty state on no data
|
||||
- Rows render in the right order
|
||||
- Tap emits the right navigation event
|
||||
- Active trip absent from the list
|
||||
|
||||
Instrumented DAO test for ordering and the `endedAt IS NULL` exclusion.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Timezone.** `Location.time` is UTC. Format in the device zone or a ride at 21:00
|
||||
local shows as tomorrow.
|
||||
- **Do not compute stats here.** If a value is missing from the `Trip` row, fix T07
|
||||
rather than loading points in the list — that path leads to a screen that janks after
|
||||
fifty rides.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Rename/delete/merge (T12); detail content (T11).
|
||||
86
rippr-src/docs/v2/11-trip-detail.md
Normal file
86
rippr-src/docs/v2/11-trip-detail.md
Normal file
@@ -0,0 +1,86 @@
|
||||
# T11 — Trip detail screen (stats only)
|
||||
|
||||
**Phase** 4 · **Depends on** T05, T08 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
A full breakdown of a single ride — summary figures, elevation profile, speed
|
||||
distribution — with a deliberate placeholder where the map will go. Done when every
|
||||
statistic renders correctly and the screen is verifiable before osmdroid lands.
|
||||
|
||||
## Context
|
||||
|
||||
Splitting this from the map is intentional. osmdroid brings tile caching, `MapView`
|
||||
lifecycle, and coordinate projection — three sources of subtle failure. Building the
|
||||
screen first means that when the map misbehaves in T13, the surrounding screen is already
|
||||
known-good and the bug has nowhere to hide.
|
||||
|
||||
Data comes from `RideStatistics.compute()` (T05) over `pointsForTrip(id)` and
|
||||
`segmentsForTrip(id)`. Unlike the list screen, this one **does** load points — a few
|
||||
thousand rows for one trip is fine, and the derived series need them.
|
||||
|
||||
## Design
|
||||
|
||||
Sections top to bottom:
|
||||
|
||||
1. **Header** — name (inline-editable in T12) and start date/time
|
||||
2. **Map placeholder** — a bordered box captioned "Map coming in T13", replaced wholesale
|
||||
by T13/T14. Reserving the space now means the layout does not shift later.
|
||||
3. **Summary grid** — distance, moving time, elapsed time, max speed, average moving
|
||||
speed, elevation gain
|
||||
4. **Elevation profile** — line chart over distance-along-path
|
||||
5. **Speed distribution** — histogram in 10 km/h buckets
|
||||
6. **Segments** — listed only when there is more than one, showing where the ride paused
|
||||
|
||||
Charts are drawn with **Compose `Canvas`**, not a charting library. Two simple series do
|
||||
not justify a dependency, and hand-drawn keeps them consistent with the existing visual
|
||||
language.
|
||||
|
||||
Both charts need: an explicit empty/insufficient-data state, axis labels with units, and
|
||||
`tabular-nums` on any numeric label.
|
||||
|
||||
**Point loading happens off the main thread** in the ViewModel, with a loading state. A
|
||||
three-hour ride is ~21,600 rows; loading that synchronously would jank the transition
|
||||
into the screen.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. `TripDetailViewModel` loading trip, points, and segments on `Dispatchers.IO`, exposing
|
||||
`Loading | Ready(summary, profile, histogram, segments) | NotFound`.
|
||||
2. Summary grid from `RideSummary`.
|
||||
3. `ElevationChart` composable — `Canvas`, path stroke, min/max labels.
|
||||
4. `SpeedHistogram` composable — `Canvas`, bars, bucket labels.
|
||||
5. Segment list, shown conditionally.
|
||||
6. Map placeholder box.
|
||||
7. `NotFound` state for a deleted trip.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] All summary figures match `RideStatistics.compute()`
|
||||
- [ ] Charts render for a real ride and degrade gracefully with <2 points
|
||||
- [x] Segments section appears only when the ride was paused
|
||||
- [x] Loading state shown while points load; no main-thread I/O
|
||||
- [ ] Deleted trip shows `NotFound` rather than crashing
|
||||
- [x] Map placeholder occupies the space the real map will take
|
||||
|
||||
> **Not verified.** The unchecked items above, and the Compose UI tests below, were
|
||||
> not done. Verification for T08–T11 was manual on the emulator (screenshots through
|
||||
> the full flow) plus the existing unit and instrumented suites. Compose UI tests are
|
||||
> outstanding — tracked in T18.
|
||||
|
||||
## Tests
|
||||
|
||||
Compose UI tests: loading, ready, not-found, single-segment vs multi-segment.
|
||||
Unit tests for the chart data-preparation helpers (scaling, bucketing) — the drawing
|
||||
itself is verified by screenshot on the emulator.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Main-thread point loading is the obvious trap here.** Keep it in the ViewModel on IO.
|
||||
- **Chart division by zero** when all altitudes or speeds are identical — a flat ride
|
||||
gives a zero-height range. Guard the scaling.
|
||||
- **Do not start integrating the map in this task.** The separation is the point.
|
||||
|
||||
## Out of scope
|
||||
|
||||
The map (T13, T14), rename/delete/merge (T12), export (T16).
|
||||
109
rippr-src/docs/v2/12-trip-management.md
Normal file
109
rippr-src/docs/v2/12-trip-management.md
Normal file
@@ -0,0 +1,109 @@
|
||||
# T12 — Trip rename / delete / merge
|
||||
|
||||
**Phase** 4 · **Depends on** T10, T11 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Let rides be renamed, deleted, and merged. Done when a ride can be given a meaningful
|
||||
name, junk test rides can be removed, and two trips can be combined into one with correct
|
||||
recomputed aggregates.
|
||||
|
||||
## Context
|
||||
|
||||
Merge exists as a **safety valve**. It was originally promised to rescue a ride wrongly
|
||||
split by the migration heuristic; that heuristic is gone now that v1 data is being
|
||||
discarded, but merge keeps earning its place:
|
||||
|
||||
- A ride accidentally stopped and restarted mid-route
|
||||
- Test rides that should be one logical trip
|
||||
- Any future automatic splitting (v3+) needs an undo
|
||||
|
||||
Delete is the mundane but most-used of the three — a session of testing leaves a dozen
|
||||
30-second trips cluttering the list.
|
||||
|
||||
`Trip.name` is already nullable in the T02 schema, with the UI deriving a date label when
|
||||
it is null. CASCADE foreign keys mean delete is a single statement.
|
||||
|
||||
## State after T08–T11
|
||||
|
||||
`TripDetailViewModel` already has `rename(name)` and `delete(onDeleted)`;
|
||||
`TripsViewModel` has `delete(tripId)`. The repository has `renameTrip` (which already
|
||||
collapses blank to null) and `deleteTrip`. What is missing is the **merge** operation and
|
||||
all of the UI: selection mode on the list, the rename affordance on detail, and the
|
||||
confirmation dialogs. `ConfirmDialog` exists in `ui/components/Stats.kt`.
|
||||
|
||||
## Design
|
||||
|
||||
### Rename
|
||||
|
||||
Inline edit on the detail header. Empty input clears back to null (date-derived label)
|
||||
rather than storing an empty string — those two states must not diverge.
|
||||
|
||||
### Delete
|
||||
|
||||
From both the list (swipe or overflow) and detail. Confirmation stating what is lost.
|
||||
CASCADE removes segments and points. After deleting from detail, navigate back.
|
||||
|
||||
### Merge
|
||||
|
||||
Selected from the trips list: long-press to enter selection mode, pick exactly two, then
|
||||
Merge.
|
||||
|
||||
Semantics:
|
||||
- The **earlier** trip (by `startedAt`) is the survivor
|
||||
- The later trip's segments are re-parented to the survivor
|
||||
- **Segments are never joined** — the boundary between the two rides becomes a segment
|
||||
boundary, exactly like a pause. This is correct: the rider genuinely was not recording
|
||||
in between, and joining them would draw a false straight line across the gap
|
||||
- `endedAt` becomes the later trip's `endedAt`
|
||||
- The now-empty later trip row is deleted
|
||||
- Aggregates are **recomputed from scratch** with `RideStatistics.compute()`, not summed
|
||||
- The survivor keeps its name if set; otherwise inherits the later trip's
|
||||
|
||||
Merge runs in a single `@Transaction` — a partial merge would leave orphaned segments.
|
||||
|
||||
Merging is not offered while either trip is active.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. `TripRepository.renameTrip(id, name: String?)`
|
||||
2. `TripRepository.deleteTrip(id)` — CASCADE
|
||||
3. `TripRepository.mergeTrips(survivorId, absorbedId)` in one transaction:
|
||||
re-parent segments → update `endedAt` → delete absorbed row → recompute aggregates
|
||||
4. Selection mode in `TripsScreen`, enabled at exactly two selections
|
||||
5. Confirmation dialogs for delete and merge; merge states the resulting distance
|
||||
6. Inline rename on the detail header
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Rename persists; clearing reverts to the date label
|
||||
- [x] Delete removes the trip and every point (verified by row count)
|
||||
- [x] Merge re-parents all segments and deletes the absorbed row
|
||||
- [x] Merged aggregates equal a fresh computation over the combined points
|
||||
- [x] Merged trip shows a segment boundary at the join, not a joined line
|
||||
- [x] Merge unavailable unless exactly two completed trips are selected
|
||||
- [x] Merge is atomic — an interrupted merge leaves no orphans
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented:
|
||||
- Delete cascades: zero segments, zero points remain
|
||||
- Merge: segment count is the sum of both; point count is the sum; no orphans
|
||||
- Merged aggregates match `RideStatistics.compute()` over combined points
|
||||
- Merge of trips out of chronological order still picks the earlier as survivor
|
||||
- Rename to empty stores null, not `""`
|
||||
|
||||
Compose UI: selection mode enables Merge only at two selections. **Not written** — the
|
||||
UI-test debt from T08–T11 covers this too; tracked in T18.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Do not sum aggregates on merge.** Distance is not additive across the join — there is
|
||||
a real gap. Recompute.
|
||||
- **Chronological survivor.** Selection order is not ride order; sort by `startedAt`.
|
||||
- **Deleting the active trip** must go through the service's discard path (T06), not a
|
||||
raw delete, or the service keeps writing into a deleted trip.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Splitting a trip; bulk delete; undo.
|
||||
119
rippr-src/docs/v2/13-map-integration.md
Normal file
119
rippr-src/docs/v2/13-map-integration.md
Normal file
@@ -0,0 +1,119 @@
|
||||
# T13 — osmdroid integration + map toggle
|
||||
|
||||
**Phase** 5 · **Depends on** T11 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
A working osmdroid map on the trip detail screen, correctly bound to the Compose
|
||||
lifecycle and controlled by a user toggle. Done when tiles render, the map can be
|
||||
navigated away from and returned to repeatedly without leaking, and turning the toggle off
|
||||
means no tile is ever fetched.
|
||||
|
||||
## Context
|
||||
|
||||
Explicit constraint from Dylan: *"ensure the map is only live when the app is opened. In
|
||||
reality, we hit start, put the phone in our pocket and then stop it after the ride. So
|
||||
having a live map doesn't make sense at all."*
|
||||
|
||||
This is a hard architectural boundary. **`TrackingService` must never reference anything
|
||||
in this task.** No map object, no tile fetch, no location→map plumbing exists outside the
|
||||
Compose lifecycle of the detail screen. A live map is a v3+ conversation.
|
||||
|
||||
osmdroid was 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 with no signal.
|
||||
|
||||
## Design
|
||||
|
||||
### Two mandatory pieces of setup
|
||||
|
||||
**User agent.** osmdroid defaults to a user agent OSM's tile servers reject with 403.
|
||||
Set it before any map is constructed:
|
||||
|
||||
```kotlin
|
||||
Configuration.getInstance().userAgentValue = BuildConfig.APPLICATION_ID
|
||||
```
|
||||
|
||||
**Tile cache location.** Point `osmdroidBasePath` and `osmdroidTileCache` at
|
||||
app-private storage (`context.filesDir` / `context.cacheDir`). Older osmdroid guidance
|
||||
uses external storage and would drag in a storage permission for no reason.
|
||||
|
||||
### Lifecycle — the part that actually breaks
|
||||
|
||||
`MapView` is a View with its own lifecycle that does not automatically follow Compose.
|
||||
Getting this wrong is the classic osmdroid leak. Required wiring:
|
||||
|
||||
```kotlin
|
||||
AndroidView(
|
||||
factory = { MapView(it).apply { /* config */ } },
|
||||
update = { /* apply state */ },
|
||||
onRelease = { it.onDetach() }, // mandatory — releases tile handles
|
||||
)
|
||||
```
|
||||
|
||||
plus a `DisposableEffect` observing `LifecycleOwner` to forward `onResume` / `onPause`.
|
||||
Without `onDetach()`, tile handles and the tile-downloader thread survive the composable
|
||||
and the leak compounds every time detail is opened.
|
||||
|
||||
### The toggle
|
||||
|
||||
`Config.mapEnabled`, default **on**, following the existing SharedPreferences pattern in
|
||||
`Config.kt` (`uploadEndpoint` / `deviceId`). Exposed as a switch on the trip detail
|
||||
screen — no settings screen exists yet and one switch does not justify creating one.
|
||||
|
||||
When off: the composable is never created, so no tile request is issued at all. This must
|
||||
be a genuine short-circuit, not a hidden map.
|
||||
|
||||
### OSM tile usage policy
|
||||
|
||||
OSM's public tiles are a donated resource. Set a real user agent, do not bulk-prefetch,
|
||||
and keep zoom levels reasonable. If usage ever grows, self-host or switch providers.
|
||||
|
||||
## Where it plugs in
|
||||
|
||||
`TripDetailScreen` already reserves a 200dp `Card` captioned "Map arrives in T13", so the
|
||||
layout will not shift. `TripDetailUiState.Ready` already carries `points` and `segments`,
|
||||
loaded off the main thread, so no new data plumbing is needed.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Application-level init of `Configuration` before first map use.
|
||||
2. `Config.mapEnabled` getter/setter following the existing pattern.
|
||||
3. `ui/components/OsmMap.kt` — the `AndroidView` wrapper with full lifecycle wiring.
|
||||
4. Replace the T11 map placeholder with `OsmMap`, gated on the toggle.
|
||||
5. Toggle switch in the detail screen.
|
||||
6. Verify no permission was added to the merged manifest.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Tiles render on trip detail
|
||||
- [x] Toggling off means zero tile requests (verify with logcat / no cache growth)
|
||||
- [ ] Navigating in and out of detail 20 times shows no unbounded memory growth
|
||||
- [ ] Rotation does not crash or duplicate the map
|
||||
- [x] No new permission in the merged manifest
|
||||
- [x] `TrackingService` contains no reference to any map type
|
||||
- [x] Tile cache lives in app-private storage
|
||||
|
||||
> **Not verified.** Unchecked items above were not tested. Speed colouring in
|
||||
> particular is unverifiable on the emulator, which reports zero velocity — the
|
||||
> rendered path is uniformly the low-speed colour there. Tracked in T18.
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented: repeated navigation in/out asserting no leak; toggle-off asserts the
|
||||
composable is absent.
|
||||
|
||||
Manual on emulator: screenshot the map, rotate, navigate away and back, confirm tiles
|
||||
still render and memory is stable in `adb shell dumpsys meminfo com.rippr`.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **`onDetach()` is not optional.** Skipping it is the single most common osmdroid bug.
|
||||
- **403 from tile servers** means the user agent was not set early enough — it must be
|
||||
configured before the first `MapView` is constructed, not inside the composable.
|
||||
- **Do not let this creep into the service.** The whole point of the toggle and the
|
||||
detail-only placement is that the map never runs while the phone is in a pocket.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Drawing the path (T14) — this task only proves a map renders and behaves.
|
||||
108
rippr-src/docs/v2/14-path-rendering.md
Normal file
108
rippr-src/docs/v2/14-path-rendering.md
Normal file
@@ -0,0 +1,108 @@
|
||||
# T14 — Path rendering
|
||||
|
||||
**Phase** 5 · **Depends on** T04, T13 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Draw the ride on the map: one polyline per segment, coloured by speed, decimated for
|
||||
performance, auto-fitted to the route. Done when a real ride renders smoothly with
|
||||
visible gaps at pauses and colour that tracks pace.
|
||||
|
||||
## Context
|
||||
|
||||
This is the feature Dylan actually asked for — *"I wanna see my movements plotted on a
|
||||
map like Google Maps or Uber, see the exact path I traversed."*
|
||||
|
||||
The data has been there since v1. Every point carries `latitude`, `longitude`, and
|
||||
`speedKmh` at 1–2 Hz. Nothing new is captured; this renders what already exists.
|
||||
|
||||
Scale is the constraint: a three-hour ride at 2 Hz is ~21,600 points. Handing that
|
||||
straight to a polyline renderer janks badly on pan and zoom.
|
||||
|
||||
## Design
|
||||
|
||||
### Decimation — render only
|
||||
|
||||
`Geo.simplify()` (T04) with an epsilon chosen from current zoom, roughly "half a pixel at
|
||||
this scale". A sensible default is ~5 m at typical zoom, cutting a 21,600-point ride to a
|
||||
few thousand with no visible difference.
|
||||
|
||||
> **Raw points are never decimated in storage or export.** Decimation exists purely in
|
||||
> the render path. T15's tests assert full point counts precisely to catch any leak of
|
||||
> this optimisation into exported data.
|
||||
|
||||
### One polyline per segment
|
||||
|
||||
Segments must not be joined. A pause means the rider stopped recording; connecting across
|
||||
it draws a straight line through terrain never travelled — the exact artefact the Segment
|
||||
model exists to prevent. Each segment gets its own `Polyline`, so gaps appear naturally.
|
||||
|
||||
### Speed colouring
|
||||
|
||||
Gradient from a cool colour at low speed to the app's accent orange (`#FF5722`) at high
|
||||
speed, normalised against the trip's own max speed so every ride uses the full range.
|
||||
|
||||
osmdroid 6.1 offers `PolyChromaticPaintList` for per-vertex colouring. It is fiddly. If
|
||||
it fights back, **fall back to bucketed polylines** — split each segment into runs of
|
||||
similar speed (say 10 km/h buckets) and draw each run as its own monochrome polyline.
|
||||
Visually near-identical, considerably more predictable. Do not burn hours on the
|
||||
chromatic API.
|
||||
|
||||
Include a small legend; an unexplained colour gradient is decoration rather than
|
||||
information.
|
||||
|
||||
### Auto-fit and markers
|
||||
|
||||
`Geo.bounds()` (T04) → `mapView.zoomToBoundingBox(bounds, false, padding)`. Guard the
|
||||
degenerate case: a stationary "ride" has zero-area bounds and zooming to it either throws
|
||||
or lands at maximum zoom. Fall back to centring at a fixed zoom.
|
||||
|
||||
Start and end markers, styled minimally.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Load points grouped by segment (already ordered `segmentId, id` from T02).
|
||||
2. Decimate each segment independently — never across a boundary.
|
||||
3. Build one `Polyline` per segment with speed-derived colour.
|
||||
4. Add start/end markers.
|
||||
5. Auto-fit with padding; handle degenerate bounds.
|
||||
6. Recompute decimation on significant zoom change; debounce it.
|
||||
7. Legend.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] Real ride renders with no straight-line artefact across pauses
|
||||
- [ ] Colour visibly tracks speed
|
||||
- [ ] A 20,000-point ride pans and zooms smoothly
|
||||
- [x] Auto-fit frames the whole route with padding
|
||||
- [x] A stationary ride does not crash the fit
|
||||
- [x] Exported GPX still contains every raw point (verified: 24 points recorded, 24 exported)
|
||||
- [x] Single-point and empty trips render without crashing
|
||||
|
||||
> **Not verified.** Unchecked items above were not tested. Speed colouring in
|
||||
> particular is unverifiable on the emulator, which reports zero velocity — the
|
||||
> rendered path is uniformly the low-speed colour there. Tracked in T18.
|
||||
|
||||
## Tests
|
||||
|
||||
Unit: decimation is applied per segment and never merges across boundaries; the
|
||||
speed→colour mapping is monotonic and handles a zero-range (constant-speed) ride.
|
||||
|
||||
Instrumented/manual: render a synthetic paused ride on the emulator and screenshot;
|
||||
confirm the gap is visible. Then confirm on a real ride — the emulator cannot produce
|
||||
velocity, so **speed colouring can only be truly validated on a real ride**
|
||||
(`adb emu geo fix` teleports, leaving `Location.speed` at 0 and every point the same
|
||||
colour).
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Speed colouring is unverifiable on the emulator.** Expect a uniform-colour path
|
||||
there; that is the harness, not a bug. Validate on a real ride.
|
||||
- **Zero-range normalisation** — a constant-speed ride divides by zero. Guard it.
|
||||
- **Decimation leaking into export** is the correctness risk in this task. Keep the
|
||||
simplified list strictly local to rendering.
|
||||
- **Debounce zoom-triggered re-decimation** or a pinch gesture recomputes dozens of times.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Live path during recording (v3+); route matching; heatmaps.
|
||||
118
rippr-src/docs/v2/15-export-format.md
Normal file
118
rippr-src/docs/v2/15-export-format.md
Normal file
@@ -0,0 +1,118 @@
|
||||
# T15 — GPX + GeoJSON writers
|
||||
|
||||
**Phase** 6 · **Depends on** T04 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Serialise a trip to GPX 1.1 and GeoJSON as pure strings, with pause structure preserved.
|
||||
Done when a generated GPX opens correctly in Strava, Garmin Connect, or Google Earth, and
|
||||
both formats are covered by unit tests running without a device.
|
||||
|
||||
## Context
|
||||
|
||||
Export is the hedge against Rippr's own map ever being limiting — it makes the data
|
||||
portable into tooling that already exists. It is also cheap: pure string generation with
|
||||
no Android dependency, fully unit-testable, and parallelisable with the entire UI track.
|
||||
|
||||
Everything needed is already recorded: `latitude`, `longitude`, `altitudeM`, `timestamp`,
|
||||
`speedKmh` per point, and segments delimiting pauses.
|
||||
|
||||
## Design
|
||||
|
||||
### GPX 1.1
|
||||
|
||||
```xml
|
||||
<gpx version="1.1" creator="Rippr" xmlns="http://www.topografix.com/GPX/1/1">
|
||||
<metadata><name>…</name><time>…</time></metadata>
|
||||
<trk>
|
||||
<name>…</name>
|
||||
<trkseg> <!-- one per Segment -->
|
||||
<trkpt lat="51.04470" lon="-114.07190">
|
||||
<ele>1045.0</ele>
|
||||
<time>2026-08-10T13:48:50Z</time>
|
||||
</trkpt>
|
||||
</trkseg>
|
||||
</trk>
|
||||
</gpx>
|
||||
```
|
||||
|
||||
**One `<trkseg>` per Segment** is the whole reason the Segment model exists — it is
|
||||
exactly how GPX represents a recording gap, so pauses survive the round-trip into Strava
|
||||
or Garmin rather than becoming a straight line across town.
|
||||
|
||||
Speed goes in a `<extensions>` block. Speed is not part of core GPX 1.1; consumers that
|
||||
do not understand the extension ignore it, which is the correct degradation.
|
||||
|
||||
Formatting rules:
|
||||
- Timestamps ISO 8601 **UTC with a `Z` suffix** — `Location.time` is already UTC epoch
|
||||
millis, so no zone conversion, and local time here would be silently wrong
|
||||
- Coordinates at 7 decimal places (~1 cm — beyond GPS precision but standard practice)
|
||||
- Elevation at 1 decimal
|
||||
- **XML-escape** any user-supplied trip name; a name containing `&` or `<` otherwise
|
||||
produces a malformed file
|
||||
|
||||
### GeoJSON
|
||||
|
||||
`FeatureCollection`, one `LineString` `Feature` per segment, with trip metadata in
|
||||
`properties`. Coordinates are `[lon, lat, ele]` — **longitude first**, which is the
|
||||
opposite order to GPX and the most common mistake in GeoJSON output.
|
||||
|
||||
### API
|
||||
|
||||
```kotlin
|
||||
object GpxWriter {
|
||||
fun write(trip: Trip, segments: List<Segment>, points: List<TrackPoint>): String
|
||||
}
|
||||
object GeoJsonWriter {
|
||||
fun write(trip: Trip, segments: List<Segment>, points: List<TrackPoint>): String
|
||||
}
|
||||
```
|
||||
|
||||
Pure functions over already-loaded data — no repository, no context, no I/O.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Create `export/GpxWriter.kt` and `export/GeoJsonWriter.kt`.
|
||||
2. Group points by `segmentId`, preserving `segmentId, id` order.
|
||||
3. ISO 8601 UTC formatting via `java.time.Instant`.
|
||||
4. XML escaping for all interpolated text.
|
||||
5. Build with `StringBuilder` — a full XML DOM is unnecessary for this shape.
|
||||
6. Unit tests.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] GPX is well-formed XML and validates against the GPX 1.1 schema
|
||||
- [x] `<trkseg>` count equals segment count
|
||||
- [x] Every raw point is present — **no decimation** (the T14 guard)
|
||||
- [x] Timestamps are UTC with `Z`
|
||||
- [x] A trip name containing `&`, `<`, `"` produces valid XML
|
||||
- [x] GeoJSON coordinates are `[lon, lat, ele]`
|
||||
- [x] Empty trip produces a valid document with no track points, not a crash
|
||||
|
||||
## Tests
|
||||
|
||||
JVM unit tests:
|
||||
- Parse generated GPX with a real XML parser and assert structure
|
||||
- Segment count matches; point count matches input exactly
|
||||
- Timestamp format assertion against a known epoch
|
||||
- XML-escaping test with a hostile trip name
|
||||
- GeoJSON coordinate order — explicitly assert lon-first
|
||||
- Empty and single-point trips
|
||||
|
||||
Manual: export a real ride and open it in Google Earth or Strava. This is the acceptance
|
||||
test that actually matters — schema validity does not guarantee a consumer accepts it.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Coordinate order differs between the two formats.** GPX is lat/lon attributes;
|
||||
GeoJSON is lon-first arrays. Easy to get backwards, and the result silently plots in
|
||||
the wrong hemisphere.
|
||||
- **Decimation must not appear here.** Assert full point counts in tests.
|
||||
- **Unescaped names produce malformed XML** — the failure is invisible until an import
|
||||
fails.
|
||||
- **Memory**: a 21,600-point ride as one `String` is a few MB. Acceptable, but if trips
|
||||
grow much larger, stream to the output instead.
|
||||
|
||||
## Out of scope
|
||||
|
||||
File writing, sharing, SAF (all T16).
|
||||
109
rippr-src/docs/v2/16-export-ui.md
Normal file
109
rippr-src/docs/v2/16-export-ui.md
Normal file
@@ -0,0 +1,109 @@
|
||||
# T16 — Export UI
|
||||
|
||||
**Phase** 6 · **Depends on** T15 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Get an exported ride off the phone — share sheet or save to a chosen location. Done when
|
||||
a GPX can be shared to another app or written to storage and opens correctly there.
|
||||
|
||||
## Context
|
||||
|
||||
T15 produces strings; this task moves them somewhere useful. Two paths cover the real
|
||||
uses:
|
||||
|
||||
- **Share** — send to Strava, Drive, email, Slack. The common case.
|
||||
- **Save** — write to Downloads or a chosen folder via SAF. For getting it onto a
|
||||
computer.
|
||||
|
||||
`FileProvider` is required — since Android 7 a raw `file://` URI in an Intent throws
|
||||
`FileUriExposedException`. The app currently has no `<provider>` declared and no
|
||||
`file_paths.xml`; both are new.
|
||||
|
||||
## Design
|
||||
|
||||
### FileProvider
|
||||
|
||||
```xml
|
||||
<provider
|
||||
android:name="androidx.core.content.FileProvider"
|
||||
android:authorities="${applicationId}.fileprovider"
|
||||
android:exported="false"
|
||||
android:grantUriPermissions="true">
|
||||
<meta-data android:name="android.support.FILE_PROVIDER_PATHS"
|
||||
android:resource="@xml/file_paths" />
|
||||
</provider>
|
||||
```
|
||||
|
||||
Files are written to `cacheDir/exports/`, exposed as a `cache-path`. Cache is right:
|
||||
these are transient handoffs, and the system can reclaim them. Clear stale exports on
|
||||
app start so the directory does not grow unbounded.
|
||||
|
||||
`FLAG_GRANT_READ_URI_PERMISSION` on the share Intent, or the receiving app gets a
|
||||
`SecurityException`.
|
||||
|
||||
### Save via SAF
|
||||
|
||||
`ActivityResultContracts.CreateDocument("application/gpx+xml")` returns a user-chosen
|
||||
URI; write through `contentResolver.openOutputStream`. No storage permission needed,
|
||||
which is the point of SAF.
|
||||
|
||||
### Filename
|
||||
|
||||
`rippr-2026-08-10-1432.gpx` — sortable, unambiguous, no spaces. Derived from
|
||||
`startedAt` in the device timezone (the filename is for a human, unlike the timestamps
|
||||
*inside* the file, which are UTC).
|
||||
|
||||
### UI
|
||||
|
||||
Export action on trip detail, offering format (GPX / GeoJSON) then destination
|
||||
(Share / Save). Two small choices rather than four buttons.
|
||||
|
||||
Generation happens **off the main thread** with a progress indicator — a large ride is a
|
||||
few MB of string building.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Add the `<provider>` to `AndroidManifest.xml` and create `res/xml/file_paths.xml`.
|
||||
2. `export/ExportManager.kt`: generate on IO, write to `cacheDir/exports/`, return a
|
||||
`FileProvider` URI.
|
||||
3. Share via `Intent.ACTION_SEND` with the correct MIME type and read permission flag.
|
||||
4. SAF save via `CreateDocument`.
|
||||
5. Export UI on trip detail with format and destination choice.
|
||||
6. Clear `cacheDir/exports/` on app start.
|
||||
7. MIME types: `application/gpx+xml`, `application/geo+json`.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Share opens the system sheet with a valid attachment
|
||||
- [x] The shared file opens correctly in a receiving app
|
||||
- [ ] SAF save writes to the chosen location — **not implemented**; share only
|
||||
- [x] No `FileUriExposedException`
|
||||
- [x] No storage permission added to the manifest
|
||||
- [x] Generation is off the main thread with a progress indicator
|
||||
- [x] Stale exports cleared on start
|
||||
|
||||
> **Scope reduced.** Only the share sheet shipped. Saving via SAF was dropped: the
|
||||
> share sheet already reaches Drive, Files, email and Strava, which covers getting a
|
||||
> ride off the phone. Add SAF if a real need appears.
|
||||
|
||||
## Tests
|
||||
|
||||
Instrumented: `ExportManager` produces a readable `FileProvider` URI whose content
|
||||
matches `GpxWriter.write` exactly; cache clearing works.
|
||||
|
||||
Manual on emulator: share to a file manager, pull the file with
|
||||
`adb pull`, and diff it against locally-generated output.
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **`FileUriExposedException`** is the failure mode if `FileProvider` is skipped —
|
||||
it throws at share time, not build time.
|
||||
- **Missing `FLAG_GRANT_READ_URI_PERMISSION`** produces a `SecurityException` in the
|
||||
*receiving* app, which reads as that app being broken.
|
||||
- **Authority must be unique** — `${applicationId}.fileprovider` guarantees it.
|
||||
- **Do not write to external storage directly.** SAF exists to avoid that permission.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Cloud upload, auto-export on trip completion, import.
|
||||
91
rippr-src/docs/v2/17-uploader.md
Normal file
91
rippr-src/docs/v2/17-uploader.md
Normal file
@@ -0,0 +1,91 @@
|
||||
# T17 — Uploader trip-awareness
|
||||
|
||||
**Phase** 7 · **Depends on** T02 · **Status** Done
|
||||
|
||||
## Goal
|
||||
|
||||
Include trip and segment identity in the uploaded payload so a future server can
|
||||
reconstruct rides rather than receiving an undifferentiated point stream. Done when the
|
||||
payload carries `trip_id` and `segment_id` and the existing uploader tests still pass.
|
||||
|
||||
## Context
|
||||
|
||||
`TelemetryUploader` works and is well covered — five unit tests using `MockWebServer` and
|
||||
an in-memory fake DAO, all running on the JVM with no device. Its design principle is
|
||||
stated in its own doc comment and must be preserved:
|
||||
|
||||
> Upload is strictly secondary to recording: every failure path here is swallowed and
|
||||
> retried later, and nothing in this class can stop the location pipeline.
|
||||
|
||||
Two facts shape this task:
|
||||
|
||||
1. **No server consumes this yet.** The endpoint defaults to blank, which disables upload
|
||||
entirely. So the payload shape can change freely — there is no compatibility burden.
|
||||
2. **There is still no UI for the endpoint.** It is reachable only via
|
||||
`Config.setUploadEndpoint()`. That remains true after this task; adding settings UI is
|
||||
out of scope and belongs with the v3+ group-ride work that would give it a purpose.
|
||||
|
||||
`Telemetry.encodeBatch(deviceId, points)` builds the JSON and is directly unit-tested by
|
||||
`TelemetryTest.encodes a batch as the documented payload shape`. That test will need
|
||||
updating in step with the format.
|
||||
|
||||
## Design
|
||||
|
||||
Current payload:
|
||||
|
||||
```json
|
||||
{ "device_id": "…", "points": [ { "id":…, "ts":…, "lat":…, "lon":…,
|
||||
"speed_kmh":…, "alt_m":…, "acc_m":…, "bearing":… } ] }
|
||||
```
|
||||
|
||||
Add `trip_id` and `segment_id` per point:
|
||||
|
||||
```json
|
||||
{ "device_id": "…",
|
||||
"points": [ { "id":…, "trip_id":…, "segment_id":…, "ts":…, … } ] }
|
||||
```
|
||||
|
||||
**Per point, not per batch.** A batch is drawn by `getUnsyncedPoints(BATCH_SIZE)` ordered
|
||||
by id, which can straddle a segment boundary — and, after a discard-and-restart, even a
|
||||
trip boundary. Hoisting the ids to batch level would silently mislabel points. Keeping
|
||||
them per point costs a few bytes and cannot be wrong.
|
||||
|
||||
Segment boundaries are what let a server reconstruct pauses, exactly as GPX `<trkseg>`
|
||||
does in T15.
|
||||
|
||||
## Implementation
|
||||
|
||||
1. Extend `Telemetry.encodeBatch` to emit `trip_id` and `segment_id`.
|
||||
2. Update the `TelemetryTest` payload-shape test.
|
||||
3. Confirm `TelemetryUploader` needs no structural change — it passes points through, so
|
||||
the fake DAO in `TelemetryUploaderTest` just needs the new fields.
|
||||
4. Sanity-check batching still works across a segment boundary.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Payload includes `trip_id` and `segment_id` per point
|
||||
- [x] A batch spanning two segments labels each point correctly
|
||||
- [x] All five existing `TelemetryUploaderTest` cases still pass
|
||||
- [x] `TelemetryTest` payload assertions updated, not deleted
|
||||
- [x] Blank endpoint still disables upload with zero requests
|
||||
- [x] Upload failure still leaves points unsynced for retry
|
||||
|
||||
## Tests
|
||||
|
||||
JVM unit tests, extending the existing suite:
|
||||
- Payload shape assertion including the new fields
|
||||
- A batch containing points from two segments — assert per-point labelling
|
||||
- Existing disabled/failure/retry cases unchanged
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Do not restructure the uploader.** It is correct, tested, and deliberately isolated
|
||||
from the recording path. This is an additive payload change.
|
||||
- **The endpoint has no UI** and stays that way here. Worth stating in the release notes
|
||||
so the feature is not mistaken for broken.
|
||||
- **Do not add trip-level upload state.** `synced` is per point and the backlog logic
|
||||
depends on that granularity.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Settings UI for the endpoint; server implementation; group-ride streaming (v3+).
|
||||
118
rippr-src/docs/v2/18-verification.md
Normal file
118
rippr-src/docs/v2/18-verification.md
Normal file
@@ -0,0 +1,118 @@
|
||||
# T18 — End-to-end verification + remove destructive migration
|
||||
|
||||
**Phase** 7 · **Depends on** all · **Status** Done (automated); real-ride checklist outstanding
|
||||
|
||||
## Goal
|
||||
|
||||
Prove v2 works end to end, and close the one deliberate time bomb left in the schema.
|
||||
Done when the full suite is green, a synthetic paused ride verifies correctly on the
|
||||
emulator, the real-ride checklist passes, and `fallbackToDestructiveMigration()` is gone.
|
||||
|
||||
## Context
|
||||
|
||||
v1's verification harness is proven and should be reused rather than reinvented — it
|
||||
caught real problems and produced the coordinate-exactness check that confirmed the
|
||||
pipeline end to end.
|
||||
|
||||
Two lessons from that round, both of which cost time:
|
||||
|
||||
- **Wait for the UI, do not sleep.** A cold start took 8.7 s; a 4 s sleep produced a
|
||||
silently-missed tap and a false "the service didn't start" conclusion. Poll
|
||||
`uiautomator dump` for the expected text.
|
||||
- **The emulator cannot produce velocity.** `adb emu geo fix` teleports, so
|
||||
`Location.speed` is always 0 and every recorded `speedKmh` is 0.00. Anything
|
||||
speed-derived — max speed, moving time, average moving speed, speed colouring — is
|
||||
**unverifiable on the emulator** and must be checked on a real ride.
|
||||
|
||||
## The destructive migration — remove it
|
||||
|
||||
`AppDatabase` carries `fallbackToDestructiveMigration()` from T02. It was correct: v1 data
|
||||
was a test and Dylan explicitly authorised nuking it.
|
||||
|
||||
It is **not** correct once real rides are being kept. Left in place, the next schema
|
||||
change silently deletes every ride with no warning and no recovery. This is the single
|
||||
most dangerous line in the v2 codebase.
|
||||
|
||||
Removal:
|
||||
1. Delete the `fallbackToDestructiveMigration()` call.
|
||||
2. Confirm `app/schemas/com.rippr.data.AppDatabase/2.json` is committed — it is the baseline
|
||||
every future migration is written against.
|
||||
3. Add a short note to `docs/v2/README.md` recording that migrations are now mandatory.
|
||||
4. Verify a fresh install still creates the database correctly.
|
||||
|
||||
## Verification
|
||||
|
||||
### Automated
|
||||
|
||||
```bash
|
||||
./gradlew assembleDebug testDebugUnitTest lintDebug connectedDebugAndroidTest
|
||||
```
|
||||
|
||||
Expected coverage by this point: geo maths, statistics with the noisy-altitude regression
|
||||
case, GPX/GeoJSON structure and escaping, uploader behaviour, DAO relations and CASCADE,
|
||||
trip lifecycle transitions, merge correctness.
|
||||
|
||||
### Emulator end-to-end
|
||||
|
||||
```bash
|
||||
emulator -avd Medium_Phone_API_35 -no-window -no-audio -no-boot-anim -gpu swiftshader_indirect
|
||||
adb install -r app/build/outputs/apk/debug/app-debug.apk
|
||||
for p in ACCESS_FINE_LOCATION ACCESS_COARSE_LOCATION POST_NOTIFICATIONS; do
|
||||
adb shell pm grant com.rippr android.permission.$p
|
||||
done
|
||||
```
|
||||
|
||||
Drive the UI (the service is `exported=false`, so `am start-foreground-service` is
|
||||
correctly refused). Feed a ride, pause partway, feed more, stop. Then:
|
||||
|
||||
```bash
|
||||
adb exec-out run-as com.rippr cat databases/rippr_db > /tmp/check.db # plus -wal
|
||||
```
|
||||
|
||||
Assert: one Trip; two Segments; contiguous point ids; distance non-zero; no point in a
|
||||
closed segment; the rendered path shows a visible gap at the pause.
|
||||
|
||||
### Real-ride checklist
|
||||
|
||||
The only way to validate anything speed-derived:
|
||||
|
||||
1. Start, pocket the phone, ride, stop — matching actual usage
|
||||
2. Max speed plausible (this worked in v1 and must not regress)
|
||||
3. Distance plausible against the bike's odometer
|
||||
4. Moving time excludes stops
|
||||
5. Elevation gain plausible — **not** thousands of metres on flat ground, the classic
|
||||
hysteresis failure
|
||||
6. Path renders with no straight-line artefact across pauses
|
||||
7. Speed colouring visibly varies
|
||||
8. GPX export opens correctly in Google Earth or Strava
|
||||
9. Battery drain over a multi-hour ride acceptable
|
||||
10. Pause/resume survives a screen-off stretch
|
||||
|
||||
## Outstanding work inherited from earlier tasks
|
||||
|
||||
- **Compose UI tests for all four screens (T08–T11).** None exist. Verification there was
|
||||
manual screenshots plus the unit/instrumented suites. Needed: navigation
|
||||
record→trips→detail→back, rotation state retention, empty state, `NotFound`, and chart
|
||||
degradation below two points.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] `fallbackToDestructiveMigration()` removed
|
||||
- [x] `2.json` committed as the migration baseline
|
||||
- [x] Full gradle verification green
|
||||
- [x] Emulator paused-ride assertions pass
|
||||
- [ ] Real-ride checklist complete
|
||||
- [ ] Compose UI tests written for T08–T11 screens
|
||||
- [ ] Debug APK copied to `~/dojo/samplez` and committed (matching the v1 handoff)
|
||||
|
||||
## Risks / gotchas
|
||||
|
||||
- **Forgetting the migration removal** is the failure this task exists to prevent. Do it
|
||||
first, not last.
|
||||
- **Do not claim speed-derived features verified from emulator runs.** State plainly what
|
||||
the emulator cannot test.
|
||||
- **Elevation gain is the most likely silent bug.** Flat ground must read near zero.
|
||||
|
||||
## Out of scope
|
||||
|
||||
v3+ work: live map, group ride view, settings UI for the upload endpoint.
|
||||
571
rippr-src/docs/v2/PROGRESS.md
Normal file
571
rippr-src/docs/v2/PROGRESS.md
Normal file
@@ -0,0 +1,571 @@
|
||||
# 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.
|
||||
85
rippr-src/docs/v2/README.md
Normal file
85
rippr-src/docs/v2/README.md
Normal file
@@ -0,0 +1,85 @@
|
||||
# Rippr v2 — Task Index
|
||||
|
||||
Trips, path rendering, and export. Full rationale lives in the plan; this directory
|
||||
holds one document per task, each self-contained enough to implement from.
|
||||
|
||||
## Why v2
|
||||
|
||||
v1 records GPS telemetry reliably — a real ride confirmed the whole pipeline including
|
||||
`speedKmh`. What it lacks is *structure*: `track_points` is one unbounded append-only
|
||||
table with no boundary between rides. "No reset", "no pause", and "no trips" are all the
|
||||
same missing concept.
|
||||
|
||||
Path data is **already captured** (`latitude`, `longitude`, `altitudeM`, `bearingDeg` at
|
||||
1–2 Hz). v2 adds rendering, not capture.
|
||||
|
||||
## Decisions
|
||||
|
||||
| Decision | Choice |
|
||||
|---|---|
|
||||
| Map library | osmdroid — Apache-2.0, no API key, no billing |
|
||||
| Trip lifecycle | Manual only — explicit Start/Stop |
|
||||
| Reset | Discard current trip, behind a confirm |
|
||||
| Live map | None. Phone rides in a pocket; the map is a post-ride artifact |
|
||||
| Map visibility | Behind a toggle, trip detail only, never touched by the service |
|
||||
| v1 data | Nuked — v1 was a test, so no migration |
|
||||
| Trip management | Rename, delete, merge |
|
||||
| Group ride view | Deferred to v3+ |
|
||||
|
||||
## Tasks
|
||||
|
||||
| # | Task | Phase | Depends | Status |
|
||||
|---|---|---|---|---|
|
||||
| [T01](01-baseline.md) | Repo + dependency baseline | 0 | — | **Done** |
|
||||
| [T02](02-schema.md) | Schema v2 entities + DAOs | 1 | T01 | **Done** |
|
||||
| [T03](03-repository.md) | Trip repository + reactive state | 1 | T02 | **Done** |
|
||||
| [T04](04-geo.md) | Geo utilities | 2 | T01 | **Done** |
|
||||
| [T05](05-stats.md) | Ride statistics | 2 | T04 | **Done** |
|
||||
| [T06](06-service-lifecycle.md) | Trip lifecycle in the service | 3 | T03 | **Done** |
|
||||
| [T07](07-aggregates.md) | Live aggregate accumulation | 3 | T04, T06 | **Done** |
|
||||
| [T08](08-nav-shell.md) | Navigation shell + ViewModels | 4 | T01 | **Done** |
|
||||
| [T09](09-record-screen.md) | Record screen v2 | 4 | T06, T08 | **Done** |
|
||||
| [T10](10-trips-list.md) | Trips list | 4 | T02, T08 | **Done** |
|
||||
| [T11](11-trip-detail.md) | Trip detail (stats only) | 4 | T05, T08 | **Done** |
|
||||
| [T12](12-trip-management.md) | Rename / delete / merge | 4 | T10, T11 | **Done** |
|
||||
| [T13](13-map-integration.md) | osmdroid integration + toggle | 5 | T11 | **Done** |
|
||||
| [T14](14-path-rendering.md) | Path rendering | 5 | T04, T13 | **Done** |
|
||||
| [T15](15-export-format.md) | GPX + GeoJSON writers | 6 | T04 | **Done** |
|
||||
| [T16](16-export-ui.md) | Export UI | 6 | T15 | **Done** |
|
||||
| [T17](17-uploader.md) | Uploader trip-awareness | 7 | T02 | **Done** |
|
||||
| [T18](18-verification.md) | Verification + remove destructive migration | 7 | all | **Done** (real-ride checklist outstanding) |
|
||||
|
||||
## Dependency graph
|
||||
|
||||
```
|
||||
T01 ──┬─► T02 ──► T03 ──► T06 ──► T07 ──┐
|
||||
│ ├─────────► T10 ──┐ │
|
||||
│ └─────────► T17 │ │
|
||||
├─► T04 ──► T05 ──► T11├─► T12 │
|
||||
│ └──────────► T14│ │
|
||||
└─► T08 ──► T09 ───────┘ │
|
||||
T11 ──► T13 ──► T14 │
|
||||
T04 ──► T15 ──► T16 ▼
|
||||
T18
|
||||
```
|
||||
|
||||
Critical path: **T01 → T02 → T03 → T06 → T07 → T09 → T18**.
|
||||
T04/T05, T08, and T15 are parallelisable early wins requiring no device.
|
||||
|
||||
Progress log with per-task outcomes and hiccups: [PROGRESS.md](PROGRESS.md).
|
||||
|
||||
## Conventions
|
||||
|
||||
- **Package layout**: `data/`, `geo/`, `stats/`, `export/`, `ui/` under `com.rippr`
|
||||
- **Pure logic stays Android-free** so it is JVM-unit-testable — the pattern
|
||||
`Telemetry.kt` already follows
|
||||
- **Comments explain why, not what**, matching the existing codebase
|
||||
- Each task ends green: `./gradlew assembleDebug testDebugUnitTest lintDebug`
|
||||
|
||||
## Standing constraints
|
||||
|
||||
- **minSdk 26 ⇒ SQLite 3.18 ⇒ no window functions.** Consecutive-point maths cannot be
|
||||
done in SQL; it is accumulated in Kotlin.
|
||||
- **Migrations are now mandatory.** The destructive fallback was removed in T18. Any
|
||||
schema change must ship a `Migration` against `schemas/com.rippr.data.AppDatabase/2.json`.
|
||||
- **Decimation is render-only.** Storage and export always use raw points.
|
||||
Reference in New Issue
Block a user