fix: v5.0.0 — TC18-conformant power-mode model + register-map config tables (BREAKING) - #135
Merged
Merged
Conversation
…tables (BREAKING)
Four independently-confirmed findings, each verified against the OPEN
Alliance TC18 v0.5.1_RC specification's own tables and figures — rendered
at 300/600 dpi where bit packing or table structure was in question, since
text extraction preserves field order but not bit widths.
rust-RCP-P01 (BREAKING, ASIL-B): cold-/hot-start mapping was inverted.
§12.4.1 (p.46): "a cold start (after power-on or wake-up from sleep) and a
hot start (=wake-up from StandBy)". try_cold_start accepted only
Unpowered->Normal, omitting the Sleep->Normal cold start; try_hot_start
claimed Sleep->Normal behind the WakeUp handshake, when TC18's hot start is
StandBy->Normal and it is the hot-start procedure that §12.4.1 attaches
that handshake to.
rust-RCP-P02 (BREAKING): is_power_mode_transition_defined accepted
StandBy<->Sleep, an edge Figure 17 does not draw, and omitted Normal->Sleep
("Go to Sleep"), which it does. Now exactly Figure 17's two "Go to" edges.
rust-RCP-P03 (BREAKING, wire): RequestStreamConfigEntry used the wrong row
layout. §12.7.7 Table 22 packs eight per-stream flags into the single
bit-addressed byte at 0x000D and closes each row with reserved blocks at
0x0012/0x0014, the next row's rx_stream_id2 at 0x0018 fixing the stride at
24 bytes. This crate gave each flag its own byte and dropped all six
reserved bytes (ENCODED_LEN 25), misplacing every field from 0x000D on.
The eight flags are now typed bool to match their 1-bit width.
rust-RCP-P04 (BREAKING, wire): EpByteBusIdMapEntry transposed EP_Nr and
BBID against §12.7.8 Table 23's own relative addresses.
rust-RCP-P05 (BREAKING, wire): SequencerStateEntry modeled one of the row's
two fields, dropping §12.7.10 Table 25's Request_stream_index — the
sequencer's access-control binding — and misaligning every row after the
first (ENCODED_LEN 1, should be 2).
Root causes addressed, not just symptoms: this module's provenance notes
asserted the specification gave "no explicit per-field bit-width or
byte-offset table" and "no textual basis for a specific bit-position
assignment" for these tables, which is false — Tables 22/23/25 each carry
explicit Relative address and Type columns. Those notes are corrected.
Separately, every affected encoder's tests asserted only
decode(encode(x)) == x, which cannot detect a wrong layout; each fix adds
literal expected-byte vectors laid out by hand from the spec's address
columns, chosen so a transposition or mis-sized row cannot pass.
.fusa-reqs.json REQ-PWR-002, REQ-PWRSTART-001/002 and REQ-RMAP-015..019,
024..026 baked the wrong behavior in as correct; all are rewritten with
citations to the real spec source.
Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four independently-confirmed conformance findings against the OPEN Alliance TC18 Remote Control Specification v0.5.1_RC. Each was verified against primary source before any code changed — and where the question was one of bit packing or table structure, against a 300/600 dpi render of the relevant PDF page rather than extracted text, since text extraction preserves field order but not bit widths.
v5.0.0(MAJOR) perdocs/SEMVER.md: three of the four are wire-format changes, and that doc makes wire-format correctness a MAJOR concern even when the change is a fix.The three register-map row types are not yet reachable from any live decode path (nothing in this crate performs register I/O against a real RC Server yet), so interop urgency is lower than
v3.0.0/v4.0.0. P01/P02 are a real behavioural bug in code callable today.rust-RCP-P01 — power-state cold/hot-start mapping was inverted (HIGH, ASIL-B)
Source: §12.4.1 "Power-On / Wake-Up / Start-Up behavior", p.46 — "There are two types of start-up: a cold start (after power-on or wake-up from sleep) and a hot start (=wake-up from StandBy)." Corroborated by §12.4 Figure 17's own arrow labels (see P02).
try_cold_startUnpowered -> NormalonlyUnpowered -> NormalandSleep -> Normaltry_hot_startSleep -> Normal, behind WakeUp handshakeStandBy -> Normal, behind WakeUp handshakeBoth origins were wrong, and the handshake was attached to the wrong path — §12.4.1 introduces it under "Hot-start-up procedure". Net effect on a caller wiring this up: every wake-from-sleep was rejected until a handshake the spec does not require there had completed, and every wake-from-standby was rejected outright, there being no path admitting that origin at all.
rust-RCP-P02 — undefined
StandBy <-> Sleeptransition, missingNormal -> SleepSource: §12.4 Figure 17 "power and operation modes", p.46 — a labelled state diagram with exactly five edges:
Unpowered -> NormalSleep -> NormalStandBy -> NormalNormal -> StandByNormal -> Sleepis_power_mode_transition_definedacceptedStandBy <-> Sleep— an edge the figure does not draw at all; it placesNormal/StandByin a "Powered" box andSleepin a separate "Only part of PHY powered" box, both low-power modes reached from and returning toNormalonly — and omittedNormal -> Sleep, which it does draw. Now exactly the two "Go to ..." edges, with the two wake-up edges owned by the start-up functions.rust-RCP-P03 —
RequestStreamConfigEntrywrong layout (wire)Source: §12.7.7 Table 22 "Request stream configuration", pp.57-58.
Table 22 addresses eight per-stream flags as
0x000D.0through0x000D.7— the only fields in the table given a.bitsuffix rather than a whole-byte address — so all eight share one byte. It then closes each row with a 16-bit reserved word at0x0012and a 32-bit reserved block at0x0014, with the next row'srx_stream_id2at0x0018fixing the stride at 24 bytes.This crate gave each flag a whole byte of its own and dropped all six reserved bytes, for
ENCODED_LEN = 25and a wrong offset for every field from0x000Donward.ENCODED_LEN25 → 24u8→bool(au8cannot round-trip losslessly through one bit)FLAGS_OFFSET, eightFLAG_*masks, and aflags_byte()accessorrust-RCP-P04 —
EpByteBusIdMapEntrytransposedEP_Nr/BBID(wire)Source: §12.7.8 Table 23 "EP_ID_config", p.59 —
Request_Stream_Indexat0x0000,EP_Nrat0x0001,BBIDat0x0002(16-bit).Encoder emitted
[stream_index, BBID_hi, BBID_lo, EP_Nr]. Row length was coincidentally right; the middle three bytes were not.rust-RCP-P05 —
SequencerStateEntrymodeled 1 of 2 fields (wire)Source: §12.7.10 Table 25 "SEQUENCER_config", p.61 — each sequencer is
Seq_stateat0x0000andRequest_stream_indexat0x0001, withSeq_2at0x0002fixing a 2-byte stride.The missing field is the sequencer's access-control binding: §12.7.10 states "Each sequencer is dedicated to a specific RC Client and its bound endpoints", and the field itself "refers the Client Nr allowed to access this sequencer". With
ENCODED_LEN = 1, a multi-sequencer table read throughdecode_rowsboth lost every sequencer's client binding and misaligned every row after the first.Root causes addressed, not just symptoms
False-scarcity provenance notes.
src/regmap.rs's "Config tables provenance note" asserted the specification recorded these tables' field names and purpose in prose only, with "no explicit per-field bit-width or byte-offset table" and "no textual basis for a specific bit-position assignment". That is false — Tables 22, 23 and 25 each carry explicit "Relative address" and "Type" columns, and Table 22 additionally gives0x000D.0-0x000D.7. Three of the five row types were laid out wrongly on the strength of that claim. The note is corrected, and each row type's doc comment now reproduces its table's address/width columns with a section and page citation.src/powerstate.rshad the equivalent problem: it derived its transition set by reading the four mode names as a depth ordering, which the spec never states, and that inference is what produced P01 and P02 both.Tautological tests. Every affected encoder's tests asserted only
decode(encode(x)) == x. A round trip through one's own encoder cannot detect a wrong layout — which is why all five findings survived. Each fix adds literal expected-byte vectors laid out by hand from the specification's address columns, not copied back from encoder output, with values chosen so a transposition or mis-sized row cannot pass:0x000Dand leaves all 23 other bytes zero (this alone would have caught P03), plus a reserved-bytes-ignored test[0x03, 0x04, 0x05, 0x55]— distinct value in every byte, so a transposition cannot passdecode_rowstest assertingSeq_2's state resolves at offset0x0002Requirements.
.fusa-reqs.jsonREQ-PWR-002,REQ-PWRSTART-001/002,REQ-RMAP-015/016/017/018/019/024/025/026all baked the wrong behavior in as correct. Every one is rewritten against the real spec source, with section, table and page cited inline.Verification
cargo test --all-features— 1092 pass, 0 fail (1060 lib + 32 bin)cargo build --release— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo fmt --check— cleanscripts/fusa-gap-check.sh— 564/564 requirements traced, no gapsscripts/hara_asil_check.py— 10 hazards match ISO 26262-3:2018 Table 4docs/PUBLIC_API.txtwas updated by hand (no nightly toolchain available locally, socargo public-apicould not regenerate it); please watch theapi-stabilityjob in particular. Public surface delta:RequestStreamConfigEntry's 8 flag fieldsu8 -> bool,encode -> [u8; 24], 10 new consts +flags_byte();SequencerStateEntrygainsrequest_stream_index,encode -> [u8; 2].Left for a follow-up (deliberately out of scope)
ResponseStreamConfigEntry's layout has not been checked against §12.7.9 in this pass and remains this crate's own inference — flagged as such in the module doc.RequestStreamConfigEntry::default()is still all-zero, where Table 22 documents a default of1forrx_resp_stream_index. Noted in the CHANGELOG rather than changed, as it is a distinct finding from the four here.