From f7247a817b346fb9272ff235736cd92743baed3a Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Wed, 1 Jul 2026 19:36:28 +0200 Subject: [PATCH 1/5] feat(gateway): manual asset inventory via CSV and manifest assets 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. --- src/ros2_medkit_gateway/CMakeLists.txt | 4 + .../discovery/manifest/asset_inventory.hpp | 102 +++++ .../discovery/manifest/manifest_parser.hpp | 2 + .../core/discovery/merge_types.hpp | 9 +- .../discovery/discovery_manager.hpp | 6 + .../discovery/manifest/manifest_manager.hpp | 31 ++ .../discovery/manifest/asset_inventory.cpp | 293 +++++++++++++ .../src/discovery/discovery_manager.cpp | 4 + .../discovery/manifest/manifest_manager.cpp | 78 ++++ .../discovery/manifest/manifest_parser.cpp | 71 ++++ src/ros2_medkit_gateway/src/gateway_node.cpp | 7 + .../test/test_asset_inventory.cpp | 398 ++++++++++++++++++ 12 files changed, 1001 insertions(+), 4 deletions(-) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp create mode 100644 src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp create mode 100644 src/ros2_medkit_gateway/test/test_asset_inventory.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 655dfb2e1..79399e221 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -646,6 +646,10 @@ if(BUILD_TESTING) ament_add_gtest(test_asset_identity test/test_asset_identity.cpp) target_link_libraries(test_asset_identity gateway_ros2) + # Asset inventory: CSV / manifest asset-list parse + merge-by-id + ament_add_gtest(test_asset_inventory test/test_asset_inventory.cpp) + target_link_libraries(test_asset_inventory 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/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp new file mode 100644 index 000000000..d8857aefd --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp @@ -0,0 +1,102 @@ +// 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/component.hpp" + +#include +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief A manually declared inventory asset (hand-authored or CSV-imported). + * + * Represents one physical/logical asset the operator knows about but that a + * protocol layer may not fully describe (or may not describe at all). The + * canonical identity fields mirror the INV1 asset-identity model; anything not + * covered by a canonical column is retained verbatim in @ref extras. + * + * @note INV1 dependency: once the structured `AssetIdentity` field lands on the + * Component entity, `asset_entry_to_component` should populate it directly + * (with per-field provenance) instead of folding identity into the + * existing description / variant / tag fields. This struct is the single + * place the two import paths (CSV and manifest `assets:` list) converge, + * so the migration is localized to `asset_entry_to_component`. + */ +struct AssetEntry { + std::string id; ///< Stable asset id (required); merge key into the tree + std::string manufacturer; ///< Vendor / OEM + std::string model; ///< Model / order code + std::string serial; ///< Serial number + std::string hardware_rev; ///< Hardware revision + std::string firmware; ///< Firmware / software version + std::string endpoint; ///< Network endpoint (URL / host:port) + std::string role; ///< Functional role (controller, sensor, ...) + + /// Non-canonical columns, kept in declared order (header -> value). + std::vector> extras; +}; + +/** + * @brief Result of parsing a CSV inventory document. + */ +struct AssetCsvResult { + std::vector entries; ///< One entry per non-empty data row with an id + std::vector warnings; ///< Non-fatal issues (skipped rows, ...) +}; + +/** + * @brief Parse a CSV inventory document into asset entries. + * + * The first non-empty line is the header. Column names are matched + * case-insensitively after trimming; recognized canonical names are + * `id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role` + * (a small set of common aliases is also accepted). Any other column is + * preserved as an extra keyed by its original (trimmed) header name. + * + * RFC-4180-style quoting is honored: double-quoted fields may contain commas, + * newlines, and escaped quotes (`""`). Unquoted fields are whitespace-trimmed. + * Blank lines are skipped. Rows whose `id` is empty are skipped and reported in + * @ref AssetCsvResult::warnings. + * + * @param csv_text Full CSV document. + * @return Parsed entries plus any non-fatal warnings. + * @throws std::runtime_error if the document has no header or no `id` column. + */ +AssetCsvResult parse_asset_csv(const std::string & csv_text); + +/** + * @brief Convert an inventory asset into a SOVD Component. + * + * The component is tagged `source = "inventory"` so its provenance stays + * visible after the merge pipeline combines it (by id) with protocol-discovered + * structure. Identity is projected onto the existing Component fields until the + * INV1 structured identity model is available (see the AssetEntry note): + * - name <- " " (falls back to id at display time) + * - variant <- hardware_rev + * - tags <- role (when present) + * - description <- human-readable identity summary (serial, firmware, + * endpoint, extras) + * + * @param entry Asset entry (must have a non-empty id). + * @return Component with `source = "inventory"` and identity populated. + */ +Component asset_entry_to_component(const AssetEntry & entry); + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp index ff85e5740..d8de2d572 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp @@ -86,6 +86,8 @@ class ManifestParser { /// Recursively parse area and its nested subareas void parse_area_recursive(const YAML::Node & node, const std::string & parent_id, std::vector & areas) const; Component parse_component(const YAML::Node & node) const; + /// Parse a manual-inventory asset entry into a Component (identity populated). + Component parse_asset(const YAML::Node & node) const; App parse_app(const YAML::Node & node) const; Function parse_function(const YAML::Node & node) const; ManifestConfig parse_config(const YAML::Node & node) const; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/merge_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/merge_types.hpp index 900604d68..7578a97c9 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/merge_types.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/merge_types.hpp @@ -166,12 +166,13 @@ inline bool is_runtime_source(const std::string & source) { /** * @brief Check if an entity source is protected from orphan suppression * - * Whitelist approach: only manifest and plugin sources are preserved during - * orphan filtering. Everything else (heuristic, topic, synthetic, node, runtime, - * peer:xxx) is eligible for suppression. + * Whitelist approach: manifest, inventory (operator-declared assets), and + * plugin sources are preserved during orphan filtering. Everything else + * (heuristic, topic, synthetic, node, runtime, peer:xxx) is eligible for + * suppression. */ inline bool is_protected_source(const std::string & source) { - return source == "manifest" || source.rfind("plugin", 0) == 0; + return source == "manifest" || source == "inventory" || source.rfind("plugin", 0) == 0; } } // namespace discovery diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_manager.hpp index e1c081943..ee7b6e2b1 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_manager.hpp @@ -61,6 +61,12 @@ struct DiscoveryConfig { /// manifest. See `ManifestManager::set_fragments_dir`. std::string manifest_fragments_dir; + /// CSV file describing manually inventoried assets. When non-empty, each row + /// becomes a Component (identity populated) appended to the manifest on every + /// load / reload and merged into the tree by id. Empty = disabled. See + /// `ManifestManager::set_inventory_csv_path`. + std::string inventory_csv_path; + /** * @brief Runtime (heuristic) discovery options * diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp index e74f58fb6..9b0ca1b3a 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp @@ -118,6 +118,29 @@ class ManifestManager { */ std::string get_fragments_dir() const; + /** + * @brief Configure a CSV file describing manually inventoried assets. + * + * When set, every `load_manifest` / `reload_manifest` call parses the CSV + * (columns `id, manufacturer, model, serial, hardware_rev, firmware, + * endpoint, role`, plus any extra columns) and appends one Component per row + * to the base manifest before validation. Each asset merges into the entity + * tree by id, combining with protocol-discovered structure the same way a + * manifest component does. A parse failure (or a missing `id` column) is + * recorded as a validation error and fails the load. + * + * Call with an empty string to disable CSV import. + * + * @param path Path to the inventory CSV. Does not need to exist at call time; + * a missing file on load is treated as "no inventory". + */ + void set_inventory_csv_path(const std::string & path); + + /** + * @brief Get the currently configured inventory CSV path (empty if unset). + */ + std::string get_inventory_csv_path() const; + /** * @brief Unload current manifest (revert to runtime-only mode) */ @@ -266,9 +289,17 @@ class ManifestManager { /// `validation_result_` so callers see them in the normal error flow. bool apply_fragments(Manifest & base); + /// Parse the configured inventory CSV (if any) and append the resulting + /// asset Components to `base`. Called with `mutex_` held. Returns true on + /// success (or when no CSV is configured / the file is absent); false when + /// the CSV cannot be read or parsed. Parse errors are appended to + /// `validation_result_` so callers see them in the normal error flow. + bool apply_inventory_csv(Manifest & base); + std::optional manifest_; std::string manifest_path_; std::string fragments_dir_; + std::string inventory_csv_path_; ValidationResult validation_result_; bool strict_mode_{true}; diff --git a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp new file mode 100644 index 000000000..8c3bd99ff --- /dev/null +++ b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp @@ -0,0 +1,293 @@ +// 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/manifest/asset_inventory.hpp" + +#include +#include +#include +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +namespace { + +std::string trim(const std::string & s) { + const auto not_space = [](char c) { + return std::isspace(static_cast(c)) == 0; + }; + auto begin = std::find_if(s.begin(), s.end(), not_space); + auto end = std::find_if(s.rbegin(), s.rend(), not_space).base(); + if (begin >= end) { + return std::string{}; + } + return std::string(begin, end); +} + +std::string to_lower(const std::string & s) { + std::string out; + out.reserve(s.size()); + for (const char c : s) { + out.push_back(static_cast(std::tolower(static_cast(c)))); + } + return out; +} + +// A single parsed CSV field plus whether it was double-quoted (quoted fields +// are not whitespace-trimmed so leading/trailing spaces inside quotes survive). +struct CsvField { + std::string value; + bool quoted{false}; +}; + +// Tokenize a CSV document into rows of fields, honoring RFC-4180 quoting +// (embedded commas / newlines / escaped `""`). Empty rows are dropped. +std::vector> tokenize(const std::string & text) { + std::vector> rows; + std::vector row; + CsvField field; + bool in_quotes = false; + bool field_has_content = false; + + const auto end_field = [&]() { + if (!field.quoted) { + field.value = trim(field.value); + } + row.push_back(std::move(field)); + field = CsvField{}; + field_has_content = false; + }; + + const auto end_row = [&]() { + end_field(); + // Drop rows that are entirely empty (e.g., a trailing newline or a blank + // separator line): a single empty, unquoted field with no siblings. + const bool blank = row.size() == 1u && !row[0].quoted && row[0].value.empty(); + if (!blank) { + rows.push_back(std::move(row)); + } + row.clear(); + }; + + for (std::size_t i = 0; i < text.size(); ++i) { + const char c = text[i]; + if (in_quotes) { + if (c == '"') { + if (i + 1 < text.size() && text[i + 1] == '"') { + field.value.push_back('"'); + ++i; + } else { + in_quotes = false; + } + } else { + field.value.push_back(c); + } + continue; + } + + if (c == '"' && !field_has_content) { + in_quotes = true; + field.quoted = true; + field_has_content = true; + } else if (c == ',') { + end_field(); + } else if (c == '\r') { + // Swallow CR; the following LF (if any) terminates the row. + } else if (c == '\n') { + end_row(); + } else { + field.value.push_back(c); + field_has_content = true; + } + } + + // Flush any trailing field/row not terminated by a newline. + if (field_has_content || field.quoted || !row.empty()) { + end_row(); + } + + return rows; +} + +// Canonical destination for a header column. +enum class Column { kIgnore, kExtra, kId, kManufacturer, kModel, kSerial, kHardwareRev, kFirmware, kEndpoint, kRole }; + +Column canonical_column(const std::string & header_lower) { + if (header_lower == "id") { + return Column::kId; + } + if (header_lower == "manufacturer") { + return Column::kManufacturer; + } + if (header_lower == "model") { + return Column::kModel; + } + if (header_lower == "serial" || header_lower == "serial_number") { + return Column::kSerial; + } + if (header_lower == "hardware_rev" || header_lower == "hardware_revision" || header_lower == "hw_rev") { + return Column::kHardwareRev; + } + if (header_lower == "firmware" || header_lower == "firmware_version" || header_lower == "fw") { + return Column::kFirmware; + } + if (header_lower == "endpoint") { + return Column::kEndpoint; + } + if (header_lower == "role") { + return Column::kRole; + } + return Column::kExtra; +} + +void assign(AssetEntry & entry, Column column, const std::string & header, const std::string & value) { + switch (column) { + case Column::kId: + entry.id = value; + break; + case Column::kManufacturer: + entry.manufacturer = value; + break; + case Column::kModel: + entry.model = value; + break; + case Column::kSerial: + entry.serial = value; + break; + case Column::kHardwareRev: + entry.hardware_rev = value; + break; + case Column::kFirmware: + entry.firmware = value; + break; + case Column::kEndpoint: + entry.endpoint = value; + break; + case Column::kRole: + entry.role = value; + break; + case Column::kExtra: + if (!value.empty()) { + entry.extras.emplace_back(header, value); + } + break; + case Column::kIgnore: + break; + } +} + +} // namespace + +AssetCsvResult parse_asset_csv(const std::string & csv_text) { + AssetCsvResult result; + + auto rows = tokenize(csv_text); + if (rows.empty()) { + throw std::runtime_error("asset CSV: no header row found"); + } + + // Build the column map from the header row. + const auto & header_row = rows.front(); + std::vector columns; + std::vector header_names; + columns.reserve(header_row.size()); + header_names.reserve(header_row.size()); + bool has_id = false; + for (const auto & cell : header_row) { + const std::string name = trim(cell.value); + Column col = name.empty() ? Column::kIgnore : canonical_column(to_lower(name)); + if (col == Column::kId) { + has_id = true; + } + columns.push_back(col); + header_names.push_back(name); + } + if (!has_id) { + throw std::runtime_error("asset CSV: missing required 'id' column"); + } + + for (std::size_t r = 1; r < rows.size(); ++r) { + const auto & data_row = rows[r]; + AssetEntry entry; + for (std::size_t c = 0; c < columns.size() && c < data_row.size(); ++c) { + assign(entry, columns[c], header_names[c], data_row[c].value); + } + if (entry.id.empty()) { + result.warnings.push_back("asset CSV: row " + std::to_string(r + 1) + " skipped (empty id)"); + continue; + } + result.entries.push_back(std::move(entry)); + } + + return result; +} + +Component asset_entry_to_component(const AssetEntry & entry) { + Component comp; + comp.id = entry.id; + comp.source = "inventory"; + comp.fqn = "/" + entry.id; + + // name <- " " (left empty when neither is set so the + // consumer falls back to the id). + std::string name = trim(entry.manufacturer + " " + entry.model); + comp.name = name; + + // variant carries the hardware revision. + comp.variant = entry.hardware_rev; + + // role becomes a tag for filtering. + if (!entry.role.empty()) { + comp.tags.push_back(entry.role); + } + + // description: human-readable identity summary. base is the manufacturer/model + // line; details append the remaining identity fields and any extras. + std::vector details; + if (!entry.serial.empty()) { + details.push_back("S/N " + entry.serial); + } + if (!entry.firmware.empty()) { + details.push_back("FW " + entry.firmware); + } + if (!entry.endpoint.empty()) { + details.push_back("endpoint " + entry.endpoint); + } + for (const auto & [key, value] : entry.extras) { + details.push_back(key + " " + value); + } + + std::string description = name; + if (!details.empty()) { + std::string joined; + for (std::size_t i = 0; i < details.size(); ++i) { + if (i > 0) { + joined += ", "; + } + joined += details[i]; + } + if (!description.empty()) { + description += " "; + } + description += "(" + joined + ")"; + } + comp.description = description; + + return comp; +} + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp index 63de692c9..f7d1bee2b 100644 --- a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp +++ b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp @@ -50,6 +50,10 @@ bool DiscoveryManager::initialize(const DiscoveryConfig & config) { manifest_manager_->set_fragments_dir(config.manifest_fragments_dir); RCLCPP_INFO(node_->get_logger(), "Manifest fragments_dir: %s", config.manifest_fragments_dir.c_str()); } + if (!config.inventory_csv_path.empty()) { + manifest_manager_->set_inventory_csv_path(config.inventory_csv_path); + RCLCPP_INFO(node_->get_logger(), "Inventory CSV: %s", config.inventory_csv_path.c_str()); + } if (config.manifest_path.empty()) { if (config.mode == DiscoveryMode::HYBRID) { diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp index abba62df7..36100e239 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp @@ -18,6 +18,10 @@ #include #include +#include +#include + +#include "ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp" namespace ros2_medkit_gateway { namespace discovery { @@ -44,6 +48,12 @@ bool ManifestManager::load_manifest(const std::string & file_path, bool strict) // regular validation step so the full error set is reported. } + // Append manually inventoried assets (CSV) on top of the manifest. Errors + // are recorded in validation_result_ and surface in the regular flow. + if (!apply_inventory_csv(loaded)) { + // apply_inventory_csv already recorded the error; fall through. + } + // Validate the merged manifest. auto merge_errors = std::move(validation_result_); validation_result_ = validator_.validate(loaded); @@ -476,6 +486,74 @@ std::string ManifestManager::get_fragments_dir() const { return fragments_dir_; } +void ManifestManager::set_inventory_csv_path(const std::string & path) { + std::lock_guard lock(mutex_); + inventory_csv_path_ = path; +} + +std::string ManifestManager::get_inventory_csv_path() const { + std::lock_guard lock(mutex_); + return inventory_csv_path_; +} + +bool ManifestManager::apply_inventory_csv(Manifest & base) { + // Called with mutex_ held by the caller. + if (inventory_csv_path_.empty()) { + return true; + } + + std::filesystem::path csv_path(inventory_csv_path_); + std::error_code ec; + if (!std::filesystem::exists(csv_path, ec)) { + // A missing file is "no inventory", not an error - mirrors fragments_dir. + log_warn("Inventory CSV not found (skipping): " + inventory_csv_path_); + return true; + } + + // Cap file size before reading to bound worst-case allocation on a + // misconfigured path (e.g., a symlink to a huge log file). + auto size = std::filesystem::file_size(csv_path, ec); + if (ec) { + validation_result_.add_error("INVENTORY_CSV", "cannot stat inventory CSV: " + ec.message(), inventory_csv_path_); + log_error("Cannot stat inventory CSV '" + inventory_csv_path_ + "': " + ec.message()); + return false; + } + if (size > ManifestParser::kMaxFragmentBytes) { + validation_result_.add_error("INVENTORY_CSV", + "inventory CSV exceeds " + std::to_string(ManifestParser::kMaxFragmentBytes) + + "-byte limit (size=" + std::to_string(size) + ")", + inventory_csv_path_); + log_error("Inventory CSV '" + inventory_csv_path_ + "' exceeds size limit"); + return false; + } + + std::ifstream file(csv_path); + if (!file.is_open()) { + validation_result_.add_error("INVENTORY_CSV", "cannot open inventory CSV", inventory_csv_path_); + log_error("Cannot open inventory CSV: " + inventory_csv_path_); + return false; + } + std::stringstream buffer; + buffer << file.rdbuf(); + + try { + auto parsed = parse_asset_csv(buffer.str()); + for (const auto & warn : parsed.warnings) { + log_warn("Inventory CSV: " + warn); + } + for (const auto & entry : parsed.entries) { + base.components.push_back(asset_entry_to_component(entry)); + } + log_info("Inventory CSV loaded: " + std::to_string(parsed.entries.size()) + " assets from " + inventory_csv_path_); + } catch (const std::exception & e) { + validation_result_.add_error("INVENTORY_CSV", e.what(), inventory_csv_path_); + log_error("Failed to parse inventory CSV '" + inventory_csv_path_ + "': " + e.what()); + return false; + } + + return true; +} + bool ManifestManager::apply_fragments(Manifest & base) { // Called with mutex_ held by the caller. if (fragments_dir_.empty()) { diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp index 0b40fa56e..5ecafd25f 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -21,7 +21,9 @@ #include #include #include +#include +#include "ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp" #include "ros2_medkit_serialization/json_serializer.hpp" namespace ros2_medkit_gateway { @@ -141,6 +143,15 @@ Manifest ManifestParser::parse_string(const std::string & yaml_content) const { } } + // Parse assets (manual inventory list). Each asset becomes a Component with + // identity populated, so it flows through the same merge-by-id path as any + // other component and combines with protocol-discovered structure. + if (root["assets"] && root["assets"].IsSequence()) { + for (const auto & node : root["assets"]) { + manifest.components.push_back(parse_asset(node)); + } + } + // Parse apps if (root["apps"] && root["apps"].IsSequence()) { for (const auto & node : root["apps"]) { @@ -257,6 +268,66 @@ Component ManifestParser::parse_component(const YAML::Node & node) const { return comp; } +Component ManifestParser::parse_asset(const YAML::Node & node) const { + // Structural / identity keys handled explicitly; any other scalar key is + // retained verbatim as an extra so operator-specific columns are not lost. + static const std::unordered_set reserved = { + "id", "manufacturer", "model", "serial", "hardware_rev", "firmware", + "endpoint", "role", "area", "tags", "parent_component_id", "name", + "description", "namespace", "variant", "type", "translation_id", "depends_on"}; + + AssetEntry entry; + entry.id = get_string(node, "id"); + entry.manufacturer = get_string(node, "manufacturer"); + entry.model = get_string(node, "model"); + entry.serial = get_string(node, "serial"); + entry.hardware_rev = get_string(node, "hardware_rev"); + entry.firmware = get_string(node, "firmware"); + entry.endpoint = get_string(node, "endpoint"); + entry.role = get_string(node, "role"); + + if (node.IsMap()) { + for (const auto & it : node) { + if (!it.first.IsScalar() || !it.second.IsScalar()) { + continue; + } + const std::string key = it.first.as(); + if (reserved.count(key) != 0) { + continue; + } + const std::string value = it.second.as(); + if (!value.empty()) { + entry.extras.emplace_back(key, value); + } + } + } + + Component comp = asset_entry_to_component(entry); + + // Optional tree-placement and presentation overrides. + const std::string area = get_string(node, "area"); + if (!area.empty()) { + comp.area = area; + } + const std::string parent = get_string(node, "parent_component_id"); + if (!parent.empty()) { + comp.parent_component_id = parent; + } + const std::string explicit_name = get_string(node, "name"); + if (!explicit_name.empty()) { + comp.name = explicit_name; + } + const std::string explicit_description = get_string(node, "description"); + if (!explicit_description.empty()) { + comp.description = explicit_description; + } + for (const auto & tag : get_string_vector(node, "tags")) { + comp.tags.push_back(tag); + } + + return comp; +} + App ManifestParser::parse_app(const YAML::Node & node) const { App app; app.id = get_string(node, "id"); diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 4024d6c66..6c323d4ea 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -279,6 +279,12 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki // disabled (default). See ManifestManager::set_fragments_dir for the // merge semantics. declare_parameter("discovery.manifest.fragments_dir", ""); + // CSV file listing manually inventoried assets (columns: id, manufacturer, + // model, serial, hardware_rev, firmware, endpoint, role, plus extras). Each + // row is appended to the manifest as a Component and merged into the tree by + // id. Requires a manifest-backed discovery mode (manifest_only / hybrid with + // discovery.manifest_path set). Empty = disabled (default). + declare_parameter("discovery.inventory.csv_path", ""); declare_parameter("discovery.runtime.enabled", true); // Software updates parameters @@ -620,6 +626,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki discovery_config.manifest_strict_validation = get_parameter("discovery.manifest_strict_validation").as_bool(); discovery_config.manifest_enabled = get_parameter("discovery.manifest.enabled").as_bool(); discovery_config.manifest_fragments_dir = get_parameter("discovery.manifest.fragments_dir").as_string(); + discovery_config.inventory_csv_path = get_parameter("discovery.inventory.csv_path").as_string(); discovery_config.runtime_enabled = get_parameter("discovery.runtime.enabled").as_bool(); // Runtime discovery options diff --git a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp new file mode 100644 index 000000000..ab33c6572 --- /dev/null +++ b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp @@ -0,0 +1,398 @@ +// 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/discovery_layer.hpp" +#include "ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp" +#include "ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp" +#include "ros2_medkit_gateway/core/discovery/merge_types.hpp" +#include "ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp" +#include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" + +#include + +#include +#include +#include +#include +#include +#include +#include + +using namespace ros2_medkit_gateway; // NOLINT(build/namespaces) +using namespace ros2_medkit_gateway::discovery; // NOLINT(build/namespaces) + +namespace { + +namespace fs = std::filesystem; + +// Find a component by id in a vector (returns nullptr when absent). +const Component * find_component(const std::vector & comps, const std::string & id) { + auto it = std::find_if(comps.begin(), comps.end(), [&](const Component & c) { + return c.id == id; + }); + return it == comps.end() ? nullptr : &*it; +} + +bool has_tag(const Component & comp, const std::string & tag) { + return std::find(comp.tags.begin(), comp.tags.end(), tag) != comp.tags.end(); +} + +// Minimal discovery layer double for exercising the real MergePipeline. +class FakeLayer : public DiscoveryLayer { + public: + FakeLayer(std::string name, LayerOutput output, std::unordered_map policies) + : name_(std::move(name)), output_(std::move(output)), policies_(std::move(policies)) { + } + + std::string name() const override { + return name_; + } + LayerOutput discover() override { + return output_; + } + MergePolicy policy_for(FieldGroup group) const override { + auto it = policies_.find(group); + return it == policies_.end() ? MergePolicy::ENRICHMENT : it->second; + } + + private: + std::string name_; + LayerOutput output_; + std::unordered_map policies_; +}; + +} // namespace + +// ── CSV parsing ───────────────────────────────────────────────────────────── + +TEST(AssetCsvParseTest, BasicHeaderAndRows) { + const std::string csv = + "id,manufacturer,model,serial,hardware_rev,firmware,endpoint,role\n" + "plc_1,Siemens,S7-1500,SN123,A2,2.9.1,opc.tcp://10.0.0.5:4840,controller\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + const auto & e = result.entries[0]; + EXPECT_EQ(e.id, "plc_1"); + EXPECT_EQ(e.manufacturer, "Siemens"); + EXPECT_EQ(e.model, "S7-1500"); + EXPECT_EQ(e.serial, "SN123"); + EXPECT_EQ(e.hardware_rev, "A2"); + EXPECT_EQ(e.firmware, "2.9.1"); + EXPECT_EQ(e.endpoint, "opc.tcp://10.0.0.5:4840"); + EXPECT_EQ(e.role, "controller"); + EXPECT_TRUE(result.warnings.empty()); +} + +TEST(AssetCsvParseTest, QuotedFieldsWithCommasAndNewlines) { + const std::string csv = + "id,manufacturer,model\n" + "vfd_1,\"Acme, Inc.\",\"Model\nX\"\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].manufacturer, "Acme, Inc."); + EXPECT_EQ(result.entries[0].model, "Model\nX"); +} + +TEST(AssetCsvParseTest, EscapedQuotes) { + const std::string csv = + "id,model\n" + "a,\"He said \"\"hi\"\"\"\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].model, "He said \"hi\""); +} + +TEST(AssetCsvParseTest, ExtraColumnsPreservedInOrder) { + const std::string csv = + "id,location,rack\n" + "io_1,cell-3,R2\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + const auto & extras = result.entries[0].extras; + ASSERT_EQ(extras.size(), 2u); + EXPECT_EQ(extras[0].first, "location"); + EXPECT_EQ(extras[0].second, "cell-3"); + EXPECT_EQ(extras[1].first, "rack"); + EXPECT_EQ(extras[1].second, "R2"); +} + +TEST(AssetCsvParseTest, BlankLinesSkipped) { + const std::string csv = + "id,model\n" + "\n" + "a,X\n" + "\n" + "b,Y\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 2u); + EXPECT_EQ(result.entries[0].id, "a"); + EXPECT_EQ(result.entries[1].id, "b"); +} + +TEST(AssetCsvParseTest, RowWithEmptyIdSkippedWithWarning) { + const std::string csv = + "id,model\n" + ",Orphan\n" + "keep,Good\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].id, "keep"); + EXPECT_EQ(result.warnings.size(), 1u); +} + +TEST(AssetCsvParseTest, MissingIdColumnThrows) { + const std::string csv = "model,serial\nX,Y\n"; + EXPECT_THROW(parse_asset_csv(csv), std::runtime_error); +} + +TEST(AssetCsvParseTest, EmptyDocumentThrows) { + EXPECT_THROW(parse_asset_csv(""), std::runtime_error); + EXPECT_THROW(parse_asset_csv(" \n\n"), std::runtime_error); +} + +TEST(AssetCsvParseTest, CaseInsensitiveHeadersAndAliases) { + const std::string csv = + "ID,Manufacturer,Serial_Number,Firmware_Version,HW_Rev\n" + "x,Bosch,SER-9,1.4.0,C1\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + const auto & e = result.entries[0]; + EXPECT_EQ(e.id, "x"); + EXPECT_EQ(e.manufacturer, "Bosch"); + EXPECT_EQ(e.serial, "SER-9"); + EXPECT_EQ(e.firmware, "1.4.0"); + EXPECT_EQ(e.hardware_rev, "C1"); +} + +TEST(AssetCsvParseTest, CrlfLineEndingsAndTrimming) { + const std::string csv = + "id, model \r\n" + " a , X \r\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].id, "a"); + EXPECT_EQ(result.entries[0].model, "X"); +} + +TEST(AssetCsvParseTest, FewerColumnsThanHeaderLeavesFieldsEmpty) { + const std::string csv = + "id,model,serial\n" + "a,ModelOnly\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].model, "ModelOnly"); + EXPECT_TRUE(result.entries[0].serial.empty()); +} + +// ── AssetEntry -> Component mapping ────────────────────────────────────────── + +TEST(AssetEntryToComponentTest, FullMapping) { + AssetEntry e; + e.id = "plc_1"; + e.manufacturer = "Siemens"; + e.model = "S7-1500"; + e.serial = "SN123"; + e.hardware_rev = "A2"; + e.firmware = "2.9.1"; + e.endpoint = "opc.tcp://10.0.0.5:4840"; + e.role = "controller"; + e.extras.emplace_back("location", "cell-3"); + + Component comp = asset_entry_to_component(e); + EXPECT_EQ(comp.id, "plc_1"); + EXPECT_EQ(comp.source, "inventory"); + EXPECT_EQ(comp.fqn, "/plc_1"); + EXPECT_EQ(comp.name, "Siemens S7-1500"); + EXPECT_EQ(comp.variant, "A2"); + EXPECT_TRUE(has_tag(comp, "controller")); + // description carries the remaining identity fields + extras + EXPECT_NE(comp.description.find("SN123"), std::string::npos); + EXPECT_NE(comp.description.find("2.9.1"), std::string::npos); + EXPECT_NE(comp.description.find("opc.tcp://10.0.0.5:4840"), std::string::npos); + EXPECT_NE(comp.description.find("location cell-3"), std::string::npos); +} + +TEST(AssetEntryToComponentTest, MinimalEntryOnlyId) { + AssetEntry e; + e.id = "bare"; + Component comp = asset_entry_to_component(e); + EXPECT_EQ(comp.id, "bare"); + EXPECT_EQ(comp.source, "inventory"); + EXPECT_TRUE(comp.name.empty()); + EXPECT_TRUE(comp.description.empty()); + EXPECT_TRUE(comp.variant.empty()); + EXPECT_TRUE(comp.tags.empty()); +} + +TEST(AssetEntryToComponentTest, NameFallsBackToManufacturerWhenNoModel) { + AssetEntry e; + e.id = "m"; + e.manufacturer = "OnlyVendor"; + Component comp = asset_entry_to_component(e); + EXPECT_EQ(comp.name, "OnlyVendor"); +} + +// ── Manifest `assets:` list ────────────────────────────────────────────────── + +TEST(ManifestAssetsTest, ParsesAssetsIntoComponents) { + const std::string yaml = R"( +manifest_version: "1.0" +assets: + - id: robot_arm + manufacturer: KUKA + model: KR-6 + serial: KS-77 + hardware_rev: R3 + firmware: 8.6 + role: actuator + location: line-B +)"; + ManifestParser parser; + Manifest manifest = parser.parse_string(yaml); + const Component * comp = find_component(manifest.components, "robot_arm"); + ASSERT_NE(comp, nullptr); + EXPECT_EQ(comp->source, "inventory"); + EXPECT_EQ(comp->name, "KUKA KR-6"); + EXPECT_EQ(comp->variant, "R3"); + EXPECT_TRUE(has_tag(*comp, "actuator")); + EXPECT_NE(comp->description.find("KS-77"), std::string::npos); + EXPECT_NE(comp->description.find("location line-B"), std::string::npos); +} + +TEST(ManifestAssetsTest, AreaAndExplicitOverrides) { + const std::string yaml = R"( +manifest_version: "1.0" +areas: + - id: floor + name: Floor +assets: + - id: pump_2 + manufacturer: Grundfos + model: CR-5 + area: floor + name: Main Pump + tags: [hydraulics] +)"; + ManifestParser parser; + Manifest manifest = parser.parse_string(yaml); + const Component * comp = find_component(manifest.components, "pump_2"); + ASSERT_NE(comp, nullptr); + EXPECT_EQ(comp->area, "floor"); + EXPECT_EQ(comp->name, "Main Pump"); // explicit override wins over derived + EXPECT_TRUE(has_tag(*comp, "hydraulics")); +} + +// ── ManifestManager CSV import ─────────────────────────────────────────────── + +class InventoryCsvManagerTest : public ::testing::Test { + protected: + void SetUp() override { + temp_dir_ = fs::temp_directory_path() / "asset_inventory_test"; + fs::create_directories(temp_dir_); + } + void TearDown() override { + fs::remove_all(temp_dir_); + } + std::string write_file(const std::string & name, const std::string & content) { + fs::path p = temp_dir_ / name; + std::ofstream ofs(p); + ofs << content; + ofs.close(); + return p.string(); + } + fs::path temp_dir_; +}; + +TEST_F(InventoryCsvManagerTest, LoadsAssetsFromCsvPath) { + const std::string manifest_path = write_file("base.yaml", "manifest_version: \"1.0\"\n"); + const std::string csv_path = write_file("inventory.csv", + "id,manufacturer,model,role\n" + "gw_1,Selfpatch,DiagBox,gateway\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + const Component * comp = find_component(comps, "gw_1"); + ASSERT_NE(comp, nullptr); + EXPECT_EQ(comp->source, "inventory"); + EXPECT_EQ(comp->name, "Selfpatch DiagBox"); + EXPECT_TRUE(has_tag(*comp, "gateway")); + EXPECT_EQ(mgr.get_inventory_csv_path(), csv_path); +} + +TEST_F(InventoryCsvManagerTest, MalformedCsvFailsLoad) { + const std::string manifest_path = write_file("base.yaml", "manifest_version: \"1.0\"\n"); + const std::string csv_path = write_file("bad.csv", "model,serial\nX,Y\n"); // no id column + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + EXPECT_FALSE(mgr.load_manifest(manifest_path, /*strict=*/false)); + EXPECT_TRUE(mgr.get_validation_result().has_errors()); +} + +TEST_F(InventoryCsvManagerTest, MissingCsvIsNotAnError) { + const std::string manifest_path = write_file("base.yaml", "manifest_version: \"1.0\"\n"); + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path((temp_dir_ / "does_not_exist.csv").string()); + EXPECT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); +} + +// ── Acceptance: CSV asset merges with protocol-discovered structure ────────── + +TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { + // Inventory (manifest-side) asset: identity, no live data. + auto result = parse_asset_csv( + "id,manufacturer,model,serial,role\n" + "drive_1,ABB,ACS880,SN-42,drive\n"); + ASSERT_EQ(result.entries.size(), 1u); + + LayerOutput inventory_out; + inventory_out.components.push_back(asset_entry_to_component(result.entries[0])); + + // Protocol-discovered component with the SAME id, carrying live topics. + Component runtime_comp; + runtime_comp.id = "drive_1"; + runtime_comp.name = "drive_1"; + runtime_comp.source = "node"; + runtime_comp.namespace_path = "/plant"; + runtime_comp.fqn = "/plant/drive_1"; + runtime_comp.topics.publishes = {"/plant/drive_1/status"}; + + LayerOutput runtime_out; + runtime_out.components.push_back(runtime_comp); + + MergePipeline pipeline(rclcpp::get_logger("test_asset_inventory")); + pipeline.add_layer(std::make_unique( + "inventory", inventory_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}, + {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}})); + pipeline.add_layer(std::make_unique( + "runtime", runtime_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}, + {FieldGroup::LIVE_DATA, MergePolicy::AUTHORITATIVE}})); + + auto merged = pipeline.execute(); + ASSERT_EQ(merged.components.size(), 1u); + const Component & c = merged.components[0]; + EXPECT_EQ(c.id, "drive_1"); + EXPECT_EQ(c.source, "inventory"); // provenance preserved + EXPECT_EQ(c.name, "ABB ACS880"); // inventory identity wins + EXPECT_TRUE(has_tag(c, "drive")); // inventory role tag survives + EXPECT_NE(c.description.find("SN-42"), std::string::npos); + ASSERT_EQ(c.topics.publishes.size(), 1u); // live data from the protocol layer + EXPECT_EQ(c.topics.publishes[0], "/plant/drive_1/status"); +} From 46fbc67dcb28bc31cb48fb33aa82c8495e2a5eed Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Wed, 1 Jul 2026 19:59:10 +0200 Subject: [PATCH 2/5] fix(gateway): harden asset inventory CSV parsing and merge placement (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. --- .../discovery/manifest/asset_inventory.cpp | 29 ++++++- .../discovery/manifest/manifest_parser.cpp | 26 ++++++- .../src/discovery/merge_pipeline.cpp | 6 ++ .../test/test_asset_inventory.cpp | 78 ++++++++++++++++++- 4 files changed, 131 insertions(+), 8 deletions(-) diff --git a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp index 8c3bd99ff..b387008b2 100644 --- a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp +++ b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp @@ -61,6 +61,9 @@ std::vector> tokenize(const std::string & text) { CsvField field; bool in_quotes = false; bool field_has_content = false; + // True while the current unquoted field has only seen leading whitespace, so a + // quote after it still opens a quoted field (e.g. `a, "x"` -> field == "x"). + bool field_only_leading_ws = true; const auto end_field = [&]() { if (!field.quoted) { @@ -69,6 +72,7 @@ std::vector> tokenize(const std::string & text) { row.push_back(std::move(field)); field = CsvField{}; field_has_content = false; + field_only_leading_ws = true; }; const auto end_row = [&]() { @@ -82,7 +86,16 @@ std::vector> tokenize(const std::string & text) { row.clear(); }; - for (std::size_t i = 0; i < text.size(); ++i) { + // Skip a leading UTF-8 BOM (0xEF 0xBB 0xBF). Excel's "CSV UTF-8" export always + // writes one; left in place it fuses onto the first header cell ("id"), + // which fails header detection and takes down the whole inventory import. + std::size_t start = 0; + if (text.size() >= 3 && static_cast(text[0]) == 0xEF && static_cast(text[1]) == 0xBB && + static_cast(text[2]) == 0xBF) { + start = 3; + } + + for (std::size_t i = start; i < text.size(); ++i) { const char c = text[i]; if (in_quotes) { if (c == '"') { @@ -98,10 +111,14 @@ std::vector> tokenize(const std::string & text) { continue; } - if (c == '"' && !field_has_content) { + if (c == '"' && field_only_leading_ws && !field.quoted) { + // Opening quote of a field, possibly after leading spaces: drop the + // accumulated leading whitespace so the quoted value stays verbatim. + field.value.clear(); in_quotes = true; field.quoted = true; field_has_content = true; + field_only_leading_ws = false; } else if (c == ',') { end_field(); } else if (c == '\r') { @@ -111,6 +128,9 @@ std::vector> tokenize(const std::string & text) { } else { field.value.push_back(c); field_has_content = true; + if (std::isspace(static_cast(c)) == 0) { + field_only_leading_ws = false; + } } } @@ -239,7 +259,10 @@ Component asset_entry_to_component(const AssetEntry & entry) { Component comp; comp.id = entry.id; comp.source = "inventory"; - comp.fqn = "/" + entry.id; + // fqn/namespace_path are intentionally left empty: a bare inventory asset + // carries no placement, so a synthetic "/id" must not override the real path + // of a discovered node it merges with. Placement comes from the manifest + // `namespace:` key (see parse_asset) when the operator declares it. // name <- " " (left empty when neither is set so the // consumer falls back to the id). diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp index 5ecafd25f..242cee949 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -304,7 +304,16 @@ Component ManifestParser::parse_asset(const YAML::Node & node) const { Component comp = asset_entry_to_component(entry); - // Optional tree-placement and presentation overrides. + // Optional tree-placement and presentation overrides. Every key listed in + // `reserved` above must be consumed here; otherwise it is silently dropped. + const std::string ns = get_string(node, "namespace"); + if (!ns.empty()) { + // Operator-declared placement: compute an authoritative fqn like a real + // component. Without a namespace, fqn stays empty so a merge with a + // discovered node keeps the node's real path instead of a synthetic "/id". + comp.namespace_path = ns; + comp.fqn = ns + "/" + comp.id; + } const std::string area = get_string(node, "area"); if (!area.empty()) { comp.area = area; @@ -321,6 +330,21 @@ Component ManifestParser::parse_asset(const YAML::Node & node) const { if (!explicit_description.empty()) { comp.description = explicit_description; } + const std::string variant = get_string(node, "variant"); + if (!variant.empty()) { + comp.variant = variant; // explicit variant wins over the hardware_rev derivation + } + const std::string type_val = get_string(node, "type"); + if (!type_val.empty()) { + comp.type = type_val; + } + const std::string translation_id = get_string(node, "translation_id"); + if (!translation_id.empty()) { + comp.translation_id = translation_id; + } + for (const auto & dep : get_string_vector(node, "depends_on")) { + comp.depends_on.push_back(dep); + } for (const auto & tag : get_string_vector(node, "tags")) { comp.tags.push_back(tag); } diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index a453febce..84f9d7e21 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -84,6 +84,12 @@ void merge_scalar(std::string & target, const std::string & source, MergeWinner } break; case MergeWinner::TARGET: + // An authoritative/higher-priority layer that supplies no value must not + // erase a value a lower-priority layer provides; fill the gap instead of + // clobbering (e.g. a bare inventory asset must not blank a node's fqn). + if (target.empty() && !source.empty()) { + target = source; + } break; } } diff --git a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp index ab33c6572..09b8bb3fa 100644 --- a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp +++ b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp @@ -195,6 +195,31 @@ TEST(AssetCsvParseTest, FewerColumnsThanHeaderLeavesFieldsEmpty) { EXPECT_TRUE(result.entries[0].serial.empty()); } +TEST(AssetCsvParseTest, Utf8BomStrippedFromHeader) { + // Excel "CSV UTF-8" export prepends a BOM; the first header cell must still be + // recognized as 'id' rather than "id" (which would fail id detection). + const std::string csv = + "\xEF\xBB\xBF" + "id,model\n" + "plc_1,S7-1500\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].id, "plc_1"); + EXPECT_EQ(result.entries[0].model, "S7-1500"); +} + +TEST(AssetCsvParseTest, LeadingSpaceBeforeQuotedFieldOpensQuote) { + // A quote after leading unquoted whitespace must open a quoted field, not be + // treated as a literal char (which would corrupt the value and add a column). + const std::string csv = + "id,model\n" + "a, \"Quoted, X\"\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].id, "a"); + EXPECT_EQ(result.entries[0].model, "Quoted, X"); +} + // ── AssetEntry -> Component mapping ────────────────────────────────────────── TEST(AssetEntryToComponentTest, FullMapping) { @@ -212,7 +237,10 @@ TEST(AssetEntryToComponentTest, FullMapping) { Component comp = asset_entry_to_component(e); EXPECT_EQ(comp.id, "plc_1"); EXPECT_EQ(comp.source, "inventory"); - EXPECT_EQ(comp.fqn, "/plc_1"); + // A bare inventory asset declares no placement, so fqn/namespace stay empty + // and never override the real path of a discovered node it merges with. + EXPECT_TRUE(comp.fqn.empty()); + EXPECT_TRUE(comp.namespace_path.empty()); EXPECT_EQ(comp.name, "Siemens S7-1500"); EXPECT_EQ(comp.variant, "A2"); EXPECT_TRUE(has_tag(comp, "controller")); @@ -293,6 +321,36 @@ manifest_version: "1.0" EXPECT_TRUE(has_tag(*comp, "hydraulics")); } +TEST(ManifestAssetsTest, NamespaceAndReservedKeysApplied) { + // namespace/variant/type/translation_id/depends_on were previously listed as + // reserved and then silently dropped; each must now land on the Component. + const std::string yaml = R"( +manifest_version: "1.0" +assets: + - id: drive_1 + manufacturer: ABB + model: ACS880 + hardware_rev: R1 + namespace: /plant + variant: R2 + type: actuator + translation_id: entity.drive + depends_on: [plc_1, bus_a] +)"; + ManifestParser parser; + Manifest manifest = parser.parse_string(yaml); + const Component * comp = find_component(manifest.components, "drive_1"); + ASSERT_NE(comp, nullptr); + EXPECT_EQ(comp->namespace_path, "/plant"); + EXPECT_EQ(comp->fqn, "/plant/drive_1"); // operator-declared placement + EXPECT_EQ(comp->variant, "R2"); // explicit variant beats hardware_rev + EXPECT_EQ(comp->type, "actuator"); + EXPECT_EQ(comp->translation_id, "entity.drive"); + ASSERT_EQ(comp->depends_on.size(), 2u); + EXPECT_EQ(comp->depends_on[0], "plc_1"); + EXPECT_EQ(comp->depends_on[1], "bus_a"); +} + // ── ManifestManager CSV import ─────────────────────────────────────────────── class InventoryCsvManagerTest : public ::testing::Test { @@ -374,16 +432,24 @@ TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { LayerOutput runtime_out; runtime_out.components.push_back(runtime_comp); + // Drive the merge with the SAME policies the production layers use + // (ManifestLayer / RuntimeLayer), so the test exercises the real hierarchy + // precedence instead of the default ENRICHMENT. MergePipeline pipeline(rclcpp::get_logger("test_asset_inventory")); pipeline.add_layer(std::make_unique( "inventory", inventory_out, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, - {FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}, - {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}})); + {FieldGroup::HIERARCHY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}, + {FieldGroup::STATUS, MergePolicy::FALLBACK}, + {FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}})); pipeline.add_layer(std::make_unique( "runtime", runtime_out, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}, - {FieldGroup::LIVE_DATA, MergePolicy::AUTHORITATIVE}})); + {FieldGroup::HIERARCHY, MergePolicy::FALLBACK}, + {FieldGroup::LIVE_DATA, MergePolicy::AUTHORITATIVE}, + {FieldGroup::STATUS, MergePolicy::AUTHORITATIVE}, + {FieldGroup::METADATA, MergePolicy::ENRICHMENT}})); auto merged = pipeline.execute(); ASSERT_EQ(merged.components.size(), 1u); @@ -395,4 +461,8 @@ TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { EXPECT_NE(c.description.find("SN-42"), std::string::npos); ASSERT_EQ(c.topics.publishes.size(), 1u); // live data from the protocol layer EXPECT_EQ(c.topics.publishes[0], "/plant/drive_1/status"); + // The bare inventory asset carries no placement, so the discovered node's real + // path must survive the merge - it must NOT be blanked or forced to "/drive_1". + EXPECT_EQ(c.namespace_path, "/plant"); + EXPECT_EQ(c.fqn, "/plant/drive_1"); } From 6d0c7ee780af401d18a0e97707d8cf38e7b97658 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 09:36:28 +0200 Subject: [PATCH 3/5] feat(gateway): put inventory assets on the structured asset identity 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 --- .../core/discovery/identity_merge.hpp | 26 +++---- .../discovery/manifest/asset_inventory.hpp | 11 ++- .../discovery/manifest/asset_inventory.cpp | 60 ++++++---------- .../discovery/manifest/manifest_parser.cpp | 2 +- .../test/test_asset_inventory.cpp | 70 +++++++++++++------ 5 files changed, 91 insertions(+), 78 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 104983a57..1dc39aefe 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 @@ -30,26 +30,28 @@ namespace discovery { * * `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. + * `Component.source` field ("manifest", "inventory", "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. * * 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". + * source, or a protocol-specific source tag) beats the hand-authored `manifest` and + * `inventory` (CSV / manifest `assets:` list) declarations, which beat 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", "plugin", "manifest", "config", "runtime", - "node", "topic", "heuristic"}; + std::vector source_precedence{"opcua", "s7", "ethernet_ip", "modbus", "ads", + "profinet", "plugin", "manifest", "inventory", "config", + "runtime", "node", "topic", "heuristic"}; }; /** diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp index d8857aefd..034c6ecad 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp @@ -31,12 +31,11 @@ namespace discovery { * canonical identity fields mirror the INV1 asset-identity model; anything not * covered by a canonical column is retained verbatim in @ref extras. * - * @note INV1 dependency: once the structured `AssetIdentity` field lands on the - * Component entity, `asset_entry_to_component` should populate it directly - * (with per-field provenance) instead of folding identity into the - * existing description / variant / tag fields. This struct is the single - * place the two import paths (CSV and manifest `assets:` list) converge, - * so the migration is localized to `asset_entry_to_component`. + * @note `asset_entry_to_component` populates the Component's structured + * `AssetIdentity` directly, with per-field provenance "inventory", so + * identity merging ranks hand-authored values against other sources per + * field. This struct is the single place the two import paths (CSV and + * manifest `assets:` list) converge. */ struct AssetEntry { std::string id; ///< Stable asset id (required); merge key into the tree diff --git a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp index b387008b2..811197bff 100644 --- a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp +++ b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp @@ -266,48 +266,30 @@ Component asset_entry_to_component(const AssetEntry & entry) { // name <- " " (left empty when neither is set so the // consumer falls back to the id). - std::string name = trim(entry.manufacturer + " " + entry.model); - comp.name = name; - - // variant carries the hardware revision. - comp.variant = entry.hardware_rev; - - // role becomes a tag for filtering. - if (!entry.role.empty()) { - comp.tags.push_back(entry.role); - } - - // description: human-readable identity summary. base is the manufacturer/model - // line; details append the remaining identity fields and any extras. - std::vector details; - if (!entry.serial.empty()) { - details.push_back("S/N " + entry.serial); - } - if (!entry.firmware.empty()) { - details.push_back("FW " + entry.firmware); - } - if (!entry.endpoint.empty()) { - details.push_back("endpoint " + entry.endpoint); - } - for (const auto & [key, value] : entry.extras) { - details.push_back(key + " " + value); - } - - std::string description = name; - if (!details.empty()) { - std::string joined; - for (std::size_t i = 0; i < details.size(); ++i) { - if (i > 0) { - joined += ", "; - } - joined += details[i]; + comp.name = trim(entry.manufacturer + " " + entry.model); + + // Structured nameplate: every canonical column lands on the typed identity + // with per-field provenance "inventory", so the identity merge can rank a + // hand-authored value against manifest / live protocol reads per field. + auto set_field = [&comp](std::string AssetIdentity::*member, const std::string & value, const char * prov_key) { + if (!value.empty()) { + comp.identity.*member = value; + comp.identity.provenance[prov_key] = "inventory"; } - if (!description.empty()) { - description += " "; + }; + set_field(&AssetIdentity::manufacturer, entry.manufacturer, "manufacturer"); + set_field(&AssetIdentity::model, entry.model, "model"); + set_field(&AssetIdentity::serial_number, entry.serial, "serial_number"); + set_field(&AssetIdentity::hardware_revision, entry.hardware_rev, "hardware_revision"); + set_field(&AssetIdentity::firmware_version, entry.firmware, "firmware_version"); + set_field(&AssetIdentity::network_endpoint, entry.endpoint, "network_endpoint"); + set_field(&AssetIdentity::role, entry.role, "role"); + for (const auto & [key, value] : entry.extras) { + if (!value.empty()) { + comp.identity.extra[key] = value; + comp.identity.provenance["extra." + key] = "inventory"; } - description += "(" + joined + ")"; } - comp.description = description; return comp; } diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp index 242cee949..48f4de4fd 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -332,7 +332,7 @@ Component ManifestParser::parse_asset(const YAML::Node & node) const { } const std::string variant = get_string(node, "variant"); if (!variant.empty()) { - comp.variant = variant; // explicit variant wins over the hardware_rev derivation + comp.variant = variant; // hardware_rev stays on the identity; variant is explicit only } const std::string type_val = get_string(node, "type"); if (!type_val.empty()) { diff --git a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp index 09b8bb3fa..b8ea5ed01 100644 --- a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp +++ b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp @@ -242,13 +242,22 @@ TEST(AssetEntryToComponentTest, FullMapping) { EXPECT_TRUE(comp.fqn.empty()); EXPECT_TRUE(comp.namespace_path.empty()); EXPECT_EQ(comp.name, "Siemens S7-1500"); - EXPECT_EQ(comp.variant, "A2"); - EXPECT_TRUE(has_tag(comp, "controller")); - // description carries the remaining identity fields + extras - EXPECT_NE(comp.description.find("SN123"), std::string::npos); - EXPECT_NE(comp.description.find("2.9.1"), std::string::npos); - EXPECT_NE(comp.description.find("opc.tcp://10.0.0.5:4840"), std::string::npos); - EXPECT_NE(comp.description.find("location cell-3"), std::string::npos); + // Every canonical column lands on the structured identity with per-field + // provenance "inventory" (not folded into description / variant / tags). + EXPECT_EQ(comp.identity.manufacturer, "Siemens"); + EXPECT_EQ(comp.identity.model, "S7-1500"); + EXPECT_EQ(comp.identity.serial_number, "SN123"); + EXPECT_EQ(comp.identity.hardware_revision, "A2"); + EXPECT_EQ(comp.identity.firmware_version, "2.9.1"); + EXPECT_EQ(comp.identity.network_endpoint, "opc.tcp://10.0.0.5:4840"); + EXPECT_EQ(comp.identity.role, "controller"); + EXPECT_EQ(comp.identity.extra.at("location"), "cell-3"); + EXPECT_EQ(comp.identity.provenance.at("manufacturer"), "inventory"); + EXPECT_EQ(comp.identity.provenance.at("serial_number"), "inventory"); + EXPECT_EQ(comp.identity.provenance.at("extra.location"), "inventory"); + EXPECT_TRUE(comp.variant.empty()); + EXPECT_TRUE(comp.tags.empty()); + EXPECT_TRUE(comp.description.empty()); } TEST(AssetEntryToComponentTest, MinimalEntryOnlyId) { @@ -261,6 +270,7 @@ TEST(AssetEntryToComponentTest, MinimalEntryOnlyId) { EXPECT_TRUE(comp.description.empty()); EXPECT_TRUE(comp.variant.empty()); EXPECT_TRUE(comp.tags.empty()); + EXPECT_TRUE(comp.identity.empty()); } TEST(AssetEntryToComponentTest, NameFallsBackToManufacturerWhenNoModel) { @@ -292,10 +302,14 @@ manifest_version: "1.0" ASSERT_NE(comp, nullptr); EXPECT_EQ(comp->source, "inventory"); EXPECT_EQ(comp->name, "KUKA KR-6"); - EXPECT_EQ(comp->variant, "R3"); - EXPECT_TRUE(has_tag(*comp, "actuator")); - EXPECT_NE(comp->description.find("KS-77"), std::string::npos); - EXPECT_NE(comp->description.find("location line-B"), std::string::npos); + EXPECT_EQ(comp->identity.manufacturer, "KUKA"); + EXPECT_EQ(comp->identity.model, "KR-6"); + EXPECT_EQ(comp->identity.serial_number, "KS-77"); + EXPECT_EQ(comp->identity.hardware_revision, "R3"); + EXPECT_EQ(comp->identity.firmware_version, "8.6"); + EXPECT_EQ(comp->identity.role, "actuator"); + EXPECT_EQ(comp->identity.extra.at("location"), "line-B"); + EXPECT_EQ(comp->identity.provenance.at("model"), "inventory"); } TEST(ManifestAssetsTest, AreaAndExplicitOverrides) { @@ -342,8 +356,9 @@ manifest_version: "1.0" const Component * comp = find_component(manifest.components, "drive_1"); ASSERT_NE(comp, nullptr); EXPECT_EQ(comp->namespace_path, "/plant"); - EXPECT_EQ(comp->fqn, "/plant/drive_1"); // operator-declared placement - EXPECT_EQ(comp->variant, "R2"); // explicit variant beats hardware_rev + EXPECT_EQ(comp->fqn, "/plant/drive_1"); // operator-declared placement + EXPECT_EQ(comp->variant, "R2"); // explicit variant only + EXPECT_EQ(comp->identity.hardware_revision, "R1"); // hardware_rev lives on the identity EXPECT_EQ(comp->type, "actuator"); EXPECT_EQ(comp->translation_id, "entity.drive"); ASSERT_EQ(comp->depends_on.size(), 2u); @@ -387,7 +402,7 @@ TEST_F(InventoryCsvManagerTest, LoadsAssetsFromCsvPath) { ASSERT_NE(comp, nullptr); EXPECT_EQ(comp->source, "inventory"); EXPECT_EQ(comp->name, "Selfpatch DiagBox"); - EXPECT_TRUE(has_tag(*comp, "gateway")); + EXPECT_EQ(comp->identity.role, "gateway"); EXPECT_EQ(mgr.get_inventory_csv_path(), csv_path); } @@ -411,7 +426,7 @@ TEST_F(InventoryCsvManagerTest, MissingCsvIsNotAnError) { // ── Acceptance: CSV asset merges with protocol-discovered structure ────────── TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { - // Inventory (manifest-side) asset: identity, no live data. + // Inventory (manifest-side) asset: identity, no live data, no placement. auto result = parse_asset_csv( "id,manufacturer,model,serial,role\n" "drive_1,ABB,ACS880,SN-42,drive\n"); @@ -420,7 +435,10 @@ TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { LayerOutput inventory_out; inventory_out.components.push_back(asset_entry_to_component(result.entries[0])); - // Protocol-discovered component with the SAME id, carrying live topics. + // Protocol-discovered component with the SAME id, carrying live topics and + // its own (lower-authority) identity guesses: a conflicting manufacturer that + // must lose to the hand-authored inventory, and a firmware version only the + // runtime knows, which must fill the gap. Component runtime_comp; runtime_comp.id = "drive_1"; runtime_comp.name = "drive_1"; @@ -428,6 +446,8 @@ TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { runtime_comp.namespace_path = "/plant"; runtime_comp.fqn = "/plant/drive_1"; runtime_comp.topics.publishes = {"/plant/drive_1/status"}; + runtime_comp.identity.manufacturer = "guessed-vendor"; + runtime_comp.identity.firmware_version = "fw-2.1.0"; LayerOutput runtime_out; runtime_out.components.push_back(runtime_comp); @@ -455,14 +475,24 @@ TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) { ASSERT_EQ(merged.components.size(), 1u); const Component & c = merged.components[0]; EXPECT_EQ(c.id, "drive_1"); - EXPECT_EQ(c.source, "inventory"); // provenance preserved - EXPECT_EQ(c.name, "ABB ACS880"); // inventory identity wins - EXPECT_TRUE(has_tag(c, "drive")); // inventory role tag survives - EXPECT_NE(c.description.find("SN-42"), std::string::npos); + EXPECT_EQ(c.source, "inventory"); // provenance preserved + EXPECT_EQ(c.name, "ABB ACS880"); // inventory identity wins ASSERT_EQ(c.topics.publishes.size(), 1u); // live data from the protocol layer EXPECT_EQ(c.topics.publishes[0], "/plant/drive_1/status"); // The bare inventory asset carries no placement, so the discovered node's real // path must survive the merge - it must NOT be blanked or forced to "/drive_1". EXPECT_EQ(c.namespace_path, "/plant"); EXPECT_EQ(c.fqn, "/plant/drive_1"); + // Identity merges per field with provenance: the hand-authored inventory + // ("inventory", above "node" in the default precedence) keeps the fields it + // declares; the runtime's conflicting manufacturer guess loses, while its + // runtime-only firmware version fills the gap and is attributed to "node". + EXPECT_EQ(c.identity.manufacturer, "ABB"); + EXPECT_EQ(c.identity.model, "ACS880"); + EXPECT_EQ(c.identity.serial_number, "SN-42"); + EXPECT_EQ(c.identity.role, "drive"); + EXPECT_EQ(c.identity.firmware_version, "fw-2.1.0"); + EXPECT_EQ(c.identity.provenance.at("manufacturer"), "inventory"); + EXPECT_EQ(c.identity.provenance.at("serial_number"), "inventory"); + EXPECT_EQ(c.identity.provenance.at("firmware_version"), "node"); } From a5a218d0933d6b5d9120aee1e42a609ba77c8f32 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 14:09:49 +0200 Subject: [PATCH 4/5] docs(gateway): document discovery.inventory.csv_path and fix stale asset comment Add the manual asset inventory CHANGELOG entry and rewrite the asset_entry_to_component doxygen to match the structured-identity behavior. Refs #490 --- src/ros2_medkit_gateway/CHANGELOG.rst | 4 ++++ .../discovery/manifest/asset_inventory.hpp | 22 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/ros2_medkit_gateway/CHANGELOG.rst b/src/ros2_medkit_gateway/CHANGELOG.rst index 15db606fb..5d130c134 100644 --- a/src/ros2_medkit_gateway/CHANGELOG.rst +++ b/src/ros2_medkit_gateway/CHANGELOG.rst @@ -2,6 +2,10 @@ Changelog for package ros2_medkit_gateway ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Manual asset inventory: a manifest ``assets:`` list and a new ``discovery.inventory.csv_path`` parameter declare assets that no protocol layer can describe (or fully describe). The CSV recognizes the canonical columns ``id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role`` (plus common aliases) and keeps any other column as an extra; RFC-4180-style quoting is honored and rows without an ``id`` are skipped with a warning. Each asset becomes a Component with ``source = "inventory"`` and a structured asset identity carrying per-field provenance, appended to the base manifest on every load / reload and merged into the tree by id alongside protocol-discovered structure. The CSV is size-capped at 1 MiB before being read; a missing file is skipped with a warning (mirrors ``fragments_dir``), while an unreadable or malformed one fails the load / reload. Requires a manifest-backed discovery mode (``manifest_only`` / ``hybrid`` with ``discovery.manifest_path`` set); empty = disabled (default) (`#490 `_) + 0.6.0 (2026-06-22) ------------------ * SOVD entity status and lifecycle control endpoints: ``GET /apps/{id}/status`` and ``GET /components/{id}/status``, plus lifecycle control routes backed by a new ``LifecycleProvider`` plugin interface and plugin-manager routing. Control returns ``501 Not Implemented`` until a provider is registered; the routes are RBAC-gated, advertised via a ``status`` link on app and component detail, and declared under the OpenAPI ``Lifecycle`` tag (`#437 `_) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp index 034c6ecad..6aef90817 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp @@ -84,16 +84,22 @@ AssetCsvResult parse_asset_csv(const std::string & csv_text); * * The component is tagged `source = "inventory"` so its provenance stays * visible after the merge pipeline combines it (by id) with protocol-discovered - * structure. Identity is projected onto the existing Component fields until the - * INV1 structured identity model is available (see the AssetEntry note): - * - name <- " " (falls back to id at display time) - * - variant <- hardware_rev - * - tags <- role (when present) - * - description <- human-readable identity summary (serial, firmware, - * endpoint, extras) + * structure. Canonical fields land on the structured `Component::identity` + * with per-field provenance `"inventory"` (see the AssetEntry note): + * - name <- " " (left empty when neither is set so + * consumers fall back to the id) + * - identity.manufacturer / model / serial_number / hardware_revision / + * firmware_version / network_endpoint / role <- the matching entry + * fields; empty fields are skipped and record no provenance + * - identity.extra[header] <- non-empty extras, provenance keyed + * `extra.
` + * + * `fqn` / `namespace_path` are left empty: a bare inventory asset carries no + * placement, so a discovered node it merges with keeps its real path. * * @param entry Asset entry (must have a non-empty id). - * @return Component with `source = "inventory"` and identity populated. + * @return Component with `source = "inventory"` and structured identity + * populated. */ Component asset_entry_to_component(const AssetEntry & entry); From 4058cde38415e6f31717ffac638c5206b7fbef53 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 22:51:07 +0200 Subject: [PATCH 5/5] fix(gateway): resolve inventory id collisions and align asset key handling 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 --- docs/config/manifest-schema.rst | 96 +++++++++ src/ros2_medkit_gateway/CHANGELOG.rst | 2 +- src/ros2_medkit_gateway/README.md | 12 +- .../config/gateway_params.yaml | 13 ++ .../discovery/manifest/asset_inventory.hpp | 52 ++++- .../discovery/manifest/manifest_manager.hpp | 19 +- .../discovery/manifest/asset_inventory.cpp | 76 ++++--- .../discovery/manifest/manifest_manager.cpp | 53 ++++- .../discovery/manifest/manifest_parser.cpp | 36 +--- src/ros2_medkit_gateway/src/gateway_node.cpp | 8 +- .../test/test_asset_inventory.cpp | 197 ++++++++++++++++++ 11 files changed, 486 insertions(+), 78 deletions(-) diff --git a/docs/config/manifest-schema.rst b/docs/config/manifest-schema.rst index 81f520f2a..af0675a2a 100644 --- a/docs/config/manifest-schema.rst +++ b/docs/config/manifest-schema.rst @@ -28,6 +28,7 @@ A manifest file has the following top-level structure: areas: [] # Optional - area definitions components: [] # Optional - component definitions + assets: [] # Optional - manual asset inventory entries apps: [] # Optional - app definitions functions: [] # Optional - function definitions scripts: [] # Optional - pre-defined script entries @@ -350,6 +351,101 @@ Example type: "sensor" area: perception +Assets +------ + +Assets declare manually inventoried equipment that no protocol layer can +describe (or fully describe): unnetworked devices, third-party hardware, spare +nameplate data. Each asset becomes a Component with ``source: "inventory"`` and +a structured asset identity carrying per-field provenance ``"inventory"``, and +merges into the entity tree by ``id`` alongside protocol-discovered structure. +In the identity merge, ``inventory`` ranks below ``manifest`` and live protocol +reads but above runtime guesses. + +Schema +~~~~~~ + +.. code-block:: yaml + + assets: + - id: string # Required - stable asset id (merge key) + manufacturer: string # Optional - vendor / OEM + model: string # Optional - model / order code + serial: string # Optional - serial number + hardware_rev: string # Optional - hardware revision + firmware: string # Optional - firmware / software version + endpoint: string # Optional - network endpoint (URL / host:port) + role: string # Optional - functional role + area: string # Optional - Area id placing the asset in the tree + namespace: string # Optional - operator-declared placement (sets fqn) + name: string # Optional - display name (default: " ") + description: string # Optional - detailed description + variant: string # Optional - hardware variant identifier + type: string # Optional - component type + translation_id: string # Optional - internationalization key + parent_component_id: string # Optional - parent component ID + depends_on: [string] # Optional - component IDs this asset depends on + tags: [string] # Optional - tags for filtering + any_other_key: string # Kept verbatim as an identity extra + +Fields +~~~~~~ + +The identity keys accept the same aliases as the CSV import: +``serial_number`` for ``serial``, ``hardware_revision`` / ``hw_rev`` for +``hardware_rev``, and ``firmware_version`` / ``fw`` for ``firmware``. Aliased +keys land on the typed identity fields, not in the extras. Any scalar key not +listed above is preserved as an identity extra. + +Placement is optional: without ``area`` (and ``namespace``) the asset is +reachable at ``/components/{id}`` and in the flat component list, but does not +appear under any Area. An ``area`` must reference an area defined in the +manifest, otherwise validation fails (rule R006). + +Example +~~~~~~~ + +.. code-block:: yaml + + areas: + - id: cell-3 + name: "Cell 3" + + assets: + - id: hyd-pump-2 + manufacturer: Grundfos + model: CR-5 + serial_number: "GP-2214-0087" + area: cell-3 + role: pump + rack: R2 # kept as identity extra "rack" + +CSV Inventory Import +~~~~~~~~~~~~~~~~~~~~ + +The same asset entries can be bulk-imported from a CSV file via the +``discovery.inventory.csv_path`` gateway parameter (requires a manifest-backed +discovery mode: ``manifest_only``, or ``hybrid`` with +``discovery.manifest_path`` set; empty = disabled). The CSV is re-read on every +manifest load / reload and appended to the merged manifest before validation. + +- **Columns**: the header row is matched case-insensitively after trimming. + Canonical columns are ``id`` (required), ``manufacturer``, ``model``, + ``serial``, ``hardware_rev``, ``firmware``, ``endpoint``, ``role`` and + ``area``, with the same aliases as the ``assets:`` list. Any other column is + kept as an identity extra keyed by its original header. +- **Quoting**: RFC-4180-style; double-quoted fields may contain commas, + newlines and escaped quotes (``""``). Unquoted fields are whitespace-trimmed; + a UTF-8 BOM (Excel "CSV UTF-8" export) is stripped. +- **Size cap**: the file is rejected before reading if it exceeds 1 MiB. +- **Row policy**: rows without an ``id`` are skipped with a warning; for + duplicate ids within the CSV the first row wins. A row whose id is already a + manifest component keeps the manifest definition and folds the row's identity + in as gap-fill; a row whose id collides with any other manifest entity is + skipped, and an unknown ``area`` value is dropped (asset kept, placement-less). + None of these fail the load. A missing file is skipped with a warning; an + unreadable or malformed file (e.g. no ``id`` column) fails the load. + Apps ---- diff --git a/src/ros2_medkit_gateway/CHANGELOG.rst b/src/ros2_medkit_gateway/CHANGELOG.rst index 5d130c134..ec7423f89 100644 --- a/src/ros2_medkit_gateway/CHANGELOG.rst +++ b/src/ros2_medkit_gateway/CHANGELOG.rst @@ -4,7 +4,7 @@ Changelog for package ros2_medkit_gateway Forthcoming ----------- -* Manual asset inventory: a manifest ``assets:`` list and a new ``discovery.inventory.csv_path`` parameter declare assets that no protocol layer can describe (or fully describe). The CSV recognizes the canonical columns ``id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role`` (plus common aliases) and keeps any other column as an extra; RFC-4180-style quoting is honored and rows without an ``id`` are skipped with a warning. Each asset becomes a Component with ``source = "inventory"`` and a structured asset identity carrying per-field provenance, appended to the base manifest on every load / reload and merged into the tree by id alongside protocol-discovered structure. The CSV is size-capped at 1 MiB before being read; a missing file is skipped with a warning (mirrors ``fragments_dir``), while an unreadable or malformed one fails the load / reload. Requires a manifest-backed discovery mode (``manifest_only`` / ``hybrid`` with ``discovery.manifest_path`` set); empty = disabled (default) (`#490 `_) +* Manual asset inventory: a manifest ``assets:`` list and a new ``discovery.inventory.csv_path`` parameter declare assets that no protocol layer can describe (or fully describe). Both paths recognize the canonical names ``id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role, area`` plus the shared aliases (``serial_number``, ``hardware_revision`` / ``hw_rev``, ``firmware_version`` / ``fw``) and keep any other column / key as an extra; RFC-4180-style quoting is honored. Each asset becomes a Component with ``source = "inventory"`` and a structured asset identity carrying per-field provenance, appended to the base manifest on every load / reload and merged into the tree by id alongside protocol-discovered structure; ``area`` places the asset under an Area, without it the asset appears only in the flat component list. CSV rows never fail the load: rows without an ``id`` are skipped with a warning, for duplicate ids the first row wins, a row whose id is already a manifest component keeps the manifest definition (the row's identity is folded in as gap-fill), and an unknown ``area`` is dropped with a warning. The CSV is size-capped at 1 MiB before being read; a missing file is skipped with a warning (mirrors ``fragments_dir``), while an unreadable or malformed one fails the load / reload. Requires a manifest-backed discovery mode (``manifest_only`` / ``hybrid`` with ``discovery.manifest_path`` set); empty = disabled (default) (`#490 `_) 0.6.0 (2026-06-22) ------------------ diff --git a/src/ros2_medkit_gateway/README.md b/src/ros2_medkit_gateway/README.md index 45d2ede2d..d4b407973 100644 --- a/src/ros2_medkit_gateway/README.md +++ b/src/ros2_medkit_gateway/README.md @@ -1585,12 +1585,12 @@ records **per-field provenance** (which source set each field) under `_provenanc 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. 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. + ("manifest", "inventory", "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` > `inventory` (CSV import / manifest `assets:` list) > + `config` > runtime sources. A higher-authority source overrides a field; lower-authority + sources only fill gaps; unknown sources rank lowest. Empty values never overwrite. 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 diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index 37e448ba6..4b0c29609 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -249,6 +249,19 @@ ros2_medkit_gateway: # Strict manifest validation (reject invalid manifests) manifest_strict_validation: true + # Manual asset inventory CSV (requires a manifest-backed mode: + # manifest_only, or hybrid with manifest_path set). Each row becomes a + # Component with source "inventory", appended to the manifest on every + # load/reload and merged into the tree by id. Canonical columns: + # id (required), manufacturer, model, serial, hardware_rev, firmware, + # endpoint, role, area (aliases: serial_number, hardware_revision/hw_rev, + # firmware_version/fw); any other column is kept as an identity extra. + # RFC-4180 quoting, 1 MiB size cap. Malformed rows / id collisions are + # skipped with a warning (manifest wins); a missing file is skipped, an + # unreadable or malformed file fails the load. Empty = disabled (default). + inventory: + csv_path: "" + # Configurable main layers (hybrid mode only) # Set false to disable a layer - useful when running manifest+plugin or runtime+plugin # without the other default layer. diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp index 6aef90817..68449ac5c 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp @@ -46,11 +46,47 @@ struct AssetEntry { std::string firmware; ///< Firmware / software version std::string endpoint; ///< Network endpoint (URL / host:port) std::string role; ///< Functional role (controller, sensor, ...) + std::string area; ///< Optional Area id placing the asset in the tree /// Non-canonical columns, kept in declared order (header -> value). std::vector> extras; }; +/** + * @brief Canonical destination of an inventory column / manifest asset key. + */ +enum class AssetColumn { + kIgnore, ///< Empty header cell: drop the value + kExtra, ///< Not a canonical name: keep as identity extra + kId, + kManufacturer, + kModel, + kSerial, + kHardwareRev, + kFirmware, + kEndpoint, + kRole, + kArea, +}; + +/** + * @brief Map a column / key name to its canonical destination. + * + * The single alias table shared by both import paths (CSV header cells and + * manifest `assets:` keys): names are trimmed and matched case-insensitively; + * `serial_number`, `hardware_revision` / `hw_rev` and `firmware_version` / `fw` + * map to their typed fields. Empty names yield kIgnore; unknown names kExtra. + */ +AssetColumn asset_column(const std::string & name); + +/** + * @brief Assign `value` to the @ref AssetEntry field selected by `column`. + * + * kExtra appends (`header`, `value`) to `entry.extras` (empty values are + * dropped); kIgnore is a no-op. + */ +void assign_asset_field(AssetEntry & entry, AssetColumn column, const std::string & header, const std::string & value); + /** * @brief Result of parsing a CSV inventory document. */ @@ -62,16 +98,17 @@ struct AssetCsvResult { /** * @brief Parse a CSV inventory document into asset entries. * - * The first non-empty line is the header. Column names are matched - * case-insensitively after trimming; recognized canonical names are - * `id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role` - * (a small set of common aliases is also accepted). Any other column is + * The first non-empty line is the header. Column names are matched via + * ::asset_column; recognized canonical names are + * `id, manufacturer, model, serial, hardware_rev, firmware, endpoint, role, + * area` (a small set of common aliases is also accepted). Any other column is * preserved as an extra keyed by its original (trimmed) header name. * * RFC-4180-style quoting is honored: double-quoted fields may contain commas, * newlines, and escaped quotes (`""`). Unquoted fields are whitespace-trimmed. * Blank lines are skipped. Rows whose `id` is empty are skipped and reported in - * @ref AssetCsvResult::warnings. + * @ref AssetCsvResult::warnings; so are rows repeating an earlier row's id + * (first row wins). * * @param csv_text Full CSV document. * @return Parsed entries plus any non-fatal warnings. @@ -95,7 +132,10 @@ AssetCsvResult parse_asset_csv(const std::string & csv_text); * `extra.
` * * `fqn` / `namespace_path` are left empty: a bare inventory asset carries no - * placement, so a discovered node it merges with keeps its real path. + * placement, so a discovered node it merges with keeps its real path. An + * `area` on the entry lands on `Component::area` (structural placement, no + * provenance entry); without one the asset appears only in the flat component + * list, never under an Area. * * @param entry Asset entry (must have a non-empty id). * @return Component with `source = "inventory"` and structured identity diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp index 9b0ca1b3a..c299c6766 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp @@ -123,11 +123,17 @@ class ManifestManager { * * When set, every `load_manifest` / `reload_manifest` call parses the CSV * (columns `id, manufacturer, model, serial, hardware_rev, firmware, - * endpoint, role`, plus any extra columns) and appends one Component per row - * to the base manifest before validation. Each asset merges into the entity - * tree by id, combining with protocol-discovered structure the same way a - * manifest component does. A parse failure (or a missing `id` column) is - * recorded as a validation error and fails the load. + * endpoint, role, area`, plus any extra columns) and appends one Component + * per row to the base manifest before validation. Each asset merges into the + * entity tree by id, combining with protocol-discovered structure the same + * way a manifest component does. A parse failure (or a missing `id` column) + * is recorded as a validation error and fails the load. + * + * Id collisions never fail the load: within the CSV the first row wins; + * a row whose id matches a manifest component keeps the manifest definition + * and folds the row's identity in as gap-fill; a row whose id matches any + * other manifest entity, or that names an unknown `area`, is skipped + * (respectively has its placement dropped) with a warning. * * Call with an empty string to disable CSV import. * @@ -294,6 +300,9 @@ class ManifestManager { /// success (or when no CSV is configured / the file is absent); false when /// the CSV cannot be read or parsed. Parse errors are appended to /// `validation_result_` so callers see them in the normal error flow. + /// Unlike fragments, id collisions are resolved HERE (manifest wins, + /// see set_inventory_csv_path) so a CSV row can never trip the validator's + /// hard duplicate-id errors and abort the load. bool apply_inventory_csv(Manifest & base); std::optional manifest_; diff --git a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp index 811197bff..09b01696b 100644 --- a/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp +++ b/src/ros2_medkit_gateway/src/core/discovery/manifest/asset_inventory.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include namespace ros2_medkit_gateway { @@ -142,75 +143,82 @@ std::vector> tokenize(const std::string & text) { return rows; } -// Canonical destination for a header column. -enum class Column { kIgnore, kExtra, kId, kManufacturer, kModel, kSerial, kHardwareRev, kFirmware, kEndpoint, kRole }; +} // namespace -Column canonical_column(const std::string & header_lower) { +AssetColumn asset_column(const std::string & name) { + const std::string header_lower = to_lower(trim(name)); + if (header_lower.empty()) { + return AssetColumn::kIgnore; + } if (header_lower == "id") { - return Column::kId; + return AssetColumn::kId; } if (header_lower == "manufacturer") { - return Column::kManufacturer; + return AssetColumn::kManufacturer; } if (header_lower == "model") { - return Column::kModel; + return AssetColumn::kModel; } if (header_lower == "serial" || header_lower == "serial_number") { - return Column::kSerial; + return AssetColumn::kSerial; } if (header_lower == "hardware_rev" || header_lower == "hardware_revision" || header_lower == "hw_rev") { - return Column::kHardwareRev; + return AssetColumn::kHardwareRev; } if (header_lower == "firmware" || header_lower == "firmware_version" || header_lower == "fw") { - return Column::kFirmware; + return AssetColumn::kFirmware; } if (header_lower == "endpoint") { - return Column::kEndpoint; + return AssetColumn::kEndpoint; } if (header_lower == "role") { - return Column::kRole; + return AssetColumn::kRole; } - return Column::kExtra; + if (header_lower == "area") { + return AssetColumn::kArea; + } + return AssetColumn::kExtra; } -void assign(AssetEntry & entry, Column column, const std::string & header, const std::string & value) { +void assign_asset_field(AssetEntry & entry, AssetColumn column, const std::string & header, const std::string & value) { switch (column) { - case Column::kId: + case AssetColumn::kId: entry.id = value; break; - case Column::kManufacturer: + case AssetColumn::kManufacturer: entry.manufacturer = value; break; - case Column::kModel: + case AssetColumn::kModel: entry.model = value; break; - case Column::kSerial: + case AssetColumn::kSerial: entry.serial = value; break; - case Column::kHardwareRev: + case AssetColumn::kHardwareRev: entry.hardware_rev = value; break; - case Column::kFirmware: + case AssetColumn::kFirmware: entry.firmware = value; break; - case Column::kEndpoint: + case AssetColumn::kEndpoint: entry.endpoint = value; break; - case Column::kRole: + case AssetColumn::kRole: entry.role = value; break; - case Column::kExtra: + case AssetColumn::kArea: + entry.area = value; + break; + case AssetColumn::kExtra: if (!value.empty()) { entry.extras.emplace_back(header, value); } break; - case Column::kIgnore: + case AssetColumn::kIgnore: break; } } -} // namespace - AssetCsvResult parse_asset_csv(const std::string & csv_text) { AssetCsvResult result; @@ -221,15 +229,15 @@ AssetCsvResult parse_asset_csv(const std::string & csv_text) { // Build the column map from the header row. const auto & header_row = rows.front(); - std::vector columns; + std::vector columns; std::vector header_names; columns.reserve(header_row.size()); header_names.reserve(header_row.size()); bool has_id = false; for (const auto & cell : header_row) { const std::string name = trim(cell.value); - Column col = name.empty() ? Column::kIgnore : canonical_column(to_lower(name)); - if (col == Column::kId) { + AssetColumn col = asset_column(name); + if (col == AssetColumn::kId) { has_id = true; } columns.push_back(col); @@ -239,16 +247,24 @@ AssetCsvResult parse_asset_csv(const std::string & csv_text) { throw std::runtime_error("asset CSV: missing required 'id' column"); } + std::unordered_set seen_ids; for (std::size_t r = 1; r < rows.size(); ++r) { const auto & data_row = rows[r]; AssetEntry entry; for (std::size_t c = 0; c < columns.size() && c < data_row.size(); ++c) { - assign(entry, columns[c], header_names[c], data_row[c].value); + assign_asset_field(entry, columns[c], header_names[c], data_row[c].value); } if (entry.id.empty()) { result.warnings.push_back("asset CSV: row " + std::to_string(r + 1) + " skipped (empty id)"); continue; } + // Dedup within the document: first row wins, later duplicates are dropped so + // one bad export line cannot fail the whole manifest load downstream. + if (!seen_ids.insert(entry.id).second) { + result.warnings.push_back("asset CSV: row " + std::to_string(r + 1) + " skipped (duplicate id '" + entry.id + + "', first row wins)"); + continue; + } result.entries.push_back(std::move(entry)); } @@ -263,6 +279,8 @@ Component asset_entry_to_component(const AssetEntry & entry) { // carries no placement, so a synthetic "/id" must not override the real path // of a discovered node it merges with. Placement comes from the manifest // `namespace:` key (see parse_asset) when the operator declares it. + // `area` is structural placement, not identity: no provenance entry. + comp.area = entry.area; // name <- " " (left empty when neither is set so the // consumer falls back to the id). diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp index 36100e239..3d3e07791 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_manager.cpp @@ -20,7 +20,9 @@ #include #include #include +#include +#include "ros2_medkit_gateway/core/discovery/identity_merge.hpp" #include "ros2_medkit_gateway/core/discovery/manifest/asset_inventory.hpp" namespace ros2_medkit_gateway { @@ -541,10 +543,57 @@ bool ManifestManager::apply_inventory_csv(Manifest & base) { for (const auto & warn : parsed.warnings) { log_warn("Inventory CSV: " + warn); } + + // Collision policy: a CSV row never fails the load (the CSV is a bulk + // import; one bad line must not take the gateway down) and never beats the + // operator-authored manifest. Duplicate ids within the CSV were already + // reduced to the first row by parse_asset_csv. An id already defined as a + // manifest component keeps the manifest definition; the CSV identity is + // folded in as gap-fill ("inventory" ranks below "manifest"). An id + // colliding with any other entity kind, or an unknown `area`, is dropped + // with a warning so the merged manifest still passes R002-R012 / R006. + std::unordered_set area_ids; + std::unordered_set non_component_ids; + for (const auto & area : base.areas) { + area_ids.insert(area.id); + non_component_ids.insert(area.id); + } + for (const auto & app : base.apps) { + non_component_ids.insert(app.id); + } + for (const auto & func : base.functions) { + non_component_ids.insert(func.id); + } + for (const auto & script : base.scripts) { + non_component_ids.insert(script.id); + } + + std::size_t added = 0; for (const auto & entry : parsed.entries) { - base.components.push_back(asset_entry_to_component(entry)); + Component comp = asset_entry_to_component(entry); + if (!comp.area.empty() && area_ids.count(comp.area) == 0) { + log_warn("Inventory CSV: asset '" + comp.id + "' references unknown area '" + comp.area + + "' (placement dropped, asset kept)"); + comp.area.clear(); + } + auto existing = std::find_if(base.components.begin(), base.components.end(), [&](const Component & c) { + return c.id == comp.id; + }); + if (existing != base.components.end()) { + stamp_identity_provenance(existing->identity, existing->source); + merge_identity(existing->identity, comp.identity, "inventory", IdentityMergeConfig{}); + log_warn("Inventory CSV: id '" + comp.id + + "' is already defined in the manifest; manifest wins, CSV identity folded as gap-fill"); + continue; + } + if (non_component_ids.count(comp.id) != 0) { + log_warn("Inventory CSV: id '" + comp.id + "' collides with a non-component manifest entity (row skipped)"); + continue; + } + base.components.push_back(std::move(comp)); + ++added; } - log_info("Inventory CSV loaded: " + std::to_string(parsed.entries.size()) + " assets from " + inventory_csv_path_); + log_info("Inventory CSV loaded: " + std::to_string(added) + " assets from " + inventory_csv_path_); } catch (const std::exception & e) { validation_result_.add_error("INVENTORY_CSV", e.what(), inventory_csv_path_); log_error("Failed to parse inventory CSV '" + inventory_csv_path_ + "': " + e.what()); diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp index 48f4de4fd..404052959 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -269,43 +269,33 @@ Component ManifestParser::parse_component(const YAML::Node & node) const { } Component ManifestParser::parse_asset(const YAML::Node & node) const { - // Structural / identity keys handled explicitly; any other scalar key is - // retained verbatim as an extra so operator-specific columns are not lost. - static const std::unordered_set reserved = { - "id", "manufacturer", "model", "serial", "hardware_rev", "firmware", - "endpoint", "role", "area", "tags", "parent_component_id", "name", - "description", "namespace", "variant", "type", "translation_id", "depends_on"}; + // Structural / presentation keys handled explicitly below. Identity keys are + // routed through the shared asset_column alias table, so `assets:` accepts + // exactly the names the CSV import does (serial / serial_number, ...); any + // other scalar key is retained verbatim as an extra so operator-specific + // columns are not lost. + static const std::unordered_set structural = {"tags", "parent_component_id", "name", + "description", "namespace", "variant", + "type", "translation_id", "depends_on"}; AssetEntry entry; - entry.id = get_string(node, "id"); - entry.manufacturer = get_string(node, "manufacturer"); - entry.model = get_string(node, "model"); - entry.serial = get_string(node, "serial"); - entry.hardware_rev = get_string(node, "hardware_rev"); - entry.firmware = get_string(node, "firmware"); - entry.endpoint = get_string(node, "endpoint"); - entry.role = get_string(node, "role"); - if (node.IsMap()) { for (const auto & it : node) { if (!it.first.IsScalar() || !it.second.IsScalar()) { continue; } const std::string key = it.first.as(); - if (reserved.count(key) != 0) { + if (structural.count(key) != 0) { continue; } - const std::string value = it.second.as(); - if (!value.empty()) { - entry.extras.emplace_back(key, value); - } + assign_asset_field(entry, asset_column(key), key, it.second.as()); } } Component comp = asset_entry_to_component(entry); // Optional tree-placement and presentation overrides. Every key listed in - // `reserved` above must be consumed here; otherwise it is silently dropped. + // `structural` above must be consumed here; otherwise it is silently dropped. const std::string ns = get_string(node, "namespace"); if (!ns.empty()) { // Operator-declared placement: compute an authoritative fqn like a real @@ -314,10 +304,6 @@ Component ManifestParser::parse_asset(const YAML::Node & node) const { comp.namespace_path = ns; comp.fqn = ns + "/" + comp.id; } - const std::string area = get_string(node, "area"); - if (!area.empty()) { - comp.area = area; - } const std::string parent = get_string(node, "parent_component_id"); if (!parent.empty()) { comp.parent_component_id = parent; diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 6c323d4ea..a1dba88e2 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -280,10 +280,10 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki // merge semantics. declare_parameter("discovery.manifest.fragments_dir", ""); // CSV file listing manually inventoried assets (columns: id, manufacturer, - // model, serial, hardware_rev, firmware, endpoint, role, plus extras). Each - // row is appended to the manifest as a Component and merged into the tree by - // id. Requires a manifest-backed discovery mode (manifest_only / hybrid with - // discovery.manifest_path set). Empty = disabled (default). + // model, serial, hardware_rev, firmware, endpoint, role, area, plus extras). + // Each row is appended to the manifest as a Component and merged into the + // tree by id. Requires a manifest-backed discovery mode (manifest_only / + // hybrid with discovery.manifest_path set). Empty = disabled (default). declare_parameter("discovery.inventory.csv_path", ""); declare_parameter("discovery.runtime.enabled", true); diff --git a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp index b8ea5ed01..97b679242 100644 --- a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp +++ b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp @@ -220,6 +220,34 @@ TEST(AssetCsvParseTest, LeadingSpaceBeforeQuotedFieldOpensQuote) { EXPECT_EQ(result.entries[0].model, "Quoted, X"); } +TEST(AssetCsvParseTest, DuplicateIdRowsFirstWinsWithWarning) { + // Duplicates must be reduced here: appended as-is they would trip the + // validator's hard duplicate-id error and abort the whole manifest load. + const std::string csv = + "id,model\n" + "plc_1,First\n" + "plc_1,Second\n" + "other,Kept\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 2u); + EXPECT_EQ(result.entries[0].id, "plc_1"); + EXPECT_EQ(result.entries[0].model, "First"); + EXPECT_EQ(result.entries[1].id, "other"); + ASSERT_EQ(result.warnings.size(), 1u); + EXPECT_NE(result.warnings[0].find("duplicate id 'plc_1'"), std::string::npos); +} + +TEST(AssetCsvParseTest, AreaColumnIsTypedNotExtra) { + const std::string csv = + "id,Area,rack\n" + "pump_1,cell-3,R2\n"; + auto result = parse_asset_csv(csv); + ASSERT_EQ(result.entries.size(), 1u); + EXPECT_EQ(result.entries[0].area, "cell-3"); + ASSERT_EQ(result.entries[0].extras.size(), 1u); // only 'rack' stays an extra + EXPECT_EQ(result.entries[0].extras[0].first, "rack"); +} + // ── AssetEntry -> Component mapping ────────────────────────────────────────── TEST(AssetEntryToComponentTest, FullMapping) { @@ -281,6 +309,15 @@ TEST(AssetEntryToComponentTest, NameFallsBackToManufacturerWhenNoModel) { EXPECT_EQ(comp.name, "OnlyVendor"); } +TEST(AssetEntryToComponentTest, AreaIsStructuralPlacementWithoutProvenance) { + AssetEntry e; + e.id = "pump_1"; + e.area = "cell-3"; + Component comp = asset_entry_to_component(e); + EXPECT_EQ(comp.area, "cell-3"); + EXPECT_EQ(comp.identity.provenance.count("area"), 0u); // not an identity field +} + // ── Manifest `assets:` list ────────────────────────────────────────────────── TEST(ManifestAssetsTest, ParsesAssetsIntoComponents) { @@ -366,6 +403,27 @@ manifest_version: "1.0" EXPECT_EQ(comp->depends_on[1], "bus_a"); } +TEST(ManifestAssetsTest, IdentityAliasesLandOnTypedFields) { + // The `assets:` list must accept the same aliases the CSV import does; + // aliased keys land on the typed identity fields, not in the extras. + const std::string yaml = R"( +manifest_version: "1.0" +assets: + - id: sensor_7 + serial_number: SER-9 + hardware_revision: C1 + firmware_version: 1.4.0 +)"; + ManifestParser parser; + Manifest manifest = parser.parse_string(yaml); + const Component * comp = find_component(manifest.components, "sensor_7"); + ASSERT_NE(comp, nullptr); + EXPECT_EQ(comp->identity.serial_number, "SER-9"); + EXPECT_EQ(comp->identity.hardware_revision, "C1"); + EXPECT_EQ(comp->identity.firmware_version, "1.4.0"); + EXPECT_TRUE(comp->identity.extra.empty()); +} + // ── ManifestManager CSV import ─────────────────────────────────────────────── class InventoryCsvManagerTest : public ::testing::Test { @@ -423,6 +481,145 @@ TEST_F(InventoryCsvManagerTest, MissingCsvIsNotAnError) { EXPECT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); } +TEST_F(InventoryCsvManagerTest, DuplicateCsvIdsDoNotFailLoad) { + // One duplicated export line must not abort the whole manifest load with a + // hard R003 error: first row wins, the load stays green. + const std::string manifest_path = write_file("base.yaml", "manifest_version: \"1.0\"\n"); + const std::string csv_path = write_file("inventory.csv", + "id,model\n" + "plc_1,First\n" + "plc_1,Second\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + const Component * comp = find_component(comps, "plc_1"); + ASSERT_NE(comp, nullptr); + EXPECT_EQ(comp->identity.model, "First"); +} + +TEST_F(InventoryCsvManagerTest, CsvIdCollidingWithManifestComponentGapFillsIdentity) { + // Manifest component wins the collision (operator-authored full definition + // beats a CSV inventory row); the row's identity is folded in as gap-fill + // and the load must succeed. + const std::string manifest_path = write_file("base.yaml", + R"(manifest_version: "1.0" +components: + - id: plc_1 + name: "Main PLC" + type: controller +)"); + const std::string csv_path = write_file("inventory.csv", + "id,manufacturer,serial\n" + "plc_1,Siemens,SN-77\n" + "io_1,Beckhoff,\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + ASSERT_EQ(comps.size(), 2u); // no duplicate plc_1 appended + const Component * plc = find_component(comps, "plc_1"); + ASSERT_NE(plc, nullptr); + EXPECT_EQ(plc->source, "manifest"); // manifest definition kept + EXPECT_EQ(plc->name, "Main PLC"); + EXPECT_EQ(plc->type, "controller"); + // CSV identity folded in as gap-fill with provenance "inventory". + EXPECT_EQ(plc->identity.manufacturer, "Siemens"); + EXPECT_EQ(plc->identity.serial_number, "SN-77"); + EXPECT_EQ(plc->identity.provenance.at("serial_number"), "inventory"); + // Non-colliding rows still land as inventory components. + const Component * io = find_component(comps, "io_1"); + ASSERT_NE(io, nullptr); + EXPECT_EQ(io->source, "inventory"); +} + +TEST_F(InventoryCsvManagerTest, CsvNeverOverridesManifestAssetIdentity) { + // A manifest `assets:` entry is also operator-authored: on a conflict the + // manifest value stays, the CSV only fills fields the manifest left empty. + const std::string manifest_path = write_file("base.yaml", + R"(manifest_version: "1.0" +assets: + - id: robot_1 + manufacturer: KUKA +)"); + const std::string csv_path = write_file("inventory.csv", + "id,manufacturer,serial\n" + "robot_1,Fanuc,SN-9\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + ASSERT_EQ(comps.size(), 1u); + EXPECT_EQ(comps[0].identity.manufacturer, "KUKA"); // conflict: manifest wins + EXPECT_EQ(comps[0].identity.serial_number, "SN-9"); // gap: CSV fills +} + +TEST_F(InventoryCsvManagerTest, CsvIdCollidingWithNonComponentEntitySkipped) { + const std::string manifest_path = write_file("base.yaml", + R"(manifest_version: "1.0" +areas: + - id: cell_3 + name: "Cell 3" +)"); + const std::string csv_path = write_file("inventory.csv", + "id,model\n" + "cell_3,NotAnAsset\n" + "pump_1,CR-5\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + EXPECT_EQ(find_component(comps, "cell_3"), nullptr); // skipped, not a duplicate id + EXPECT_NE(find_component(comps, "pump_1"), nullptr); +} + +TEST_F(InventoryCsvManagerTest, CsvAreaColumnPlacesAssetUnderArea) { + const std::string manifest_path = write_file("base.yaml", + R"(manifest_version: "1.0" +areas: + - id: cell_3 + name: "Cell 3" +)"); + const std::string csv_path = write_file("inventory.csv", + "id,model,area\n" + "pump_1,CR-5,cell_3\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + const Component * pump = find_component(comps, "pump_1"); + ASSERT_NE(pump, nullptr); + EXPECT_EQ(pump->area, "cell_3"); +} + +TEST_F(InventoryCsvManagerTest, CsvUnknownAreaDroppedAssetKept) { + // An unknown area would hard-fail validation (R006); the CSV path drops the + // placement with a warning and keeps the asset, so the load stays green. + const std::string manifest_path = write_file("base.yaml", "manifest_version: \"1.0\"\n"); + const std::string csv_path = write_file("inventory.csv", + "id,model,area\n" + "pump_1,CR-5,nowhere\n"); + + ManifestManager mgr(nullptr); + mgr.set_inventory_csv_path(csv_path); + ASSERT_TRUE(mgr.load_manifest(manifest_path, /*strict=*/false)); + + auto comps = mgr.get_components(); + const Component * pump = find_component(comps, "pump_1"); + ASSERT_NE(pump, nullptr); + EXPECT_TRUE(pump->area.empty()); +} + // ── Acceptance: CSV asset merges with protocol-discovered structure ────────── TEST(InventoryMergeTest, AssetMergesWithRuntimeComponentById) {