diff --git a/include/rcp/cli.hpp b/include/rcp/cli.hpp index 42778db..ccba511 100644 --- a/include/rcp/cli.hpp +++ b/include/rcp/cli.hpp @@ -92,6 +92,33 @@ inline std::string version_text() { // ── §12.2 capabilities document (always JSON) ──────────────────────────────── // Field set is constrained by cli-capabilities.json (additionalProperties:false). +// +// "features" (parity-audit gap-closure vs c-RCP's cli.c): alongside the +// existing physical-endpoint-type names, this now also reports the five +// TC18 §12.7.5 Table 20 svr_implemented_options names (REQ-RMAP-030, +// rcp/regmap.hpp's kOptCompoundWait/kOptTrigger/kOptChained/kOptTimeSync/ +// kOptEnhCancel) — matching c-RCP's own cli.c features_json()/ +// RCP_CLI_IMPLEMENTED_OPTIONS, which reports the identical five names from +// the identical five bits. This is a static, build-time answer (this CLI +// never connects to a live RC Server whose own runtime +// svr_implemented_options could be read instead — same rationale as +// c-RCP's own comment) reflecting that rcp/request.hpp implements +// compound/compound-wait/triggered/chained/timed requests and both +// clear-all/clear-single cancellation, and rcp/regmap.hpp defines the five +// corresponding bits. Unlike c-RCP's cli.c, no per-feature wire-conformance +// caveat is asserted here one way or the other — that would need a +// dedicated audit of request.hpp/e2e.hpp against the spec, out of scope for +// this pass. One known related gap, left alone here: request.hpp/e2e.hpp's +// own conditional-request gating (implemented_options_bits()/ +// timed_feature_enabled()) still reads regmap.hpp's coarser legacy +// kOptConditionalRequests bit rather than these five independent ones +// directly (regmap.hpp's own "Legacy...retained ONLY because" comment) — +// neither header is in this batch's scope to rewire. +// +// "transports" additionally reports "tsn" (rcp/tsn.hpp, REQ-TSN-001..006, a +// real 802.1p PCP-tagging transport wrapper, not a stub) — c-RCP's own +// cli.c advertises its structurally-equivalent tsn.c; cpp-RCP's cli.hpp +// previously did not. inline std::string capabilities_json() { std::string spec(relay::kRelaySpecVersion); @@ -103,9 +130,10 @@ inline std::string capabilities_json() { + "\"version\":\"" + std::string(kVersion) + "\"," + "\"spec_version\":\"" + spec + "\"," + "\"commands\":[\"version\",\"capabilities\",\"status\",\"send\"]," - + "\"transports\":[\"udp\",\"shmem\",\"mock\"]," + + "\"transports\":[\"udp\",\"shmem\",\"mock\",\"tsn\"]," + "\"features\":[\"gpio\",\"spi\",\"i2c\",\"uart\",\"adc\",\"pwm\",\"lin\",\"can\"," - "\"iseled\",\"mdio\",\"wakeup\"]," + "\"iseled\",\"mdio\",\"wakeup\",\"time_sync\",\"enhanced_cancel\",\"trigger\"," + "\"chained\",\"compound_bundles\"]," + "\"interfaces\":[\"Node\",\"Caller\"]," + "\"optional_interfaces\":[]," + "\"adapt\":true" diff --git a/include/rcp/config.hpp b/include/rcp/config.hpp index d6d4a7a..73fd631 100644 --- a/include/rcp/config.hpp +++ b/include/rcp/config.hpp @@ -35,13 +35,48 @@ // Disposition table's entry for `config.hpp` — "rebuilt around a // server/endpoint manifest schema; still useful for bootstrapping known // topologies alongside discovery." +// +// Parity-audit gap-closure (Phase 8 batch A) — c-RCP's own config.c/config.h +// went through a LATER "Satellite Package Rework" (its own milestone 77) +// that replaced ITS zone-manifest schema with a richer one: a "server" +// object (vendor_id/device_id/magic/svr_implemented_options), a +// "hw_pin_map" array, an "endpoints" array shaped completely differently +// from the one above (byte_bus_id/ep_type/ep_enable, not stream_key/ +// byte_bus_id/priority), and a "streams" array — all feeding +// rcp_mock_server_t, a test double, via rcp_config_apply_to_mock(). That is +// a different purpose (SiL/HIL test-double configuration) from this +// loader's own (bootstrapping a real rcp::shmem::Registry ahead of/instead +// of discovery), so the two "endpoints" shapes are NOT unified here — doing +// so would silently break either this loader's existing stream_key/ +// byte_bus_id manifests or any future test-double manifest, for no shared +// benefit. Two of c-RCP's four new sections ARE ported below, additively, +// as their own top-level keys that cannot collide with "endpoints" above: +// - "server" (ServerManifest below) + apply_to_mock() — vendor_id/ +// device_id/magic/svr_implemented_options are plain rcp::mock::Server +// regmap fields (Server::registers().general), reachable with no +// mock.hpp change at all. +// - "hw_pin_map" (HwPinManifestEntry below) + apply_to_mock() — likewise +// just a rcp::regmap::RegisterMap::hw_pin_map vector, reachable the +// same way. +// NOT ported: c-RCP's "endpoints" (ep_type/ep_enable) and its +// rcp_mock_server_add_endpoint() target. rcp::mock::Server has no dynamic +// endpoint-registration API — its representative endpoint set (gpio/spi/ +// i2c/...) is fixed at compile time inside Server::dispatch() — so there is +// nothing in this tree yet for a ported apply_to_mock() to register a +// manifest-named endpoint into. Adding one is a rcp/mock.hpp change, out of +// this batch's scope. c-RCP's "streams" section is likewise not ported: +// c-RCP's own rcp_config_apply_to_mock() never applies it either (parsed +// data only, by that function's own design) — nothing to port there. #pragma once #include "avtp.hpp" +#include "mock.hpp" #include "rcp.hpp" +#include "regmap.hpp" #include "shmem.hpp" #include +#include #include #include #include @@ -58,10 +93,41 @@ struct EndpointManifestEntry { std::string extra; // opaque metadata }; +// ── ServerManifest ──────────────────────────────────────────────────────────── +// An optional top-level "server" object naming this RC Server's own +// vendor_id/device_id/magic and svr_implemented_options — see this file's +// own header comment. `magic` of 0 means "not specified": apply_to_mock() +// only overwrites Server::registers().general.magic when the manifest +// supplies a nonzero value, matching c-RCP's own rcp_config_apply_to_mock() +// (which would otherwise stomp GeneralMap's own real default with an +// absent manifest field's zero). svr_implemented_options is OR'd into the +// target's existing bits, not overwritten, for the same reason (also +// matching c-RCP). +struct ServerManifest { + uint16_t vendor_id = 0; + uint16_t device_id = 0; + uint32_t magic = 0; + uint8_t svr_implemented_options = 0; // regmap::kOpt{TimeSync,EnhCancel,Trigger,Chained,CompoundWait} +}; + +// ── HwPinManifestEntry ──────────────────────────────────────────────────────── +// One row of an optional top-level "hw_pin_map" array — field-for-field the +// same shape as regmap::HwPinMapEntry (hw_ep_nr/hw_ep_pin_nr/hw_pin_type), +// matching c-RCP's own rcp_config_hw_pin_t/rcp_regmap_hw_pin_map_entry_t +// (this file's own header comment). + +struct HwPinManifestEntry { + uint8_t hw_ep_nr = 0; + uint8_t hw_ep_pin_nr = 0; + uint8_t hw_pin_type = 0; // regmap::hw_pin::k* bitmask, named-bit array in JSON +}; + // ── Manifest ────────────────────────────────────────────────────────────────── struct Manifest { std::vector endpoints; + ServerManifest server; + std::vector hw_pin_map; }; // ── ParseError ──────────────────────────────────────────────────────────────── @@ -70,6 +136,187 @@ struct ParseError : std::runtime_error { using std::runtime_error::runtime_error; }; +namespace detail { + +// extract_str_in reads the first quoted string found at or after `start` +// within `s`. Shared by the endpoint/hw_pin_map/server parsing below — +// operates on whichever string (`json` or one object's own substring) the +// caller passes, so callers need no absolute/relative position bookkeeping. +inline std::string extract_str_in(const std::string& s, size_t start) { + auto q1 = s.find('"', start); + if (q1 == std::string::npos) throw ParseError("missing string value"); + auto q2 = s.find('"', q1 + 1); + if (q2 == std::string::npos) throw ParseError("unterminated string value"); + return s.substr(q1 + 1, q2 - q1 - 1); +} + +// extract_uint_in reads the decimal digits immediately following the ':' at +// colon_pos (skipping whitespace), stopping at the first non-digit. +inline unsigned long long extract_uint_in(const std::string& s, size_t colon_pos) { + size_t p = colon_pos + 1; + while (p < s.size() && (s[p] == ' ' || s[p] == '\t')) ++p; + size_t begin = p; + while (p < s.size() && s[p] >= '0' && s[p] <= '9') ++p; + if (p == begin) throw ParseError("missing numeric value"); + return std::stoull(s.substr(begin, p - begin)); +} + +struct NamedBit { const char* name; uint8_t bit; }; + +// or_named_bits ORs into `out` every name in `names` whose quoted form +// (e.g. "time_sync") appears anywhere within [begin, end) of `s`. An +// unrecognized name already present in the manifest is silently ignored — +// matching c-RCP's own or_named_bits_u8()/or_named_bits_options() +// leniency, not a validating parse. +inline void or_named_bits(const std::string& s, size_t begin, size_t end, uint8_t& out, + const NamedBit* names, size_t count) { + for (size_t i = 0; i < count; ++i) { + std::string needle = std::string("\"") + names[i].name + "\""; + auto pos = s.find(needle, begin); + if (pos != std::string::npos && pos < end) out |= names[i].bit; + } +} + +// find_array_span locates the '[' ... ']' span of the array value for `key` +// within `s`, starting the search at or after `key_pos` (typically just +// past a matched key). Non-nesting, matching every other array this parser +// understands (svr_implemented_options/hw_pin_type name arrays never +// themselves nest brackets). +inline bool find_array_span(const std::string& s, size_t key_pos, size_t& out_begin, size_t& out_end) { + auto lb = s.find('[', key_pos); + if (lb == std::string::npos) return false; + auto rb = s.find(']', lb); + if (rb == std::string::npos) return false; + out_begin = lb + 1; + out_end = rb; + return true; +} + +inline const NamedBit* option_bit_names() { + static const NamedBit kNames[] = { + {"time_sync", regmap::kOptTimeSync}, + {"enhanced_cancel", regmap::kOptEnhCancel}, + {"trigger", regmap::kOptTrigger}, + {"chained", regmap::kOptChained}, + {"compound_bundles", regmap::kOptCompoundWait}, + }; + return kNames; +} +constexpr size_t kOptionBitNamesLen = 5; + +inline const NamedBit* pin_prop_names() { + static const NamedBit kNames[] = { + {"pull_down", regmap::hw_pin::kPullDown}, + {"pull_up", regmap::hw_pin::kPullUp}, + {"open_drain", regmap::hw_pin::kStageOpenDrain}, + {"open_source", regmap::hw_pin::kStageOpenSource}, + {"push_pull", regmap::hw_pin::kStagePushPull}, + {"low_drive", regmap::hw_pin::kDriveLow}, + {"medium_drive", regmap::hw_pin::kDriveMedium}, + {"high_drive", regmap::hw_pin::kDriveHigh}, + {"schmitt_trigger", regmap::hw_pin::kSchmittTrigger}, + }; + return kNames; +} +constexpr size_t kPinPropNamesLen = 9; + +// parse_server_fields scans the whole document (not object-bounded — it +// names a single object, not a repeated list, same rationale c-RCP's own +// parse_server_fields() documents) for the four ServerManifest fields. +inline void parse_server_fields(const std::string& json, ServerManifest& out) { + if (auto k = json.find("\"vendor_id\""); k != std::string::npos) { + if (auto colon = json.find(':', k); colon != std::string::npos) + out.vendor_id = static_cast(extract_uint_in(json, colon)); + } + if (auto k = json.find("\"device_id\""); k != std::string::npos) { + if (auto colon = json.find(':', k); colon != std::string::npos) + out.device_id = static_cast(extract_uint_in(json, colon)); + } + if (auto k = json.find("\"magic\""); k != std::string::npos) { + if (auto colon = json.find(':', k); colon != std::string::npos) + out.magic = static_cast(extract_uint_in(json, colon)); + } + if (auto k = json.find("\"svr_implemented_options\""); k != std::string::npos) { + size_t begin = 0, end = 0; + if (find_array_span(json, k, begin, end)) + or_named_bits(json, begin, end, out.svr_implemented_options, + option_bit_names(), kOptionBitNamesLen); + } +} + +} // namespace detail + +// parse_endpoint_entry parses one endpoint-manifest object ("stream_key" + +// "byte_bus_id", the pre-existing schema above). Both keys are required — +// this function is only reached once at least one of the two is already +// known present (see parse_json's own routing below), so a call here with +// only one of them actually found is a genuinely malformed entry, not an +// object of some other kind to be silently skipped (2026-08-13 fix, +// matching a bug class c-RCP's own config.c independently found and fixed +// in its "either key routes, so the real validator can reject" entry- +// sniffing dispatch — this parser used to require BOTH keys just to be +// *routed* here at all, which meant an entry with only one of the two was +// silently dropped instead of rejected). +inline EndpointManifestEntry parse_endpoint_entry(const std::string& obj) { + EndpointManifestEntry entry; + + auto sk = obj.find("\"stream_key\""); + if (sk == std::string::npos) throw ParseError("endpoint entry missing stream_key"); + // stream_key is a decimal or "0x"-prefixed hex string. + auto sk_str = detail::extract_str_in(obj, sk + 12); + try { + size_t consumed = 0; + entry.stream_key = std::stoull(sk_str, &consumed, 0); + if (consumed != sk_str.size()) throw std::invalid_argument(sk_str); + } catch (const std::exception&) { + throw ParseError("invalid stream_key: " + sk_str); + } + + auto bk = obj.find("\"byte_bus_id\""); + if (bk == std::string::npos) throw ParseError("endpoint entry missing byte_bus_id"); + // byte_bus_id is a bare JSON number in [0, 255]. + auto bk_colon = obj.find(':', bk); + if (bk_colon == std::string::npos) throw ParseError("malformed byte_bus_id"); + auto bus_val = detail::extract_uint_in(obj, bk_colon); + if (bus_val > 0xFF) throw ParseError("byte_bus_id out of range: " + std::to_string(bus_val)); + entry.byte_bus_id = static_cast(bus_val); + + if (auto pk = obj.find("\"priority\""); pk != std::string::npos) + entry.priority = detail::extract_str_in(obj, pk + 10); + if (auto ek = obj.find("\"extra\""); ek != std::string::npos) + entry.extra = detail::extract_str_in(obj, ek + 7); + + return entry; +} + +// parse_hw_pin_entry parses one "hw_pin_map" array element. hw_ep_nr and +// hw_ep_pin_nr are required (matches c-RCP's own parse_pin_entry()); +// hw_pin_type is an optional named-bit array, OR'd bit by bit. +inline HwPinManifestEntry parse_hw_pin_entry(const std::string& obj) { + HwPinManifestEntry entry; + + auto nr = obj.find("\"hw_ep_nr\""); + if (nr == std::string::npos) throw ParseError("hw_pin_map entry missing hw_ep_nr"); + auto nr_colon = obj.find(':', nr); + if (nr_colon == std::string::npos) throw ParseError("malformed hw_ep_nr"); + entry.hw_ep_nr = static_cast(detail::extract_uint_in(obj, nr_colon)); + + auto pn = obj.find("\"hw_ep_pin_nr\""); + if (pn == std::string::npos) throw ParseError("hw_pin_map entry missing hw_ep_pin_nr"); + auto pn_colon = obj.find(':', pn); + if (pn_colon == std::string::npos) throw ParseError("malformed hw_ep_pin_nr"); + entry.hw_ep_pin_nr = static_cast(detail::extract_uint_in(obj, pn_colon)); + + if (auto pt = obj.find("\"hw_pin_type\""); pt != std::string::npos) { + size_t begin = 0, end = 0; + if (detail::find_array_span(obj, pt, begin, end)) + detail::or_named_bits(obj, begin, end, entry.hw_pin_type, + detail::pin_prop_names(), detail::kPinPropNamesLen); + } + + return entry; +} + // ── parse_json ──────────────────────────────────────────────────────────────── // Minimal JSON manifest parser (hand-rolled; no external dependency). @@ -77,24 +324,10 @@ struct ParseError : std::runtime_error { inline Manifest parse_json(const std::string& json) { Manifest m; - auto extract_str = [&](size_t start) -> std::string { - auto q1 = json.find('"', start); - if (q1 == std::string::npos) throw ParseError("missing string value"); - auto q2 = json.find('"', q1 + 1); - if (q2 == std::string::npos) throw ParseError("unterminated string value"); - return json.substr(q1 + 1, q2 - q1 - 1); - }; - - // extract_uint reads the decimal digits immediately following the ':' - // at colon_pos (skipping whitespace), stopping at the first non-digit. - auto extract_uint = [&](size_t colon_pos) -> unsigned long long { - size_t p = colon_pos + 1; - while (p < json.size() && (json[p] == ' ' || json[p] == '\t')) ++p; - size_t begin = p; - while (p < json.size() && json[p] >= '0' && json[p] <= '9') ++p; - if (p == begin) throw ParseError("missing numeric value"); - return std::stoull(json.substr(begin, p - begin)); - }; + // "server" is a single object, not a repeated list, so it is scanned + // across the whole document rather than through the object loop below + // — same rationale as c-RCP's own parse_server_fields(). + detail::parse_server_fields(json, m.server); size_t pos = 0; while ((pos = json.find('{', pos)) != std::string::npos) { @@ -102,45 +335,71 @@ inline Manifest parse_json(const std::string& json) { if (close == std::string::npos) break; std::string obj = json.substr(pos, close - pos + 1); - // Only objects carrying both "stream_key" and "byte_bus_id" are - // endpoint entries; anything else (e.g. the enclosing manifest - // object itself) is skipped. - auto sk = obj.find("\"stream_key\""); - auto bk = obj.find("\"byte_bus_id\""); - if (sk == std::string::npos || bk == std::string::npos) { pos = close + 1; continue; } - - EndpointManifestEntry entry; - - // stream_key is a decimal or "0x"-prefixed hex string. - auto sk_str = extract_str(pos + sk + 12); - try { - size_t consumed = 0; - entry.stream_key = std::stoull(sk_str, &consumed, 0); - if (consumed != sk_str.size()) throw std::invalid_argument(sk_str); - } catch (const std::exception&) { - throw ParseError("invalid stream_key: " + sk_str); + // Route by whichever repeated-entry-kind's own distinctive key(s) + // appear in this object; each kind's own parser then validates its + // own required fields are ALL actually present (an object routed + // here on the strength of just one matching key, but missing the + // other required one, is rejected, not silently skipped — see + // parse_endpoint_entry's own comment above). + if (obj.find("\"hw_ep_nr\"") != std::string::npos) { + m.hw_pin_map.push_back(parse_hw_pin_entry(obj)); + } else if (obj.find("\"stream_key\"") != std::string::npos || + obj.find("\"byte_bus_id\"") != std::string::npos) { + m.endpoints.push_back(parse_endpoint_entry(obj)); } + // Anything else (e.g. the enclosing manifest object itself, or the + // "server" object already handled above) is silently skipped. - // byte_bus_id is a bare JSON number in [0, 255]. - auto bk_colon = obj.find(':', bk); - if (bk_colon == std::string::npos) throw ParseError("malformed byte_bus_id"); - auto bus_val = extract_uint(pos + bk_colon); - if (bus_val > 0xFF) throw ParseError("byte_bus_id out of range: " + std::to_string(bus_val)); - entry.byte_bus_id = static_cast(bus_val); + pos = close + 1; + } - auto pk = obj.find("\"priority\""); - if (pk != std::string::npos) - entry.priority = extract_str(pos + pk + 10); + return m; +} - auto ek = obj.find("\"extra\""); - if (ek != std::string::npos) - entry.extra = extract_str(pos + ek + 7); +// ── apply_to_mock ───────────────────────────────────────────────────────────── +// +// Applies m's "server"/"hw_pin_map" sections to srv's own regmap — see +// this file's own header comment for why only these two of c-RCP's four +// rcp_config_apply_to_mock() sections are ported. Mirrors c-RCP's own +// merge semantics exactly: vendor_id/device_id are overwritten +// unconditionally; magic is overwritten only if the manifest supplied a +// nonzero value; svr_implemented_options is OR'd into whatever the target +// already had, not overwritten. hw_pin_map (if the manifest names any +// entries at all) replaces srv's table wholesale, capacity-checked against +// regmap::hw_pin_map::kMaxEntries — matching c-RCP's own +// RCP_MOCK_ERR_CAPACITY contract there — returned here as the standard +// std::errc::value_too_large (not regmap::HwPinMapReconfigErrc:: +// out_of_range, which names a different, wire-reconfig-write-specific +// condition; reusing that enum's out_of_range value for this unrelated +// "the manifest named more rows than this table can hold" condition would +// be exactly the kind of same-ID-different-meaning collision this audit +// pass flagged elsewhere). +inline std::error_code apply_to_mock(const Manifest& m, mock::Server& srv) { + auto& regs = srv.registers(); - m.endpoints.push_back(entry); - pos = close + 1; + regs.general.vendor_id = m.server.vendor_id; + regs.general.device_id = m.server.device_id; + if (m.server.magic != 0) regs.general.magic = m.server.magic; + regs.general.svr_implemented_options |= m.server.svr_implemented_options; + + if (!m.hw_pin_map.empty()) { + if (m.hw_pin_map.size() > regmap::hw_pin_map::kMaxEntries) + return std::make_error_code(std::errc::value_too_large); + + std::vector rows; + rows.reserve(m.hw_pin_map.size()); + for (auto& e : m.hw_pin_map) { + regmap::HwPinMapEntry row; + row.hw_ep_nr = e.hw_ep_nr; + row.hw_ep_pin_nr = e.hw_ep_pin_nr; + row.hw_pin_type = e.hw_pin_type; + rows.push_back(row); + } + regs.hw_pin_map_table.capacity = static_cast(rows.size()); + regs.hw_pin_map = std::move(rows); } - return m; + return {}; } // ── load ────────────────────────────────────────────────────────────────────── @@ -168,5 +427,13 @@ inline std::error_code load(const std::string& json, shmem::Registry& reg) { return {}; } +// load_to_mock is apply_to_mock()'s own convenience combinator — parses +// json and applies its "server"/"hw_pin_map" sections to srv in one call, +// the same parse-then-apply shape as load() above (and as c-RCP's own +// rcp_config_load()). +inline std::error_code load_to_mock(const std::string& json, mock::Server& srv) { + return apply_to_mock(parse_json(json), srv); +} + } // namespace config } // namespace rcp diff --git a/include/rcp/observe.hpp b/include/rcp/observe.hpp index f1fefa3..e839cb2 100644 --- a/include/rcp/observe.hpp +++ b/include/rcp/observe.hpp @@ -51,6 +51,13 @@ using RequestFn = std::function spans() const { std::lock_guard lk(mu_); @@ -116,6 +131,41 @@ class InMemorySink final : public MetricsSink { std::vector spans_; }; +// ── record ──────────────────────────────────────────────────────────────────── +// +// record is the single, caller-driven recording primitive analogous to +// c-RCP's rcp_observe_record() (ROADMAP.md milestone 80's "Satellite +// Package Rework" rebind of observe.h): builds a Span from caller-supplied +// name/addressing/timestamps/result and forwards it to sink, then +// increments sink's "rcp.requests.total" counter (and, iff result is set, +// "rcp.requests.errors" too) — all without requiring the call to have gone +// through an ObservingClient-wrapped RequestFn at all. A caller that drives +// its own endpoint-specific send outside RequestFn's fixed shape (e.g. +// directly against rcp::mock::Server, or from a transport this module +// doesn't itself wrap) can call this directly and supply its own span name +// and pre-measured start/end timestamps, the same caller-driven convention +// rcp/watchdog.hpp's Manager::on_request_received and rcp/deadline.hpp's +// Monitor already use. ObservingClient::request() below is now implemented +// in terms of this function rather than duplicating its body. +inline void record(const std::shared_ptr& sink, const std::string& name, + avtp::ByteBusId byte_bus_id, uint64_t stream_key, uint8_t acf_msg_type, + std::chrono::steady_clock::time_point start_time, + std::chrono::steady_clock::time_point end_time, + std::error_code result) { + Span span; + span.name = name; + span.byte_bus_id = byte_bus_id; + span.stream_key = stream_key; + span.acf_msg_type = acf_msg_type; + span.start_time = start_time; + span.end_time = end_time; + span.result = result; + + sink->record_span(span); + sink->record_counter("rcp.requests.total", stream_key, byte_bus_id, 1.0); + if (result) sink->record_counter("rcp.requests.errors", stream_key, byte_bus_id, 1.0); +} + // ── ObservingClient ─────────────────────────────────────────────────────────── class ObservingClient { @@ -135,19 +185,12 @@ class ObservingClient { const std::vector& req_payload, acf::AcfMessageInfo& out_resp, std::vector& out_resp_payload) { - Span span; - span.name = "rcp.request"; - span.byte_bus_id = req.byte_bus_id; - span.acf_msg_type = req.acf_msg_type; - span.start_time = std::chrono::steady_clock::now(); - + auto start_time = std::chrono::steady_clock::now(); auto ec = inner_(ctx, req, req_payload, out_resp, out_resp_payload); + auto end_time = std::chrono::steady_clock::now(); - span.end_time = std::chrono::steady_clock::now(); - span.result = ec; - sink_->record_span(span); - sink_->record_counter("rcp.requests.total", stream_key_, 1.0); - if (ec) sink_->record_counter("rcp.requests.errors", stream_key_, 1.0); + record(sink_, "rcp.request", req.byte_bus_id, stream_key_, req.acf_msg_type, + start_time, end_time, ec); return ec; } diff --git a/tests/test_cli.cpp b/tests/test_cli.cpp index fe03d94..6d95372 100644 --- a/tests/test_cli.cpp +++ b/tests/test_cli.cpp @@ -309,3 +309,40 @@ TEST_CASE("cli: send with an invalid --endpoint value returns invalid-args (2)", capture({"send", "--server", "0", "--endpoint", "300", "--op", "read"}, code); REQUIRE(code == rcp::cli::kInvalidArgs); } + +// ── Test-gap closure (parity audit vs c-RCP's test_cli.c) ────────────────────── + +TEST_CASE("cli: capabilities reports the tsn transport and REQ-RMAP-030 feature bits", + "[cli][conformance]") { + int code = 0; + auto s = capture({"capabilities"}, code); + REQUIRE(code == rcp::cli::kOk); + REQUIRE(s.find("\"tsn\"") != std::string::npos); + for (auto k : {"time_sync", "enhanced_cancel", "trigger", "chained", "compound_bundles"}) { + REQUIRE(s.find(std::string("\"") + k + "\"") != std::string::npos); + } +} + +TEST_CASE("cli: status default format is text", "[cli][conformance]") { + int code = 0; + auto s = capture({"status"}, code); + REQUIRE(code == rcp::cli::kOk); + REQUIRE(s.find("cpp-rcp: healthy=true") != std::string::npos); + REQUIRE(s.find("{") == std::string::npos); // not JSON +} + +TEST_CASE("cli: --format with no following value returns invalid-args (2)", + "[cli][conformance]") { + int code = 0; + capture({"version", "--format"}, code); + REQUIRE(code == rcp::cli::kInvalidArgs); +} + +TEST_CASE("cli: help/--help/-h all print usage and return OK", "[cli][conformance]") { + for (const char* spelling : {"help", "--help", "-h"}) { + int code = 0; + auto s = capture({spelling}, code); + REQUIRE(code == rcp::cli::kOk); + REQUIRE(s.find("Usage: cpp-rcp") != std::string::npos); + } +} diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 11e9c75..2d2b528 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -95,3 +95,159 @@ TEST_CASE("config: ParseError is a std::runtime_error subclass", "[config][REQ-C REQUIRE(caught); REQUIRE(std::is_base_of::value); } + +// ── Gap-closure (parity audit vs c-RCP's config.c/config.h) ─────────────────── + +TEST_CASE("config: parse_json test-gap closure — \"extra\" metadata is actually asserted", + "[config]") { + const std::string json = R"({ + "endpoints": [ { "stream_key": "0", "byte_bus_id": 1, "extra": "note-42" } ] + })"; + auto m = config::parse_json(json); + REQUIRE(m.endpoints.size() == 1); + REQUIRE(m.endpoints[0].extra == "note-42"); +} + +TEST_CASE("config: bug-fix — an endpoint entry missing byte_bus_id is now rejected, " + "not silently skipped", "[config]") { + // Previously: an object carrying only "stream_key" (or only + // "byte_bus_id") failed the old "both keys present" routing check and + // was dropped with no error at all — the same latent defect class + // c-RCP's own config.c independently found and fixed. It must now be + // rejected as malformed. + const std::string json = R"({ "endpoints": [{ "stream_key": "0" }] })"; + REQUIRE_THROWS_AS(config::parse_json(json), config::ParseError); +} + +TEST_CASE("config: bug-fix — an endpoint entry missing stream_key is now rejected, " + "not silently skipped", "[config]") { + const std::string json = R"({ "endpoints": [{ "byte_bus_id": 1 }] })"; + REQUIRE_THROWS_AS(config::parse_json(json), config::ParseError); +} + +TEST_CASE("config: parse_json parses the \"server\" block (vendor_id/device_id/magic)", + "[config]") { + const std::string json = + R"({ "server": { "vendor_id": 17, "device_id": 42, "magic": 12345 } })"; + auto m = config::parse_json(json); + REQUIRE(m.server.vendor_id == 17); + REQUIRE(m.server.device_id == 42); + REQUIRE(m.server.magic == 12345); +} + +TEST_CASE("config: parse_json parses svr_implemented_options named bits " + "(REQ-RMAP-030 five independent bits)", "[config]") { + const std::string json = + R"({ "server": { "svr_implemented_options": ["time_sync", "compound_bundles"] } })"; + auto m = config::parse_json(json); + REQUIRE((m.server.svr_implemented_options & regmap::kOptTimeSync) != 0); + REQUIRE((m.server.svr_implemented_options & regmap::kOptCompoundWait) != 0); + REQUIRE((m.server.svr_implemented_options & regmap::kOptEnhCancel) == 0); + REQUIRE((m.server.svr_implemented_options & regmap::kOptTrigger) == 0); + REQUIRE((m.server.svr_implemented_options & regmap::kOptChained) == 0); +} + +TEST_CASE("config: parse_json parses svr_implemented_options' trigger/chained bits", + "[config]") { + const std::string json = + R"({ "server": { "svr_implemented_options": ["trigger", "chained"] } })"; + auto m = config::parse_json(json); + REQUIRE((m.server.svr_implemented_options & regmap::kOptTrigger) != 0); + REQUIRE((m.server.svr_implemented_options & regmap::kOptChained) != 0); +} + +TEST_CASE("config: parse_json parses a \"hw_pin_map\" array", "[config]") { + const std::string json = R"({ + "hw_pin_map": [ + { "hw_ep_nr": 0, "hw_ep_pin_nr": 3, "hw_pin_type": ["push_pull", "pull_up"] } + ] + })"; + auto m = config::parse_json(json); + REQUIRE(m.hw_pin_map.size() == 1); + REQUIRE(m.hw_pin_map[0].hw_ep_nr == 0); + REQUIRE(m.hw_pin_map[0].hw_ep_pin_nr == 3); + REQUIRE((m.hw_pin_map[0].hw_pin_type & regmap::hw_pin::kStagePushPull) != 0); + REQUIRE((m.hw_pin_map[0].hw_pin_type & regmap::hw_pin::kPullUp) != 0); +} + +TEST_CASE("config: parse_json hw_pin_map entry missing hw_ep_pin_nr throws", "[config]") { + const std::string json = R"({ "hw_pin_map": [{ "hw_ep_nr": 0 }] })"; + REQUIRE_THROWS_AS(config::parse_json(json), config::ParseError); +} + +TEST_CASE("config: parse_json empty object parses to an all-default manifest", "[config]") { + auto m = config::parse_json("{}"); + REQUIRE(m.endpoints.empty()); + REQUIRE(m.hw_pin_map.empty()); + REQUIRE(m.server.vendor_id == 0); +} + +TEST_CASE("config: apply_to_mock writes vendor_id/device_id/magic/svr_implemented_options " + "into the mock::Server's regmap", "[config]") { + const std::string json = R"({ + "server": { "vendor_id": 7, "device_id": 9, "magic": 555, + "svr_implemented_options": ["time_sync"] } + })"; + mock::Server srv; + REQUIRE_FALSE(config::apply_to_mock(config::parse_json(json), srv)); + + REQUIRE(srv.registers().general.vendor_id == 7); + REQUIRE(srv.registers().general.device_id == 9); + REQUIRE(srv.registers().general.magic == 555); + REQUIRE((srv.registers().general.svr_implemented_options & regmap::kOptTimeSync) != 0); +} + +TEST_CASE("config: apply_to_mock preserves the existing magic when the manifest's is zero", + "[config]") { + mock::Server srv; + auto original_magic = srv.registers().general.magic; + REQUIRE(original_magic != 0); // GeneralMap defaults magic to a real, nonzero value + + REQUIRE_FALSE(config::apply_to_mock(config::parse_json(R"({ "server": { "vendor_id": 1 } })"), srv)); + REQUIRE(srv.registers().general.magic == original_magic); // untouched +} + +TEST_CASE("config: apply_to_mock ORs svr_implemented_options into existing bits, " + "never clearing what was already set", "[config]") { + mock::Server srv; + srv.registers().general.svr_implemented_options = regmap::kOptChained; + + REQUIRE_FALSE(config::apply_to_mock( + config::parse_json(R"({ "server": { "svr_implemented_options": ["time_sync"] } })"), srv)); + + REQUIRE((srv.registers().general.svr_implemented_options & regmap::kOptChained) != 0); + REQUIRE((srv.registers().general.svr_implemented_options & regmap::kOptTimeSync) != 0); +} + +TEST_CASE("config: apply_to_mock installs hw_pin_map rows into the mock::Server's regmap", + "[config]") { + const std::string json = R"({ + "hw_pin_map": [ + { "hw_ep_nr": 1, "hw_ep_pin_nr": 2, "hw_pin_type": ["schmitt_trigger"] } + ] + })"; + mock::Server srv; + REQUIRE_FALSE(config::apply_to_mock(config::parse_json(json), srv)); + + REQUIRE(srv.registers().hw_pin_map.size() == 1); + REQUIRE(srv.registers().hw_pin_map[0].hw_ep_nr == 1); + REQUIRE(srv.registers().hw_pin_map[0].hw_ep_pin_nr == 2); + REQUIRE((srv.registers().hw_pin_map[0].hw_pin_type & regmap::hw_pin::kSchmittTrigger) != 0); + REQUIRE(srv.registers().hw_pin_map_table.capacity == 1); +} + +TEST_CASE("config: apply_to_mock rejects a hw_pin_map larger than kMaxEntries", "[config]") { + config::Manifest m; + m.hw_pin_map.resize(regmap::hw_pin_map::kMaxEntries + 1); + + mock::Server srv; + auto ec = config::apply_to_mock(m, srv); + REQUIRE(ec == std::make_error_code(std::errc::value_too_large)); +} + +TEST_CASE("config: load_to_mock combines parse_json + apply_to_mock in one call", "[config]") { + const std::string json = R"({ "server": { "vendor_id": 3 } })"; + mock::Server srv; + REQUIRE_FALSE(config::load_to_mock(json, srv)); + REQUIRE(srv.registers().general.vendor_id == 3); +} diff --git a/tests/test_observe.cpp b/tests/test_observe.cpp index ebd21e5..92d08a3 100644 --- a/tests/test_observe.cpp +++ b/tests/test_observe.cpp @@ -19,38 +19,65 @@ #include #include #include +#include #include using namespace rcp; using namespace rcp::observe; namespace { -// CountingSink records counter deltas by metric name and the last span -// seen, so tests can assert on rcp.requests.total / rcp.requests.errors -// and span.result. +// CountingSink records counter deltas by metric name (and, separately, by +// (name, byte_bus_id) pair — REQ-OBS gap-closure: record_counter now +// carries byte_bus_id, not just stream_key, so a sink can distinguish two +// endpoints on the same stream), the last span seen, and every gauge +// recorded, so tests can assert on rcp.requests.total / rcp.requests.errors, +// span.result, and per-byte_bus_id granularity. class CountingSink final : public MetricsSink { public: void record_span(const Span& s) override { std::lock_guard lk(mu_); last_span_ = s; + spans_.push_back(s); } - void record_gauge(const Metric&) override {} - void record_counter(const std::string& name, uint64_t, double delta) override { + void record_gauge(const Metric& m) override { + std::lock_guard lk(mu_); + gauges_.push_back(m); + } + void record_counter(const std::string& name, uint64_t stream_key, + avtp::ByteBusId byte_bus_id, double delta) override { std::lock_guard lk(mu_); counters_[name] += delta; + per_bus_counters_[{name, byte_bus_id}] += delta; + (void)stream_key; } double counter(const std::string& name) const { std::lock_guard lk(mu_); auto it = counters_.find(name); return it == counters_.end() ? 0.0 : it->second; } + double counter_for_bus(const std::string& name, avtp::ByteBusId byte_bus_id) const { + std::lock_guard lk(mu_); + auto it = per_bus_counters_.find({name, byte_bus_id}); + return it == per_bus_counters_.end() ? 0.0 : it->second; + } Span last_span() const { std::lock_guard lk(mu_); return last_span_; } + std::vector spans() const { + std::lock_guard lk(mu_); + return spans_; + } + std::vector gauges() const { + std::lock_guard lk(mu_); + return gauges_; + } private: mutable std::mutex mu_; std::map counters_; + std::map, double> per_bus_counters_; + std::vector spans_; + std::vector gauges_; Span last_span_{}; }; @@ -168,3 +195,121 @@ TEST_CASE("observe: rcp.requests.total counter increments per request", "[observ REQUIRE(sink->counter("rcp.requests.total") == 3.0); REQUIRE(sink->counter("rcp.requests.errors") == 0.0); } + +// ── Gap-closure: Span::stream_key / Metric+record_counter::byte_bus_id ──────── +// (audit finding: c-RCP's rcp_span_t/rcp_metric_t carry the full +// rcp_avtp_addr_t (stream_id + byte_bus_id); this module previously carried +// only one half of that pair on each side — Span had byte_bus_id but not +// stream_key, Metric/record_counter had stream_key but not byte_bus_id.) + +TEST_CASE("observe: span carries the ObservingClient's stream_key", "[observe]") { + auto sink = std::make_shared(); + auto oc = new_observing_client(ok_request(), /*stream_key=*/0xC0FFEE, sink); + + auto req = standard_request(3, 1); + acf::AcfMessageInfo resp; + std::vector resp_payload; + REQUIRE_FALSE(oc->request(Context{}, req, {}, resp, resp_payload)); + + REQUIRE(sink->spans()[0].stream_key == 0xC0FFEE); +} + +TEST_CASE("observe: record_counter carries the request's byte_bus_id, " + "distinguishing endpoints on the same stream", "[observe]") { + auto sink = std::make_shared(); + auto oc = new_observing_client(ok_request(), /*stream_key=*/1, sink); + + acf::AcfMessageInfo resp; + std::vector resp_payload; + REQUIRE_FALSE(oc->request(Context{}, standard_request(5, 0), {}, resp, resp_payload)); + REQUIRE_FALSE(oc->request(Context{}, standard_request(7, 0), {}, resp, resp_payload)); + REQUIRE_FALSE(oc->request(Context{}, standard_request(5, 1), {}, resp, resp_payload)); + + REQUIRE(sink->counter("rcp.requests.total") == 3.0); + REQUIRE(sink->counter_for_bus("rcp.requests.total", 5) == 2.0); + REQUIRE(sink->counter_for_bus("rcp.requests.total", 7) == 1.0); +} + +TEST_CASE("observe: record_gauge is exercised directly and is not a dead no-op path", + "[observe]") { + auto sink = std::make_shared(); + Metric m{"rcp.queue_depth", 4.0, /*stream_key=*/1, /*byte_bus_id=*/9}; + sink->record_gauge(m); + + auto gauges = sink->gauges(); + REQUIRE(gauges.size() == 1); + REQUIRE(gauges[0].name == "rcp.queue_depth"); + REQUIRE(gauges[0].value == 4.0); + REQUIRE(gauges[0].stream_key == 1); + REQUIRE(gauges[0].byte_bus_id == 9); + + // NoopSink/InMemorySink must also tolerate a direct record_gauge() call + // without crashing (previously only exercised indirectly, if at all). + NoopSink noop; + noop.record_gauge(m); + InMemorySink mem; + mem.record_gauge(m); +} + +TEST_CASE("observe: span name is \"rcp.request\"", "[observe]") { + auto sink = std::make_shared(); + auto oc = new_observing_client(ok_request(), 1, sink); + + auto req = standard_request(1, 1); + acf::AcfMessageInfo resp; + std::vector resp_payload; + REQUIRE_FALSE(oc->request(Context{}, req, {}, resp, resp_payload)); + + REQUIRE(sink->spans()[0].name == "rcp.request"); +} + +TEST_CASE("observe: sequential spans are recorded in call order", "[observe]") { + auto sink = std::make_shared(); + auto oc = new_observing_client(ok_request(), 1, sink); + + for (avtp::ByteBusId bus : + {avtp::ByteBusId{3}, avtp::ByteBusId{1}, avtp::ByteBusId{4}, avtp::ByteBusId{1}, avtp::ByteBusId{5}}) { + auto req = standard_request(bus, 0); + acf::AcfMessageInfo resp; + std::vector resp_payload; + REQUIRE_FALSE(oc->request(Context{}, req, {}, resp, resp_payload)); + } + + auto spans = sink->spans(); + REQUIRE(spans.size() == 5); + std::vector expected{3, 1, 4, 1, 5}; + for (size_t i = 0; i < spans.size(); ++i) { + REQUIRE(spans[i].byte_bus_id == expected[i]); + } +} + +// ── Gap-closure: standalone record() (analogous to c-RCP's rcp_observe_record()) + +TEST_CASE("observe: record() records a span/counters directly, with no " + "ObservingClient/RequestFn involved", "[observe]") { + auto sink = std::make_shared(); + + auto start = std::chrono::steady_clock::now(); + auto end = start + std::chrono::microseconds(5); + record(sink, "custom.span", /*byte_bus_id=*/5, /*stream_key=*/99, + acf::kAcfMsgTypeGbb, start, end, std::error_code{}); + + auto s = sink->last_span(); + REQUIRE(s.name == "custom.span"); + REQUIRE(s.byte_bus_id == 5); + REQUIRE(s.stream_key == 99); + REQUIRE(s.acf_msg_type == acf::kAcfMsgTypeGbb); + REQUIRE_FALSE(s.result); + REQUIRE(sink->counter("rcp.requests.total") == 1.0); + REQUIRE(sink->counter("rcp.requests.errors") == 0.0); +} + +TEST_CASE("observe: record() increments the error counter when result is set", "[observe]") { + auto sink = std::make_shared(); + auto now = std::chrono::steady_clock::now(); + record(sink, "custom.span", 5, 99, acf::kAcfMsgTypeAbb, now, now, ErrClosed); + + REQUIRE(sink->last_span().result == ErrClosed); + REQUIRE(sink->counter("rcp.requests.total") == 1.0); + REQUIRE(sink->counter("rcp.requests.errors") == 1.0); +} diff --git a/tests/test_record.cpp b/tests/test_record.cpp index 1a67afe..a2601bd 100644 --- a/tests/test_record.cpp +++ b/tests/test_record.cpp @@ -191,3 +191,88 @@ TEST_CASE("record: Playback::run_all replays every entry against the target", REQUIRE_FALSE(pb.run_all(Context{})); REQUIRE(replayed == 3); } + +// ── Test-gap closure (parity audit vs c-RCP's recorder.c/test_recorder.c) ───── + +TEST_CASE("record: write_binary returns io_error for an unopenable path", "[record][REQ-REC-003]") { + auto rec = std::make_shared(); + auto rc = new_recording_client(echo_request(), rec); + auto req = standard_request(1, 0); + acf::AcfMessageInfo resp; + std::vector resp_payload; + auto ec = rc->request(Context{}, req, {}, resp, resp_payload); + (void)ec; + + // A path through a directory that does not exist can never be opened + // for writing — matches c-RCP's own + // test_write_binary_returns_busy_when_path_unopenable (test_recorder.c). + auto bad_path = (std::filesystem::temp_directory_path() / + "rcp_test_record_no_such_dir" / "x" / "y.bin").string(); + REQUIRE(rec->write_binary(bad_path) == std::make_error_code(std::errc::io_error)); +} + +TEST_CASE("record: read_binary returns io_error for a missing file", "[record][REQ-REC-003]") { + Record rec; + auto path = (std::filesystem::temp_directory_path() / + "rcp_test_record_definitely_missing.bin").string(); + std::filesystem::remove(path); // ensure it really doesn't exist + REQUIRE(rec.read_binary(path) == std::make_error_code(std::errc::io_error)); + REQUIRE(rec.size() == 0); +} + +TEST_CASE("record: stored entries are immune to caller mutation after request() returns", + "[record]") { + auto rec = std::make_shared(); + auto rc = new_recording_client(echo_request({0xAA}), rec); + + auto req = standard_request(1, 0); + std::vector req_payload{0x01, 0x02}; + acf::AcfMessageInfo resp; + std::vector resp_payload; + auto ec = rc->request(Context{}, req, req_payload, resp, resp_payload); + (void)ec; + + // Mutate the caller's own copies after the call returns. + req.transaction_num = 0xFF; + req_payload.push_back(0xEE); + resp_payload.push_back(0xDD); + + auto stored = rec->entries(); + REQUIRE(stored[0].request.transaction_num == 0); + REQUIRE(stored[0].request_payload == std::vector{0x01, 0x02}); + REQUIRE(stored[0].response_payload == std::vector{0xAA}); +} + +TEST_CASE("record: Playback::run_all actually sleeps to honor a positive inter-entry gap", + "[record][REQ-REC-004]") { + // Build two entries directly (bypassing wall-clock timing) with a + // controlled 5ms gap, well above run_all()'s own >1ms sleep threshold — + // exercises the `gap_ns > 0 && cfg_.speed_factor > 0.0` branch that the + // existing speed_factor=0.0 playback test above never takes. + Record rec; + Entry e0; e0.timestamp_ns = 0; e0.request = standard_request(1, 0); + Entry e1; e1.timestamp_ns = 5'000'000; e1.request = standard_request(1, 1); // +5ms + rec.append(e0); + rec.append(e1); + + int replayed = 0; + RequestFn target = [&](const Context&, const acf::AcfMessageInfo&, + const std::vector&, acf::AcfMessageInfo&, + std::vector&) { + ++replayed; + return std::error_code{}; + }; + + auto start = std::chrono::steady_clock::now(); + Playback pb(target, rec, PlaybackConfig{/*speed_factor=*/1.0}); + REQUIRE_FALSE(pb.run_all(Context{})); + auto elapsed = std::chrono::steady_clock::now() - start; + + REQUIRE(replayed == 2); + REQUIRE(elapsed >= std::chrono::milliseconds(5)); +} + +TEST_CASE("record: PlaybackConfig{} defaults speed_factor to 1.0", "[record]") { + PlaybackConfig cfg; + REQUIRE(cfg.speed_factor == 1.0); +}