From b85842a73f2169c62ace534775ff13ee65250760 Mon Sep 17 00:00:00 2001 From: Matt <47545907+SoundMatt@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:13:25 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20v2.2.0=20=E2=80=94=20discovery=20reques?= =?UTF-8?q?t=20+=20discovery-stream=20claiming=20(=C2=A7Phase=2013=20miles?= =?UTF-8?q?tone=2046)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds rcp/discovery.hpp: the broadcastable ACF_ABB discovery read addressed to byte_bus_id 0 (kDiscoveryByteBusId, matching regmap::kEp0) at register-map address 0 (kDiscoveryRegisterAddress), always NTSCF-headed — a TSCF-headed discovery request is dropped by decode_discovery_request as a decode failure (DiscoveryErrc::tscf_headed_request_dropped), not a flag a caller could forget to check. Rides directly on rcp/wire.hpp's existing NTSCF/ACF_ABB framing with no changes to that codec. DiscoveryClaim implements discovery-stream claiming: the first discovery request accepted while rcp::lifecycle::ServerLifecycle reports HwUnconfigured/HwConfigured reserves the stream for that client's configuration writes; the reservation lapses after a configurable Discovery_TimeOut (~20ms default) if no configuration request follows; concurrent discovery requests from other clients never dislodge an active claim nor block their own reads from being answered; and a request in RcpConfigured is always NotEligible, since claiming itself is scoped to the two pre-RCP-configured states. should_answer_discovery is a single, always-true, testable call site documenting the any-state answering invariant (extraction §3.1, §3.5). New coverage in tests/test_discovery.cpp (REQ-DISC-001..009), traced in .fusa-reqs.json. Bumps version.hpp and the CMake project version to 2.2.0, and adds ROADMAP.md milestone 46's "Done (v2.2.0)" note following the milestone 44/45 precedent. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> --- .fusa-reqs.json | 72 ++++++++++ CMakeLists.txt | 2 +- ROADMAP.md | 28 ++++ include/rcp/discovery.hpp | 267 ++++++++++++++++++++++++++++++++++++++ include/rcp/version.hpp | 2 +- tests/CMakeLists.txt | 4 + tests/test_discovery.cpp | 230 ++++++++++++++++++++++++++++++++ 7 files changed, 603 insertions(+), 2 deletions(-) create mode 100644 include/rcp/discovery.hpp create mode 100644 tests/test_discovery.cpp diff --git a/.fusa-reqs.json b/.fusa-reqs.json index 87dbdc6..5d2b848 100644 --- a/.fusa-reqs.json +++ b/.fusa-reqs.json @@ -2927,6 +2927,78 @@ "standard": "iso26262", "level": "ASIL-B", "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-001", + "title": "Discovery request framing \u2014 byte_bus_id 0, register-map address 0", + "text": "rcp::discovery::make_discovery_request shall build an unconditional ACF_ABB read (op=false) targeting byte_bus_id 0 (kDiscoveryByteBusId), and kDiscoveryRegisterAddress shall be 0, fixing discovery reads at the general bootstrap/magic-number register-map region.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-002", + "title": "Discovery is NTSCF-only; a TSCF-headed discovery request is dropped", + "text": "rcp::discovery::encode_discovery_request shall always produce an NTSCF-headed frame, and rcp::discovery::decode_discovery_request shall return DiscoveryErrc::tscf_headed_request_dropped without decoding further when given a frame whose AVTPDU subtype byte is TSCF.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-003", + "title": "The first discovery request claims the discovery stream", + "text": "DiscoveryClaim::on_discovery_request shall return ClaimOutcome::Claimed and record `client` as the claim holder when no active claim currently exists and server_state is HwUnconfigured or HwConfigured.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-004", + "title": "The discovery claim lapses after Discovery_TimeOut", + "text": "DiscoveryClaim shall treat a claim as lapsed once (now - claimed_at) is at least Discovery_TimeOut (kDefaultTimeout, ~20ms by default), after which may_configure shall return false for the prior holder and a subsequent discovery request from any client shall be able to claim the stream anew.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-005", + "title": "A claim does not block concurrent discovery reads from other clients", + "text": "DiscoveryClaim::on_discovery_request from a client other than the active holder shall return ClaimOutcome::HeldByOther without altering the existing claim, and shall never affect whether that client's underlying discovery read is answered; the same holder re-requesting before the claim lapses shall return ClaimOutcome::AlreadyHeld.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-006", + "title": "Only the active claim holder may issue a configuration request", + "text": "DiscoveryClaim::on_configuration_request shall return true and consume (release) the claim only when called by the client currently holding an active, unlapsed claim; a call from any other client shall return false and shall leave an existing other-held claim untouched.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-007", + "title": "Discovery-stream claiming is scoped to HW_UNCONFIGURED/HW_CONFIGURED", + "text": "DiscoveryClaim::on_discovery_request shall return ClaimOutcome::NotEligible and leave any existing claim state unchanged whenever server_state is RcpConfigured.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-008", + "title": "DiscoveryErrc is a distinct error category with non-empty messages", + "text": "rcp::discovery::make_error_code(DiscoveryErrc::tscf_headed_request_dropped) shall report rcp::discovery::discovery_category() as its category and shall have a non-empty diagnostic message.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" + }, + { + "id": "REQ-DISC-009", + "title": "A server answers discovery in every lifecycle state", + "text": "rcp::discovery::should_answer_discovery shall return true for HwUnconfigured, HwConfigured, and RcpConfigured alike, reflecting the mandatory-baseline requirement that discovery is answered regardless of lifecycle state.", + "standard": "iso26262", + "level": "ASIL-B", + "asil": "ASIL-B" } ] } diff --git a/CMakeLists.txt b/CMakeLists.txt index 7b9d1ad..4a6950d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.21) project(cpp-rcp - VERSION 2.1.0 + VERSION 2.2.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 ee70859..6eda91a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -266,6 +266,34 @@ onward); see the header comment in `rcp.hpp` and the disposition table. ### 46. Discovery (v2.2.0) +**Done (v2.2.0):** `rcp/discovery.hpp` implements the discovery mechanism +described below on top of the three prior milestones without changing any +of them. `make_discovery_request`/`encode_discovery_request` build an +unconditional ACF_ABB read targeting `byte_bus_id 0` (`kDiscoveryByteBusId`, +matching `regmap::kEp0`) at register-map address 0 (`kDiscoveryRegisterAddress` +— the general bootstrap/magic-number field region `rcp/regmap.hpp` already +models), always wrapped in an NTSCF header, riding directly on `rcp/wire.hpp`'s +existing framing with no changes to that codec. `decode_discovery_request` +enforces the NTSCF-only rule by returning +`DiscoveryErrc::tscf_headed_request_dropped` — modeled as a decode failure, +not a flag a caller could forget to check — for any TSCF-headed frame. +`DiscoveryClaim` implements discovery-stream claiming: the first discovery +request accepted while `rcp::lifecycle::ServerLifecycle` reports +`HwUnconfigured`/`HwConfigured` reserves the stream for that client's +configuration writes (`ClaimOutcome::Claimed`); the reservation lapses after +a configurable `Discovery_TimeOut` (`kDefaultTimeout`, ~20 ms default) if no +configuration request follows (`on_configuration_request` returning `false` +once lapsed, freeing the stream for the next claimant); other clients' +concurrent discovery requests are reported as `HeldByOther`/`AlreadyHeld` +without ever touching whether their own reads get answered, since claiming +and read-answering are deliberately independent concerns in this +implementation; and a request in `RcpConfigured` is always `NotEligible`, +since the claiming mechanism itself is scoped to the two pre-RCP-configured +states. `should_answer_discovery` is a trivial always-true function over +`ServerState` — a deliberate, testable single call site documenting the +any-state answering invariant rather than an implicit assumption. New +coverage lives in `tests/test_discovery.cpp` (REQ-DISC-001..009). + - Implement the discovery request: a broadcastable ACF_ABB read addressed to `byte_bus_id 0`, NTSCF-only (a TSCF-headed discovery request is dropped), reading from register-map address 0 (extraction §3.5) diff --git a/include/rcp/discovery.hpp b/include/rcp/discovery.hpp new file mode 100644 index 0000000..ce8bedc --- /dev/null +++ b/include/rcp/discovery.hpp @@ -0,0 +1,267 @@ +// fusa:req REQ-DISC-001 +// fusa:req REQ-DISC-002 +// fusa:req REQ-DISC-003 +// fusa:req REQ-DISC-004 +// fusa:req REQ-DISC-005 +// fusa:req REQ-DISC-006 +// fusa:req REQ-DISC-007 +// fusa:req REQ-DISC-008 +// fusa:req REQ-DISC-009 + +// RC Server discovery — the broadcastable, byte_bus_id-0 read request every +// OPEN Alliance TC18 Remote Control Protocol Specification v0.5.1_RC server +// must answer regardless of lifecycle state, plus discovery-stream claiming +// for configuration (extraction §3.1, §3.5). +// +// ROADMAP.md milestone 46, "Discovery (v2.2.0)": this header rides directly +// on rcp/wire.hpp's NTSCF/ACF_ABB framing and byte_bus_id addressing (v2.0.0) +// — it needs no changes to that codec — queries rcp/lifecycle.hpp's +// ServerState to decide whether an incoming discovery request is eligible to +// claim the discovery stream (v2.1.0), and targets rcp/regmap.hpp's EP0 +// (byte_bus_id 0, register-map address 0 — the general bootstrap/magic-number +// field region) as the thing being read (also v2.1.0). This module does not +// itself implement a byte-level serialization of the whole register map to +// and from the wire; per regmap.hpp's own header comment that remains a +// later-milestone concern. It only fixes the address a discovery request +// must carry so that later serialization work has a stable, already-decided +// target to hook into. +// +// 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 claim-state +// machine and default timeout chosen in this file are this implementation's +// own encoding of that behavior, same as the equivalent disclaimers in +// rcp/wire.hpp, rcp/lifecycle.hpp, and rcp/regmap.hpp. +#pragma once + +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +namespace rcp { +namespace discovery { + +// ── Discovery addressing ───────────────────────────────────────────────────── +// A discovery request targets byte_bus_id 0 — the same id regmap::kEp0 uses +// — and, within whatever EP0 exposes, register-map address 0: the general +// bootstrap fields, magic number first among them (extraction §3.5). + +constexpr wire::ByteBusId kDiscoveryByteBusId = static_cast(regmap::kEp0); +constexpr uint32_t kDiscoveryRegisterAddress = 0; + +// A discovery read's default response size: just enough to carry the 32-bit +// magic number at register-map address 0. Callers reading more of the +// bootstrap block may pass a larger read_size to make_discovery_request. +constexpr uint16_t kDiscoveryDefaultReadSize = sizeof(uint32_t); + +// ── Errors ──────────────────────────────────────────────────────────────────── + +enum class DiscoveryErrc : int { + // A TSCF-headed discovery request was received. Discovery is NTSCF-only + // (extraction §3.5); such a request is dropped rather than answered or + // partially processed. + tscf_headed_request_dropped = 1, +}; + +inline const std::error_category& discovery_category() noexcept { + struct Cat : std::error_category { + const char* name() const noexcept override { return "rcp.discovery"; } + std::string message(int ev) const override { + switch (static_cast(ev)) { + case DiscoveryErrc::tscf_headed_request_dropped: + return "rcp/discovery: TSCF-headed discovery request dropped (discovery is NTSCF-only)"; + default: + return "rcp/discovery: unknown error"; + } + } + }; + static Cat instance; + return instance; +} + +inline std::error_code make_error_code(DiscoveryErrc e) noexcept { + return {static_cast(e), discovery_category()}; +} + +// ── Discovery request framing ──────────────────────────────────────────────── +// make_discovery_request builds the ACF_ABB-level header for a discovery +// read: unconditional read (op=false), targeting byte_bus_id 0 +// (extraction §3.5). It rides on wire::make_standard_request unchanged — +// discovery is the mandatory standard request kind addressed at a fixed +// endpoint, not a distinct wire-level message shape of its own. +inline wire::AcfMessageInfo make_discovery_request(uint8_t transaction_num, + uint16_t read_size = kDiscoveryDefaultReadSize) noexcept { + return wire::make_standard_request(kDiscoveryByteBusId, transaction_num, + /*write=*/false, read_size); +} + +// encode_discovery_request wraps make_discovery_request's ACF_ABB message in +// an NTSCF header addressed to `stream_id`. Discovery requests are +// NTSCF-only and broadcastable (extraction §3.5) — this function never +// produces a TSCF-headed frame, so there is no way to misuse it into +// building the kind of request decode_discovery_request below must drop. +inline std::vector encode_discovery_request(const wire::StreamId& stream_id, + uint16_t sequence_num, + uint8_t transaction_num, + uint16_t read_size = kDiscoveryDefaultReadSize) { + const auto info = make_discovery_request(transaction_num, read_size); + const auto acf = wire::encode_acf_abb(info, {}); + + wire::NtscfHeader hdr; + hdr.stream_id = stream_id; + hdr.sequence_num = sequence_num; + hdr.control_data_length = static_cast(acf.size()); + + auto out = wire::encode_ntscf_header(hdr); + out.insert(out.end(), acf.begin(), acf.end()); + return out; +} + +// decode_discovery_request decodes a raw AVTPDU frame as a discovery +// request. Per extraction §3.5, discovery is NTSCF-only: if the frame's +// AVTPDU subtype byte is TSCF, this function returns +// DiscoveryErrc::tscf_headed_request_dropped without decoding further — +// modeling "drop" as a decode failure the caller cannot accidentally ignore +// the way it could ignore a boolean flag on an otherwise-successful decode. +inline std::error_code decode_discovery_request(const uint8_t* buf, size_t len, + wire::NtscfHeader& out_hdr, + wire::AcfMessageInfo& out_info, + std::vector& out_payload) { + if (len < 1) return wire::make_error_code(wire::WireErrc::short_buffer); + if (buf[0] == wire::kSubtypeTscf) { + return make_error_code(DiscoveryErrc::tscf_headed_request_dropped); + } + + auto ec = wire::decode_ntscf_header(buf, len, out_hdr); + if (ec) return ec; + + const size_t acf_off = wire::kNtscfHeaderLen; + if (len < acf_off) return wire::make_error_code(wire::WireErrc::short_buffer); + return wire::decode_acf_abb(buf + acf_off, len - acf_off, out_info, out_payload); +} + +// should_answer_discovery documents, as a single always-true call site +// rather than an implicit assumption scattered across a lifecycle switch +// statement, that a server answers a (NTSCF-headed) discovery request in +// *every* lifecycle state — including RCP_CONFIGURED, where the claiming +// mechanism below no longer applies but discovery reads still must +// (extraction §3.1, §3.5's mandatory-baseline requirement). +constexpr bool should_answer_discovery(lifecycle::ServerState /*state*/) noexcept { + return true; +} + +// ── Discovery-stream claiming ──────────────────────────────────────────────── +// DiscoveryClaim models the reservation described in extraction §3.5: the +// first discovery request a server receives while it is in HW_UNCONFIGURED +// or HW_CONFIGURED reserves the discovery stream for that client's +// subsequent configuration writes. If no configuration request follows +// within Discovery_TimeOut, the reservation lapses and any client's next +// discovery request may claim it anew. A claim never blocks reads — every +// client, holder or not, keeps getting discovery reads answered — it only +// gates whether a *configuration* (write) request is allowed to proceed. +// +// Time is supplied by the caller as a std::chrono::steady_clock::time_point +// rather than read internally, so embedding code (and tests) control the +// clock explicitly instead of this class racing a real timer. +class DiscoveryClaim { +public: + using Clock = std::chrono::steady_clock; + using TimePoint = Clock::time_point; + + // Discovery_TimeOut default, ~20 ms per extraction §3.5. The + // specification calls this out as configurable; this is this + // implementation's chosen default, not a mandated value. + static constexpr std::chrono::milliseconds kDefaultTimeout{20}; + + // Outcome of a discovery request arriving at on_discovery_request. + enum class ClaimOutcome { + NotEligible, // server_state is neither HwUnconfigured nor HwConfigured; no claim made + Claimed, // no active claim existed (or the prior one had lapsed); `client` now holds it + AlreadyHeld, // `client` already held the still-active claim; unchanged + HeldByOther, // a different client's claim is still active; this request did not claim + }; + + explicit DiscoveryClaim(std::chrono::milliseconds timeout = kDefaultTimeout) noexcept + : timeout_(timeout) {} + + // on_discovery_request should be called for every discovery request a + // server accepts (i.e. every one decode_discovery_request did not drop), + // tagged with the requesting client's opaque id, the server's current + // lifecycle state, and the current time. The claiming mechanism itself + // only applies in HW_UNCONFIGURED/HW_CONFIGURED (extraction §3.5); in + // RCP_CONFIGURED this always reports NotEligible without touching any + // existing claim, since by that point configuration is already done. + // This call never affects whether the request's *read* gets answered — + // callers must answer it regardless of the outcome returned here. + ClaimOutcome on_discovery_request(size_t client, lifecycle::ServerState server_state, + TimePoint now) noexcept { + if (server_state != lifecycle::ServerState::HwUnconfigured && + server_state != lifecycle::ServerState::HwConfigured) { + return ClaimOutcome::NotEligible; + } + if (holder_.has_value() && !lapsed(now)) { + return (*holder_ == client) ? ClaimOutcome::AlreadyHeld : ClaimOutcome::HeldByOther; + } + holder_ = client; + claimed_at_ = now; + return ClaimOutcome::Claimed; + } + + // may_configure reports whether `client` currently holds an active, + // unlapsed claim — i.e. is presently allowed to issue a configuration + // (write) request on the discovery stream. It does not itself lapse or + // consume the claim; it is a pure query. + bool may_configure(size_t client, TimePoint now) const noexcept { + return holder_.has_value() && *holder_ == client && !lapsed(now); + } + + // on_configuration_request should be called when a configuration + // request arrives on the discovery stream. It returns true iff `client` + // held an active claim at `now`, in which case that claim is consumed + // (released) — a spent reservation is not carried forward, so the very + // next discovery request, from any client, is free to claim the stream + // again. A request from a client that does not hold the active claim + // returns false and leaves the existing claim (if any, held by someone + // else) untouched, so the real holder's window is not disturbed by an + // unrelated client's rejected attempt. + bool on_configuration_request(size_t client, TimePoint now) noexcept { + if (!may_configure(client, now)) return false; + holder_.reset(); + return true; + } + + // has_active_claim / current_holder are read-only introspection for + // callers (e.g. admin/observability surfaces) that want to report claim + // state without being able to mutate it. + bool has_active_claim(TimePoint now) const noexcept { + return holder_.has_value() && !lapsed(now); + } + std::optional current_holder(TimePoint now) const noexcept { + return has_active_claim(now) ? holder_ : std::nullopt; + } + +private: + bool lapsed(TimePoint now) const noexcept { + return !holder_.has_value() || (now - claimed_at_) >= timeout_; + } + + std::chrono::milliseconds timeout_; + std::optional holder_; + TimePoint claimed_at_{}; +}; + +} // namespace discovery +} // namespace rcp + +// Enable std::error_code construction from rcp::discovery::DiscoveryErrc. +namespace std { +template <> +struct is_error_code_enum : true_type {}; +} // namespace std diff --git a/include/rcp/version.hpp b/include/rcp/version.hpp index c7ccc18..a185b80 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.1.0"; +constexpr std::string_view kVersion = "2.2.0"; } // namespace rcp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 61cf275..5e388c1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -22,6 +22,10 @@ add_executable(test_regmap test_regmap.cpp) target_link_libraries(test_regmap PRIVATE rcp::rcp Catch2::Catch2WithMain) add_test(NAME rcp_regmap COMMAND test_regmap) +add_executable(test_discovery test_discovery.cpp) +target_link_libraries(test_discovery PRIVATE rcp::rcp Catch2::Catch2WithMain) +add_test(NAME rcp_discovery COMMAND test_discovery) + add_executable(test_e2e test_e2e.cpp) target_link_libraries(test_e2e PRIVATE rcp::rcp Catch2::Catch2WithMain) add_test(NAME rcp_e2e COMMAND test_e2e) diff --git a/tests/test_discovery.cpp b/tests/test_discovery.cpp new file mode 100644 index 0000000..9792104 --- /dev/null +++ b/tests/test_discovery.cpp @@ -0,0 +1,230 @@ +// fusa:test REQ-DISC-001 +// fusa:test REQ-DISC-002 +// fusa:test REQ-DISC-003 +// fusa:test REQ-DISC-004 +// fusa:test REQ-DISC-005 +// fusa:test REQ-DISC-006 +// fusa:test REQ-DISC-007 +// fusa:test REQ-DISC-008 +// fusa:test REQ-DISC-009 + +// Tests for rcp/discovery.hpp — the RC Server discovery request and +// discovery-stream claiming mechanism (ROADMAP.md milestone 46, "Discovery", +// v2.2.0). + +#include +#include + +using namespace rcp::discovery; +using rcp::lifecycle::ServerState; + +namespace { + +using Clock = DiscoveryClaim::Clock; +using TimePoint = DiscoveryClaim::TimePoint; + +TimePoint at(int64_t ms) { + return TimePoint(std::chrono::milliseconds(ms)); +} + +} // namespace + +// ── Discovery request framing ──────────────────────────────────────────────── + +TEST_CASE("make_discovery_request targets byte_bus_id 0 as an unconditional read", "[discovery][REQ-DISC-001]") { + auto info = make_discovery_request(/*transaction_num=*/7); + REQUIRE(info.byte_bus_id == kDiscoveryByteBusId); + REQUIRE(info.byte_bus_id == 0); + REQUIRE_FALSE(info.op); // read, not write + REQUIRE_FALSE(info.rsp); + REQUIRE(info.transaction_num == 7); + REQUIRE(info.read_size_or_segment_num == kDiscoveryDefaultReadSize); +} + +TEST_CASE("kDiscoveryRegisterAddress fixes the discovery read at register-map address 0", + "[discovery][REQ-DISC-001]") { + REQUIRE(kDiscoveryRegisterAddress == 0); +} + +// ── NTSCF-only framing; TSCF-headed discovery is dropped ──────────────────────── + +TEST_CASE("encode_discovery_request always produces an NTSCF-headed frame", "[discovery][REQ-DISC-002]") { + rcp::wire::StreamId sid; + sid.mac = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06}; + sid.suffix = 1; + + auto frame = encode_discovery_request(sid, /*sequence_num=*/0, /*transaction_num=*/1); + REQUIRE_FALSE(frame.empty()); + REQUIRE(frame[0] == rcp::wire::kSubtypeNtscf); +} + +TEST_CASE("decode_discovery_request round-trips an encoded discovery request", "[discovery][REQ-DISC-002]") { + rcp::wire::StreamId sid; + sid.mac = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + sid.suffix = 42; + + auto frame = encode_discovery_request(sid, /*sequence_num=*/3, /*transaction_num=*/9, /*read_size=*/4); + + rcp::wire::NtscfHeader hdr; + rcp::wire::AcfMessageInfo info; + std::vector payload; + auto ec = decode_discovery_request(frame.data(), frame.size(), hdr, info, payload); + + REQUIRE_FALSE(ec); + REQUIRE(hdr.stream_id == sid); + REQUIRE(hdr.sequence_num == 3); + REQUIRE(info.byte_bus_id == kDiscoveryByteBusId); + REQUIRE(info.transaction_num == 9); + REQUIRE_FALSE(info.op); +} + +TEST_CASE("A TSCF-headed discovery request is dropped, not decoded", "[discovery][REQ-DISC-002]") { + // Build a TSCF frame around the same discovery-shaped ACF_ABB payload — + // this must never be treated as a valid discovery request. + rcp::wire::TscfHeader hdr; + hdr.stream_id.mac = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06}; + hdr.stream_id.suffix = 1; + + auto info = make_discovery_request(/*transaction_num=*/1); + auto acf = rcp::wire::encode_acf_abb(info, {}); + hdr.control_data_length = static_cast(acf.size()); + + auto frame = rcp::wire::encode_tscf_header(hdr); + frame.insert(frame.end(), acf.begin(), acf.end()); + + rcp::wire::NtscfHeader out_hdr; + rcp::wire::AcfMessageInfo out_info; + std::vector out_payload; + auto ec = decode_discovery_request(frame.data(), frame.size(), out_hdr, out_info, out_payload); + + REQUIRE(ec); + REQUIRE(ec == make_error_code(DiscoveryErrc::tscf_headed_request_dropped)); +} + +// ── Any-state answering ────────────────────────────────────────────────────────── + +TEST_CASE("A server answers discovery in every lifecycle state", "[discovery][REQ-DISC-009]") { + REQUIRE(should_answer_discovery(ServerState::HwUnconfigured)); + REQUIRE(should_answer_discovery(ServerState::HwConfigured)); + REQUIRE(should_answer_discovery(ServerState::RcpConfigured)); +} + +// ── Discovery-stream claiming: first request claims it ────────────────────────── + +TEST_CASE("The first discovery request in HW_UNCONFIGURED claims the discovery stream", + "[discovery][REQ-DISC-003]") { + DiscoveryClaim claim; + auto outcome = claim.on_discovery_request(/*client=*/1, ServerState::HwUnconfigured, at(0)); + REQUIRE(outcome == DiscoveryClaim::ClaimOutcome::Claimed); + REQUIRE(claim.has_active_claim(at(0))); + REQUIRE(claim.current_holder(at(0)) == std::optional(1)); +} + +TEST_CASE("The first discovery request in HW_CONFIGURED also claims the discovery stream", + "[discovery][REQ-DISC-003]") { + DiscoveryClaim claim; + auto outcome = claim.on_discovery_request(/*client=*/5, ServerState::HwConfigured, at(0)); + REQUIRE(outcome == DiscoveryClaim::ClaimOutcome::Claimed); + REQUIRE(claim.may_configure(5, at(1))); +} + +// ── Claim scoped to HW_UNCONFIGURED / HW_CONFIGURED only ──────────────────────── + +TEST_CASE("A discovery request in RCP_CONFIGURED never claims the stream", "[discovery][REQ-DISC-007]") { + DiscoveryClaim claim; + auto outcome = claim.on_discovery_request(/*client=*/1, ServerState::RcpConfigured, at(0)); + REQUIRE(outcome == DiscoveryClaim::ClaimOutcome::NotEligible); + REQUIRE_FALSE(claim.has_active_claim(at(0))); + REQUIRE_FALSE(claim.may_configure(1, at(0))); +} + +// ── Claim lapse after Discovery_TimeOut ────────────────────────────────────────── + +TEST_CASE("An unclaimed-follow-up claim lapses after Discovery_TimeOut elapses", "[discovery][REQ-DISC-004]") { + DiscoveryClaim claim(std::chrono::milliseconds(20)); + REQUIRE(claim.on_discovery_request(1, ServerState::HwUnconfigured, at(0)) + == DiscoveryClaim::ClaimOutcome::Claimed); + REQUIRE(claim.may_configure(1, at(19))); + + // Past the timeout, the claim has lapsed: the original holder may no + // longer configure ... + REQUIRE_FALSE(claim.may_configure(1, at(21))); + REQUIRE_FALSE(claim.has_active_claim(at(21))); + + // ... and a different client's next discovery request is free to claim + // the now-lapsed stream. + auto outcome = claim.on_discovery_request(2, ServerState::HwUnconfigured, at(25)); + REQUIRE(outcome == DiscoveryClaim::ClaimOutcome::Claimed); + REQUIRE(claim.may_configure(2, at(25))); +} + +TEST_CASE("Discovery_TimeOut defaults to approximately 20ms", "[discovery][REQ-DISC-004]") { + REQUIRE(DiscoveryClaim::kDefaultTimeout == std::chrono::milliseconds(20)); +} + +// ── Configuration request consumes the active claim ───────────────────────────── + +TEST_CASE("A configuration request from the claim holder succeeds and consumes the claim", + "[discovery][REQ-DISC-006]") { + DiscoveryClaim claim; + REQUIRE(claim.on_discovery_request(1, ServerState::HwUnconfigured, at(0)) + == DiscoveryClaim::ClaimOutcome::Claimed); + + REQUIRE(claim.on_configuration_request(1, at(5))); + // The claim is spent: neither the same client's follow-up configuration + // request, nor a query, sees it as still active. + REQUIRE_FALSE(claim.has_active_claim(at(5))); + REQUIRE_FALSE(claim.on_configuration_request(1, at(6))); +} + +TEST_CASE("A configuration request from a client that does not hold the claim is refused, " + "leaving the real holder's claim untouched", + "[discovery][REQ-DISC-006]") { + DiscoveryClaim claim; + REQUIRE(claim.on_discovery_request(1, ServerState::HwUnconfigured, at(0)) + == DiscoveryClaim::ClaimOutcome::Claimed); + + REQUIRE_FALSE(claim.on_configuration_request(/*client=*/2, at(1))); + // Client 1's claim survives client 2's rejected attempt. + REQUIRE(claim.may_configure(1, at(2))); +} + +// ── Concurrent reads: other clients keep being answered during an active claim ── + +TEST_CASE("Another client's discovery request during an active claim does not dislodge or " + "duplicate it, but that client's read is unaffected", + "[discovery][REQ-DISC-005]") { + DiscoveryClaim claim; + REQUIRE(claim.on_discovery_request(1, ServerState::HwUnconfigured, at(0)) + == DiscoveryClaim::ClaimOutcome::Claimed); + + // A second client's discovery request during the still-active window: it + // does not claim the stream ... + auto outcome = claim.on_discovery_request(2, ServerState::HwUnconfigured, at(5)); + REQUIRE(outcome == DiscoveryClaim::ClaimOutcome::HeldByOther); + // ... yet answering that client's underlying discovery *read* is a + // separate, unconditional concern the claim never gates — modeled here + // by should_answer_discovery being state-only and never consulting claim + // state at all. + REQUIRE(should_answer_discovery(ServerState::HwUnconfigured)); + + // The original holder's claim is unaffected by the other client's request. + REQUIRE(claim.may_configure(1, at(6))); +} + +TEST_CASE("The claim holder re-requesting discovery before it lapses is reported as AlreadyHeld", + "[discovery][REQ-DISC-005]") { + DiscoveryClaim claim; + REQUIRE(claim.on_discovery_request(1, ServerState::HwUnconfigured, at(0)) + == DiscoveryClaim::ClaimOutcome::Claimed); + auto outcome = claim.on_discovery_request(1, ServerState::HwUnconfigured, at(2)); + REQUIRE(outcome == DiscoveryClaim::ClaimOutcome::AlreadyHeld); +} + +// ── DiscoveryErrc category sanity ──────────────────────────────────────────────── + +TEST_CASE("DiscoveryErrc reports a non-empty message in its own category", "[discovery][REQ-DISC-008]") { + auto ec = make_error_code(DiscoveryErrc::tscf_headed_request_dropped); + REQUIRE(ec.category() == discovery_category()); + REQUIRE_FALSE(ec.message().empty()); +}