Repository navigation
rewrite(phase4): mock.hpp batch D1 — wire fragment.hpp/respqueue.hpp for E2E fragmented dispatch - #149
Merged
Conversation
…for E2E fragmented dispatch Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. First of two sub-batches (D1/D2) making up Phase 4's fourth and final batch. Built on batches A (admission), B (response suppression + regmap/discovery), and C (single-member E2E dispatch with RxSequenceGuard/StreamFaultTracker/RxWatchdog). fragment.hpp and respqueue.hpp were both fully ported in Phase 1 (fixed-capacity, zero heap allocation, with the c-RCP issue #614/#616 oversized-reassembly error-handling lesson baked in from day one) but neither was ever wired into any dispatch path. This closes that gap. ## Ported (c-RCP src/mock.c:2175-2504, rcp_mock_server_dispatch_e2e_fragment) REQ-E2E-038/039/046, REQ-ISELED-025, REQ-FRAG-*. Confirmed by direct source read that c-RCP's own fragment-dispatch function has no sequence-gate call and no TSCF variant reachable from this rewrite's scope -- mirroring batch C's own established "NTSCF-only, no per-frame seq gate" simplification for dispatch_e2e(), neither dispatch_e2e_fragment_tscf nor a sequence-number gate were ported here; both are legitimately deferred to D2's future frame-level dispatch_frame_e2e(). ## New public surface on mock::Server - dispatch_e2e_fragment(client, stream_id, fragment, out_resp, out_resp_payload) -- feeds fragments into a per-request-stream fragment::Reassembler (keyed by regmap::request_stream_cfg::resolve_index(), same indexing as seq_trackers_/rx_watchdogs_/stream_status_). On completion validates e2e::compute_fragmented_crc() (over the first fragment's header + reassembled payload, deliberately ignoring unwrap()'s own wrong single-frame verdict, matching c-RCP's own comment), then delegates into the existing single-member decode_and_dispatch()/dispatch() path -- no duplicated CRC-unwrap/admission logic. - maybe_fragment_response(...) -- new (c-RCP's own equivalent, rcp_mock_server_dispatch_multi_response(), is a per-endpoint registered-handler pattern this rewrite's hardcoded ten-dispatch_*()-method architecture has no analog for). Slices an oversized genuine response via fragment::plan() and pushes each fragment onto a respqueue::RespQueue keyed by rx_resp_stream_index (1-based, resolved against regs_.response_streams -- multiple request streams can share one response queue). - Accessors: fragment_reassembler(stream_id), resp_queue_for_stream( stream_id) -- nullable pointers mirroring c-RCP's own rcp_mock_server_fragment_reassembler() precedent. - Three new DispatchErrc values: fragment_pending, fragment_rejected (both no-wire-response, mirroring c-RCP's identical dispositions), response_fragmented (real multi-frame response now on the RespQueue). ## Oversized-reassembly error path (c-RCP issue #614/#616) -- verified real, not silent After CRC validation, the reassembled total is checked against the ACF wire ceiling before re-encoding; on overflow, set_error_response() produces a genuine RequestRejected ErrorResponse. Verified by test with a payload under the Reassembler's own capacity but over the ACF wire ceiling, asserting response_kind_of(resp) == ErrorResponse -- distinct from a separate test proving the Reassembler's own internal kErrTooLarge bound produces fragment_rejected with no wire response, matching c-RCP's own RCP_MOCK_DISPATCH_REJECTED precedent. Symmetric coverage on the response side: an unconfigured response-stream also produces a genuine RequestRejected ErrorResponse rather than dropping silently. ## Bug found and fixed acf::decode_acf_abb()/decode_acf_gbb() are deliberately lenient (a documented Phase-1 TODO) and don't trim wire pad octets off the decoded payload -- left as-is this would have corrupted both the reassembled payload and the fragmented-CRC comparison whenever pad != 0. Added a local trim_wire_pad() helper applied to both intermediate- and final-fragment decodes, scoped to this batch rather than touching the shared decoder (out of D1's fragmentation-only scope). ## Build-clean fix std::array<fragment::Reassembler, N>/std::array<respqueue::RespQueue, N> member declarations with a trailing {} triggered a "converting from initializer list would use explicit constructor" warning (both types have explicit sole constructors) -- fixed by dropping the {}. ## Deferred (doc-only TODO update) apply_cancellation()'s ClearSingle TODO relabeled from TODO(phase4-batch-d) to TODO(phase4-batch-d2) now that D is split; D1's own dispatch_e2e_fragment() still routes everything through admit_and_classify(), so AdmitOutcome::Cancellation remains as unreachable as it was after batch A -- no behavior touched. ## Verification 13 new TEST_CASEs: plain-command-mode passthrough, unresolvable-stream fallback, never-fragmented fallback, genuine 2-fragment reassembly, out-of-order rejection + recovery, fragmented-CRC mismatch + stream- fault latch, both oversized paths, three response-fragmentation scenarios (successful multi-entry split with byte-exact reassembly, unconfigured-queue rejection, max_avtpdu_size-driven tighter splitting), accessor nullptr behavior. include/rcp/mock.hpp: 2162 -> 2723 lines. tests/test_mock.cpp: 1832 -> 2349 lines, 77 -> 89 TEST_CASEs (764 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. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
10 tasks
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), branch
rewrite/v3-from-c-rcp. First of two sub-batches (D1/D2) making up Phase 4's fourth and final batch. Built on batches A (admission), B (response suppression + regmap/discovery), and C (single-member E2E dispatch withRxSequenceGuard/StreamFaultTracker/RxWatchdog).Closes another day-one gap
fragment.hppandrespqueue.hppwere both fully ported in Phase 1 (fixed-capacity, zero heap allocation, with the c-RCP issue #614/#616 oversized-reassembly lesson baked in from day one) but neither was ever wired into any dispatch path. This closes that.Ported
REQ-E2E-038/039/046, REQ-ISELED-025, REQ-FRAG-*. New
dispatch_e2e_fragment()feeds fragments into a per-request-streamfragment::Reassembler, validatescompute_fragmented_crc(), then delegates into the existing single-member dispatch path. Newmaybe_fragment_response()slices an oversized genuine response viafragment::plan()onto arespqueue::RespQueue. Confirmed via source read that c-RCP's own fragment-dispatch has no sequence-gate or TSCF variant reachable here — mirrors batch C's own "NTSCF-only" simplification; both deferred to D2's frame-level dispatch.Oversized-reassembly error path — verified real, not silent
Distinct tests prove: (1) a reassembled total over the ACF wire ceiling produces a genuine
RequestRejectedErrorResponse, and (2) the Reassembler's own internal capacity bound produces a no-wire-response rejection matching c-RCP'sRCP_MOCK_DISPATCH_REJECTEDprecedent. Symmetric coverage on the response side too.Bug found and fixed
acf::decode_acf_abb/gbb()'s known-lenient pad-trimming gap (a documented Phase-1 TODO) would have corrupted both the reassembled payload and the fragmented-CRC comparison wheneverpad != 0. Added a localtrim_wire_pad()helper scoped to this batch.Verification
13 new TEST_CASEs covering plain passthrough, genuine reassembly, out-of-order rejection, CRC mismatch + stream-fault latch, both oversized paths, three response-fragmentation scenarios, accessor behavior.
mock.hpp: 2162→2723 lines.test_mock.cpp: 1832→2349 lines, 77→89 TEST_CASEs (764 assertions).Full tree rebuilds clean (0 errors/warnings), 58/58 ctest suites pass (100%) — independently re-verified from a fresh build.