From 321cf91f178e6a1ad9116203206ec6f8ea6a2e57 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 12:36:08 +0200 Subject: [PATCH 01/11] feat(gateway): add external classification to Component model (#516) --- .../core/discovery/models/component.hpp | 14 ++++++++++- .../test/test_discovery_models.cpp | 23 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp index f1b34499e..276e2a80e 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/discovery/models/component.hpp @@ -53,6 +53,12 @@ struct Component { std::optional host_metadata; ///< Host system metadata (for runtime default component) AssetIdentity identity; ///< Asset-identity nameplate (merged across sources, per-field provenance) + /// Tri-state: nullopt = no layer classified this component, true = non-ROS + /// external asset (PLC/fieldbus/device), false = explicitly a ROS component. + /// Mirrors App::external so a device Component owns its fault_manager faults + /// without a synthetic child App (#516). + std::optional external; + /** * @brief Convert to JSON representation * @return JSON object with component data @@ -99,6 +105,11 @@ struct Component { if (!identity.empty()) { x_medkit["identity"] = identity.to_json(); } + // Emit only when effectively external; omitted and explicit-false both leave + // the field absent, keeping the wire shape stable (absence == not external). + if (external.value_or(false)) { + x_medkit["external"] = true; + } j["x-medkit"] = x_medkit; // Add operations array combining services and actions @@ -183,7 +194,8 @@ inline bool operator==(const Component & a, const Component & b) { a.description == b.description && a.variant == b.variant && a.tags == b.tags && a.parent_component_id == b.parent_component_id && a.depends_on == b.depends_on && a.contributors == b.contributors && a.services == b.services && a.actions == b.actions && - a.topics == b.topics && a.host_metadata == b.host_metadata && a.identity == b.identity; + a.topics == b.topics && a.host_metadata == b.host_metadata && a.identity == b.identity && + a.external == b.external; } inline bool operator!=(const Component & a, const Component & b) { return !(a == b); diff --git a/src/ros2_medkit_gateway/test/test_discovery_models.cpp b/src/ros2_medkit_gateway/test/test_discovery_models.cpp index 896feda35..23d0b574f 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_models.cpp +++ b/src/ros2_medkit_gateway/test/test_discovery_models.cpp @@ -174,6 +174,29 @@ TEST_F(ComponentModelTest, ToCapabilities_ContainsConfigurationsForNodes) { EXPECT_EQ(j["configurations"], "http://localhost:8080/api/v1/components/motor_controller/configurations"); } +TEST_F(ComponentModelTest, ToJson_OmitsExternalWhenUnsetOrFalse) { + // Default: external unset -> field absent (stable wire shape). + json j = comp_.to_json(); + EXPECT_FALSE(j["x-medkit"].contains("external")); + + comp_.external = false; // explicit ROS component -> still absent + json j2 = comp_.to_json(); + EXPECT_FALSE(j2["x-medkit"].contains("external")); +} + +TEST_F(ComponentModelTest, ToJson_ExternalWhenTrue) { + comp_.external = true; + json j = comp_.to_json(); + EXPECT_EQ(j["x-medkit"]["external"], true); +} + +TEST_F(ComponentModelTest, Equality_DistinguishesExternal) { + Component other = comp_; + ASSERT_EQ(comp_, other); + other.external = true; + EXPECT_NE(comp_, other); +} + // ============================================================================= // App Model Tests // ============================================================================= From fc222fbac6b3ddcbae5c0796edc092086f7415fa Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 12:47:36 +0200 Subject: [PATCH 02/11] feat(gateway): parse component external: key (#516) --- .../discovery/manifest/manifest_parser.cpp | 7 +++++ .../test/test_manifest_parser.cpp | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+) 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 2a226c9df..aa5c4774f 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -255,6 +255,13 @@ Component ManifestParser::parse_component(const YAML::Node & node) const { comp.source = "manifest"; comp.identity = parse_identity(node); + // Preserve the omitted-vs-explicit distinction: an absent `external:` key + // leaves nullopt so the hybrid merge cannot let a stub's default erase a + // plugin's introspected classification (#516, mirrors the App rule #517). + if (node["external"]) { + comp.external = node["external"].as(); + } + // Parse type if provided (e.g., "controller", "sensor", "actuator") std::string type_val = get_string(node, "type"); if (!type_val.empty()) { diff --git a/src/ros2_medkit_gateway/test/test_manifest_parser.cpp b/src/ros2_medkit_gateway/test/test_manifest_parser.cpp index 18466196c..abe34ca47 100644 --- a/src/ros2_medkit_gateway/test/test_manifest_parser.cpp +++ b/src/ros2_medkit_gateway/test/test_manifest_parser.cpp @@ -26,6 +26,7 @@ #include "ros2_medkit_gateway/core/discovery/manifest/manifest_parser.hpp" +using ros2_medkit_gateway::Component; using ros2_medkit_gateway::discovery::ManifestConfig; using ros2_medkit_gateway::discovery::ManifestParser; @@ -295,6 +296,35 @@ manifest_version: "1.0" EXPECT_FALSE(manifest.apps[0].external.value()); } +TEST_F(ManifestParserTest, ParseComponent_ExternalKeyTriState) { + const std::string yaml = R"( +manifest_version: "1.0" +metadata: { name: t, version: "1.0.0" } +components: + - id: plc + external: true + - id: ros_node + external: false + - id: unclassified +)"; + auto manifest = parser_.parse_string(yaml); + ASSERT_EQ(manifest.components.size(), 3u); + auto by_id = [&](const std::string & id) -> const Component & { + for (const auto & c : manifest.components) { + if (c.id == id) { + return c; + } + } + ADD_FAILURE() << "component not found: " << id; + static Component none; + return none; + }; + EXPECT_TRUE(by_id("plc").external.value_or(false)); + ASSERT_TRUE(by_id("ros_node").external.has_value()); + EXPECT_FALSE(by_id("ros_node").external.value()); + EXPECT_FALSE(by_id("unclassified").external.has_value()); +} + TEST_F(ManifestParserTest, ParseFunctions) { const std::string yaml = R"( manifest_version: "1.0" From 9f19b4c4aa06b8016bd25f8e9f2657fa6dacf06d Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 12:59:43 +0200 Subject: [PATCH 03/11] feat(gateway): merge component external classification across layers (#516) --- .../src/discovery/merge_pipeline.cpp | 1 + .../test/test_merge_pipeline.cpp | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index d10bdb657..c6b394922 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -263,6 +263,7 @@ void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup case FieldGroup::METADATA: merge_scalar(target.source, source.source, res.scalar); merge_scalar(target.variant, source.variant, res.scalar); + merge_external(target.external, source.external, res.scalar); break; case FieldGroup::STATUS: default: diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index b4dafe9f2..8c4cb84a0 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -1513,6 +1513,60 @@ TEST_F(MergePipelineTest, AppExternalField_AuthoritativeExplicitFalseOverridesPl EXPECT_FALSE(result.apps[0].external.value()) << "authoritative external: false must override plugin true"; } +TEST_F(MergePipelineTest, PluginExternalClassificationSurvivesManifestComponentMetadataMerge) { + // Manifest stub: declares the component id, no external key -> nullopt. + Component manifest_comp = make_component("s7_1500"); + manifest_comp.source = "manifest"; + + // Plugin introspection: same id, classified external. + Component plugin_comp = make_component("s7_1500"); + plugin_comp.external = true; + plugin_comp.source = "opcua"; + + LayerOutput manifest_out, plugin_out; + manifest_out.components.push_back(manifest_comp); + plugin_out.components.push_back(plugin_comp); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}})); + pipeline_.add_layer(std::make_unique( + "plugin", plugin_out, + std::unordered_map{{FieldGroup::METADATA, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_TRUE(result.components[0].external.value_or(false)) + << "hybrid merge dropped the plugin's external classification"; + EXPECT_EQ(result.components[0].source, "manifest"); +} + +TEST_F(MergePipelineTest, AuthoritativeManifestExternalFalseCorrectsPluginComponent) { + Component manifest_comp = make_component("s7_1500"); + manifest_comp.external = false; // explicit, authoritative: this IS a ROS node + manifest_comp.source = "manifest"; + + Component plugin_comp = make_component("s7_1500"); + plugin_comp.external = true; // plugin misclassified it + plugin_comp.source = "opcua"; + + LayerOutput manifest_out, plugin_out; + manifest_out.components.push_back(manifest_comp); + plugin_out.components.push_back(plugin_comp); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}})); + pipeline_.add_layer(std::make_unique( + "plugin", plugin_out, + std::unordered_map{{FieldGroup::METADATA, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + ASSERT_TRUE(result.components[0].external.has_value()); + EXPECT_FALSE(result.components[0].external.value()) << "authoritative external: false must override plugin true"; +} + // --- Area field group merge tests --- TEST_F(MergePipelineTest, AreaIdentityMerge_ManifestAuthoritativeWinsName) { From 77ba9d873dc91da6f839b1aee0a1e6526810b01b Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 13:13:33 +0200 Subject: [PATCH 04/11] feat(gateway): external Component owns its fault_manager faults (#516) --- .../src/core/faults/fault_scope.cpp | 14 +++- .../test/test_handler_context.cpp | 74 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/ros2_medkit_gateway/src/core/faults/fault_scope.cpp b/src/ros2_medkit_gateway/src/core/faults/fault_scope.cpp index 9b2d20341..92bb43c45 100644 --- a/src/ros2_medkit_gateway/src/core/faults/fault_scope.cpp +++ b/src/ros2_medkit_gateway/src/core/faults/fault_scope.cpp @@ -50,9 +50,21 @@ void collect_app_fqn(const ThreadSafeEntityCache & cache, const std::string & ap void collect_component_app_fqns(const ThreadSafeEntityCache & cache, const std::string & comp_id, std::set & out) { + std::set local; for (const auto & app_id : cache.get_apps_for_component(comp_id)) { - collect_app_fqn(cache, app_id, out); + collect_app_fqn(cache, app_id, local); } + if (local.empty()) { + // No child app contributed a source (no apps, or all unbound/empty-FQN). + // An external component owns its fault_manager faults under its own id - + // mirrors the external-App rule (#516). A non-external component stays + // empty: an unbound ROS component must not claim faults it never reported. + auto comp = cache.get_component(comp_id); + if (comp && comp->external.value_or(false)) { + local.insert(comp_id); + } + } + out.insert(local.begin(), local.end()); } void collect_area_app_fqns(const ThreadSafeEntityCache & cache, const std::string & area_id, diff --git a/src/ros2_medkit_gateway/test/test_handler_context.cpp b/src/ros2_medkit_gateway/test/test_handler_context.cpp index dadd49c5a..ea185bbfc 100644 --- a/src/ros2_medkit_gateway/test/test_handler_context.cpp +++ b/src/ros2_medkit_gateway/test/test_handler_context.cpp @@ -1758,6 +1758,80 @@ TEST(ResolveEntitySourceFqnsTest, ExternalAppWithStrayRosBindingOwnsFaultsByBare EXPECT_EQ(fqns, std::set{"process"}); } +TEST(ResolveEntitySourceFqnsTest, ExternalComponentWithNoAppsOwnsFaultsUnderItsOwnId) { + // The #516 case: a PLC modeled as an external Component with no child App. + // It reports faults under its own component id; the scope must be exactly + // {its id} so GET /components//faults is non-empty. + ThreadSafeEntityCache cache; + Component plc; + plc.id = "s7_1500"; + plc.external = true; + cache.update_components({plc}); + + auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::COMPONENT, "s7_1500")); + EXPECT_EQ(fqns, std::set{"s7_1500"}); +} + +TEST(ResolveEntitySourceFqnsTest, NonExternalComponentWithNoAppsStaysEmpty) { + // Security guardrail: a non-external Component with no apps must NOT claim its + // bare id - an unbound ROS component must not own faults it never reported. + ThreadSafeEntityCache cache; + Component comp; + comp.id = "nav_comp"; + comp.external = false; + cache.update_components({comp}); + + auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::COMPONENT, "nav_comp")); + EXPECT_TRUE(fqns.empty()); +} + +TEST(ResolveEntitySourceFqnsTest, ExternalComponentWithContributingAppKeepsAppScope) { + // An external Component that DOES host a ROS-bound app resolves to the app's + // FQN; the component id is NOT added (fallback dormant when apps contribute). + ThreadSafeEntityCache cache; + Component plc; + plc.id = "s7_1500"; + plc.external = true; + cache.update_components({plc}); + cache.update_apps({make_owned_app("planner", "s7_1500", "planner", "/perception/nav")}); + + auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::COMPONENT, "s7_1500")); + EXPECT_EQ(fqns, std::set{"/perception/nav/planner"}); +} + +TEST(ResolveEntitySourceFqnsTest, FunctionHostingExternalComponentOwnsItsFaults) { + // Function.hosted_by lists the device Component directly (Component-host). + // The Function rollup must include the external Component's bare id (#516: + // no synthetic App needed). + ThreadSafeEntityCache cache; + Component plc; + plc.id = "s7_1500"; + plc.external = true; + cache.update_components({plc}); + Function f; + f.id = "material_flow"; + f.hosts = {"s7_1500"}; + cache.update_functions({f}); + + auto fqns = + HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::FUNCTION, "material_flow")); + EXPECT_EQ(fqns, std::set{"s7_1500"}); +} + +TEST(ResolveEntitySourceFqnsTest, AreaHostingExternalComponentOwnsItsFaults) { + // Area rollup walks the same component gate; an external device Component in + // the area owns its faults on the area route. + ThreadSafeEntityCache cache; + Component plc; + plc.id = "s7_1500"; + plc.area = "cell_a"; + plc.external = true; + cache.update_components({plc}); + + auto fqns = HandlerContext::resolve_entity_source_fqns(cache, make_entity_info(EntityType::AREA, "cell_a")); + EXPECT_EQ(fqns, std::set{"s7_1500"}); +} + int main(int argc, char ** argv) { testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); From 933645bb514e531738d2ce5bedff97c621831c5b Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 13:31:01 +0200 Subject: [PATCH 05/11] feat(gateway): add external field to XMedkitComponent/XMedkitApp DTOs (#516) --- .../ros2_medkit_gateway/dto/x_medkit.hpp | 14 +++++--- .../test/test_dto_contract.cpp | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp index 734914d13..8124d3ef4 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp @@ -103,6 +103,7 @@ inline constexpr std::string_view dto_name = "XMedkitArea"; // // Also used in sub-collection responses (depends-on, subcomponents, hosts, contains, etc.): // missing <- true when component reference cannot be resolved +// external <- comp.external, non-ROS external asset classification (#516) // // Note: "parentComponentId" uses camelCase on the wire per discovery_handlers.cpp. // "dependsOn" uses camelCase on the wire per discovery_handlers.cpp. @@ -122,7 +123,8 @@ struct XMedkitComponent { // "_provenance"). Free-form JSON so the DTO layer reuses the exact // serialization emitted by Component::to_json and consumed by peer parsing. std::optional identity; - std::optional missing; // broken reference sentinel + std::optional missing; // broken reference sentinel + std::optional external; // non-ROS external asset classification (#516) }; template <> @@ -132,7 +134,8 @@ inline constexpr auto dto_fields = std::make_tuple( field("dependsOn", &XMedkitComponent::depends_on), field("area", &XMedkitComponent::area), field("variant", &XMedkitComponent::variant), field("description", &XMedkitComponent::description), field("contributors", &XMedkitComponent::contributors), field("capabilities", &XMedkitComponent::capabilities), - field("identity", &XMedkitComponent::identity), field("missing", &XMedkitComponent::missing)); + field("identity", &XMedkitComponent::identity), field("missing", &XMedkitComponent::missing), + field("external", &XMedkitComponent::external)); template <> inline constexpr std::string_view dto_name = "XMedkitComponent"; @@ -149,6 +152,7 @@ inline constexpr std::string_view dto_name = "XMedkitComponent // // Also used in sub-collection responses (depends-on, hosts, function-hosts): // missing <- true when app reference cannot be resolved +// external <- app.external, non-ROS external asset classification (#516/#517) // --------------------------------------------------------------------------- struct XMedkitApp { std::optional ros2; @@ -156,14 +160,16 @@ struct XMedkitApp { std::optional is_online; std::optional component_id; std::optional> contributors; - std::optional missing; // broken reference sentinel + std::optional missing; // broken reference sentinel + std::optional external; // non-ROS external asset classification (#516/#517) }; template <> inline constexpr auto dto_fields = std::make_tuple(field("ros2", &XMedkitApp::ros2), field("source", &XMedkitApp::source), field("is_online", &XMedkitApp::is_online), field("component_id", &XMedkitApp::component_id), - field("contributors", &XMedkitApp::contributors), field("missing", &XMedkitApp::missing)); + field("contributors", &XMedkitApp::contributors), field("missing", &XMedkitApp::missing), + field("external", &XMedkitApp::external)); template <> inline constexpr std::string_view dto_name = "XMedkitApp"; diff --git a/src/ros2_medkit_gateway/test/test_dto_contract.cpp b/src/ros2_medkit_gateway/test/test_dto_contract.cpp index 9bd500550..e3a1fe964 100644 --- a/src/ros2_medkit_gateway/test/test_dto_contract.cpp +++ b/src/ros2_medkit_gateway/test/test_dto_contract.cpp @@ -500,3 +500,35 @@ void check_all(std::index_sequence /*seq*/) { TEST(DtoRegistry, EveryRegisteredDtoRoundTrips) { check_all(std::make_index_sequence>{}); } + +// ============================================================================= +// external field contract (#516) +// ============================================================================= + +TEST(XMedkitExternalContract, ComponentEmitsExternalWhenSet) { + dto::XMedkitComponent c; + c.external = true; + auto j = dto::JsonWriter::write(c); + ASSERT_TRUE(j.contains("external")); + EXPECT_EQ(j["external"], true); +} + +TEST(XMedkitExternalContract, ComponentOmitsExternalWhenUnset) { + dto::XMedkitComponent c; // external nullopt + auto j = dto::JsonWriter::write(c); + EXPECT_FALSE(j.contains("external")); +} + +TEST(XMedkitExternalContract, AppEmitsExternalWhenSet) { + dto::XMedkitApp a; + a.external = true; + auto j = dto::JsonWriter::write(a); + ASSERT_TRUE(j.contains("external")); + EXPECT_EQ(j["external"], true); +} + +TEST(XMedkitExternalContract, AppOmitsExternalWhenUnset) { + dto::XMedkitApp a; // external nullopt + auto j = dto::JsonWriter::write(a); + EXPECT_FALSE(j.contains("external")); +} From 84a58bdb17d45365e3e29116083000fa6158e5a6 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 13:44:19 +0200 Subject: [PATCH 06/11] feat(gateway): surface external classification on component/app REST routes (#516) --- .../src/http/handlers/discovery_handlers.cpp | 12 +++++ .../test/test_discovery_handlers.cpp | 54 +++++++++++++++++-- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp index d8a177ba1..f9bc6b85f 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp @@ -561,6 +561,9 @@ DiscoveryHandlers::get_components(const http::TypedRequest & req) { if (!component.identity.empty()) { x_medkit_comp.identity = component.identity.to_json(); } + if (component.external.value_or(false)) { + x_medkit_comp.external = true; + } item.x_medkit = x_medkit_comp; response.items.push_back(std::move(item)); @@ -691,6 +694,9 @@ http::Result DiscoveryHandlers::get_component(const http:: if (!comp.identity.empty()) { x_medkit_comp.identity = comp.identity.to_json(); } + if (comp.external.value_or(false)) { + x_medkit_comp.external = true; + } using Cap = CapabilityBuilder::Capability; std::vector caps = { @@ -975,6 +981,9 @@ http::Result> DiscoveryHandlers::get_apps(cons ros2.node = *app.bound_fqn; x_medkit_app.ros2 = ros2; } + if (app.external.value_or(false)) { + x_medkit_app.external = true; + } item.x_medkit = x_medkit_app; response.items.push_back(std::move(item)); @@ -1113,6 +1122,9 @@ http::Result DiscoveryHandlers::get_app(const http::TypedRequest if (!app.contributors.empty()) { x_medkit_app.contributors = sorted_contributors(app.contributors); } + if (app.external.value_or(false)) { + x_medkit_app.external = true; + } detail.x_medkit = x_medkit_app; return detail; diff --git a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp index 7377b07ca..a3c2ec3d2 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp @@ -92,6 +92,12 @@ manifest_version: "1.0" parent_component_id: "main_ecu" description: "Lidar aggregation" tags: ["sensor"] + - id: ext_plc + name: "External PLC" + external: true + identity: + manufacturer: "Siemens" + serial_number: "SN-1" apps: - id: "planner" name: "Planner" @@ -104,6 +110,10 @@ manifest_version: "1.0" - id: "standalone" name: "Standalone" description: "Standalone app without a hosting component" + - id: ext_app + name: "External App" + external: true + is_located_on: ext_plc functions: - id: "navigation" name: "Navigation" @@ -258,7 +268,7 @@ class DiscoveryHandlersFixtureTest : public ::testing::Test { auto apps = suite_node_->get_discovery_manager()->discover_apps(); auto functions = suite_node_->get_discovery_manager()->discover_functions(); - ASSERT_EQ(apps.size(), 3u); + ASSERT_EQ(apps.size(), 4u); apps[0].is_online = true; apps[0].bound_fqn = "/vehicle/main_ecu/planner"; apps[1].bound_fqn = "/sensors/lidar_unit/mapper"; @@ -436,7 +446,7 @@ TEST_F(DiscoveryHandlersFixtureTest, ListComponentsReturnsMetadata) { auto result = handlers_->get_components(typed_req); auto body = body_json(result); // lidar_unit has parent_component_id, so it's filtered from top-level list - ASSERT_EQ(body["items"].size(), 1u); + ASSERT_EQ(body["items"].size(), 2u); EXPECT_EQ(body["items"][0]["id"], "main_ecu"); EXPECT_EQ(body["items"][0]["description"], "Vehicle control unit"); EXPECT_EQ(body["items"][0]["x-medkit"]["source"], "manifest"); @@ -452,7 +462,7 @@ TEST_F(DiscoveryHandlersFixtureTest, ListComponentsIncludesManifestIdentity) { auto result = handlers_->get_components(typed_req); auto body = body_json(result); - ASSERT_EQ(body["items"].size(), 1u); + ASSERT_EQ(body["items"].size(), 2u); const auto & identity = body["items"][0]["x-medkit"]["identity"]; // Typed fields serialize camelCase (AssetIdentity::to_json). EXPECT_EQ(identity["manufacturer"], "Acme Robotics"); @@ -491,6 +501,30 @@ TEST_F(DiscoveryHandlersFixtureTest, GetComponentWithoutIdentityOmitsField) { EXPECT_FALSE(body["x-medkit"].contains("identity")); } +// @verifies REQ_INTEROP_003 +// A manifest component explicitly marked `external: true` surfaces +// x-medkit.external on the wire (#516). +TEST_F(DiscoveryHandlersFixtureTest, GetComponentEmitsExternalOnWire) { + httplib::Request req; + auto typed_req = make_typed_request(req, "/api/v1/components/ext_plc", R"(/api/v1/components/([^/]+))"); + + auto result = handlers_->get_component(typed_req); + auto body = body_json(result); + EXPECT_EQ(body["x-medkit"]["external"], true); +} + +// @verifies REQ_INTEROP_003 +// A ROS-native manifest component omits x-medkit.external entirely (absence +// means "not external", never emitted as an explicit `false`) (#516). +TEST_F(DiscoveryHandlersFixtureTest, GetComponentOmitsExternalForRosComponent) { + httplib::Request req; + auto typed_req = make_typed_request(req, "/api/v1/components/main_ecu", R"(/api/v1/components/([^/]+))"); + + auto result = handlers_->get_component(typed_req); + auto body = body_json(result); + EXPECT_FALSE(body["x-medkit"].contains("external")); +} + // @verifies REQ_INTEROP_003 TEST_F(DiscoveryHandlersValidationTest, GetComponentInvalidIdReturns400) { httplib::Request req; @@ -636,7 +670,7 @@ TEST_F(DiscoveryHandlersFixtureTest, ListAppsReturnsSeededMetadata) { auto result = handlers_->get_apps(typed_req); auto body = body_json(result); - ASSERT_EQ(body["items"].size(), 3u); + ASSERT_EQ(body["items"].size(), 4u); EXPECT_EQ(body["items"][0]["id"], "planner"); EXPECT_EQ(body["items"][0]["x-medkit"]["component_id"], "main_ecu"); EXPECT_EQ(body["items"][0]["x-medkit"]["is_online"], true); @@ -653,6 +687,18 @@ TEST_F(DiscoveryHandlersFixtureTest, GetAppUnknownIdReturns404) { EXPECT_EQ(result.error().http_status, 404); } +// @verifies REQ_INTEROP_003 +// A manifest app explicitly marked `external: true` surfaces +// x-medkit.external on the wire (#516). +TEST_F(DiscoveryHandlersFixtureTest, GetAppEmitsExternalOnWire) { + httplib::Request req; + auto typed_req = make_typed_request(req, "/api/v1/apps/ext_app", R"(/api/v1/apps/([^/]+))"); + + auto result = handlers_->get_app(typed_req); + auto body = body_json(result); + EXPECT_EQ(body["x-medkit"]["external"], true); +} + // @verifies REQ_INTEROP_003 TEST_F(DiscoveryHandlersFixtureTest, GetAppReturnsLinksAndCapabilities) { httplib::Request req; From 1416b55e393f4ab6213f8d17f8eaf3d16271e987 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 14:02:21 +0200 Subject: [PATCH 07/11] test(gateway): end-to-end external Component fault rollup + wire parity (#516) --- .../external_component_fault_manifest.yaml | 71 +++++++ ...st_external_component_fault_rollup.test.py | 198 ++++++++++++++++++ 2 files changed, 269 insertions(+) create mode 100644 src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml create mode 100644 src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py diff --git a/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml b/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml new file mode 100644 index 000000000..106b543f6 --- /dev/null +++ b/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml @@ -0,0 +1,71 @@ +# Copyright 2026 bburda +# +# SOVD manifest fixture for #516: an external Component (a PLC bridged into SOVD +# by a protocol plugin) owns its fault_manager faults with NO child App located +# on it. Mirrors the #517 external-App fixture but at the Component level. Also +# carries AAS-Nameplate identity to confirm identity is orthogonal to the +# external classification, and an internal (ROS) Component as the guardrail. +# +# `plc-diag` is a separate external App used only to pin App external wire +# parity (test_external_flag_and_identity_on_the_wire) - it is deliberately NOT +# `is_located_on: s7-plc`. Locating it on the Component would give +# `s7-plc` a contributing child app, and per the fault-scope rule +# (ResolveEntitySourceFqnsTest.ExternalComponentWithContributingAppKeepsAppScope) +# the component's fault scope would then resolve to the app's id/FQN instead of +# falling back to the component's own bare id +# (ResolveEntitySourceFqnsTest.ExternalComponentWithNoAppsOwnsFaultsUnderItsOwnId), +# which would silently break the component/function/area rollup this fixture +# exists to pin. +# +# Used by: test/features/test_external_component_fault_rollup.test.py +manifest_version: "1.0" + +metadata: + name: "external-component-fault-rollup" + version: "1.0.0" + description: "External (non-ROS) Component fault-rollup fixture (#516)" + +config: + unmanifested_nodes: warn + inherit_runtime_resources: true + +areas: + - id: plc-cell + name: "PLC Cell" + namespace: /plc_cell + description: "Industrial cell hosting a PLC bridged into SOVD" + +components: + - id: s7-plc + name: "Siemens S7 PLC" + type: "controller" + area: plc-cell + external: true + description: "Non-ROS PLC introspected by a protocol plugin; owns its faults" + identity: + manufacturer: "Siemens" + model: "S7-1500" + order_code: "6ES7515-2AM02-0AB0" + serial_number: "SN-PLC-0001" + role: "plc" + - id: nav-controller + name: "Nav Controller" + type: "controller" + area: plc-cell + description: "Internal ROS component (guardrail: must NOT own bare-id faults)" + identity: + manufacturer: "Acme" + model: "NavCtl-1" + +apps: + - id: plc-diag + name: "PLC Diagnostics" + external: true + description: "External diagnostic app (guards App external wire parity); not located on s7-plc on purpose (see header)" + +functions: + - id: material_flow + name: "Material Flow" + category: "process" + hosted_by: + - s7-plc diff --git a/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py b/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py new file mode 100644 index 000000000..a80db75d0 --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py @@ -0,0 +1,198 @@ +#!/usr/bin/env python3 +# 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. + +"""End-to-end regression for #516: external-Component faults across every rollup. + +An external Component (``external: true`` - e.g. a PLC bridged into SOVD by a +protocol plugin) can report faults to the fault_manager under its own entity id +with NO child App located on it. Fault-scope resolution must fall back to that +bare id when no app contributes a scope, so the component's faults surface on +the component route AND on every aggregate route that rolls it up (function, +area). This mirrors the #517 external-App rule +(``ResolveEntitySourceFqnsTest.ExternalAppWithStrayRosBindingOwnsFaultsByBareId``) +one level up the entity hierarchy +(``ResolveEntitySourceFqnsTest.ExternalComponentWithNoAppsOwnsFaultsUnderItsOwnId``). + +The fixture also carries a separate external App (``plc-diag``) that is +deliberately NOT located on the PLC Component: locating it there would give the +Component a contributing child app, and the fault scope would resolve to the +app's id instead of falling back to the component's own bare id +(``ResolveEntitySourceFqnsTest.ExternalComponentWithContributingAppKeepsAppScope``), +silently breaking the rollup this test exists to pin. The App instead pins +``external`` wire parity on the App route. + +The fixture's second Component (``nav-controller``) is internal (no +``external`` key) and has no apps located on it either - the guardrail case +(``ResolveEntitySourceFqnsTest.NonExternalComponentWithNoAppsStaysEmpty``): an +unbound ROS component must not claim faults it never reported. + +The merge-level preservation is unit-covered +(``MergePipelineTest.PluginExternalClassificationSurvivesManifestMetadataMerge``) +and the per-route scope resolution is unit-covered +(``ResolveEntitySourceFqnsTest.*``). This test pins the full HTTP-stack +behaviour those two disjoint unit layers never exercise together: report a +fault under the Component's bare id, then observe it on every rollup route. +""" + +import os +import unittest + +from ament_index_python.packages import get_package_share_directory +import launch_testing +import rclpy +from rclpy.node import Node +from ros2_medkit_msgs.msg import Fault +from ros2_medkit_msgs.srv import ReportFault + +from ros2_medkit_test_utils.constants import ALLOWED_EXIT_CODES +from ros2_medkit_test_utils.gateway_test_case import GatewayTestCase +from ros2_medkit_test_utils.launch_helpers import create_test_launch + + +EXTERNAL_COMPONENT = 's7-plc' +EXTERNAL_APP = 'plc-diag' +INTERNAL_COMPONENT = 'nav-controller' +HOST_FUNCTION = 'material_flow' +HOST_AREA = 'plc-cell' +PLC_FAULT = 'PLC_JAM_INFEED' +NAV_FAULT = 'NAV_STALL' + + +def generate_test_description(): + manifest_path = os.path.join( + get_package_share_directory('ros2_medkit_gateway'), + 'config', 'examples', 'external_component_fault_manifest.yaml', + ) + return create_test_launch( + demo_nodes=[], + fault_manager=True, + # Negative threshold: the fault confirms after a couple of FAILED + # events (see _report_fault, which reports several times). + fault_manager_params={'confirmation_threshold': -2}, + gateway_params={ + 'discovery.mode': 'hybrid', + 'discovery.manifest_path': manifest_path, + 'discovery.manifest_strict_validation': False, + }, + ) + + +class TestExternalComponentFaultRollup(GatewayTestCase): + """An external Component owns its faults with no child app (#516).""" + + MIN_EXPECTED_APPS = 1 + REQUIRED_APPS = {EXTERNAL_APP} + REQUIRED_AREAS = {HOST_AREA} + REQUIRED_FUNCTIONS = {HOST_FUNCTION} + + @classmethod + def setUpClass(cls): + # Wait for the gateway + discovery (the external component, app, area + # and function must exist before we exercise the rollup routes). + super().setUpClass() + rclpy.init() + cls._reporter = Node('external_component_fault_reporter') + cls._report_client = cls._reporter.create_client( + ReportFault, '/fault_manager/report_fault' + ) + assert cls._report_client.wait_for_service(timeout_sec=15.0), \ + 'report_fault service not available' + + @classmethod + def tearDownClass(cls): + cls._reporter.destroy_node() + rclpy.shutdown() + + def _report_fault(self, source_id, fault_code, times=4): + """Fire-and-forget fault reports. + + Mirror the #517 helper: drop the reply future to avoid + sanitizer-timing flakes; report several times so the negative + confirmation_threshold latches CONFIRMED. + """ + for _ in range(times): + req = ReportFault.Request() + req.fault_code = fault_code + req.event_type = ReportFault.Request.EVENT_FAILED + req.severity = Fault.SEVERITY_ERROR + req.source_id = source_id + # Fire-and-forget: the request is sent synchronously by + # call_async; spin briefly to flush and pace the reports for the + # debounce, and intentionally drop the reply future. + self._report_client.call_async(req) + rclpy.spin_once(self._reporter, timeout_sec=0.1) + + def test_external_component_owns_faults_on_every_rollup(self): + """External Component fault surfaces on component, function, area. + + @verifies REQ_INTEROP_012 + """ + self._report_fault(EXTERNAL_COMPONENT, PLC_FAULT) + + # #516: the component's bare-id fault must roll up to every aggregate + # route that includes it. Before the fix these rollups were empty + # because a non-external component (or one with no contributing app) + # never resolved to its own bare id. + for endpoint in ( + f'/components/{EXTERNAL_COMPONENT}', + f'/functions/{HOST_FUNCTION}', + f'/areas/{HOST_AREA}', + ): + with self.subTest(rollup=endpoint): + fault = self.wait_for_fault(endpoint, PLC_FAULT) + self.assertEqual(fault.get('fault_code'), PLC_FAULT) + + def test_internal_component_does_not_own_bare_id_faults(self): + """Guardrail: a non-external Component must not claim bare-id faults. + + Race-free: confirm the fault is recorded on the server-wide /faults + list first, then assert it is filtered out of the component route. + """ + self._report_fault(INTERNAL_COMPONENT, NAV_FAULT) + # wait_for_fault('', NAV_FAULT) polls the server-wide '/faults' list + # (it appends '/faults' to the empty entity endpoint), proving the + # fault_manager recorded it regardless of scope resolution. + self.wait_for_fault('', NAV_FAULT) + comp_faults = self.get_json(f'/components/{INTERNAL_COMPONENT}/faults') + codes = [f.get('fault_code') for f in comp_faults.get('items', [])] + self.assertNotIn(NAV_FAULT, codes) + + def test_external_flag_and_identity_on_the_wire(self): + """External flag present on external Component + App, absent internal. + + AAS identity is present on both (orthogonal to external). + """ + comp = self.get_json(f'/components/{EXTERNAL_COMPONENT}') + self.assertTrue(comp['x-medkit'].get('external')) + self.assertIn('identity', comp['x-medkit']) + + app = self.get_json(f'/apps/{EXTERNAL_APP}') + self.assertTrue(app['x-medkit'].get('external')) + + internal = self.get_json(f'/components/{INTERNAL_COMPONENT}') + self.assertNotIn('external', internal['x-medkit']) + self.assertIn('identity', internal['x-medkit']) + + +@launch_testing.post_shutdown_test() +class TestShutdown(unittest.TestCase): + + def test_exit_codes(self, proc_info): + """Check all processes exited cleanly (SIGTERM allowed).""" + for info in proc_info: + self.assertIn( + info.returncode, ALLOWED_EXIT_CODES, + f'{info.process_name} exited with code {info.returncode}' + ) From 51137b47f4f74af871d5afa6b320d06e44516ebe Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 14:21:33 +0200 Subject: [PATCH 08/11] docs(gateway): document component external classification (#516) --- docs/config/discovery-options.rst | 6 ++++++ docs/config/manifest-schema.rst | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/docs/config/discovery-options.rst b/docs/config/discovery-options.rst index a77ed6b7b..6999bb3ae 100644 --- a/docs/config/discovery-options.rst +++ b/docs/config/discovery-options.rst @@ -188,6 +188,12 @@ Each layer declares a policy per field-group: plugin's ``external: true``. Only omission is a no-op; an explicit value is never silently discarded. + The same tri-state field and merge priority apply to Components: an + external Component with no bound child Apps owns its fault-scope by entity + id, so an Area or Function hosting it rolls up its faults without a + synthetic child App. Components have no ``ros_binding``, so there is no + equivalent to the App's R013 contradiction check. + Override per-layer policies in ``gateway_params.yaml``. Empty string means "use layer default". Policy values are **case-sensitive** and must be lowercase (``authoritative``, ``enrichment``, ``fallback``): diff --git a/docs/config/manifest-schema.rst b/docs/config/manifest-schema.rst index fa29d85ad..b1b783630 100644 --- a/docs/config/manifest-schema.rst +++ b/docs/config/manifest-schema.rst @@ -236,6 +236,7 @@ Schema parent_component_id: string # Optional - parent component depends_on: [string] # Optional - component IDs this depends on subcomponents: [] # Optional - nested definitions + external: boolean # Optional - non-ROS external asset (default: false) identity: # Optional - asset-identity nameplate manufacturer: string # Vendor / manufacturer name @@ -321,6 +322,17 @@ Fields - [Component] - No - Nested component definitions + * - ``external`` + - boolean + - No + - True if the component is a non-ROS external asset (PLC, fieldbus device, + any asset a protocol plugin bridges into SOVD). Tri-state: **omitting** + ``external:`` leaves it unset, so in hybrid mode it does not clear an + ``external`` classification contributed by another discovery layer (e.g. a + protocol plugin). An **explicit** value is authoritative and resolves by + normal layer priority. An external component with no bound child apps owns + its fault_manager faults under its own entity id, so a Function or Area + hosting it rolls up its faults without a synthetic child app (#516). * - ``identity`` - object - No @@ -377,6 +389,11 @@ Example type: "sensor" area: perception + # External device component (not a ROS node); owns its faults by entity id + - id: line-plc + name: "Line PLC" + external: true + Assets ------ From acbc3a5356eb0ce237abdfa4cd10e6931ec1186b Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 15:32:06 +0200 Subject: [PATCH 09/11] docs(gateway): scope external DTO comment to primary routes and fix test docstring reference (#516) --- .../include/ros2_medkit_gateway/dto/x_medkit.hpp | 4 ++-- .../features/test_external_component_fault_rollup.test.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp index 8124d3ef4..d6c0080f9 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp @@ -100,10 +100,10 @@ inline constexpr std::string_view dto_name = "XMedkitArea"; // description <- comp.description (via ext.add()) // contributors <- comp.contributors // capabilities <- capabilities JSON array (via ext.add()) +// external <- comp.external, non-ROS external asset classification; detail + list routes only (#516) // // Also used in sub-collection responses (depends-on, subcomponents, hosts, contains, etc.): // missing <- true when component reference cannot be resolved -// external <- comp.external, non-ROS external asset classification (#516) // // Note: "parentComponentId" uses camelCase on the wire per discovery_handlers.cpp. // "dependsOn" uses camelCase on the wire per discovery_handlers.cpp. @@ -149,10 +149,10 @@ inline constexpr std::string_view dto_name = "XMedkitComponent // ros2.node <- app.bound_fqn // component_id <- app.component_id // contributors <- app.contributors (detail only) +// external <- app.external, non-ROS external asset classification; detail + list routes only (#516/#517) // // Also used in sub-collection responses (depends-on, hosts, function-hosts): // missing <- true when app reference cannot be resolved -// external <- app.external, non-ROS external asset classification (#516/#517) // --------------------------------------------------------------------------- struct XMedkitApp { std::optional ros2; diff --git a/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py b/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py index a80db75d0..925d6a954 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_external_component_fault_rollup.test.py @@ -39,7 +39,7 @@ unbound ROS component must not claim faults it never reported. The merge-level preservation is unit-covered -(``MergePipelineTest.PluginExternalClassificationSurvivesManifestMetadataMerge``) +(``MergePipelineTest.PluginExternalClassificationSurvivesManifestComponentMetadataMerge``) and the per-route scope resolution is unit-covered (``ResolveEntitySourceFqnsTest.*``). This test pins the full HTTP-stack behaviour those two disjoint unit layers never exercise together: report a From 1b6f98ca4539cfb76b0d58a272a1911028f3daf6 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 16:59:28 +0200 Subject: [PATCH 10/11] test(gateway): match example-manifest header convention and locate main_ecu by id in list tests (#516) --- .../external_component_fault_manifest.yaml | 6 ++-- .../test/test_discovery_handlers.cpp | 30 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml b/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml index 106b543f6..8c3f56d12 100644 --- a/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml +++ b/src/ros2_medkit_gateway/config/examples/external_component_fault_manifest.yaml @@ -1,6 +1,6 @@ -# Copyright 2026 bburda -# -# SOVD manifest fixture for #516: an external Component (a PLC bridged into SOVD +# SOVD System Manifest: External-component fault-rollup fixture (#516) +# ==================================================================== +# An external Component (a PLC bridged into SOVD # by a protocol plugin) owns its fault_manager faults with NO child App located # on it. Mirrors the #517 external-App fixture but at the Component level. Also # carries AAS-Nameplate identity to confirm identity is orthogonal to the diff --git a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp index a3c2ec3d2..4c4fc2cbd 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp @@ -445,11 +445,21 @@ TEST_F(DiscoveryHandlersFixtureTest, ListComponentsReturnsMetadata) { auto result = handlers_->get_components(typed_req); auto body = body_json(result); - // lidar_unit has parent_component_id, so it's filtered from top-level list + // lidar_unit has parent_component_id, so it's filtered from top-level list. ASSERT_EQ(body["items"].size(), 2u); - EXPECT_EQ(body["items"][0]["id"], "main_ecu"); - EXPECT_EQ(body["items"][0]["description"], "Vehicle control unit"); - EXPECT_EQ(body["items"][0]["x-medkit"]["source"], "manifest"); + // get_components() does not sort; find main_ecu by id rather than assuming order. + const auto & items = body["items"]; + size_t main_ecu_idx = items.size(); + for (size_t i = 0; i < items.size(); ++i) { + if (items[i]["id"] == "main_ecu") { + main_ecu_idx = i; + break; + } + } + ASSERT_LT(main_ecu_idx, items.size()) << "main_ecu not found in /components list"; + const auto & main_ecu = items[main_ecu_idx]; + EXPECT_EQ(main_ecu["description"], "Vehicle control unit"); + EXPECT_EQ(main_ecu["x-medkit"]["source"], "manifest"); } // @verifies REQ_INTEROP_003 @@ -463,7 +473,17 @@ TEST_F(DiscoveryHandlersFixtureTest, ListComponentsIncludesManifestIdentity) { auto result = handlers_->get_components(typed_req); auto body = body_json(result); ASSERT_EQ(body["items"].size(), 2u); - const auto & identity = body["items"][0]["x-medkit"]["identity"]; + // get_components() does not sort; find main_ecu by id rather than assuming order. + const auto & items = body["items"]; + size_t main_ecu_idx = items.size(); + for (size_t i = 0; i < items.size(); ++i) { + if (items[i]["id"] == "main_ecu") { + main_ecu_idx = i; + break; + } + } + ASSERT_LT(main_ecu_idx, items.size()) << "main_ecu not found in /components list"; + const auto & identity = items[main_ecu_idx]["x-medkit"]["identity"]; // Typed fields serialize camelCase (AssetIdentity::to_json). EXPECT_EQ(identity["manufacturer"], "Acme Robotics"); EXPECT_EQ(identity["model"], "ECU-9000"); From 43b2609307e39fc2b915db766c9238d7501bb9e9 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 13 Jul 2026 18:09:31 +0200 Subject: [PATCH 11/11] feat(gateway): emit external on relationship routes, centralize the wire guard, and classify external assets (#516) --- docs/config/manifest-schema.rst | 6 ++ .../ros2_medkit_gateway/dto/x_medkit.hpp | 6 +- .../discovery/manifest/manifest_parser.cpp | 14 ++++- .../src/http/handlers/discovery_handlers.cpp | 39 ++++++++---- .../test/test_asset_inventory.cpp | 39 ++++++++++++ .../test/test_discovery_handlers.cpp | 60 ++++++++++++++++++- 6 files changed, 146 insertions(+), 18 deletions(-) diff --git a/docs/config/manifest-schema.rst b/docs/config/manifest-schema.rst index b1b783630..152a7fabf 100644 --- a/docs/config/manifest-schema.rst +++ b/docs/config/manifest-schema.rst @@ -429,6 +429,7 @@ Schema parent_component_id: string # Optional - parent component ID depends_on: [string] # Optional - component IDs this asset depends on tags: [string] # Optional - tags for filtering + external: boolean # Optional - non-ROS external asset (default: false) any_other_key: string # Kept verbatim as an identity extra Fields @@ -440,6 +441,11 @@ The identity keys accept the same aliases as the CSV import: keys land on the typed identity fields, not in the extras. Any scalar key not listed above is preserved as an identity extra. +``external`` classifies the asset as a non-ROS device, the same tri-state field +as on a ``components:`` entry. An external asset with no bound child apps owns +its fault scope by its own entity id. It is a recognized key, so it classifies +the Component instead of being kept 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 diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp index d6c0080f9..caacbb74d 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/x_medkit.hpp @@ -100,7 +100,8 @@ inline constexpr std::string_view dto_name = "XMedkitArea"; // description <- comp.description (via ext.add()) // contributors <- comp.contributors // capabilities <- capabilities JSON array (via ext.add()) -// external <- comp.external, non-ROS external asset classification; detail + list routes only (#516) +// external <- comp.external, non-ROS external asset classification; emitted true-only on every +// route that carries the component x-medkit, so "absence == not external" holds (#516) // // Also used in sub-collection responses (depends-on, subcomponents, hosts, contains, etc.): // missing <- true when component reference cannot be resolved @@ -149,7 +150,8 @@ inline constexpr std::string_view dto_name = "XMedkitComponent // ros2.node <- app.bound_fqn // component_id <- app.component_id // contributors <- app.contributors (detail only) -// external <- app.external, non-ROS external asset classification; detail + list routes only (#516/#517) +// external <- app.external, non-ROS external asset classification; emitted true-only on every route +// that carries the app x-medkit, so "absence == not external" holds (#516/#517) // // Also used in sub-collection responses (depends-on, hosts, function-hosts): // missing <- true when app reference cannot be resolved 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 aa5c4774f..006f1abb6 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -284,9 +284,9 @@ Component ManifestParser::parse_asset(const YAML::Node & node) const { // 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"}; + static const std::unordered_set structural = { + "tags", "parent_component_id", "name", "description", "namespace", "variant", + "type", "translation_id", "depends_on", "external"}; AssetEntry entry; if (node.IsMap()) { @@ -345,6 +345,14 @@ Component ManifestParser::parse_asset(const YAML::Node & node) const { comp.tags.push_back(tag); } + // An `assets:` entry may classify the device as a non-ROS external asset, + // mirroring `parse_component`. Tri-state: an absent key stays nullopt. + // `external` is in the `structural` set above so it is not swallowed as an + // identity extra (#516). + if (node["external"]) { + comp.external = node["external"].as(); + } + return comp; } diff --git a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp index f9bc6b85f..47f955a6e 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp @@ -34,6 +34,22 @@ namespace handlers { namespace { +/// Emit the x-medkit `external` flag only when the entity is effectively +/// external (tri-state true). Explicit false and unset both leave the field +/// absent, so the "absence == not external" wire contract holds. Centralised +/// here so no call site can leak `external: false` by assigning the raw +/// optional; every route that carries a Component/App x-medkit calls this (#516). +void set_x_medkit_external(dto::XMedkitComponent & x_medkit, const std::optional & external) { + if (external.value_or(false)) { + x_medkit.external = true; + } +} +void set_x_medkit_external(dto::XMedkitApp & x_medkit, const std::optional & external) { + if (external.value_or(false)) { + x_medkit.external = true; + } +} + /// Check if a capability name is already present in the capabilities array bool has_capability(const json & capabilities, const std::string & name) { for (const auto & cap : capabilities) { @@ -336,6 +352,7 @@ DiscoveryHandlers::get_area_components(const http::TypedRequest & req) { ros2.ns = component.namespace_path; x_medkit_comp.ros2 = ros2; } + set_x_medkit_external(x_medkit_comp, component.external); item.x_medkit = x_medkit_comp; response.items.push_back(std::move(item)); @@ -490,6 +507,7 @@ DiscoveryHandlers::get_area_contains(const http::TypedRequest & req) { ros2.ns = comp.namespace_path; x_medkit_comp.ros2 = ros2; } + set_x_medkit_external(x_medkit_comp, comp.external); item.x_medkit = x_medkit_comp; response.items.push_back(std::move(item)); @@ -561,9 +579,7 @@ DiscoveryHandlers::get_components(const http::TypedRequest & req) { if (!component.identity.empty()) { x_medkit_comp.identity = component.identity.to_json(); } - if (component.external.value_or(false)) { - x_medkit_comp.external = true; - } + set_x_medkit_external(x_medkit_comp, component.external); item.x_medkit = x_medkit_comp; response.items.push_back(std::move(item)); @@ -694,9 +710,7 @@ http::Result DiscoveryHandlers::get_component(const http:: if (!comp.identity.empty()) { x_medkit_comp.identity = comp.identity.to_json(); } - if (comp.external.value_or(false)) { - x_medkit_comp.external = true; - } + set_x_medkit_external(x_medkit_comp, comp.external); using Cap = CapabilityBuilder::Capability; std::vector caps = { @@ -776,6 +790,7 @@ DiscoveryHandlers::get_subcomponents(const http::TypedRequest & req) { ros2.ns = sub.namespace_path; x_medkit_comp.ros2 = ros2; } + set_x_medkit_external(x_medkit_comp, sub.external); item.x_medkit = x_medkit_comp; response.items.push_back(std::move(item)); @@ -850,6 +865,7 @@ http::Result> DiscoveryHandlers::get_component ros2.node = *app.bound_fqn; x_medkit_app.ros2 = ros2; } + set_x_medkit_external(x_medkit_app, app.external); item.x_medkit = x_medkit_app; response.items.push_back(std::move(item)); @@ -914,6 +930,7 @@ DiscoveryHandlers::get_component_depends_on(const http::TypedRequest & req) { if (!dep_opt->source.empty()) { x_medkit_comp.source = dep_opt->source; } + set_x_medkit_external(x_medkit_comp, dep_opt->external); item.x_medkit = x_medkit_comp; } else { item.name = dep_id; @@ -981,9 +998,7 @@ http::Result> DiscoveryHandlers::get_apps(cons ros2.node = *app.bound_fqn; x_medkit_app.ros2 = ros2; } - if (app.external.value_or(false)) { - x_medkit_app.external = true; - } + set_x_medkit_external(x_medkit_app, app.external); item.x_medkit = x_medkit_app; response.items.push_back(std::move(item)); @@ -1122,9 +1137,7 @@ http::Result DiscoveryHandlers::get_app(const http::TypedRequest if (!app.contributors.empty()) { x_medkit_app.contributors = sorted_contributors(app.contributors); } - if (app.external.value_or(false)) { - x_medkit_app.external = true; - } + set_x_medkit_external(x_medkit_app, app.external); detail.x_medkit = x_medkit_app; return detail; @@ -1184,6 +1197,7 @@ http::Result> DiscoveryHandlers::get_app_depen x_medkit_app.source = dep_opt->source; } x_medkit_app.is_online = dep_opt->is_online; + set_x_medkit_external(x_medkit_app, dep_opt->external); item.x_medkit = x_medkit_app; } else { item.name = dep_id; @@ -1571,6 +1585,7 @@ http::Result> DiscoveryHandlers::get_function_ ros2.node = *app_opt->bound_fqn; x_medkit_app.ros2 = ros2; } + set_x_medkit_external(x_medkit_app, app_opt->external); item.x_medkit = x_medkit_app; response.items.push_back(std::move(item)); diff --git a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp index 4e3a9ffe5..c4c29da35 100644 --- a/src/ros2_medkit_gateway/test/test_asset_inventory.cpp +++ b/src/ros2_medkit_gateway/test/test_asset_inventory.cpp @@ -368,6 +368,45 @@ manifest_version: "1.0" EXPECT_EQ(comp->identity.provenance.at("model"), "inventory"); } +TEST(ManifestAssetsTest, ExternalKeyClassifiesAssetComponent) { + // An `assets:` entry is the inventory path for non-ROS hardware (#516), so an + // explicit `external:` must classify the derived Component instead of being + // swallowed as an identity extra. Tri-state is preserved (mirrors parse_component). + const std::string yaml = R"( +manifest_version: "1.0" +assets: + - id: plc_ext + manufacturer: Siemens + model: S7-1500 + external: true + - id: drive_ros + manufacturer: ABB + model: ACS880 + external: false + - id: sensor_unset + manufacturer: Bosch + model: BMI-088 +)"; + ManifestParser parser; + Manifest manifest = parser.parse_string(yaml); + + const Component * plc = find_component(manifest.components, "plc_ext"); + ASSERT_NE(plc, nullptr); + ASSERT_TRUE(plc->external.has_value()); + EXPECT_TRUE(plc->external.value()); + // The `external` key must classify, not fall through to identity extras. + EXPECT_EQ(plc->identity.extra.count("external"), 0u); + + const Component * drive = find_component(manifest.components, "drive_ros"); + ASSERT_NE(drive, nullptr); + ASSERT_TRUE(drive->external.has_value()); + EXPECT_FALSE(drive->external.value()); + + const Component * sensor = find_component(manifest.components, "sensor_unset"); + ASSERT_NE(sensor, nullptr); + EXPECT_FALSE(sensor->external.has_value()); +} + TEST(ManifestAssetsTest, AreaAndExplicitOverrides) { const std::string yaml = R"( manifest_version: "1.0" diff --git a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp index 4c4fc2cbd..a9ab720b4 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp @@ -98,6 +98,12 @@ manifest_version: "1.0" identity: manufacturer: "Siemens" serial_number: "SN-1" + - id: ext_sensor + name: "External Sensor" + parent_component_id: "main_ecu" + external: true + identity: + manufacturer: "Bosch" apps: - id: "planner" name: "Planner" @@ -577,8 +583,60 @@ TEST_F(DiscoveryHandlersFixtureTest, GetSubcomponentsReturnsChildren) { auto result = handlers_->get_subcomponents(typed_req); auto body = body_json(result); + // main_ecu has two subcomponents: lidar_unit (ROS) and ext_sensor (external). + ASSERT_EQ(body["items"].size(), 2u); + bool has_lidar = false; + bool has_ext_sensor = false; + for (const auto & item : body["items"]) { + if (item["id"] == "lidar_unit") { + has_lidar = true; + } + if (item["id"] == "ext_sensor") { + has_ext_sensor = true; + } + } + EXPECT_TRUE(has_lidar); + EXPECT_TRUE(has_ext_sensor); +} + +// A device Component browsed via a relationship/sub-collection route must still +// read as external (the "absence == not external" invariant), so the external +// subcomponent carries x-medkit.external while the plain ROS one does not. +TEST_F(DiscoveryHandlersFixtureTest, SubcomponentsEmitExternalForExternalChild) { + httplib::Request req; + auto typed_req = make_typed_request(req, "/api/v1/components/main_ecu/subcomponents", + R"(/api/v1/components/([^/]+)/subcomponents)"); + + auto result = handlers_->get_subcomponents(typed_req); + auto body = body_json(result); + const auto & items = body["items"]; + size_t ext_idx = items.size(); + size_t lidar_idx = items.size(); + for (size_t i = 0; i < items.size(); ++i) { + if (items[i]["id"] == "ext_sensor") { + ext_idx = i; + } + if (items[i]["id"] == "lidar_unit") { + lidar_idx = i; + } + } + ASSERT_LT(ext_idx, items.size()) << "ext_sensor not found in subcomponents"; + ASSERT_LT(lidar_idx, items.size()) << "lidar_unit not found in subcomponents"; + EXPECT_EQ(items[ext_idx]["x-medkit"]["external"], true); + EXPECT_FALSE(items[lidar_idx]["x-medkit"].contains("external")); +} + +// The hosts (App) projection must also carry x-medkit.external: ext_plc hosts +// the external app ext_app, so a consumer browsing hosts sees the classification. +TEST_F(DiscoveryHandlersFixtureTest, ComponentHostsEmitExternalForExternalApp) { + httplib::Request req; + auto typed_req = make_typed_request(req, "/api/v1/components/ext_plc/hosts", R"(/api/v1/components/([^/]+)/hosts)"); + + auto result = handlers_->get_component_hosts(typed_req); + auto body = body_json(result); ASSERT_EQ(body["items"].size(), 1u); - EXPECT_EQ(body["items"][0]["id"], "lidar_unit"); + EXPECT_EQ(body["items"][0]["id"], "ext_app"); + EXPECT_EQ(body["items"][0]["x-medkit"]["external"], true); } // @verifies REQ_INTEROP_005