From ea02b94641d172a822e4cf04274cf40217c0cdd6 Mon Sep 17 00:00:00 2001 From: Matt <47545907+SoundMatt@users.noreply.github.com> Date: Thu, 30 Jul 2026 11:17:34 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20v2.19.0=20=E2=80=94=20wire=20format?= =?UTF-8?q?=20conformance=20fix=20pass=20(=C2=A7Phase=2016=20milestone=206?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-derives rcp/avtp.hpp's NTSCF/TSCF header bit-packing and rcp/acf.hpp's ACF_ABB/ACF_GBB Message Info bit-packing field by field from the OPEN Alliance TC18 Remote Control Protocol Specification v0.5.1_RC's own bit-position diagrams, replacing the self-admitted "not full bit-for-bit wire conformance" placeholder encoding landed at milestone 44 (v2.0.0). - avtp.hpp: NTSCF is 12 bytes (was 14), TSCF is 24 bytes (was 18); sequence_num is an 8-bit rolling counter (was 16-bit); ntscf_data_length is an 11-bit sub-octet field; TSCF's stream_data_length lives in a dedicated 4-byte word after a previously-missing 4-byte reserved word. ByteBusId widens uint8_t -> uint16_t (byte_bus_id is an 11-bit wire field). - acf.hpp: the Message Info header is 8 bytes (was 10), matching the specification's own two worked CRC examples that label it "8 byte Message Info"; acf_msg_type/acf_msg_length share a byte (7+9 bits), byte_bus_id is 11 bits, evt is a 4-bit field, read_size_or_segment_num is 12 bits. Fixes two real dispatch bugs this exposed elsewhere in the tree (rcp/udp.hpp, rcp/record.hpp compared a raw wire byte against kAcfMsgTypeAbb/kAcfMsgTypeGbb directly, which almost never matches once acf_msg_type shares byte0 with acf_msg_length's MSB) via a new peek_acf_msg_type() helper. Also corrects make_response/response_kind_of: every response (including Acknowledge) carries rsp=1 per the specification's own response table; Acknowledge is distinguished by evt[3:0] == 0xF, not by rsp being clear. - e2e.hpp: 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). E2eErrc::crc_error gains a numeric TC18 wire error code accessor (POCI_FAILURE = 12). - acf.hpp gains ControlFlags/Message, an additive, documented thin view over AcfMessageInfo matching RELAY spec §15.5's canonical decoded-frame shape. Closes #73, #66, #76, #89. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> --- .fusa-reqs.json | 8 + ROADMAP.md | 65 +++++++- include/rcp/acf.hpp | 356 ++++++++++++++++++++++++++++++++--------- include/rcp/avtp.hpp | 110 ++++++++++--- include/rcp/e2e.hpp | 113 +++++++++---- include/rcp/record.hpp | 7 +- include/rcp/udp.hpp | 7 +- tests/test_acf.cpp | 263 +++++++++++++++++++++++++++++- tests/test_avtp.cpp | 101 +++++++++++- tests/test_e2e.cpp | 98 ++++++++++-- tests/test_udp.cpp | 6 +- 11 files changed, 984 insertions(+), 150 deletions(-) diff --git a/.fusa-reqs.json b/.fusa-reqs.json index 44c56f4..9a6d3c1 100644 --- a/.fusa-reqs.json +++ b/.fusa-reqs.json @@ -2432,6 +2432,14 @@ "level": "ASIL-B", "asil": "ASIL-B" }, + { + "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.", + "standard": "relay", + "level": "QM", + "asil": "QM" + }, { "id": "REQ-LIFECYCLE-001", "title": "ServerState byte encoding", diff --git a/ROADMAP.md b/ROADMAP.md index ab6882c..be86cb9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ work, since every endpoint type's functional config block depends on it. | **Phase 15** | v2.16.0 | C ABI & CLI rebuild | `capi.h`/`capi_impl.hpp`, `cli.hpp` rebuilt against the new request model | | **Phase 16** | v2.17.0 | Deprecation sweep | Remove packages with no TC18 analog (see disposition table) | | **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** | v3.0.0 | **TC18 RCP — General Availability** | First release where cpp-RCP *is* the OPEN Alliance TC18 Remote Control Protocol | --- @@ -1564,10 +1565,66 @@ wire-format or CLI-surface code. `README.md` is left unmodified: it 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 (TC18 RCP General Availability, v3.0.0) -is next and, per its own scope, is the final milestone in this roadmap. - -### 63. TC18 RCP — General Availability (v3.0.0) +text does not name. Milestone #63 (Wire Format Conformance Fix Pass, +v2.19.0) is next, ahead of #64 (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. + +### 63. Wire Format Conformance Fix Pass (v2.19.0) + +- `rcp/avtp.hpp`'s NTSCF/TSCF header bit-packing and `rcp/acf.hpp`'s + ACF_ABB/ACF_GBB Message Info bit-packing, both landed at milestone 44 + (v2.0.0) with a self-admitted "not full bit-for-bit wire conformance" + disclaimer, are re-derived field by field from the specification's own + bit-position diagrams (counted pixel by pixel from the source PDF, since + those diagrams exist only as figures, not extractable text) and + 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 +- `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 + `std::error_code` ordinal +- Additive only: `rcp/acf.hpp` gains `ControlFlags`/`Message`, a thin, + documented view over `AcfMessageInfo` matching RELAY spec §15.5's + canonical decoded-frame shape (no existing type is restructured to + accommodate it) + +**Done (v2.19.0):** See this milestone's own pull request description for +the full field-by-field derivation of both header shapes and an honest +statement of confidence — briefly: every sub-octet field width below was +counted from the specification's own bit-position diagrams rather than +assumed from generic IEEE 1722 defaults, and cross-checked against a second +in-specification presentation of the same header shapes plus two fully +worked numeric examples this file's own quadlet-counting logic reproduces +exactly. No independent second TC18 implementation was available in this +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 #64) remains a distinct, later milestone rather than being +folded into this one. + +Both `AcfMessageInfo`'s and the NTSCF/TSCF headers' public field names are +kept stable through this fix — only the bit positions/widths the codec +packs them into changed — so no consumer outside `rcp/avtp.hpp`/ +`rcp/acf.hpp`/`rcp/e2e.hpp` needed a source change for its own sake. Two +real dispatch bugs this fix exposed *did* need a one-line mechanical fix +each, since they compared a raw wire byte against `acf::kAcfMsgTypeAbb`/ +`kAcfMsgTypeGbb` directly rather than through the now-necessary +`acf::peek_acf_msg_type()` (acf_msg_type is a 7-bit field sharing byte0 +with `acf_msg_length`'s MSB, not a whole octet): `rcp/udp.hpp`'s +`decode_frame` and `rcp/record.hpp`'s `read_msg`. `avtp::ByteBusId` widens +`uint8_t` → `uint16_t` (byte_bus_id is an 11-bit wire field); this is a +pure widening change with no consumer breakage found across the tree +(verified by full-tree build), except one pre-existing narrowing-warning +call site in `tests/test_udp.cpp` given an explicit cast to stay +warning-clean. Full local build (C++17) and `ctest` (53/53 tests) pass. + +### 64. 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 0ab6014..0b67c30 100644 --- a/include/rcp/acf.hpp +++ b/include/rcp/acf.hpp @@ -7,6 +7,7 @@ // fusa:req REQ-WIRE-012 // fusa:req REQ-WIRE-013 // fusa:req REQ-WIRE-014 +// fusa:req REQ-WIRE-015 // TC18 wire codec, message half — the ACF_ABB / ACF_GBB message types (ACF — // AVTP Control Format) the OPEN Alliance TC18 Remote Control Protocol @@ -30,11 +31,18 @@ // // Field names and behavior below implement TC18's *behavior* as described in // an internal structured extraction of the specification named above; no -// text from that document is reproduced here. The concrete bit-packing -// chosen in this file is this implementation's own encoding of that -// behavior for milestone 44 — full bit-for-bit wire conformance against -// other TC18 implementations is not claimed until v2.6.0 lands, per the -// Phase 13 introduction in ROADMAP.md. +// text from that document is reproduced here. Wire conformance note +// (v2.19.0, issue cpp-RCP-04): the ACF Message Info bit-packing below was +// re-derived field by field from the specification's own header diagram +// (its bit-position figure, not prose) and cross-checked two ways: (1) +// against a second, independent diagram of the same header shape used for a +// different message subtype elsewhere in the specification, and (2) against +// two fully worked numeric examples elsewhere in the specification (one +// ACF_ABB, one ACF_GBB) that state concrete byte counts and an +// acf_msg_length value for a chosen payload/padding — this file's own +// quadlet-counting logic reproduces both stated values exactly. See this +// repository's pull request description for the full derivation and its +// honestly-stated confidence level. #pragma once #include @@ -90,80 +98,145 @@ inline std::error_code make_error_code(AcfErrc e) noexcept { // left packed in an opaque byte, so later milestones (compound requests, // safety variants, …) can extend behavior around a given field without // having to re-derive its bit position from scratch. +// +// Wire layout — 8 bytes total (re-derived from the specification's own +// bit-position diagram for this header; several fields are *not* whole +// octets, which only that diagram shows): +// byte0 bits7:1 acf_msg_type — 7 bits (0x0D ACF_GBB / 0x0E ACF_ABB), +// not a full octet +// byte0 bit0 + +// byte1 acf_msg_length — 9 bits total (byte0's LSB is the MSB +// of the value, byte1 is its low 8 bits); quadlets, +// including this shared header +// byte2 bits7:6 pad — 2 bits, 0–3 trailing pad octets +// byte2 bit5 mtv — message_timestamp valid (ACF_GBB only) +// byte2 bits4:3 rsv — always 0; not surfaced as a field +// byte2 bits2:0 + +// byte3 byte_bus_id — 11 bits total (byte2's low 3 bits are +// its MSBs, byte3 is its low 8 bits) +// byte4 bits7:4 evt — 4 bits; evt_ack is evt's top bit +// (evt[3]), evt_op is its low 3 bits (evt[2:0]) — see +// evt_ack/evt_op field comments below for why this codec +// keeps that split rather than one raw nibble +// byte4 bits3:2 rsv — always 0; not surfaced as a field +// byte4 bit1 hs +// byte4 bit0 cs +// byte5 transaction_num — a full octet +// byte6 bit7 op +// byte6 bit6 rsp +// byte6 bit5 err +// byte6 bit4 ms +// byte6 bits3:0 + +// byte7 read_size_or_segment_num — 12 bits total (byte6's low +// 4 bits are its MSBs, byte7 is its low 8 bits) +// +// This 8-byte total (not the 10 bytes this codec used before this pass) is +// independently confirmed by two fully worked numeric examples elsewhere in +// the specification that label this exact header "8 byte Message Info" and +// state concrete acf_msg_length values this file's own quadlet-counting +// reproduces exactly — see the header comment above and this repository's +// pull request description. struct AcfMessageInfo { - uint8_t acf_msg_type = kAcfMsgTypeAbb; // ACF_ABB or ACF_GBB - uint16_t acf_msg_length = 0; // quadlets, including this shared header + uint8_t acf_msg_type = kAcfMsgTypeAbb; // ACF_ABB or ACF_GBB — 7-bit wire field, values 0x0D/0x0E fit either way + uint16_t acf_msg_length = 0; // quadlets, including this shared header — 9-bit wire field (0–511), encoder masks uint8_t pad = 0; // 0–3 trailing pad bytes added for quadlet alignment bool mtv = false; // message_timestamp valid (ACF_GBB only) - avtp::ByteBusId byte_bus_id = 0; // target endpoint, unique within stream_id only - bool evt_ack = false; // evt[3] — acknowledge flag - uint8_t evt_op = 0; // evt[2:0] — endpoint-defined sub-opcode + avtp::ByteBusId byte_bus_id = 0; // target endpoint, unique within stream_id only — 11-bit wire field (0–2047), encoder masks + // evt is a 4-bit wire field (evt[3:0]) whose interpretation is message- + // kind-dependent per the specification's own response/request tables: + // for a "clear specific request" and for every response this codec + // builds via make_response() below, evt[3] is cleanly an + // acknowledge-request/acknowledge-response flag and evt[2:0] is either + // reserved or a small counter — which is exactly the evt_ack (bit3) + + // evt_op (bits2:0) split kept here. For a standard request, evt[3:0] is + // instead fully endpoint-defined; callers in that role are still free to + // use evt_op alone (masked to 3 bits) or set evt_ack for whatever their + // endpoint defines bit3 to mean. Either way the two fields together + // always reproduce the exact 4-bit wire value: evt = (evt_ack<<3)|evt_op. + bool evt_ack = false; // evt[3] + uint8_t evt_op = 0; // evt[2:0], masked to 3 bits on encode bool hs = false; // reserved for endpoint-specific use (e.g. I2C high-speed) bool cs = false; // conditional-start; meaning depends on request kind (v2.5.0) uint8_t transaction_num = 0; // correlates a request with its response/ack bool op = false; // false = read, true = write (this codec's convention) - bool rsp = false; // set on every response/ack, clear on requests + bool rsp = false; // set on every response (including Acknowledge — extraction Table 15), clear on requests bool err = false; // set alongside rsp for an error response bool ms = false; // "more segments" — fragmentation, deferred to v2.8.0 - uint16_t read_size_or_segment_num = 0; // read_size when !ms, segment_num when ms + uint16_t read_size_or_segment_num = 0; // read_size when !ms, segment_num when ms — 12-bit wire field (0–4095), encoder masks }; -constexpr size_t kAcfCommonHeaderLen = 10; +constexpr size_t kAcfCommonHeaderLen = 8; constexpr size_t kAcfGbbTimestampLen = 8; +// evt[3:0] == 0xF identifies a response as an Acknowledge (extraction +// Table 15) — the wire value this codec's own make_response()/ +// response_kind_of() below use for that purpose. Not meaningful for +// standard requests, whose evt is fully endpoint-defined. +constexpr uint8_t kEvtAcknowledge = 0x0F; + namespace detail { -constexpr uint8_t kByte3MtvBit = 0x80; -constexpr uint8_t kByte3PadShift = 5; -constexpr uint8_t kByte3PadMask = 0x03; -constexpr uint8_t kByte5AckBit = 0x80; -constexpr uint8_t kByte5OpShift = 4; -constexpr uint8_t kByte5OpMask = 0x07; -constexpr uint8_t kByte5HsBit = 0x08; -constexpr uint8_t kByte5CsBit = 0x04; -constexpr uint8_t kByte7OpBit = 0x80; -constexpr uint8_t kByte7RspBit = 0x40; -constexpr uint8_t kByte7ErrBit = 0x20; -constexpr uint8_t kByte7MsBit = 0x10; +constexpr uint8_t kMsgTypeMask = 0x7F; // acf_msg_type: 7 bits +constexpr uint16_t kMsgLengthMask = 0x01FF; // acf_msg_length: 9 bits +constexpr uint8_t kPadMask = 0x03; // pad: 2 bits +constexpr uint8_t kByte2MtvBit = 0x20; // mtv: byte2 bit5 +constexpr uint16_t kByteBusIdMask = 0x07FF; // byte_bus_id: 11 bits +constexpr uint8_t kEvtMask = 0x0F; // evt: 4 bits +constexpr uint8_t kEvtOpMask = 0x07; // evt_op: low 3 bits of evt +constexpr uint8_t kEvtAckBit = 0x08; // evt_ack: top bit of evt +constexpr uint8_t kByte4HsBit = 0x02; +constexpr uint8_t kByte4CsBit = 0x01; +constexpr uint8_t kByte6OpBit = 0x80; +constexpr uint8_t kByte6RspBit = 0x40; +constexpr uint8_t kByte6ErrBit = 0x20; +constexpr uint8_t kByte6MsBit = 0x10; +constexpr uint16_t kReadSizeMask = 0x0FFF; // read_size_or_segment_num: 12 bits } // namespace detail -inline void encode_acf_message_info(const AcfMessageInfo& info, uint8_t* out10) noexcept { - out10[0] = info.acf_msg_type; - avtp::detail::put_u16(&out10[1], info.acf_msg_length); - out10[3] = static_cast( - (info.mtv ? detail::kByte3MtvBit : 0) | - (static_cast(info.pad & detail::kByte3PadMask) << detail::kByte3PadShift)); - out10[4] = info.byte_bus_id; - out10[5] = static_cast( - (info.evt_ack ? detail::kByte5AckBit : 0) | - (static_cast(info.evt_op & detail::kByte5OpMask) << detail::kByte5OpShift) | - (info.hs ? detail::kByte5HsBit : 0) | - (info.cs ? detail::kByte5CsBit : 0)); - out10[6] = info.transaction_num; - out10[7] = static_cast( - (info.op ? detail::kByte7OpBit : 0) | - (info.rsp ? detail::kByte7RspBit : 0) | - (info.err ? detail::kByte7ErrBit : 0) | - (info.ms ? detail::kByte7MsBit : 0)); - avtp::detail::put_u16(&out10[8], info.read_size_or_segment_num); +inline void encode_acf_message_info(const AcfMessageInfo& info, uint8_t* out8) noexcept { + const uint16_t msg_length = static_cast(info.acf_msg_length & detail::kMsgLengthMask); + const uint16_t bus_id = static_cast(info.byte_bus_id & detail::kByteBusIdMask); + const uint8_t evt = static_cast( + (info.evt_ack ? detail::kEvtAckBit : 0) | (info.evt_op & detail::kEvtOpMask)); + const uint16_t read_size = static_cast(info.read_size_or_segment_num & detail::kReadSizeMask); + + out8[0] = static_cast(((info.acf_msg_type & detail::kMsgTypeMask) << 1) | + ((msg_length >> 8) & 0x01)); + out8[1] = static_cast(msg_length & 0xFF); + out8[2] = static_cast((static_cast(info.pad & detail::kPadMask) << 6) | + (info.mtv ? detail::kByte2MtvBit : 0) | + ((bus_id >> 8) & 0x07)); + out8[3] = static_cast(bus_id & 0xFF); + out8[4] = static_cast((evt << 4) | + (info.hs ? detail::kByte4HsBit : 0) | + (info.cs ? detail::kByte4CsBit : 0)); + out8[5] = info.transaction_num; + out8[6] = static_cast( + (info.op ? detail::kByte6OpBit : 0) | + (info.rsp ? detail::kByte6RspBit : 0) | + (info.err ? detail::kByte6ErrBit : 0) | + (info.ms ? detail::kByte6MsBit : 0) | + ((read_size >> 8) & 0x0F)); + out8[7] = static_cast(read_size & 0xFF); } -inline void decode_acf_message_info(const uint8_t* in10, AcfMessageInfo& out) noexcept { - out.acf_msg_type = in10[0]; - out.acf_msg_length = avtp::detail::get_u16(&in10[1]); - out.mtv = (in10[3] & detail::kByte3MtvBit) != 0; - out.pad = static_cast((in10[3] >> detail::kByte3PadShift) & detail::kByte3PadMask); - out.byte_bus_id = in10[4]; - out.evt_ack = (in10[5] & detail::kByte5AckBit) != 0; - out.evt_op = static_cast((in10[5] >> detail::kByte5OpShift) & detail::kByte5OpMask); - out.hs = (in10[5] & detail::kByte5HsBit) != 0; - out.cs = (in10[5] & detail::kByte5CsBit) != 0; - out.transaction_num = in10[6]; - out.op = (in10[7] & detail::kByte7OpBit) != 0; - out.rsp = (in10[7] & detail::kByte7RspBit) != 0; - out.err = (in10[7] & detail::kByte7ErrBit) != 0; - out.ms = (in10[7] & detail::kByte7MsBit) != 0; - out.read_size_or_segment_num = avtp::detail::get_u16(&in10[8]); +inline void decode_acf_message_info(const uint8_t* in8, AcfMessageInfo& out) noexcept { + out.acf_msg_type = static_cast(in8[0] >> 1); + out.acf_msg_length = static_cast(((in8[0] & 0x01) << 8) | in8[1]); + out.pad = static_cast((in8[2] >> 6) & detail::kPadMask); + out.mtv = (in8[2] & detail::kByte2MtvBit) != 0; + out.byte_bus_id = static_cast(((in8[2] & 0x07) << 8) | in8[3]); + const uint8_t evt = static_cast((in8[4] >> 4) & detail::kEvtMask); + out.evt_ack = (evt & detail::kEvtAckBit) != 0; + out.evt_op = static_cast(evt & detail::kEvtOpMask); + out.hs = (in8[4] & detail::kByte4HsBit) != 0; + out.cs = (in8[4] & detail::kByte4CsBit) != 0; + out.transaction_num = in8[5]; + out.op = (in8[6] & detail::kByte6OpBit) != 0; + out.rsp = (in8[6] & detail::kByte6RspBit) != 0; + out.err = (in8[6] & detail::kByte6ErrBit) != 0; + out.ms = (in8[6] & detail::kByte6MsBit) != 0; + out.read_size_or_segment_num = static_cast(((in8[6] & 0x0F) << 8) | in8[7]); } // ── ACF_ABB / ACF_GBB message encode / decode ───────────────────────────────── @@ -171,6 +244,18 @@ inline void decode_acf_message_info(const uint8_t* in10, AcfMessageInfo& out) no // 64-bit message_timestamp slot immediately after the shared header, // 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 +// acf_msg_type is *not* a whole octet on the wire (byte0 bit0 is +// acf_msg_length's MSB, not part of the type) — callers that need to decide +// which of decode_acf_abb/decode_acf_gbb to call before fully decoding +// (e.g. rcp/udp.hpp's Frame decoder) must go through this rather than +// comparing a raw buffer byte against kAcfMsgTypeAbb/kAcfMsgTypeGbb +// directly, which would almost never match. `len` must be >= 1. +inline uint8_t peek_acf_msg_type(const uint8_t* b) noexcept { + return static_cast(b[0] >> 1); +} + inline std::vector encode_acf_abb(AcfMessageInfo info, const std::vector& payload) { info.acf_msg_type = kAcfMsgTypeAbb; @@ -185,7 +270,7 @@ inline std::error_code decode_acf_abb(const uint8_t* b, size_t len, AcfMessageInfo& out_info, std::vector& out_payload) { if (len < 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); - if (b[0] != kAcfMsgTypeAbb) return make_error_code(AcfErrc::bad_acf_msg_type); + if (peek_acf_msg_type(b) != kAcfMsgTypeAbb) return make_error_code(AcfErrc::bad_acf_msg_type); if (len < kAcfCommonHeaderLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); decode_acf_message_info(b, out_info); out_payload.assign(b + kAcfCommonHeaderLen, b + len); @@ -211,7 +296,7 @@ inline std::error_code decode_acf_gbb(const uint8_t* b, size_t len, uint64_t& out_message_timestamp, std::vector& out_payload) { if (len < 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); - if (b[0] != kAcfMsgTypeGbb) return make_error_code(AcfErrc::bad_acf_msg_type); + if (peek_acf_msg_type(b) != kAcfMsgTypeGbb) return make_error_code(AcfErrc::bad_acf_msg_type); if (len < kAcfCommonHeaderLen + kAcfGbbTimestampLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); decode_acf_message_info(b, out_info); @@ -244,18 +329,29 @@ inline AcfMessageInfo make_standard_request(avtp::ByteBusId bus_id, uint8_t tran return info; } -// The four response semantic types a request can produce, mapped onto the -// shared header's evt/op/rsp/err fields (extraction §2.7, §2.8). +// The four response semantic types a request can produce (extraction +// Table 14/Table 15). Per Table 15, IEEE1722 formally only knows +// "responses" — RCP's Acknowledge is one of the four response variants, not +// a separate rsp=0 message: every response, Acknowledge included, carries +// rsp=1, and Acknowledge is distinguished purely by evt[3:0] == 0xF +// (kEvtAcknowledge). This is a correction from this codec's pre-v2.19.0 +// behavior, which modeled Acknowledge as rsp=0 — that matched neither this +// header's own request-vs-response `rsp` field meaning (0 = request, +// 1 = response — Acknowledge is unambiguously a response) nor evt[3:0]'s +// real role as the field that actually distinguishes a plain/counted +// response from an acknowledge. enum class ResponseKind : uint8_t { - Acknowledge = 0, // received, no data — evt_ack set, rsp clear - WriteResponse = 1, // completed write — rsp set, op set, err clear - ReadResponse = 2, // completed read, payload carries the data — rsp set, op clear, err clear - ErrorResponse = 3, // failed — rsp set, err set + Acknowledge = 0, // rsp set, evt[3:0] == 0xF (kEvtAcknowledge) + WriteResponse = 1, // completed write — rsp set, op set, err clear, evt[3:0] != 0xF + ReadResponse = 2, // completed read, payload carries the data — rsp set, op clear, err clear, evt[3:0] != 0xF + ErrorResponse = 3, // failed — rsp set, err set, evt[3:0] != 0xF }; inline ResponseKind response_kind_of(const AcfMessageInfo& info) noexcept { - if (!info.rsp) return ResponseKind::Acknowledge; - if (info.err) return ResponseKind::ErrorResponse; + const uint8_t evt = static_cast((info.evt_ack ? detail::kEvtAckBit : 0) | + (info.evt_op & detail::kEvtOpMask)); + if (evt == kEvtAcknowledge) return ResponseKind::Acknowledge; + if (info.err) return ResponseKind::ErrorResponse; return info.op ? ResponseKind::WriteResponse : ResponseKind::ReadResponse; } @@ -269,10 +365,13 @@ inline AcfMessageInfo make_response(const AcfMessageInfo& request, ResponseKind resp.acf_msg_type = request.acf_msg_type; resp.byte_bus_id = request.byte_bus_id; resp.transaction_num = request.transaction_num; - resp.evt_ack = (kind == ResponseKind::Acknowledge); - resp.rsp = (kind != ResponseKind::Acknowledge); - resp.err = (kind == ResponseKind::ErrorResponse); - resp.op = (kind == ResponseKind::WriteResponse); + resp.rsp = true; // every response, including Acknowledge, carries rsp=1 + if (kind == ResponseKind::Acknowledge) { + resp.evt_ack = true; + resp.evt_op = detail::kEvtOpMask; // together, evt[3:0] == 0xF == kEvtAcknowledge + } + resp.err = (kind == ResponseKind::ErrorResponse); + resp.op = (kind == ResponseKind::WriteResponse); return resp; } @@ -296,5 +395,114 @@ inline std::optional effective_timestamp(const avtp::TscfHeader* tscf, return std::nullopt; } +// ── RELAY spec §15.5 canonical types (additive; issue cpp-RCP-FS-06) ───────── +// RELAY spec §15.5 ("RCP — StreamID, ByteBusID, TransactionNum, ControlFlags, +// Message") defines a canonical, protocol-agnostic shape for a decoded +// ACF_ABB/ACF_GBB request/response/acknowledge, matching go-RCP's own types +// field-for-field for zero-copy casting across the two implementations. +// ControlFlags and Message below are new, additive types implementing that +// canonical shape as a thin, documented view over this file's own +// AcfMessageInfo (which remains the codec's own working representation — +// nothing above this section changes to accommodate the mapping below). +// +// Two field widths intentionally differ from AcfMessageInfo/StreamId's own, +// because RELAY spec §15.5 fixes them independently of TC18's real wire +// widths (re-derived elsewhere in this file, see the "ACF shared header" +// section above): +// - ControlFlags::ByteBusID's underlying type in the spec is uint8_t, +// narrower than avtp::ByteBusId's 11-bit range; to_message() truncates, +// from_message() zero-extends. Round-trips losslessly only for +// byte_bus_id values 0–255. +// - Message::TransactionNum is uint16_t, wider than AcfMessageInfo's +// 8-bit transaction_num; the conversion is always lossless in that +// direction (a widening store), never the reverse. + +enum class ControlFlags : uint8_t { + None = 0, + FlagAck = 1u << 7, + FlagRead = 1u << 6, + FlagWrite = 1u << 5, + FlagResponse = 1u << 4, + FlagError = 1u << 3, + FlagMoreSegments = 1u << 2, +}; + +// ControlFlags is a bitmask enum by design (RELAY spec §15.5's own Go +// definition is likewise a set of independent bit constants, not an +// exhaustive enumeration); operator|/operator& legitimately produce/consume +// combined values with no enumerator of their own, which is exactly what +// clang-analyzer-optin.core.EnumCastOutOfRange (a strict-by-design check +// aimed at accidental enum casts, not intentional bitmask ones) flags below +// — NOLINT'd on both cast lines as a deliberate, reviewed exception. +constexpr ControlFlags operator|(ControlFlags a, ControlFlags b) noexcept { + return static_cast(static_cast(a) | static_cast(b)); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) +} +constexpr ControlFlags operator&(ControlFlags a, ControlFlags b) noexcept { + return static_cast(static_cast(a) & static_cast(b)); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) +} +constexpr bool has_flag(ControlFlags value, ControlFlags flag) noexcept { + return (value & flag) == flag; +} + +// Message is the RELAY spec §15.5 canonical decoded-frame shape: a +// protocol/direction-agnostic view of one ACF_ABB/ACF_GBB request, response, +// or acknowledge, independent of the AVTPDU (NTSCF/TSCF, rcp/avtp.hpp) that +// carried it — stream_id/avtp_timestamp are deliberately not part of this +// type, same as the Go definition it mirrors. +struct Message { + uint8_t byte_bus_id = 0; // RELAY spec ByteBusID — see width note above + uint16_t transaction_num = 0; // RELAY spec TransactionNum — see width note above + ControlFlags control = ControlFlags::None; + uint16_t read_size_or_segment = 0; // read_size (FlagMoreSegments clear) or segment_num (set) + uint64_t timestamp = 0; // meaningful only for the ACF_GBB (mtv=1) encoding + std::vector body; +}; + +// to_message converts a decoded AcfMessageInfo + its ACF_GBB message_timestamp +// (pass 0 for ACF_ABB, which has none) + payload into the RELAY spec §15.5 +// canonical shape. +inline Message to_message(const AcfMessageInfo& info, uint64_t message_timestamp, + std::vector body) { + Message msg; + msg.byte_bus_id = static_cast(info.byte_bus_id & 0xFF); + msg.transaction_num = info.transaction_num; + ControlFlags flags = ControlFlags::None; + if (info.evt_ack) flags = flags | ControlFlags::FlagAck; + flags = flags | (info.op ? ControlFlags::FlagWrite : ControlFlags::FlagRead); + if (info.rsp) flags = flags | ControlFlags::FlagResponse; + if (info.err) flags = flags | ControlFlags::FlagError; + if (info.ms) flags = flags | ControlFlags::FlagMoreSegments; + msg.control = flags; + msg.read_size_or_segment = info.read_size_or_segment_num; + msg.timestamp = (info.acf_msg_type == kAcfMsgTypeGbb) ? message_timestamp : 0; + msg.body = std::move(body); + return msg; +} + +// from_message is to_message's inverse: it rebuilds an AcfMessageInfo (plus +// the separate message_timestamp/body outputs encode_acf_abb/encode_acf_gbb +// take) from a canonical Message. Since Message carries no acf_msg_type of +// its own, the caller states whether to target ACF_ABB or ACF_GBB; mtv is +// set exactly when the caller asks for ACF_GBB with a non-zero timestamp +// (an all-zero valid timestamp cannot be distinguished from "no timestamp" +// through this narrower canonical type alone — pass `mtv` explicitly when +// that distinction matters, same as AcfMessageInfo::mtv always requires). +inline AcfMessageInfo from_message(const Message& msg, bool as_gbb, bool mtv, + uint64_t& out_message_timestamp) noexcept { + AcfMessageInfo info; + info.acf_msg_type = as_gbb ? kAcfMsgTypeGbb : kAcfMsgTypeAbb; + info.mtv = as_gbb && mtv; + info.byte_bus_id = msg.byte_bus_id; + info.evt_ack = has_flag(msg.control, ControlFlags::FlagAck); + info.op = has_flag(msg.control, ControlFlags::FlagWrite); + info.rsp = has_flag(msg.control, ControlFlags::FlagResponse); + info.err = has_flag(msg.control, ControlFlags::FlagError); + info.ms = has_flag(msg.control, ControlFlags::FlagMoreSegments); + info.transaction_num = static_cast(msg.transaction_num & 0xFF); + info.read_size_or_segment_num = msg.read_size_or_segment; + out_message_timestamp = msg.timestamp; + return info; +} + } // namespace acf } // namespace rcp diff --git a/include/rcp/avtp.hpp b/include/rcp/avtp.hpp index ba3d2cf..96da7c2 100644 --- a/include/rcp/avtp.hpp +++ b/include/rcp/avtp.hpp @@ -38,11 +38,19 @@ // // Field names and behavior below implement TC18's *behavior* as described in // an internal structured extraction of the specification named above; no -// text from that document is reproduced here. The concrete bit-packing -// chosen in this file is this implementation's own encoding of that -// behavior for milestone 44; full bit-for-bit wire conformance against -// other TC18 implementations is not claimed until v2.6.0 lands, per the -// Phase 13 introduction in ROADMAP.md. +// text from that document is reproduced here. Wire conformance note (v2.19.0, +// issue cpp-RCP-04): the NTSCF/TSCF bit-packing below was re-derived field by +// field from the specification's own header diagrams (bit-position figures, +// not prose) and cross-checked against a second, independent worked example +// elsewhere in the specification that plugs concrete field values into the +// same header shape. Both derivations agree, which is the strongest +// consistency check available without a second, independent TC18 +// implementation to interoperate against directly — see this repository's +// pull request description for the full derivation and its honestly-stated +// confidence level. The one part of this file *not* re-derived from a figure +// is byte order/endianness of multi-byte integer fields (big-endian +// throughout), which was already big-endian before this pass and is kept +// unchanged. #pragma once #include @@ -173,8 +181,14 @@ inline bool operator!=(const StreamId& a, const StreamId& b) noexcept { return ! // enforces the echo half of that rule structurally: see its make_response(), // which always copies byte_bus_id (and transaction_num) from the request it // is built from rather than accepting them as free parameters. +// +// Width: on the wire (rcp/acf.hpp's ACF Message Info header) byte_bus_id is +// an 11-bit field (0–2047), not a full octet — a detail only visible in the +// specification's bit-position diagram for that header, not in prose. This +// alias is uint16_t, wide enough for that full range; rcp/acf.hpp's encoder +// masks to 11 bits, matching every other sub-octet field in that header. -using ByteBusId = uint8_t; +using ByteBusId = uint16_t; // ── NTSCF / TSCF AVTPDU headers ─────────────────────────────────────────────── // @@ -187,35 +201,78 @@ using ByteBusId = uint8_t; // exposes the two header shapes distinctly enough that misusing one for the // other's direction is a visible, deliberate choice rather than an accident. +// Bit layouts below (extraction §2.2's figures — the specification's actual +// header diagrams, each with a per-bit ruler, not the prose) diverge from +// generic IEEE 1722 subtype-header assumptions in field widths, so every +// width here was counted from those diagrams rather than assumed: +// +// NTSCF, 12 bytes total: +// byte0 subtype (0x82) +// byte1 bit7 sv (stream_valid — always written 1 here) +// byte1 bits6:4 version (always 0) +// byte1 bit3 r (reserved, always 0; not surfaced as a field) +// byte1 bits2:0 + +// byte2 ntscf_data_length — 11 bits total (byte1's low 3 bits +// are its MSBs, byte2 is its low 8 bits): byte length of +// the ACF message that follows, same quantity +// NtscfHeader::control_data_length already held, just +// narrower than a full octet pair on the wire +// byte3 sequence_num — 8 bits (not 16 — a per-stream_id +// rolling counter that wraps at 256) +// bytes4-11 stream_id, 64 bits +// +// TSCF, 24 bytes total — noticeably longer than the 18 bytes this codec +// used before this pass, because the real header carries a 32-bit reserved +// word (bytes16-19) and a 16-bit reserved half (bytes22-23 — the other half +// of the 32-bit word carrying stream_data_length) that this codec +// previously omitted entirely: +// byte0 subtype (0x05) +// byte1 bit7 sv (always 1) +// byte1 bits6:4 version (always 0) +// byte1 bit3 mr (reserved here, always 0; not surfaced) +// byte1 bits2:1 rsv (always 0) +// byte1 bit0 tv (timestamp_valid) +// byte2 sequence_num — 8 bits, same rolling-counter field as +// NTSCF's +// byte3 reserved (7 bits) + tu (1 bit) — always 0; not +// surfaced (no RCP-specific behavior found for either) +// bytes4-11 stream_id, 64 bits +// bytes12-15 avtp_timestamp, 32 bits +// bytes16-19 reserved, always 0 +// bytes20-21 stream_data_length(octets) — 16 bits; this is +// TscfHeader::control_data_length's wire position +// bytes22-23 reserved, always 0 struct NtscfHeader { StreamId stream_id{}; - uint16_t sequence_num = 0; // per-stream_id AVTPDU counter - uint16_t control_data_length = 0; // byte length of the ACF message that follows + uint8_t sequence_num = 0; // per-stream_id AVTPDU counter, 8-bit rolling + uint16_t control_data_length = 0; // byte length of the ACF message that follows; wire field is 11 bits (0–2047) }; struct TscfHeader { StreamId stream_id{}; - uint16_t sequence_num = 0; - uint16_t control_data_length = 0; + uint8_t sequence_num = 0; // per-stream_id AVTPDU counter, 8-bit rolling + uint16_t control_data_length = 0; // byte length of the ACF message that follows; wire field is a full 16 bits bool timestamp_valid = false; // "tv" — whether avtp_timestamp below is meaningful uint32_t avtp_timestamp = 0; // 32-bit; TSCF-only (extraction §2.6) }; -constexpr size_t kNtscfHeaderLen = 14; // 1 subtype + 1 flags + 2 seq + 8 stream_id + 2 cdl -constexpr size_t kTscfHeaderLen = 18; // kNtscfHeaderLen + 4 avtp_timestamp +constexpr size_t kNtscfHeaderLen = 12; // 1 subtype + 1 flags/length-hi + 1 length-lo + 1 seq + 8 stream_id +constexpr size_t kTscfHeaderLen = 24; // kNtscfHeaderLen + 4 avtp_timestamp + 4 reserved + 2 stream_data_length + 2 reserved namespace detail { -constexpr uint8_t kFlagStreamValid = 0x80; // "sv" — always set for these control formats -constexpr uint8_t kFlagTimestampValid = 0x40; // "tv" — TSCF only +constexpr uint8_t kFlagStreamValid = 0x80; // "sv" — always set for these control formats, byte1 bit7 in both headers +constexpr uint8_t kFlagTimestampValid = 0x01; // "tv" — TSCF only, byte1 bit0 (NOT bit6 — see header comment above) +constexpr uint16_t kNtscfDataLengthMask = 0x07FF; // ntscf_data_length is 11 bits wide } // namespace detail inline std::vector encode_ntscf_header(const NtscfHeader& h) { std::vector buf(kNtscfHeaderLen, 0); + const uint16_t data_len = static_cast(h.control_data_length & detail::kNtscfDataLengthMask); buf[0] = kSubtypeNtscf; - buf[1] = detail::kFlagStreamValid; - detail::put_u16(&buf[2], h.sequence_num); + buf[1] = static_cast(detail::kFlagStreamValid | ((data_len >> 8) & 0x07)); + buf[2] = static_cast(data_len & 0xFF); + buf[3] = h.sequence_num; detail::put_u64(&buf[4], h.stream_id.to_u64()); - detail::put_u16(&buf[12], h.control_data_length); return buf; } @@ -223,9 +280,9 @@ inline std::error_code decode_ntscf_header(const uint8_t* b, size_t len, NtscfHe if (len < 1) return make_error_code(AvtpErrc::short_buffer); if (b[0] != kSubtypeNtscf) return make_error_code(AvtpErrc::bad_subtype); if (len < kNtscfHeaderLen) return make_error_code(AvtpErrc::short_buffer); - out.sequence_num = detail::get_u16(&b[2]); + out.control_data_length = static_cast(((b[1] & 0x07) << 8) | b[2]); + out.sequence_num = b[3]; out.stream_id = StreamId::from_u64(detail::get_u64(&b[4])); - out.control_data_length = detail::get_u16(&b[12]); return {}; } @@ -234,10 +291,13 @@ inline std::vector encode_tscf_header(const TscfHeader& h) { buf[0] = kSubtypeTscf; buf[1] = static_cast(detail::kFlagStreamValid | (h.timestamp_valid ? detail::kFlagTimestampValid : 0)); - detail::put_u16(&buf[2], h.sequence_num); + buf[2] = h.sequence_num; + // buf[3] (reserved + tu) stays 0. detail::put_u64(&buf[4], h.stream_id.to_u64()); - detail::put_u16(&buf[12], h.control_data_length); - detail::put_u32(&buf[14], h.avtp_timestamp); + detail::put_u32(&buf[12], h.avtp_timestamp); + // buf[16..19] (reserved) stays 0. + detail::put_u16(&buf[20], h.control_data_length); + // buf[22..23] (reserved) stays 0. return buf; } @@ -246,10 +306,10 @@ inline std::error_code decode_tscf_header(const uint8_t* b, size_t len, TscfHead if (b[0] != kSubtypeTscf) return make_error_code(AvtpErrc::bad_subtype); if (len < kTscfHeaderLen) return make_error_code(AvtpErrc::short_buffer); out.timestamp_valid = (b[1] & detail::kFlagTimestampValid) != 0; - out.sequence_num = detail::get_u16(&b[2]); + out.sequence_num = b[2]; out.stream_id = StreamId::from_u64(detail::get_u64(&b[4])); - out.control_data_length = detail::get_u16(&b[12]); - out.avtp_timestamp = detail::get_u32(&b[14]); + out.avtp_timestamp = detail::get_u32(&b[12]); + out.control_data_length = detail::get_u16(&b[20]); return {}; } diff --git a/include/rcp/e2e.hpp b/include/rcp/e2e.hpp index aaeba55..0310698 100644 --- a/include/rcp/e2e.hpp +++ b/include/rcp/e2e.hpp @@ -42,16 +42,22 @@ // // Field names and behavior below implement TC18's *behavior* as described // in an internal structured extraction of the specification named above; -// no text from that document is reproduced here. The concrete CRC -// bit-packing, byte layout of the coverage buffer, and watchdog/safe-state -// class shapes chosen in this file are this implementation's own encoding -// of that behavior — full bit-for-bit conformance against other TC18 -// implementations is not claimed, same as the equivalent disclaimers in -// rcp/avtp.hpp, rcp/acf.hpp, rcp/regmap.hpp, and rcp/request.hpp. This -// header provides primitives, not a running scheduler or timer thread — -// deciding *when* to call overflowed()/kick()/should_emit_info_notification() -// is left to the embedding application, same as every other header in this -// codebase. +// no text from that document is reproduced here. The CRC32 algorithm itself +// (polynomial, init/refin/refout/xorout) was already independently +// verifiable and unchanged by this note. The coverage buffer's *byte +// layout* was corrected alongside rcp/acf.hpp's/rcp/avtp.hpp's own wire +// conformance pass (v2.19.0, issue cpp-RCP-N2-03): it now inserts an +// ACF_GBB message's 8-byte message_timestamp between the Message Info +// header and the payload — matching what rcp/acf.hpp's encode_acf_gbb +// actually transmits — where it previously omitted that field from +// coverage entirely. See rcp/acf.hpp's and rcp/avtp.hpp's own header +// comments, and this repository's pull request description, for the +// specification derivation and its honestly-stated confidence level; this +// file's coverage_buffer is a direct, mechanical consequence of that +// derivation rather than a second independent one. This header provides +// primitives, not a running scheduler or timer thread — deciding *when* to +// call overflowed()/kick()/should_emit_info_notification() is left to the +// embedding application, same as every other header in this codebase. #pragma once #include @@ -95,6 +101,29 @@ inline std::error_code make_error_code(E2eErrc e) noexcept { return {static_cast(e), e2e_category()}; } +// ── Numeric TC18 wire error code (extraction Table 27; issue cpp-RCP-08) ───── +// The E2eErrc enum's own ordinal values (1, 2, ...) are this codec's +// internal std::error_code values — unrelated to, and not meant to collide +// with, the numeric error codes TC18 actually puts in an error response's +// byte_msg_payload (extraction Table 27, "Error codes in responses"; no +// other error enum in this codebase currently exposes that numeric-code +// mapping either, so there is no existing convention here to match beyond +// keeping the C++ enum's own ordinal untouched). Of E2eErrc's two values, +// only crc_error has a direct TC18 wire error code: POCI_FAILURE (12), +// "CRC of request does not match". sequence_violation has no dedicated TC18 +// error code of its own in that table, so wire_error_code() reports +// std::nullopt for it rather than guessing a mapping the specification +// does not state. + +constexpr int kPociFailureErrorCode = 12; // TC18 POCI_FAILURE — CRC of request does not match + +inline std::optional wire_error_code(E2eErrc e) noexcept { + switch (e) { + case E2eErrc::crc_error: return kPociFailureErrorCode; + default: return std::nullopt; + } +} + // ── CRC32 primitive (extraction §4.7) ──────────────────────────────────────── // 32-bit width, polynomial 0xF4ACFB13, initial value 0xFFFFFFFF, both input // and output reflected, final XOR 0xFFFFFFFF. This replaces the ad-hoc @@ -153,11 +182,20 @@ inline uint32_t crc32(const std::vector& data) noexcept { // ── CRC coverage & the trailing-CRC length pre-adjustment (extraction §4.7) ── // Coverage is exactly: stream_id + avtp_timestamp (zero-filled when the // frame rides under an NTSCF header, since NTSCF carries no avtp_timestamp -// field to begin with) + the full ACF shared header + payload. Because the -// CRC is computed *over* the ACF header, and the header's own -// acf_msg_length field must already reflect the trailer's length before -// that header is serialized, the length adjustment has to be applied -// first, not patched in after the fact. +// field to begin with) + the complete ACF header + the complete payload. +// "Complete ACF header" (issue cpp-RCP-N2-03, fixed alongside cpp-RCP-04): +// for ACF_GBB this means the 8-byte Message Info *and* the 8-byte +// message_timestamp field that immediately follows it on the wire — the +// same message_timestamp encode_acf_gbb (rcp/acf.hpp) places right after +// the Message Info — not the Message Info alone. ACF_ABB has no +// message_timestamp field at all, so its coverage is the Message Info only, +// same as before. Before this fix, coverage_buffer had no way to include +// message_timestamp at all, so a GBB message's CRC silently omitted 8 real +// wire bytes and folded the payload in 8 bytes early relative to what +// encode_acf_gbb actually transmits. Because the CRC is computed *over* the +// ACF header, and the header's own acf_msg_length field must already +// reflect the trailer's length before that header is serialized, the +// length adjustment has to be applied first, not patched in after the fact. constexpr uint16_t kCrcLengthAdjustQuadlets = 1; // +1 quadlet on AcfMessageInfo::acf_msg_length constexpr uint16_t kCrcLengthAdjustOctets = 4; // +4 octets on an outer AVTPDU frame-length field @@ -198,16 +236,26 @@ inline void put_u64_be(std::vector& buf, uint64_t v) { // rides under an NTSCF header — coverage_buffer supplies the documented // zero-filled stand-in in that case rather than requiring the caller to // pass an explicit zero (making the NTSCF case a visible, named choice at -// every call site instead of an easy-to-miss "just pass 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 it is given. +// every call site instead of an easy-to-miss "just pass 0"). `message_timestamp` +// is std::nullopt for ACF_ABB (which has no such field) and the actual +// 64-bit value passed to encode_acf_gbb for ACF_GBB — coverage_buffer only +// 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. +// `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) +// it is given. inline std::vector coverage_buffer(const avtp::StreamId& stream_id, std::optional avtp_timestamp, const acf::AcfMessageInfo& info, + std::optional message_timestamp, const std::vector& payload) { + const bool is_gbb = (info.acf_msg_type == acf::kAcfMsgTypeGbb); + std::vector buf; - buf.reserve(8 + 4 + acf::kAcfCommonHeaderLen + payload.size()); + buf.reserve(8 + 4 + acf::kAcfCommonHeaderLen + + (is_gbb ? acf::kAcfGbbTimestampLen : 0) + payload.size()); 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 @@ -216,6 +264,10 @@ inline std::vector coverage_buffer(const avtp::StreamId& stream_id, acf::encode_acf_message_info(info, hdr); buf.insert(buf.end(), hdr, hdr + acf::kAcfCommonHeaderLen); + if (is_gbb) { + detail::put_u64_be(buf, message_timestamp.value_or(0)); + } + buf.insert(buf.end(), payload.begin(), payload.end()); return buf; } @@ -223,8 +275,9 @@ inline std::vector coverage_buffer(const avtp::StreamId& stream_id, // compute_crc is coverage_buffer() + crc32() in one call — the usual way a // caller actually wants this used. inline uint32_t compute_crc(const avtp::StreamId& stream_id, std::optional avtp_timestamp, - const acf::AcfMessageInfo& info, const std::vector& payload) { - return crc32(coverage_buffer(stream_id, avtp_timestamp, info, payload)); + const acf::AcfMessageInfo& info, std::optional message_timestamp, + const std::vector& payload) { + return crc32(coverage_buffer(stream_id, avtp_timestamp, info, message_timestamp, payload)); } // append_crc appends the 4-octet big-endian CRC trailer to `frame` — the @@ -237,14 +290,16 @@ inline void append_crc(std::vector& frame, uint32_t crc) { frame.push_back(static_cast(crc & 0xFF)); } -// verify_crc recomputes the CRC over stream_id/avtp_timestamp/info/payload -// and compares it against `received_crc` (as decoded from a frame's -// trailing 4 octets). Returns E2eErrc::crc_error — the CRC_ERROR failure -// path — on mismatch. +// verify_crc recomputes the CRC over +// stream_id/avtp_timestamp/info/message_timestamp/payload and compares it +// against `received_crc` (as decoded from a frame's trailing 4 octets). +// Returns E2eErrc::crc_error — the CRC_ERROR failure path, whose numeric +// TC18 wire error code is e2e::kPociFailureErrorCode / +// e2e::wire_error_code() above — on mismatch. inline std::error_code verify_crc(const avtp::StreamId& stream_id, std::optional avtp_timestamp, - const acf::AcfMessageInfo& info, const std::vector& payload, - uint32_t received_crc) { - if (compute_crc(stream_id, avtp_timestamp, info, payload) != received_crc) + const acf::AcfMessageInfo& info, std::optional message_timestamp, + const std::vector& payload, uint32_t received_crc) { + if (compute_crc(stream_id, avtp_timestamp, info, message_timestamp, payload) != received_crc) return make_error_code(E2eErrc::crc_error); return {}; } diff --git a/include/rcp/record.hpp b/include/rcp/record.hpp index 0f841ab..ba078ea 100644 --- a/include/rcp/record.hpp +++ b/include/rcp/record.hpp @@ -173,7 +173,12 @@ class Record { f.read(reinterpret_cast(encoded.data()), static_cast(len)); if (!f.good()) return std::make_error_code(std::errc::io_error); } - if (!encoded.empty() && encoded[0] == acf::kAcfMsgTypeGbb) { + // acf_msg_type is a 7-bit field, not a whole octet (rcp/acf.hpp + // v2.19.0 wire conformance pass, issue cpp-RCP-04) — go through + // acf::peek_acf_msg_type() rather than comparing encoded[0] + // directly, which would almost never match kAcfMsgTypeGbb now that + // byte0's LSB is acf_msg_length's MSB. + if (!encoded.empty() && acf::peek_acf_msg_type(encoded.data()) == acf::kAcfMsgTypeGbb) { uint64_t ts = 0; return acf::decode_acf_gbb(encoded.data(), encoded.size(), info, ts, payload); } diff --git a/include/rcp/udp.hpp b/include/rcp/udp.hpp index 7a25584..dbd86f5 100644 --- a/include/rcp/udp.hpp +++ b/include/rcp/udp.hpp @@ -161,7 +161,12 @@ inline std::error_code decode_frame(const uint8_t* b, size_t len, Frame& out) { } if (len < acf_off + 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); - if (b[acf_off] == acf::kAcfMsgTypeGbb) { + // acf_msg_type is a 7-bit field, not a whole octet (rcp/acf.hpp v2.19.0 + // wire conformance pass, issue cpp-RCP-04) — go through + // acf::peek_acf_msg_type() rather than comparing b[acf_off] directly, + // which would almost never match kAcfMsgTypeGbb now that byte0's LSB is + // acf_msg_length's MSB. + if (acf::peek_acf_msg_type(b + acf_off) == acf::kAcfMsgTypeGbb) { return acf::decode_acf_gbb(b + acf_off, len - acf_off, out.info, out.message_timestamp, out.payload); } diff --git a/tests/test_acf.cpp b/tests/test_acf.cpp index 3a103f3..d5cb67f 100644 --- a/tests/test_acf.cpp +++ b/tests/test_acf.cpp @@ -7,6 +7,7 @@ // fusa:test REQ-WIRE-012 // fusa:test REQ-WIRE-013 // fusa:test REQ-WIRE-014 +// fusa:test REQ-WIRE-015 // Tests for rcp/acf.hpp — the ACF_ABB/ACF_GBB message-format half of the TC18 // wire codec (ROADMAP.md milestone 44, "Wire Format Core", v2.0.0; split @@ -40,7 +41,10 @@ TEST_CASE("ACF_ABB round-trips the shared header and payload with no timestamp f std::vector payload = {0x01, 0x02, 0x03}; auto frame = encode_acf_abb(info, payload); REQUIRE(frame.size() == kAcfCommonHeaderLen + payload.size()); - REQUIRE(frame[0] == kAcfMsgTypeAbb); + // acf_msg_type is a 7-bit field sharing byte0 with acf_msg_length's MSB + // (see acf.hpp's "ACF shared header" comment) — frame[0] is therefore + // NOT acf_msg_type directly; peek_acf_msg_type() extracts it correctly. + REQUIRE(peek_acf_msg_type(frame.data()) == kAcfMsgTypeAbb); AcfMessageInfo out; std::vector out_payload; @@ -76,7 +80,7 @@ TEST_CASE("ACF_GBB round-trips a 64-bit message_timestamp alongside the shared h std::vector payload = {0xAA, 0xBB, 0xCC, 0xDD}; auto frame = encode_acf_gbb(info, ts, payload); REQUIRE(frame.size() == kAcfCommonHeaderLen + kAcfGbbTimestampLen + payload.size()); - REQUIRE(frame[0] == kAcfMsgTypeGbb); + REQUIRE(peek_acf_msg_type(frame.data()) == kAcfMsgTypeGbb); AcfMessageInfo out; uint64_t out_ts = 0; @@ -87,6 +91,194 @@ TEST_CASE("ACF_GBB round-trips a 64-bit message_timestamp alongside the shared h REQUIRE(out_payload == payload); } +// ── 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): +// +// 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 +// 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 +// remaining bytes = payload, unchanged + +TEST_CASE("ACF_ABB hand-computed expected byte sequence", "[acf][REQ-WIRE-004][REQ-WIRE-006]") { + AcfMessageInfo info; + info.acf_msg_length = 5; // 9 bits: 0b0_0000_0101 + info.pad = 2; // 2 bits: 0b10 + info.byte_bus_id = 7; // 11 bits: 0b000_0000_0111 + info.evt_ack = true; // evt[3] + info.evt_op = 5; // evt[2:0] = 0b101 -> evt = 0b1101 = 0xD + info.hs = true; + info.cs = false; + info.transaction_num = 42; // 0x2A + info.op = true; + info.rsp = false; + info.err = false; + info.ms = false; + info.read_size_or_segment_num = 16; // 12 bits: 0b0000_0001_0000 + + // byte0 = (0x0E << 1) | (5 >> 8 & 1) = 0x1C | 0 = 0x1C + // byte1 = 5 & 0xFF = 0x05 + // byte2 = (2 << 6) | (0 << 5) | (7 >> 8 & 7) = 0x80 | 0 | 0 = 0x80 + // byte3 = 7 & 0xFF = 0x07 + // byte4 = (0xD << 4) | (1 << 1) | 0 = 0xD0 | 0x02 = 0xD2 + // byte5 = 42 = 0x2A + // byte6 = (1<<7) | 0 | 0 | 0 | (16 >> 8 & 0xF) = 0x80 | 0 = 0x80 + // byte7 = 16 & 0xFF = 0x10 + const std::vector expected_header = {0x1C, 0x05, 0x80, 0x07, 0xD2, 0x2A, 0x80, 0x10}; + const std::vector payload = {0x01, 0x02, 0x03}; + + auto frame = encode_acf_abb(info, payload); + REQUIRE(frame.size() == expected_header.size() + payload.size()); + std::vector header(frame.begin(), frame.begin() + static_cast(kAcfCommonHeaderLen)); + REQUIRE(header == expected_header); + REQUIRE(std::vector(frame.begin() + static_cast(kAcfCommonHeaderLen), frame.end()) == + payload); + + // Re-encoding a decode of this exact hand-computed buffer must reproduce + // it byte for byte (encode -> decode -> re-encode == identity). + AcfMessageInfo decoded; + std::vector decoded_payload; + REQUIRE_FALSE(decode_acf_abb(frame.data(), frame.size(), decoded, decoded_payload)); + auto re_encoded = encode_acf_abb(decoded, decoded_payload); + REQUIRE(re_encoded == frame); +} + +TEST_CASE("ACF_GBB hand-computed expected byte sequence", "[acf][REQ-WIRE-005][REQ-WIRE-006]") { + AcfMessageInfo info; + info.acf_msg_length = 7; // 9 bits + info.pad = 1; // 2 bits + info.mtv = true; + info.byte_bus_id = 300; // 11 bits, exercises the >8-bit range: 0b001_0010_1100 + info.evt_ack = false; + info.evt_op = 3; // evt = 0b0011 = 0x3 + info.hs = false; + info.cs = true; + info.transaction_num = 200; // 0xC8 + info.op = false; + info.rsp = true; + info.err = true; + 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}; + 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}; + + 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); + + AcfMessageInfo decoded; + uint64_t decoded_ts = 0; + std::vector decoded_payload; + REQUIRE_FALSE(decode_acf_gbb(frame.data(), frame.size(), decoded, decoded_ts, decoded_payload)); + REQUIRE(decoded.byte_bus_id == 300); + auto re_encoded = encode_acf_gbb(decoded, decoded_ts, decoded_payload); + REQUIRE(re_encoded == frame); +} + +// This is an independent cross-check against a fully worked numeric example +// elsewhere in the specification (an ACF_ABB message with a 6-byte payload +// and 2 bytes of padding): quadlet-counting the 8-byte Message Info (2 +// quadlets) + 8-byte padded payload (2 quadlets) + a 4-byte CRC trailer (1 +// quadlet, per rcp/e2e.hpp's kCrcLengthAdjustQuadlets) gives 5 quadlets — +// this test only checks that this file's own encoder faithfully places +// whatever acf_msg_length/pad values a caller (such as rcp/e2e.hpp) computed +// into the correct wire bits; it does not itself compute quadlet counts. +TEST_CASE("acf_msg_length/pad wire bits round-trip the specification's own worked-example values", + "[acf][REQ-WIRE-004]") { + AcfMessageInfo info; + info.acf_msg_length = 5; // Message Info (2 quadlets) + padded payload (2) + CRC trailer (1) + info.pad = 2; // 6 payload bytes + 2 pad bytes = 8 bytes = 2 quadlets + std::vector payload(8, 0); // 6 "real" + 2 padding bytes, caller-padded per convention + + auto frame = encode_acf_abb(info, payload); + AcfMessageInfo out; + std::vector out_payload; + REQUIRE_FALSE(decode_acf_abb(frame.data(), frame.size(), out, out_payload)); + REQUIRE(out.acf_msg_length == 5); + REQUIRE(out.pad == 2); +} + +// ── evt[3:0] / response semantics ────────────────────────────────────────────── + +TEST_CASE("evt occupies exactly 4 bits and evt_ack/evt_op together reproduce evt[3:0] == 0xF " + "for kEvtAcknowledge", + "[acf][REQ-WIRE-010]") { + AcfMessageInfo info; + info.evt_ack = true; + info.evt_op = 0x7; // together: 0b1111 = 0xF + + uint8_t hdr[kAcfCommonHeaderLen]; + encode_acf_message_info(info, hdr); + REQUIRE(((hdr[4] >> 4) & 0x0F) == kEvtAcknowledge); + + AcfMessageInfo out; + decode_acf_message_info(hdr, out); + REQUIRE(out.evt_ack == true); + REQUIRE(out.evt_op == 0x7); +} + +TEST_CASE("make_response sets rsp on every response kind, including Acknowledge", + "[acf][REQ-WIRE-010]") { + // Table 15 (extraction): every response — Acknowledge included — carries + // rsp=1; only requests carry rsp=0. This is a correction from this + // codec's pre-v2.19.0 behavior, which modeled Acknowledge as rsp=0. + auto req = make_standard_request(1, 1, false, 4); + for (auto kind : {ResponseKind::Acknowledge, ResponseKind::WriteResponse, + ResponseKind::ReadResponse, ResponseKind::ErrorResponse}) { + auto resp = make_response(req, kind); + REQUIRE(resp.rsp == true); + REQUIRE(response_kind_of(resp) == kind); + } +} + +// ── Sub-octet field width masking ───────────────────────────────────────────── + +TEST_CASE("byte_bus_id round-trips values above 255 (11-bit wire field)", "[acf][REQ-WIRE-004]") { + AcfMessageInfo info; + info.byte_bus_id = 2047; // max 11-bit value + uint8_t hdr[kAcfCommonHeaderLen]; + encode_acf_message_info(info, hdr); + AcfMessageInfo out; + decode_acf_message_info(hdr, out); + REQUIRE(out.byte_bus_id == 2047); +} + +TEST_CASE("read_size_or_segment_num round-trips its full 12-bit range", "[acf][REQ-WIRE-004]") { + AcfMessageInfo info; + info.read_size_or_segment_num = 4095; // max 12-bit value + uint8_t hdr[kAcfCommonHeaderLen]; + encode_acf_message_info(info, hdr); + AcfMessageInfo out; + decode_acf_message_info(hdr, out); + REQUIRE(out.read_size_or_segment_num == 4095); +} + TEST_CASE("decode_acf_abb rejects an ACF_GBB-typed buffer and vice versa", "[acf][REQ-WIRE-014]") { AcfMessageInfo info; auto gbb_frame = encode_acf_gbb(info, 0, {}); @@ -199,3 +391,70 @@ TEST_CASE("effective_timestamp is nullopt, not zero, when neither source is vali auto ts2 = effective_timestamp(nullptr, nullptr, 0); REQUIRE_FALSE(ts2.has_value()); } + +// ── RELAY spec §15.5 canonical types (cpp-RCP-FS-06) ────────────────────────── + +TEST_CASE("ControlFlags bit values match RELAY spec §15.5 exactly", "[acf][relay-spec][REQ-WIRE-015]") { + REQUIRE(static_cast(ControlFlags::FlagAck) == 0x80); + REQUIRE(static_cast(ControlFlags::FlagRead) == 0x40); + REQUIRE(static_cast(ControlFlags::FlagWrite) == 0x20); + REQUIRE(static_cast(ControlFlags::FlagResponse) == 0x10); + REQUIRE(static_cast(ControlFlags::FlagError) == 0x08); + REQUIRE(static_cast(ControlFlags::FlagMoreSegments) == 0x04); +} + +TEST_CASE("to_message/from_message round-trip an ACF_ABB write request", "[acf][relay-spec][REQ-WIRE-015]") { + AcfMessageInfo info; + info.byte_bus_id = 9; + info.transaction_num = 3; + info.op = true; // write + info.ms = false; + info.read_size_or_segment_num = 0; + std::vector body = {0x01, 0x02}; + + auto msg = to_message(info, /*message_timestamp=*/0, body); + REQUIRE(msg.byte_bus_id == 9); + REQUIRE(msg.transaction_num == 3); + REQUIRE(has_flag(msg.control, ControlFlags::FlagWrite)); + REQUIRE_FALSE(has_flag(msg.control, ControlFlags::FlagRead)); + REQUIRE_FALSE(has_flag(msg.control, ControlFlags::FlagResponse)); + REQUIRE(msg.timestamp == 0); + REQUIRE(msg.body == body); + + uint64_t out_ts = 0xFFFFFFFFFFFFFFFFULL; // poisoned, must be overwritten to 0 for ABB + auto back = from_message(msg, /*as_gbb=*/false, /*mtv=*/false, out_ts); + REQUIRE(back.byte_bus_id == info.byte_bus_id); + REQUIRE(back.transaction_num == info.transaction_num); + REQUIRE(back.op == info.op); + REQUIRE(out_ts == msg.timestamp); +} + +TEST_CASE("to_message reflects Acknowledge/Response/Error/MoreSegments flags", "[acf][relay-spec][REQ-WIRE-015]") { + auto req = make_standard_request(1, 1, false, 4); + auto ack = make_response(req, ResponseKind::Acknowledge); + auto msg = to_message(ack, 0, {}); + REQUIRE(has_flag(msg.control, ControlFlags::FlagAck)); + REQUIRE(has_flag(msg.control, ControlFlags::FlagResponse)); + + auto err_resp = make_response(req, ResponseKind::ErrorResponse); + auto err_msg = to_message(err_resp, 0, {}); + REQUIRE(has_flag(err_msg.control, ControlFlags::FlagError)); + REQUIRE(has_flag(err_msg.control, ControlFlags::FlagResponse)); + + AcfMessageInfo frag_info; + frag_info.ms = true; + auto frag_msg = to_message(frag_info, 0, {}); + REQUIRE(has_flag(frag_msg.control, ControlFlags::FlagMoreSegments)); +} + +TEST_CASE("to_message carries message_timestamp only for ACF_GBB", "[acf][relay-spec][REQ-WIRE-015]") { + AcfMessageInfo abb_info; + abb_info.acf_msg_type = kAcfMsgTypeAbb; + auto abb_msg = to_message(abb_info, /*message_timestamp=*/0xDEADBEEF, {}); + REQUIRE(abb_msg.timestamp == 0); // ACF_ABB has no timestamp field; not leaked through + + AcfMessageInfo gbb_info; + gbb_info.acf_msg_type = kAcfMsgTypeGbb; + auto gbb_msg = to_message(gbb_info, /*message_timestamp=*/0xDEADBEEF, {}); + REQUIRE(gbb_msg.timestamp == 0xDEADBEEF); +} diff --git a/tests/test_avtp.cpp b/tests/test_avtp.cpp index 0a95e3a..b1e9177 100644 --- a/tests/test_avtp.cpp +++ b/tests/test_avtp.cpp @@ -47,7 +47,7 @@ TEST_CASE("Distinct MAC halves produce distinct StreamId values", "[avtp][REQ-WI TEST_CASE("NTSCF header round-trips stream_id, sequence_num, control_data_length", "[avtp][REQ-WIRE-001]") { NtscfHeader hdr; hdr.stream_id = make_stream_id(0xAA, 7); - hdr.sequence_num = 4242; + hdr.sequence_num = 242; // sequence_num is an 8-bit field (0-255) hdr.control_data_length = 128; auto buf = encode_ntscf_header(hdr); @@ -93,6 +93,105 @@ TEST_CASE("TSCF header marks timestamp invalid distinctly from a zero timestamp" REQUIRE_FALSE(out.timestamp_valid); } +// ── 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 avtp.hpp's "NTSCF / TSCF AVTPDU headers" +// comment) — not copied from anywhere. MSB-first bit numbering throughout +// (bit0 = MSB of byte0), matching the specification's own diagrams. + +TEST_CASE("NTSCF header hand-computed expected byte sequence", "[avtp][REQ-WIRE-001]") { + NtscfHeader hdr; + hdr.stream_id = make_stream_id(0x02, 0xBEEF); // mac 02 03 04 05 06 07, suffix BEEF + hdr.sequence_num = 200; // 0xC8, a full octet + hdr.control_data_length = 1500; // 0x5DC, 11-bit field: top3=0b101=5, low8=0xDC + + // byte0 = subtype = 0x82 + // byte1 = sv(0x80) | control_data_length[10:8] = 0x80 | 0x05 = 0x85 + // byte2 = control_data_length[7:0] = 0xDC + // byte3 = sequence_num = 0xC8 + // bytes4-11 = stream_id = 02 03 04 05 06 07 BE EF + const std::vector expected = {0x82, 0x85, 0xDC, 0xC8, + 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0xBE, 0xEF}; + auto buf = encode_ntscf_header(hdr); + REQUIRE(buf == expected); + + NtscfHeader decoded; + REQUIRE_FALSE(decode_ntscf_header(buf.data(), buf.size(), decoded)); + REQUIRE(decoded.sequence_num == 200); + REQUIRE(decoded.control_data_length == 1500); + REQUIRE(decoded.stream_id == hdr.stream_id); + REQUIRE(encode_ntscf_header(decoded) == buf); // encode -> decode -> re-encode == identity +} + +TEST_CASE("TSCF header hand-computed expected byte sequence", "[avtp][REQ-WIRE-002][REQ-WIRE-011]") { + TscfHeader hdr; + hdr.stream_id = make_stream_id(0x10, 0x2233); // mac 10 11 12 13 14 15, suffix 2233 + hdr.sequence_num = 77; // 0x4D + hdr.timestamp_valid = true; + hdr.avtp_timestamp = 0xCAFEBABE; + hdr.control_data_length = 0x1234; // full 16-bit field (stream_data_length) + + // byte0 = subtype = 0x05 + // byte1 = sv(0x80) | tv(0x01) = 0x81 (version/mr/rsv all 0) + // byte2 = sequence_num = 0x4D + // byte3 = reserved(7)+tu(1) = 0x00 + // bytes4-11 = stream_id = 10 11 12 13 14 15 22 33 + // bytes12-15 = avtp_timestamp = CA FE BA BE + // bytes16-19 = reserved = 00 00 00 00 + // bytes20-21 = control_data_length = 12 34 + // bytes22-23 = reserved = 00 00 + const std::vector expected = { + 0x05, 0x81, 0x4D, 0x00, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x22, 0x33, + 0xCA, 0xFE, 0xBA, 0xBE, + 0x00, 0x00, 0x00, 0x00, + 0x12, 0x34, + 0x00, 0x00}; + auto buf = encode_tscf_header(hdr); + REQUIRE(buf.size() == 24); + REQUIRE(buf == expected); + + TscfHeader decoded; + REQUIRE_FALSE(decode_tscf_header(buf.data(), buf.size(), decoded)); + REQUIRE(decoded.sequence_num == 77); + REQUIRE(decoded.timestamp_valid == true); + REQUIRE(decoded.avtp_timestamp == 0xCAFEBABE); + REQUIRE(decoded.control_data_length == 0x1234); + REQUIRE(decoded.stream_id == hdr.stream_id); + REQUIRE(encode_tscf_header(decoded) == buf); // encode -> decode -> re-encode == identity +} + +// ── Sub-octet field width ────────────────────────────────────────────────────── + +TEST_CASE("NTSCF sequence_num is an 8-bit field, not 16", "[avtp][REQ-WIRE-001]") { + NtscfHeader hdr; + hdr.sequence_num = 255; + auto buf = encode_ntscf_header(hdr); + REQUIRE(buf[3] == 255); + NtscfHeader out; + REQUIRE_FALSE(decode_ntscf_header(buf.data(), buf.size(), out)); + REQUIRE(out.sequence_num == 255); +} + +TEST_CASE("NTSCF control_data_length round-trips its full 11-bit range", "[avtp][REQ-WIRE-001]") { + NtscfHeader hdr; + hdr.control_data_length = 2047; // max 11-bit value + auto buf = encode_ntscf_header(hdr); + NtscfHeader out; + REQUIRE_FALSE(decode_ntscf_header(buf.data(), buf.size(), out)); + REQUIRE(out.control_data_length == 2047); +} + +TEST_CASE("ByteBusId round-trips values above 255 through AcfMessageInfo (11-bit wire field)", + "[avtp][REQ-WIRE-001]") { + // ByteBusId itself (avtp.hpp) is just a uint16_t alias; this checks the + // alias is actually wide enough for the wire field it names, since the + // sub-octet packing itself is exercised in tests/test_acf.cpp. + static_assert(sizeof(ByteBusId) >= sizeof(uint16_t), "ByteBusId must be wide enough for the 11-bit wire field"); + ByteBusId id = 2047; + REQUIRE(id == 2047); +} + // ── AVTPDU subtype validation ────────────────────────────────────────────────── TEST_CASE("decode_ntscf_header rejects a TSCF-subtype buffer", "[avtp][REQ-WIRE-003]") { diff --git a/tests/test_e2e.cpp b/tests/test_e2e.cpp index 4aba577..52b5723 100644 --- a/tests/test_e2e.cpp +++ b/tests/test_e2e.cpp @@ -77,19 +77,19 @@ TEST_CASE("coverage_buffer zero-fills avtp_timestamp under NTSCF (nullopt)", "[e info.byte_bus_id = 5; std::vector payload{0xAA, 0xBB}; - auto with_zero = coverage_buffer(sid, uint32_t{0}, info, payload); - auto with_nullopt = coverage_buffer(sid, std::nullopt, info, payload); + auto with_zero = coverage_buffer(sid, uint32_t{0}, info, std::nullopt, payload); + auto with_nullopt = coverage_buffer(sid, std::nullopt, info, std::nullopt, payload); REQUIRE(with_zero == with_nullopt); } -TEST_CASE("coverage_buffer layout is stream_id + avtp_timestamp + ACF header + payload", +TEST_CASE("coverage_buffer layout is stream_id + avtp_timestamp + ACF header + payload for ACF_ABB", "[e2e][REQ-E2E-002]") { auto sid = make_stream_id(0x02, 0x0001); - AcfMessageInfo info; + AcfMessageInfo info; // acf_msg_type defaults to kAcfMsgTypeAbb info.byte_bus_id = 7; std::vector payload{1, 2, 3}; - auto buf = coverage_buffer(sid, uint32_t{0xDEADBEEF}, info, payload); + auto buf = coverage_buffer(sid, uint32_t{0xDEADBEEF}, info, std::nullopt, payload); REQUIRE(buf.size() == 8 + 4 + rcp::acf::kAcfCommonHeaderLen + payload.size()); // stream_id occupies the first 8 bytes, big-endian. @@ -104,22 +104,67 @@ 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", + "[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; + std::vector payload{0x11, 0x22}; + const uint64_t ts = 0x0102030405060708ULL; + + 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); + + // An ACF_ABB message with the same nominal message_timestamp argument + // must NOT grow by those 8 bytes — the type gates inclusion, not merely + // whether the caller happens to pass a value. + AcfMessageInfo abb_info; + abb_info.byte_bus_id = 9; + auto abb_buf = coverage_buffer(sid, uint32_t{0}, abb_info, ts, payload); + REQUIRE(abb_buf.size() == 8 + 4 + rcp::acf::kAcfCommonHeaderLen + payload.size()); +} + TEST_CASE("compute_crc changes when covered fields change", "[e2e][REQ-E2E-002]") { auto sid = make_stream_id(0x02, 0x0001); AcfMessageInfo info; info.byte_bus_id = 1; std::vector payload{9, 9, 9}; - uint32_t base = compute_crc(sid, std::nullopt, info, payload); + uint32_t base = compute_crc(sid, std::nullopt, info, std::nullopt, payload); AcfMessageInfo different_info = info; different_info.byte_bus_id = 2; - REQUIRE(compute_crc(sid, std::nullopt, different_info, payload) != base); + REQUIRE(compute_crc(sid, std::nullopt, different_info, std::nullopt, payload) != base); std::vector different_payload{9, 9, 8}; - REQUIRE(compute_crc(sid, std::nullopt, info, different_payload) != base); + REQUIRE(compute_crc(sid, std::nullopt, info, std::nullopt, different_payload) != base); + + REQUIRE(compute_crc(sid, uint32_t{1}, info, std::nullopt, payload) != base); - REQUIRE(compute_crc(sid, uint32_t{1}, info, payload) != base); + // A GBB message's CRC must also be sensitive to message_timestamp itself + // (cpp-RCP-N2-03) — this would have been silently ignored before the fix. + AcfMessageInfo gbb_info = info; + gbb_info.acf_msg_type = rcp::acf::kAcfMsgTypeGbb; + uint32_t gbb_base = compute_crc(sid, std::nullopt, gbb_info, uint64_t{1}, payload); + REQUIRE(compute_crc(sid, std::nullopt, gbb_info, uint64_t{2}, payload) != gbb_base); + REQUIRE(compute_crc(sid, std::nullopt, gbb_info, std::nullopt, payload) != gbb_base); } TEST_CASE("apply_acf_length_adjustment adds exactly one quadlet", "[e2e][REQ-E2E-003]") { @@ -153,12 +198,41 @@ TEST_CASE("verify_crc accepts a matching CRC and rejects a corrupted one", "[e2e info.byte_bus_id = 3; std::vector payload{1, 2, 3, 4}; - uint32_t crc = compute_crc(sid, std::nullopt, info, payload); - REQUIRE_FALSE(verify_crc(sid, std::nullopt, info, payload, crc)); - REQUIRE(verify_crc(sid, std::nullopt, info, payload, crc ^ 0xFFFFFFFFu) == + uint32_t crc = compute_crc(sid, std::nullopt, info, std::nullopt, payload); + REQUIRE_FALSE(verify_crc(sid, std::nullopt, info, std::nullopt, payload, crc)); + REQUIRE(verify_crc(sid, std::nullopt, info, std::nullopt, payload, crc ^ 0xFFFFFFFFu) == make_error_code(E2eErrc::crc_error)); } +TEST_CASE("verify_crc for ACF_GBB fails if the message_timestamp used to verify differs from " + "the one used to compute", + "[e2e][REQ-E2E-004]") { + auto sid = make_stream_id(0x02, 0x0005); + AcfMessageInfo info; + info.acf_msg_type = rcp::acf::kAcfMsgTypeGbb; + info.byte_bus_id = 4; + std::vector payload{5, 6, 7}; + + uint32_t crc = compute_crc(sid, std::nullopt, info, uint64_t{42}, payload); + REQUIRE_FALSE(verify_crc(sid, std::nullopt, info, uint64_t{42}, payload, crc)); + REQUIRE(verify_crc(sid, std::nullopt, info, uint64_t{43}, payload, crc) == + make_error_code(E2eErrc::crc_error)); +} + +// ── Numeric TC18 wire error code (cpp-RCP-08) ───────────────────────────────── + +TEST_CASE("wire_error_code maps crc_error to TC18's numeric POCI_FAILURE (12)", + "[e2e][REQ-E2E-004]") { + auto code = wire_error_code(E2eErrc::crc_error); + REQUIRE(code.has_value()); + REQUIRE(*code == 12); + REQUIRE(*code == kPociFailureErrorCode); + + // sequence_violation has no TC18 Table 27 entry of its own — this + // module reports "no mapping" rather than guessing one. + REQUIRE_FALSE(wire_error_code(E2eErrc::sequence_violation).has_value()); +} + TEST_CASE("append_crc appends exactly 4 big-endian octets", "[e2e][REQ-E2E-004]") { std::vector frame{0x11, 0x22}; append_crc(frame, 0x01020304); diff --git a/tests/test_udp.cpp b/tests/test_udp.cpp index e639ba4..8e4931d 100644 --- a/tests/test_udp.cpp +++ b/tests/test_udp.cpp @@ -289,7 +289,11 @@ TEST_CASE("Client::request correlates concurrent requests by byte_bus_id/transac const std::vector&, acf::AcfMessageInfo& out_resp, std::vector& out_resp_payload) { out_resp = acf::make_response(req, acf::ResponseKind::ReadResponse); - out_resp_payload = {req.byte_bus_id, req.transaction_num}; + // avtp::ByteBusId widened to uint16_t (rcp/avtp.hpp v2.19.0 wire + // conformance pass, issue cpp-RCP-04, since byte_bus_id is an + // 11-bit wire field) — narrow explicitly for this single-byte test + // payload rather than relying on an implicit narrowing conversion. + out_resp_payload = {static_cast(req.byte_bus_id), req.transaction_num}; return std::error_code{}; }); From c0974dd5bacdf666babd1044774899577c8310e9 Mon Sep 17 00:00:00 2001 From: Matt <47545907+SoundMatt@users.noreply.github.com> Date: Thu, 30 Jul 2026 11:40:08 -0700 Subject: [PATCH 2/2] fix: make test_capi's byte_bus_id C-ABI truncation explicit for MSVC /WX test_capi.cpp's mock_server_request_fn assigns AcfMessageInfo::byte_bus_id (now avtp::ByteBusId = uint16_t, an 11-bit wire field) into rcp_response_t::info.byte_bus_id (rcp_byte_bus_id_t = uint8_t, a pre-existing, narrower C-ABI type unrelated to and out of scope for this fix pass). GCC/Clang silently truncate; MSVC's warnings-as-errors build fails on the implicit narrowing (C4244 -> C2220). Add an explicit cast with a comment explaining the pre-existing ABI width gap, rather than changing capi.h's public C ABI. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> --- tests/test_capi.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_capi.cpp b/tests/test_capi.cpp index 319df61..57dd02a 100644 --- a/tests/test_capi.cpp +++ b/tests/test_capi.cpp @@ -88,7 +88,12 @@ rcp_err_t mock_server_request_fn(void* userdata, const rcp_request_t* req, std::memcpy(resp->payload, out_payload.data(), out_payload.size()); resp->payload_len = static_cast(out_payload.size()); resp->info.acf_msg_type = out_info.acf_msg_type; - resp->info.byte_bus_id = out_info.byte_bus_id; + // rcp_byte_bus_id_t (capi.h) is a pre-existing 8-bit C ABI type, narrower + // than avtp::ByteBusId's corrected 11-bit wire range (wire-format + // conformance fix pass) -- out of scope to widen the C ABI here, so this + // cast just makes the pre-existing truncation explicit instead of an + // implicit narrowing conversion. + resp->info.byte_bus_id = static_cast(out_info.byte_bus_id); resp->info.rsp = out_info.rsp ? 1 : 0; resp->info.err = out_info.err ? 1 : 0; resp->info.op = out_info.op ? 1 : 0;