Repository navigation
rewrite(phase1): port acf.hpp/avtp.hpp from c-RCP's RC5-conformant reference - #130
Merged
Merged
Conversation
…ference Part of the v3.0.0 ground-up rewrite (cpp-RCP #129, ROADMAP.md Phase 17), branch rewrite/v3-from-c-rcp. First module pair: the wire-protocol core everything else in Phase 1 depends on. Real content deltas found and fixed against c-RCP's src/acf.c+include/rcp/ acf.h and src/avtp.c+include/rcp/avtp.h: - ACF_GBB Message Info geometry was wrong (regression from a prior v2.22.0 pass): message_timestamp was spliced *between* the header's two quadlets. c-RCP's rcp_acf_encode_gbb() and REQ-ACF-044 are unanimous the real layout is contiguous -- 8-byte header + 8-byte timestamp + payload. Every ACF_GBB frame this library emitted since v2.22.0 was unreadable by a conformant peer. Reverted to contiguous. - Added missing ACF protocol-logic surface (additive, no renames): pad_len(), peek_msg_type(), header_is_request(), request_header_constraints_valid(), evt_row2_is_plain()/ evt_requests_acknowledge() (Table 33), compound_wait_evt_valid()/ compound_wait_match() (all 8 TC18 §13.5.1 comparison modes), reg_write_len() (TC18 §13.7.1.2, RC5-corrected -2 formula), peek_gbb_request_type(), and response builders build_error_response()/build_acknowledge_response()/ build_acknowledge_rejected_response(). - avtp.hpp: NtscfHeader gained sv/version; TscfHeader gained sv/version/mr/tu/reserved0/reserved1 (round-tripped, decode-populated, defaults reproduce prior encode behavior exactly). Added extend_timestamp() (48-bit gPTP reconstruction), TscfFallback + should_drop_tscf(), tscf_reserved_all_zero() -- TC18 §13.3 rules, previously entirely absent. test_acf.cpp: 24 -> 63 test cases (267 assertions). test_avtp.cpp: 15 -> 34 test cases (86 assertions). The GBB geometry fix broke 3 test_e2e.cpp assertions that hardcoded the old (wrong) spliced layout as their expected value -- corrected those 3 assertions plus their surrounding comments to the contiguous layout; did not touch e2e.hpp's own production logic (that's Phase 2 scope). Deliberately NOT ported from c-RCP, with rationale: - c-RCP's stricter decode_acf_abb/_gbb (rejects a payload not exactly acf_msg_length*4 bytes, trims trailing pad) -- every real payload builder in this tree hands non-quadlet-aligned payloads to encode_* without pre-padding, and test_e2e.cpp asserts decode does NOT trim pad today. Left a TODO(phase1-followup) with the full rationale; revisit once a later phase makes every payload builder pre-pad via pad_len(). - c-RCP's always-recompute/always-auto-pad encode contract -- kept cpp-RCP's existing 0-means-auto-fill/nonzero-means-override contract, since e2e.hpp's apply_acf_length_adjustment() (CRC trailer accounting) is load-bearing on it and e2e.hpp is out of Phase 1 scope. - c-RCP's transport vtable + in-process loopback transport -- cpp-RCP's own idiom (l2.hpp/udp.hpp's concrete per-transport classes, no shared interface, documented in l2.hpp's own header comment) is deliberately different; porting a shared vtable would contradict it. Verified: full tree rebuilds clean (217/217 targets), 53/53 ctest suites pass (100%), zero call-site fixes needed outside the 5 files touched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.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). First module pair of Phase 1 — the wire-protocol core everything else in Phase 1 depends on.
Real content deltas found and fixed
message_timestampwas spliced between the header's two quadlets. c-RCP'srcp_acf_encode_gbb()andREQ-ACF-044are unanimous the real layout is contiguous — 8-byte header + 8-byte timestamp + payload. Every ACF_GBB frame this library emitted since v2.22.0 was unreadable by a conformant peer. Reverted to contiguous.pad_len(),peek_msg_type(),header_is_request(),request_header_constraints_valid(),evt_row2_is_plain()/evt_requests_acknowledge()(Table 33),compound_wait_evt_valid()/compound_wait_match()(all 8 TC18 §13.5.1 comparison modes),reg_write_len()(TC18 §13.7.1.2, RC5-corrected-2formula),peek_gbb_request_type(), and response builders.avtp.hpp:NtscfHeader/TscfHeadergained several previously-missing fields (round-tripped, defaults reproduce prior encode behavior exactly). Addedextend_timestamp(),TscfFallback/should_drop_tscf(),tscf_reserved_all_zero()— TC18 §13.3 rules, previously entirely absent.test_acf.cpp: 24 → 63 test cases (267 assertions).test_avtp.cpp: 15 → 34 test cases (86 assertions).The GBB geometry fix broke 3
test_e2e.cppassertions that hardcoded the old (wrong) spliced layout as their expected value — corrected those 3 assertions + surrounding comments; did not touche2e.hpp's own production logic (Phase 2 scope).Deliberately not ported (with rationale, see commit message for full detail)
decode_acf_abb/_gbb— every real payload builder in this tree hands non-quadlet-aligned payloads toencode_*without pre-padding; left aTODO(phase1-followup).e2e.hpp's CRC trailer accounting is load-bearing on the existing contract;e2e.hppis out of Phase 1 scope.Verified
Full tree rebuilds clean (217/217 targets), 53/53 ctest suites pass (100%), zero call-site fixes needed outside the 5 files touched.
Tracked at #129 (Phase 1 checklist item).