From 051646af6c837607a04f3179e45f3e2fbf83b033 Mon Sep 17 00:00:00 2001 From: Matt <47545907+SoundMatt@users.noreply.github.com> Date: Fri, 31 Jul 2026 05:37:46 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20v2.20.0=20=E2=80=94=20TC18=20conformance?= =?UTF-8?q?=20fix=20pass=20II?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second gap-audit pass against the specification's own worked numeric examples and per-endpoint request-handling text. Fixes, in priority order: - acf_msg_length is now actually computed (cpp-RCP-01, critical): AcfMessageInfo::acf_msg_length defaulted to 0 and nothing filled it in, so every frame this library emitted carried acf_msg_length=0 despite the header *bit layout* already being correct. New acf::compute_acf_msg_length() computes the real quadlet count; encode_acf_abb/encode_acf_gbb call it automatically whenever a caller leaves acf_msg_length at its 0 default, while still respecting a caller-computed nonzero value (e.g. e2e.hpp's apply_acf_length_adjustment baking in a CRC trailer's +1 quadlet). Verified end to end against the spec's own two worked examples (ACF_ABB -> 0x05 for a 20-byte frame, ACF_GBB -> 0x07 for a 28-byte frame) in tests/test_e2e.cpp. - Multiple ACF requests packed into one AVTPDU are now handled (cpp-RCP-04-fresh, critical): §12.9.1.1 mandates it. New acf::decode_acf_messages/AcfEntry walk a buffer that may hold more than one ACF_ABB/ACF_GBB message; udp.hpp gains a parallel MultiFrame/encode_multi_frame/decode_multi_frame envelope, and udp::Server::serve() now dispatches and answers every request in an inbound datagram individually instead of only the first one. - Error responses now carry a Table 27 error-code byte_msg_payload (cpp-RCP-02, high): acf::WireErrorCode fixes Table 27's 17 numeric values (deliberately independent of regmap.hpp's own RegMapErrc ordinals); mock.hpp gains wire_error_code_for() and routes every ErrorResponse through one set_error_response() helper. - GPIO's payload decoder now rejects a mis-sized buffer (cpp-RCP-05-fresh, medium), matching PWM's exact-four-bytes fix (cpp-RCP-03, applied from a pre-verified diff in this same pass). - ISELED gains a real ACF payload codec (cpp-RCP-A4-iseled, medium): encode/decode for both the Instruction/Address/Data request shape (Figure 40) and the Address/Data[11:0] response shape (Figure 41) — previously only validated in-memory structs existed, with no way to actually build or parse an ISELED request/response for the wire. - PWM_IN's trigger signals now match Table 44 (cpp-RCP-A4-pwmin, low): RisingEdge/FallingEdge replace the single invented MidPulse signal. - control_data_length is now validated against the buffer it actually arrived in (cpp-RCP-A3, low), via the already-defined-but-unused avtp::AvtpErrc::length_mismatch. Deferred: cpp-RCP-A1 (CI's cpfusa analyze step is non-gating) and cpp-RCP-A2 (GitHub Actions pinned to mutable tags, not SHAs) — both process/CI items with no TC18 wire-conformance content, deferred because this environment has no cpfusa binary/build to verify an analyze-gating change against and no way to test a SHA-pin bump's effect on hosted Actions runners from here. Wire-format-adjacent, non-breaking-at-the-API-level: every emitted ACF frame's acf_msg_length bytes and every error response's payload bytes now differ from what v2.19.0 emitted (0 -> correct value; empty -> one octet). No C++ function signature changed. kVersion/CMakeLists.txt bump 2.19.0 -> 2.20.0 accordingly. Full local build (C++17, all targets) and ctest (52/52 suites) pass. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> --- CMakeLists.txt | 2 +- ROADMAP.md | 115 ++++++++++++++++++++- include/rcp/acf.hpp | 216 ++++++++++++++++++++++++++++++++++++++-- include/rcp/e2e.hpp | 15 ++- include/rcp/gpio.hpp | 6 +- include/rcp/iseled.hpp | 69 +++++++++++++ include/rcp/mock.hpp | 78 +++++++++------ include/rcp/pwm.hpp | 38 +++++-- include/rcp/udp.hpp | 170 +++++++++++++++++++++++++------ include/rcp/version.hpp | 2 +- tests/test_acf.cpp | 78 +++++++++++++++ tests/test_e2e.cpp | 76 ++++++++++++++ tests/test_gpio.cpp | 10 ++ tests/test_iseled.cpp | 75 ++++++++++++++ tests/test_mock.cpp | 23 +++++ tests/test_pwm.cpp | 41 ++++++-- tests/test_udp.cpp | 112 +++++++++++++++++++++ 17 files changed, 1037 insertions(+), 89 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d73d7c1..6d0ddb5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.21) project(cpp-rcp - VERSION 2.19.0 + VERSION 2.20.0 DESCRIPTION "Remote Control Protocol for automotive zonal architecture (C++)" HOMEPAGE_URL "https://github.com/SoundMatt/cpp-RCP" LANGUAGES CXX diff --git a/ROADMAP.md b/ROADMAP.md index 9db964e..fa625ed 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -133,6 +133,7 @@ work, since every endpoint type's functional config block depends on it. | **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** | 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** | v3.0.0 | **TC18 RCP — General Availability** | First release where cpp-RCP *is* the OPEN Alliance TC18 Remote Control Protocol | --- @@ -1566,9 +1567,10 @@ 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 #64 (TC18 RCP General Availability, v3.0.0), +v2.19.0) is next, ahead of #65 (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. +requires to be correct. (Milestone #64, TC18 Conformance Fix Pass II, +v2.20.0, was added later in the same vein — see its own entry.) ### Retired-model residue cleanup (2026-07-30, v2.19.0 — audit findings cpp-RCP-FS-01..05, cpp-RCP-05, cpp-RCP-07) @@ -1765,7 +1767,7 @@ 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 +(milestone #65) remains a distinct, later milestone rather than being folded into this one. Both `AcfMessageInfo`'s and the NTSCF/TSCF headers' public field names are @@ -1784,7 +1786,112 @@ pure widening change with no consumer breakage found across the tree 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) +### 64. TC18 Conformance Fix Pass II (v2.20.0) + +A second gap-audit pass (2026-07-31) against the specification's own worked +numeric examples and per-endpoint request-handling text found that +milestone 63's header-layout fix had left `acf_msg_length` itself unfilled, +missed one mandatory §12.9.1.1 requirement outright, and left several +smaller per-endpoint gaps. This pass addresses each: + +- **`acf_msg_length` is now actually computed (cpp-RCP-01, critical).** + `AcfMessageInfo::acf_msg_length` defaulted to 0 and nothing filled it in — + every frame this library emitted carried `acf_msg_length=0`, despite + milestone 63 having already fixed the *bit layout* that field lives in. + `rcp::acf::compute_acf_msg_length()` computes the real quadlet count + (header, plus the ACF_GBB timestamp when present, plus payload); + `encode_acf_abb`/`encode_acf_gbb` call it automatically whenever a caller + leaves `acf_msg_length` at its 0 default (the common case for every real + request/response builder in this tree), while still respecting a nonzero + value a caller — e.g. `rcp/e2e.hpp`'s `apply_acf_length_adjustment()`, + baking in a trailing CRC's +1 quadlet — has already computed itself. + Verified end to end against the specification's own two worked examples + (ACF_ABB → `acf_msg_length` 0x05 for a 20-byte frame; ACF_GBB → + `acf_msg_length` 0x07 for a 28-byte frame) in `tests/test_e2e.cpp`. +- **Multiple ACF requests packed into one AVTPDU are now handled + (cpp-RCP-04-fresh, critical).** §12.9.1.1: "An RC Server shall support to + handle multiple requests in one frame and check each of them individually + ... The RC Server shall support the handling of multiple request types in + one frame." `rcp::acf::decode_acf_messages`/`AcfEntry` walk a buffer that + may hold more than one ACF_ABB/ACF_GBB message back to back, using each + message's own (now correctly non-zero) `acf_msg_length` to find the next + one; `rcp/udp.hpp` gains a parallel `MultiFrame`/`encode_multi_frame`/ + `decode_multi_frame` AVTPDU envelope, and `udp::Server::serve()` is + rebuilt on it — every request in an inbound datagram is now dispatched + and answered individually, in order, in a single reply datagram, instead + of only ever seeing the first one. `udp::Client` intentionally keeps + sending one request per call (see its own updated header comment) — the + spec's "shall support" wording is an RC *Server*-side requirement. + Known scope limit: precise multi-message boundary-finding requires each + non-final message's payload to already be quadlet-aligned (true for e.g. + GPIO/PWM's fixed 4-byte payloads); a non-final message with a non-aligned + payload falls outside this pass — see `decode_acf_messages`'s own comment. +- **Error responses now carry a Table 27 error-code `byte_msg_payload` + (cpp-RCP-02, high).** §12.9.6: "The error response shall contain a + byte_msg_payload with an error code." Every `ErrorResponse` this tree + builds left that payload empty before this pass. `rcp::acf::WireErrorCode` + fixes Table 27's 17 numeric values (deliberately independent of + `rcp/regmap.hpp`'s own `RegMapErrc` ordinals, which do not match Table + 27's numbering for the same-named codes) and `encode_error_payload()` + builds the one-octet payload; `rcp/mock.hpp` gains `wire_error_code_for()` + translating each subsystem's internal `std::error_code` and routes every + `ErrorResponse` call site through one `set_error_response()` helper. +- **GPIO's payload decoder now rejects a mis-sized buffer, matching PWM's + fix from the first pass (cpp-RCP-05-fresh, medium).** §13.7.4: "A request + not having exactly four bytes is rejected." `decode_gpio_payload` guarded + only against too-short buffers; a >4-byte buffer was silently accepted + with its trailing bytes ignored. Now checks `len != kGpioPayloadLen`, + the same fix `decode_pwm_payload` already had for §13.7.5.3's identical + rule (issue cpp-RCP-03, applied in this same pass from a pre-verified + diff). +- **ISELED gains a real ACF payload codec (cpp-RCP-A4-iseled, medium).** + `IseledRequest`/`IseledResponse` previously only modeled validated + in-memory structs — nothing packed/unpacked the 4-bit Instruction/12-bit + Address/variable Data request shape (Figure 40) or the 12-bit + Address/12-bit Data[11:0] response shape (Figure 41) into actual wire + bytes, despite ep_type 0x0C being otherwise wired up. `encode_iseled_request`/ + `decode_iseled_request` and `encode_iseled_response`/`decode_iseled_response` + are that missing codec. The optional 4-bit CRC/rsvd trailer Figure 41 + also shows is deliberately still not encoded — this repository has no + verified copy of the ISELED standard's own CRC algorithm (same reason the + header's previously-invented CRC-8 was removed outright, not replaced, at + the time of issue #71). +- **PWM_IN's trigger signals now match Table 44 (cpp-RCP-A4-pwmin, low).** + The endpoint modeled a single invented `MidPulse` signal; Table 44 defines + two independent trigger outputs, rising edge (0) and falling edge (1). + `PwmInSignal` is now `RisingEdge`/`FallingEdge`; `record_measurement` + (one full input-capture cycle, inherently spanning both edges) fires both, + and a new `record_edge()` fires either one independently for a caller + that observes edges at finer granularity. +- **`control_data_length` is now validated against the buffer it actually + arrived in (cpp-RCP-A3, low).** `rcp/udp.hpp`'s `decode_frame`/ + `decode_multi_frame` decoded the NTSCF/TSCF header's own declared ACF byte + length but never checked it against the real remaining buffer size; both + now reject a mismatch via the already-defined-but-previously-unused + `avtp::AvtpErrc::length_mismatch`. + +**Deferred from this pass:** cpp-RCP-A1 (CI's `cpfusa analyze` step runs +non-gating via `|| true`, only `cpfusa cyber --strict` gates the build) and +cpp-RCP-A2 (GitHub Actions pinned to mutable major-version tags, not commit +SHAs) — both process/CI-hardening items with no TC18 wire-conformance +content, and both risk changing what CI actually gates in ways this pass +could not verify locally (no `cpfusa` binary/build available in this +environment to test an analyze-gating change against; no way to test a +SHA-pin bump's effect on the hosted Actions runners from here). Left for a +follow-up pass with the tooling available to verify the change first. + +Full local build (C++17, all 213+ targets) and `ctest` (52/52 suites) pass +after this change. This is a **behavior-changing, non-breaking-at-the-API +level** release: every emitted ACF frame's `acf_msg_length` bytes and every +error response's payload bytes differ from what v2.19.0 emitted (0 → +correct value; empty → one octet), which is a wire-format correction, not a +regression — no C++ function signature in this tree changed. `kVersion`/ +`CMakeLists.txt` bump 2.19.0 → 2.20.0 accordingly, per this repository's +convention of reserving version bumps for commits that touch library +code/behavior (contrast the v0.36.0→v0.48.0 cpp-FuSa CI pin bump, which +stayed at the same cpp-rcp version since it touched no library code). + +### 65. 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 0b67c30..ca581cc 100644 --- a/include/rcp/acf.hpp +++ b/include/rcp/acf.hpp @@ -39,10 +39,32 @@ // 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. +// acf_msg_length value for a chosen payload/padding. +// +// acf_msg_length conformance note (v2.20.0, issue cpp-RCP-01): the bit +// layout above was already correct, but until this pass nothing in this +// codec ever *computed* acf_msg_length from a real message's actual size — +// AcfMessageInfo::acf_msg_length defaulted to 0 and stayed 0 unless a +// caller (only rcp/e2e.hpp's own unit tests did) set it by hand, so every +// frame this library actually emitted over rcp/udp.hpp carried +// acf_msg_length=0. compute_acf_msg_length() below now does that +// computation (quadlets over the shared header, the ACF_GBB timestamp when +// present, and the payload as given — see its own comment for the exact +// worked-example derivation), and encode_acf_abb()/encode_acf_gbb() call it +// automatically whenever the caller leaves AcfMessageInfo::acf_msg_length at +// its 0 default, which is what every real request/response builder in this +// tree does today (rcp::acf::make_standard_request, rcp::acf::make_response, +// rcp/mock.hpp's dispatch_*, rcp/discovery.hpp, rcp/record.hpp). A caller +// that has already computed a specific value itself — e.g. rcp/e2e.hpp's +// apply_acf_length_adjustment(), which must bake a trailing CRC's +1 +// quadlet into the header *before* that header is serialized for CRC +// coverage — sets AcfMessageInfo::acf_msg_length to a nonzero value first, +// which this codec always takes as an explicit override and never +// recomputes out from under it. See compute_acf_msg_length()'s own comment +// for this fix's known scope limit (it counts quadlets from the payload +// 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). #pragma once #include @@ -71,7 +93,8 @@ constexpr uint8_t kAcfMsgTypeAbb = 0x0E; // ACF_ABB — no timestamp field at al // second short_buffer value defined here. enum class AcfErrc : int { - bad_acf_msg_type = 1, // acf_msg_type is neither ACF_ABB nor ACF_GBB + bad_acf_msg_type = 1, // acf_msg_type is neither ACF_ABB nor ACF_GBB + bad_acf_msg_length = 2, // a decoded acf_msg_length does not fit the buffer it was found in }; inline const std::error_category& acf_category() noexcept { @@ -79,8 +102,9 @@ inline const std::error_category& acf_category() noexcept { const char* name() const noexcept override { return "rcp.acf"; } std::string message(int ev) const override { switch (static_cast(ev)) { - case AcfErrc::bad_acf_msg_type: return "rcp/acf: unrecognized ACF message type"; - default: return "rcp/acf: unknown error"; + case AcfErrc::bad_acf_msg_type: return "rcp/acf: unrecognized ACF message type"; + case AcfErrc::bad_acf_msg_length: return "rcp/acf: acf_msg_length inconsistent with buffer size"; + default: return "rcp/acf: unknown error"; } } }; @@ -169,6 +193,39 @@ struct AcfMessageInfo { constexpr size_t kAcfCommonHeaderLen = 8; constexpr size_t kAcfGbbTimestampLen = 8; +// 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 +// message about to be encoded (issue cpp-RCP-01). Confirmed against the +// specification's own two fully worked numeric examples: +// ACF_ABB, 6-byte payload + 2 pad bytes already folded into the payload +// the caller passes (8 bytes total) + a 4-byte CRC32 trailer the caller +// accounts for separately (not part of `payload_len` here, see +// rcp/e2e.hpp's apply_acf_length_adjustment): header(8) + payload(8) = 16 +// bytes = 4 quadlets; +1 quadlet for the trailer = 5 = 0x05. +// ACF_GBB, 7-byte payload + 1 pad byte already folded in (8 bytes total): +// header(8) + timestamp(8) + payload(8) = 24 bytes = 6 quadlets; +1 +// quadlet for the trailer = 7 = 0x07. +// Both match the specification's stated acf_msg_length for those examples +// exactly (see this repository's pull request description for the +// byte-by-byte derivation). +// +// Known scope limit: this function counts whatever `payload_len` it is +// given — it does not itself round `payload_len` up to a quadlet boundary +// or append real pad octets, since padding has always been a caller-owned +// concern in this codec (the `pad` field exists precisely so a caller that +// *does* pad its payload can still tell a decoder how many trailing octets +// to treat as padding — see the "ACF shared header" section above). A +// caller whose payload is not already quadlet-aligned and wants a +// byte-exact acf_msg_length must pad the payload itself (adjusting `pad` +// to match) before calling this — the same convention every existing +// caller of AcfMessageInfo::pad in this codebase already follows. +inline uint16_t compute_acf_msg_length(uint8_t acf_msg_type, size_t payload_len) noexcept { + const size_t fixed_len = kAcfCommonHeaderLen + (acf_msg_type == kAcfMsgTypeGbb ? kAcfGbbTimestampLen : 0); + const size_t total = fixed_len + payload_len; + return static_cast((total + 3) / 4); // ceiling division to whole quadlets +} + // 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 @@ -260,6 +317,17 @@ inline std::vector encode_acf_abb(AcfMessageInfo info, const std::vector& payload) { info.acf_msg_type = kAcfMsgTypeAbb; info.mtv = false; + // acf_msg_length auto-fill (issue cpp-RCP-01): 0 is never a valid real + // wire value (the shared header alone is already 2 quadlets), so a + // caller leaving AcfMessageInfo::acf_msg_length at its default means + // "compute it for me from the payload I'm actually giving you" — + // exactly what every real caller in this tree does today. A caller + // that has already computed and set a specific nonzero value itself + // (e.g. rcp/e2e.hpp's apply_acf_length_adjustment(), to bake a trailing + // CRC's +1 quadlet in before this header is serialized for CRC + // coverage) is always respected unchanged. + if (info.acf_msg_length == 0) + info.acf_msg_length = compute_acf_msg_length(info.acf_msg_type, payload.size()); std::vector buf(kAcfCommonHeaderLen + payload.size()); encode_acf_message_info(info, buf.data()); std::copy(payload.begin(), payload.end(), buf.begin() + static_cast(kAcfCommonHeaderLen)); @@ -283,6 +351,11 @@ inline std::error_code decode_acf_abb(const uint8_t* b, size_t len, inline std::vector encode_acf_gbb(AcfMessageInfo info, uint64_t message_timestamp, const std::vector& payload) { info.acf_msg_type = kAcfMsgTypeGbb; + // acf_msg_length auto-fill (issue cpp-RCP-01) — see encode_acf_abb's + // equivalent comment above for the full rationale; same "0 means + // 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); @@ -305,6 +378,91 @@ inline std::error_code decode_acf_gbb(const uint8_t* b, size_t len, return {}; } +// ── Multiple ACF requests in one frame (extraction §12.9.1.1; issue cpp-RCP-04-fresh) ── +// "An RC Server shall support to handle multiple requests in one frame and +// check each of them individually if to be processed or not ... The RC +// Server shall support the handling of multiple request types in one +// frame." decode_acf_abb/decode_acf_gbb above only ever decode a single +// message occupying the *entire* remaining buffer — correct for the common +// one-request-per-AVTPDU case, but unable to find where a second, packed-in +// request begins. decode_acf_messages below walks a buffer that may contain +// more than one ACF_ABB/ACF_GBB message back to back, using each message's +// own acf_msg_length (now populated for real by encode_acf_abb/ +// encode_acf_gbb, cpp-RCP-01 above) to find the next one. + +// AcfEntry is one decoded ACF_ABB/ACF_GBB message out of a possibly-multi- +// message buffer — the same three outputs decode_acf_abb/decode_acf_gbb +// split into separate out-parameters, bundled here so decode_acf_messages +// can return a sequence of them. +struct AcfEntry { + AcfMessageInfo info; + uint64_t message_timestamp = 0; // meaningful only when info.acf_msg_type == kAcfMsgTypeGbb + std::vector payload; +}; + +// decode_acf_messages decodes every well-formed ACF_ABB/ACF_GBB message in +// [b, b+len), advancing by each message's own acf_msg_length*4 bytes to +// find the next one. The first message is held to the same strict rules +// decode_acf_abb/decode_acf_gbb apply to a single-message buffer (a short +// buffer or an unrecognized acf_msg_type there is a hard error). Once at +// least one message has decoded successfully, this function treats a +// declared acf_msg_length that does not fit the bytes actually remaining as +// "trust the buffer, not the length" and consumes the rest of the buffer as +// that message's payload instead of failing outright — the same behavior +// decode_acf_abb/decode_acf_gbb have always had for a lone message — so a +// sender whose acf_msg_length is a caller-computed estimate rather than a +// byte-exact count (see compute_acf_msg_length's own documented scope +// limit: it does not itself pad a non-quadlet-aligned payload) still +// decodes correctly as long as it is the last (or only) message in the +// buffer. A message boundary can only be found precisely when the sender's +// acf_msg_length is byte-exact, which requires the payload to already be +// quadlet-aligned (as e.g. rcp/gpio.hpp's and rcp/pwm.hpp's fixed 4-byte +// payloads always are) — a non-final message with a non-aligned payload is +// a known residual gap of this pass, noted in this repository's pull +// request description. +inline std::error_code decode_acf_messages(const uint8_t* b, size_t len, std::vector& out) { + out.clear(); + size_t off = 0; + while (off < len) { + const size_t remaining = len - off; + const bool is_gbb = (peek_acf_msg_type(b + off) == kAcfMsgTypeGbb); + const bool is_abb = (peek_acf_msg_type(b + off) == kAcfMsgTypeAbb); + if (!is_gbb && !is_abb) { + if (out.empty()) return make_error_code(AcfErrc::bad_acf_msg_type); + break; + } + + const size_t fixed_len = kAcfCommonHeaderLen + (is_gbb ? kAcfGbbTimestampLen : 0); + 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); + + size_t msg_bytes = static_cast(info.acf_msg_length) * 4; + if (msg_bytes < fixed_len || msg_bytes > remaining) { + // Declared length doesn't fit this message's own fixed header, + // or overclaims past the end of the buffer (see this function's + // own comment above) — fall back to "consume the rest of the + // buffer", same convention decode_acf_abb/decode_acf_gbb use. + msg_bytes = remaining; + } + + AcfEntry entry; + 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.payload.assign(b + payload_off, b + payload_off + payload_len); + out.push_back(std::move(entry)); + + off += msg_bytes; + } + return {}; +} + // ── Standard request kind & response semantics ──────────────────────────────── // The mandatory baseline request kind: best-effort, unconditional dispatch // via ACF_ABB (extraction §2.7). Conditional kinds (compound, triggered, @@ -375,6 +533,50 @@ inline AcfMessageInfo make_response(const AcfMessageInfo& request, ResponseKind return resp; } +// ── Wire error codes in error responses (extraction Table 27; issue cpp-RCP-02) ── +// "The error response shall contain a byte_msg_payload with an error code" +// (extraction §12.9.6). WireErrorCode below fixes the 17 numeric values +// Table 27 assigns — the byte_msg_payload this codec builds for an +// ErrorResponse (acf::ResponseKind::ErrorResponse) is that one octet, per +// encode_error_payload(). Before this pass, nothing in this codebase built +// that payload at all: every ErrorResponse call site in rcp/mock.hpp left +// out_resp_payload empty, and the only existing numeric Table 27 mapping +// anywhere in the tree (rcp/e2e.hpp's kPociFailureErrorCode) was never +// actually wired into a response payload either. This enum is intentionally +// independent of any endpoint/subsystem's own internal std::error_code +// ordinals (e.g. rcp/regmap.hpp's RegMapErrc, whose own enumerator values +// 1..4 do NOT match Table 27's numbering for the same-named codes — compare +// RegMapErrc::unauthorized_access == 1 there against +// WireErrorCode::UnauthorizedAccess == 3 here) — callers translate their own +// internal error condition to a WireErrorCode explicitly (see +// rcp/mock.hpp's wire_error_code_for()) rather than this codec guessing a +// mapping from an arbitrary std::error_code. +enum class WireErrorCode : uint8_t { + UnsupportedCmd = 1, // requested feature/command not supported + SequencerNotKnown = 2, + UnauthorizedAccess = 3, + LockedMemAccess = 4, + RequestCanceled = 5, + RequestNotFound = 6, + EpError = 7, // error occurred during request execution; see ep_status + EpNotFound = 8, // a Trigger request refers to a nonexisting EP + PwmInNoSignal = 9, + ReqStorageOverflow = 10, + RequestRejected = 11, // other than STANDARD request during RC Server initial config phase + PociFailure = 12, // CRC of request does not match + PresentationTimeTooFar = 13, + GptpFail = 14, + InvalidParameter = 15, // request parameter out of range + ChainAborted = 16, + ChainError = 17, +}; + +// encode_error_payload builds the single-octet byte_msg_payload an +// ErrorResponse carries the numeric Table 27 code in. +inline std::vector encode_error_payload(WireErrorCode code) { + return {static_cast(code)}; +} + // ── Timestamp fallback rules ────────────────────────────────────────────────── // avtp_timestamp (TSCF-only, 32-bit, rcp/avtp.hpp) and message_timestamp // (ACF_GBB-only, 64-bit, valid only when `mtv` is set) are two independent, diff --git a/include/rcp/e2e.hpp b/include/rcp/e2e.hpp index 0310698..7143e64 100644 --- a/include/rcp/e2e.hpp +++ b/include/rcp/e2e.hpp @@ -201,9 +201,18 @@ constexpr uint16_t kCrcLengthAdjustQuadlets = 1; // +1 quadlet on AcfMessageInfo constexpr uint16_t kCrcLengthAdjustOctets = 4; // +4 octets on an outer AVTPDU frame-length field // apply_acf_length_adjustment mutates `info.acf_msg_length` in place, per -// kCrcLengthAdjustQuadlets. Call this before encode_acf_abb/encode_acf_gbb -// so the trailing CRC's length is already baked into the header the CRC -// itself covers. +// kCrcLengthAdjustQuadlets. This function only ever *adds* one quadlet to +// whatever `info.acf_msg_length` already holds — it does not compute a base +// length itself — so a CRC-protected caller must call +// rcp::acf::compute_acf_msg_length(info.acf_msg_type, payload.size()) (or +// otherwise set `info.acf_msg_length` to the correct base value) FIRST, and +// only then call this function, before calling encode_acf_abb/encode_acf_gbb +// (whose own cpp-RCP-01 auto-fill only fires when acf_msg_length is still +// the 0 default — it never overwrites a nonzero value this function already +// adjusted). This two-step order is required so the trailing CRC's length +// is already baked into the header before that header is serialized both +// for the real wire frame and for coverage_buffer()'s own CRC-coverage +// header encode below. inline void apply_acf_length_adjustment(acf::AcfMessageInfo& info) noexcept { info.acf_msg_length = static_cast(info.acf_msg_length + kCrcLengthAdjustQuadlets); } diff --git a/include/rcp/gpio.hpp b/include/rcp/gpio.hpp index 2fbda0f..65015f4 100644 --- a/include/rcp/gpio.hpp +++ b/include/rcp/gpio.hpp @@ -119,7 +119,11 @@ inline std::vector encode_gpio_payload(PinMask mask) { } inline std::error_code decode_gpio_payload(const uint8_t* buf, size_t len, PinMask& out) noexcept { - if (len < kGpioPayloadLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); + // Spec §13.7.4: "A request not having exactly four bytes is rejected" — + // reject both too-short and too-long buffers, not just too-short + // (cpp-RCP-05-fresh; same bug class as PWM's decode_pwm_payload, + // cpp-RCP-03). + if (len != kGpioPayloadLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); out = avtp::detail::get_u32(buf); return {}; } diff --git a/include/rcp/iseled.hpp b/include/rcp/iseled.hpp index 4e47019..952a825 100644 --- a/include/rcp/iseled.hpp +++ b/include/rcp/iseled.hpp @@ -53,8 +53,10 @@ // rcp/avtp.hpp, rcp/regmap.hpp, rcp/endpoint.hpp, and rcp/e2e.hpp. #pragma once +#include #include +#include #include #include #include @@ -127,6 +129,73 @@ inline std::error_code validate_response(const IseledResponse& resp) noexcept { return {}; } +// ── ACF byte_msg_payload codec (Figure 40/41, §13.7.12.3; issue cpp-RCP-A4-iseled) ── +// Before this pass, IseledRequest/IseledResponse and IseledEndpoint::transact +// operated purely on in-memory structs with range validation — nothing in +// this file packed/unpacked the 12-bit address + 12-bit data fields into +// actual ACF wire payload bytes, so no real ISELED request/response could be +// built or parsed for the wire despite ep_type 0x0C being otherwise wired +// up. encode_iseled_request/decode_iseled_request and +// encode_iseled_response/decode_iseled_response below are that missing +// byte-level codec. + +// Figure 40: "Instruction | Address | Data1 | Data2 | Data3 | padding" — the +// 4-bit Instruction and 12-bit Address share the first two payload bytes +// (instruction in byte0's top nibble, address's top 4 bits in byte0's low +// nibble, address's low 8 bits in byte1 — the same nibble-then-byte split +// this specification uses throughout, e.g. rcp/acf.hpp's own +// evt/byte_bus_id fields), followed by `data` verbatim. Any trailing +// padding octets Figure 40 shows are this codec's caller's concern to add, +// same as every other endpoint payload in this codebase (see e.g. +// rcp/acf.hpp's own AcfMessageInfo::pad convention) — encode_iseled_request +// does not itself round `data` up to any particular length. +constexpr size_t kIseledRequestFixedLen = 2; // Instruction(4 bits) + Address(12 bits), before variable-length Data + +inline std::vector encode_iseled_request(const IseledRequest& req) { + std::vector buf(kIseledRequestFixedLen + req.data.size()); + buf[0] = static_cast(((req.instruction & kIseledInstructionMask) << 4) | + ((req.address >> 8) & 0x0F)); + buf[1] = static_cast(req.address & 0xFF); + std::copy(req.data.begin(), req.data.end(), buf.begin() + static_cast(kIseledRequestFixedLen)); + return buf; +} + +inline std::error_code decode_iseled_request(const uint8_t* buf, size_t len, IseledRequest& out) noexcept { + if (len < kIseledRequestFixedLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); + out.instruction = static_cast((buf[0] >> 4) & kIseledInstructionMask); + out.address = static_cast(((buf[0] & 0x0F) << 8) | buf[1]); + out.data.assign(buf + kIseledRequestFixedLen, buf + len); + return {}; +} + +// Figure 41: "Address | Data[11:0] | CRC (optional) | rsvd" — Address (12 +// bits) and Data[11:0] (12 bits) pack into 3 bytes (address's 8 high bits +// in byte0, address's low 4 bits + data's top 4 bits sharing byte1's two +// nibbles, data's low 8 bits in byte2). The optional 4-bit CRC + rsvd bits +// Figure 41 also shows are NOT encoded here: this repository has no +// verified copy of the ISELED standard's own CRC algorithm (see this +// header's own top-of-file comment on the removed invented CRC-8), so — +// same as the rest of this file — this codec only represents the +// Address/Data fields it can source confidently, not the CRC/rsvd trailer. +constexpr size_t kIseledResponseLen = 3; // Address(12 bits) + Data[11:0](12 bits); no CRC/rsvd + +inline std::vector encode_iseled_response(const IseledResponse& resp) { + std::vector buf(kIseledResponseLen); + const uint16_t addr = resp.address & kIseledFieldMask; + const uint16_t data = resp.data & kIseledFieldMask; + buf[0] = static_cast(addr >> 4); + buf[1] = static_cast(((addr & 0x0F) << 4) | ((data >> 8) & 0x0F)); + buf[2] = static_cast(data & 0xFF); + return buf; +} + +inline std::error_code decode_iseled_response(const uint8_t* buf, size_t len, IseledResponse& out) noexcept { + if (len != kIseledResponseLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); + out.address = static_cast((static_cast(buf[0]) << 4) | (buf[1] >> 4)); + out.data = static_cast(((buf[1] & 0x0F) << 8) | buf[2]); + return {}; +} + // ── Trigger signals ─────────────────────────────────────────────────────────── // One TransferComplete signal per IseledEndpoint instance, built on // rcp/endpoint.hpp's generic TriggerRegistry, same primitive diff --git a/include/rcp/mock.hpp b/include/rcp/mock.hpp index 3d9084d..415eafe 100644 --- a/include/rcp/mock.hpp +++ b/include/rcp/mock.hpp @@ -101,6 +101,29 @@ constexpr avtp::ByteBusId kSpiByteBusId = static_cast(kSpiEndp // magic number below — see this header's own scope note above. constexpr size_t kEp0PartialReadLen = sizeof(uint32_t); +// ── Error-response byte_msg_payload (extraction §12.9.6, Table 27; issue cpp-RCP-02) ── +// "The error response shall contain a byte_msg_payload with an error code." +// Every dispatch_*() error path below carries an internal std::error_code +// from whichever subsystem raised it (rcp/regmap.hpp's RegMapErrc, an +// rcp/avtp.hpp short_buffer from a payload-length check, ...); this +// translates that internal condition to the acf::WireErrorCode +// (Table 27) the wire error response payload must actually carry — the two +// are deliberately independent numbering spaces (see acf.hpp's own +// WireErrorCode comment), so this mapping is name-based, not a numeric +// cast. Falls back to UnsupportedCmd (Table 27's own value 1) for anything +// this mock does not otherwise recognize, a reasonably conservative default +// rather than silently picking an unrelated specific code. +inline acf::WireErrorCode wire_error_code_for(const std::error_code& ec) noexcept { + if (ec == make_error_code(regmap::RegMapErrc::unauthorized_access)) return acf::WireErrorCode::UnauthorizedAccess; + if (ec == make_error_code(regmap::RegMapErrc::locked_mem_access)) return acf::WireErrorCode::LockedMemAccess; + if (ec == make_error_code(regmap::RegMapErrc::request_rejected)) return acf::WireErrorCode::RequestRejected; + if (ec == make_error_code(regmap::RegMapErrc::invalid_parameter)) return acf::WireErrorCode::InvalidParameter; + if (ec == avtp::make_error_code(avtp::AvtpErrc::short_buffer)) return acf::WireErrorCode::InvalidParameter; // e.g. GPIO/SPI payload not exactly the required length + if (ec == make_error_code(gpio::GpioErrc::pin_index_out_of_range)) return acf::WireErrorCode::InvalidParameter; + if (ec == make_error_code(spi::SpiErrc::channel_out_of_range)) return acf::WireErrorCode::UnsupportedCmd; // reserved evt[2:0] selection, extraction §5.4/Table entries at 110b + return acf::WireErrorCode::UnsupportedCmd; +} + // ── Server ──────────────────────────────────────────────────────────────────── // A single simulated RC Server instance. Not copyable — regmap::Ep0 holds // references into this object's own RegisterMap/ServerLifecycle members, @@ -172,11 +195,23 @@ class Server final { if (req.byte_bus_id == kGpioByteBusId) return dispatch_gpio(req, req_payload, out_resp, out_resp_payload); if (req.byte_bus_id == kSpiByteBusId) return dispatch_spi(req, req_payload, out_resp, out_resp_payload); - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return make_error_code(regmap::RegMapErrc::invalid_parameter); + return set_error_response(req, make_error_code(regmap::RegMapErrc::invalid_parameter), + out_resp, out_resp_payload); } private: + // set_error_response is the single place every dispatch_*() error path + // below builds an ErrorResponse header AND its Table 27 error-code + // byte_msg_payload (cpp-RCP-02) — replacing the pre-fix pattern of + // building the header alone and leaving out_resp_payload empty. + static std::error_code set_error_response(const acf::AcfMessageInfo& req, std::error_code ec, + acf::AcfMessageInfo& out_resp, + std::vector& out_resp_payload) noexcept { + out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); + out_resp_payload = acf::encode_error_payload(wire_error_code_for(ec)); + return ec; + } + static regmap::RegisterMap make_initial_register_map() { regmap::RegisterMap regs; regs.endpoint_count = 2; @@ -196,14 +231,11 @@ class Server final { // Whole-map writes go through ep0().write_whole_map() at the // object level, not through this byte-oriented path — see this // header's own scope note. - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return make_error_code(regmap::RegMapErrc::request_rejected); + return set_error_response(req, make_error_code(regmap::RegMapErrc::request_rejected), + out_resp, out_resp_payload); } auto ec = ep0_.check_read_access(regmap::kEp0); - if (ec) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return ec; - } + if (ec) return set_error_response(req, ec, out_resp, out_resp_payload); out_resp_payload.resize(kEp0PartialReadLen); avtp::detail::put_u32(out_resp_payload.data(), regs_.magic); out_resp = acf::make_response(req, acf::ResponseKind::ReadResponse); @@ -218,8 +250,8 @@ class Server final { acf::AcfMessageInfo& out_resp, std::vector& out_resp_payload) noexcept { if (!operational_requests_allowed()) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return make_error_code(regmap::RegMapErrc::request_rejected); + return set_error_response(req, make_error_code(regmap::RegMapErrc::request_rejected), + out_resp, out_resp_payload); } if (!req.op) { out_resp_payload = gpio::encode_gpio_payload(gpio_.read()); @@ -228,16 +260,11 @@ class Server final { } gpio::PinMask operand = 0; auto ec = gpio::decode_gpio_payload(payload.data(), payload.size(), operand); - if (ec) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return ec; - } + if (ec) return set_error_response(req, ec, out_resp, out_resp_payload); + gpio::PinMask new_value = 0; ec = gpio_.handle_write(endpoint::write_semantics_of(req.evt_op), operand, new_value); - if (ec) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return ec; - } + if (ec) return set_error_response(req, ec, out_resp, out_resp_payload); out_resp_payload = gpio::encode_gpio_payload(new_value); out_resp = acf::make_response(req, req.evt_ack ? acf::ResponseKind::Acknowledge : acf::ResponseKind::WriteResponse); @@ -254,20 +281,15 @@ class Server final { acf::AcfMessageInfo& out_resp, std::vector& out_resp_payload) noexcept { if (!operational_requests_allowed()) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return make_error_code(regmap::RegMapErrc::request_rejected); + return set_error_response(req, make_error_code(regmap::RegMapErrc::request_rejected), + out_resp, out_resp_payload); } uint8_t channel = 0; auto ec = spi::channel_of(req.evt_op, channel); - if (ec) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return ec; - } + if (ec) return set_error_response(req, ec, out_resp, out_resp_payload); + ec = spi_.transfer(channel, payload, spi_poci_[channel]); - if (ec) { - out_resp = acf::make_response(req, acf::ResponseKind::ErrorResponse); - return ec; - } + if (ec) return set_error_response(req, ec, out_resp, out_resp_payload); out_resp_payload = spi_.last_received(channel); out_resp = acf::make_response(req, acf::ResponseKind::ReadResponse); return {}; diff --git a/include/rcp/pwm.hpp b/include/rcp/pwm.hpp index f1a0c2f..a60c816 100644 --- a/include/rcp/pwm.hpp +++ b/include/rcp/pwm.hpp @@ -104,7 +104,10 @@ inline PwmWireBytes encode_pwm_payload(const PwmValue& value) noexcept { } inline std::error_code decode_pwm_payload(const uint8_t* buf, size_t len, PwmValue& out) noexcept { - if (len < kPwmPayloadLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); + // Spec §13.7.5.3: "A request not having exactly four bytes is rejected" — + // reject both too-short and too-long buffers, not just too-short + // (cpp-RCP-03). + if (len != kPwmPayloadLen) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); out.period = avtp::detail::get_u16(&buf[0]); out.active_duration = avtp::detail::get_u16(&buf[2]); return {}; @@ -161,11 +164,17 @@ class PwmOutEndpoint { // ── PwmInEndpoint (ep_type 0x08) ───────────────────────────────────────────── // Response-only read model (extraction §5.6): PWM_IN has no write request -// shape at all in this milestone's scope. record_measurement models one -// input-capture cycle completing and firing the MidPulse trigger signal -// rcp/adc.hpp's AdcCadence::ExternalTrigger pattern is expected to key -// sampling cadence off of. -enum class PwmInSignal : uint8_t { MidPulse = 0 }; +// shape at all in this milestone's scope. Trigger signals fixed to match +// Table 44 ("pwmi trigger outputs") exactly (issue cpp-RCP-A4-pwmin): the +// spec defines two independent trigger outputs — rising edge (0) and +// falling edge (1) of the measured PWM_IN signal — not the single invented +// "MidPulse" signal this header modeled before. record_edge fires whichever +// one edge actually occurred; record_measurement (kept for +// rcp/adc.hpp's AdcCadence::ExternalTrigger pattern and existing callers) +// models one full input-capture cycle completing — which inherently spans +// both a rising and a falling edge of the measured signal — by recording +// the new value once and then firing both signals. +enum class PwmInSignal : uint8_t { RisingEdge = 0, FallingEdge = 1 }; constexpr endpoint::TriggerRegistry::SignalId pwm_in_signal_id(PwmInSignal sig) noexcept { return static_cast(sig); @@ -173,12 +182,23 @@ constexpr endpoint::TriggerRegistry::SignalId pwm_in_signal_id(PwmInSignal sig) class PwmInEndpoint { public: - // record_measurement records one completed input-capture cycle and - // fires MidPulse for any armed listener. + // record_edge fires exactly one of Table 44's two trigger signals for + // an armed listener, without updating the last-measured value — for a + // caller that observes rising/falling edges independently rather than + // only at whole-cycle granularity. + void record_edge(PwmInSignal edge) noexcept { + triggers_.notify(pwm_in_signal_id(edge)); + } + + // record_measurement records one completed input-capture cycle (a full + // period, spanning one rising and one falling edge of the measured + // signal) and fires both Table 44 trigger signals for any armed + // listener. void record_measurement(PwmValue value) noexcept { last_value_ = value; has_signal_ = true; - triggers_.notify(pwm_in_signal_id(PwmInSignal::MidPulse)); + triggers_.notify(pwm_in_signal_id(PwmInSignal::RisingEdge)); + triggers_.notify(pwm_in_signal_id(PwmInSignal::FallingEdge)); } // clear_signal models signal loss (e.g. the measured line goes idle diff --git a/include/rcp/udp.hpp b/include/rcp/udp.hpp index dbd86f5..e511b50 100644 --- a/include/rcp/udp.hpp +++ b/include/rcp/udp.hpp @@ -150,6 +150,13 @@ inline std::error_code decode_frame(const uint8_t* b, size_t len, Frame& out) { out.timestamp_valid = hdr.timestamp_valid; out.avtp_timestamp = hdr.avtp_timestamp; acf_off = avtp::kTscfHeaderLen; + // control_data_length integrity check (issue cpp-RCP-A3): the + // header's own declared ACF byte length must match what's actually + // left in the buffer, or a receiver trusting it for anything else + // (e.g. finding where a *next* AVTPDU starts in a larger buffer) + // would be silently misled by a corrupt/spoofed length field. + if (static_cast(hdr.control_data_length) != len - acf_off) + return avtp::make_error_code(avtp::AvtpErrc::length_mismatch); } else { avtp::NtscfHeader hdr; if (auto ec = avtp::decode_ntscf_header(b, len, hdr)) return ec; @@ -158,6 +165,8 @@ inline std::error_code decode_frame(const uint8_t* b, size_t len, Frame& out) { out.timestamp_valid = false; out.avtp_timestamp = 0; acf_off = avtp::kNtscfHeaderLen; + if (static_cast(hdr.control_data_length) != len - acf_off) + return avtp::make_error_code(avtp::AvtpErrc::length_mismatch); } if (len < acf_off + 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); @@ -174,15 +183,105 @@ inline std::error_code decode_frame(const uint8_t* b, size_t len, Frame& out) { return acf::decode_acf_abb(b + acf_off, len - acf_off, out.info, out.payload); } +// ── MultiFrame — multiple ACF requests in one AVTPDU (extraction §12.9.1.1; issue cpp-RCP-04-fresh) ── +// "An RC Server shall support to handle multiple requests in one frame and +// check each of them individually if to be processed or not ... The RC +// Server shall support the handling of multiple request types in one +// frame." Frame/encode_frame/decode_frame above model exactly one ACF_ABB/ +// ACF_GBB message per AVTPDU — correct for the common case, but unable to +// represent more than one request (or response) sharing a single NTSCF/TSCF +// header. MultiFrame is the same AVTPDU envelope carrying one-or-more +// acf::AcfEntry messages (rcp/acf.hpp's decode_acf_messages, cpp-RCP-01's +// acf_msg_length fix above); Server and Client below are built on this, not +// on Frame, so a single incoming request continues to behave exactly as it +// did through Frame (MultiFrame::messages.size() == 1 is the common case), +// while a sender that packs several requests into one datagram is now +// actually handled per-request rather than having its later requests +// silently swallowed into the first one's payload or dropped. +struct MultiFrame { + bool use_tscf = false; // false = NTSCF, true = TSCF + avtp::StreamId stream_id{}; + uint16_t sequence_num = 0; + bool timestamp_valid = false; // TSCF "tv" bit; ignored under NTSCF + uint32_t avtp_timestamp = 0; // TSCF-only; ignored under NTSCF + + std::vector messages; // one or more ACF_ABB/ACF_GBB messages, in wire order +}; + +inline std::vector encode_multi_frame(const MultiFrame& f) { + std::vector acf_bytes; + for (const auto& m : f.messages) { + auto enc = (m.info.acf_msg_type == acf::kAcfMsgTypeGbb) + ? acf::encode_acf_gbb(m.info, m.message_timestamp, m.payload) + : acf::encode_acf_abb(m.info, m.payload); + acf_bytes.insert(acf_bytes.end(), enc.begin(), enc.end()); + } + + std::vector out; + if (f.use_tscf) { + avtp::TscfHeader hdr; + hdr.stream_id = f.stream_id; + hdr.sequence_num = f.sequence_num; + hdr.control_data_length = static_cast(acf_bytes.size()); + hdr.timestamp_valid = f.timestamp_valid; + hdr.avtp_timestamp = f.avtp_timestamp; + out = avtp::encode_tscf_header(hdr); + } else { + avtp::NtscfHeader hdr; + hdr.stream_id = f.stream_id; + hdr.sequence_num = f.sequence_num; + hdr.control_data_length = static_cast(acf_bytes.size()); + out = avtp::encode_ntscf_header(hdr); + } + out.insert(out.end(), acf_bytes.begin(), acf_bytes.end()); + return out; +} + +inline std::error_code decode_multi_frame(const uint8_t* b, size_t len, MultiFrame& out) { + if (len < 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); + + size_t acf_off; + out.use_tscf = (b[0] == avtp::kSubtypeTscf); + if (out.use_tscf) { + avtp::TscfHeader hdr; + if (auto ec = avtp::decode_tscf_header(b, len, hdr)) return ec; + out.stream_id = hdr.stream_id; + out.sequence_num = hdr.sequence_num; + out.timestamp_valid = hdr.timestamp_valid; + out.avtp_timestamp = hdr.avtp_timestamp; + acf_off = avtp::kTscfHeaderLen; + // control_data_length integrity check (issue cpp-RCP-A3) — see + // decode_frame's equivalent comment above. + if (static_cast(hdr.control_data_length) != len - acf_off) + return avtp::make_error_code(avtp::AvtpErrc::length_mismatch); + } else { + avtp::NtscfHeader hdr; + if (auto ec = avtp::decode_ntscf_header(b, len, hdr)) return ec; + out.stream_id = hdr.stream_id; + out.sequence_num = hdr.sequence_num; + out.timestamp_valid = false; + out.avtp_timestamp = 0; + acf_off = avtp::kNtscfHeaderLen; + if (static_cast(hdr.control_data_length) != len - acf_off) + return avtp::make_error_code(avtp::AvtpErrc::length_mismatch); + } + + if (len < acf_off + 1) return avtp::make_error_code(avtp::AvtpErrc::short_buffer); + return acf::decode_acf_messages(b + acf_off, len - acf_off, out.messages); +} + // ── Server ──────────────────────────────────────────────────────────────────── -// Server binds a UDP socket, decodes each inbound datagram as a Frame, and -// dispatches the carried ACF request to a caller-supplied Handler — shaped -// to match rcp::mock::Server::dispatch's signature — then encodes and sends -// the handler's response back to the sender under the same header kind -// (NTSCF/TSCF) the request arrived under. Malformed datagrams (short buffer, -// bad subtype, unrecognized ACF message type) are dropped silently, the same -// "drop rather than partially process" choice rcp/discovery.hpp documents -// for its own decode path. +// Server binds a UDP socket, decodes each inbound datagram as a MultiFrame, +// and dispatches every ACF request the datagram carries — one, the common +// case, or more than one packed back to back (extraction §12.9.1.1, issue +// cpp-RCP-04-fresh) — to a caller-supplied Handler individually, shaped to +// match rcp::mock::Server::dispatch's signature, then encodes every +// response into a single reply MultiFrame (same order as the requests +// arrived in) and sends it back to the sender under the same header kind +// (NTSCF/TSCF) the request arrived under. Malformed datagrams (short +// buffer, bad subtype, unrecognized ACF message type on the very first +// message) are dropped silently, the same "drop rather than partially +// process" choice rcp/discovery.hpp documents for its own decode path. class Server { public: using Handler = std::function(&from), &flen); if (n <= 0) break; - Frame req; - if (decode_frame(buf.data(), static_cast(n), req)) continue; + MultiFrame req; + if (decode_multi_frame(buf.data(), static_cast(n), req)) continue; - acf::AcfMessageInfo out_info; - std::vector out_payload; - size_t client; - { - std::lock_guard lk(mu_); - client = client_id_for(from); - if (handler_) { - auto ec = handler_(client, req.info, req.payload, out_info, out_payload); - (void)ec; // Handler always populates out_info even on failure, - // same contract rcp::mock::Server::dispatch documents. - } else { - out_info = acf::make_response(req.info, acf::ResponseKind::Acknowledge); - } - } - - Frame resp; + MultiFrame resp; resp.use_tscf = req.use_tscf; resp.stream_id = stream_id_; resp.sequence_num = static_cast(++seq_); resp.timestamp_valid = req.timestamp_valid; resp.avtp_timestamp = req.avtp_timestamp; - resp.info = out_info; - resp.payload = std::move(out_payload); + resp.messages.reserve(req.messages.size()); + { + std::lock_guard lk(mu_); + size_t client = client_id_for(from); + // §12.9.1.1: "check each of them individually if to be + // processed or not" — each request in the datagram is + // dispatched and answered on its own, not as a batch. + for (const auto& m : req.messages) { + acf::AcfEntry out_entry; + if (handler_) { + auto ec = handler_(client, m.info, m.payload, out_entry.info, out_entry.payload); + (void)ec; // Handler always populates out_entry.info even on + // failure, same contract rcp::mock::Server::dispatch + // documents. + } else { + out_entry.info = acf::make_response(m.info, acf::ResponseKind::Acknowledge); + } + resp.messages.push_back(std::move(out_entry)); + } + } - auto out_frame = encode_frame(resp); + auto out_frame = encode_multi_frame(resp); ::sendto(fd_, out_frame.data(), out_frame.size(), 0, reinterpret_cast(&from), flen); } @@ -332,6 +434,16 @@ class Server { // via acf::make_standard_request or rcp/discovery.hpp's // make_discovery_request) so this transport stays a pure carrier, not a // second place request semantics are decided. +// +// Client::request() always sends exactly one ACF request per call, via the +// single-message Frame codec above — so a Server it talks to always sees +// exactly one message in that datagram, and always answers with exactly one +// message bundled in its MultiFrame reply (see Server::serve() above), +// which decode_frame below decodes correctly. §12.9.1.1's "shall support +// multiple requests in one frame" is a requirement on the RC *Server* side +// (which Server above now implements via MultiFrame/decode_acf_messages, +// cpp-RCP-04-fresh) — this Client intentionally does not add a batching API +// of its own client-side in this pass. class Client { public: Client(avtp::StreamId stream_id, const char* server_host, uint16_t server_port) diff --git a/include/rcp/version.hpp b/include/rcp/version.hpp index 6f76ce0..fd5d19d 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.19.0"; +constexpr std::string_view kVersion = "2.20.0"; } // namespace rcp diff --git a/tests/test_acf.cpp b/tests/test_acf.cpp index d5cb67f..618232c 100644 --- a/tests/test_acf.cpp +++ b/tests/test_acf.cpp @@ -304,6 +304,84 @@ TEST_CASE("Decoders reject buffers shorter than their fixed header", "[acf][REQ- REQUIRE(decode_acf_abb(too_short_acf.data(), too_short_acf.size(), abb_out, abb_payload)); } +// ── Multiple ACF requests in one frame (extraction §12.9.1.1; issue cpp-RCP-04-fresh) ── + +TEST_CASE("decode_acf_messages decodes two ACF_ABB requests packed back to back", + "[acf][REQ-WIRE-004]") { + AcfMessageInfo first; + first.byte_bus_id = 1; + first.transaction_num = 10; + std::vector first_payload(4, 0xAA); // quadlet-aligned so acf_msg_length is byte-exact + + AcfMessageInfo second; + second.byte_bus_id = 2; + second.transaction_num = 20; + std::vector second_payload(4, 0xBB); + + auto first_bytes = encode_acf_abb(first, first_payload); + auto second_bytes = encode_acf_abb(second, second_payload); + std::vector buf = first_bytes; + buf.insert(buf.end(), second_bytes.begin(), second_bytes.end()); + + std::vector out; + REQUIRE_FALSE(decode_acf_messages(buf.data(), buf.size(), out)); + REQUIRE(out.size() == 2); + REQUIRE(out[0].info.byte_bus_id == 1); + REQUIRE(out[0].info.transaction_num == 10); + REQUIRE(out[0].payload == first_payload); + REQUIRE(out[1].info.byte_bus_id == 2); + REQUIRE(out[1].info.transaction_num == 20); + REQUIRE(out[1].payload == second_payload); +} + +TEST_CASE("decode_acf_messages decodes a single ACF_ABB message occupying the whole buffer", + "[acf][REQ-WIRE-004]") { + AcfMessageInfo info; + info.byte_bus_id = 5; + std::vector payload{0x01, 0x02, 0x03}; // not quadlet-aligned + auto buf = encode_acf_abb(info, payload); + + std::vector out; + REQUIRE_FALSE(decode_acf_messages(buf.data(), buf.size(), out)); + REQUIRE(out.size() == 1); + REQUIRE(out[0].payload == payload); +} + +TEST_CASE("decode_acf_messages handles a mix of ACF_ABB and ACF_GBB in one buffer", + "[acf][REQ-WIRE-004][REQ-WIRE-005]") { + AcfMessageInfo abb; + abb.byte_bus_id = 1; + std::vector abb_payload(4, 0x11); + + AcfMessageInfo gbb; + gbb.byte_bus_id = 2; + gbb.mtv = true; + std::vector gbb_payload(4, 0x22); + uint64_t ts = 0xAABBCCDDEEFF0011ULL; + + auto abb_bytes = encode_acf_abb(abb, abb_payload); + auto gbb_bytes = encode_acf_gbb(gbb, ts, gbb_payload); + std::vector buf = abb_bytes; + buf.insert(buf.end(), gbb_bytes.begin(), gbb_bytes.end()); + + std::vector out; + REQUIRE_FALSE(decode_acf_messages(buf.data(), buf.size(), out)); + REQUIRE(out.size() == 2); + REQUIRE(out[0].info.acf_msg_type == kAcfMsgTypeAbb); + REQUIRE(out[0].payload == abb_payload); + REQUIRE(out[1].info.acf_msg_type == kAcfMsgTypeGbb); + REQUIRE(out[1].message_timestamp == ts); + REQUIRE(out[1].payload == gbb_payload); +} + +TEST_CASE("decode_acf_messages rejects an empty/unrecognized-type buffer outright", "[acf][REQ-WIRE-004]") { + std::vector bogus(kAcfCommonHeaderLen, 0x00); // acf_msg_type bits decode to neither ABB nor GBB + std::vector out; + auto ec = decode_acf_messages(bogus.data(), bogus.size(), out); + REQUIRE(ec); + REQUIRE(out.empty()); +} + // ── byte_bus_id echo rule ─────────────────────────────────────────────────────── TEST_CASE("make_response echoes byte_bus_id and transaction_num from the request unchanged", "[acf][REQ-WIRE-008]") { diff --git a/tests/test_e2e.cpp b/tests/test_e2e.cpp index 52b5723..c109a4b 100644 --- a/tests/test_e2e.cpp +++ b/tests/test_e2e.cpp @@ -190,6 +190,82 @@ TEST_CASE("apply_frame_length_adjustment adds exactly four octets to NTSCF and T REQUIRE(kCrcLengthAdjustOctets == 4); } +// ── acf_msg_length worked-example pins (issue cpp-RCP-01) ──────────────────── +// These two cases reproduce the specification's own two fully worked +// numeric examples end to end (compute_acf_msg_length -> +// apply_acf_length_adjustment -> encode_acf_abb/encode_acf_gbb -> +// append_crc), confirming the final wire acf_msg_length equals the +// specification's own stated value in each case, not just that some +// nonzero value comes out. +TEST_CASE("acf_msg_length end-to-end matches the specification's own worked examples " + "(ACF_ABB -> 0x05, ACF_GBB -> 0x07)", + "[e2e][acf][REQ-E2E-003][REQ-WIRE-004][REQ-WIRE-005]") { + using rcp::acf::compute_acf_msg_length; + using rcp::acf::encode_acf_abb; + using rcp::acf::encode_acf_gbb; + using rcp::acf::kAcfMsgTypeAbb; + using rcp::acf::kAcfMsgTypeGbb; + using rcp::acf::kAcfCommonHeaderLen; + using rcp::acf::kAcfGbbTimestampLen; + + // ACF_ABB: 6 real payload bytes + 2 pad bytes (caller-padded, per this + // codec's own AcfMessageInfo::pad convention — see acf.hpp's "ACF + // shared header" section) = 8 payload bytes, plus a trailing 4-byte + // CRC32 trailer. header(2 quadlets) + payload(2 quadlets) + CRC(1 + // quadlet) = 5 quadlets = 0x05, header + payload = 20 bytes total. + { + AcfMessageInfo info; + info.pad = 2; + std::vector payload(8, 0); // 6 "real" bytes + 2 pad bytes + + info.acf_msg_length = compute_acf_msg_length(kAcfMsgTypeAbb, payload.size()); + REQUIRE(info.acf_msg_length == 4); // base, before the CRC trailer is counted + apply_acf_length_adjustment(info); + REQUIRE(info.acf_msg_length == 5); // 0x05 + + auto frame = encode_acf_abb(info, payload); + REQUIRE(frame.size() == kAcfCommonHeaderLen + payload.size()); // 8 + 8 = 16 bytes + + auto sid = make_stream_id(0x02, 1); + uint32_t crc = compute_crc(sid, std::nullopt, info, std::nullopt, payload); + append_crc(frame, crc); + REQUIRE(frame.size() == 20); // 8 header + 8 payload + 4 CRC = 20 bytes = 5 quadlets + + // The wire header actually carries 5: byte0 bit0 is acf_msg_length's + // MSB (0, since 5 < 256) and byte1 is its low 8 bits. + REQUIRE((frame[0] & 0x01) == 0); + REQUIRE(frame[1] == 5); + } + + // ACF_GBB: 7 real payload bytes + 1 pad byte = 8 payload bytes, plus the + // mandatory 8-byte message_timestamp and a trailing 4-byte CRC32. + // header(2) + timestamp(2) + payload(2) + CRC(1) = 7 quadlets = 0x07, + // 28 bytes total. + { + AcfMessageInfo info; + info.pad = 1; + info.mtv = true; + std::vector payload(8, 0); // 7 "real" bytes + 1 pad byte + const uint64_t ts = 0x1122334455667788ULL; + + info.acf_msg_length = compute_acf_msg_length(kAcfMsgTypeGbb, payload.size()); + REQUIRE(info.acf_msg_length == 6); // base, before the CRC trailer is counted + apply_acf_length_adjustment(info); + 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 + + auto sid = make_stream_id(0x03, 1); + uint32_t crc = compute_crc(sid, std::nullopt, info, ts, payload); + append_crc(frame, crc); + REQUIRE(frame.size() == 28); // 24 + 4 CRC = 28 bytes = 7 quadlets + + REQUIRE((frame[0] & 0x01) == 0); + REQUIRE(frame[1] == 7); + } +} + // ── verify_crc / append_crc ──────────────────────────────────────────────────── TEST_CASE("verify_crc accepts a matching CRC and rejects a corrupted one", "[e2e][REQ-E2E-004]") { diff --git a/tests/test_gpio.cpp b/tests/test_gpio.cpp index 93587d2..28f792f 100644 --- a/tests/test_gpio.cpp +++ b/tests/test_gpio.cpp @@ -40,6 +40,16 @@ TEST_CASE("decode_gpio_payload rejects a short buffer", "[gpio][REQ-GPIO-001]") REQUIRE(ec == rcp::avtp::make_error_code(rcp::avtp::AvtpErrc::short_buffer)); } +TEST_CASE("decode_gpio_payload rejects an over-long buffer (spec requires exactly 4 bytes)", + "[gpio][REQ-GPIO-001]") { + // §13.7.4: "A request not having exactly four bytes is rejected" — + // cpp-RCP-05-fresh. Trailing bytes must not be silently ignored. + std::vector long_buf{0x01, 0x02, 0x03, 0x04, 0x05}; + PinMask out = 0; + auto ec = decode_gpio_payload(long_buf.data(), long_buf.size(), out); + REQUIRE(ec == rcp::avtp::make_error_code(rcp::avtp::AvtpErrc::short_buffer)); +} + // ── Write semantics: the 6 generic combinators ─────────────────────────────── TEST_CASE("apply_gpio_write applies Replace/Or/And/Xor to state.values", "[gpio][REQ-GPIO-002]") { diff --git a/tests/test_iseled.cpp b/tests/test_iseled.cpp index aee6776..671a685 100644 --- a/tests/test_iseled.cpp +++ b/tests/test_iseled.cpp @@ -109,6 +109,81 @@ TEST_CASE("IseledEndpoint::transact rejects an out-of-range field without record REQUIRE_FALSE(ep.triggers().has_pending()); } +// ── ACF byte_msg_payload codec (Figure 40/41; issue cpp-RCP-A4-iseled) ─────── + +TEST_CASE("encode_iseled_request / decode_iseled_request round-trip Instruction/Address/Data", + "[iseled][REQ-ISELED-002]") { + IseledRequest req; + req.instruction = 0x0A; // 4 bits + req.address = 0x0ABC; // 12 bits + req.data = {0xDE, 0xAD, 0xBE}; + + auto buf = encode_iseled_request(req); + REQUIRE(buf.size() == kIseledRequestFixedLen + req.data.size()); + + IseledRequest out; + REQUIRE_FALSE(decode_iseled_request(buf.data(), buf.size(), out)); + REQUIRE(out.instruction == req.instruction); + REQUIRE(out.address == req.address); + REQUIRE(out.data == req.data); +} + +TEST_CASE("encode_iseled_request hand-computed expected byte sequence", "[iseled][REQ-ISELED-002]") { + IseledRequest req; + req.instruction = 0x3; // 0b0011 + req.address = 0x0102; // 0b0001_0000_0010 + req.data = {0x11, 0x22}; + + // byte0 = (instruction[3:0] << 4) | address[11:8] = (0x3 << 4) | 0x1 = 0x31 + // byte1 = address[7:0] = 0x02 + const std::vector expected{0x31, 0x02, 0x11, 0x22}; + REQUIRE(encode_iseled_request(req) == expected); +} + +TEST_CASE("decode_iseled_request rejects a buffer shorter than Instruction+Address", + "[iseled][REQ-ISELED-002]") { + uint8_t short_buf[1] = {0x00}; + IseledRequest out; + auto ec = decode_iseled_request(short_buf, sizeof(short_buf), out); + REQUIRE(ec == rcp::avtp::make_error_code(rcp::avtp::AvtpErrc::short_buffer)); +} + +TEST_CASE("encode_iseled_response / decode_iseled_response round-trip Address/Data[11:0]", + "[iseled][REQ-ISELED-002]") { + IseledResponse resp; + resp.address = kIseledFieldMask; // max 12-bit value + resp.data = 0x0ABC; + + auto buf = encode_iseled_response(resp); + REQUIRE(buf.size() == kIseledResponseLen); + + IseledResponse out; + REQUIRE_FALSE(decode_iseled_response(buf.data(), buf.size(), out)); + REQUIRE(out.address == resp.address); + REQUIRE(out.data == resp.data); +} + +TEST_CASE("encode_iseled_response hand-computed expected byte sequence", "[iseled][REQ-ISELED-002]") { + IseledResponse resp; + resp.address = 0x0102; // 0b0001_0000_0010 + resp.data = 0x0BEF; // 0b1011_1110_1111 + + // byte0 = address[11:4] = 0b0001_0000 = 0x10 + // byte1 = (address[3:0] << 4) | data[11:8] = (0x2 << 4) | 0xB = 0x2B + // byte2 = data[7:0] = 0xEF + const std::vector expected{0x10, 0x2B, 0xEF}; + REQUIRE(encode_iseled_response(resp) == expected); +} + +TEST_CASE("decode_iseled_response rejects a buffer not exactly 3 bytes", "[iseled][REQ-ISELED-002]") { + uint8_t short_buf[2] = {0x00, 0x00}; + IseledResponse out; + REQUIRE(decode_iseled_response(short_buf, sizeof(short_buf), out)); + + uint8_t long_buf[4] = {0x00, 0x00, 0x00, 0x00}; + REQUIRE(decode_iseled_response(long_buf, sizeof(long_buf), out)); +} + // ── Trigger signals ─────────────────────────────────────────────────────────── TEST_CASE("IseledEndpoint::transact fires TransferComplete on a valid transaction", diff --git a/tests/test_mock.cpp b/tests/test_mock.cpp index cb9c981..5c8e71f 100644 --- a/tests/test_mock.cpp +++ b/tests/test_mock.cpp @@ -82,6 +82,9 @@ TEST_CASE("EP0 write via dispatch() is always rejected", "[mock][REQ-MOCK-004]") auto ec = server.dispatch(/*client=*/1, req, {}, resp, resp_payload); REQUIRE(ec == regmap::make_error_code(regmap::RegMapErrc::request_rejected)); REQUIRE(acf::response_kind_of(resp) == acf::ResponseKind::ErrorResponse); + // cpp-RCP-02: the error response's byte_msg_payload must carry Table 27's + // numeric error code (RequestRejected = 11), not be left empty. + REQUIRE(resp_payload == std::vector{static_cast(acf::WireErrorCode::RequestRejected)}); } TEST_CASE("write_whole_map requires the root client", "[mock][REQ-MOCK-005]") { @@ -189,4 +192,24 @@ TEST_CASE("dispatch to an unmapped byte_bus_id reports invalid_parameter", auto ec = server.dispatch(0, req, {}, resp, resp_payload); REQUIRE(ec == regmap::make_error_code(regmap::RegMapErrc::invalid_parameter)); REQUIRE(acf::response_kind_of(resp) == acf::ResponseKind::ErrorResponse); + REQUIRE(resp_payload == std::vector{static_cast(acf::WireErrorCode::InvalidParameter)}); +} + +TEST_CASE("A GPIO write with a mis-sized payload is rejected with wire error code INVALID_PARAMETER", + "[mock][REQ-MOCK-010][REQ-GPIO-001]") { + // cpp-RCP-02 + cpp-RCP-05-fresh together: decode_gpio_payload now + // rejects a non-exactly-4-byte buffer, and that rejection's wire + // error-response payload must carry Table 27's INVALID_PARAMETER (15). + mock::Server server; + REQUIRE_FALSE(server.advance_to_rcp_configured()); + + auto req = standard_request(mock::kGpioByteBusId, /*write=*/true, + static_cast(WriteSemantics::Or)); + std::vector too_long_payload{0x00, 0x00, 0x00, 0x0F, 0xFF}; + acf::AcfMessageInfo resp; + std::vector resp_payload; + auto ec = server.dispatch(0, req, too_long_payload, resp, resp_payload); + REQUIRE(ec); + REQUIRE(acf::response_kind_of(resp) == acf::ResponseKind::ErrorResponse); + REQUIRE(resp_payload == std::vector{static_cast(acf::WireErrorCode::InvalidParameter)}); } diff --git a/tests/test_pwm.cpp b/tests/test_pwm.cpp index d8a23a9..12c4e69 100644 --- a/tests/test_pwm.cpp +++ b/tests/test_pwm.cpp @@ -66,6 +66,16 @@ TEST_CASE("decode_pwm_payload reports short_buffer for fewer than 4 bytes", "[pw REQUIRE(ec == rcp::avtp::make_error_code(rcp::avtp::AvtpErrc::short_buffer)); } +TEST_CASE("decode_pwm_payload rejects an over-long buffer (spec requires exactly 4 bytes)", + "[pwm][REQ-PWM-001]") { + // §13.7.5.3: "A request not having exactly four bytes is rejected" — + // cpp-RCP-03. Trailing bytes must not be silently ignored. + uint8_t long_buf[5] = {0, 0, 0, 0, 0}; + PwmValue out; + auto ec = decode_pwm_payload(long_buf, sizeof(long_buf), out); + REQUIRE(ec == rcp::avtp::make_error_code(rcp::avtp::AvtpErrc::short_buffer)); +} + // ── PWM_OUT: Replace applies directly, every other semantics is rejected ──── // The spec's PWM_OUT request-handling section describes only direct // application of the request's period/active values — no GPIO-style @@ -164,26 +174,45 @@ TEST_CASE("PwmInEndpoint::clear_signal re-arms PWM_IN_NO_SIGNAL", "[pwm][REQ-PWM REQUIRE(ec == make_error_code(PwmErrc::no_signal)); } -// ── Mid-pulse trigger signal (keys ADC sampling cadence) ───────────────────── +// ── Rising/falling-edge trigger signals (Table 44; issue cpp-RCP-A4-pwmin) ─── -TEST_CASE("PwmInEndpoint::record_measurement fires MidPulse when armed", "[pwm][REQ-PWM-006]") { +TEST_CASE("PwmInEndpoint::record_measurement fires both RisingEdge and FallingEdge when armed", + "[pwm][REQ-PWM-006]") { PwmInEndpoint ep; - ep.triggers().enable(pwm_in_signal_id(PwmInSignal::MidPulse)); + ep.triggers().enable(pwm_in_signal_id(PwmInSignal::RisingEdge)); + ep.triggers().enable(pwm_in_signal_id(PwmInSignal::FallingEdge)); ep.record_measurement({100, 50}); auto drained = ep.triggers().drain(); - REQUIRE(drained.size() == 1); - REQUIRE(drained[0] == pwm_in_signal_id(PwmInSignal::MidPulse)); + REQUIRE(drained.size() == 2); + REQUIRE(drained[0] == pwm_in_signal_id(PwmInSignal::RisingEdge)); + REQUIRE(drained[1] == pwm_in_signal_id(PwmInSignal::FallingEdge)); } -TEST_CASE("PwmInEndpoint::record_measurement fires nothing when MidPulse is not armed", +TEST_CASE("PwmInEndpoint::record_measurement fires nothing when neither edge is armed", "[pwm][REQ-PWM-006]") { PwmInEndpoint ep; ep.record_measurement({100, 50}); REQUIRE_FALSE(ep.triggers().has_pending()); } +TEST_CASE("PwmInEndpoint::record_edge fires exactly the requested edge", "[pwm][REQ-PWM-006]") { + PwmInEndpoint ep; + ep.triggers().enable(pwm_in_signal_id(PwmInSignal::RisingEdge)); + ep.triggers().enable(pwm_in_signal_id(PwmInSignal::FallingEdge)); + + ep.record_edge(PwmInSignal::RisingEdge); + auto drained = ep.triggers().drain(); + REQUIRE(drained.size() == 1); + REQUIRE(drained[0] == pwm_in_signal_id(PwmInSignal::RisingEdge)); + + ep.record_edge(PwmInSignal::FallingEdge); + drained = ep.triggers().drain(); + REQUIRE(drained.size() == 1); + REQUIRE(drained[0] == pwm_in_signal_id(PwmInSignal::FallingEdge)); +} + // ── PwmErrc category sanity ─────────────────────────────────────────────────── TEST_CASE("PwmErrc reports a non-empty message in its own category", "[pwm][REQ-PWM-007]") { diff --git a/tests/test_udp.cpp b/tests/test_udp.cpp index 8e4931d..092e2c6 100644 --- a/tests/test_udp.cpp +++ b/tests/test_udp.cpp @@ -174,6 +174,57 @@ TEST_CASE("decode_frame rejects an unrecognized ACF message type", "[udp][REQ-UD REQUIRE(decode_frame(bytes.data(), bytes.size(), out)); } +TEST_CASE("decode_frame rejects a buffer whose control_data_length disagrees with its actual size", + "[udp][REQ-UDP-006]") { + // cpp-RCP-A3: control_data_length was decoded but never checked against + // the buffer it actually came with. + Frame f; + f.stream_id = make_stream_id(0x01, 2); + f.info = standard_request(1, 1); + f.payload = {0x01, 0x02, 0x03}; + auto bytes = encode_frame(f); + bytes.push_back(0xFF); // one extra byte the NTSCF header's own length field does not account for + + Frame out; + auto ec = decode_frame(bytes.data(), bytes.size(), out); + REQUIRE(ec); + REQUIRE(ec == avtp::make_error_code(avtp::AvtpErrc::length_mismatch)); +} + +// ── MultiFrame — multiple ACF requests in one AVTPDU (cpp-RCP-04-fresh) ────── + +TEST_CASE("MultiFrame round-trips two ACF_ABB messages packed into one AVTPDU", + "[udp][REQ-UDP-001]") { + // Payloads are quadlet-aligned (4 bytes each) so the first message's + // acf_msg_length is byte-exact and the second message's boundary can be + // found precisely — see acf::decode_acf_messages's own documented scope + // limit for the non-aligned case. + MultiFrame f; + f.use_tscf = false; + f.stream_id = make_stream_id(0x02, 0x5555); + f.sequence_num = 3; + + acf::AcfEntry a; + a.info = standard_request(/*bus_id=*/1, /*transaction_num=*/10, /*write=*/true); + a.payload = {0x01, 0x02, 0x03, 0x04}; + acf::AcfEntry b; + b.info = standard_request(/*bus_id=*/2, /*transaction_num=*/20, /*write=*/false); + b.payload = {0x05, 0x06, 0x07, 0x08}; + f.messages = {a, b}; + + auto bytes = encode_multi_frame(f); + + MultiFrame out; + REQUIRE_FALSE(decode_multi_frame(bytes.data(), bytes.size(), out)); + REQUIRE(out.messages.size() == 2); + REQUIRE(out.messages[0].info.byte_bus_id == 1); + REQUIRE(out.messages[0].info.transaction_num == 10); + REQUIRE(out.messages[0].payload == a.payload); + REQUIRE(out.messages[1].info.byte_bus_id == 2); + REQUIRE(out.messages[1].info.transaction_num == 20); + REQUIRE(out.messages[1].payload == b.payload); +} + // ── Server (real UDP sockets, loopback) ────────────────────────────────────── TEST_CASE("Server dispatches a decoded request through its handler and answers the sender", @@ -208,6 +259,67 @@ TEST_CASE("Server dispatches a decoded request through its handler and answers t client.close(); } +TEST_CASE("Server handles multiple requests packed into a single datagram individually " + "(extraction §12.9.1.1)", + "[udp][REQ-UDP-007]") { + // rcp::udp::Client always sends one request per datagram (see Client's + // own header comment), so this test drives Server with a raw socket to + // actually exercise the multi-request-in-one-frame path (cpp-RCP-04-fresh). + Server server(make_stream_id(0x02, 7), "127.0.0.1", 0); + REQUIRE(server.ok()); + + std::vector seen_bus_ids; + server.set_handler([&](size_t, const acf::AcfMessageInfo& req, + const std::vector& req_payload, + acf::AcfMessageInfo& out_resp, std::vector& out_resp_payload) { + seen_bus_ids.push_back(req.byte_bus_id); + out_resp_payload = req_payload; + out_resp = acf::make_response(req, acf::ResponseKind::ReadResponse); + return std::error_code{}; + }); + + MultiFrame req_frame; + req_frame.use_tscf = false; + req_frame.stream_id = make_stream_id(0x03, 7); + req_frame.sequence_num = 1; + + acf::AcfEntry req_a; + req_a.info = standard_request(/*bus_id=*/1, /*transaction_num=*/1); + req_a.payload = {0xAA, 0xAA, 0xAA, 0xAA}; + acf::AcfEntry req_b; + req_b.info = standard_request(/*bus_id=*/2, /*transaction_num=*/2); + req_b.payload = {0xBB, 0xBB, 0xBB, 0xBB}; + req_frame.messages = {req_a, req_b}; + + auto req_bytes = encode_multi_frame(req_frame); + + int raw_fd = ::socket(AF_INET, SOCK_DGRAM, 0); + REQUIRE(raw_fd >= 0); + sockaddr_in server_addr{}; + server_addr.sin_family = AF_INET; + server_addr.sin_port = htons(server.port()); + server_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + auto sent = ::sendto(raw_fd, req_bytes.data(), req_bytes.size(), 0, + reinterpret_cast(&server_addr), sizeof(server_addr)); + REQUIRE(sent == static_cast(req_bytes.size())); + + std::vector recv_buf(kMaxDatagram); + ssize_t n = ::recv(raw_fd, recv_buf.data(), recv_buf.size(), 0); + REQUIRE(n > 0); + ::close(raw_fd); + + MultiFrame resp_frame; + REQUIRE_FALSE(decode_multi_frame(recv_buf.data(), static_cast(n), resp_frame)); + REQUIRE(resp_frame.messages.size() == 2); + REQUIRE(resp_frame.messages[0].info.byte_bus_id == 1); + REQUIRE(resp_frame.messages[0].payload == req_a.payload); + REQUIRE(resp_frame.messages[1].info.byte_bus_id == 2); + REQUIRE(resp_frame.messages[1].payload == req_b.payload); + REQUIRE(seen_bus_ids == std::vector{1, 2}); // both dispatched individually, in order + + server.close(); +} + TEST_CASE("Server answers Acknowledge by default when no handler is registered", "[udp][REQ-UDP-008]") { Server server(make_stream_id(0x02, 2), "127.0.0.1", 0);