Manifest/CSV manual asset inventory#493
Conversation
Import hand-authored assets from a CSV (id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role, plus extras) or a manifest assets list. Each becomes a Component with identity populated and merges into the entity tree by id, combining with protocol-discovered structure.
…(open-core INV3) Strip a leading UTF-8 BOM and open a quoted field after leading spaces in the CSV tokenizer, so Excel "CSV UTF-8" exports and ` "a, b"` spacing parse instead of failing id detection or corrupting columns. Apply namespace/variant/type/translation_id/depends_on from manifest assets and stop synthesizing an "/id" fqn, so a bare inventory asset no longer overrides a discovered node's real path. An authoritative layer that supplies no value now fills the gap from a lower layer instead of blanking it.
Inventory assets (CSV and manifest assets list) now populate the Component's typed AssetIdentity with per-field provenance "inventory" instead of folding nameplate data into description/variant/tags. "inventory" ranks between "manifest" and "config" in the identity source precedence, and the merge acceptance test proves composition: authoritative sparse asset + runtime structure keeps real fqn/namespace while identity merges per field. Refs #490
There was a problem hiding this comment.
Pull request overview
Adds a manual asset-inventory input path (manifest assets: list and CSV import) that feeds into the existing manifest discovery and merge pipeline, enabling operator-authored asset identity to merge-by-id with protocol/runtime discovered structure while preserving per-field provenance.
Changes:
- Introduces
AssetEntry+ CSV parser and maps inventory assets intoComponentidentity with"inventory"provenance. - Extends manifest parsing (
assets:) and manifest loading (optional CSV import) to append inventory assets into the manifest prior to validation. - Adjusts scalar merge behavior to gap-fill when a higher-priority layer provides an empty scalar, and adds dedicated tests covering CSV parsing, manifest assets, CSV import, and merge behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_gateway/test/test_asset_inventory.cpp | New unit tests covering CSV parsing edge cases, manifest assets: parsing, CSV import via ManifestManager, and merge-by-id behavior. |
| src/ros2_medkit_gateway/src/gateway_node.cpp | Declares and wires a new ROS parameter discovery.inventory.csv_path into discovery configuration. |
| src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp | Updates scalar merge to fill gaps even when the higher-priority layer “wins” but provides an empty value. |
| src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp | Adds parsing of manifest assets: entries into inventory Components. |
| src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp | Adds CSV import support and appends inventory assets to the manifest before validation. |
| src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp | Plumbs inventory CSV path into ManifestManager when configured. |
| src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp | Implements RFC-4180-style CSV tokenization/parsing and AssetEntry -> Component mapping. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp | Exposes inventory CSV path configuration/getters and documents CSV import behavior. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_manager.hpp | Adds DiscoveryConfig::inventory_csv_path. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/merge_types.hpp | Treats "inventory" sources as protected from orphan suppression. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp | Declares parse_asset helper for manifest assets: entries. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp | Adds public API for CSV parsing and inventory asset mapping types/functions. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/identity_merge.hpp | Inserts "inventory" into identity source precedence (between manifest and config). |
| src/ros2_medkit_gateway/CMakeLists.txt | Registers the new test_asset_inventory gtest target. |
…set comment Add the manual asset inventory CHANGELOG entry and rewrite the asset_entry_to_component doxygen to match the structured-identity behavior. Refs #490
bburda
left a comment
bburda
left a comment
There was a problem hiding this comment.
Additional findings outside the diff:
-
Doc-code mismatch: the gateway README source-precedence prose (
ros2_medkit_gateway/README.md, the identity source-tag list and default order) still reads... manifest > config > runtimeand never mentionsinventory, but this PR insertedinventorybetweenmanifestandconfig(and updated theidentity_merge.hppdoc comment). Update the README prose to match. -
Docs gate: the new
assets:manifest list and thediscovery.inventory.csv_pathparameter with its CSV column schema (id/manufacturer/model/serial/hardware_rev/firmware/endpoint/role + aliases + extras + RFC-4180 quoting + size cap) are documented only in the CHANGELOG. Add them todocs/config/manifest-schema.rstand the annotatedconfig/gateway_params.yamlwhere users find manifest/discovery config.
…dling CSV rows no longer abort the manifest load on duplicate ids: first row wins within the CSV, manifest definitions win over CSV rows (identity folded in as gap-fill). Manifest assets: accepts the CSV aliases, both paths gain an area column, and README/schema/params docs cover the feature. Refs #490
|
Both docs points addressed in 4058cde: the gateway README source-precedence prose now includes inventory (manifest > inventory > config > runtime), and manifest-schema.rst + gateway_params.yaml document the assets: list and discovery.inventory.csv_path (columns, aliases, quoting, size cap, collision policy). sphinx -W clean. |
Merges hand-authored or CSV-imported asset entries into the entity tree via the existing manifest discovery.
inventory, ranked betweenmanifestandconfigin the source precedence.Tested: asset-inventory, merge-pipeline, identity, manifest and peer suites all pass.
Closes #490