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>
92 lines
3.7 KiB
Markdown
92 lines
3.7 KiB
Markdown
# 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+).
|