Asset-identity model + merge-by-identity#488
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an AAS Nameplate-aligned asset identity model for Component and wires it through discovery merging and peer aggregation so identity survives end-to-end (including per-field provenance), while keeping JSON backward compatible by only emitting identity when populated.
Changes:
- Add
AssetIdentityto theComponentmodel and emit it underx-medkit.identityonly when non-empty (with optional_provenance). - Implement identity merge logic with configurable source precedence and per-field provenance tracking; integrate it into the discovery merge pipeline and peer entity merging.
- Extend peer parsing to round-trip
x-medkit.identityand add unit tests for the model, merge behavior, pipeline integration, and peer round-trip.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_gateway/test/test_peer_client.cpp | Adds a peer round-trip + merge test ensuring x-medkit.identity is preserved through fetch + merge. |
| src/ros2_medkit_gateway/test/test_merge_pipeline.cpp | Adds merge-pipeline tests for identity precedence + provenance and configurability. |
| src/ros2_medkit_gateway/test/test_asset_identity.cpp | New unit tests for AssetIdentity JSON behavior and merge rules. |
| src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp | Integrates identity merge into component identity field-group merging and seeds provenance on the base component. |
| src/ros2_medkit_gateway/src/core/aggregation/peer_client.cpp | Parses x-medkit.identity from peer component JSON so identity isn’t dropped. |
| src/ros2_medkit_gateway/src/core/aggregation/entity_merger.cpp | Gap-fills local component identity from peer components using identity merge + provenance stamping. |
| src/ros2_medkit_gateway/README.md | Documents the identity model, JSON keys, AAS mapping intent, and precedence/provenance behavior. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp | Exposes set_identity_merge_config() on the merge pipeline. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp | Adds AssetIdentity identity to Component and conditionally emits it in JSON. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/asset_identity.hpp | New AssetIdentity type with typed fields, extra map, provenance map, and JSON (de)serialization. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp | New merge implementation with configurable source precedence and per-field provenance handling. |
| src/ros2_medkit_gateway/CMakeLists.txt | Registers the new test_asset_identity target. |
mfaferek93
force-pushed
the
feat/asset-identity-merge
branch
from
3f74174 to
36c5e31
Compare
bburda
left a comment
bburda
left a comment
There was a problem hiding this comment.
Additional findings outside the diff:
-
No code populates
Component.identityon a live gateway. The manifest model has no identity field and the manifest parser never sets one;host_info_provider.cppsets onlyhost_metadata; discovery plugins tagsource="plugin"and never set identity.set_identity_merge_config()has no caller. Outside unit tests the only writers of.identityare the peer parse and the merge itself. Sox-medkit.identityis never emitted at runtime, peers receive empty identities, and the whole merge/precedence/provenance path is unreachable in production. If model-only is the intended scope, please say so and add at least one producer (a manifest identity field, or the host provider) so the acceptance criterion (an asset shows merged identity from more than one source) can actually be met. -
This overlaps an existing open issue for host hardware identity in
x-medkit(#358), which asked for manufacturer/serial/board read from/sys/firmware/devicetree/base/model,/etc/machine-id, and DMI. This PR supersedes the data-model half of it: the nestedx-medkit.identityhere is a superset of its flatx-medkit.serial/hardware_id/boardproposal. It does not cover the producer half, since nothing reads those host files yet. Please decide the disposition of #358: close it as superseded by this model, or repurpose it to the host-producer follow-up. If repurposed, it should targetx-medkit.identityand drop the flat keys, and note that/etc/machine-idis a software UUID that belongs inidentity.extra, notserialNumber.
|
On the no-producer point: model plus merge is the intended scope of this PR. x-medkit.identity is emitted once a producer lands (the OPC UA nameplate reader, a follow-up); until then the merge / precedence / provenance path is covered by the unit tests. |
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
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
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
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
Stamp source="plugin" only when empty so a concrete protocol tag (e.g. opcua) survives and its identity precedence is reachable. On peer merge keep an incoming field's own provenance instead of the low-authority peer tag, matching the no-collision path. Mirror the destructor teardown in the SSE test helper and pin the camelCase field / snake_case provenance wire mapping. Refs #482
mfaferek93
force-pushed
the
feat/asset-identity-merge
branch
from
0c022ed to
19bc6fc
Compare
Closes #482
Adds an AAS-Nameplate-aligned
AssetIdentity(manufacturer, model, serial, hardware/firmware/software version, network endpoint, role) plus an extensible map to the Component entity. Identity is merged across sources by a configurable precedence ranked onComponent.source, with per-field provenance, wired through the discovery and peer-aggregation paths. JSON is backward compatible: identity is emitted only when set.Tested: identity / merge-pipeline / peer-client / entity-merger suites pass.