feat: v0.108.0 — enforce TC18 Table 30's Row-2 evt[2:0] rule for I2C/UART/ISELED/MDIO - #154
Open
SoundMatt wants to merge 1 commit into
Open
feat: v0.108.0 — enforce TC18 Table 30's Row-2 evt[2:0] rule for I2C/UART/ISELED/MDIO#154SoundMatt wants to merge 1 commit into
SoundMatt wants to merge 1 commit into
Conversation
…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>
7 tasks
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
Completes the Table 30 centralization started in #153 (ADC): none of I2C's
decode_transfer_request, UART'sdecode_write_request/decode_read_request, ISELED'sdecode_command_request, or MDIO'sdecode_read_request/decode_write_requestcheckedevt[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 legalevt[2:0]value for a plain request is000b; every other value is either reserved (reject withUNSUPPORTED_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 (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 — but each is a real conformance defect any future dispatch wiring would inherit.
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) + real decode-behavior tightening across 5 modules — MINOR bump.
Test plan
ctest— 64/64 passcfusa check(v0.5.50) — 0 errorscfusa trace --sec-tested 100— 977/977 traced and tested