Skip to content

T2 — Generate the settings codec from one settings_table! declaration #1506

Description

@timohueser

Parent epic: #1399

Program tracker: #1448

Program position: Settings workstream, slice 2 of 8. An independent codec slice under the Gate 3
rule "Independent baselines, codecs, formatters, and pure algorithms can start earlier when their
local gate permits them." This is #1448 §2.3's planning-ready row, refined.

Start gate: open — no upstream work is required.

This slice touches no DeviceCore type, no effect, no screen, and no persistence policy. It does not
touch SettingsMachine (S2 #1485 / PR #1486), which owns the save handshake and shares the file but
not one line of the codec. Gate 2's frontier (S6b/S6c) does not block it. Two sibling boundaries,
neither blocking:

Goal

Replace the hand-written half of firmware/obc-app/src/settings.rs's blob codec — the Settings
struct body, Settings::DEFAULT, the 22-link offset chain, encode, decode, sanitize, and
adopt_ble_fields — with one settings_table! declaration of one row per persisted field.

Today those seven things restate the same 22 facts seven times, in seven syntaxes, in declaration
order that nothing enforces. #1399 §1.3 counts the cost: of the 13 edit sites a new setting needs,
sites 2 through 8 are this file, and they are pure transcription of a row that site 1 already
declares (T1 #1466 turned site 1 into a setting_enum! row).

After this slice a persisted field is one table row, and the codec is derived from it.

This slice does not change the version gate, the stored bytes, the blob length, any screen, or any
behavior. encode output must stay byte-identical — and unlike T1, that is not a claim needing new
evidence: T1 committed the golden arrays this slice is measured against.

Read this first: what has drifted since #1399 was written

The epic's T2 row is sound in substance. Four premises around it have moved, and one is good news.

1. "Golden v16 blob committed" is already done. The row's second clause was T1's work in the
end: encode_matches_the_golden_blobs (settings.rs, test module) pins encode(&Settings::DEFAULT)
and encode(&every_field_set()) as two committed 176-byte literals, captured from develop
@ b44b9abc before any macro existed. T2 does not write a golden blob. T2's gate is that those
two arrays are untouched in the diff and still pass
— a stronger and cheaper gate than the row's
"encode-output byte-diff empty", because the reference bytes predate both macros.

2. The file grew, but not in T2's span. settings.rs is 2,422 lines at origin/develop
@ 6725478f, not #1399's 2,268: T1 took production 1,290 → 992, and S2 #1485 appended
SettingsMachine (lines 2035–2307, 273 production lines) plus its own test module. Current split is
1,265 production / 1,157 test. None of the S2 code reads encode, decode, Settings::DEFAULT,
or an offset const — it carries a revision, an operation token, a persist state and a retry deadline,
and is pinned at ≤ 12 bytes. T2's premise is intact; only the epic's §1.1 size table is stale.

3. The test win is bigger than §3.1 estimated; the production win is not there at all. §3.1 line
B says −40 production lines. It will not hold, for exactly the two reasons line A did not (see the
re-base comment on this epic): the doc comments are preserved verbatim, and the generator is larger
than assumed. 247 of the 482 lines in T2's span are doc or comment. The honest budget is below —
production ≈ +20 (band −20 … +60), tests ≈ −280. Against that, §3.1's "eight copied per-field
tests" undercounts: there are thirteen, worth ~520 test lines, and the table makes one property
test able to replace their codec halves.

4. One amendment sentence is already overtaken. #1399's amendment says SettingsMachine owns "the
live settings value". The merged machine owns the handshake only; the value still lives in App. Not
this slice's business — noted so a later T-slice does not plan against a sentence the code answered
differently.

Nothing here changes the row's content. T2 is still "settings_table!; generated codec byte-identical
to v16". It is re-scoped, not re-aimed.

Should this be its own slice?

Yes, and the reason is not the line count — same as T1.

T2 is LOC-neutral in production and rider-invisible by construction. Its value is that it is the
only slice that can prove byte-identity in isolation.
T3 changes what decode accepts; if T3 also
introduced the generator, a moved byte would have two possible authors. Separating them means the
committed golden arrays are the whole review of T2, and T3's diff contains only semantics.

The marginal-cost claim is also settled here, not in T3: sites 2, 3, 5, 6, 7 and 8 of #1399 §1.3 die
in this slice (site 4, the VERSION bump, dies in T3). That is 7 of the 9 transcription sites, ~53
lines and 37 test lines per new setting, gone.

If the epic owner would rather see fewer PRs, the defensible merge is not T2+T3 but splitting T3:
its weather_alert_marks record split touches the SettingsStore port, both adapters and App,
and has nothing to do with tail-defaulting decode. See open question 3.

Source of truth

Architecture and acceptance: #1399 §2.1, §2.2 and §3.2.

Slice row: #1399 §5, row T2 — "settings_table!; generated codec byte-identical to v16; golden v16
blob committed. Gate: encode-output byte-diff empty."

House precedent for the macro's form: setting_enum!
(settings_enum.rs,
207 lines) and the screens! table it followed — "deliberately a dumb token-pasting table, not a
framework."

Precedent for what a generated compile-time assert may claim: PR #1472's review found the
per-variant discriminant asserts tautological — a macro comparing a value against the literal that
declared it cannot fail. That lesson binds this slice (see Locked decisions, offsets).

Target files

Create firmware/obc-app/src/settings_table.rs — the settings_table! macro, the SettingCodec
trait, and its impls for every blob field kind. Register it in lib.rs as mod settings_table;
(private, like settings_enum).

Edit firmware/obc-app/src/settings.rs — the declaration replaces the struct body, DEFAULT,
the offset chain, encode, decode, sanitize and adopt_ble_fields.

Edit firmware/obc-app/src/settings_enum.rs — one added line so a declared enum also gets its
SettingCodec impl.

Edit firmware/obc-app/src/lib.rs — module registration.

No other file changes. encode, decode and ENCODED_LEN keep their signatures and values, so
apps/obc-sim/src/settings_store.rs, firmware/obc-fw-nrf54l/src/settings.rs (SLOT_LEN), and the
two harness round-trips are untouched. Do not create a crate. Do not convert settings.rs into a
directory module. Do not touch retention.rs, stat_fields.rs, weather_alerts.rs, or any screen.

Current shape (measured on origin/develop @ 6725478f)

The span this slice replaces, split by content:

Span Lines Doc + comment Code
Settings struct declaration (500–638) 139 113 25
size_of assert (639–642) 4 3 1
impl Default (644–648) 5 0 5
Settings::DEFAULT (650–682) 33 7 26
adopt_ble_fields (704–713) 10 4 5
sanitize (715–721) 7 0 7
VERSION + its history doc (723–738) 16 15 1
length + offset consts (740–794) 55 31 23
encode (796–871) 76 21 55
decode (873–955) 83 36 46
decode_alert_marks (957–978) 22 3 18
decode_saved_sensors (980–992) 13 0 12
Total 482 247 228

local_clock (684–702) is not in scope: it is calendar logic, not codec.

Three structural facts that make the generator small — each verified, not assumed:

  1. The blob is already a strict running sum of field lengths in declaration order. Byte 0 is
    VERSION, byte 1 units, byte 2 the frozen retired gps_time tombstone, then clock (6 B),
    utc_offset_min (2), fix_interval_s (2), power_saver (1) → STAT_FIELDS_OFF = 14, and so on
    to PAYLOAD_LEN = 168, CRC at 168/169, ENCODED_LEN = 176 (11 RRAM lines) with 6 B of zero pad.
    Summing the declared lengths reproduces every current offset exactly. Byte-identity is therefore a
    structural property of the generated chain, not a coincidence to be re-checked field by field.
  2. There are six field kinds, not twenty-two. u8-backed enum (8 fields: the seven
    setting_enum! types plus ride_retention), bool (5), raw u8 (1), LE u16 (2), LE i16 (1),
    and composite (5: clock, stat_fields, device_name, saved_sensors, weather_alert_marks),
    plus one reserved(1) row for byte 2. The composites already have hand-written codecs;
    two of them (decode_alert_marks, decode_saved_sensors) are already named functions and their
    encode halves are inline loops in encode.
  3. sanitize is three ranges and one hook. utc_offset_min, fix_interval_s and stat_cycle_s
    clamp to existing MIN/MAX consts; clock.sanitize() is a hook; stat_fields and
    device_name sanitise inside their own decode; bike_profile_idx is deliberately never
    clamped
    (its doc says why — a stale index resolves to profile 0 at plan time).

Locked decisions

The table

One row per persisted field, in blob order, carrying: doc comments, name, type, default expression,
and optional markers.

settings_table! {
    pub struct Settings {
        /// Metric or imperial readouts.
        units: Units = Units::Metric, ble_writable;
        reserved(1);                                  // the retired `gps_time` byte (#641)
        /// The last time source's **UTC** set-point …
        clock: DateTime = DateTime::DEFAULT, sanitize_with = DateTimeEditorExt::sanitize;
        /// Local time's offset from UTC, in minutes (`+02:00` → `120`).
        utc_offset_min: i16 = 0, range = UTC_OFFSET_MIN..=UTC_OFFSET_MAX;/// The per-class weather-alert marks (WX12) …
        weather_alert_marks: AlertMarks = [None; ALERT_CLASSES];
    }
}

The macro accepts $(#[$m:meta])* per row, so every existing field doc survives verbatim — 113
of the 139 struct lines are doc, and they are design record, not this slice's target. Same rule as
T1.

Generated from it: the Settings struct, Settings::DEFAULT (const, arbitrary const expressions
in the default column), the offset consts, encode, decode, sanitize, and adopt_ble_fields
(from ble_writable — today units, device_name, weather_refresh).

Not generated: local_clock, Units' conversions and labels, DeviceName, SavedSensor, the
DateTimeEditorExt sanitiser, and the VERSION const with its history doc (T3's material).

bike_profile_idx's no-clamp rule is expressed as the absence of a range column, and its field
doc — preserved verbatim — states the reason. No no_clamp marker: a marker that means "do nothing"
is decoration.

Field kinds go in a trait, not in macro arms

pub(crate) trait SettingCodec: Sized {
    const LEN: usize;
    fn write(&self, dst: &mut [u8]);
    fn read(src: &[u8]) -> Self;
}

Impls for u8, bool, u16, i16, DateTime, DeviceName, StatFieldList,
[SavedSensor; SENSOR_SLOTS], AlertMarks and RideRetention (through its existing
as_u8/from_u8#1398 owns that enum), plus one generated impl per setting_enum! type. All of
them live in settings_table.rs, so the blob's kinds are readable in one place.

Offsets stay const: const OFF_n: usize = OFF_prev + <Ty as SettingCodec>::LEN; — associated
consts are stable in const context, which is what makes this work without a const fn in a trait.

The alternative — a codec column with one macro arm per kind — is rejected. T1's macro hit exactly
that wall: an optional group wrapping a repeating group does not expand, which forced three entry
arms delegating to a shared core and took the macro from ~100 to 210 lines. Six kinds would multiply
that. Expect the same expansion hazard and design around it from the start. With the trait, the
macro emits a struct, a const, a chain of const offsets, and two loops of field.write(…) /
Ty::read(…).

Offsets are pinned by hand-written literals

#1399 §3.2 risk 2 asks the generator to "emit a static assert per field pinning its offset". Taken
literally that repeats PR #1472's tautology: an assert generated from the same token that produced
the value cannot fail. Instead, settings.rs carries a hand-written block of literals, transcribed
from today's develop values:

const _: () = assert!(STAT_FIELDS_OFF == 14 && NAME_OFF == 29 && BLE_OFF == 78 && … && PAYLOAD_LEN == 168);

Written independently of the chain, these are a real gate: a reordered row, a wrong LEN, or a
changed composite size stops the build. Twenty-two literals plus PAYLOAD_LEN and ENCODED_LEN.
Whatever the block's final form, the report must show that an edit to the table's order actually
fires it
— the same demonstration PR #1472 gave for the contiguity assert.

decode keeps its version gate

b[0] != VERSION → None is unchanged, and so is the CRC check, the sanitize call, and every
per-field clamp comment. Inverting the gate is T3's whole content; doing any of it here would put a
behavior change inside the slice whose only claim is that nothing changed.

No since column yet

#1399 §2.1's sketch carries a since: column. Nothing in T2 reads it, and T1 refused a column
exactly one consumer would use on the speculative-capability rule. T3 adds the column in the same
edit that generates PAYLOAD_LEN(v) from it. See open question 1 — there is a real argument the
other way.

Implementation steps

  1. Add firmware/obc-app/src/settings_table.rs: the SettingCodec trait and its impls for the
    scalar kinds (u8, bool, u16, i16). Register the module. Document it in the voice of
    settings_enum.rs.
  2. Add the composite impls, moving code rather than rewriting it: DateTime (bytes 3–8),
    DeviceName (length byte + 48, keeping the past-the-cap clamp), StatFieldList (length byte +
    MAX_STAT_FIELDS, delegating to its own encode/decode), [SavedSensor; SENSOR_SLOTS] and
    AlertMarks (the two block loops from encode plus the two existing decode_* functions,
    including the ALERT_MARK_PRESENT / ALERT_MARK_HAS_POS flag-pair rule).
  3. Generate SettingCodec from setting_enum! (one line, using the from_byte T1 made uniform and
    const), and hand-write the one impl for RideRetention.
  4. Add settings_table!: struct + DEFAULT + offsets only. Land it with encode/decode still
    hand-written and confirm the golden blobs still pass — this isolates the layout from the codec.
  5. Generate encode and decode from the table; delete the hand-written bodies and the offset
    chain. Keep every per-field decode comment on its row.
  6. Generate adopt_ble_fields from ble_writable and sanitize from range / sanitize_with.
    Delete both bodies.
  7. Add the hand-written offset-literal assert block. Demonstrate it fires on a deliberately swapped
    row, then revert.
  8. Collapse the test module (below), and report the LOC, size_of, and cargo size deltas.

Do not touch VERSION, the version gate, weather_alert_marks' record placement, any screen, or
local_clock.

Required tests

The two golden arrays in encode_matches_the_golden_blobs must appear untouched in the diff and
must pass.
Re-capturing them is the one thing that would void this slice; a reviewer should be able
to confirm it by reading the diff alone.

Everything else in the settings test module either passes with no edited assertion or is deleted
with its coverage demonstrably moved. Specifically:

  • Replace the thirteen copied per-field codec testsweather_refresh_*, up_ahead_source_*,
    ride_retention_*, ble_enabled_*, climb_mode_*, idle_return_*, map_overlays_*,
    map_contours_*, bike_profile_idx_*, waypoint_mode_*, language_*, saved_sensors_*,
    device_name_codec_and_sanitising (~520 lines) — with one table-driven test that, for every
    declared field, round-trips a non-default value (the every_field_set() fixture already exists and
    already feeds the golden blob) and asserts the device-only / BLE-writable split.
  • Keep every semantic assertion those tests also carryup_ahead_source_cycles_and_scopes…,
    idle_return_timeout_and_stepping, language_stepping_and_cycling, the saved-sensor migration
    case, and bike_profile_idx's deliberate no-clamp. Where such an assertion lives inside a deleted
    test, move it verbatim into the neighbouring semantic test. The report must map each deleted test
    to the assertion that now covers it.
  • One re_stamp_crc(&mut blob) helper replaces the ten copied "re-stamp the CRC so only the
    payload byte is wrong" blocks.
  • Keep codec_round_trips, codec_round_trips_default, codec_rejects_bad_blobs,
    codec_sanitises_stat_tail, decode_sanitises_out_of_range_fields,
    old_gps_time_byte_is_ignored_on_decode, const_default_matches_every_field_default, and every
    DateTime / adopt_ble_fields test verbatim.
  • Compile-time: the offset-literal block. No trybuild case — a wrong offset failing the normal
    build is stronger.

Do not add a test per generated accessor. The generator is one implementation; test it once.

Resource limits

  • encode output byte-identical: both committed golden arrays unedited and passing.
  • VERSION = 16, PAYLOAD_LEN = 168, ENCODED_LEN = 176, and every field offset unchanged — pinned
    by the hand-written literal block.
  • size_of::<Settings>() = 184, unchanged; its existing assert stays.
  • Settings::DEFAULT stays a const in .rodata (the WX12 — Implement route-aware wind, ride decisions and deduplicated weather alerts #1197 guarantee — the board's object store is
    built from that image, and a stack temporary here is the 2026-08-03 STKOF fault class).
  • cargo size on obc-fw-nrf54l (release, default features): .text + .rodata not larger. A
    trait-dispatched codec can cost flash where a straight-line pack cost none; growth is a review
    failure and the number is reported either way.
  • firmware/tools/resource_baseline.json unchanged: no resident-RAM movement is expected, and any
    would be a defect in this slice.
  • No new dependency, no alloc, no runtime allocation, no unsafe.

Line budget — stated up front, not reported against

#1399 §3.1 line B estimates −40 production lines. It will not hold. The measured span is 482
lines of which 247 are doc or comment that this slice preserves verbatim, leaving 228 code lines
to delete, and roughly 50 of those (the two composite block codecs) move into SettingCodec impls
rather than disappearing.

Lines
Removed: struct fields, DEFAULT, Default, adopt, sanitize, offsets, encode, decode ~−193
Moved: the two block codecs into trait impls (net, with wrappers) ~+10
Added: settings_table! ~+110
Added: SettingCodec + eleven impls (excluding the moved block bodies) ~+60
Added: 23 table rows, three range columns, offset-literal asserts ~+50
Net production ≈ +20 (band −20 … +60)
Tests: thirteen per-field tests → one property test + CRC helper ≈ −280
Net repo ≈ −260

Line B should re-base −40 → ≈ +20 when this closes, moving the epic's ≈ −520 total to ≈ −460.
Say so plainly in the closing comment rather than reporting against a target the slice cannot hit —
and if the implementation lands materially better or worse than the band, say that instead.

The justification is where the issue put it: 7 of the 9 transcription sites for a new setting die
here
, and T3 gets a generated offset chain to derive PAYLOAD_LEN(v) from. The LOC win in this
epic is in the screens (line C), not the codec.

Verification

obc test -p obc-app
cargo clippy -p obc-app --all-targets -- -D warnings
obc test -p obc-sim
obc suites check
cargo fmt --all

Board leg (flash, and the store that reads the blob):

obc check board
cd firmware/obc-fw-nrf54l && cargo size --release --bin obc-fw-nrf54l -- -A
cargo fmt --manifest-path firmware/obc-fw-nrf54l/Cargo.toml

No --png renders: this slice changes no draw code and no screen file. If any render moves,
something in scope was wrong.

No docs: commit: nothing user-visible changes. ui.md's six-groups drift and its persistence
section are #1399 §1.8's, owned by T3 (the migration rule) and T8 (the group count).

Definition of done

  • settings_table! and SettingCodec live in firmware/obc-app/src/settings_table.rs; the
    settings blob has one codec.
  • Settings, Settings::DEFAULT, every offset const, encode, decode, sanitize and
    adopt_ble_fields are generated from one declaration; the hand-written bodies are deleted in
    the same PR.
  • Every field doc comment survives verbatim.
  • The two golden arrays are untouched in the diff and pass; VERSION, PAYLOAD_LEN,
    ENCODED_LEN and all 22 offsets are unchanged and pinned by hand-written literals.
  • The offset-literal block is demonstrated to fire on a swapped row (shown in the PR, reverted).
  • Thirteen per-field codec tests are one table-driven test; the report maps each deleted test to
    its replacement assertion; no surviving assertion is edited.
  • The ten CRC re-stamp blocks are one helper.
  • size_of::<Settings>() = 184; Settings::DEFAULT is still a const.
  • cargo size on the board crate is not larger; the delta is reported.
  • retention.rs, stat_fields.rs, weather_alerts.rs, SettingsMachine, the version gate, and
    every settings screen are untouched.
  • The report lists the exact commands run, the production and test LOC deltas, and the
    cargo size delta.
  • EPIC — Settings & stat fields: one table, one codec that migrates, one formatting vocabulary #1399 §3.1's line-B estimate is re-based in a comment on the epic, and §1.1's size table is
    corrected for T1 + S2 in the same comment.

Open questions for the epic owner

  1. since column now or in T3? Locked above as "in T3", on T1's code-column precedent — a
    column nothing reads is the shape this repo refuses. The argument the other way is real: the
    VERSION doc comment already carries the per-version history as 15 lines of prose, and a since
    column would turn that prose into data in the same edit that moves the fields. Say the word and it
    goes in T2 as a documented column with no generated consumer.
  2. SettingCodec as a trait. It adds one small abstraction to obc-app to keep the macro dumb.
    The alternative is a codec column with six macro arms, which is where T1's expansion wall lives.
    Locked as the trait; if you would rather have zero new traits and a bigger macro, that is a taste
    call, not a technical one.
  3. T3's shape, decided here or there. T3's row bundles two unrelated things: tail-defaulting
    decode (which consumes this slice's generator) and splitting weather_alert_marks into its own
    record (which touches the SettingsStore port, both adapters, and App's alert writer). They
    share no code. Recommend splitting them when T3 is refined; T2 is unaffected either way.
  4. Thirteen tests deleted, not eight. §3.1 assumed eight. If any of the thirteen is load-bearing
    for a reason the test name does not show, name it now and it stays verbatim.

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