feat(trips): a ride start on a trip day makes that trip active

[timohueser]

What changed

A ride that starts on a trip day makes that trip the active trip. A Finish after Keep riding past the end of a day moves the position into the next day when the rider is on its route. These are the last code items of #2069.

A start makes its trip active

• Start record. The fresh start of a ride on a trip day, and its continuation after a reset, write a start record: day 0, day 0's route, 0 m, no finished day, no dates (TripSummary::start, App::note_trip_start). The device writes none when the trip's record is already the last one.
• The store moves the record. The bound rules (obc_formats::trip_progress::record) never let a start record replace a record. When the key has a record, that record moves to the end, byte for byte. The start record goes in only when the key has none. The resident records and the store apply the same function. So a start before the device has read the records cannot blank a trip's progress, and every written record takes the store's Revision.
• Opening without a start. Opening a day from the day list is a preview; Back restores the route loaded before. So the start, not the preview, makes the trip active.
• Owed Finish. A start does not displace a Finish record that the store does not hold yet.

The latest progress record already names the active trip (§7.7, next_trip_day), and the Metadata object keeps the records in write order. So a move gives a durable "last started trip" with no new format, field or request type.

Keep riding past the end

• A Finish after the rider arrived and rode on past the end (Arrival::RodeOn) owes, beside its record, the last fix and the next day (trip::RodeOn).
• The executor (host dispatch, board flat_store) reads the next day's route and projects the fix once with RodeOn::settle. Within 50 m (RODE_ON_MATCH_M) the position moves to the nearest point of that route, in the next day. Otherwise it stays at the end of the finished day.
• obc_route::nearest_along walks the route through one 112-byte block without a RouteIndex, so the board needs no second index.

Spec

obc-ble-interface-spec.md §7.7: the start record bytes and its move rule, the "Rode on" rule, and the "Active trip" rule.

Tests

• obc-formats: a_start_record_moves_the_stored_record_and_never_replaces_it.
• obc-storage: progress_records_survive_row_and_checkpoint_edits_and_a_remount writes a start record after a real record; the stored record comes back unchanged.
• obc-app: a_ride_on_a_day_of_another_trip_makes_that_trip_active (integration), a_start_moves_the_record_and_gives_a_trip_without_one_no_progress, a_ride_that_rode_on_moves_into_the_next_day_when_the_fix_is_on_its_route (pure rule: within 50 m, past 50 m, no line).
• obc-route: nearest_along_agrees_with_the_join_scan across chunk seams.
• The arrival tests' executor moves the store revision after each write and answers the setup passes.

Checks

• obc test -p obc-app (1,163 pass), -p obc-route (259), -p obc-storage (213), -p obc-formats (56), -p obc-host-core (132).
• cargo clippy -D warnings --all-targets on obc-app, obc-route, obc-storage, obc-formats, obc-host-core; cargo clippy --locked -D warnings in firmware/obc-fw-nrf54l. cargo fmt --all and cargo fmt in the board root.
• obc suites check: OK.
• Left out locally: obc shot --check (no frame changes; CI runs it), obc test affected (CI), the board release build and resource guard (CI's embedded job).

Hardware checks pending

• The Trips (device): ride a trip — progress on the line, the next day, and the day row on the start card #2069 DK session in the epic list. Suggested extra step: start a day of a second trip from its day list, then power-cycle; the start card's day row shows the second trip.

Public docs: no change (the contract specs/obc-ble-interface-spec.md changed).

Closes #2069

Requirements: none

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0388f8ca-39e5-407a-8eac-ea5a39a8b7d6

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[timohueser]

VERDICT: CHANGES

1. A start can overwrite a stored record with a blank one (blocking). firmware/obc-app/src/app.rs:1159-1168 builds the start record from the resident records. On the board, read_catalogs (firmware/obc-fw-nrf54l/src/ride.rs:135-143) loads routes and trips before load_metadata. When the metadata read fails, the trips are resident but the progress is empty (it is empty at boot). A start on a trip day then owes TripSummary::start(None). The write waits for the next complete read (pass.rs:495), that read loads the real record, and then the blank record replaces it in the store: the trip loses its finished days, dates and position. The test a_ride_on_a_day_of_another_trip_makes_that_trip_active shows the blank write for a trip without a resident record. Before this PR only a Finish used the resident copy. Now every start on a trip that is not last does. Fix: add a progress_loaded flag to MetadataMachine. set_progress sets it and reset_store clears it. note_trip_start returns while it is false. A better fix: make the start a store-side move (WriteProgress { start: true }: keep the stored record for the key, else write the blank one). That also removes the Revision exception in obc-storage/src/flat/metadata.rs:655.

2. Spec §7.7 is not bytes (specs/obc-ble-interface-spec.md:320-322). "A record without a position" does not say what gets written. Write: position day 0, day 0's route, Revision of that route, 0 m, no last finished day, all dates 0. "Unchanged" is also not exact. The device writes the record as it read it, so the metres are 0 when the route has another Revision now. Say "the fresh start of a ride", because a recovered continuation does not write.

3. Crash window (nit). Only SessionStart::Fresh writes (device_core/pass.rs:558). If power goes before the owed write lands, the continuation does not write it again. note_trip_start writes nothing when its trip is already last, so it can also run on a continuation after fix 1 is in.

Owner decision, not a code change. #2069 says "Opening a trip day from the day list also makes that trip active". The PR makes only the start do this. I agree with the PR: route_overview.rs:13,181 makes an open a preview, and Back restores the route that was loaded before. Please edit the issue so it matches the PR.

Verified OK. A torn write cannot lose records: the image is copy-on-write, and the old head goes in the same commit (flat/metadata.rs:343-420). Every Finish record has Revision 0 (trip.rs:297). The two executors are the only callers of write_progress, and the read sets metres to 0 when the Revision does not match. So old metres never get a new Revision. An owed Finish is never replaced, and a start and its Finish are always for the same trip. MAX_TRIPS is 16 and stored holds at most 16 keys, so a start never evicts a record of a live trip. A start writes the Metadata object at most once, and not at all when its trip is last. The harness changes are small and needed.

Ran: obc test -p obc-app (1162 pass), obc test -p obc-storage (213 pass), cargo clippy -p obc-app -p obc-storage --all-targets -D warnings clean.

[timohueser]

Fixes pushed (71efe93, cf482a3).

1. Blank start record (blocking): the store does the move. I took the store-side move, because it is simpler than a progress_loaded flag: it needs no new request field and no executor change. The start always writes a start record (day 0, day 0's route, 0 m, no finished day, no dates). The shared bound rule (obc_formats::trip_progress::record) never lets a record without a finished day replace one: the stored record of its key moves to the end byte for byte, and the start record goes in only when the key has none. The store and the resident list run the same function, so an unread store is safe. The Revision exception in write_progress is gone. Tests: a_start_record_moves_the_stored_record_and_never_replaces_it (formats) and the storage remount test, which writes a start record after a real record.
2. §7.7 bytes. The "Start record" and "A start record never replaces a record" bullets now name every field, the byte-for-byte move, the kept Revision (metres read as 0 when the route changed), and the fresh start plus the continuation.
3. Nit 3 taken. note_trip_start runs after both a fresh start and a continuation. It is one line, and it writes nothing when the trip is already last.

Also in this PR, as its own commit: the last code item of #2069, Keep riding past the end. The executor projects the last fix onto the next day's route once; within 50 m the position moves into the next day. The PR body now says "Closes #2069".

🤖 Generated with Claude Code

[timohueser]

VERDICT: CHANGES

Delta: 71efe93, cf482a3.

1. A Finish owed during a start write is lost (blocking). firmware/obc-app/src/metadata.rs:139-143, 124-127. While the start write is in flight, owe_progress puts the Finish in progress_owed. The start's ProgressWritten then clears it, and the re-read drops it from the resident list too. A quick Save from recovery can hit this. Fix: owe_progress sets writing_progress = false. Add one machine test.

2. nearest_along has no tie bias (blocking). firmware/obc-route/src/reader.rs:237 keeps only a strictly nearer point. When day N+1 is an out-and-back and its return leg is 3 m nearer the fix, the position goes near the end of day N+1. The live first lock takes the earliest point within TIE_EPS_M (8 m, matcher.rs:24). Use the same rule, and add an out-and-back case to the test. The test compares with RouteMatch::nearest(.., 0.0), so it misses this. The spec's "Rode on" bullet must name the rule.

Verified OK.

• The first review's finding 1 is closed. The resident list and write_progress both run trip_progress::record. set_progress replaces an unread list. The store moves its stored bytes and keeps their Revision.
• A Finish always has last_finished: Some (trip.rs:335), so the rule never treats a Finish as a start.
• §7.7 gives the bytes and has no history.
• Each write attempt projects once, in one pass. It reads 44 B of meta for each chunk and at most 112 B at a time. This matches Trips (device): ride a trip — progress on the line, the next day, and the day row on the start card #2069.
• nearest_along is not a duplicate. RouteMatch::nearest needs a RouteIndex of about 12 KB, which is too large for the store executor.

Ran: obc test -p obc-app -p obc-host-core -p obc-formats -p obc-route -p obc-storage (1823 pass) and clippy -D warnings (clean). The board build was not run because the DK is in use.

[timohueser]

Fixed in 5fb5eb1.

1. Finish owed during a start write. owe_progress now clears writing_progress, so the in-flight write's answer no longer clears the new record. Test: metadata::tests::a_finish_owed_while_a_start_write_is_in_flight_is_written_next.
2. Tie bias. nearest_along replaces the kept point only when a later one is more than TIE_EPS_M nearer. It uses the matcher's constant, now pub(crate). The test compares against RouteMatch::nearest(.., 8.0) and adds an out-and-back whose return leg is 5.6 m nearer than the outbound leg; the outbound leg wins. The spec's "Rode on" bullet names the 8 m rule.

obc test -p obc-route (259) and -p obc-app (1,164) pass; clippy is clean.

🤖 Generated with Claude Code