diff --git a/.fusa-reqs.json b/.fusa-reqs.json index 96d7be1..4006734 100644 --- a/.fusa-reqs.json +++ b/.fusa-reqs.json @@ -267,7 +267,7 @@ { "id": "REQ-E2E-002", "title": "CRC coverage buffer layout", - "text": "rcp::e2e::coverage_buffer shall assemble, in order, the 8-byte big-endian stream_id, the 4-byte big-endian avtp_timestamp (zero-filled when std::nullopt is passed, the NTSCF stand-in), the 10-byte ACF shared header, and the payload; rcp::e2e::compute_crc shall compute the CRC over exactly that buffer.", + "text": "rcp::e2e::coverage_buffer shall assemble, in order, the 8-byte big-endian stream_id, the 4-byte big-endian avtp_timestamp (zero-filled when std::nullopt is passed, the NTSCF stand-in), the ACF Message Info block in its exact on-the-wire byte order, and the payload; rcp::e2e::compute_crc shall compute the CRC over exactly that buffer. The Message Info block is the contiguous 8-byte shared header for ACF_ABB, and for ACF_GBB is the 16-byte form in which the 64-bit big-endian message_timestamp is spliced between the shared header's two quadlets (block octets 0-3 = quadlet 0, octets 4-11 = message_timestamp, octets 12-15 = quadlet 1), so that the CRC covers byte-for-byte the octets rcp::acf::encode_acf_gbb transmits.", "standard": "iso26262", "level": "ASIL-B", "asil": "ASIL-B" @@ -1923,7 +1923,7 @@ { "id": "REQ-WIRE-005", "title": "ACF_GBB carries a 64-bit message_timestamp", - "text": "encode_acf_gbb/decode_acf_gbb shall round-trip the shared ACF header, a 64-bit message_timestamp value, and the payload unchanged for an ACF_GBB (0x0D) message.", + "text": "encode_acf_gbb/decode_acf_gbb shall round-trip the shared ACF header, a 64-bit message_timestamp value, and the payload unchanged for an ACF_GBB (0x0D) message, and shall place them on the wire in the order octets 0-3 = shared-header quadlet 0 (acf_msg_type/acf_msg_length/pad/mtv/rsv/byte_bus_id), octets 4-11 = message_timestamp big-endian, octets 12-15 = shared-header quadlet 1 (evt/rsv/hs/cs/transaction_num/op/rsp/err/ms/read_size_or_segment_num), octets 16 onwards = byte_msg_payload.", "standard": "iso26262", "level": "ASIL-B", "asil": "ASIL-B" @@ -2002,8 +2002,8 @@ }, { "id": "REQ-WIRE-015", - "title": "RELAY spec §15.5 canonical ControlFlags/Message conversion", - "text": "rcp::acf::ControlFlags shall define FlagAck/FlagRead/FlagWrite/FlagResponse/FlagError/FlagMoreSegments at bit values 0x80/0x40/0x20/0x10/0x08/0x04 respectively, matching RELAY spec §15.5's Go definition exactly; rcp::acf::to_message/from_message shall convert between AcfMessageInfo and the RELAY spec §15.5 canonical Message shape, preserving byte_bus_id, transaction_num, read_size_or_segment, and body, and carrying message_timestamp only for the ACF_GBB encoding.", + "title": "RELAY spec \u00a715.5 canonical ControlFlags/Message conversion", + "text": "rcp::acf::ControlFlags shall define FlagAck/FlagRead/FlagWrite/FlagResponse/FlagError/FlagMoreSegments at bit values 0x80/0x40/0x20/0x10/0x08/0x04 respectively, matching RELAY spec \u00a715.5's Go definition exactly; rcp::acf::to_message/from_message shall convert between AcfMessageInfo and the RELAY spec \u00a715.5 canonical Message shape, preserving byte_bus_id, transaction_num, read_size_or_segment, and body, and carrying message_timestamp only for the ACF_GBB encoding.", "standard": "relay", "level": "QM", "asil": "QM" @@ -2611,7 +2611,7 @@ { "id": "REQ-SEQ-001", "title": "message_timestamp-repurposing request_type decode/encode (mtv=0)", - "text": "rcp::request::decode_request_type shall return RequestErrc::timestamp_not_repurposed when mtv is true, shall return RequestErrc::unknown_request_type when the top byte of message_timestamp is not one of the 8 defined opcodes, and shall otherwise recover the opcode and the remaining 7 parameter bytes exactly as produced by encode_request_type for the same inputs; rcp::request::make_conditional_request shall always build an ACF_GBB AcfMessageInfo with mtv clear.", + "text": "rcp::request::decode_request_type shall return RequestErrc::timestamp_not_repurposed when mtv is true, shall return RequestErrc::unknown_request_type when the top byte of message_timestamp is not one of the 8 defined opcodes, and shall otherwise recover the opcode and the remaining 7 parameter bytes exactly as produced by encode_request_type for the same inputs; rcp::request::make_conditional_request shall always build an ACF_GBB AcfMessageInfo with mtv clear. The repurposed slot is the ACF_GBB message_timestamp slot at wire octets 4-11 (see REQ-WIRE-005), whose first octet carries the opcode.", "standard": "iso26262", "level": "ASIL-B", "asil": "ASIL-B" @@ -2987,7 +2987,7 @@ { "id": "REQ-L2-007", "title": "MultiFrame round-trips two ACF_ABB messages packed into one L2 frame", - "text": "rcp::l2::encode_l2_multi_frame/decode_l2_multi_frame shall round-trip two or more ACF_ABB/ACF_GBB messages packed into a single Ethernet frame's AVTPDU, per extraction §12.9.1.1's multiple-requests-in-one-frame requirement, the same as rcp::udp::MultiFrame's equivalent guarantee.", + "text": "rcp::l2::encode_l2_multi_frame/decode_l2_multi_frame shall round-trip two or more ACF_ABB/ACF_GBB messages packed into a single Ethernet frame's AVTPDU, per extraction \u00a712.9.1.1's multiple-requests-in-one-frame requirement, the same as rcp::udp::MultiFrame's equivalent guarantee.", "standard": "iso26262", "level": "ASIL-B", "asil": "ASIL-B" diff --git a/CMakeLists.txt b/CMakeLists.txt index 59af01d..5ef3b10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.21) project(cpp-rcp - VERSION 2.21.0 + VERSION 2.22.0 DESCRIPTION "Remote Control Protocol for automotive zonal architecture (C++)" HOMEPAGE_URL "https://github.com/SoundMatt/cpp-RCP" LANGUAGES CXX diff --git a/README.md b/README.md index 3b5fa11..91af81a 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ reference. | Header | Description | |---|---| | `` | TC18 wire codec, framing half — IEEE 1722 AVTPDU (NTSCF/TSCF) header framing, `StreamId`, `ByteBusId` | -| `` | TC18 wire codec, message half — ACF_ABB/ACF_GBB message format, `AcfMessageInfo` | +| `` | TC18 wire codec, message half — ACF_ABB/ACF_GBB message format, `AcfMessageInfo`. ACF_ABB Message Info is 8 contiguous octets; ACF_GBB is 16, with the 64-bit `message_timestamp` spliced between the header's two quadlets (octets 0–3 quadlet 0, 4–11 timestamp, 12–15 quadlet 1) — corrected in v2.22.0, see ROADMAP.md milestone 66 | | `` | RC Server register-map model (generic + functional config, EP0) | | `` | RC Server lifecycle state machine | | `` | Conditional-request taxonomy (compound, triggered, timed, chained, ...) and sequencers | diff --git a/ROADMAP.md b/ROADMAP.md index 5203e3b..9f07e6e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -134,6 +134,8 @@ work, since every endpoint type's functional config block depends on it. | **Phase 16** | v2.18.0 | Certification refresh | HARA/TARA/FMEA/formal verification/audit pack regenerated against the new requirement set | | **Phase 16** | v2.19.0 | Wire format conformance fix pass | `avtp.hpp`/`acf.hpp` NTSCF/TSCF/ACF Message Info bit-packing re-derived from the specification's own header diagrams; `e2e.hpp` CRC coverage and numeric error-code mapping corrected to match | | **Phase 16** | v2.20.0 | TC18 conformance fix pass II | `acf_msg_length` actually computed; multiple-requests-in-one-frame (§12.9.1.1) supported; error responses carry a Table 27 error code; GPIO exact-length payload check; ISELED gains a real wire codec; PWM_IN rising/falling-edge triggers; `control_data_length` validated | +| **Phase 16** | v2.21.0 | L2 (native Ethernet) transport + UDP Annex J conformance fix | New `l2.hpp` raw-Ethernet transport (EtherType 0x22F0); `udp.hpp` gains Annex J's 4-byte encapsulation sequence number and port 17221 default | +| **Phase 16** | v2.22.0 | ACF_GBB `message_timestamp` wire-position fix | The 64-bit `message_timestamp` is spliced *between* the ACF Message Info header's two quadlets (octet 4), not appended after both (octet 8); `e2e.hpp` CRC coverage follows; wire-breaking for ACF_GBB only | | **Phase 16** | v3.0.0 | **TC18 RCP — General Availability** | First release where cpp-RCP *is* the OPEN Alliance TC18 Remote Control Protocol | --- @@ -175,7 +177,9 @@ incrementally through v2.6.0 per the Phase 13 introduction above. (time-synchronous, client-only) AVTPDU headers (extraction §2.2) - Implement the two ACF message types RCP defines on top of IEEE 1722: ACF_ABB (`0x0E`, no timestamp field) and ACF_GBB (`0x0D`, 64-bit - `message_timestamp` field) (extraction §2.3) + `message_timestamp` field, spliced between the shared header's two + quadlets at octets 4..11 — corrected at milestone 66/v2.22.0) + (extraction §2.3) - Implement the shared `byte_message_info` header fields common to both message types: `acf_msg_type`, `acf_msg_length`, `pad`, `mtv`, `rsv`, `byte_bus_id`, `evt` (ack flag + 3-bit sub-opcode), `hs`, `cs`, @@ -1567,11 +1571,12 @@ already predates milestones 44-61 (still documenting `rcp/firmware.hpp` and other since-deleted packages) and bringing it in line with the current architecture is a substantial, separately-scoped task this milestone's own text does not name. Milestone #63 (Wire Format Conformance Fix Pass, -v2.19.0) is next, ahead of #66 (TC18 RCP General Availability, v3.0.0), +v2.19.0) is next, ahead of #67 (TC18 RCP General Availability, v3.0.0), since it corrects a defect in the exact wire surface GA's own scope requires to be correct. (Milestone #64, TC18 Conformance Fix Pass II, -v2.20.0, and milestone #65, L2 Transport + UDP Annex J Conformance Fix, -v2.21.0, were added later in the same vein — see their own entries.) +v2.20.0; milestone #65, L2 Transport + UDP Annex J Conformance Fix, +v2.21.0; and milestone #66, ACF_GBB `message_timestamp` Wire-Position Fix, +v2.22.0, were added later in the same vein — see their own entries.) ### Retired-model residue cleanup (2026-07-30, v2.19.0 — audit findings cpp-RCP-FS-01..05, cpp-RCP-05, cpp-RCP-07) @@ -1744,9 +1749,11 @@ check/lint/trace and RELAY `conform --strict` were not run locally (the cross-checked against a second, independent presentation of the same header shapes elsewhere in the specification - `rcp/e2e.hpp`'s `coverage_buffer`/`compute_crc`/`verify_crc` gain a - `message_timestamp` parameter and include it, at the correct byte offset, - in an ACF_GBB message's CRC coverage — previously omitted entirely, - understating what the CRC actually protects for every GBB message + `message_timestamp` parameter and include it in an ACF_GBB message's CRC + coverage — previously omitted entirely, understating what the CRC + actually protects for every GBB message. (This milestone put it at the + offset the codec used at the time, which milestone 66/v2.22.0 later + found was itself wrong — see that milestone.) - `rcp/e2e.hpp`'s `E2eErrc::crc_error` gains a numeric-TC18-wire-error-code accessor (`POCI_FAILURE` = 12) for use when building an error response's `byte_msg_payload`, distinct from the enum's own internal @@ -1768,8 +1775,11 @@ environment to interoperate-test against directly, so "full bit-for-bit wire conformance" is not claimed as an absolute fact here either — this milestone's own honest position is "high confidence, derivation-and-cross- check verified, not independently interop-tested," which is why GA -(milestone #66) remains a distinct, later milestone rather than being -folded into this one. +(milestone #67) remains a distinct, later milestone rather than being +folded into this one. (Milestone 66/v2.22.0 vindicated that caution: this +milestone re-derived the shared header's *bit* packing correctly but never +questioned the ACF_GBB message's *byte* geometry around it, which was +wrong the whole time.) Both `AcfMessageInfo`'s and the NTSCF/TSCF headers' public field names are kept stable through this fix — only the bit positions/widths the codec @@ -1977,7 +1987,116 @@ development (see this milestone's own pull request description for the byte-for-byte evidence), ahead of the dedicated CI job doing the same on every future change. -### 66. TC18 RCP — General Availability (v3.0.0) +### 66. ACF_GBB `message_timestamp` Wire-Position Fix (v2.22.0) + +A single-field byte-geometry defect, found by an independent audit of this +repository, that made every ACF_GBB message this library has ever emitted +unreadable by a conformant peer — and every ACF_GBB E2E CRC wrong. + +**The bug.** `encode_acf_gbb`/`decode_acf_gbb` placed the 64-bit +`message_timestamp` *after* the complete 8-byte shared Message Info header, +at wire octet 8. The specification splices it *between* that header's two +4-byte quadlets, at wire octet 4 — which pushes the entire second quadlet +(`evt`/`rsv`/`hs`/`cs`/`transaction_num`/`op`/`rsp`/`err`/`ms`/ +`read_size_or_segment_num`) from octet 8 to octet 12, and the payload from +octet 16 to… also octet 16, which is exactly why nothing caught it: the +message length was right, the round trip through this codec's own encoder +and decoder was right, and only the four bytes in the middle were in the +wrong place. Milestones 63 (v2.19.0) and 64 (v2.20.0) both audited this +header and both missed it, because both were looking at *bit* packing +within the header and at `acf_msg_length`, not at where the header's halves +sit relative to the timestamp. ACF_ABB is entirely unaffected — it has no +timestamp field, so its two quadlets have always been correctly contiguous +at octets 0..7. + +**How the correct layout was verified**, independently and before any code +changed, against the specification PDF itself (figures rendered at high DPI +and read directly, since these layouts exist only as figures, not as +extractable text) rather than against this codec's prior behavior: + +1. The **single-ACF_GBB CRC-coverage figure** draws one "Byte Message Info" + group of three rows in this order: the + `acf_msg_type`/`acf_msg_length`/`pad`/`mtv`/`rsv`/`byte_bus_id` quadlet; + then `message_time_stamp` as a double-height 64-bit block (rendered the + same way that figure's own 64-bit `stream_id` is); then the + `evt`/`rsv`/`hs`/`cs`/`transaction_num`/`op`/`rsp`/`err`/`ms`/ + `read_size` quadlet. +2. **Arithmetic cross-check from that same figure**, which states + `acf_msg_length` = 0x07 for its example: 7 quadlets = 28 octets = + quadlet0(4) + timestamp(8) + quadlet1(4) + payload(7 real + 1 pad = 8) + + CRC32(4). No other placement of a 64-bit timestamp closes that sum. Its + ACF_ABB counterpart states 0x05 = 20 octets = 4 + 4 + (6 real + 2 pad = + 8) + 4, confirming the same two quadlets with no timestamp between them. +3. The **compound-request figure** (an ACF_GBB with `mtv=0`) shows the + fields that repurpose the timestamp slot — `request_type`/ + `cmp_start_state`/`cmp_next_state`/`cmp_sequencer`, then + `cmp_exec_delay`/`cmp_repetitions` — occupying exactly the two quadlets + *between* the same two header quadlets, i.e. octets 4..11. That figure + also independently confirms `rcp/request.hpp`'s existing byte assignment + *within* the slot (opcode in the slot's first octet, seven parameter + octets after it), which needed no change. +4. The **response-field table** lists the Message Info fields in wire order + and places `message_timestamp` ("Present in ACF_GBB, omitted in + ACF_ABB") between `byte_bus_id` — quadlet 0's last field — and `evt` — + quadlet 1's first field. + +**What changed.** + +- `rcp/acf.hpp` gains `kAcfHeaderQuadletLen`/`kAcfGbbTimestampOffset`/ + `kAcfGbbSecondQuadletOffset`/`kAcfGbbMessageInfoLen` and a pair of + ACF_GBB-specific Message Info codecs, + `encode_acf_gbb_message_info`/`decode_acf_gbb_message_info`, which own the + spliced byte geometry while delegating all bit-level packing to the + existing `encode_acf_message_info`/`decode_acf_message_info` — so the bit + layout keeps exactly one definition in the file. + `encode_acf_gbb`/`decode_acf_gbb` and the multi-message walker + `decode_acf_messages` all route through them. +- `rcp/e2e.hpp`'s `coverage_buffer` serialized the ACF_GBB Message Info + block by hand (header, then timestamp) and so inherited the same wrong + order; it now calls `acf::encode_acf_gbb_message_info`, making the + CRC-covered bytes byte-identical to the transmitted bytes *by + construction* rather than by two code paths agreeing. A new assertion in + `tests/test_e2e.cpp` pins that identity directly. +- `rcp/udp.hpp`, `rcp/l2.hpp` and `rcp/record.hpp` needed no change: all + three already route every ACF_GBB message through + `encode_acf_gbb`/`decode_acf_gbb` and hardcode no offsets of their own. + `rcp/request.hpp` likewise needed no change — it deals only in the + *value* of the 64-bit slot, never its wire position — and gained only a + comment recording that, plus the figure-3 confirmation above. +- `.fusa-reqs.json`: `REQ-WIRE-005` and `REQ-E2E-002` now state the exact + octet positions rather than leaving the geometry implicit (`REQ-E2E-002` + also still said "the 10-byte ACF shared header", stale since milestone 63 + made it 8), and `REQ-SEQ-001` records where the repurposed slot actually + sits. + +**Tests.** `tests/test_acf.cpp`'s and `tests/test_e2e.cpp`'s ACF_GBB +byte-sequence expectations had *pinned the bug as correct* — the classic +tautology failure of testing an encoder against its own output. They are +rewritten around literal octet positions derived from the figures above, +with the derivation written out in the test file itself, plus a new +full-message conformance vector reproducing the CRC-coverage figure's own +message (`acf_msg_type` 0x0D, `acf_msg_length` 0x07, `pad` 1, 7+1-octet +payload, CRC32 trailer, 28 octets total) octet for octet, asserted in both +directions (encode to literal, and decode *from* the literal back to +fields, so the decoder is proven against spec-shaped bytes rather than its +own encoder's bytes). The new expectations were mutation-checked: reverting +the two offset constants to their pre-fix values fails 4 test cases across +the two suites, confirming the tests actually discriminate between the two +layouts. Full local build and `ctest` (53/53 suites) pass. + +**Breaking change.** This changes bytes on the wire for every ACF_GBB +message, so a v2.22.0 peer cannot interoperate with a pre-v2.22.0 peer for +GBB traffic (ACF_ABB traffic is unaffected). No public API signature, +struct field, or function name changes — `AcfMessageInfo` and the +`encode_acf_gbb`/`decode_acf_gbb` signatures are identical — so this is a +source-compatible, wire-incompatible release. Given the pre-v2.22.0 bytes +were never conformant with anything but themselves, correcting them is +strictly a move toward the interoperability GA requires; `kVersion`/ +`CMakeLists.txt` bump 2.21.0 → 2.22.0 accordingly, following the same minor +-bump convention milestones 63/64/65 used for their own wire-format +corrections during this pre-GA v2.x line. + +### 67. TC18 RCP — General Availability (v3.0.0) - First release where cpp-RCP's `RCP` conforms to the OPEN Alliance TC18 Remote Control Protocol Specification at the wire level: an RC Client diff --git a/include/rcp/acf.hpp b/include/rcp/acf.hpp index ca581cc..3c8e971 100644 --- a/include/rcp/acf.hpp +++ b/include/rcp/acf.hpp @@ -65,6 +65,22 @@ // length exactly as given, since this codec has always left padding a // caller-owned concern — see the "ACF shared header" section above — rather // than silently appending real pad octets a caller did not ask for). +// +// ACF_GBB timestamp-position conformance note (v2.22.0, issue +// cpp-RCP-GBB-TS): both notes above concern the shared header's *bit* +// packing and its length field; neither noticed that the ACF_GBB message's +// *byte* geometry was also wrong. encode_acf_gbb/decode_acf_gbb placed the +// 64-bit message_timestamp after the complete 8-byte shared header (wire +// octet 8), when the specification splices it between that header's two +// quadlets (wire octet 4), pushing evt/rsv/hs/cs/transaction_num/op/rsp/ +// err/ms/read_size_or_segment_num from octet 8 to octet 12. Every ACF_GBB +// message this library emitted before v2.22.0 was therefore unreadable by +// a conformant peer (and vice versa), and every ACF_GBB E2E CRC was +// computed over the wrong byte sequence. Fixed below; see the +// kAcfGbbTimestampOffset constant block for the point-by-point +// verification against the specification's own figures. This is a +// wire-format-breaking change against every prior cpp-RCP release — +// ACF_ABB is entirely unaffected, since it has no timestamp field. #pragma once #include @@ -193,6 +209,52 @@ struct AcfMessageInfo { constexpr size_t kAcfCommonHeaderLen = 8; constexpr size_t kAcfGbbTimestampLen = 8; +// ── ACF_GBB Message Info wire geometry (v2.22.0 fix) ────────────────────────── +// The 8 shared-header bytes above are *contiguous* on the wire only for +// ACF_ABB. For ACF_GBB the specification splices the 64-bit +// message_timestamp **between** the shared header's two quadlets, not after +// both of them — so an ACF_GBB Message Info block is 16 bytes laid out as: +// +// offset 0..3 shared-header quadlet 0 +// (acf_msg_type / acf_msg_length / pad / mtv / rsv / byte_bus_id) +// offset 4..11 message_timestamp, 64-bit big-endian +// offset 12..15 shared-header quadlet 1 +// (evt / rsv / hs / cs / transaction_num / op / rsp / err / ms / +// read_size_or_segment_num) +// offset 16.. byte_msg_payload +// +// Independently verified against the specification's own figures, not +// inferred from this codec's prior behavior (which had the timestamp after +// *both* header quadlets, at offset 8, pushing quadlet 1 to offset 8 +// instead of 12 — the bug this constant block exists to prevent +// recurring): +// (a) The single-ACF_GBB CRC-coverage figure draws one "Byte Message +// Info" group of three rows in this exact order: the +// acf_msg_type/acf_msg_length/pad/mtv/rsv/byte_bus_id quadlet, then +// message_time_stamp rendered as a double-height 64-bit block (the +// same way that figure's own 64-bit stream_id is drawn), then the +// evt/rsv/hs/cs/transaction_num/op/rsp/err/ms/read_size quadlet. +// (b) That figure's own stated acf_msg_length (7 quadlets = 28 octets) +// is only reproducible with a 64-bit timestamp inside the Message +// Info block: quadlet0(4) + timestamp(8) + quadlet1(4) + payload +// (7 real + 1 pad = 8) + CRC32(4) = 28. Its ACF_ABB counterpart, with +// no timestamp at all, states 5 quadlets = 20 octets: 4 + 4 + (6 real +// + 2 pad = 8) + 4 = 20. Both check out exactly. +// (c) The compound-request figure (an ACF_GBB with mtv=0) shows the +// mtv=0 repurposing fields — request_type/cmp_start_state/ +// cmp_next_state/cmp_sequencer, then cmp_exec_delay/cmp_repetitions — +// occupying exactly the two quadlets *between* the same two header +// quadlets, i.e. octets 4..11, which is only consistent with the +// timestamp slot they repurpose living there too. +// (d) The response-field table lists the Message Info fields in wire +// order and places message_timestamp ("Present in ACF_GBB, omitted in +// ACF_ABB") between byte_bus_id (quadlet 0's last field) and evt +// (quadlet 1's first field). +constexpr size_t kAcfHeaderQuadletLen = 4; +constexpr size_t kAcfGbbTimestampOffset = 4; // == kAcfHeaderQuadletLen +constexpr size_t kAcfGbbSecondQuadletOffset = 12; // == kAcfHeaderQuadletLen + kAcfGbbTimestampLen +constexpr size_t kAcfGbbMessageInfoLen = 16; // == kAcfCommonHeaderLen + kAcfGbbTimestampLen + // compute_acf_msg_length computes the wire acf_msg_length value (quadlets, // counted over the *entire* ACF message: the 8-byte shared header, the // 8-byte message_timestamp for ACF_GBB, and the payload as given) for a @@ -296,10 +358,42 @@ inline void decode_acf_message_info(const uint8_t* in8, AcfMessageInfo& out) noe out.read_size_or_segment_num = static_cast(((in8[6] & 0x0F) << 8) | in8[7]); } +// ── ACF_GBB Message Info: the spliced 16-byte form ──────────────────────────── +// encode_acf_gbb_message_info/decode_acf_gbb_message_info are the ACF_GBB +// analogues of encode_acf_message_info/decode_acf_message_info above. They +// deliberately reuse those two functions for the bit-level packing (so the +// bit layout has exactly one definition in this file) and only own the +// ACF_GBB-specific *byte geometry*: quadlet 0, then the 64-bit +// message_timestamp, then quadlet 1 — see the kAcfGbbTimestampOffset / +// kAcfGbbSecondQuadletOffset comment block above for the verification of +// that ordering against the specification's own figures. `out16`/`in16` +// must point at kAcfGbbMessageInfoLen (16) accessible bytes. +inline void encode_acf_gbb_message_info(const AcfMessageInfo& info, uint64_t message_timestamp, + uint8_t* out16) noexcept { + uint8_t hdr[kAcfCommonHeaderLen]; + encode_acf_message_info(info, hdr); + std::copy(hdr, hdr + kAcfHeaderQuadletLen, out16); + avtp::detail::put_u64(out16 + kAcfGbbTimestampOffset, message_timestamp); + std::copy(hdr + kAcfHeaderQuadletLen, hdr + kAcfCommonHeaderLen, + out16 + kAcfGbbSecondQuadletOffset); +} + +inline void decode_acf_gbb_message_info(const uint8_t* in16, AcfMessageInfo& out_info, + uint64_t& out_message_timestamp) noexcept { + uint8_t hdr[kAcfCommonHeaderLen]; + std::copy(in16, in16 + kAcfHeaderQuadletLen, hdr); + std::copy(in16 + kAcfGbbSecondQuadletOffset, + in16 + kAcfGbbSecondQuadletOffset + kAcfHeaderQuadletLen, + hdr + kAcfHeaderQuadletLen); + decode_acf_message_info(hdr, out_info); + out_message_timestamp = avtp::detail::get_u64(in16 + kAcfGbbTimestampOffset); +} + // ── ACF_ABB / ACF_GBB message encode / decode ───────────────────────────────── // ACF_ABB carries no timestamp field at all; ACF_GBB always reserves a -// 64-bit message_timestamp slot immediately after the shared header, -// regardless of whether `mtv` marks it valid (extraction §2.3, §2.7). +// 64-bit message_timestamp slot inside the Message Info block (spliced +// between the block's two header quadlets, see above), regardless of +// whether `mtv` marks it valid (extraction §2.3, §2.7). // peek_acf_msg_type reads the 7-bit acf_msg_type field out of a raw buffer's // first octet without decoding the rest of the header. Needed because @@ -356,11 +450,10 @@ inline std::vector encode_acf_gbb(AcfMessageInfo info, uint64_t message // compute it, nonzero is an explicit caller override" contract. if (info.acf_msg_length == 0) info.acf_msg_length = compute_acf_msg_length(info.acf_msg_type, payload.size()); - std::vector buf(kAcfCommonHeaderLen + kAcfGbbTimestampLen + payload.size()); - encode_acf_message_info(info, buf.data()); - avtp::detail::put_u64(&buf[kAcfCommonHeaderLen], message_timestamp); + std::vector buf(kAcfGbbMessageInfoLen + payload.size()); + encode_acf_gbb_message_info(info, message_timestamp, buf.data()); std::copy(payload.begin(), payload.end(), - buf.begin() + static_cast(kAcfCommonHeaderLen + kAcfGbbTimestampLen)); + buf.begin() + static_cast(kAcfGbbMessageInfoLen)); return buf; } @@ -370,11 +463,10 @@ inline std::error_code decode_acf_gbb(const uint8_t* b, size_t len, std::vector& out_payload) { if (len < 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); if (peek_acf_msg_type(b) != kAcfMsgTypeGbb) return make_error_code(AcfErrc::bad_acf_msg_type); - if (len < kAcfCommonHeaderLen + kAcfGbbTimestampLen) + if (len < kAcfGbbMessageInfoLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); - decode_acf_message_info(b, out_info); - out_message_timestamp = avtp::detail::get_u64(&b[kAcfCommonHeaderLen]); - out_payload.assign(b + kAcfCommonHeaderLen + kAcfGbbTimestampLen, b + len); + decode_acf_gbb_message_info(b, out_info, out_message_timestamp); + out_payload.assign(b + kAcfGbbMessageInfoLen, b + len); return {}; } @@ -432,14 +524,19 @@ inline std::error_code decode_acf_messages(const uint8_t* b, size_t len, std::ve break; } - const size_t fixed_len = kAcfCommonHeaderLen + (is_gbb ? kAcfGbbTimestampLen : 0); + // For ACF_GBB the fixed part is the whole 16-byte spliced Message + // Info block (quadlet0 + timestamp + quadlet1); for ACF_ABB it is + // the contiguous 8-byte header. + const size_t fixed_len = is_gbb ? kAcfGbbMessageInfoLen : kAcfCommonHeaderLen; if (remaining < fixed_len) { if (out.empty()) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); break; } AcfMessageInfo info; - decode_acf_message_info(b + off, info); + uint64_t message_timestamp = 0; + if (is_gbb) decode_acf_gbb_message_info(b + off, info, message_timestamp); + else decode_acf_message_info(b + off, info); size_t msg_bytes = static_cast(info.acf_msg_length) * 4; if (msg_bytes < fixed_len || msg_bytes > remaining) { @@ -454,7 +551,7 @@ inline std::error_code decode_acf_messages(const uint8_t* b, size_t len, std::ve entry.info = info; const size_t payload_off = off + fixed_len; const size_t payload_len = msg_bytes - fixed_len; - if (is_gbb) entry.message_timestamp = avtp::detail::get_u64(&b[off + kAcfCommonHeaderLen]); + entry.message_timestamp = message_timestamp; // 0 for ACF_ABB, which has no such field entry.payload.assign(b + payload_off, b + payload_off + payload_len); out.push_back(std::move(entry)); diff --git a/include/rcp/e2e.hpp b/include/rcp/e2e.hpp index 7143e64..53cfc6e 100644 --- a/include/rcp/e2e.hpp +++ b/include/rcp/e2e.hpp @@ -251,6 +251,11 @@ inline void put_u64_be(std::vector& buf, uint64_t v) { // inserts it into the buffer when `info.acf_msg_type == acf::kAcfMsgTypeGbb`, // regardless of what the caller passes for a non-GBB `info`, so a caller // cannot accidentally cover 8 bytes of timestamp for an ACF_ABB message. +// When it is inserted it goes at its real wire position — spliced between +// the Message Info block's two header quadlets (block offsets 4..11), i.e. +// buffer offsets 16..23 after the 8-byte stream_id and 4-byte +// avtp_timestamp — not after both quadlets, which is where this function +// put it before v2.22.0. // `info` must already have apply_acf_length_adjustment() applied if the // caller wants the trailer's length reflected in the coverage — this // function only serializes whatever AcfMessageInfo (and message_timestamp) @@ -269,12 +274,24 @@ inline std::vector coverage_buffer(const avtp::StreamId& stream_id, detail::put_u64_be(buf, stream_id.to_u64()); detail::put_u32_be(buf, avtp_timestamp.value_or(0)); // zero-filled stand-in under NTSCF - uint8_t hdr[acf::kAcfCommonHeaderLen]; - acf::encode_acf_message_info(info, hdr); - buf.insert(buf.end(), hdr, hdr + acf::kAcfCommonHeaderLen); - + // The Message Info block is serialized here in exactly the byte order + // it has on the wire, which differs between the two message types: for + // ACF_ABB the 8 header bytes are contiguous, while for ACF_GBB the + // 64-bit message_timestamp is spliced *between* the header's two + // quadlets (bytes 4..11 of a 16-byte block), not appended after both of + // them. Going through acf::encode_acf_gbb_message_info rather than + // re-laying-out the fields here keeps this coverage buffer and + // acf::encode_acf_gbb byte-identical by construction — the CRC must + // cover the bytes that are actually transmitted, so any divergence + // between the two would make every ACF_GBB CRC wrong on the wire. if (is_gbb) { - detail::put_u64_be(buf, message_timestamp.value_or(0)); + uint8_t hdr[acf::kAcfGbbMessageInfoLen]; + acf::encode_acf_gbb_message_info(info, message_timestamp.value_or(0), hdr); + buf.insert(buf.end(), hdr, hdr + acf::kAcfGbbMessageInfoLen); + } else { + uint8_t hdr[acf::kAcfCommonHeaderLen]; + acf::encode_acf_message_info(info, hdr); + buf.insert(buf.end(), hdr, hdr + acf::kAcfCommonHeaderLen); } buf.insert(buf.end(), payload.begin(), payload.end()); diff --git a/include/rcp/request.hpp b/include/rcp/request.hpp index f029957..9ddaca0 100644 --- a/include/rcp/request.hpp +++ b/include/rcp/request.hpp @@ -143,6 +143,20 @@ inline std::error_code make_error_code(RequestErrc e) noexcept { // kinds. The mandatory "standard" request kind (rcp::acf::RequestKind) has // no opcode here at all — it is always carried as ACF_ABB, which has no // message_timestamp slot to repurpose in the first place. +// +// Wire-position note (v2.22.0): this header deals only in the *value* of +// that 64-bit slot, never in where it sits on the wire, so the v2.22.0 fix +// that moved the slot to its real position (spliced between the ACF_GBB +// Message Info block's two header quadlets, octets 4..11, rather than +// after both of them at octets 8..15 — see rcp/acf.hpp's +// kAcfGbbTimestampOffset comment block) changed nothing here. The +// specification's own compound-request figure independently confirms this +// header's byte assignment within the slot: at octets 4..11 of an mtv=0 +// ACF_GBB it draws request_type / cmp_start_state / cmp_next_state / +// cmp_sequencer then cmp_exec_delay / cmp_repetitions — i.e. the opcode is +// the slot's first (most significant) byte and the parameters are the +// remaining seven, exactly as encode_request_type/decode_request_type +// below already treated them. // ROADMAP.md milestone 50 (v2.6.0) adds three more: the MSB-set (`0x8x`) // safety-tagged variants of compound, compound-wait, and triggered. Each diff --git a/include/rcp/version.hpp b/include/rcp/version.hpp index df7ec9d..5499f74 100644 --- a/include/rcp/version.hpp +++ b/include/rcp/version.hpp @@ -6,6 +6,6 @@ namespace rcp { // Semantic version of the cpp-RCP implementation (matches the latest git tag). -constexpr std::string_view kVersion = "2.21.0"; +constexpr std::string_view kVersion = "2.22.0"; } // namespace rcp diff --git a/tests/test_acf.cpp b/tests/test_acf.cpp index 618232c..9a70e83 100644 --- a/tests/test_acf.cpp +++ b/tests/test_acf.cpp @@ -79,7 +79,7 @@ TEST_CASE("ACF_GBB round-trips a 64-bit message_timestamp alongside the shared h uint64_t ts = 0x0123456789ABCDEFULL; std::vector payload = {0xAA, 0xBB, 0xCC, 0xDD}; auto frame = encode_acf_gbb(info, ts, payload); - REQUIRE(frame.size() == kAcfCommonHeaderLen + kAcfGbbTimestampLen + payload.size()); + REQUIRE(frame.size() == kAcfGbbMessageInfoLen + payload.size()); REQUIRE(peek_acf_msg_type(frame.data()) == kAcfMsgTypeGbb); AcfMessageInfo out; @@ -94,19 +94,51 @@ TEST_CASE("ACF_GBB round-trips a 64-bit message_timestamp alongside the shared h // ── Hand-computed expected-byte-sequence vectors ────────────────────────────── // Every byte below is computed by hand from the field values and this file's // own derived bit layout (see acf.hpp's "ACF shared header" comment) — not -// copied from anywhere. Byte-by-byte derivation (MSB-first bit numbering, -// bit0 = MSB of byte0, matching the specification's own diagrams): +// copied from anywhere, and in particular not read back out of the +// encoder's own output (which would only prove the encoder agrees with +// itself). Byte-by-byte derivation (MSB-first bit numbering, bit0 = MSB of +// byte0, matching the specification's own diagrams): // +// Shared-header quadlet 0 (wire octets 0..3 of every ACF message): // byte0 = (acf_msg_type[6:0] << 1) | acf_msg_length[8] // byte1 = acf_msg_length[7:0] // byte2 = (pad[1:0] << 6) | (mtv << 5) | (rsv=00 << 3) | byte_bus_id[10:8] // byte3 = byte_bus_id[7:0] -// byte4 = (evt[3:0] << 4) | (rsv=00 << 2) | (hs << 1) | cs +// Shared-header quadlet 1: +// byte0 = (evt[3:0] << 4) | (rsv=00 << 2) | (hs << 1) | cs // where evt[3:0] = (evt_ack << 3) | evt_op[2:0] -// byte5 = transaction_num -// byte6 = (op << 7) | (rsp << 6) | (err << 5) | (ms << 4) | read_size[11:8] -// byte7 = read_size[7:0] -// (ACF_GBB only) bytes8-15 = message_timestamp, big-endian +// byte1 = transaction_num +// byte2 = (op << 7) | (rsp << 6) | (err << 5) | (ms << 4) | read_size[11:8] +// byte3 = read_size[7:0] +// +// The two quadlets' *positions* differ by message type, and this is the +// one thing that must be pinned from the specification rather than from +// this codec (see acf.hpp's kAcfGbbTimestampOffset comment block for the +// full verification, summarized here): +// +// ACF_ABB — no message_timestamp field exists at all: +// octets 0..3 quadlet 0 +// octets 4..7 quadlet 1 +// octets 8.. byte_msg_payload +// +// ACF_GBB — the specification's single-ACF_GBB CRC-coverage figure draws +// one "Byte Message Info" group whose three rows are, in order, quadlet +// 0, then message_time_stamp as a double-height 64-bit block, then +// quadlet 1; its compound-request figure (an mtv=0 ACF_GBB) likewise +// puts the fields that repurpose the timestamp slot between the same two +// quadlets; and its response-field table lists message_timestamp +// ("Present in ACF_GBB, omitted in ACF_ABB") between byte_bus_id +// (quadlet 0's last field) and evt (quadlet 1's first field): +// octets 0..3 quadlet 0 +// octets 4..11 message_timestamp, big-endian +// octets 12..15 quadlet 1 +// octets 16.. byte_msg_payload +// Arithmetic cross-check from the same figure: it states +// acf_msg_length = 0x07 quadlets = 28 octets for a 7-real-byte, +// 1-pad-byte payload with a CRC32 trailer, and 4 + 8 + 4 + 8 + 4 = 28 +// only works out with the timestamp inside the Message Info block. (Its +// ACF_ABB counterpart states 0x05 = 20 octets: 4 + 4 + 8 + 4 = 20.) +// // remaining bytes = payload, unchanged TEST_CASE("ACF_ABB hand-computed expected byte sequence", "[acf][REQ-WIRE-004][REQ-WIRE-006]") { @@ -169,28 +201,85 @@ TEST_CASE("ACF_GBB hand-computed expected byte sequence", "[acf][REQ-WIRE-005][R info.ms = true; info.read_size_or_segment_num = 4095; // 12 bits, max value: 0xFFF - // byte0 = (0x0D << 1) | (7 >> 8 & 1) = 0x1A | 0 = 0x1A - // byte1 = 7 & 0xFF = 0x07 - // byte2 = (1 << 6) | (1 << 5) | (300 >> 8 & 7) = 0x40 | 0x20 | 0x01 = 0x61 - // byte3 = 300 & 0xFF = 0x2C - // byte4 = (0x3 << 4) | (0 << 1) | 1 = 0x30 | 0x01 = 0x31 - // byte5 = 200 = 0xC8 - // byte6 = 0 | (1<<6) | (1<<5) | (1<<4) | (4095 >> 8 & 0xF) = 0x40|0x20|0x10|0x0F = 0x7F - // byte7 = 4095 & 0xFF = 0xFF - const std::vector expected_header = {0x1A, 0x07, 0x61, 0x2C, 0x31, 0xC8, 0x7F, 0xFF}; + // Quadlet 0 (wire octets 0..3): + // byte0 = (0x0D << 1) | (7 >> 8 & 1) = 0x1A | 0 = 0x1A + // byte1 = 7 & 0xFF = 0x07 + // byte2 = (1 << 6) | (1 << 5) | (300 >> 8 & 7) = 0x40 | 0x20 | 0x01 = 0x61 + // byte3 = 300 & 0xFF = 0x2C + // Quadlet 1 (wire octets 12..15 for ACF_GBB — see this file's layout + // comment above; these are octets 4..7 only for ACF_ABB): + // byte0 = (0x3 << 4) | (0 << 1) | 1 = 0x30 | 0x01 = 0x31 + // byte1 = 200 = 0xC8 + // byte2 = 0 | (1<<6) | (1<<5) | (1<<4) | (4095 >> 8 & 0xF) = 0x40|0x20|0x10|0x0F = 0x7F + // byte3 = 4095 & 0xFF = 0xFF + const std::vector expected_q0 = {0x1A, 0x07, 0x61, 0x2C}; + const std::vector expected_q1 = {0x31, 0xC8, 0x7F, 0xFF}; const uint64_t ts = 0x1122334455667788ULL; const std::vector expected_ts = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88}; const std::vector payload = {0xAA, 0xBB, 0xCC, 0xDD}; + // The whole expected ACF_GBB message, written out as one literal at the + // spec-derived octet positions rather than assembled from the encoder's + // own constants: quadlet 0 || message_timestamp || quadlet 1 || payload. + const std::vector expected_frame = { + // octets 0..3 — quadlet 0 + 0x1A, 0x07, 0x61, 0x2C, + // octets 4..11 — message_timestamp, big-endian (spliced *between* + // the two header quadlets, per the specification's + // single-ACF_GBB CRC-coverage figure and its + // compound-request figure) + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, + // octets 12..15 — quadlet 1 + 0x31, 0xC8, 0x7F, 0xFF, + // octets 16..19 — byte_msg_payload + 0xAA, 0xBB, 0xCC, 0xDD, + }; + auto frame = encode_acf_gbb(info, ts, payload); - REQUIRE(frame.size() == expected_header.size() + expected_ts.size() + payload.size()); - std::vector header(frame.begin(), frame.begin() + static_cast(kAcfCommonHeaderLen)); - REQUIRE(header == expected_header); - std::vector ts_bytes(frame.begin() + static_cast(kAcfCommonHeaderLen), - frame.begin() + static_cast(kAcfCommonHeaderLen + kAcfGbbTimestampLen)); - REQUIRE(ts_bytes == expected_ts); - REQUIRE(std::vector(frame.begin() + static_cast(kAcfCommonHeaderLen + kAcfGbbTimestampLen), - frame.end()) == payload); + REQUIRE(frame.size() == 20); // 4 + 8 + 4 + 4 + REQUIRE(frame == expected_frame); + + // Same assertion again, sliced field by field at literal offsets, so a + // failure names which field moved rather than just "the buffer differs". + REQUIRE(std::vector(frame.begin() + 0, frame.begin() + 4) == expected_q0); + REQUIRE(std::vector(frame.begin() + 4, frame.begin() + 12) == expected_ts); + REQUIRE(std::vector(frame.begin() + 12, frame.begin() + 16) == expected_q1); + REQUIRE(std::vector(frame.begin() + 16, frame.end()) == payload); + + // Regression guard for the pre-v2.22.0 layout specifically: back then + // octet 4 was transaction-info (quadlet 1's evt/hs/cs byte) and octet 8 + // was the timestamp's first byte. Those two octets are the cheapest + // possible discriminator between the two layouts. + REQUIRE(frame[4] == 0x11); // timestamp MSB, not 0x31 (quadlet 1 byte0) + REQUIRE(frame[8] == 0x55); // still inside the timestamp, not 0x31 either + + // Decoding the hand-written literal (not the encoder's output) must + // recover every field — this is the direction that proves the decoder + // reads real spec-shaped bytes, not just its own encoder's bytes. + { + AcfMessageInfo from_literal; + uint64_t from_literal_ts = 0; + std::vector from_literal_payload; + REQUIRE_FALSE(decode_acf_gbb(expected_frame.data(), expected_frame.size(), from_literal, + from_literal_ts, from_literal_payload)); + REQUIRE(from_literal.acf_msg_type == kAcfMsgTypeGbb); + REQUIRE(from_literal.acf_msg_length == 7); + REQUIRE(from_literal.pad == 1); + REQUIRE(from_literal.mtv == true); + REQUIRE(from_literal.byte_bus_id == 300); + REQUIRE(from_literal.evt_ack == false); + REQUIRE(from_literal.evt_op == 3); + REQUIRE(from_literal.hs == false); + REQUIRE(from_literal.cs == true); + REQUIRE(from_literal.transaction_num == 200); + REQUIRE(from_literal.op == false); + REQUIRE(from_literal.rsp == true); + REQUIRE(from_literal.err == true); + REQUIRE(from_literal.ms == true); + REQUIRE(from_literal.read_size_or_segment_num == 4095); + REQUIRE(from_literal_ts == ts); + REQUIRE(from_literal_payload == payload); + } AcfMessageInfo decoded; uint64_t decoded_ts = 0; diff --git a/tests/test_e2e.cpp b/tests/test_e2e.cpp index c109a4b..e0fdffd 100644 --- a/tests/test_e2e.cpp +++ b/tests/test_e2e.cpp @@ -104,33 +104,64 @@ TEST_CASE("coverage_buffer layout is stream_id + avtp_timestamp + ACF header + p REQUIRE(buf[buf.size() - 1] == 3); } -// cpp-RCP-N2-03: for ACF_GBB, the wire carries an 8-byte message_timestamp -// between the Message Info header and the payload (see rcp/acf.hpp's -// encode_acf_gbb) — coverage_buffer must include those exact 8 bytes at -// that exact offset, not omit them. -TEST_CASE("coverage_buffer inserts the 8-byte message_timestamp between the ACF header and " - "payload for ACF_GBB", +// cpp-RCP-N2-03 / cpp-RCP-GBB-TS: for ACF_GBB the wire carries an 8-byte +// message_timestamp *inside* the Message Info block, spliced between its +// two header quadlets — the specification's single-ACF_GBB CRC-coverage +// figure draws one "Byte Message Info" group whose rows are, in order, +// the acf_msg_type/acf_msg_length/pad/mtv/rsv/byte_bus_id quadlet, then +// message_time_stamp as a double-height 64-bit block, then the +// evt/rsv/hs/cs/transaction_num/op/rsp/err/ms/read_size quadlet. The CRC +// coverage buffer must reproduce that byte order exactly, because the CRC +// has to cover the bytes actually transmitted. Before v2.22.0 this test +// pinned the wrong layout (timestamp after *both* quadlets, at coverage +// offset 20); the offsets below are derived from the figure, not from +// coverage_buffer's own output. +TEST_CASE("coverage_buffer splices the 8-byte message_timestamp between the ACF header's two " + "quadlets for ACF_GBB", "[e2e][REQ-E2E-002]") { auto sid = make_stream_id(0x02, 0x0003); AcfMessageInfo info; info.acf_msg_type = rcp::acf::kAcfMsgTypeGbb; info.byte_bus_id = 9; info.mtv = true; + info.transaction_num = 0x5A; std::vector payload{0x11, 0x22}; const uint64_t ts = 0x0102030405060708ULL; + // Set explicitly so coverage_buffer (which never auto-fills) and + // encode_acf_gbb (which auto-fills only a 0) serialize the same header. + info.acf_msg_length = rcp::acf::compute_acf_msg_length(rcp::acf::kAcfMsgTypeGbb, payload.size()); auto buf = coverage_buffer(sid, uint32_t{0}, info, ts, payload); - REQUIRE(buf.size() == 8 + 4 + rcp::acf::kAcfCommonHeaderLen + rcp::acf::kAcfGbbTimestampLen + - payload.size()); - - // The 8 message_timestamp bytes sit right after stream_id(8) + - // avtp_timestamp(4) + Message Info(8) = offset 20, big-endian. - const size_t ts_off = 8 + 4 + rcp::acf::kAcfCommonHeaderLen; - REQUIRE(buf[ts_off + 0] == 0x01); - REQUIRE(buf[ts_off + 7] == 0x08); - // Payload follows immediately after the timestamp, unchanged. - REQUIRE(buf[ts_off + 8] == 0x11); - REQUIRE(buf[ts_off + 8 + 1] == 0x22); + REQUIRE(buf.size() == 8 + 4 + 16 + payload.size()); // stream_id + avtp_ts + 16-byte GBB block + + // Fixed prefix: stream_id (8, big-endian) + avtp_timestamp (4). + // Message Info block therefore begins at coverage offset 12: + // 12..15 quadlet 0 + // 16..23 message_timestamp, big-endian + // 24..27 quadlet 1 + // 28.. payload + const size_t mi_off = 8 + 4; + // Quadlet 0: byte0 = (0x0D << 1) | 0 = 0x1A; byte2 has mtv (bit5) set + // and byte_bus_id's high bits clear = 0x20; byte3 = byte_bus_id & 0xFF. + REQUIRE(buf[mi_off + 0] == 0x1A); + REQUIRE(buf[mi_off + 2] == 0x20); + REQUIRE(buf[mi_off + 3] == 9); + // message_timestamp at block offset 4 (coverage offset 16), big-endian. + REQUIRE(buf[mi_off + 4] == 0x01); + REQUIRE(buf[mi_off + 11] == 0x08); + // Quadlet 1 at block offset 12 (coverage offset 24): byte1 is + // transaction_num, which is the cheapest positive proof the second + // quadlet really moved from offset 4 to offset 12. + REQUIRE(buf[mi_off + 13] == 0x5A); + // Payload follows the complete 16-byte block, unchanged. + REQUIRE(buf[mi_off + 16] == 0x11); + REQUIRE(buf[mi_off + 17] == 0x22); + + // The CRC-covered bytes must be byte-identical to the bytes + // encode_acf_gbb actually puts on the wire — otherwise a peer + // recomputing the CRC over the received frame can never match. + auto wire = rcp::acf::encode_acf_gbb(info, ts, payload); + REQUIRE(std::vector(buf.begin() + static_cast(mi_off), buf.end()) == wire); // An ACF_ABB message with the same nominal message_timestamp argument // must NOT grow by those 8 bytes — the type gates inclusion, not merely @@ -254,7 +285,9 @@ TEST_CASE("acf_msg_length end-to-end matches the specification's own worked exam REQUIRE(info.acf_msg_length == 7); // 0x07 auto frame = encode_acf_gbb(info, ts, payload); - REQUIRE(frame.size() == kAcfCommonHeaderLen + kAcfGbbTimestampLen + payload.size()); // 8+8+8=24 + // 16-byte Message Info block (quadlet0 + 8-byte message_timestamp + + // quadlet1) + 8-byte payload = 24. + REQUIRE(frame.size() == rcp::acf::kAcfGbbMessageInfoLen + payload.size()); // 16+8=24 auto sid = make_stream_id(0x03, 1); uint32_t crc = compute_crc(sid, std::nullopt, info, ts, payload); @@ -263,9 +296,90 @@ TEST_CASE("acf_msg_length end-to-end matches the specification's own worked exam REQUIRE((frame[0] & 0x01) == 0); REQUIRE(frame[1] == 7); + + // The timestamp occupies octets 4..11 of the message, not 8..15 — + // this is the arithmetic the 0x07 figure only closes with the + // timestamp inside the Message Info block (4 + 8 + 4 + 8 + 4 = 28). + REQUIRE(frame[4] == 0x11); // message_timestamp MSB + REQUIRE(frame[11] == 0x88); // message_timestamp LSB + // Quadlet 1 begins at octet 12; with every field left at its + // default in this fixture, all four of its bytes are 0. + REQUIRE(frame[12] == 0x00); + REQUIRE(frame[15] == 0x00); } } +// Full-message conformance vector reproducing the specification's own +// single-ACF_GBB CRC-coverage figure field for field: acf_msg_type = 0x0D, +// acf_msg_length = 0x07, pad = 1, a 7-real-byte payload padded to 8, and a +// trailing CRC32 — 28 octets total. Every octet position below comes from +// that figure's row structure (quadlet 0 || 64-bit message_time_stamp || +// quadlet 1 || byte_msg_payload || CRC32), not from this codec's output. +TEST_CASE("ACF_GBB full-message layout matches the specification's CRC-coverage figure " + "octet for octet", + "[e2e][acf][REQ-WIRE-005][REQ-E2E-002]") { + using rcp::acf::AcfMessageInfo; + + AcfMessageInfo info; + info.acf_msg_type = rcp::acf::kAcfMsgTypeGbb; + info.acf_msg_length = 0x07; // the figure's own stated value + info.pad = 1; // the figure's own stated value + info.mtv = true; + info.byte_bus_id = 0x123; + info.transaction_num = 0x42; + info.op = true; + info.read_size_or_segment_num = 0x0AB; + + const uint64_t ts = 0xDEADBEEFCAFEF00DULL; + // 7 real payload octets + 1 pad octet, matching the figure's PL_Byte1..7 + // plus one 0x00 (padding) cell. + const std::vector payload{0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, 0x00}; + + // Octet-by-octet expectation, hand-derived: + // [0] = (0x0D << 1) | (0x07 >> 8) = 0x1A + // [1] = 0x07 & 0xFF = 0x07 + // [2] = (1 << 6) | (mtv << 5) | (0x123>>8) = 0x40|0x20|0x01 = 0x61 + // [3] = 0x123 & 0xFF = 0x23 + // [4..11] = message_timestamp, big-endian + // [12] = (evt=0 << 4) | (hs=0 << 1) | cs=0 = 0x00 + // [13] = transaction_num = 0x42 + // [14] = (op << 7) | (0x0AB >> 8) = 0x80|0x00 = 0x80 + // [15] = 0x0AB & 0xFF = 0xAB + // [16..23] = byte_msg_payload (7 real + 1 pad) + const std::vector expected{ + 0x1A, 0x07, 0x61, 0x23, + 0xDE, 0xAD, 0xBE, 0xEF, 0xCA, 0xFE, 0xF0, 0x0D, + 0x00, 0x42, 0x80, 0xAB, + 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, 0x00, + }; + + auto frame = rcp::acf::encode_acf_gbb(info, ts, payload); + REQUIRE(frame == expected); + + // ...and with the CRC32 trailer the figure also shows, the frame is + // exactly acf_msg_length * 4 = 28 octets. + auto sid = make_stream_id(0x03, 0x0007); + append_crc(frame, compute_crc(sid, std::nullopt, info, ts, payload)); + REQUIRE(frame.size() == static_cast(info.acf_msg_length) * 4); + REQUIRE(frame.size() == 28); + + // Round trip from the hand-written literal back to fields. + AcfMessageInfo decoded; + uint64_t decoded_ts = 0; + std::vector decoded_payload; + REQUIRE_FALSE(rcp::acf::decode_acf_gbb(expected.data(), expected.size(), decoded, decoded_ts, + decoded_payload)); + REQUIRE(decoded.acf_msg_length == 0x07); + REQUIRE(decoded.pad == 1); + REQUIRE(decoded.mtv == true); + REQUIRE(decoded.byte_bus_id == 0x123); + REQUIRE(decoded.transaction_num == 0x42); + REQUIRE(decoded.op == true); + REQUIRE(decoded.read_size_or_segment_num == 0x0AB); + REQUIRE(decoded_ts == ts); + REQUIRE(decoded_payload == payload); +} + // ── verify_crc / append_crc ──────────────────────────────────────────────────── TEST_CASE("verify_crc accepts a matching CRC and rejects a corrupted one", "[e2e][REQ-E2E-004]") {