uhryniuk
280fd7f988
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>
147 lines
6.7 KiB
Markdown
147 lines
6.7 KiB
Markdown
# 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).
|