From c7a965cb1e80c794e319c06b4f9f7d99611220ad Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Tue, 30 Jun 2026 18:34:57 +0200 Subject: [PATCH 1/6] Add asset-identity model and merge-by-identity Add an AssetIdentity nameplate (manufacturer, model, serial, hardware/ firmware/software version, network endpoint, role) plus an extensible key-value map to the Component entity, AAS Nameplate-aligned. Merge identity across sources by a configurable precedence with per-field provenance, integrated into the discovery merge pipeline. JSON is backward compatible: identity is emitted under x-medkit only when set. Refs #482 --- src/ros2_medkit_gateway/CMakeLists.txt | 4 + src/ros2_medkit_gateway/README.md | 55 ++++ .../core/discovery/identity_merge.hpp | 235 +++++++++++++++ .../core/discovery/models/asset_identity.hpp | 173 +++++++++++ .../core/discovery/models/component.hpp | 7 +- .../discovery/merge_pipeline.hpp | 12 + .../src/discovery/merge_pipeline.cpp | 21 +- .../test/test_asset_identity.cpp | 277 ++++++++++++++++++ .../test/test_merge_pipeline.cpp | 67 +++++ 9 files changed, 847 insertions(+), 4 deletions(-) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp create mode 100644 src/ros2_medkit_gateway/test/test_asset_identity.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 64216edeb..655dfb2e1 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -642,6 +642,10 @@ if(BUILD_TESTING) ament_add_gtest(test_merge_pipeline test/test_merge_pipeline.cpp) target_link_libraries(test_merge_pipeline gateway_ros2) + # Add asset-identity model + merge tests + ament_add_gtest(test_asset_identity test/test_asset_identity.cpp) + target_link_libraries(test_asset_identity gateway_ros2) + # Add capability builder tests ament_add_gtest(test_capability_builder test/test_capability_builder.cpp) target_link_libraries(test_capability_builder gateway_ros2) diff --git a/src/ros2_medkit_gateway/README.md b/src/ros2_medkit_gateway/README.md index 0e94bb882..36746f397 100644 --- a/src/ros2_medkit_gateway/README.md +++ b/src/ros2_medkit_gateway/README.md @@ -1544,6 +1544,61 @@ Runtime mode mapping: /standalone_node -> Component: , App: standalone_node ``` +### Asset Identity (nameplate) and merge-by-identity + +Each Component can carry an **asset identity**: the stable nameplate of the physical +or logical asset it represents (an ECU, PLC, drive, sensor), separate from its runtime +ROS 2 footprint. The model lives in `core/discovery/models/asset_identity.hpp`. + +Typed fields (queryable, AAS-mappable) plus a generic `extra` key-value map for +vendor-specific attributes we do not model up front: + +| Field | JSON key | AAS Nameplate (IDTA 02006) | +|-------|----------|----------------------------| +| `manufacturer` | `manufacturer` | ManufacturerName | +| `model` (order code) | `model` | ManufacturerProductDesignation | +| `serial_number` | `serialNumber` | SerialNumber | +| `hardware_revision` | `hardwareRevision` | HardwareVersion | +| `firmware_version` | `firmwareVersion` | FirmwareVersion | +| `software_version` | `softwareVersion` | SoftwareVersion | +| `network_endpoint` | `networkEndpoint` | Asset Interface Description (AID) endpoint | +| `role` | `role` | functional role (BoM/usage context) | +| `extra` | `extra` | additional Nameplate properties | + +The whole block maps to an Asset Administration Shell later without rework: the asset +itself is the shell (`Component.id` as localId), this block is the **Nameplate** submodel, +the Component hierarchy is the **BoM** submodel, and `network_endpoint` is an **AID** entry. + +Identity is emitted under `x-medkit.identity` only when populated, so identity-less +components keep their existing JSON unchanged. + +**Why typed fields and a map?** Typed fields are the common, queryable, standard-mappable +attributes; the `extra` map is the escape hatch for vendor keys (rack/slot, MAC, asset tag) +that do not warrant a schema change. Both merge with the same rules. + +**Merge-by-identity with provenance.** A single asset is usually described by several +sources (a hand-authored manifest, a live protocol device-info read, runtime discovery). +`merge_identity()` (`core/discovery/identity_merge.hpp`) combines them field by field and +records **per-field provenance** (which source set each field) under `_provenance`. + +- **Precedence** is config-driven (`IdentityMergeConfig::source_precedence`, highest first) + and is deliberately **decoupled from the structural `MergePolicy`**: a manifest can be the + authoritative *structure* source while a live protocol read is the authoritative *identity* + source. Default order: protocol device-info (`opcua`, `s7`, `ethernet_ip`, `modbus`, `ads`, + `profinet`) > `manifest` > `config` > runtime sources. A higher-authority source overrides a + field; lower-authority sources only fill gaps; unknown sources rank lowest. Empty values never + overwrite. +- **Identity key.** `compute_identity_key()` derives the key that decides whether two records + describe the same asset. Strategies: `serial`, `order_code_slot` (`model` + `extra["slot"]`), + `endpoint`, `configured_id`, and `auto` (serial -> order-code+slot -> endpoint -> configured id). + The discovery pipeline merges Components by `Component.id`, which is the `configured_id` + strategy; the other strategies let sources that assign ids (CSV/manifest import, protocol + probes) correlate records that arrive under different ids before they reach the pipeline. + +The pipeline calls `merge_identity` inside the `IDENTITY` field group for Components, seeding +provenance with the base (highest-priority) layer. Configure via +`MergePipeline::set_identity_merge_config()`. + ## Demo Nodes The package includes demo automotive nodes for testing: diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp new file mode 100644 index 000000000..5cd8047f2 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp @@ -0,0 +1,235 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include "ros2_medkit_gateway/core/discovery/models/asset_identity.hpp" + +#include +#include +#include +#include +#include +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief How the identity key for an asset is derived. + * + * The identity key is what decides whether two records describe the *same* asset + * and should therefore have their identity merged. + */ +enum class IdentityKeyStrategy { + AUTO, ///< serial -> model+slot -> endpoint -> configured id (first that resolves) + SERIAL, ///< serial number only + ORDER_CODE_SLOT, ///< model (order code) + extra["slot"] + ENDPOINT, ///< network endpoint + CONFIGURED_ID ///< the configured Component id (always available, never cross-source) +}; + +inline std::optional identity_key_strategy_from_string(const std::string & s) { + if (s == "auto") { + return IdentityKeyStrategy::AUTO; + } + if (s == "serial") { + return IdentityKeyStrategy::SERIAL; + } + if (s == "order_code_slot") { + return IdentityKeyStrategy::ORDER_CODE_SLOT; + } + if (s == "endpoint") { + return IdentityKeyStrategy::ENDPOINT; + } + if (s == "configured_id") { + return IdentityKeyStrategy::CONFIGURED_ID; + } + return std::nullopt; +} + +/** + * @brief Configuration for identity merging. + * + * `source_precedence` lists source names from highest to lowest authority. A source + * not in the list is treated as the lowest authority (it can still fill empty fields + * but never overrides a known source). Identity authority is deliberately decoupled + * from the structural merge policy: a manifest may be the authoritative *structure* + * source while a live protocol read is the authoritative *identity* source. + * + * Default precedence (highest first): live protocol device-info beats the hand + * authored manifest, which beats whatever runtime discovery guessed. + */ +struct IdentityMergeConfig { + std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", + "ads", "profinet", "manifest", "config", + "runtime", "node", "heuristic"}; + IdentityKeyStrategy key_strategy{IdentityKeyStrategy::AUTO}; +}; + +/** + * @brief Rank of a source: lower number = higher authority. Unknown sources rank + * just below every listed source (all unknowns share the same lowest rank). + */ +inline size_t source_rank(const std::string & source, const IdentityMergeConfig & config) { + for (size_t i = 0; i < config.source_precedence.size(); ++i) { + if (config.source_precedence[i] == source) { + return i; + } + } + return config.source_precedence.size(); +} + +/** + * @brief Compute the identity key for an asset. + * @param identity The (possibly partial) asset identity. + * @param configured_id Fallback stable id (typically Component.id). + * @param strategy Key derivation strategy. + * @return A non-empty identity key, or empty string if the chosen strategy cannot + * resolve one (e.g. SERIAL strategy on an asset with no serial). + */ +inline std::string compute_identity_key(const AssetIdentity & identity, const std::string & configured_id, + IdentityKeyStrategy strategy) { + auto slot = [&]() -> std::string { + auto it = identity.extra.find("slot"); + return it != identity.extra.end() ? it->second : std::string{}; + }; + switch (strategy) { + case IdentityKeyStrategy::SERIAL: + return identity.serial_number; + case IdentityKeyStrategy::ORDER_CODE_SLOT: + if (identity.model.empty()) { + return std::string{}; + } + return identity.model + "/" + slot(); + case IdentityKeyStrategy::ENDPOINT: + return identity.network_endpoint; + case IdentityKeyStrategy::CONFIGURED_ID: + return configured_id; + case IdentityKeyStrategy::AUTO: + default: + if (!identity.serial_number.empty()) { + return "serial:" + identity.serial_number; + } + if (!identity.model.empty() && !slot().empty()) { + return "ordercode:" + identity.model + "/" + slot(); + } + if (!identity.network_endpoint.empty()) { + return "endpoint:" + identity.network_endpoint; + } + return "id:" + configured_id; + } +} + +namespace detail { + +/// All typed identity fields as (provenance-key, member-pointer) pairs. +inline const std::array, 8> & identity_fields() { + static const std::array, 8> fields{{ + {"manufacturer", &AssetIdentity::manufacturer}, + {"model", &AssetIdentity::model}, + {"serial_number", &AssetIdentity::serial_number}, + {"hardware_revision", &AssetIdentity::hardware_revision}, + {"firmware_version", &AssetIdentity::firmware_version}, + {"software_version", &AssetIdentity::software_version}, + {"network_endpoint", &AssetIdentity::network_endpoint}, + {"role", &AssetIdentity::role}, + }}; + return fields; +} + +/// Decide whether an incoming value should overwrite the current field value. +/// @param field_set Whether the target field already holds a value. +/// @param current_owner Source that owns the current value ("" if unstamped/unknown). +inline bool incoming_wins(bool field_set, const std::string & current_owner, const std::string & incoming_source, + const IdentityMergeConfig & config) { + if (!field_set) { + return true; // field unset -> fill it + } + // Unstamped existing value -> treat its owner as the lowest authority. + const size_t current_rank = + current_owner.empty() ? config.source_precedence.size() : source_rank(current_owner, config); + // Strictly higher authority (lower rank) wins; ties keep the existing value. + return source_rank(incoming_source, config) < current_rank; +} + +} // namespace detail + +/** + * @brief Stamp provenance for every populated typed field / extra of `identity` + * to `source`, unless that field already has a provenance entry. + * + * Used to seed provenance for the first (base) source before merging others. + */ +inline void stamp_identity_provenance(AssetIdentity & identity, const std::string & source) { + for (const auto & [prov_key, member] : detail::identity_fields()) { + if (!(identity.*member).empty() && identity.provenance.find(prov_key) == identity.provenance.end()) { + identity.provenance[prov_key] = source; + } + } + for (const auto & [key, value] : identity.extra) { + const std::string prov_key = "extra." + key; + if (!value.empty() && identity.provenance.find(prov_key) == identity.provenance.end()) { + identity.provenance[prov_key] = source; + } + } +} + +/** + * @brief Merge `source` identity (tagged `source_name`) into `target` in place. + * + * For each field: if the target field is unset, take the incoming value; otherwise + * the incoming value wins only if `source_name` has strictly higher authority than + * the source currently owning the field (per `config.source_precedence`). Provenance + * is updated whenever a value is written. Empty incoming values never overwrite. + * + * `target` should have had ::stamp_identity_provenance called on it (directly or via + * a previous merge) so existing fields carry provenance; otherwise existing fields + * are treated as owned by an unknown (lowest authority) source. + */ +inline void merge_identity(AssetIdentity & target, const AssetIdentity & source, const std::string & source_name, + const IdentityMergeConfig & config) { + for (const auto & [prov_key, member] : detail::identity_fields()) { + const std::string & incoming = source.*member; + if (incoming.empty()) { + continue; + } + auto prov_it = target.provenance.find(prov_key); + const std::string current_owner = prov_it != target.provenance.end() ? prov_it->second : std::string{}; + const bool target_set = !(target.*member).empty(); + if (detail::incoming_wins(target_set, current_owner, source_name, config)) { + target.*member = incoming; + target.provenance[prov_key] = source_name; + } + } + + for (const auto & [key, value] : source.extra) { + if (value.empty()) { + continue; + } + const std::string prov_key = "extra." + key; + auto prov_it = target.provenance.find(prov_key); + const std::string current_owner = prov_it != target.provenance.end() ? prov_it->second : std::string{}; + auto extra_it = target.extra.find(key); + const bool target_set = extra_it != target.extra.end() && !extra_it->second.empty(); + if (detail::incoming_wins(target_set, current_owner, source_name, config)) { + target.extra[key] = value; + target.provenance[prov_key] = source_name; + } + } +} + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp new file mode 100644 index 000000000..778b37c25 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp @@ -0,0 +1,173 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include + +#include +#include + +namespace ros2_medkit_gateway { + +using json = nlohmann::json; + +/** + * @brief Asset-identity (nameplate) of a physical or logical asset behind a Component. + * + * Captures the stable identity of the thing a Component represents (an ECU, a PLC, + * a drive, a sensor) as opposed to its runtime ROS 2 footprint (topics/services). + * One source rarely knows everything: a manifest may carry manufacturer + role, a + * protocol device-info read (OPC UA nameplate, S7 order code, EtherNet/IP Identity + * Object) fills in serial and firmware. The fields below are merged across sources + * by ::merge_identity, which records per-field provenance. + * + * Design: typed fields AND a generic `extra` map are intentional. Typed fields are + * the common, queryable, AAS-mappable identity attributes; `extra` is the escape + * hatch for vendor-specific keys we do not want to model up front (rack/slot, + * MAC address, asset tag, ...). Both are merged with the same precedence rules. + * + * AAS alignment (IEC 63278) so the model maps cleanly to/from an Asset + * Administration Shell without rework: + * - the asset itself <-> the AAS / Submodel "shell" (Component.id == localId) + * - this identity block <-> Nameplate submodel (IDTA 02006) + * manufacturer <-> ManufacturerName + * model <-> ManufacturerProductDesignation (a.k.a. order code) + * serial_number <-> SerialNumber + * hardware_revision <-> HardwareVersion + * firmware_version <-> FirmwareVersion + * software_version <-> SoftwareVersion + * network_endpoint <-> Asset Interface Description (AID) endpoint + * role <-> functional role (BoM/usage context) + * extra <-> additional Nameplate properties + * - Component hierarchy <-> Bill of Material (BoM) submodel + */ +struct AssetIdentity { + std::string manufacturer; ///< Manufacturer / vendor name (AAS ManufacturerName) + std::string model; ///< Product designation / order code (AAS ManufacturerProductDesignation) + std::string serial_number; ///< Unit serial number (AAS SerialNumber) + std::string hardware_revision; ///< Hardware revision (AAS HardwareVersion) + std::string firmware_version; ///< Firmware version (AAS FirmwareVersion) + std::string software_version; ///< Software/application version (AAS SoftwareVersion) + std::string network_endpoint; ///< Network endpoint, e.g. "opc.tcp://host:4840" (AAS AID) + std::string role; ///< Functional role of the asset (e.g. "plc", "drive") + + std::map extra; ///< Vendor-specific extras (rack/slot, MAC, asset tag, ...) + + /// Per-field source provenance: field name -> source that set it. + /// Typed fields use their snake_case names ("serial_number"); `extra` entries + /// use the key prefixed with "extra." ("extra.slot"). + std::map provenance; + + /// True when no identity information is present (typed fields + extras all empty). + /// Provenance alone does not make an identity non-empty. + bool empty() const { + return manufacturer.empty() && model.empty() && serial_number.empty() && hardware_revision.empty() && + firmware_version.empty() && software_version.empty() && network_endpoint.empty() && role.empty() && + extra.empty(); + } + + /** + * @brief Serialize to JSON (camelCase keys, only non-empty fields emitted). + * + * Provenance is emitted under "_provenance" when present so consumers can audit + * which source set each field. Returns an empty object when ::empty(). + */ + json to_json() const { + json j = json::object(); + if (!manufacturer.empty()) { + j["manufacturer"] = manufacturer; + } + if (!model.empty()) { + j["model"] = model; + } + if (!serial_number.empty()) { + j["serialNumber"] = serial_number; + } + if (!hardware_revision.empty()) { + j["hardwareRevision"] = hardware_revision; + } + if (!firmware_version.empty()) { + j["firmwareVersion"] = firmware_version; + } + if (!software_version.empty()) { + j["softwareVersion"] = software_version; + } + if (!network_endpoint.empty()) { + j["networkEndpoint"] = network_endpoint; + } + if (!role.empty()) { + j["role"] = role; + } + if (!extra.empty()) { + j["extra"] = extra; + } + if (!provenance.empty()) { + j["_provenance"] = provenance; + } + return j; + } + + /** + * @brief Parse from JSON produced by ::to_json (camelCase keys). Tolerant of + * missing keys and of a null/empty object. + */ + static AssetIdentity from_json(const json & j) { + AssetIdentity id; + if (!j.is_object()) { + return id; + } + auto get_str = [&](const char * key, std::string & dst) { + auto it = j.find(key); + if (it != j.end() && it->is_string()) { + dst = it->get(); + } + }; + get_str("manufacturer", id.manufacturer); + get_str("model", id.model); + get_str("serialNumber", id.serial_number); + get_str("hardwareRevision", id.hardware_revision); + get_str("firmwareVersion", id.firmware_version); + get_str("softwareVersion", id.software_version); + get_str("networkEndpoint", id.network_endpoint); + get_str("role", id.role); + if (auto it = j.find("extra"); it != j.end() && it->is_object()) { + for (auto & [k, v] : it->items()) { + if (v.is_string()) { + id.extra[k] = v.get(); + } + } + } + if (auto it = j.find("_provenance"); it != j.end() && it->is_object()) { + for (auto & [k, v] : it->items()) { + if (v.is_string()) { + id.provenance[k] = v.get(); + } + } + } + return id; + } +}; + +inline bool operator==(const AssetIdentity & a, const AssetIdentity & b) { + return a.manufacturer == b.manufacturer && a.model == b.model && a.serial_number == b.serial_number && + a.hardware_revision == b.hardware_revision && a.firmware_version == b.firmware_version && + a.software_version == b.software_version && a.network_endpoint == b.network_endpoint && a.role == b.role && + a.extra == b.extra && a.provenance == b.provenance; +} +inline bool operator!=(const AssetIdentity & a, const AssetIdentity & b) { + return !(a == b); +} + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp index 62bf0d9f3..f1b34499e 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp @@ -14,6 +14,7 @@ #pragma once +#include "ros2_medkit_gateway/core/discovery/models/asset_identity.hpp" #include "ros2_medkit_gateway/core/discovery/models/common.hpp" #include @@ -50,6 +51,7 @@ struct Component { std::vector actions; ///< Actions exposed by this component ComponentTopics topics; ///< Topics this component publishes/subscribes std::optional host_metadata; ///< Host system metadata (for runtime default component) + AssetIdentity identity; ///< Asset-identity nameplate (merged across sources, per-field provenance) /** * @brief Convert to JSON representation @@ -94,6 +96,9 @@ struct Component { if (host_metadata.has_value()) { x_medkit["host"] = host_metadata.value(); } + if (!identity.empty()) { + x_medkit["identity"] = identity.to_json(); + } j["x-medkit"] = x_medkit; // Add operations array combining services and actions @@ -178,7 +183,7 @@ inline bool operator==(const Component & a, const Component & b) { a.description == b.description && a.variant == b.variant && a.tags == b.tags && a.parent_component_id == b.parent_component_id && a.depends_on == b.depends_on && a.contributors == b.contributors && a.services == b.services && a.actions == b.actions && - a.topics == b.topics && a.host_metadata == b.host_metadata; + a.topics == b.topics && a.host_metadata == b.host_metadata && a.identity == b.identity; } inline bool operator!=(const Component & a, const Component & b) { return !(a == b); diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp index 4d2db9ac9..91a670171 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp @@ -15,6 +15,7 @@ #pragma once #include "ros2_medkit_gateway/core/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/core/discovery/identity_merge.hpp" #include "ros2_medkit_gateway/core/discovery/manifest/manifest.hpp" #include "ros2_medkit_gateway/core/discovery/merge_types.hpp" #include "ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp" @@ -57,6 +58,16 @@ class MergePipeline { */ void add_layer(std::unique_ptr layer); + /** + * @brief Set the asset-identity merge configuration (precedence + key strategy). + * + * Identity authority is independent of the structural MergePolicy: this controls + * which source wins per identity field and how the identity key is derived. + */ + void set_identity_merge_config(IdentityMergeConfig config) { + identity_config_ = std::move(config); + } + /** * @brief Execute all layers and merge results * @return Merged entities with diagnostics report @@ -97,6 +108,7 @@ class MergePipeline { rclcpp::Logger logger_; std::vector> layers_; + IdentityMergeConfig identity_config_; MergeReport last_report_; std::unique_ptr linker_; ManifestConfig manifest_config_; diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index 745da4dcc..4641fe302 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -168,9 +168,14 @@ void merge_topics(ComponentTopics & target, const ComponentTopics & source, Merg merge_collection(target.subscribes, source.subscribes, winner); } -// Per-entity-type field-group merge dispatch +// Per-entity-type field-group merge dispatch. +// `source_name` and `id_cfg` are only consumed by the Component identity merge; +// they are accepted for all entity types so the caller can stay generic. template -void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup group, const MergeResolution & res) { +void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup group, const MergeResolution & res, + const std::string & source_name, const IdentityMergeConfig & id_cfg) { + (void)source_name; + (void)id_cfg; if constexpr (std::is_same_v) { switch (group) { case FieldGroup::IDENTITY: @@ -198,6 +203,9 @@ void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup merge_scalar(target.translation_id, source.translation_id, res.scalar); merge_scalar(target.description, source.description, res.scalar); merge_collection(target.tags, source.tags, res.collection); + // Asset identity has its own per-field precedence (config-driven, decoupled + // from the structural MergePolicy) and records provenance per field. + merge_identity(target.identity, source.identity, source_name, id_cfg); break; case FieldGroup::HIERARCHY: merge_scalar(target.namespace_path, source.namespace_path, res.scalar); @@ -342,6 +350,12 @@ std::vector MergePipeline::merge_entities(std::vectorname(); + // Seed asset-identity provenance with the base layer so subsequent identity + // merges can compare authority per field. + if constexpr (std::is_same_v) { + stamp_identity_provenance(merged.identity, layers_[owner_layer_idx]->name()); + } + // Track current owning layer per field group (initially all owned by first layer) std::array fg_owner; fg_owner.fill(owner_layer_idx); @@ -363,7 +377,8 @@ std::vector MergePipeline::merge_entities(std::vectorname(), + identity_config_); // If source won with a strictly higher-priority policy, it becomes // the owner of this field group for subsequent merge comparisons. diff --git a/src/ros2_medkit_gateway/test/test_asset_identity.cpp b/src/ros2_medkit_gateway/test/test_asset_identity.cpp new file mode 100644 index 000000000..45df7ed4b --- /dev/null +++ b/src/ros2_medkit_gateway/test/test_asset_identity.cpp @@ -0,0 +1,277 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "ros2_medkit_gateway/core/discovery/identity_merge.hpp" +#include "ros2_medkit_gateway/core/discovery/models/asset_identity.hpp" +#include "ros2_medkit_gateway/core/discovery/models/component.hpp" + +#include + +using ros2_medkit_gateway::AssetIdentity; +using ros2_medkit_gateway::Component; +using namespace ros2_medkit_gateway::discovery; + +// --------------------------------------------------------------------------- +// AssetIdentity model +// --------------------------------------------------------------------------- + +TEST(AssetIdentityModel, DefaultIsEmpty) { + AssetIdentity id; + EXPECT_TRUE(id.empty()); + EXPECT_TRUE(id.to_json().empty()); // empty JSON object +} + +TEST(AssetIdentityModel, ProvenanceAloneDoesNotMakeItNonEmpty) { + AssetIdentity id; + id.provenance["serial_number"] = "opcua"; + EXPECT_TRUE(id.empty()); +} + +TEST(AssetIdentityModel, ToJsonOnlyEmitsNonEmptyFields) { + AssetIdentity id; + id.manufacturer = "Siemens"; + id.serial_number = "SN-42"; + id.extra["slot"] = "3"; + id.provenance["manufacturer"] = "manifest"; + + auto j = id.to_json(); + EXPECT_EQ(j["manufacturer"], "Siemens"); + EXPECT_EQ(j["serialNumber"], "SN-42"); + EXPECT_EQ(j["extra"]["slot"], "3"); + EXPECT_EQ(j["_provenance"]["manufacturer"], "manifest"); + EXPECT_FALSE(j.contains("model")); + EXPECT_FALSE(j.contains("firmwareVersion")); +} + +TEST(AssetIdentityModel, JsonRoundTrip) { + AssetIdentity id; + id.manufacturer = "Siemens"; + id.model = "6ES7"; + id.serial_number = "SN-42"; + id.hardware_revision = "A2"; + id.firmware_version = "2.9.4"; + id.software_version = "1.0"; + id.network_endpoint = "opc.tcp://192.168.1.10:4840"; + id.role = "plc"; + id.extra["mac"] = "00:11:22:33:44:55"; + id.provenance["serial_number"] = "opcua"; + + AssetIdentity parsed = AssetIdentity::from_json(id.to_json()); + EXPECT_EQ(parsed, id); +} + +TEST(AssetIdentityModel, FromJsonTolerantOfNonObject) { + EXPECT_TRUE(AssetIdentity::from_json(nlohmann::json(nullptr)).empty()); + EXPECT_TRUE(AssetIdentity::from_json(nlohmann::json("oops")).empty()); +} + +// --------------------------------------------------------------------------- +// Backward compatibility of the Component DTO / JSON +// --------------------------------------------------------------------------- + +TEST(ComponentIdentityCompat, NoIdentityKeyWhenEmpty) { + Component c; + c.id = "motor_controller"; + c.name = "motor_controller"; + c.namespace_path = "/powertrain"; + c.fqn = "/powertrain/motor_controller"; + + auto j = c.to_json(); + ASSERT_TRUE(j.contains("x-medkit")); + // Existing consumers must not see a new "identity" key for an identity-less component. + EXPECT_FALSE(j["x-medkit"].contains("identity")); + // Existing fields untouched. + EXPECT_EQ(j["x-medkit"]["fqn"], "/powertrain/motor_controller"); +} + +TEST(ComponentIdentityCompat, IdentityEmittedUnderXMedkitWhenPresent) { + Component c; + c.id = "plc_1"; + c.identity.manufacturer = "Siemens"; + c.identity.serial_number = "SN-42"; + c.identity.provenance["manufacturer"] = "manifest"; + + auto j = c.to_json(); + ASSERT_TRUE(j["x-medkit"].contains("identity")); + EXPECT_EQ(j["x-medkit"]["identity"]["manufacturer"], "Siemens"); + EXPECT_EQ(j["x-medkit"]["identity"]["serialNumber"], "SN-42"); + EXPECT_EQ(j["x-medkit"]["identity"]["_provenance"]["manufacturer"], "manifest"); +} + +// --------------------------------------------------------------------------- +// Identity key derivation +// --------------------------------------------------------------------------- + +TEST(IdentityKey, AutoPrefersSerial) { + AssetIdentity id; + id.serial_number = "SN-42"; + id.network_endpoint = "opc.tcp://h:4840"; + EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::AUTO), "serial:SN-42"); +} + +TEST(IdentityKey, AutoFallsBackThroughOrderCodeEndpointConfiguredId) { + AssetIdentity id; + id.model = "6ES7"; + id.extra["slot"] = "3"; + EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::AUTO), "ordercode:6ES7/3"); + + AssetIdentity ep; + ep.network_endpoint = "opc.tcp://h:4840"; + EXPECT_EQ(compute_identity_key(ep, "cfg", IdentityKeyStrategy::AUTO), "endpoint:opc.tcp://h:4840"); + + AssetIdentity none; + EXPECT_EQ(compute_identity_key(none, "cfg", IdentityKeyStrategy::AUTO), "id:cfg"); +} + +TEST(IdentityKey, ExplicitStrategiesReturnEmptyWhenUnresolvable) { + AssetIdentity id; // no serial, no model + EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::SERIAL), ""); + EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::ORDER_CODE_SLOT), ""); + EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::CONFIGURED_ID), "cfg"); +} + +// --------------------------------------------------------------------------- +// merge_identity: precedence + per-field provenance + extensible map +// --------------------------------------------------------------------------- + +TEST(MergeIdentity, MergesTwoSourcesWithProvenancePerField) { + IdentityMergeConfig cfg; // default: opcua outranks manifest + + // Base: manifest knows manufacturer + role. + AssetIdentity merged; + merged.manufacturer = "Siemens"; + merged.role = "plc"; + stamp_identity_provenance(merged, "manifest"); + + // Incoming: opcua nameplate read fills serial + firmware. + AssetIdentity opcua; + opcua.serial_number = "SN-42"; + opcua.firmware_version = "2.9.4"; + merge_identity(merged, opcua, "opcua", cfg); + + EXPECT_EQ(merged.manufacturer, "Siemens"); + EXPECT_EQ(merged.role, "plc"); + EXPECT_EQ(merged.serial_number, "SN-42"); + EXPECT_EQ(merged.firmware_version, "2.9.4"); + + // Provenance records which source set each field. + EXPECT_EQ(merged.provenance.at("manufacturer"), "manifest"); + EXPECT_EQ(merged.provenance.at("role"), "manifest"); + EXPECT_EQ(merged.provenance.at("serial_number"), "opcua"); + EXPECT_EQ(merged.provenance.at("firmware_version"), "opcua"); +} + +TEST(MergeIdentity, HigherAuthoritySourceOverridesLowerForSameField) { + IdentityMergeConfig cfg; + + AssetIdentity merged; + merged.manufacturer = "GuessedVendor"; + stamp_identity_provenance(merged, "manifest"); + + // opcua outranks manifest -> overrides the manufacturer. + AssetIdentity opcua; + opcua.manufacturer = "Siemens AG"; + merge_identity(merged, opcua, "opcua", cfg); + + EXPECT_EQ(merged.manufacturer, "Siemens AG"); + EXPECT_EQ(merged.provenance.at("manufacturer"), "opcua"); +} + +TEST(MergeIdentity, LowerAuthoritySourceDoesNotOverrideExistingField) { + IdentityMergeConfig cfg; + + AssetIdentity merged; + merged.manufacturer = "Siemens AG"; + stamp_identity_provenance(merged, "opcua"); + + // manifest is lower authority -> must not override opcua's value, but still fills gaps. + AssetIdentity manifest; + manifest.manufacturer = "Manifest Vendor"; + manifest.role = "plc"; + merge_identity(merged, manifest, "manifest", cfg); + + EXPECT_EQ(merged.manufacturer, "Siemens AG"); + EXPECT_EQ(merged.provenance.at("manufacturer"), "opcua"); + EXPECT_EQ(merged.role, "plc"); + EXPECT_EQ(merged.provenance.at("role"), "manifest"); +} + +TEST(MergeIdentity, ExtensibleMapMergedWithProvenanceAndPrecedence) { + IdentityMergeConfig cfg; + + AssetIdentity merged; + merged.extra["slot"] = "3"; + merged.extra["asset_tag"] = "OLD-TAG"; + stamp_identity_provenance(merged, "manifest"); + + AssetIdentity opcua; + opcua.extra["asset_tag"] = "NEW-TAG"; // higher authority overrides + opcua.extra["mac"] = "00:11:22:33:44:55"; // new key fills gap + merge_identity(merged, opcua, "opcua", cfg); + + EXPECT_EQ(merged.extra.at("slot"), "3"); + EXPECT_EQ(merged.extra.at("asset_tag"), "NEW-TAG"); + EXPECT_EQ(merged.extra.at("mac"), "00:11:22:33:44:55"); + EXPECT_EQ(merged.provenance.at("extra.slot"), "manifest"); + EXPECT_EQ(merged.provenance.at("extra.asset_tag"), "opcua"); + EXPECT_EQ(merged.provenance.at("extra.mac"), "opcua"); +} + +TEST(MergeIdentity, EmptyIncomingNeverOverwrites) { + IdentityMergeConfig cfg; + AssetIdentity merged; + merged.serial_number = "SN-42"; + stamp_identity_provenance(merged, "opcua"); + + AssetIdentity empty_incoming; // higher authority but empty + empty_incoming.role = ""; + merge_identity(merged, empty_incoming, "s7", cfg); + + EXPECT_EQ(merged.serial_number, "SN-42"); + EXPECT_EQ(merged.provenance.at("serial_number"), "opcua"); +} + +TEST(MergeIdentity, UnknownSourceFillsGapsButDoesNotOverrideKnown) { + IdentityMergeConfig cfg; + AssetIdentity merged; + merged.manufacturer = "Siemens"; + stamp_identity_provenance(merged, "manifest"); + + AssetIdentity unknown; + unknown.manufacturer = "Whatever"; // unknown source must not beat a listed source + unknown.serial_number = "SN-1"; // but fills an empty field + merge_identity(merged, unknown, "some_unlisted_source", cfg); + + EXPECT_EQ(merged.manufacturer, "Siemens"); + EXPECT_EQ(merged.provenance.at("manufacturer"), "manifest"); + EXPECT_EQ(merged.serial_number, "SN-1"); + EXPECT_EQ(merged.provenance.at("serial_number"), "some_unlisted_source"); +} + +TEST(MergeIdentity, ConfigurablePrecedenceOrder) { + // Flip the default so manifest outranks opcua. + IdentityMergeConfig cfg; + cfg.source_precedence = {"manifest", "opcua"}; + + AssetIdentity merged; + merged.manufacturer = "ManifestVendor"; + stamp_identity_provenance(merged, "manifest"); + + AssetIdentity opcua; + opcua.manufacturer = "OpcuaVendor"; + merge_identity(merged, opcua, "opcua", cfg); + + EXPECT_EQ(merged.manufacturer, "ManifestVendor"); + EXPECT_EQ(merged.provenance.at("manufacturer"), "manifest"); +} diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index b6c7c5212..f27cb6239 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -284,6 +284,73 @@ TEST_F(MergePipelineTest, EnrichmentFillsEmptyFields) { EXPECT_FALSE(result.components[0].topics.publishes.empty()); } +TEST_F(MergePipelineTest, AssetIdentityMergedFromMultipleSourcesWithProvenance) { + // Same Component id from two sources. The manifest carries manufacturer + role; + // a protocol read ("opcua") carries serial + firmware and a better manufacturer. + // Identity precedence (opcua > manifest) is independent of structural policy. + Component manifest_comp = make_component("plc_1", "line", "/line"); + manifest_comp.source = "manifest"; + manifest_comp.identity.manufacturer = "Siemens"; + manifest_comp.identity.role = "plc"; + + Component opcua_comp = make_component("plc_1", "line", "/line"); + opcua_comp.source = "plugin"; + opcua_comp.identity.manufacturer = "Siemens AG"; + opcua_comp.identity.serial_number = "SN-42"; + opcua_comp.identity.firmware_version = "2.9.4"; + opcua_comp.identity.extra["slot"] = "3"; + + LayerOutput manifest_out; + manifest_out.components.push_back(manifest_comp); + LayerOutput opcua_out; + opcua_out.components.push_back(opcua_comp); + + // manifest added first (highest structural priority -> base). + pipeline_.add_layer(std::make_unique("manifest", manifest_out)); + pipeline_.add_layer(std::make_unique("opcua", opcua_out)); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + const auto & id = result.components[0].identity; + + EXPECT_EQ(id.manufacturer, "Siemens AG"); // opcua outranks manifest + EXPECT_EQ(id.role, "plc"); // only manifest had it + EXPECT_EQ(id.serial_number, "SN-42"); + EXPECT_EQ(id.firmware_version, "2.9.4"); + EXPECT_EQ(id.extra.at("slot"), "3"); + + EXPECT_EQ(id.provenance.at("manufacturer"), "opcua"); + EXPECT_EQ(id.provenance.at("role"), "manifest"); + EXPECT_EQ(id.provenance.at("serial_number"), "opcua"); + EXPECT_EQ(id.provenance.at("firmware_version"), "opcua"); + EXPECT_EQ(id.provenance.at("extra.slot"), "opcua"); +} + +TEST_F(MergePipelineTest, AssetIdentityPrecedenceConfigurable) { + // Flip precedence so the manifest wins identity over the protocol read. + IdentityMergeConfig cfg; + cfg.source_precedence = {"manifest", "opcua"}; + pipeline_.set_identity_merge_config(cfg); + + Component manifest_comp = make_component("plc_1", "line", "/line"); + manifest_comp.identity.manufacturer = "Manifest Vendor"; + Component opcua_comp = make_component("plc_1", "line", "/line"); + opcua_comp.identity.manufacturer = "Opcua Vendor"; + + LayerOutput manifest_out; + manifest_out.components.push_back(manifest_comp); + LayerOutput opcua_out; + opcua_out.components.push_back(opcua_comp); + + pipeline_.add_layer(std::make_unique("manifest", manifest_out)); + pipeline_.add_layer(std::make_unique("opcua", opcua_out)); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].identity.manufacturer, "Manifest Vendor"); + EXPECT_EQ(result.components[0].identity.provenance.at("manufacturer"), "manifest"); +} + TEST_F(MergePipelineTest, AuthoritativeVsAuthoritativeHigherPriorityWins) { // Both layers claim AUTHORITATIVE for IDENTITY // Higher priority (first added) wins, conflict logged From e54771da689b15a0f8e5f3fbcea383211a2dce83 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Tue, 30 Jun 2026 16:36:35 +0000 Subject: [PATCH 2/6] style: apply clang-format-18 --- .../core/discovery/identity_merge.hpp | 13 ++++++------- .../test/test_asset_identity.cpp | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp index 5cd8047f2..d5f791442 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp @@ -34,11 +34,11 @@ namespace discovery { * and should therefore have their identity merged. */ enum class IdentityKeyStrategy { - AUTO, ///< serial -> model+slot -> endpoint -> configured id (first that resolves) - SERIAL, ///< serial number only + AUTO, ///< serial -> model+slot -> endpoint -> configured id (first that resolves) + SERIAL, ///< serial number only ORDER_CODE_SLOT, ///< model (order code) + extra["slot"] - ENDPOINT, ///< network endpoint - CONFIGURED_ID ///< the configured Component id (always available, never cross-source) + ENDPOINT, ///< network endpoint + CONFIGURED_ID ///< the configured Component id (always available, never cross-source) }; inline std::optional identity_key_strategy_from_string(const std::string & s) { @@ -73,9 +73,8 @@ inline std::optional identity_key_strategy_from_string(cons * authored manifest, which beats whatever runtime discovery guessed. */ struct IdentityMergeConfig { - std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", - "ads", "profinet", "manifest", "config", - "runtime", "node", "heuristic"}; + std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", "ads", "profinet", + "manifest", "config", "runtime", "node", "heuristic"}; IdentityKeyStrategy key_strategy{IdentityKeyStrategy::AUTO}; }; diff --git a/src/ros2_medkit_gateway/test/test_asset_identity.cpp b/src/ros2_medkit_gateway/test/test_asset_identity.cpp index 45df7ed4b..0053f8994 100644 --- a/src/ros2_medkit_gateway/test/test_asset_identity.cpp +++ b/src/ros2_medkit_gateway/test/test_asset_identity.cpp @@ -216,7 +216,7 @@ TEST(MergeIdentity, ExtensibleMapMergedWithProvenanceAndPrecedence) { stamp_identity_provenance(merged, "manifest"); AssetIdentity opcua; - opcua.extra["asset_tag"] = "NEW-TAG"; // higher authority overrides + opcua.extra["asset_tag"] = "NEW-TAG"; // higher authority overrides opcua.extra["mac"] = "00:11:22:33:44:55"; // new key fills gap merge_identity(merged, opcua, "opcua", cfg); @@ -250,7 +250,7 @@ TEST(MergeIdentity, UnknownSourceFillsGapsButDoesNotOverrideKnown) { AssetIdentity unknown; unknown.manufacturer = "Whatever"; // unknown source must not beat a listed source - unknown.serial_number = "SN-1"; // but fills an empty field + unknown.serial_number = "SN-1"; // but fills an empty field merge_identity(merged, unknown, "some_unlisted_source", cfg); EXPECT_EQ(merged.manufacturer, "Siemens"); From 85754c60451bc4cb790d2d923dde07b278b00a25 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Tue, 30 Jun 2026 19:15:55 +0200 Subject: [PATCH 3/6] fix(identity): wire identity across peers and rank on source Parse x-medkit.identity in peer aggregation and gap-fill remote identity on an id collision (was silently dropped). Rank merge precedence on the canonical Component.source tag instead of the free-form layer name, so protocol reads actually outrank the manifest. Drop the unused identity-key API and correct the README to match what the pipeline does. Refs #482 --- src/ros2_medkit_gateway/README.md | 22 ++-- .../core/discovery/identity_merge.hpp | 105 ++++-------------- .../src/core/aggregation/entity_merger.cpp | 11 ++ .../src/core/aggregation/peer_client.cpp | 6 + .../src/discovery/merge_pipeline.cpp | 22 ++-- .../test/test_asset_identity.cpp | 49 +++----- .../test/test_merge_pipeline.cpp | 21 ++-- .../test/test_peer_client.cpp | 77 +++++++++++++ 8 files changed, 166 insertions(+), 147 deletions(-) diff --git a/src/ros2_medkit_gateway/README.md b/src/ros2_medkit_gateway/README.md index 36746f397..45d2ede2d 100644 --- a/src/ros2_medkit_gateway/README.md +++ b/src/ros2_medkit_gateway/README.md @@ -1584,20 +1584,22 @@ records **per-field provenance** (which source set each field) under `_provenanc - **Precedence** is config-driven (`IdentityMergeConfig::source_precedence`, highest first) and is deliberately **decoupled from the structural `MergePolicy`**: a manifest can be the authoritative *structure* source while a live protocol read is the authoritative *identity* - source. Default order: protocol device-info (`opcua`, `s7`, `ethernet_ip`, `modbus`, `ads`, - `profinet`) > `manifest` > `config` > runtime sources. A higher-authority source overrides a + source. Authority is ranked on each contributing entity's canonical `Component.source` tag + ("manifest", "plugin", "runtime", "node", "config", or a protocol-class tag a provider sets + such as "opcua"), **not** the free-form discovery-layer name. Default order: protocol + device-info (`opcua`, `s7`, `ethernet_ip`, `modbus`, `ads`, `profinet`, and the generic + `plugin`) > `manifest` > `config` > runtime sources. A higher-authority source overrides a field; lower-authority sources only fill gaps; unknown sources rank lowest. Empty values never overwrite. -- **Identity key.** `compute_identity_key()` derives the key that decides whether two records - describe the same asset. Strategies: `serial`, `order_code_slot` (`model` + `extra["slot"]`), - `endpoint`, `configured_id`, and `auto` (serial -> order-code+slot -> endpoint -> configured id). - The discovery pipeline merges Components by `Component.id`, which is the `configured_id` - strategy; the other strategies let sources that assign ids (CSV/manifest import, protocol - probes) correlate records that arrive under different ids before they reach the pipeline. + +Components are correlated for merging by `Component.id`; identity is merged whenever two +sources contribute the same Component id (in the discovery pipeline, and gap-filled across +peer aggregation). The pipeline calls `merge_identity` inside the `IDENTITY` field group for Components, seeding -provenance with the base (highest-priority) layer. Configure via -`MergePipeline::set_identity_merge_config()`. +provenance with the base (highest-priority) layer's source tag. Configure via +`MergePipeline::set_identity_merge_config()`; precedence entries must equal the canonical +`Component.source` values you expect at runtime. ## Demo Nodes diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp index d5f791442..fe8f0cecd 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp @@ -16,10 +16,8 @@ #include "ros2_medkit_gateway/core/discovery/models/asset_identity.hpp" -#include #include #include -#include #include #include #include @@ -27,55 +25,31 @@ namespace ros2_medkit_gateway { namespace discovery { -/** - * @brief How the identity key for an asset is derived. - * - * The identity key is what decides whether two records describe the *same* asset - * and should therefore have their identity merged. - */ -enum class IdentityKeyStrategy { - AUTO, ///< serial -> model+slot -> endpoint -> configured id (first that resolves) - SERIAL, ///< serial number only - ORDER_CODE_SLOT, ///< model (order code) + extra["slot"] - ENDPOINT, ///< network endpoint - CONFIGURED_ID ///< the configured Component id (always available, never cross-source) -}; - -inline std::optional identity_key_strategy_from_string(const std::string & s) { - if (s == "auto") { - return IdentityKeyStrategy::AUTO; - } - if (s == "serial") { - return IdentityKeyStrategy::SERIAL; - } - if (s == "order_code_slot") { - return IdentityKeyStrategy::ORDER_CODE_SLOT; - } - if (s == "endpoint") { - return IdentityKeyStrategy::ENDPOINT; - } - if (s == "configured_id") { - return IdentityKeyStrategy::CONFIGURED_ID; - } - return std::nullopt; -} - /** * @brief Configuration for identity merging. * - * `source_precedence` lists source names from highest to lowest authority. A source - * not in the list is treated as the lowest authority (it can still fill empty fields - * but never overrides a known source). Identity authority is deliberately decoupled - * from the structural merge policy: a manifest may be the authoritative *structure* - * source while a live protocol read is the authoritative *identity* source. + * `source_precedence` ranks identity authority from highest to lowest. Entries are + * matched against a source's canonical identifier - the contributing entity's + * `Component.source` field ("manifest", "plugin", "runtime", "node", "heuristic", + * "config", or a protocol-class tag a provider sets such as "opcua"/"s7"), NOT the + * free-form discovery-layer / plugin name. A source not in the list ranks lowest: it + * can still fill empty fields but never overrides a known source. * - * Default precedence (highest first): live protocol device-info beats the hand - * authored manifest, which beats whatever runtime discovery guessed. + * Identity authority is deliberately decoupled from the structural merge policy: a + * manifest may be the authoritative *structure* source while a live protocol read is + * the authoritative *identity* source. + * + * Default precedence (highest first): a live protocol device-info read (a `plugin` + * source, or a protocol-specific source tag) beats the hand-authored `manifest`, + * which beats whatever runtime discovery guessed. The protocol-specific tags lead the + * list so that a provider which sets a concrete `Component.source` (e.g. "opcua") is + * honoured; the generic "plugin" tag covers the common case where the plugin layer + * stamps every plugin entity with source="plugin". */ struct IdentityMergeConfig { - std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", "ads", "profinet", - "manifest", "config", "runtime", "node", "heuristic"}; - IdentityKeyStrategy key_strategy{IdentityKeyStrategy::AUTO}; + std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", "ads", "profinet", + "plugin", "manifest", "config", "runtime", "node", "topic", + "heuristic"}; }; /** @@ -91,47 +65,6 @@ inline size_t source_rank(const std::string & source, const IdentityMergeConfig return config.source_precedence.size(); } -/** - * @brief Compute the identity key for an asset. - * @param identity The (possibly partial) asset identity. - * @param configured_id Fallback stable id (typically Component.id). - * @param strategy Key derivation strategy. - * @return A non-empty identity key, or empty string if the chosen strategy cannot - * resolve one (e.g. SERIAL strategy on an asset with no serial). - */ -inline std::string compute_identity_key(const AssetIdentity & identity, const std::string & configured_id, - IdentityKeyStrategy strategy) { - auto slot = [&]() -> std::string { - auto it = identity.extra.find("slot"); - return it != identity.extra.end() ? it->second : std::string{}; - }; - switch (strategy) { - case IdentityKeyStrategy::SERIAL: - return identity.serial_number; - case IdentityKeyStrategy::ORDER_CODE_SLOT: - if (identity.model.empty()) { - return std::string{}; - } - return identity.model + "/" + slot(); - case IdentityKeyStrategy::ENDPOINT: - return identity.network_endpoint; - case IdentityKeyStrategy::CONFIGURED_ID: - return configured_id; - case IdentityKeyStrategy::AUTO: - default: - if (!identity.serial_number.empty()) { - return "serial:" + identity.serial_number; - } - if (!identity.model.empty() && !slot().empty()) { - return "ordercode:" + identity.model + "/" + slot(); - } - if (!identity.network_endpoint.empty()) { - return "endpoint:" + identity.network_endpoint; - } - return "id:" + configured_id; - } -} - namespace detail { /// All typed identity fields as (provenance-key, member-pointer) pairs. diff --git a/src/ros2_medkit_gateway/src/core/aggregation/entity_merger.cpp b/src/ros2_medkit_gateway/src/core/aggregation/entity_merger.cpp index 581d90fef..aeb9cbdd6 100644 --- a/src/ros2_medkit_gateway/src/core/aggregation/entity_merger.cpp +++ b/src/ros2_medkit_gateway/src/core/aggregation/entity_merger.cpp @@ -16,6 +16,8 @@ #include +#include "ros2_medkit_gateway/core/discovery/identity_merge.hpp" + namespace ros2_medkit_gateway { EntityMerger::EntityMerger(const std::string & peer_name) : peer_name_(peer_name) { @@ -171,6 +173,15 @@ std::vector EntityMerger::merge_components(const std::vector is not in the default precedence), + // so this is a gap-fill: empty local fields are populated from the remote, but a + // value the local side already knows is never overridden. Stamp local provenance + // first so existing local fields are protected during the merge. + static const discovery::IdentityMergeConfig kIdentityConfig; + discovery::stamp_identity_provenance(merged.identity, merged.source); + discovery::merge_identity(merged.identity, remote_comp.identity, peer_source(), kIdentityConfig); + // A Component ID refers to one physical ECU. When the same ID is // present locally and remotely, the peer is the authoritative owner // of the Component's runtime state (data, logs, hosts, operations, diff --git a/src/ros2_medkit_gateway/src/core/aggregation/peer_client.cpp b/src/ros2_medkit_gateway/src/core/aggregation/peer_client.cpp index 26bc3d2d0..1204eba85 100644 --- a/src/ros2_medkit_gateway/src/core/aggregation/peer_client.cpp +++ b/src/ros2_medkit_gateway/src/core/aggregation/peer_client.cpp @@ -181,6 +181,12 @@ Component parse_component(const nlohmann::json & j) { if (xm.contains("dependsOn") && xm["dependsOn"].is_array()) { comp.depends_on = xm["dependsOn"].get>(); } + // Asset-identity nameplate (with per-field provenance). Emitted by peers under + // x-medkit.identity only when populated; parse it back so identity survives + // aggregation instead of being silently dropped. + if (xm.contains("identity") && xm["identity"].is_object()) { + comp.identity = AssetIdentity::from_json(xm["identity"]); + } } if (j.contains("translationId")) { comp.translation_id = j["translationId"].get(); diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index 4641fe302..a453febce 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -169,12 +169,11 @@ void merge_topics(ComponentTopics & target, const ComponentTopics & source, Merg } // Per-entity-type field-group merge dispatch. -// `source_name` and `id_cfg` are only consumed by the Component identity merge; -// they are accepted for all entity types so the caller can stay generic. +// `id_cfg` is only consumed by the Component identity merge; it is accepted for all +// entity types so the caller can stay generic. template void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup group, const MergeResolution & res, - const std::string & source_name, const IdentityMergeConfig & id_cfg) { - (void)source_name; + const IdentityMergeConfig & id_cfg) { (void)id_cfg; if constexpr (std::is_same_v) { switch (group) { @@ -204,8 +203,10 @@ void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup merge_scalar(target.description, source.description, res.scalar); merge_collection(target.tags, source.tags, res.collection); // Asset identity has its own per-field precedence (config-driven, decoupled - // from the structural MergePolicy) and records provenance per field. - merge_identity(target.identity, source.identity, source_name, id_cfg); + // from the structural MergePolicy) and records provenance per field. Authority + // is ranked on the contributing entity's canonical `source` tag (e.g. + // "manifest", "plugin", "opcua"), not the free-form discovery-layer name. + merge_identity(target.identity, source.identity, source.source, id_cfg); break; case FieldGroup::HIERARCHY: merge_scalar(target.namespace_path, source.namespace_path, res.scalar); @@ -350,10 +351,10 @@ std::vector MergePipeline::merge_entities(std::vectorname(); - // Seed asset-identity provenance with the base layer so subsequent identity - // merges can compare authority per field. + // Seed asset-identity provenance with the base entity's canonical source tag so + // subsequent identity merges can compare authority per field. if constexpr (std::is_same_v) { - stamp_identity_provenance(merged.identity, layers_[owner_layer_idx]->name()); + stamp_identity_provenance(merged.identity, merged.source); } // Track current owning layer per field group (initially all owned by first layer) @@ -377,8 +378,7 @@ std::vector MergePipeline::merge_entities(std::vectorname(), - identity_config_); + apply_field_group_merge(merged, entries[i].entity, fg, res, identity_config_); // If source won with a strictly higher-priority policy, it becomes // the owner of this field group for subsequent merge comparisons. diff --git a/src/ros2_medkit_gateway/test/test_asset_identity.cpp b/src/ros2_medkit_gateway/test/test_asset_identity.cpp index 0053f8994..6f19b9a91 100644 --- a/src/ros2_medkit_gateway/test/test_asset_identity.cpp +++ b/src/ros2_medkit_gateway/test/test_asset_identity.cpp @@ -109,38 +109,6 @@ TEST(ComponentIdentityCompat, IdentityEmittedUnderXMedkitWhenPresent) { EXPECT_EQ(j["x-medkit"]["identity"]["_provenance"]["manufacturer"], "manifest"); } -// --------------------------------------------------------------------------- -// Identity key derivation -// --------------------------------------------------------------------------- - -TEST(IdentityKey, AutoPrefersSerial) { - AssetIdentity id; - id.serial_number = "SN-42"; - id.network_endpoint = "opc.tcp://h:4840"; - EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::AUTO), "serial:SN-42"); -} - -TEST(IdentityKey, AutoFallsBackThroughOrderCodeEndpointConfiguredId) { - AssetIdentity id; - id.model = "6ES7"; - id.extra["slot"] = "3"; - EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::AUTO), "ordercode:6ES7/3"); - - AssetIdentity ep; - ep.network_endpoint = "opc.tcp://h:4840"; - EXPECT_EQ(compute_identity_key(ep, "cfg", IdentityKeyStrategy::AUTO), "endpoint:opc.tcp://h:4840"); - - AssetIdentity none; - EXPECT_EQ(compute_identity_key(none, "cfg", IdentityKeyStrategy::AUTO), "id:cfg"); -} - -TEST(IdentityKey, ExplicitStrategiesReturnEmptyWhenUnresolvable) { - AssetIdentity id; // no serial, no model - EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::SERIAL), ""); - EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::ORDER_CODE_SLOT), ""); - EXPECT_EQ(compute_identity_key(id, "cfg", IdentityKeyStrategy::CONFIGURED_ID), "cfg"); -} - // --------------------------------------------------------------------------- // merge_identity: precedence + per-field provenance + extensible map // --------------------------------------------------------------------------- @@ -275,3 +243,20 @@ TEST(MergeIdentity, ConfigurablePrecedenceOrder) { EXPECT_EQ(merged.manufacturer, "ManifestVendor"); EXPECT_EQ(merged.provenance.at("manufacturer"), "manifest"); } + +TEST(MergeIdentity, KnownSourceOverridesUnseededTarget) { + // Documented footgun: a target whose fields were set WITHOUT stamping provenance is + // treated as owned by an unknown (lowest authority) source, so any listed source + // overrides it. This guards that contract. + IdentityMergeConfig cfg; + + AssetIdentity target; + target.manufacturer = "UnstampedGuess"; // value present, provenance NOT seeded + + AssetIdentity opcua; + opcua.manufacturer = "Siemens AG"; // listed (known) source + merge_identity(target, opcua, "opcua", cfg); + + EXPECT_EQ(target.manufacturer, "Siemens AG"); + EXPECT_EQ(target.provenance.at("manufacturer"), "opcua"); +} diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index f27cb6239..b78ae4d15 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -286,15 +286,17 @@ TEST_F(MergePipelineTest, EnrichmentFillsEmptyFields) { TEST_F(MergePipelineTest, AssetIdentityMergedFromMultipleSourcesWithProvenance) { // Same Component id from two sources. The manifest carries manufacturer + role; - // a protocol read ("opcua") carries serial + firmware and a better manufacturer. - // Identity precedence (opcua > manifest) is independent of structural policy. + // a protocol read carries serial + firmware and a better manufacturer. Identity + // precedence (opcua > manifest) is independent of structural policy. The layers are + // deliberately NOT named after their source tags: authority is ranked on each + // entity's canonical `Component.source`, never the free-form layer name. Component manifest_comp = make_component("plc_1", "line", "/line"); manifest_comp.source = "manifest"; manifest_comp.identity.manufacturer = "Siemens"; manifest_comp.identity.role = "plc"; Component opcua_comp = make_component("plc_1", "line", "/line"); - opcua_comp.source = "plugin"; + opcua_comp.source = "opcua"; // protocol-class tag set by the provider opcua_comp.identity.manufacturer = "Siemens AG"; opcua_comp.identity.serial_number = "SN-42"; opcua_comp.identity.firmware_version = "2.9.4"; @@ -305,9 +307,9 @@ TEST_F(MergePipelineTest, AssetIdentityMergedFromMultipleSourcesWithProvenance) LayerOutput opcua_out; opcua_out.components.push_back(opcua_comp); - // manifest added first (highest structural priority -> base). - pipeline_.add_layer(std::make_unique("manifest", manifest_out)); - pipeline_.add_layer(std::make_unique("opcua", opcua_out)); + // First layer added is the highest structural priority -> base. Names are arbitrary. + pipeline_.add_layer(std::make_unique("structure_layer", manifest_out)); + pipeline_.add_layer(std::make_unique("device_info_layer", opcua_out)); auto result = pipeline_.execute(); ASSERT_EQ(result.components.size(), 1u); @@ -333,8 +335,10 @@ TEST_F(MergePipelineTest, AssetIdentityPrecedenceConfigurable) { pipeline_.set_identity_merge_config(cfg); Component manifest_comp = make_component("plc_1", "line", "/line"); + manifest_comp.source = "manifest"; manifest_comp.identity.manufacturer = "Manifest Vendor"; Component opcua_comp = make_component("plc_1", "line", "/line"); + opcua_comp.source = "opcua"; opcua_comp.identity.manufacturer = "Opcua Vendor"; LayerOutput manifest_out; @@ -342,8 +346,9 @@ TEST_F(MergePipelineTest, AssetIdentityPrecedenceConfigurable) { LayerOutput opcua_out; opcua_out.components.push_back(opcua_comp); - pipeline_.add_layer(std::make_unique("manifest", manifest_out)); - pipeline_.add_layer(std::make_unique("opcua", opcua_out)); + // Layer names intentionally unrelated to the source tags. + pipeline_.add_layer(std::make_unique("layer_a", manifest_out)); + pipeline_.add_layer(std::make_unique("layer_b", opcua_out)); auto result = pipeline_.execute(); ASSERT_EQ(result.components.size(), 1u); diff --git a/src/ros2_medkit_gateway/test/test_peer_client.cpp b/src/ros2_medkit_gateway/test/test_peer_client.cpp index e5bd2e887..048fa750d 100644 --- a/src/ros2_medkit_gateway/test/test_peer_client.cpp +++ b/src/ros2_medkit_gateway/test/test_peer_client.cpp @@ -18,6 +18,7 @@ #include #include +#include "ros2_medkit_gateway/core/aggregation/entity_merger.hpp" #include "ros2_medkit_gateway/core/aggregation/peer_client.hpp" #include "ros2_medkit_gateway/http/handlers/handler_context.hpp" @@ -916,3 +917,79 @@ TEST(PeerClientHappyPath, fetch_entities_rejects_collection_exceeding_limit) { svr.stop(); t.join(); } + +// Asset-identity survives a full peer round-trip: a Component with an identity +// nameplate is serialized via Component::to_json (as a peer would emit it under +// x-medkit.identity), parsed back by PeerClient::fetch_entities, then merged into a +// local entity by EntityMerger. The remote nameplate must be preserved end-to-end. +// Refs #482 +TEST(PeerClientHappyPath, asset_identity_survives_fetch_and_merge) { + // Build the remote Component with a populated identity and serialize it exactly the + // way the gateway emits component detail responses. + Component remote; + remote.id = "plc_1"; + remote.name = "Line PLC"; + remote.identity.manufacturer = "Siemens AG"; + remote.identity.serial_number = "SN-42"; + remote.identity.firmware_version = "2.9.4"; + remote.identity.extra["slot"] = "3"; + remote.identity.provenance["serial_number"] = "opcua"; + const std::string remote_detail = remote.to_json().dump(); + + httplib::Server svr; + svr.Get("/api/v1/areas", [](const httplib::Request &, httplib::Response & res) { + res.set_content(R"({"items":[]})", "application/json"); + }); + svr.Get("/api/v1/components", [&](const httplib::Request &, httplib::Response & res) { + res.set_content(R"({"items":[{"id":"plc_1","name":"Line PLC"}]})", "application/json"); + }); + svr.Get("/api/v1/components/plc_1", [&](const httplib::Request &, httplib::Response & res) { + res.set_content(remote_detail, "application/json"); + }); + svr.Get("/api/v1/components/plc_1/subcomponents", [](const httplib::Request &, httplib::Response & res) { + res.set_content(R"({"items":[]})", "application/json"); + }); + svr.Get("/api/v1/apps", [](const httplib::Request &, httplib::Response & res) { + res.set_content(R"({"items":[]})", "application/json"); + }); + svr.Get("/api/v1/functions", [](const httplib::Request &, httplib::Response & res) { + res.set_content(R"({"items":[]})", "application/json"); + }); + + int port = svr.bind_to_any_port("127.0.0.1"); + std::thread t([&]() { + svr.listen_after_bind(); + }); + + PeerClient client("http://127.0.0.1:" + std::to_string(port), "peer_plc", 5000); + auto result = client.fetch_entities(); + svr.stop(); + t.join(); + + ASSERT_TRUE(result.has_value()); + ASSERT_EQ(result->components.size(), 1u); + const auto & parsed = result->components[0].identity; + + // parse_component must have read x-medkit.identity back into the model. + EXPECT_EQ(parsed.manufacturer, "Siemens AG"); + EXPECT_EQ(parsed.serial_number, "SN-42"); + EXPECT_EQ(parsed.firmware_version, "2.9.4"); + EXPECT_EQ(parsed.extra.at("slot"), "3"); + EXPECT_EQ(parsed.provenance.at("serial_number"), "opcua"); + + // Collision merge: a local component with the same id but no identity must inherit + // the remote nameplate (gap-fill), and a remote-only component carries it directly. + Component local; + local.id = "plc_1"; + local.name = "Line PLC"; + local.source = "manifest"; + + EntityMerger merger("peer_plc"); + auto merged = merger.merge_components({local}, result->components); + ASSERT_EQ(merged.size(), 1u); + const auto & merged_id = merged[0].identity; + EXPECT_EQ(merged_id.manufacturer, "Siemens AG"); + EXPECT_EQ(merged_id.serial_number, "SN-42"); + EXPECT_EQ(merged_id.firmware_version, "2.9.4"); + EXPECT_EQ(merged_id.extra.at("slot"), "3"); +} From 7aa9eb58f23c8c5d1d5aab18552f0a859587e50a Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Tue, 30 Jun 2026 17:21:36 +0000 Subject: [PATCH 4/6] style: apply clang-format-18 --- .../ros2_medkit_gateway/core/discovery/identity_merge.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp index fe8f0cecd..78b0ac7fa 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp @@ -47,9 +47,9 @@ namespace discovery { * stamps every plugin entity with source="plugin". */ struct IdentityMergeConfig { - std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", "ads", "profinet", - "plugin", "manifest", "config", "runtime", "node", "topic", - "heuristic"}; + std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", "ads", + "profinet", "plugin", "manifest", "config", "runtime", + "node", "topic", "heuristic"}; }; /** From 36c5e31e095c27d5f5e3c8baa7b96358f47f2e94 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Tue, 30 Jun 2026 19:43:55 +0200 Subject: [PATCH 5/6] fix(identity): align identity serialization, provenance, and docs to_json() now returns {} for an empty identity instead of emitting a lone _provenance block. stamp_identity_provenance() is a no-op for an empty source so it no longer writes empty-valued _provenance entries. Corrected set_identity_merge_config() docstring after key-strategy removal. Refs #482 --- .../ros2_medkit_gateway/core/discovery/identity_merge.hpp | 5 +++++ .../core/discovery/models/asset_identity.hpp | 7 ++++++- .../ros2_medkit_gateway/discovery/merge_pipeline.hpp | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp index 78b0ac7fa..e8f8fb480 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp @@ -104,8 +104,13 @@ inline bool incoming_wins(bool field_set, const std::string & current_owner, con * to `source`, unless that field already has a provenance entry. * * Used to seed provenance for the first (base) source before merging others. + * A no-op when `source` is empty: an unknown owner is the implicit default, so + * stamping it would only add `_provenance` entries with empty values. */ inline void stamp_identity_provenance(AssetIdentity & identity, const std::string & source) { + if (source.empty()) { + return; + } for (const auto & [prov_key, member] : detail::identity_fields()) { if (!(identity.*member).empty() && identity.provenance.find(prov_key) == identity.provenance.end()) { identity.provenance[prov_key] = source; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp index 778b37c25..ce44ca8a3 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp @@ -82,10 +82,15 @@ struct AssetIdentity { * @brief Serialize to JSON (camelCase keys, only non-empty fields emitted). * * Provenance is emitted under "_provenance" when present so consumers can audit - * which source set each field. Returns an empty object when ::empty(). + * which source set each field. Returns an empty object when ::empty(): provenance + * alone (which should never exist without fields) is not emitted for an empty + * identity, so the empty case truly serializes to {}. */ json to_json() const { json j = json::object(); + if (empty()) { + return j; + } if (!manufacturer.empty()) { j["manufacturer"] = manufacturer; } diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp index 91a670171..691bd5507 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp @@ -59,10 +59,10 @@ class MergePipeline { void add_layer(std::unique_ptr layer); /** - * @brief Set the asset-identity merge configuration (precedence + key strategy). + * @brief Set the asset-identity merge configuration (source precedence). * * Identity authority is independent of the structural MergePolicy: this controls - * which source wins per identity field and how the identity key is derived. + * which source wins per identity field, ranked on `Component.source`. */ void set_identity_merge_config(IdentityMergeConfig config) { identity_config_ = std::move(config); From 80ac43598822d9112e3e9f1ff4c9fe78375ddd1c Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Wed, 1 Jul 2026 08:06:41 +0200 Subject: [PATCH 6/6] test(gateway): stop discovery refresh in SSE handler tests The SSE handler tests inject entity-cache state directly, but the node's graph-event refresh_cache() reconciles it to the live ROS graph and wipes node_to_app, dropping x-medkit. ASan widened the window so a refresh landed between injection and fault delivery. Stop the refresh drivers in the fixture. Refs #482 --- .../include/ros2_medkit_gateway/gateway_node.hpp | 12 ++++++++++++ src/ros2_medkit_gateway/src/gateway_node.cpp | 13 +++++++++++++ .../test/test_sse_fault_handler.cpp | 10 ++++++++++ 3 files changed, 35 insertions(+) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp index 1e86a29ea..32885a740 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp @@ -253,6 +253,18 @@ class GatewayNode : public rclcpp::Node { */ void trigger_reentrant_notification_for_testing(const EntityChangeScope & scope); + /** + * @brief Test hook: stop the gateway's own discovery refresh. + * + * Cancels the graph-event and backstop refresh timers and drops the graph + * event so `refresh_cache()` never runs again after this call. Tests that + * inject a known entity-cache state directly (via `get_thread_safe_cache()`) + * must call this first, otherwise a graph-event-driven `refresh_cache()` + * reconciles the cache back to the live ROS graph and silently wipes the + * injected entities. Do NOT call this from production code. + */ + void stop_discovery_refresh_for_testing(); + private: void refresh_cache(); void start_rest_server(); diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 716b5e8ea..ccb847835 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -1963,6 +1963,19 @@ void GatewayNode::trigger_reentrant_notification_for_testing(const EntityChangeS handle_entity_change_notification(scope); } +void GatewayNode::stop_discovery_refresh_for_testing() { + // Cancel the two refresh drivers and drop the graph event so no further + // refresh_cache() pass can clobber a test-injected entity cache. Safe to + // call right after construction (the constructor already ran one refresh). + if (graph_check_timer_) { + graph_check_timer_->cancel(); + } + if (backstop_timer_) { + backstop_timer_->cancel(); + } + graph_event_.reset(); +} + void GatewayNode::refresh_cache() { // Serialize refresh passes across the refresh timer, plugin // `notify_entities_changed` notifications, and any other caller. The diff --git a/src/ros2_medkit_gateway/test/test_sse_fault_handler.cpp b/src/ros2_medkit_gateway/test/test_sse_fault_handler.cpp index 2a04f99b0..731a4c5bb 100644 --- a/src/ros2_medkit_gateway/test/test_sse_fault_handler.cpp +++ b/src/ros2_medkit_gateway/test/test_sse_fault_handler.cpp @@ -183,6 +183,16 @@ class SSEFaultHandlerTest : public ::testing::Test { node_ = std::make_shared(options); + // These tests inject a known entity-cache state directly (apps / node_to_app + // via get_thread_safe_cache()) and assert on the x-medkit context the SSE + // handler snapshots at fault-arrival time. The gateway's own discovery + // refresh, driven by the rclcpp graph-event timer, reconciles that cache + // back to the live ROS graph and would wipe the injected entities. Under a + // sanitizer's wider timing window a graph-event refresh can land between the + // injection and the fault-event delivery, dropping x-medkit. Stop the + // refresh drivers so the injected cache is the single source of truth. + node_->stop_discovery_refresh_for_testing(); + CorsConfig cors_config; AuthConfig auth_config; TlsConfig tls_config;