Asset-identity model + merge-by-identity#488
Draft
mfaferek93 wants to merge 6 commits into
Draft
Conversation
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
Contributor
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. |
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
mfaferek93
force-pushed
the
feat/asset-identity-merge
branch
from
3f74174 to
36c5e31
Compare
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.