Skip to content

S2 — Move the answered legacy rows into Navigator, Settings, DFU, and StorageInfo #1485

Description

@timohueser

Parent epic: #1397

Program tracker: #1448

Program position: App core, revised S2

Start gate: DC7 #1440 is complete (merged via PR #1484). Met.

Goal

Give the four domains whose legacy answer is already terminal their own pending protocol state and
operation seam, so the pass — not Activity, not HostPending, not the drain — owns their
lifecycles.

This issue does not delete the legacy protocol and does not move any host onto App::run_pass.
Both belong to S6.

Read this first: the revised S2 row is stale

The #1397 amendment's revised S2 says S2 adds OperationToken, Capability, named external facts,
and one intent/effect/outcome enum per domain with named bounded slots. All of that has since
landed
— DC2 #1435 / PR #1468 (tokens, capabilities, ExternalFacts) and DC3 #1436 / PR #1475
(nine domain protocols, EffectSlots / OutcomeSlots). DC5 #1438 / PR #1481 added the fourteen-stage
coordinator, DC6 #1439 / PR #1483 the LegacyAdapter, DC7 #1440 / PR #1484 the conformance gate.

The original S2 row — "protocol slot table (non-storage rows); delete the four hand lists" — is also
not implementable as written: the amendment explicitly bans a global generated table
("One global table must not own all domain policy"), and the four hand lists cannot be deleted while
the production hosts still drain them, which is S6's cutover.

What is genuinely left is the sentence in the amendment that nothing has yet acted on:

Each domain owns its pending state.

Six domains still do not. device_core_conformance.rs names them in its own source — "the legacy
half, for the six domains without a machine"
: Navigator, Recorder, Settings, DFU, Bond,
StorageInfo
. Their pending state sits in Activity one-shot slots, HostPending, and two App
fields, and stages 7, 9 and 11 of the pass are empty because of it.

This issue takes four of the six. The scope line is below and it is not arbitrary.

Source of truth

  • Ownership rules and the domain list: #1433 §5 and §6.
  • Per-domain effect ownership: #1433 §7.2 (navigation) and §7.4 (settings and platform).
  • The pass order and the two delivery rules: firmware/obc-app/src/device_core/pass.rs module docs.
  • The legacy mapping for every variant: firmware/obc-app/src/device_core/migration.rs.
  • What the legacy side still owns and who deletes it: LegacyOwned in firmware/obc-app/src/device_core/compat.rs.

The scope line

LegacyReply::ALL in compat.rs lists the seven legacy classes that have a terminal
HostEvent answer. Those seven classes belong to exactly four domains:

Domain Reply classes Legacy commands it takes over Legacy events it takes over
Navigator RoutePlan, DetourPlan, DetourCommit PlanRoute, PlanDetour, CommitDetour, CancelRoutePlan, CancelDetour NavPlanned, DetourPlanned, DetourCommitted
SettingsMachine SettingsWrite PersistSettings SettingsPersisted, SettingsPersistFailed
DfuState DfuScan, DfuInstall Dfu DfuScanned, DfuInstallBegan, DfuInstallFailed
StorageInfo CardScan ScanCardFree CardScanned

Eight of the eighteen command classes and nine of the fifteen events.

#1433 §4.3 states the rule this follows: a domain that cannot validate a token cannot be the owner
of an outcome
. These four can validate one today, on the legacy transport, because the
legacy protocol already delivers their terminal answer. The two domains left out cannot:

Building a machine for Bond or Recorder in this slice would be a state machine with nothing to
validate and no answer to consume — the speculative structure this repository bans.

Current code

Everything below is at origin/develop after PR #1484.

The four hand lists (unchanged since #800):

List Location Size
HostCommand::DRAIN_ORDER firmware/obc-app/src/host.rs:262 18 entries
HostCommand::class() firmware/obc-app/src/host.rs:284 18 arms
App::peek_host_command firmware/obc-app/src/app.rs:3286 18 arms
App::drain_host_command firmware/obc-app/src/app.rs:3320 18 arms plus retention_expiry_command

The pending state the eight classes read (this is what moves):

  • Activity: nav_request, nav_cancel, detour_request, detour_commit, detour_cancel,
    dfu_request, card_scan_request — seven fields with a take_* / has_* / pending_* /
    clear_* family each (firmware/obc-app/src/activity.rs:246–880).
  • HostPending: settings_rev, retry_at_ms, persist, plus PersistState,
    retry_deadline_reached, SETTINGS_RETRY_BACKOFF_MS and seven methods
    (firmware/obc-app/src/host.rs:500–685).
  • App::card_free_bytes, written straight from apply_event.
  • The freeze edges note_plan_started / note_plan_ended, fired from inside
    drain_host_command's PlanRoute / PlanDetour / CancelRoutePlan / CancelDetour arms.
  • App::on_nav_planned / on_detour_planned / on_detour_committed in apply_event.

What already exists and must be used, not rebuilt:

  • navigator.rs (158 lines), settings.rs:2043–2090, dfu.rs:249–300,
    device_core/storage_info.rs — vocabulary only, no machine. navigator.rs's own header says the
    machine "arrive[s] in later slices".
  • CatalogState::admit_intent / next_effect / apply_outcome
    (firmware/obc-app/src/catalog_state.rs:721–776) — the precedent for the operation seam. Copy
    its shape: the seam lives beside the domain's existing state, owns the token and the in-flight
    count, and nothing else.
  • compat.rs's navigator_row, settings_row, dfu_row, storage_info_row — the translation each
    effect already has.
  • The nine EffectSlots / OutcomeSlots fields. No new slot field is needed.

The empty stages: stage_recorder (position only), stage_settings (records and returns),
stage_platform (records and returns), stage_navigator (handles ActiveRouteRemoved and the
activation deferral, nothing else).

Locked design

  1. Each of the four domains gets one operation seam: admit_intent, next_effect,
    apply_outcome, on the type that already holds that domain's state. No new module for
    Settings, DFU or StorageInfo. Navigator's machine lives in the existing navigator.rs.
  2. The seam is the only owner of that domain's pending state. After this slice a rider request
    exists in exactly one place. Two copies is the defect this whole epic exists to remove; do not
    leave the Activity slot behind "for the drain".
  3. peek_host_command and drain_host_command keep their eight arms, and each becomes a
    one-line delegation: ask the domain for its next effect, translate it with the existing
    compat.rs row. Translation only — no policy in the arm.
  4. DRAIN_ORDER, class(), HostCommand and HostEvent are not touched. The wire vocabulary
    is unchanged (EPIC — Device core: one product kernel, typed effects, thin platform executors #1433 §16). All four lists die at S6 with the protocol.
  5. Navigator owns the planning lifecycle: Idle → Planning → PreviewReady → Committing → Active,
    or Failed. It owns cancellation, replacement, and late-answer rejection. It keeps the two rules
    the current code hides in the drain:
  6. Navigator becomes the single writer of RerouteFreeze. reroute_freeze.rs, arena_gate.rs
    and link_gate.rs stay exactly as they are — S5 deletes them and derives the freeze from
    CoreMode. S2 replaces four scattered edge calls with one call site driven by a state change.
  7. App::on_nav_planned and friends stop being protocol handlers. What survives is the UI's
    reaction to a Navigator state change.
  8. No new deferred connection unless a stage genuinely needs to reach backwards. The wake ratchet
    in the conformance replay is the check.

Target files

firmware/obc-app/src/navigator.rs                 # + the machine
firmware/obc-app/src/settings.rs                  # + the seam, absorbing HostPending's persist half
firmware/obc-app/src/dfu.rs                       # + the seam
firmware/obc-app/src/device_core/storage_info.rs  # + the seam and the free-bytes level
firmware/obc-app/src/device_core/pass.rs          # stages 8, 9, 11 advance; stage 4 gains three intents
firmware/obc-app/src/device_core/connections.rs   # + the ui→navigator / ui→dfu / ui→storage intents
firmware/obc-app/src/activity.rs                  # − seven slots and their method families
firmware/obc-app/src/host.rs                      # − the persist half of HostPending
firmware/obc-app/src/app.rs                       # eight drain/peek arms become delegations
host/obc-host-core/tests/device_core_conformance.rs
host/obc-host-core/tests/device_core_corpus/mod.rs

Implementation steps

  1. Add NavigatorMachine's state and seam in navigator.rs, with its TokenSource<NavigatorTag>.
    Port the annihilation and per-family-release rules with their tests before touching Activity.
  2. Wire stage 8: take the UI's NavigatorIntent, advance, emit at most one NavigatorEffect into
    effects.navigator, consume outcomes.navigator at stage 1. Keep the existing
    ActiveRouteRemoved and activation handling.
  3. Make Navigator drive RerouteFreeze from its own transitions, and delete the four
    note_plan_started / note_plan_ended calls in drain_host_command.
  4. Move the settings persist handshake out of HostPending into settings.rs behind
    SettingsIntent / SettingsEffect::PersistRevision / SettingsOutcome. HostPending is left
    holding only the store-changed counter; fold it into App if nothing else remains.
  5. Add the DFU seam in dfu.rs: the single most-recent-wins phase slot, the scan and install
    admissions, and the deferral the remote BLE door already relies on
    (App::open_remote_dfu_check).
  6. Add the StorageInfo seam: the request level and the free-bytes result, replacing
    App::card_free_bytes and Activity::card_scan_request.
  7. Rewrite the eight peek_host_command / drain_host_command arms as delegations through the
    existing compat.rs rows.
  8. Delete the seven Activity slots and their method families, and the HostPending persist half.
  9. Extend the conformance harness: serve_mailbox's comment and its refusal set now cover two
    domains, not six; the_pass_owns_the_classes_it_took_over gains the eight classes.
  10. Re-measure every resource value against the DC7 baseline and itemize any increase.

Deletions

This slice must remove, in the same PR that replaces them:

  • Activity::nav_request, nav_cancel, detour_request, detour_commit, detour_cancel,
    dfu_request, card_scan_request and every take_* / has_* / pending_* / clear_* method
    reaching them.
  • HostPending::settings_rev, retry_at_ms, persist; PersistState; retry_deadline_reached;
    SETTINGS_RETRY_BACKOFF_MS; note_settings_edited; reset_settings_clean;
    settings_persist_ready; drain_settings_persist; on_settings_persisted;
    on_settings_persist_failed; arm_settings_save.
  • App::card_free_bytes and its apply_event arm.
  • The four note_plan_started / note_plan_ended call sites inside drain_host_command.
  • The protocol halves of App::on_nav_planned, on_detour_planned, on_detour_committed.

LOC honesty. #1397 §3.1 budgets Area C at −195 production lines. That number described the
generated slot table the amendment cancelled. This slice is expected to be roughly flat to +250:
it moves state rather than removing a mechanism, and the deletion — the four hand lists, the legacy
enums, compat.rs, migration.rs, feeders.rs — lands at S6. A PR that claims a net deletion here
is probably leaving a second copy of some pending state behind. Update the epic's Area C row rather
than bending the slice to it.

Required tests

Port before deleting, per the epic's rule. Focused, not exhaustive.

Navigator

Settings

  • No emit while the rider is inside the settings subtree; one emit on exit.
  • A stale ack (a newer edit bumped the revision) does not clear the newer state.
  • A failed write keeps the revision dirty, backs off, and retries once.
  • A host that drains but never acks parks without re-emitting.

DFU

  • The single phase slot is most-recent-wins; the remote door defers instead of replacing.
  • A scan failure and an install failure both reach the rider's terminal card.

StorageInfo

  • The request is an idempotent refresh; CardScanned { free_bytes: None } is a reported failure,
    not a retry loop.

Conformance (obc-host-core)

Resource limits

Baseline is DC7's, recorded in PR #1484 and firmware/tools/resource_baseline.json. Report every
value; itemize any increase.

Item Baseline Rule
size_of::<App>() (target) 50,904 B Must not grow. Seven Activity slots leave; four token generations arrive.
Board resident (.bss + .data) 304,808 B Must not grow.
size_of::<EffectSlots>() 160 B Unchanged — no new slot field.
size_of::<OutcomeSlots>() 224 B Unchanged.
Largest domain effect 56 B (NavigatorEffect) Ceiling 96 B.
Largest domain outcome 96 B (DfuOutcome) Ceiling 96 B.
Flash 1,448,904 B Ceiling 1,524,676 B.
Residual main stack 54,616 B Floor 38,808 B.
Guarded poll frame 9,792 B Limit 12,288 B.
Main task future 1,100 B Limit 8,192 B.
Scratch arena (USB / render / nav) 131,072 / 131,072 / 97,344 B Unchanged.
Replay wake profile 366 passes, 6 immediate wakes Must not rise.
Mean / worst pass time 0.215 µs / 5.50 µs Ratcheted in the committed test.

No allocator, no new task, no combined Effect/Outcome/Intent enum, no event bus (#1433 §14, §16).

Verification

obc test -p obc-app
obc test -p obc-host-core
obc test fixtures -p obc-host-core
cargo clippy -p obc-app -p obc-host-core --all-targets -- -D warnings
obc check fmt device board

Board build and resource gate:

cd firmware/obc-fw-nrf54l
cargo build --release --locked
python3 ../tools/resource_guard.py board --profile default --elf target/thumbv8m.main-none-eabihf/release/obc-fw-nrf54l
cargo build --release --locked --features resource-report
python3 ../tools/resource_guard.py report --profile default --elf ...

Run obc suites check if test sources or the registry move. obc test full / obc check full only
if CONTRIBUTING.md requires it — this change is confined to obc-app and its conformance tests.

Explicitly out of scope

Definition of done

Open questions for the epic owner

  1. Should the three small domains ship here or at S6? Only Navigator is a hard precondition for
    S5. Settings, DFU and StorageInfo are the same shape and are cheap to do now, but if capacity is
    short they can be deferred to S6 without blocking anything. Deferring means their pending state
    stays hand-unrolled through S5.
  2. One PR or two? Navigator alone is the load-bearing half and the riskiest. A Navigator PR
    followed by a "three small seams" PR is the safer split.
  3. The epic's Area C LOC row (−195) should be rewritten to describe this slice, and the amendment's
    revised-S2 paragraph should be replaced with this scope — it currently describes work that DC2 and
    DC3 delivered.
  4. store_writable cannot come back down (pass.rs stage 12): ExternalFacts has no unmount
    fact, so a pulled card leaves catalog mutation asserted. That is a fact-vocabulary gap, not a
    stage-12 gap. Does it belong here, in S5 with CoreMode, or in S6 with the store executor?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions