feat: v0.107.0 — enforce TC18 Table 30's Row-2 evt[2:0] rule for ADC - #153
Merged
Conversation
Points to RELAY's docs/RCP-ARCHITECTURE.md (the canonical architecture shared with go-RCP/cpp-RCP/rust-RCP), with this repo's file-path mapping and current conformance status against it. c-RCP is the reference implementation for the per-function requirement-tagging convention, the .fusa-reqs.json tc18/scope/status fields, and the conditional-request req-id grouping scheme. No code change. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
Found during a cross-repo architecture inventory (RELAY
docs/RCP-ARCHITECTURE.md, task tracking c-RCP's Table 30 centralization):
rcp_ep_adc_decode_read_request() never checked evt[2:0] at all. Per TC18
§13.5 Table 30 (TC18.txt:3666-3710), the {ADC, PWM_IN, I2C, LIN, CAN,
UART, ISELED, MDIO} endpoint-type row requires evt[2:0] = 000b for a
plain request; every other value is either reserved (001b-110b, reject
with UNSUPPORTED_CMD) or selects a differently-shaped configuration-write
request (111b, §12.7.1) that a plain read decoder must not accept.
ADC's own doc comment already said "the ACF header's evt field is always
encoded/decoded as 0" -- true for the encoder, never enforced on decode.
Adds rcp_acf_evt_row2_is_plain() (acf.h/acf.c) as the shared primitive
this endpoint-type row's rule should be checked with -- the actual
centralization the architecture doc calls for -- and wires it into
ADC's decode_read_request as the first of 7 affected endpoint types.
I2C/UART/ISELED/MDIO need the same wiring (tracked); CAN/LIN each
invented a *different*, spec-incompatible use of evt[2:0] (frame-format
selection, comparison-mode selection) and need a separate redesign
moving that behavior into their existing functional-config structs
instead -- not fixed here.
rcp_ep_adc_decode_read_request() is not currently wired into any real
server dispatch loop (confirmed: no caller in src/server.c/src/mock.c),
so this has no observed behavioral impact on any live path today, but is
a real conformance defect any future dispatch wiring would inherit.
Mutation-tested: reverting the evt check makes the new rejection test
fail (decode incorrectly succeeds on a reserved evt[2:0] value).
New public API (RCP_EP_ADC_ERR_BAD_EVT, rcp_acf_evt_row2_is_plain) and a
real, if not-yet-live, decode-behavior tightening -- MINOR bump.
Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
This was referenced Aug 1, 2026
SoundMatt
added a commit
that referenced
this pull request
Aug 1, 2026
…REAKING) (#155) * feat: v0.108.0 — enforce TC18 Table 30's Row-2 evt[2:0] rule for I2C/UART/ISELED/MDIO Completes the Table 30 centralization started in #153 (ADC): none of I2C's decode_transfer_request, UART's decode_write_request/ decode_read_request, ISELED's decode_command_request, or MDIO's decode_read_request/decode_write_request checked evt[2:0] at all, despite every one of their own encoders already hardcoding it to 0 ("no channel selector" per each module's own file header). Per TC18 §13.5 Table 30, this endpoint-type row's only legal evt[2:0] value for a plain request is 000b; every other value is either reserved (reject with UNSUPPORTED_CMD) or selects a differently-shaped configuration-write request (111b, §12.7.1) these decoders must not accept. All five now call the shared rcp_acf_evt_row2_is_plain() primitive (acf.h/acf.c, introduced in #153) at the same point in their validation sequence ADC already established. None of these decode functions are wired into any real server dispatch loop today (same finding as #153), so no observed behavioral impact on any live path -- but each is a real conformance defect any future dispatch wiring would inherit. Mutation-tested per endpoint: reverting each evt check makes exactly its own new rejection test(s) fail, and no others. Remaining Table 30 work, tracked separately: CAN and LIN each invented a different, spec-incompatible use of evt[2:0] (frame-format selection, comparison-mode selection) and need a real redesign, not this same additive fix. New public API (5 new error variants) and real decode-behavior tightening across 5 modules -- MINOR bump. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> * fix: v0.109.0 — CAN FrameFormat lives in the payload, not evt[2:0] (BREAKING) Verified directly against TC18 §13.7.11.3 Figure 39 (rendered from the source PDF at 400dpi, not just the text extraction): FrameFormat is the payload's leading quadlet's top 3 bits, packed with a 29-bit CAN ID (right-aligned for an 11-bit base id, per the same section's own note) -- NOT evt[2:0], which for CAN (a member of TC18 §13.5 Table 30's {ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO} row) has the same ordinary meaning every other endpoint in that row gives it: 000b plain, 111b configuration-write, else UNSUPPORTED_CMD. This module's own doc comments candidly admitted the evt[2:0] packing was invented, modeled on ep_spi.h's real, TC18-sanctioned evt[2:0] channel-selector row -- without checking that CAN belongs to the *other* Table 30 row, whose evt[2:0] values TC18 reserves. REQ-CANEP-031 had even formally catalogued the wrong claim as "implemented". Found while investigating this repo's Table 30 centralization (task tracking: CAN/LIN evt[2:0] non-conformance) and confirmed by rendering the actual spec figure rather than trusting the existing design's own reasoning. Fix: pack frame_format into the leading quadlet (write_prefix/read_prefix), leave evt at 0 everywhere this module encodes a request/response, and check evt[2:0] via the shared rcp_acf_evt_row2_is_plain() primitive (acf.h/acf.c, from #153/#154) on decode. Adds RCP_EP_CAN_ERR_BAD_EVT and RCP_EP_CAN_ERR_BAD_ARBITRATION_ID (the latter enforcing Figure 39's own "11-bit id shall be right aligned" rule, previously unchecked). rcp_ep_can_decode_frame_response_fragment() no longer outputs frame_format (it isn't determinable per-fragment once it moves into the payload) -- rcp_ep_can_decode_reassembled_frame_response() now derives it from the reassembled buffer instead of taking it as an input, the only place it's actually recoverable after fragmentation. Golden-vector tested: a hand-computed leading-quadlet value ((1<<29)|0x01ABCDEF = 0x21ABCDEF for CEFF, id 0x01ABCDEF), independently derived from Figure 39 rather than round-tripped through the encoder under test. Mutation-tested: dropping the format bits from the packed quadlet fails exactly the golden-vector and round-trip tests. Also fixes a pre-existing test (test_tc18_gaps_ep2.c) that had encoded the same wrong evt[2:0] assumption as its own expected behavior. Verified clean under ASan/UBSan. Neither rcp_ep_can_decode_frame_request() nor _decode_frame_response() is wired into any real server dispatch loop today (no caller in src/server.c/src/mock.c), so no observed behavioral impact on any live path -- but this was a real interop-breaking wire-format defect any future dispatch wiring, or any external client implementing TC18 correctly, would have hit immediately. BREAKING: every function's own wire encoding changes for every frame_format value, not just an added validation -- no c-RCP CAN endpoint traffic (real or test) was wire-compatible with a conformant TC18 peer before this fix, so there is no meaningful backward-compat concern to preserve. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> --------- 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
1 participant
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.
Summary
Found during a cross-repo architecture inventory (RELAY
docs/RCP-ARCHITECTURE.md):rcp_ep_adc_decode_read_request()never checkedevt[2:0]at all. Per TC18 §13.5 Table 30 (TC18.txt:3666-3710), the{ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}endpoint-type row requiresevt[2:0] = 000bfor a plain request — every other value is either reserved (001b-110b, reject withUNSUPPORTED_CMD) or selects a differently-shaped configuration-write request (111b, §12.7.1) a plain read decoder must not accept.Adds
rcp_acf_evt_row2_is_plain()(acf.h/acf.c) as the shared primitive, wired into ADC'sdecode_read_requestas the first of 7 affected endpoint types. I2C/UART/ISELED/MDIO need the same wiring (tracked in task #63 on the RCP-ARCHITECTURE effort). CAN/LIN each invented a different, spec-incompatible use ofevt[2:0](frame-format / comparison-mode selection) and need a separate redesign — not fixed here.rcp_ep_adc_decode_read_request()isn't wired into any real server dispatch loop today (confirmed: no caller insrc/server.c/src/mock.c), so no observed behavioral impact on any live path — but it's a real conformance defect any future dispatch wiring would inherit.New public API + a real decode-behavior tightening — MINOR bump.
Test plan
ctest— 64/64 passcfusa check(v0.5.50) — 0 errorscfusa trace --sec-tested 100— 977/977 traced and tested (was 976)