Repository navigation
rewrite(phase2): port e2e/lifecycle from c-RCP, correct HARA.md's overstated H-004 claim - #134
Merged
Merged
Conversation
…rstated H-004 claim Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. Second and final batch of Phase 2 -- the safety-critical core: CRC32 safe points, safety-request handling, replay/staleness detection, and the RC-Server lifecycle state machine. Real content deltas found and fixed against c-RCP's e2e.h/e2e.c and lifecycle.h/lifecycle.c: - CRC coverage was missing 3 leading bytes (c-RCP issue #465): avtp_subtype/header_octet1/tu -- TC18 Figures 20/21's own 'header CRC' region -- were absent entirely from coverage_buffer()/ compute_crc()/verify_crc(). A conformant peer's CRC32 would never have matched this library's prior output. - wrap()/unwrap() didn't exist at all -- only append_crc() (appends at buffer end), which produces the wrong wire order [header][real][pad][CRC] whenever a payload carries trailing pad bytes; TC18 requires [header][real][CRC][pad] (c-RCP issue #420). - RxSequenceGuard (REQ-E2E-028/029, the mechanism a prior audit this session flagged as implemented-but-unwired) was substantively wrong, not just incomplete: compared sequence numbers as a non-wrapping uint32_t (would spuriously reject every request once an 8-bit AVTPDU sequence_num wraps) and never consulted rx_seq_safestate_enable at all. Replaced with c-RCP's RFC 1982 forward-window comparison. - endpoint_in_configured_safe_state() was missing REQ-SEQ-012's fail-closed rule -- a disabled sequencer (state==0) could satisfy a safe-state check if rx_safe_sequencer_state was also misconfigured to 0. - Missing entirely, now ported: fragment_carries_crc()/ compute_fragmented_crc() (REQ-E2E-010/038), crc_error_should_enter_ safe_state() (REQ-E2E-045), StreamFaultTracker (REQ-E2E-021), StreamStatus (REQ-E2E-046), length_with_crc()/ data_length_for_protected_members() (REQ-E2E-004/037), overflow_should_enter_safe_state() (REQ-E2E-030). lifecycle.hpp had only a bare 3-state machine with no real access-control layer -- c-RCP's whole issue #198 (writer authorization, unicast enforcement, idle-gating) was absent. Added, additive: EndpointPlausibility/RequestStreamPlausibility/PlausibilitySnapshot + check_hw_cfg()/check_rcp_cfg() (real TC18 Sec12.3.1.2 plausibility content, not just an opaque callback); WriterCtx; ServerLifecycle::transition() (full writer-authorization + idle-gating state machine, including the RCP_CONFIGURED->HW_CONFIGURED demotion and both authorized resets advance() never modeled); Disposition/ should_accept() (per-state request filtering); field_writable()/ field_write_error() family (register-locking-by-state). advance() kept unchanged (mock.hpp and existing tests depend on its differing same-state semantics, a documented pre-existing design choice); transition() coexists as the richer, spec-complete entry point. HARA.md/FORMAL_VERIFICATION.md correction: HARA.md's H-004 section described RxSequenceGuard::check in present-tense mitigating language with SG-004 listed as a clean ASIL-B mitigation and no caveat. Added a 'STATUS CORRECTED' block: the mechanism is implemented and unit-tested but never instantiated outside its own test -- not in mock::Server's dispatch or any transport Server -- citing c-RCP's #601/#606 resolution as the parallel precedent, stating H-004 is 'effectively Open' pending that wiring (Phase 4/server-dispatch scope, not this pass -- mock.hpp was NOT touched, RxSequenceGuard is referenced only in e2e.hpp itself). FORMAL_VERIFICATION.md got the same caveat, plus a second, independently-found correction: tla/RxSequenceGuard.tla itself still models the old non-wrapping comparison, not the RFC 1982 algorithm now in e2e.hpp -- flagged as a real, un-re-derived divergence and tracked as a follow-up (re-deriving the TLA+ spec is its own formal-modeling task). Fixed-capacity: StreamFaultTracker uses a std::array<Slot, 16>, no heap allocation -- bound taken directly from c-RCP's RCP_E2E_STREAM_FAULT_TRACKER_MAX_STREAMS. lifecycle.c confirmed to have no alloc.h/heap dependency; no fixed-capacity work needed there. Left as // TODO(phase2-followup): apply_acf_length_adjustment() doesn't fail-safe on 9-bit field overflow the way c-RCP's adapt_acf_msg_length() does (low real exposure, but a fallible return type would touch every existing call site) -- named gap, not silently dropped. RxSequenceGuard.tla's staleness (above) is a second, separate tracked follow-up. test_e2e.cpp: 32 -> 72 TEST_CASEs. test_lifecycle.cpp: 16 -> 47 TEST_CASEs. Verified: full tree rebuilds clean (223/223 targets, incl. the merged watchdog/alloc-seam batch), 56/56 ctest suites pass (100%) -- independently re-verified from a fresh build, not just the porting agent's own report. No call sites outside target files required changes (regmap.hpp/mock.hpp depend on lifecycle.hpp, watchdog.hpp depends on e2e.hpp, all compile and pass unchanged since every new piece is additive). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
This was referenced Aug 21, 2026
SoundMatt
added a commit
that referenced
this pull request
Aug 22, 2026
…Tracker, RxWatchdog (#148) Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. Phase 4 batch C of a four-batch mock.hpp port, built on batch A (admission) and batch B (response suppression + regmap/discovery wiring). This closes the exact gap flagged during Phase 2 (PR #134, e2e.hpp's own header comment): RxSequenceGuard, StreamFaultTracker, and RxWatchdog were all fully implemented in e2e.hpp but never called from any dispatch path anywhere in cpp-RCP -- e2e.hpp's own comment explicitly deferred this wiring to "Phase 4 (server/dispatch) scope, matching c-RCP's own mock.c wiring (frame_seq_gate_admits(), dispatch_frame_e2e())." That deferred work lands here. ## Ported (c-RCP src/mock.c -> Server) - rcp_mock_server_dispatch_e2e() (src/mock.c:1892-2038, REQ-E2E-021/ 031/041, REQ-WDG-010) -> new Server::dispatch_e2e(). - frame_seq_gate_admits() (src/mock.c:3038-3063, REQ-E2E-028/029) -> private Server::seq_gate_admits(), folded into single-member dispatch per this batch's scope (multi-member AVTPDU frame splitting is batch D's job; this batch's guard logic is built at the single-member level so batch D can reuse the same guard instance). - rcp_mock_server_check_watchdog() (src/mock.c:991-1021, REQ-E2E-046) -> public Server::check_watchdog_overflow(). - rcp_mock_server_stream_status_rx_blocked() (src/mock.c:981-989) -> Server::stream_rx_blocked(). - Watchdog-kick call sites (REQ-WDG-010) -> unconditional rx_watchdog_kick() at the top of dispatch_e2e(). ## Shape of the new E2E dispatch surface One public Server::dispatch_e2e() entry point, not ten dispatch_<type>_e2e() siblings. ep_req_crc_enable (TC18 Sec12.7.1) is a per-endpoint, not per-endpoint-type, toggle, and c-RCP's own rcp_mock_server_dispatch_e2e() is likewise a single function parameterized by byte_bus_id, not duplicated per handler -- mirrors dispatch()'s own established single-entry-point convention. All 10 operational endpoint types get E2E support uniformly via regs_.generic_configs[].ep_req_crc_enable (the real register). EP0 gets none -- it has no generic_configs[] row, matching c-RCP's own EP0 exclusion from admission entirely. ## New Server state e2e::StreamFaultTracker stream_fault_tracker_; std::array<e2e::RxSequenceGuard/RxWatchdog/StreamStatus, kMaxEntries=64> keyed by resolved request_streams[] index (mirrors c-RCP's srv->seq_tracker[]/srv->stream_status[]) -- all owned directly, matching mock.hpp's existing convention over c-RCP's pointer/setter style. ## Deliberate scope boundaries (documented in-file, not TODO-stubbed -- none block correctness) - REQ-E2E-045/029's full stream-wide "broadcast safe state to every bound endpoint" actuation is not implemented (no SequencerTable- driving actuator exists in this file yet) -- only the observable StreamStatus latch is. Pre-existing gap class shared with batch A/B's own admission-reject overflow-safestate consequence, not new here. - check_watchdog_overflow()'s purge targets server::Endpoint:: watchdog_purge() (the pending/conditional store) rather than rcp::watchdog::Manager (RequestLedger-keyed, architecturally mismatched to this mock's admission storage). - No TSCF variant of dispatch_e2e (c-RCP's dispatch_e2e_tscf, REQ- TIMED-012/013) -- out of scope since plain dispatch() has no TSCF timing gate either yet. No content bugs found versus c-RCP. One test-fixture-only issue found while writing coverage: GPIO's apply_gpio_write() masks writes by pin direction (default all-input, TC18 Sec13.7.4.3) -- tests needed a configure_gpio_all_outputs() fixture helper before asserting OR-write effects; not a library bug. ## Verification New Server::dispatch_e2e() + 6 new TEST_CASEs covering replay detection, stream-fault short-circuit, and watchdog kick/overflow, integration-tested via mock.hpp's dispatch (not re-testing e2e.hpp's own already-unit-tested internals). include/rcp/mock.hpp: 1759 -> 2162 lines. tests/test_mock.cpp: 1561 -> 1832 lines, 71 -> 77 TEST_CASEs (632 assertions). include/rcp/e2e.hpp: header comment updated to record the closed deferral (primitives unchanged). Full tree rebuilds clean (0 errors, 0 warnings, verified with -Wall -Wextra), 58/58 ctest suites pass (100%) -- independently re-verified from a fresh build, not just the porting agent's own report. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
SoundMatt
added a commit
that referenced
this pull request
Aug 22, 2026
…ses out Phase 4 (#150) Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. Second of two sub-batches (D1/D2) making up Phase 4's fourth and FINAL batch. This completes mock.hpp -- the last module of Phase 4 -- and closes out the server/dispatch layer in full. Built on batches A (admission), B (response suppression + regmap/discovery), C (single-member E2E dispatch), and D1 (E2E fragmented dispatch). ## Ported (c-RCP src/mock.c/src/scheduler.c/src/request.c, REQ-MOCK-019/ 020/029, REQ-E2E-033, REQ-CANCEL-012) - split_frame_members() -- ported from rcp_sched_split_frame_members() (scheduler.c:63-106), a new namespace-scope helper (request.hpp's own header explicitly notes this splitting logic lives nowhere else in the rewrite -- c-RCP keeps it mock.c-local too). - FrameMemberResult + Server::dispatch_frame()/dispatch_frame_e2e() -- ported from rcp_mock_server_dispatch_frame()/_dispatch_frame_e2e() (mock.c:3068-3382). Each returns per-member results (mirrors c-RCP's own result-array shape -- c-RCP does not aggregate members' responses into one outgoing wire frame either, confirmed by source read). - Chain-group/position bookkeeping (REQ-CANCEL-012, ported from rcp_chained_advance/is_chained_member/last_pending_index, mock.c:2950-3231, request.c:673-694) -- via a new shared dispatch_frame_impl() template, using request::should_execute_chained() for the cs-polarity predicate and a new last_pending_index() helper. - apply_cancellation()'s ClearSingle TODO (open since batch A) filled in: decodes request::decode_clear_single() from the raw frame, captures the target's chain_group/chain_position before removing it, cascades via cancel_chain_from(). ## Bug found and fixed (beyond the literal ask) admit_and_classify() always re-encoded ABB from req/payload, silently stripping any genuine GBB conditional/cancellation opcode's semantics -- meaning ClearSingle/Chained frames were never actually reachable before this batch, regardless of how they'd been decoded. Refactored to take the raw frame directly (mechanical edit across its 10 call sites, zero behavior change for them) and added decode_and_dispatch()'s own conditional-opcode-aware routing branch, so dispatch_frame()/dispatch_frame_e2e() -- and every existing dispatch_e2e()/dispatch_e2e_fragment() caller -- can now genuinely admit Chained/Compound/Triggered/Timed/Cancellation GBB members. Also extracted dispatch_e2e_core()/dispatch_e2e_fragment_core() with an apply_seq_gate flag so the frame-level sequence gate runs exactly once per frame (matching the pre-existing doc comment's own stated intent), not once per member. ## Delta from c-RCP (documented in-file) dispatch_frame_e2e() routes every member through dispatch_e2e_fragment_core() rather than plain dispatch_e2e_core() -- c-RCP has no frame-level fragment-aware dispatch at all; this is a safe superset since the fragment-core function falls back identically for non-fragmented members. ## Test-writing findings (documented, not library bugs) split_frame_members() requires each member's declared acf_msg_length*4 to exactly match its real encoded byte count, but acf::encode_acf_abb()/e2e::wrap_framed() never auto-pad payloads to quadlet boundaries (documented as caller-owned in acf.hpp) -- never mattered for single-member dispatch before; all new multi-member test frames use quadlet-aligned payloads with an explanatory comment. Confirmed DispatchErrc::unknown_bus is unreachable in this codebase (acf.hpp's decode is deliberately lenient vs c-RCP's strict decode) -- documented as defensive-only with a test pinning the actual lenient behavior. ## Final Phase-4 sweep Grepped the whole file for TODO(phase4-batch-*) markers: only apply_cancellation()'s ClearSingle stub existed, now resolved. No dangling phase-4 markers remain anywhere in mock.hpp. ## Verification New split_frame_members()/dispatch_frame()/dispatch_frame_e2e() + ClearSingle cascade, tested end-to-end via multi-member AVTPDU frames. include/rcp/mock.hpp: 2723 -> 3413 lines. tests/test_mock.cpp: 2349 -> 2775 lines, test_mock: 101 TEST_CASEs (863 assertions). Full tree rebuilds clean (0 errors, 0 warnings), 58/58 ctest suites pass (100%) -- independently re-verified from a fresh build, not just the porting agent's own report. ## Phase 4 complete mock.hpp grew from its pre-Phase-4 baseline of 917 lines to 3413 lines across four batches: A added server::Endpoint admission wiring, B added Table 24 response suppression plus regmap/discovery storage, C wired RxSequenceGuard/StreamFaultTracker/RxWatchdog into single-member E2E dispatch (closing a gap flagged since PR #134), D1 wired fragment.hpp/respqueue.hpp for E2E fragmented dispatch (closing a gap open since Phase 1), and this D2 batch added AVTPDU frame-level multi-member splitting/routing/chain-cascade cancellation. Combined with regmap (batches A/B), the new server.hpp, discovery, and adapt -- all merged earlier in Phase 4 -- the server/dispatch layer is now complete with no phase-4-scoped work left deferred. Signed-off-by: Matt <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.
Part of the v3.0.0 ground-up rewrite (#129, ROADMAP.md Phase 17). Second and final batch of Phase 2 — the safety-critical core: CRC32 safe points, safety-request handling, replay/staleness detection, and the RC-Server lifecycle state machine.
Real content deltas found and fixed
avtp_subtype/header_octet1/tu— TC18 Figures 20/21's own "header CRC" region — were absent entirely. A conformant peer's CRC32 would never have matched this library's prior output.wrap()/unwrap()didn't exist at all — onlyappend_crc()existed, producing the wrong wire order whenever a payload carries trailing pad bytes (c-RCP issue #420).RxSequenceGuardwas substantively wrong, not just incomplete: compared sequence numbers as a non-wrappinguint32_t(spurious rejection on every 8-bitsequence_numwrap) and never consultedrx_seq_safestate_enableat all. Replaced with c-RCP's RFC 1982 forward-window comparison.endpoint_in_configured_safe_state()was missing REQ-SEQ-012's fail-closed rule.fragment_carries_crc()/compute_fragmented_crc(),crc_error_should_enter_safe_state(),StreamFaultTracker,StreamStatus,length_with_crc()/data_length_for_protected_members(),overflow_should_enter_safe_state().lifecycle.hpphad only a bare 3-state machine — c-RCP's whole access-control layer (writer authorization, unicast enforcement, idle-gating, issue #198) was absent. Added, all additive: real TC18 §12.3.1.2 plausibility content,ServerLifecycle::transition()(full writer-auth + idle-gating state machine),Disposition/should_accept(), register-locking-by-state.advance()kept unchanged (existing dependents rely on its documented same-state semantics);transition()coexists as the richer entry point.HARA.md/FORMAL_VERIFICATION.mdcorrectionHARA.md's H-004 section describedRxSequenceGuard::checkin present-tense mitigating language with no caveat. Added a "STATUS CORRECTED" block: the mechanism is implemented and unit-tested but never instantiated outside its own test — citing c-RCP's #601/#606 resolution as precedent, stating H-004 is "effectively Open" pending dispatch wiring (Phase 4 scope —mock.hppwas not touched here).FORMAL_VERIFICATION.mdgot the same caveat plus an independently-found second issue:tla/RxSequenceGuard.tlaitself still models the old non-wrapping comparison — flagged as a tracked follow-up.Fixed-capacity
StreamFaultTracker:std::array<Slot, 16>, no heap allocation, bound from c-RCP'sRCP_E2E_STREAM_FAULT_TRACKER_MAX_STREAMS.lifecycle.cconfirmed to have no heap dependency.Left as TODO(phase2-followup)
apply_acf_length_adjustment()'s 9-bit overflow fail-safe (low exposure, would touch every call site);RxSequenceGuard.tla's staleness (separate formal-modeling task).test_e2e.cpp: 32→72 cases.test_lifecycle.cpp: 16→47 cases.Verified
Full tree rebuilds clean (223/223 targets, incl. the merged watchdog/alloc-seam batch), 56/56 ctest suites pass (100%) — independently re-verified from a fresh build.
Tracked at #129 — completes Phase 2.