Repository navigation
feat: implement Table 30/33 Row-2 evt[2:0] validation — ISELED (7th endpoint type) - #120
Merged
Merged
Conversation
…ndpoint type)
Wires §13.5 Table 33 Row 2's Plain/Reserved/ConfigWrite evt[2:0]
classification into the ISELED endpoint (§13.7.12), the 7th of the 8
endpoint types sharing this row ({ADC, PWM_IN, I2C, LIN, CAN, UART,
ISELED, MDIO}), following the pattern established by I2C (#114), ADC
(#115), PWM_IN (#116), LIN (#117), CAN (#118), and UART (#119).
- rcp/iseled.hpp: add IseledEndpoint::handle_request(evt_op, request,
response), which classifies evt[2:0] via the existing shared
endpoint::evt_row2_kind_of() predicate before doing anything else —
Plain (000b) delegates straight to the existing transact(request,
response) unchanged, Reserved (001b-110b) returns
endpoint::EndpointErrc::reserved_evt_row2 without recording
anything, and ConfigWrite (111b) returns the new
IseledErrc::config_write_not_supported (§12.7.1, deliberately
deferred, same scope decision every prior Row 2 endpoint made).
This does NOT touch, reinterpret, or extend IseledRequest's/
IseledResponse's existing field shapes, validate_request/
validate_response, or the encode_iseled_request/decode_iseled_request/
encode_iseled_response/decode_iseled_response Figure 40/41 codec in
any way — those were already corrected against TC18's real wire
format under issue #71/cpp-RCP-02, and this PR is strictly the
separate, orthogonal Table 33 evt[2:0] classification concern. No
new encoding of any kind is invented here.
- rcp/mock.hpp: wire ISELED in as the 9th mock endpoint (byte_bus_id
9), following UART's (#119) wiring shape: new iseled::IseledEndpoint
member, kIseledEndpointId/kIseledByteBusId constants, dispatch_iseled
routing (decodes the wire payload via ISELED's own pre-existing
decode_iseled_request, drives handle_request, encodes the response
back via encode_iseled_response — reusing existing codec functions,
inventing nothing), a set_iseled_response() scripting hook mirroring
set_i2c_response/set_lin_response, and wire_error_code_for mappings
for IseledErrc. REQ-MOCK-001's endpoint count is updated to nine.
- .fusa-reqs.json: adds REQ-ISELED-006/007 and REQ-MOCK-023/024,
reusing tc18_master_id TC18-13.5-001 (Table 33 Row 2, which
explicitly names ISELED) and TC18-12.7.1-001 (§12.7.1 config-write),
independently re-verified against RELAY's docs/tc18-master-catalog.json
rather than assumed from precedent. REQ-ISELED-006 (the low-level
classification requirement) is tagged severity: cybersecurity,
matching every prior Row 2 endpoint's precedent.
- tests/test_iseled.cpp, tests/test_mock.cpp: Plain/Reserved(x6)/
ConfigWrite coverage at both the IseledEndpoint::handle_request level
and the mock dispatch level, plus pre-RCP_CONFIGURED gating.
Verification:
- cmake --build + ctest: 53/53 test binaries pass.
- cpfusa check --strict: 0 errors (2 pre-existing warnings/1 info
unrelated to this change: FUSA004 evidence bundle, HARA005 hazard
ASIL mismatch — both present on main before this PR).
- cpfusa trace: 406/406 requirements 100% annotated, 100% tested.
- cpfusa trace --sec-tested 17: passes, secTestedRequirements 71/406
= 17.49%.
- Mutation test: temporarily made the ConfigWrite branch call
transact() instead of returning config_write_not_supported;
confirmed it broke 2 test_iseled assertions and 1 test_mock
assertion; reverted cleanly (git diff showed no leftover) and
confirmed tests pass again.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
SoundMatt
added a commit
that referenced
this pull request
Aug 20, 2026
… LAST endpoint type) (#121) Implements TC18 (OPEN Alliance TC18 Remote Control Protocol Specification v0.5.1_RC) §13.5 Table 33's "Row 2" evt[2:0] request classification for the MDIO endpoint type (include/rcp/mdio.hpp), the eighth and LAST of the eight endpoint types sharing this row ({ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}), following the pattern established by I2C (#114), ADC (#115), PWM_IN (#116), LIN (#117), CAN (#118), UART (#119), and ISELED (#120). Classification rule (via the shared rcp::endpoint::evt_row2_kind_of / EndpointErrc::reserved_evt_row2 predicate already in rcp/endpoint.hpp — not reinvented here): - evt[2:0] == 000b -> Plain: delegates to the existing transaction logic - evt[2:0] in 001b-110b -> Reserved: rejected with EndpointErrc::reserved_evt_row2 (wire error code UNSUPPORTED_CMD) - evt[2:0] == 111b -> ConfigWrite (§12.7.1, not yet implemented): reported via new MdioErrc::config_write_not_supported ## handle_request -> transact rename (naming collision) MdioEndpoint already had a pre-existing method literally named `handle_request(MdioRequest, MdioResponse&)` — the read/write register transaction logic, unrelated to evt[2:0] classification, predating this milestone. To free that name for the new evt[2:0]-classifying entry point: 1. Renamed the existing method to `transact(MdioRequest, MdioResponse&)`, matching the name rcp/iseled.hpp's IseledEndpoint::transact already established for the identical "existing single-call transaction method, now wrapped by a classifying handle_request" role. 2. Updated all 8 call sites in tests/test_mdio.cpp from `.handle_request(` to `.transact(`. Grepped the whole repo (including mock.hpp/mdio_.*) to confirm no other call sites existed before this PR (mock.hpp had zero MDIO wiring at all prior to this change). 3. Added a NEW `handle_request(uint8_t evt_op, MdioRequest, MdioResponse&)` that classifies evt_op first: Plain delegates straight to transact() unchanged, Reserved returns EndpointErrc::reserved_evt_row2 without calling transact(), ConfigWrite returns MdioErrc::config_write_not_supported. Confirmed clean: `grep -rn "\.handle_request("` across the repo shows zero remaining 2-argument MdioEndpoint::handle_request call sites; every call site now either invokes transact() directly (register-access tests) or the new 3-argument evt[2:0]-classifying handle_request (classification tests). ## Addressing model — untouched include/rcp/mdio.hpp's own header comment documents that this header previously modeled MDIO addressing as an invented IEEE 802.3 Clause 22/Clause 45-style PHY-address scheme with no basis in TC18, corrected in issue #72 to the real mdio_mode/mdio_address/mdio_payload model. This PR does NOT touch, reinterpret, or extend MdioRequest's/MdioResponse's field shapes or the mdio_mode/register_key logic in any way — Table 33's evt[2:0] classification is a completely separate, orthogonal concern, and no new MDIO wire encoding is invented anywhere in mdio.hpp itself. ## mock.hpp wiring (10th mock endpoint, byte_bus_id=10) Wires MdioEndpoint into rcp/mock.hpp for the FIRST time (zero prior MDIO wiring, verified via `grep -n "mdio\|Mdio\|MDIO" include/rcp/mock.hpp` on main returning nothing), following UART's (#119) and ISELED's (#120) wiring shape: - byte_bus_id/endpoint id 10, after ISELED's 9 - REQ-MOCK-001's endpoint count updated eight -> nine -> ten (nine was already current on main from the ISELED PR) - new dispatch_mdio(), branching on req.op like dispatch_uart (since MdioRequest::is_write is intrinsic to the request itself, unlike I2C/LIN's op-agnostic full-duplex/raw-transfer dispatch) - Unlike ISELED, NO MDIO byte-level wire codec (Figure 43/Table 60) exists anywhere in this codebase — mdio.hpp deliberately stops at the (mode, mdio_address, mdio_payload) struct level. dispatch_mdio makes the same kind of deliberately simple, explicitly disclaimed choice dispatch_can makes for CAN's fixed CanIdentifier/FrameFormat: a fixed representative MdioMode::MmdSingleWord/mdio_address=0, with mdio_payload packed/unpacked via avtp::detail::put_u16/get_u16 (the same generic helper dispatch_ep0 already uses for its own magic-number encoding) purely so the mock has *some* round-trippable wire value — this is the mock's own simulation choice, not a TC18-derived encoding. - No set_mdio_response() hook: MDIO's register model is self-contained (a write round-trips straight back out on a subsequent read), unlike I2C/LIN/ISELED's external-bus scripting hooks. ## .fusa-reqs.json Added REQ-MDIO-006/007 (evt[2:0] classification + config-write-not-yet- supported) and REQ-MOCK-025/026 (mock dispatch classification + pre- RCP_CONFIGURED gating), reusing TC18-13.5-001 and TC18-12.7.1-001 — independently re-verified against RELAY's docs/tc18-master-catalog.json (both entries confirm MDIO by name in Table 33's Row 2 list) and against the actual TC18.txt line ranges cited (L4085-4092 and L2844-2865 respectively; both match the source text exactly). Also updated REQ-MDIO-003/004's requirement text (transact rename), since those describe the exact method being renamed. Tagged REQ-MDIO-006 (the reserved-value-rejection requirement) severity: cybersecurity, matching every prior Row-2 endpoint's pattern; sec-tested ratio after this change is 72/410 = 17.56%, above cpfusa's 17% floor. ## Pre-existing citation spot-check (not touched by this diff) Spot-checked MDIO's own PRE-EXISTING TC18.txt citations against the actual document content at /Users/matt/Coding/SoundMatt/TC18.txt: - REQ-MDIO-001 (§13.2 Table 32, "L3998-4017"): confirmed exact match — Table 32's "0x0D MDIO" row is at line 4017, table starts ~3998. - REQ-MDIO-002 (§13.7.13.3 Table 60, "L6080-6088"): confirmed exact match — Table 60's mdio_mode/mdio_address/mdio_payload rows and its own caption line are within 6081-6088. Unlike ISELED's citations in the rust-RCP repo (found ~400 lines stale in PR #161), MDIO's pre-existing citations in cpp-RCP show NO drift against the current TC18.txt — both line ranges verified correct as-is. No corrective action taken (not needed). Separately (not a TC18-citation issue, but noted for completeness): REQ-MDIO-005's requirement text still references `MdioErrc::phy_address_out_of_range`, a symbol that no longer exists in mdio.hpp's enum — a leftover from the pre-issue-#72 Clause 22/Clause 45 addressing model that was apparently never updated when MdioErrc was rebuilt. This requirement's text/enum-name mismatch is NOT part of this PR's diff and is left as-is per the same "flag, don't silently rewrite" policy applied to citation staleness; worth a small follow-up. ## Testing - tests/test_mdio.cpp: renamed 8 call sites to transact(); added 6 new TEST_CASEs covering Plain/Reserved(x6)/ConfigWrite/masking/field- independence classification, plus a config_write_not_supported message sanity check. - tests/test_mock.cpp: REQ-MOCK-001 nine -> ten endpoints; new MDIO section mirroring CAN/UART/ISELED's shape (write-then-read round-trip through dispatch(), Reserved x6, ConfigWrite, pre-RCP_CONFIGURED gating). - Full build (cmake+Ninja) and `ctest --test-dir build`: 53/53 suites pass, 0 failures. - `cpfusa check --strict`: 0 errors (2 warnings/1 info are pre-existing baseline conditions, confirmed identical on main before this change — FUSA004 missing .fusa-evidence.json, HARA005 hazard ASIL mismatch). - `cpfusa trace`: 410/410 requirements 100% annotated, 100% tested; `--sec-tested 17` passes (17.56% cybersecurity-tagged ratio). - Mutation test: temporarily changed the ConfigWrite branch in MdioEndpoint::handle_request to call transact() instead of returning the error; rebuilt; confirmed 2 test_mdio assertions and 1 test_mock assertion failed as expected; reverted cleanly; confirmed all tests pass again and `grep -n "MUTATION-TEST"` finds nothing left behind. Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.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.
What
Implements TC18 §13.5 Table 33 Row 2's evt[2:0] request classification (Plain / Reserved / ConfigWrite) for the ISELED endpoint (
include/rcp/iseled.hpp), TC18 §13.7.12 — the 7th of 8 endpoint types sharing this row ({ADC, PWM_IN, I2C, LIN, CAN, UART, ISELED, MDIO}), following the pattern established by I2C (#114), ADC (#115), PWM_IN (#116), LIN (#117), CAN (#118), and UART (#119).Confirmation: existing ISELED wire-format fields are untouched
include/rcp/iseled.hpp's own header comment documents a prior, genuinely invented, non-spec-derived wire format (open-ended response data vector + invented CRC-8) that had to be corrected against TC18's real Figure 40/41 request/response layout (issue #71 / cpp-RCP-02). This PR does not touch any of that.git diffoninclude/rcp/iseled.hppremoves exactly one line (a disclaimer sentence, replaced with an expanded header-name list) —IseledRequest/IseledResponse's field shapes,validate_request/validate_response, and theencode_iseled_request/decode_iseled_request/encode_iseled_response/decode_iseled_responseFigure 40/41 codec are all unmodified. This PR is strictly the separate, orthogonal Table 33 evt[2:0] classification concern (§13.5), layered on top via a newhandle_requestentry point that treats the existingIseledRequest/IseledResponsetypes as opaque, already-decoded values — exactly the way CAN'shandle_requesttook an already-decodedCanDataFramerather than raw bytes.Changes
rcp/iseled.hpp:IseledEndpoint::handle_request(evt_op, request, response)classifiesevt_opvia the existing sharedendpoint::evt_row2_kind_of()predicate:000b) → delegates straight to the existingtransact(request, response), unchanged.001b-110b) → returnsendpoint::EndpointErrc::reserved_evt_row2without recording anything.111b) → returns newIseledErrc::config_write_not_supported(§12.7.1, deliberately deferred — same scope decision every prior Row 2 endpoint made).rcp/mock.hpp: wires ISELED in as the 9th mock endpoint (byte_bus_id9), following UART's (feat: implement Table 30 Row-2 evt[2:0] validation — UART (6th endpoint type) #119) from-scratch wiring shape — newiseled::IseledEndpointmember,kIseledEndpointId/kIseledByteBusIdconstants,dispatch_iseledrouting (decodes the wire payload via ISELED's own pre-existingdecode_iseled_request, driveshandle_request, encodes the response back viaencode_iseled_response— reusing existing codec functions, inventing nothing), aset_iseled_response()scripting hook mirroringset_i2c_response/set_lin_response, andwire_error_code_formappings forIseledErrc.REQ-MOCK-001's endpoint count is updated to nine.Verified before wiring that
mock.hpphad zero prior ISELED references (grep -n "iseled\|Iseled\|ISELED" include/rcp/mock.hppreturned nothing onmain)..fusa-reqs.json: addsREQ-ISELED-006/REQ-ISELED-007andREQ-MOCK-023/REQ-MOCK-024, reusingtc18_master_idTC18-13.5-001(Table 33 Row 2 — its summary explicitly names ISELED) andTC18-12.7.1-001(§12.7.1 config-write), independently re-verified against RELAY'sdocs/tc18-master-catalog.jsonrather than assumed from precedent.REQ-ISELED-006(the low-level classification requirement) is taggedseverity: cybersecurity, matching every prior Row 2 endpoint's precedent.tests/test_iseled.cpp,tests/test_mock.cpp: Plain / Reserved (×6) / ConfigWrite coverage at both theIseledEndpoint::handle_requestlevel and the mock dispatch level, plus pre-RCP_CONFIGUREDgating.Verification
cmake --build+ctest --test-dir build: 53/53 test binaries pass.cpfusa check --strict: 0 errors (2 pre-existing warnings / 1 info unrelated to this change — FUSA004 evidence bundle, HARA005 hazard ASIL mismatch — both already present onmain).cpfusa trace: 406/406 requirements, 100% annotated, 100% tested.cpfusa trace --sec-tested 17: passes —secTestedRequirements71/406 = 17.49%.transact()instead of returningconfig_write_not_supported; rebuilt; confirmed it broke 2test_iseledassertions and 1test_mockassertion; reverted cleanly (git diffshowed no leftover) and confirmed all tests pass again.Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com