From 2ba5b35620cadd6bf85ff4c7b98cedfa2efd9575 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 4 Mar 2026 17:35:13 +0100 Subject: [PATCH 1/6] feat(discovery): add layered MergePipeline with field-group merge Introduce MergePolicy (AUTHORITATIVE/ENRICHMENT/FALLBACK), FieldGroup enums, MergeReport diagnostics, DiscoveryLayer interface and LayerOutput struct. Implement MergePipeline core with single-layer passthrough and multi-layer field-group merge with conflict resolution. --- src/ros2_medkit_gateway/CMakeLists.txt | 6 + .../discovery/discovery_layer.hpp | 64 +++ .../discovery/merge_pipeline.hpp | 82 ++++ .../discovery/merge_types.hpp | 87 ++++ .../src/discovery/merge_pipeline.cpp | 422 ++++++++++++++++++ .../test/test_merge_pipeline.cpp | 321 +++++++++++++ 6 files changed, 982 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp create mode 100644 src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp create mode 100644 src/ros2_medkit_gateway/test/test_merge_pipeline.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 835d05ec..62b932be 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -92,6 +92,7 @@ add_library(gateway_lib STATIC src/discovery/discovery_manager.cpp src/discovery/runtime_discovery.cpp src/discovery/hybrid_discovery.cpp + src/discovery/merge_pipeline.cpp # Discovery models (with .cpp serialization) src/discovery/models/app.cpp src/discovery/models/function.cpp @@ -325,6 +326,10 @@ if(BUILD_TESTING) ament_add_gtest(test_runtime_linker test/test_runtime_linker.cpp) target_link_libraries(test_runtime_linker gateway_lib) + # Add merge pipeline tests + ament_add_gtest(test_merge_pipeline test/test_merge_pipeline.cpp) + target_link_libraries(test_merge_pipeline gateway_lib) + # Add capability builder tests ament_add_gtest(test_capability_builder test/test_capability_builder.cpp) target_link_libraries(test_capability_builder gateway_lib) @@ -488,6 +493,7 @@ if(BUILD_TESTING) test_plugin_manager test_log_manager test_log_handlers + test_merge_pipeline ) foreach(_target ${_test_targets}) target_compile_options(${_target} PRIVATE --coverage -O0 -g) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp new file mode 100644 index 00000000..22d107eb --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp @@ -0,0 +1,64 @@ +// 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/discovery/merge_types.hpp" +#include "ros2_medkit_gateway/discovery/models/app.hpp" +#include "ros2_medkit_gateway/discovery/models/area.hpp" +#include "ros2_medkit_gateway/discovery/models/component.hpp" +#include "ros2_medkit_gateway/discovery/models/function.hpp" + +#include + +#include +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief Output produced by a discovery layer + */ +struct LayerOutput { + std::vector areas; + std::vector components; + std::vector apps; + std::vector functions; + std::unordered_map entity_metadata; ///< entity id -> metadata +}; + +/** + * @brief Interface for a pluggable discovery data source + * + * Each layer produces entities and declares how its data should be merged + * with other layers via per-field-group MergePolicy. + */ +class DiscoveryLayer { + public: + virtual ~DiscoveryLayer() = default; + + /// Human-readable layer name (e.g., "manifest", "runtime", plugin name) + virtual std::string name() const = 0; + + /// Discover entities from this layer's source + virtual LayerOutput discover() = 0; + + /// Merge policy this layer uses for the given field group + virtual MergePolicy policy_for(FieldGroup group) const = 0; +}; + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp new file mode 100644 index 00000000..bd75b957 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp @@ -0,0 +1,82 @@ +// 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/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/discovery/merge_types.hpp" + +#include +#include + +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief Result of a merge pipeline execution + */ +struct MergeResult { + std::vector areas; + std::vector components; + std::vector apps; + std::vector functions; + MergeReport report; +}; + +/** + * @brief Orchestrates multiple discovery layers with configurable merge policies + * + * Layers are added in priority order (first added = highest priority). + * Each layer produces entities and declares per-field-group MergePolicy. + * The pipeline merges entities by ID, resolving conflicts per policy. + */ +class MergePipeline { + public: + explicit MergePipeline(rclcpp::Logger logger = rclcpp::get_logger("merge_pipeline")); + + /** + * @brief Add a discovery layer to the pipeline + * @param layer Layer to add (priority = insertion order, first = highest) + */ + void add_layer(std::unique_ptr layer); + + /** + * @brief Execute all layers and merge results + * @return Merged entities with diagnostics report + */ + MergeResult execute(); + + /** + * @brief Get the last merge report + */ + const MergeReport & get_last_report() const { + return last_report_; + } + + private: + /// Merge a vector of entities from multiple layers by ID + template + std::vector merge_entities(const std::vector>> & layer_entities, + MergeReport & report); + + rclcpp::Logger logger_; + std::vector> layers_; + MergeReport last_report_; +}; + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp new file mode 100644 index 00000000..e27be588 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp @@ -0,0 +1,87 @@ +// 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 + +#include +#include +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief Merge precedence policy for a discovery layer's field group + */ +enum class MergePolicy { + AUTHORITATIVE, ///< This layer's value wins over lower-priority layers + ENRICHMENT, ///< Fill empty fields only, don't override existing values + FALLBACK ///< Use only if no other layer provides the value +}; + +/** + * @brief Logical groupings of entity fields with the same merge behavior + */ +enum class FieldGroup { + IDENTITY, ///< id, name, translation_id, description, tags + HIERARCHY, ///< area, component_id, parent_*, depends_on, hosts + LIVE_DATA, ///< topics, services, actions + STATUS, ///< is_online, bound_fqn + METADATA ///< source, x-medkit extensions, custom fields +}; + +/** + * @brief Record of a merge conflict between two layers + */ +struct MergeConflict { + std::string entity_id; + FieldGroup field_group; + std::string winning_layer; + std::string losing_layer; +}; + +/** + * @brief Diagnostics report from a merge pipeline execution + */ +struct MergeReport { + std::vector layers; + std::vector conflicts; + std::unordered_map entity_source; ///< entity id -> primary layer name + size_t total_entities{0}; + size_t enriched_count{0}; + size_t conflict_count{0}; + size_t id_collision_count{0}; + + nlohmann::json to_json() const { + return {{"layers", layers}, + {"total_entities", total_entities}, + {"enriched_count", enriched_count}, + {"conflict_count", conflict_count}, + {"id_collisions", id_collision_count}}; + } +}; + +} // namespace discovery +} // namespace ros2_medkit_gateway + +// Required: C++17 does not provide std::hash for enum class types +template <> +struct std::hash { + size_t operator()(ros2_medkit_gateway::discovery::FieldGroup fg) const noexcept { + return std::hash{}(static_cast(fg)); + } +}; diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp new file mode 100644 index 00000000..27d6efa7 --- /dev/null +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -0,0 +1,422 @@ +// 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/discovery/merge_pipeline.hpp" + +#include +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +namespace { + +// Policy priority: AUTH=2, ENRICH=1, FALLBACK=0 +int policy_priority(MergePolicy p) { + switch (p) { + case MergePolicy::AUTHORITATIVE: + return 2; + case MergePolicy::ENRICHMENT: + return 1; + case MergePolicy::FALLBACK: + return 0; + } + return 0; +} + +enum class MergeWinner { TARGET, SOURCE, BOTH }; + +struct MergeResolution { + MergeWinner scalar; + MergeWinner collection; + bool is_conflict{false}; +}; + +MergeResolution resolve_policies(MergePolicy target_policy, MergePolicy source_policy) { + int tp = policy_priority(target_policy); + int sp = policy_priority(source_policy); + + if (tp > sp) { + return {MergeWinner::TARGET, MergeWinner::TARGET, false}; + } else if (sp > tp) { + return {MergeWinner::SOURCE, MergeWinner::SOURCE, false}; + } else { + // Same level + if (target_policy == MergePolicy::AUTHORITATIVE) { + return {MergeWinner::TARGET, MergeWinner::TARGET, true}; // AUTH vs AUTH -> conflict + } else if (target_policy == MergePolicy::ENRICHMENT) { + return {MergeWinner::BOTH, MergeWinner::BOTH, false}; // mutual enrichment + } else { + return {MergeWinner::TARGET, MergeWinner::TARGET, false}; // FALLBACK vs FALLBACK + } + } +} + +void merge_scalar(std::string & target, const std::string & source, MergeWinner winner) { + switch (winner) { + case MergeWinner::SOURCE: + if (!source.empty()) { + target = source; + } + break; + case MergeWinner::BOTH: + // First non-empty (target preferred since higher priority) + if (target.empty() && !source.empty()) { + target = source; + } + break; + case MergeWinner::TARGET: + break; + } +} + +void merge_bool(bool & target, bool source, MergeWinner winner) { + switch (winner) { + case MergeWinner::SOURCE: + target = source; + break; + case MergeWinner::BOTH: + // Either true wins + target = target || source; + break; + case MergeWinner::TARGET: + break; + } +} + +void merge_collection(std::vector & target, const std::vector & source, MergeWinner winner) { + switch (winner) { + case MergeWinner::SOURCE: + target = source; + break; + case MergeWinner::BOTH: { + std::unordered_set seen(target.begin(), target.end()); + for (const auto & s : source) { + if (seen.insert(s).second) { + target.push_back(s); + } + } + break; + } + case MergeWinner::TARGET: + break; + } +} + +template +void merge_by_key(std::vector & target, const std::vector & source, KeyFn key_fn, MergeWinner winner) { + switch (winner) { + case MergeWinner::SOURCE: + target = source; + break; + case MergeWinner::BOTH: { + std::unordered_set seen; + for (const auto & t : target) { + seen.insert(key_fn(t)); + } + for (const auto & s : source) { + if (seen.insert(key_fn(s)).second) { + target.push_back(s); + } + } + break; + } + case MergeWinner::TARGET: + break; + } +} + +template +void merge_optional(std::optional & target, const std::optional & source, MergeWinner winner) { + switch (winner) { + case MergeWinner::SOURCE: + if (source.has_value()) { + target = source; + } + break; + case MergeWinner::BOTH: + if (!target.has_value() && source.has_value()) { + target = source; + } + break; + case MergeWinner::TARGET: + break; + } +} + +void merge_topics(ComponentTopics & target, const ComponentTopics & source, MergeWinner winner) { + merge_collection(target.publishes, source.publishes, winner); + merge_collection(target.subscribes, source.subscribes, winner); +} + +// Per-entity-type field-group merge dispatch +template +void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup group, const MergeResolution & res) { + if constexpr (std::is_same_v) { + switch (group) { + case FieldGroup::IDENTITY: + merge_scalar(target.name, source.name, res.scalar); + merge_scalar(target.translation_id, source.translation_id, res.scalar); + merge_scalar(target.description, source.description, res.scalar); + merge_collection(target.tags, source.tags, res.collection); + break; + case FieldGroup::HIERARCHY: + merge_scalar(target.namespace_path, source.namespace_path, res.scalar); + merge_scalar(target.parent_area_id, source.parent_area_id, res.scalar); + break; + default: + break; + } + } else if constexpr (std::is_same_v) { + switch (group) { + case FieldGroup::IDENTITY: + merge_scalar(target.name, source.name, res.scalar); + merge_scalar(target.translation_id, source.translation_id, res.scalar); + merge_scalar(target.description, source.description, res.scalar); + merge_collection(target.tags, source.tags, res.collection); + break; + case FieldGroup::HIERARCHY: + merge_scalar(target.namespace_path, source.namespace_path, res.scalar); + merge_scalar(target.fqn, source.fqn, res.scalar); + merge_scalar(target.area, source.area, res.scalar); + merge_scalar(target.parent_component_id, source.parent_component_id, res.scalar); + merge_collection(target.depends_on, source.depends_on, res.collection); + break; + case FieldGroup::LIVE_DATA: + merge_topics(target.topics, source.topics, res.collection); + merge_by_key( + target.services, source.services, + [](const ServiceInfo & s) { + return s.full_path; + }, + res.collection); + merge_by_key( + target.actions, source.actions, + [](const ActionInfo & a) { + return a.full_path; + }, + res.collection); + break; + case FieldGroup::METADATA: + merge_scalar(target.source, source.source, res.scalar); + merge_scalar(target.variant, source.variant, res.scalar); + break; + default: + break; + } + } else if constexpr (std::is_same_v) { + switch (group) { + case FieldGroup::IDENTITY: + merge_scalar(target.name, source.name, res.scalar); + merge_scalar(target.translation_id, source.translation_id, res.scalar); + merge_scalar(target.description, source.description, res.scalar); + merge_collection(target.tags, source.tags, res.collection); + break; + case FieldGroup::HIERARCHY: + merge_scalar(target.component_id, source.component_id, res.scalar); + merge_collection(target.depends_on, source.depends_on, res.collection); + break; + case FieldGroup::LIVE_DATA: + merge_topics(target.topics, source.topics, res.collection); + merge_by_key( + target.services, source.services, + [](const ServiceInfo & s) { + return s.full_path; + }, + res.collection); + merge_by_key( + target.actions, source.actions, + [](const ActionInfo & a) { + return a.full_path; + }, + res.collection); + break; + case FieldGroup::STATUS: + merge_bool(target.is_online, source.is_online, res.scalar); + merge_optional(target.bound_fqn, source.bound_fqn, res.scalar); + merge_bool(target.external, source.external, res.scalar); + break; + case FieldGroup::METADATA: + merge_scalar(target.source, source.source, res.scalar); + merge_optional(target.ros_binding, source.ros_binding, res.scalar); + break; + } + } else if constexpr (std::is_same_v) { + switch (group) { + case FieldGroup::IDENTITY: + merge_scalar(target.name, source.name, res.scalar); + merge_scalar(target.translation_id, source.translation_id, res.scalar); + merge_scalar(target.description, source.description, res.scalar); + merge_collection(target.tags, source.tags, res.collection); + break; + case FieldGroup::HIERARCHY: + merge_collection(target.hosts, source.hosts, res.collection); + merge_collection(target.depends_on, source.depends_on, res.collection); + break; + case FieldGroup::METADATA: + merge_scalar(target.source, source.source, res.scalar); + break; + default: + break; + } + } +} + +constexpr FieldGroup ALL_FIELD_GROUPS[] = {FieldGroup::IDENTITY, FieldGroup::HIERARCHY, FieldGroup::LIVE_DATA, + FieldGroup::STATUS, FieldGroup::METADATA}; + +} // namespace + +MergePipeline::MergePipeline(rclcpp::Logger logger) : logger_(std::move(logger)) { +} + +void MergePipeline::add_layer(std::unique_ptr layer) { + layers_.push_back(std::move(layer)); +} + +template +std::vector +MergePipeline::merge_entities(const std::vector>> & layer_entities, + MergeReport & report) { + // Collect all entities by ID with their layer index + struct LayerEntity { + size_t layer_idx; + Entity entity; + }; + std::unordered_map> by_id; + std::vector insertion_order; + + for (const auto & [layer_idx, entities] : layer_entities) { + for (const auto & entity : entities) { + if (by_id.find(entity.id) == by_id.end()) { + insertion_order.push_back(entity.id); + } + by_id[entity.id].push_back({layer_idx, entity}); + } + } + + std::vector result; + result.reserve(insertion_order.size()); + + for (const auto & id : insertion_order) { + auto & entries = by_id[id]; + + // Start with highest-priority layer's entity as base + Entity merged = std::move(entries[0].entity); + size_t owner_layer_idx = entries[0].layer_idx; + report.entity_source[id] = layers_[owner_layer_idx]->name(); + + // Merge with each subsequent (lower-priority) layer + for (size_t i = 1; i < entries.size(); i++) { + size_t source_layer_idx = entries[i].layer_idx; + report.enriched_count++; + + for (auto fg : ALL_FIELD_GROUPS) { + auto target_policy = layers_[owner_layer_idx]->policy_for(fg); + auto source_policy = layers_[source_layer_idx]->policy_for(fg); + auto res = resolve_policies(target_policy, source_policy); + + if (res.is_conflict) { + report.conflicts.push_back({id, fg, layers_[owner_layer_idx]->name(), layers_[source_layer_idx]->name()}); + report.conflict_count++; + } + + apply_field_group_merge(merged, entries[i].entity, fg, res); + } + } + + result.push_back(std::move(merged)); + } + + return result; +} + +MergeResult MergePipeline::execute() { + MergeReport report; + for (const auto & layer : layers_) { + report.layers.push_back(layer->name()); + } + + // Collect outputs from all layers + std::vector>> area_layers; + std::vector>> component_layers; + std::vector>> app_layers; + std::vector>> function_layers; + + for (size_t i = 0; i < layers_.size(); ++i) { + auto output = layers_[i]->discover(); + if (!output.areas.empty()) { + area_layers.emplace_back(i, std::move(output.areas)); + } + if (!output.components.empty()) { + component_layers.emplace_back(i, std::move(output.components)); + } + if (!output.apps.empty()) { + app_layers.emplace_back(i, std::move(output.apps)); + } + if (!output.functions.empty()) { + function_layers.emplace_back(i, std::move(output.functions)); + } + } + + MergeResult result; + result.areas = merge_entities(area_layers, report); + result.components = merge_entities(component_layers, report); + result.apps = merge_entities(app_layers, report); + result.functions = merge_entities(function_layers, report); + + report.total_entities = result.areas.size() + result.components.size() + result.apps.size() + result.functions.size(); + + // Cross-type ID collision detection + std::unordered_map global_ids; + auto check_ids = [&](const auto & entities, const std::string & type) { + for (const auto & e : entities) { + auto [it, inserted] = global_ids.emplace(e.id, type); + if (!inserted && it->second != type) { + report.id_collision_count++; + RCLCPP_ERROR(logger_, "ID collision: '%s' used by both %s and %s", e.id.c_str(), it->second.c_str(), + type.c_str()); + } + } + }; + check_ids(result.areas, "Area"); + check_ids(result.components, "Component"); + check_ids(result.apps, "App"); + check_ids(result.functions, "Function"); + + RCLCPP_INFO(logger_, "MergePipeline: %zu entities from %zu layers, %zu enriched, %zu conflicts", + report.total_entities, report.layers.size(), report.enriched_count, report.conflict_count); + for (const auto & conflict : report.conflicts) { + RCLCPP_WARN(logger_, "Merge conflict: entity '%s' field_group %d - '%s' wins over '%s'", conflict.entity_id.c_str(), + static_cast(conflict.field_group), conflict.winning_layer.c_str(), conflict.losing_layer.c_str()); + } + + result.report = std::move(report); + last_report_ = result.report; + return result; +} + +// Explicit template instantiations +template std::vector +MergePipeline::merge_entities(const std::vector>> &, MergeReport &); +template std::vector +MergePipeline::merge_entities(const std::vector>> &, MergeReport &); +template std::vector MergePipeline::merge_entities(const std::vector>> &, + MergeReport &); +template std::vector +MergePipeline::merge_entities(const std::vector>> &, MergeReport &); + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp new file mode 100644 index 00000000..5150b926 --- /dev/null +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -0,0 +1,321 @@ +// 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/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" +#include "ros2_medkit_gateway/discovery/merge_types.hpp" + +#include + +using namespace ros2_medkit_gateway::discovery; +using namespace ros2_medkit_gateway; + +// Concrete test layer for unit testing +class TestLayer : public DiscoveryLayer { + public: + TestLayer(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); + if (it != policies_.end()) { + return it->second; + } + return MergePolicy::ENRICHMENT; // default + } + + private: + std::string name_; + LayerOutput output_; + std::unordered_map policies_; +}; + +TEST(MergeTypesTest, MergePolicyValues) { + // Verify enum values exist and are distinct + EXPECT_NE(static_cast(MergePolicy::AUTHORITATIVE), static_cast(MergePolicy::ENRICHMENT)); + EXPECT_NE(static_cast(MergePolicy::ENRICHMENT), static_cast(MergePolicy::FALLBACK)); +} + +TEST(MergeTypesTest, FieldGroupValues) { + // Verify all 5 field groups exist + EXPECT_NE(static_cast(FieldGroup::IDENTITY), static_cast(FieldGroup::HIERARCHY)); + EXPECT_NE(static_cast(FieldGroup::LIVE_DATA), static_cast(FieldGroup::STATUS)); + auto metadata = FieldGroup::METADATA; + (void)metadata; +} + +TEST(MergeTypesTest, MergeReportDefaultEmpty) { + MergeReport report; + EXPECT_TRUE(report.conflicts.empty()); + EXPECT_TRUE(report.entity_source.empty()); + EXPECT_EQ(report.total_entities, 0u); + EXPECT_EQ(report.enriched_count, 0u); + EXPECT_EQ(report.conflict_count, 0u); + EXPECT_EQ(report.id_collision_count, 0u); +} + +TEST(MergeTypesTest, MergeReportToJson) { + MergeReport report; + report.layers = {"manifest", "runtime"}; + report.total_entities = 10; + report.enriched_count = 7; + report.conflict_count = 2; + report.id_collision_count = 0; + + auto j = report.to_json(); + EXPECT_EQ(j["total_entities"], 10); + EXPECT_EQ(j["enriched_count"], 7); + EXPECT_EQ(j["conflict_count"], 2); + EXPECT_EQ(j["id_collisions"], 0); + EXPECT_EQ(j["layers"].size(), 2u); +} + +TEST(DiscoveryLayerTest, TestLayerReturnsConfiguredOutput) { + LayerOutput output; + Area area; + area.id = "powertrain"; + area.name = "Powertrain"; + output.areas.push_back(area); + + TestLayer layer("test", output); + EXPECT_EQ(layer.name(), "test"); + + auto result = layer.discover(); + ASSERT_EQ(result.areas.size(), 1u); + EXPECT_EQ(result.areas[0].id, "powertrain"); +} + +TEST(DiscoveryLayerTest, PolicyForReturnsConfiguredPolicy) { + TestLayer layer("test", {}, + {{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, {FieldGroup::LIVE_DATA, MergePolicy::FALLBACK}}); + + EXPECT_EQ(layer.policy_for(FieldGroup::IDENTITY), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::LIVE_DATA), MergePolicy::FALLBACK); + EXPECT_EQ(layer.policy_for(FieldGroup::STATUS), MergePolicy::ENRICHMENT); // default +} + +// Helper to create test entities +namespace { + +Area make_area(const std::string & id, const std::string & name = "") { + Area a; + a.id = id; + a.name = name.empty() ? id : name; + a.namespace_path = "/" + id; + return a; +} + +Component make_component(const std::string & id, const std::string & area = "", const std::string & ns = "/") { + Component c; + c.id = id; + c.name = id; + c.area = area; + c.namespace_path = ns; + c.fqn = ns == "/" ? "/" + id : ns + "/" + id; + c.source = "test"; + return c; +} + +App make_app(const std::string & id, const std::string & component_id = "") { + App a; + a.id = id; + a.name = id; + a.component_id = component_id; + a.source = "test"; + return a; +} + +} // namespace + +class MergePipelineTest : public ::testing::Test { + protected: + MergePipeline pipeline_; +}; + +TEST_F(MergePipelineTest, EmptyPipelineReturnsEmptyResult) { + auto result = pipeline_.execute(); + EXPECT_TRUE(result.areas.empty()); + EXPECT_TRUE(result.components.empty()); + EXPECT_TRUE(result.apps.empty()); + EXPECT_TRUE(result.functions.empty()); + EXPECT_EQ(result.report.total_entities, 0u); +} + +TEST_F(MergePipelineTest, SingleLayerPassthrough) { + LayerOutput output; + output.areas.push_back(make_area("powertrain")); + output.components.push_back(make_component("engine", "powertrain", "/powertrain")); + output.apps.push_back(make_app("engine_app", "engine")); + + pipeline_.add_layer(std::make_unique("manifest", output)); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.areas.size(), 1u); + EXPECT_EQ(result.areas[0].id, "powertrain"); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].id, "engine"); + ASSERT_EQ(result.apps.size(), 1u); + EXPECT_EQ(result.apps[0].id, "engine_app"); + EXPECT_EQ(result.report.total_entities, 3u); + EXPECT_EQ(result.report.conflict_count, 0u); +} + +TEST_F(MergePipelineTest, MultipleLayersDisjointEntities) { + LayerOutput manifest_output; + manifest_output.areas.push_back(make_area("powertrain")); + + LayerOutput runtime_output; + runtime_output.areas.push_back(make_area("chassis")); + + pipeline_.add_layer(std::make_unique("manifest", manifest_output)); + pipeline_.add_layer(std::make_unique("runtime", runtime_output)); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.areas.size(), 2u); + EXPECT_EQ(result.report.total_entities, 2u); + EXPECT_EQ(result.report.conflict_count, 0u); +} + +TEST_F(MergePipelineTest, AuthoritativeWinsOverEnrichment) { + // Manifest: IDENTITY=AUTH, LIVE_DATA=ENRICH + // Runtime: IDENTITY=FALLBACK, LIVE_DATA=AUTH + // Same component in both - manifest name wins, runtime topics win + + Component manifest_comp = make_component("engine", "powertrain", "/powertrain"); + manifest_comp.name = "Engine ECU"; + manifest_comp.source = "manifest"; + + Component runtime_comp = make_component("engine", "powertrain", "/powertrain"); + runtime_comp.name = "engine"; + runtime_comp.source = "node"; + runtime_comp.topics.publishes = {"/powertrain/engine/rpm"}; + + LayerOutput manifest_out; + manifest_out.components.push_back(manifest_comp); + + LayerOutput runtime_out; + runtime_out.components.push_back(runtime_comp); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::IDENTITY, 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 result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].name, "Engine ECU"); // manifest IDENTITY wins + EXPECT_EQ(result.components[0].topics.publishes.size(), 1u); // runtime LIVE_DATA wins + EXPECT_EQ(result.components[0].source, "manifest"); // higher priority source +} + +TEST_F(MergePipelineTest, EnrichmentFillsEmptyFields) { + // Manifest has name but no topics + // Runtime has topics + // Both declare ENRICHMENT for LIVE_DATA + // Result: topics filled from runtime + + Component manifest_comp = make_component("engine", "powertrain", "/powertrain"); + manifest_comp.source = "manifest"; + + Component runtime_comp = make_component("engine", "powertrain", "/powertrain"); + runtime_comp.topics.publishes = {"/powertrain/engine/rpm"}; + runtime_comp.topics.subscribes = {"/powertrain/engine/throttle"}; + + LayerOutput manifest_out; + manifest_out.components.push_back(manifest_comp); + + LayerOutput runtime_out; + runtime_out.components.push_back(runtime_comp); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}})); + pipeline_.add_layer(std::make_unique( + "runtime", runtime_out, + std::unordered_map{{FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_FALSE(result.components[0].topics.publishes.empty()); +} + +TEST_F(MergePipelineTest, AuthoritativeVsAuthoritativeHigherPriorityWins) { + // Both layers claim AUTHORITATIVE for IDENTITY + // Higher priority (first added) wins, conflict logged + + Component manifest_comp = make_component("engine", "powertrain", "/powertrain"); + manifest_comp.name = "Manifest Engine"; + + Component runtime_comp = make_component("engine", "powertrain", "/powertrain"); + runtime_comp.name = "Runtime Engine"; + + LayerOutput manifest_out; + manifest_out.components.push_back(manifest_comp); + LayerOutput runtime_out; + runtime_out.components.push_back(runtime_comp); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}})); + pipeline_.add_layer(std::make_unique( + "runtime", runtime_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].name, "Manifest Engine"); // higher priority wins + EXPECT_GE(result.report.conflict_count, 1u); // conflict recorded +} + +TEST_F(MergePipelineTest, CollectionFieldsUnionOnEnrichment) { + // Both layers provide services for the same component with ENRICHMENT + // Result: union of services (deduped by full_path) + + Component layer1_comp = make_component("engine", "", "/powertrain"); + layer1_comp.services.push_back( + ServiceInfo{"calibrate", "/powertrain/engine/calibrate", "std_srvs/srv/Trigger", std::nullopt}); + + Component layer2_comp = make_component("engine", "", "/powertrain"); + layer2_comp.services.push_back( + ServiceInfo{"calibrate", "/powertrain/engine/calibrate", "std_srvs/srv/Trigger", std::nullopt}); + layer2_comp.services.push_back( + ServiceInfo{"reset", "/powertrain/engine/reset", "std_srvs/srv/Trigger", std::nullopt}); + + LayerOutput out1, out2; + out1.components.push_back(layer1_comp); + out2.components.push_back(layer2_comp); + + pipeline_.add_layer(std::make_unique( + "layer1", out1, std::unordered_map{{FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}})); + pipeline_.add_layer(std::make_unique( + "layer2", out2, std::unordered_map{{FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + // Union: calibrate (deduped) + reset = 2 services + EXPECT_EQ(result.components[0].services.size(), 2u); +} From 224b8f209788a8d44f4f90f1b416fa3124af31e8 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 4 Mar 2026 18:20:22 +0100 Subject: [PATCH 2/6] feat(discovery): add ManifestLayer, RuntimeLayer, and PluginLayer Implement concrete DiscoveryLayer wrappers: ManifestLayer (delegates to ManifestManager, AUTHORITATIVE for identity/hierarchy/metadata), RuntimeLayer (delegates to RuntimeDiscoveryStrategy, AUTHORITATIVE for live_data/status), PluginLayer (wraps IntrospectionProvider, ENRICHMENT for all fields). Add GapFillConfig for RuntimeLayer namespace filtering in hybrid mode. --- src/ros2_medkit_gateway/CMakeLists.txt | 3 + .../discovery/layers/manifest_layer.hpp | 49 +++++++ .../discovery/layers/plugin_layer.hpp | 58 ++++++++ .../discovery/layers/runtime_layer.hpp | 56 ++++++++ .../discovery/merge_types.hpp | 15 ++ .../src/discovery/layers/manifest_layer.cpp | 53 +++++++ .../src/discovery/layers/plugin_layer.cpp | 66 +++++++++ .../src/discovery/layers/runtime_layer.cpp | 130 ++++++++++++++++++ .../test/test_merge_pipeline.cpp | 130 ++++++++++++++++++ 9 files changed, 560 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/manifest_layer.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp create mode 100644 src/ros2_medkit_gateway/src/discovery/layers/manifest_layer.cpp create mode 100644 src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp create mode 100644 src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 62b932be..a41164e2 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -93,6 +93,9 @@ add_library(gateway_lib STATIC src/discovery/runtime_discovery.cpp src/discovery/hybrid_discovery.cpp src/discovery/merge_pipeline.cpp + src/discovery/layers/manifest_layer.cpp + src/discovery/layers/runtime_layer.cpp + src/discovery/layers/plugin_layer.cpp # Discovery models (with .cpp serialization) src/discovery/models/app.cpp src/discovery/models/function.cpp diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/manifest_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/manifest_layer.hpp new file mode 100644 index 00000000..97bc0ede --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/manifest_layer.hpp @@ -0,0 +1,49 @@ +// 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/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp" + +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief Discovery layer wrapping ManifestManager + * + * Default policies: IDENTITY=AUTH, HIERARCHY=AUTH, LIVE_DATA=ENRICH, + * STATUS=FALLBACK, METADATA=AUTH + */ +class ManifestLayer : public DiscoveryLayer { + public: + explicit ManifestLayer(ManifestManager * manifest_manager); + + std::string name() const override { + return "manifest"; + } + LayerOutput discover() override; + MergePolicy policy_for(FieldGroup group) const override; + + void set_policy(FieldGroup group, MergePolicy policy); + + private: + ManifestManager * manifest_manager_; + std::unordered_map policies_; +}; + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp new file mode 100644 index 00000000..f93ec5ec --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp @@ -0,0 +1,58 @@ +// 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/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/providers/introspection_provider.hpp" + +#include + +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief Discovery layer wrapping an IntrospectionProvider plugin + * + * Default policies: all ENRICHMENT except METADATA=AUTHORITATIVE + */ +class PluginLayer : public DiscoveryLayer { + public: + PluginLayer(std::string plugin_name, IntrospectionProvider * provider); + + std::string name() const override { + return name_; + } + LayerOutput discover() override; + MergePolicy policy_for(FieldGroup group) const override; + + void set_policy(FieldGroup group, MergePolicy policy); + + /// Get per-entity metadata from last discover() call + const std::unordered_map & get_metadata() const { + return last_metadata_; + } + + private: + std::string name_; + IntrospectionProvider * provider_; + std::unordered_map policies_; + std::unordered_map last_metadata_; +}; + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp new file mode 100644 index 00000000..746a03d4 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp @@ -0,0 +1,56 @@ +// 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/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/discovery/merge_types.hpp" +#include "ros2_medkit_gateway/discovery/runtime_discovery.hpp" + +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +/** + * @brief Discovery layer wrapping RuntimeDiscoveryStrategy + * + * Default policies: IDENTITY=FALLBACK, HIERARCHY=FALLBACK, LIVE_DATA=AUTH, + * STATUS=AUTH, METADATA=ENRICH + */ +class RuntimeLayer : public DiscoveryLayer { + public: + explicit RuntimeLayer(RuntimeDiscoveryStrategy * runtime_strategy); + + std::string name() const override { + return "runtime"; + } + LayerOutput discover() override; + MergePolicy policy_for(FieldGroup group) const override; + + void set_policy(FieldGroup group, MergePolicy policy); + void set_gap_fill_config(GapFillConfig config); + + /// Direct access to runtime services (for operation/data endpoints, not part of pipeline) + std::vector discover_services(); + std::vector discover_actions(); + + private: + RuntimeDiscoveryStrategy * runtime_strategy_; + std::unordered_map policies_; + GapFillConfig gap_fill_config_; +}; + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp index e27be588..1183af06 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp @@ -75,6 +75,21 @@ struct MergeReport { } }; +/** + * @brief Controls what heuristic (runtime) discovery is allowed to create + * + * When manifest is present, runtime entities fill gaps. This struct + * controls which entity types and namespaces are eligible for gap-fill. + */ +struct GapFillConfig { + bool allow_heuristic_areas{true}; + bool allow_heuristic_components{true}; + bool allow_heuristic_apps{true}; + bool allow_heuristic_functions{false}; + std::vector namespace_whitelist; + std::vector namespace_blacklist; +}; + } // namespace discovery } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/discovery/layers/manifest_layer.cpp b/src/ros2_medkit_gateway/src/discovery/layers/manifest_layer.cpp new file mode 100644 index 00000000..dbbd43c2 --- /dev/null +++ b/src/ros2_medkit_gateway/src/discovery/layers/manifest_layer.cpp @@ -0,0 +1,53 @@ +// 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/discovery/layers/manifest_layer.hpp" + +namespace ros2_medkit_gateway { +namespace discovery { + +ManifestLayer::ManifestLayer(ManifestManager * manifest_manager) : manifest_manager_(manifest_manager) { + policies_ = {{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::HIERARCHY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}, + {FieldGroup::STATUS, MergePolicy::FALLBACK}, + {FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}}; +} + +LayerOutput ManifestLayer::discover() { + LayerOutput output; + if (!manifest_manager_ || !manifest_manager_->is_manifest_active()) { + return output; + } + output.areas = manifest_manager_->get_areas(); + output.components = manifest_manager_->get_components(); + output.apps = manifest_manager_->get_apps(); + output.functions = manifest_manager_->get_functions(); + return output; +} + +MergePolicy ManifestLayer::policy_for(FieldGroup group) const { + auto it = policies_.find(group); + if (it != policies_.end()) { + return it->second; + } + return MergePolicy::ENRICHMENT; +} + +void ManifestLayer::set_policy(FieldGroup group, MergePolicy policy) { + policies_[group] = policy; +} + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp b/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp new file mode 100644 index 00000000..3ec8f8a2 --- /dev/null +++ b/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp @@ -0,0 +1,66 @@ +// 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/discovery/layers/plugin_layer.hpp" + +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +PluginLayer::PluginLayer(std::string plugin_name, IntrospectionProvider * provider) + : name_(std::move(plugin_name)), provider_(provider) { + policies_ = {{FieldGroup::IDENTITY, MergePolicy::ENRICHMENT}, + {FieldGroup::HIERARCHY, MergePolicy::ENRICHMENT}, + {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}, + {FieldGroup::STATUS, MergePolicy::ENRICHMENT}, + {FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}}; +} + +LayerOutput PluginLayer::discover() { + LayerOutput output; + if (!provider_) { + return output; + } + + // Build input (currently empty - pipeline will provide current entities in a future step) + IntrospectionInput input; + auto result = provider_->introspect(input); + + // Map new_entities to LayerOutput (no functions - plugins cannot create functions) + output.areas = std::move(result.new_entities.areas); + output.components = std::move(result.new_entities.components); + output.apps = std::move(result.new_entities.apps); + + // Store metadata and pass through LayerOutput for pipeline consumption + last_metadata_ = result.metadata; + output.entity_metadata = result.metadata; + + return output; +} + +MergePolicy PluginLayer::policy_for(FieldGroup group) const { + auto it = policies_.find(group); + if (it != policies_.end()) { + return it->second; + } + return MergePolicy::ENRICHMENT; +} + +void PluginLayer::set_policy(FieldGroup group, MergePolicy policy) { + policies_[group] = policy; +} + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp b/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp new file mode 100644 index 00000000..2842688c --- /dev/null +++ b/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp @@ -0,0 +1,130 @@ +// 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/discovery/layers/runtime_layer.hpp" + +#include +#include + +namespace ros2_medkit_gateway { +namespace discovery { + +namespace { + +// Check if a namespace is allowed by the gap-fill config +bool is_namespace_allowed(const std::string & ns, const GapFillConfig & config) { + // If whitelist is non-empty, namespace must match + if (!config.namespace_whitelist.empty()) { + bool found = + std::any_of(config.namespace_whitelist.begin(), config.namespace_whitelist.end(), [&ns](const std::string & w) { + return ns == w || ns.find(w + "/") == 0; + }); + if (!found) { + return false; + } + } + // Check blacklist + for (const auto & b : config.namespace_blacklist) { + if (ns == b || ns.find(b + "/") == 0) { + return false; + } + } + return true; +} + +// Filter entities with namespace_path by gap-fill config +template +void filter_by_namespace(std::vector & entities, const GapFillConfig & config) { + entities.erase(std::remove_if(entities.begin(), entities.end(), + [&config](const Entity & e) { + return !is_namespace_allowed(e.namespace_path, config); + }), + entities.end()); +} + +} // namespace + +RuntimeLayer::RuntimeLayer(RuntimeDiscoveryStrategy * runtime_strategy) : runtime_strategy_(runtime_strategy) { + policies_ = {{FieldGroup::IDENTITY, MergePolicy::FALLBACK}, + {FieldGroup::HIERARCHY, MergePolicy::FALLBACK}, + {FieldGroup::LIVE_DATA, MergePolicy::AUTHORITATIVE}, + {FieldGroup::STATUS, MergePolicy::AUTHORITATIVE}, + {FieldGroup::METADATA, MergePolicy::ENRICHMENT}}; +} + +LayerOutput RuntimeLayer::discover() { + LayerOutput output; + if (!runtime_strategy_) { + return output; + } + + if (gap_fill_config_.allow_heuristic_areas) { + output.areas = runtime_strategy_->discover_areas(); + filter_by_namespace(output.areas, gap_fill_config_); + } + + if (gap_fill_config_.allow_heuristic_components) { + output.components = runtime_strategy_->discover_components(); + + // Topic components are discovered separately and must be included + auto topic_components = runtime_strategy_->discover_topic_components(); + output.components.insert(output.components.end(), std::make_move_iterator(topic_components.begin()), + std::make_move_iterator(topic_components.end())); + + filter_by_namespace(output.components, gap_fill_config_); + } + + if (gap_fill_config_.allow_heuristic_apps) { + output.apps = runtime_strategy_->discover_apps(); + } + + if (gap_fill_config_.allow_heuristic_functions) { + output.functions = runtime_strategy_->discover_functions(); + } + + return output; +} + +MergePolicy RuntimeLayer::policy_for(FieldGroup group) const { + auto it = policies_.find(group); + if (it != policies_.end()) { + return it->second; + } + return MergePolicy::ENRICHMENT; +} + +void RuntimeLayer::set_policy(FieldGroup group, MergePolicy policy) { + policies_[group] = policy; +} + +void RuntimeLayer::set_gap_fill_config(GapFillConfig config) { + gap_fill_config_ = std::move(config); +} + +std::vector RuntimeLayer::discover_services() { + if (!runtime_strategy_) { + return {}; + } + return runtime_strategy_->discover_services(); +} + +std::vector RuntimeLayer::discover_actions() { + if (!runtime_strategy_) { + return {}; + } + return runtime_strategy_->discover_actions(); +} + +} // namespace discovery +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index 5150b926..224718b4 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -13,6 +13,9 @@ // limitations under the License. #include "ros2_medkit_gateway/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/discovery/layers/manifest_layer.hpp" +#include "ros2_medkit_gateway/discovery/layers/plugin_layer.hpp" +#include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" #include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" #include "ros2_medkit_gateway/discovery/merge_types.hpp" @@ -319,3 +322,130 @@ TEST_F(MergePipelineTest, CollectionFieldsUnionOnEnrichment) { // Union: calibrate (deduped) + reset = 2 services EXPECT_EQ(result.components[0].services.size(), 2u); } + +// --- ManifestLayer and RuntimeLayer tests --- + +TEST(ManifestLayerTest, DefaultPolicies) { + ManifestLayer layer(nullptr); + EXPECT_EQ(layer.name(), "manifest"); + EXPECT_EQ(layer.policy_for(FieldGroup::IDENTITY), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::HIERARCHY), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::LIVE_DATA), MergePolicy::ENRICHMENT); + EXPECT_EQ(layer.policy_for(FieldGroup::STATUS), MergePolicy::FALLBACK); + EXPECT_EQ(layer.policy_for(FieldGroup::METADATA), MergePolicy::AUTHORITATIVE); +} + +TEST(ManifestLayerTest, PolicyOverride) { + ManifestLayer layer(nullptr); + layer.set_policy(FieldGroup::LIVE_DATA, MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::LIVE_DATA), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::IDENTITY), MergePolicy::AUTHORITATIVE); +} + +TEST(ManifestLayerTest, DiscoverReturnsEmptyWhenNoManifest) { + ManifestLayer layer(nullptr); + auto output = layer.discover(); + EXPECT_TRUE(output.areas.empty()); + EXPECT_TRUE(output.components.empty()); + EXPECT_TRUE(output.apps.empty()); + EXPECT_TRUE(output.functions.empty()); +} + +TEST(RuntimeLayerTest, DefaultPolicies) { + RuntimeLayer layer(nullptr); + EXPECT_EQ(layer.name(), "runtime"); + EXPECT_EQ(layer.policy_for(FieldGroup::IDENTITY), MergePolicy::FALLBACK); + EXPECT_EQ(layer.policy_for(FieldGroup::HIERARCHY), MergePolicy::FALLBACK); + EXPECT_EQ(layer.policy_for(FieldGroup::LIVE_DATA), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::STATUS), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::METADATA), MergePolicy::ENRICHMENT); +} + +TEST(RuntimeLayerTest, DiscoverReturnsEmptyWhenNoStrategy) { + RuntimeLayer layer(nullptr); + auto output = layer.discover(); + EXPECT_TRUE(output.areas.empty()); + EXPECT_TRUE(output.components.empty()); +} + +// --- PluginLayer tests --- + +class MockIntrospectionProvider : public IntrospectionProvider { + public: + IntrospectionResult introspect(const IntrospectionInput & input) override { + (void)input; + return result_; + } + + IntrospectionResult result_; +}; + +TEST(PluginLayerTest, DefaultPolicies) { + auto provider = std::make_shared(); + PluginLayer layer("lidar_mapper", provider.get()); + EXPECT_EQ(layer.name(), "lidar_mapper"); + EXPECT_EQ(layer.policy_for(FieldGroup::IDENTITY), MergePolicy::ENRICHMENT); + EXPECT_EQ(layer.policy_for(FieldGroup::METADATA), MergePolicy::AUTHORITATIVE); +} + +TEST(PluginLayerTest, MapsNewEntitiesToLayerOutput) { + auto provider = std::make_shared(); + + Component new_comp; + new_comp.id = "lidar_unit"; + new_comp.name = "LiDAR Processing Unit"; + new_comp.source = "plugin"; + provider->result_.new_entities.components.push_back(new_comp); + + PluginLayer layer("lidar_mapper", provider.get()); + auto output = layer.discover(); + ASSERT_EQ(output.components.size(), 1u); + EXPECT_EQ(output.components[0].id, "lidar_unit"); +} + +TEST(PluginLayerTest, MetadataPassedThrough) { + auto provider = std::make_shared(); + provider->result_.metadata["engine"] = {{"x-medkit-temperature", 85}}; + + PluginLayer layer("sensor_plugin", provider.get()); + auto output = layer.discover(); + ASSERT_EQ(output.entity_metadata.size(), 1u); + EXPECT_TRUE(output.entity_metadata.count("engine")); + EXPECT_EQ(layer.get_metadata().at("engine")["x-medkit-temperature"], 85); +} + +TEST(PluginLayerTest, DiscoverReturnsEmptyWhenNoProvider) { + PluginLayer layer("broken_plugin", nullptr); + auto output = layer.discover(); + EXPECT_TRUE(output.areas.empty()); + EXPECT_TRUE(output.components.empty()); + EXPECT_TRUE(output.apps.empty()); + EXPECT_TRUE(output.entity_metadata.empty()); +} + +// --- GapFillConfig tests --- + +TEST(GapFillConfigTest, DefaultAllowsAll) { + GapFillConfig config; + EXPECT_TRUE(config.allow_heuristic_areas); + EXPECT_TRUE(config.allow_heuristic_components); + EXPECT_TRUE(config.allow_heuristic_apps); + EXPECT_FALSE(config.allow_heuristic_functions); +} + +TEST(GapFillConfigTest, NamespaceBlacklist) { + GapFillConfig config; + config.namespace_blacklist = {"/rosout", "/parameter_events"}; + EXPECT_EQ(config.namespace_blacklist.size(), 2u); + EXPECT_EQ(config.namespace_blacklist[0], "/rosout"); +} + +TEST(RuntimeLayerTest, GapFillFilterBlocksApps) { + GapFillConfig gap_fill; + gap_fill.allow_heuristic_apps = false; + + RuntimeLayer layer(nullptr); + layer.set_gap_fill_config(gap_fill); + auto output = layer.discover(); + EXPECT_TRUE(output.apps.empty()); +} From a85fc175e37797966cf10cf4ac73db63012b4d70 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 4 Mar 2026 18:35:06 +0100 Subject: [PATCH 3/6] feat(discovery): add post-merge RuntimeLinker and wire into HybridDiscoveryStrategy Implement RuntimeLinker that binds manifest apps (with ros_binding) to runtime-discovered ROS 2 nodes (with bound_fqn) after pipeline merge. Includes multi-match detection, binding conflict reporting, orphan node warnings, and deterministic namespace matching. Wire MergePipeline into HybridDiscoveryStrategy replacing the ad-hoc merge logic. --- src/ros2_medkit_gateway/design/index.rst | 1 - .../discovery/discovery_manager.hpp | 5 + .../discovery/hybrid_discovery.hpp | 88 ++--- .../discovery/manifest/runtime_linker.hpp | 36 +- .../discovery/merge_pipeline.hpp | 19 ++ .../discovery/runtime_discovery.hpp | 16 +- .../src/discovery/discovery_manager.cpp | 47 ++- .../src/discovery/hybrid_discovery.cpp | 127 ++----- .../src/discovery/manifest/runtime_linker.cpp | 203 +++++++----- .../src/discovery/merge_pipeline.cpp | 35 ++ .../src/discovery/runtime_discovery.cpp | 123 +++---- src/ros2_medkit_gateway/src/gateway_node.cpp | 51 ++- .../test/test_merge_pipeline.cpp | 73 +++++ .../test/test_runtime_linker.cpp | 310 +++++++++++++----- 14 files changed, 650 insertions(+), 484 deletions(-) diff --git a/src/ros2_medkit_gateway/design/index.rst b/src/ros2_medkit_gateway/design/index.rst index ea0ad6ae..7b9886d0 100644 --- a/src/ros2_medkit_gateway/design/index.rst +++ b/src/ros2_medkit_gateway/design/index.rst @@ -55,7 +55,6 @@ The following diagram shows the relationships between the main components of the } class RuntimeDiscoveryStrategy { - + discover_node_components(): vector + discover_synthetic_components(): vector + discover_topic_components(): vector - config_: RuntimeConfig 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 0b33f35a..6a76dca1 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 @@ -17,7 +17,9 @@ #include "ros2_medkit_gateway/discovery/discovery_enums.hpp" #include "ros2_medkit_gateway/discovery/discovery_strategy.hpp" #include "ros2_medkit_gateway/discovery/hybrid_discovery.hpp" +#include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" #include "ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp" +#include "ros2_medkit_gateway/discovery/merge_types.hpp" #include "ros2_medkit_gateway/discovery/models/app.hpp" #include "ros2_medkit_gateway/discovery/models/area.hpp" #include "ros2_medkit_gateway/discovery/models/common.hpp" @@ -349,6 +351,9 @@ class DiscoveryManager { std::unique_ptr manifest_manager_; std::unique_ptr hybrid_strategy_; + // Non-owning pointer to RuntimeLayer within the pipeline (for gap-fill config) + discovery::RuntimeLayer * runtime_layer_{nullptr}; + // Active strategy pointer (points to one of the above) discovery::DiscoveryStrategy * active_strategy_{nullptr}; }; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp index 1a42afc9..d8dcc83c 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp @@ -14,10 +14,9 @@ #pragma once +#include "ros2_medkit_gateway/discovery/discovery_layer.hpp" #include "ros2_medkit_gateway/discovery/discovery_strategy.hpp" -#include "ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp" -#include "ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp" -#include "ros2_medkit_gateway/discovery/runtime_discovery.hpp" +#include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" #include @@ -30,103 +29,60 @@ namespace ros2_medkit_gateway { namespace discovery { /** - * @brief Hybrid discovery combining manifest and runtime discovery + * @brief Hybrid discovery using a MergePipeline * - * Uses manifest as source of truth for entity IDs and hierarchy while - * linking to runtime ROS 2 nodes for live data (topics, services, actions). - * - * Behavior: - * - Areas: From manifest (runtime areas not exposed unless orphan policy allows) - * - Components: From manifest, enriched with runtime data if linked - * - Apps: From manifest, bound to runtime nodes via RuntimeLinker - * - Functions: From manifest only - * - * The hybrid strategy maintains a RuntimeLinker that binds manifest Apps - * to actual ROS 2 nodes discovered at runtime. + * Thin wrapper around MergePipeline that caches the merged result + * and exposes it through the DiscoveryStrategy interface. + * The pipeline orchestrates ManifestLayer, RuntimeLayer, and any + * PluginLayers with per-field-group merge policies. */ class HybridDiscoveryStrategy : public DiscoveryStrategy { public: /** * @brief Construct hybrid discovery strategy * @param node ROS 2 node for logging - * @param manifest_manager Manifest manager (must be loaded before use) - * @param runtime_strategy Runtime discovery strategy for ROS graph introspection + * @param pipeline Pre-configured merge pipeline */ - HybridDiscoveryStrategy(rclcpp::Node * node, ManifestManager * manifest_manager, - RuntimeDiscoveryStrategy * runtime_strategy); + HybridDiscoveryStrategy(rclcpp::Node * node, MergePipeline pipeline); - /** - * @brief Discover areas from manifest - * @return Areas defined in manifest - */ std::vector discover_areas() override; - - /** - * @brief Discover components from manifest, linked to runtime - * @return Components with runtime data if linked - */ std::vector discover_components() override; - - /** - * @brief Discover apps from manifest, linked to runtime nodes - * @return Apps with is_online and bound_fqn set - */ std::vector discover_apps() override; - - /** - * @brief Discover functions from manifest - * @return Functions defined in manifest - */ std::vector discover_functions() override; - /** - * @brief Get strategy name - * @return "hybrid" - */ std::string get_name() const override { return "hybrid"; } /** - * @brief Refresh runtime linking - * - * Call this after runtime discovery refresh to update app-node bindings. - * This will re-run the RuntimeLinker with fresh runtime component data. + * @brief Re-execute the pipeline and cache the result */ - void refresh_linking(); + void refresh(); /** - * @brief Get the last linking result - * @return Reference to last linking result + * @brief Get the last merge report */ - const LinkingResult & get_linking_result() const { - return linking_result_; - } + const MergeReport & get_merge_report() const; /** - * @brief Get orphan nodes from last linking - * @return Vector of node FQNs not bound to any manifest app + * @brief Get the last linking result */ - const std::vector & get_orphan_nodes() const { - return linking_result_.orphan_nodes; - } + const LinkingResult & get_linking_result() const; - private: /** - * @brief Perform initial linking on construction + * @brief Get orphan nodes from last linking */ - void perform_linking(); + std::vector get_orphan_nodes() const; /** - * @brief Log message at info level + * @brief Add a discovery layer to the pipeline (e.g., plugin layers) */ - void log_info(const std::string & msg) const; + void add_layer(std::unique_ptr layer); + private: rclcpp::Node * node_; - ManifestManager * manifest_manager_; - RuntimeDiscoveryStrategy * runtime_strategy_; - RuntimeLinker linker_; - LinkingResult linking_result_; + MergePipeline pipeline_; + MergeResult cached_result_; mutable std::mutex mutex_; }; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp index 93cf4492..1a9ed2a4 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp @@ -16,7 +16,6 @@ #include "ros2_medkit_gateway/discovery/manifest/manifest.hpp" #include "ros2_medkit_gateway/discovery/models/app.hpp" -#include "ros2_medkit_gateway/discovery/models/component.hpp" #include @@ -51,6 +50,15 @@ struct LinkingResult { /// Mapping from node FQN to App ID (reverse lookup) std::unordered_map node_to_app; + /// Number of times two apps competed for the same node + size_t binding_conflicts{0}; + + /// Number of wildcard bindings that matched >1 node + size_t wildcard_multi_match{0}; + + /// Human-readable diagnostic warnings + std::vector warnings; + /// Check if linking produced any errors based on policy bool has_errors(ManifestConfig::UnmanifestedNodePolicy policy) const { return policy == ManifestConfig::UnmanifestedNodePolicy::ERROR && !orphan_nodes.empty(); @@ -58,8 +66,12 @@ struct LinkingResult { /// Get statistics summary std::string summary() const { - return std::to_string(app_to_node.size()) + " linked, " + std::to_string(unlinked_app_ids.size()) + " unlinked, " + - std::to_string(orphan_nodes.size()) + " orphan nodes"; + std::string s = std::to_string(app_to_node.size()) + " linked, " + std::to_string(unlinked_app_ids.size()) + + " unlinked, " + std::to_string(orphan_nodes.size()) + " orphan nodes"; + if (binding_conflicts > 0) { + s += ", " + std::to_string(binding_conflicts) + " binding conflict(s)"; + } + return s; } }; @@ -86,14 +98,14 @@ class RuntimeLinker { explicit RuntimeLinker(rclcpp::Node * node = nullptr); /** - * @brief Link manifest apps to runtime nodes + * @brief Link manifest apps to runtime apps (nodes) * - * @param apps Apps from manifest - * @param runtime_components Components discovered from ROS graph + * @param manifest_apps Apps from manifest + * @param runtime_apps Apps discovered from ROS graph (each node is an App) * @param config Manifest config with orphan policy * @return LinkingResult with linked apps and orphan info */ - LinkingResult link(const std::vector & apps, const std::vector & runtime_components, + LinkingResult link(const std::vector & manifest_apps, const std::vector & runtime_apps, const ManifestConfig & config); /** @@ -139,17 +151,17 @@ class RuntimeLinker { /** * @brief Try to match by topic namespace * @param topic_namespace Topic namespace pattern from binding - * @param component Component with topic info + * @param runtime_app Runtime app with topic info * @return true if any topic matches the prefix */ - bool matches_topic_namespace(const std::string & topic_namespace, const Component & component) const; + bool matches_topic_namespace(const std::string & topic_namespace, const App & runtime_app) const; /** - * @brief Enrich app with runtime data from matched component + * @brief Enrich manifest app with runtime data from matched node * @param app App to enrich (modified in place) - * @param component Component with runtime data + * @param runtime_app Runtime app with live data */ - void enrich_app(App & app, const Component & component); + void enrich_app(App & app, const App & runtime_app); /** * @brief Log message at info level diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp index bd75b957..653d09aa 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp @@ -15,6 +15,8 @@ #pragma once #include "ros2_medkit_gateway/discovery/discovery_layer.hpp" +#include "ros2_medkit_gateway/discovery/manifest/manifest.hpp" +#include "ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp" #include "ros2_medkit_gateway/discovery/merge_types.hpp" #include @@ -67,6 +69,20 @@ class MergePipeline { return last_report_; } + /** + * @brief Set RuntimeLinker for post-merge app-to-node binding + * @param linker RuntimeLinker instance + * @param config ManifestConfig needed by RuntimeLinker::link() for unmanifested node policy + */ + void set_linker(std::unique_ptr linker, const ManifestConfig & config); + + /** + * @brief Get the last linking result + */ + const LinkingResult & get_linking_result() const { + return linking_result_; + } + private: /// Merge a vector of entities from multiple layers by ID template @@ -76,6 +92,9 @@ class MergePipeline { rclcpp::Logger logger_; std::vector> layers_; MergeReport last_report_; + std::unique_ptr linker_; + ManifestConfig manifest_config_; + LinkingResult linking_result_; }; } // namespace discovery diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp index 2cea3521..92631480 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp @@ -103,22 +103,10 @@ class RuntimeDiscoveryStrategy : public DiscoveryStrategy { // Runtime-specific methods (from current DiscoveryManager) // ========================================================================= - /** - * @brief Discover node-based components (individual ROS 2 nodes) - * - * This returns the traditional component discovery where each node - * becomes a Component. Used internally when synthetic components - * are not enabled or for building Apps. - * - * @return Vector of node-based components - */ - std::vector discover_node_components(); - /** * @brief Discover synthetic components (grouped by namespace) * - * Creates aggregated Components that group multiple nodes by namespace. - * Only used when create_synthetic_components is enabled. + * Groups runtime apps by namespace into aggregated Component entities. * * @return Vector of synthetic components */ @@ -203,7 +191,7 @@ class RuntimeDiscoveryStrategy : public DiscoveryStrategy { static bool is_internal_service(const std::string & service_path); /// Derive component ID for a node based on grouping strategy - std::string derive_component_id(const Component & node); + std::string derive_component_id(const std::string & node_id, const std::string & area); /// Apply naming pattern for synthetic component ID std::string apply_component_name_pattern(const std::string & area); diff --git a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp index f54d3809..a2837a7a 100644 --- a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp +++ b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp @@ -15,6 +15,10 @@ #include "ros2_medkit_gateway/discovery/discovery_manager.hpp" #include "ros2_medkit_gateway/discovery/hybrid_discovery.hpp" +#include "ros2_medkit_gateway/discovery/layers/manifest_layer.hpp" +#include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" +#include "ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp" +#include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" namespace ros2_medkit_gateway { @@ -75,12 +79,23 @@ void DiscoveryManager::create_strategy() { RCLCPP_INFO(node_->get_logger(), "Discovery mode: manifest_only"); break; - case DiscoveryMode::HYBRID: - hybrid_strategy_ = - std::make_unique(node_, manifest_manager_.get(), runtime_strategy_.get()); + case DiscoveryMode::HYBRID: { + discovery::MergePipeline pipeline(node_->get_logger()); + pipeline.add_layer(std::make_unique(manifest_manager_.get())); + + auto runtime_layer = std::make_unique(runtime_strategy_.get()); + runtime_layer_ = runtime_layer.get(); + pipeline.add_layer(std::move(runtime_layer)); + + // Set up RuntimeLinker for post-merge app-to-node binding + auto manifest_config = manifest_manager_ ? manifest_manager_->get_config() : discovery::ManifestConfig{}; + pipeline.set_linker(std::make_unique(node_), manifest_config); + + hybrid_strategy_ = std::make_unique(node_, std::move(pipeline)); active_strategy_ = hybrid_strategy_.get(); - RCLCPP_INFO(node_->get_logger(), "Discovery mode: hybrid"); + RCLCPP_INFO(node_->get_logger(), "Discovery mode: hybrid (merge pipeline)"); break; + } default: active_strategy_ = runtime_strategy_.get(); @@ -119,10 +134,11 @@ std::vector DiscoveryManager::discover_functions() { } std::optional DiscoveryManager::get_area(const std::string & id) { - if (manifest_manager_ && manifest_manager_->is_manifest_active()) { + // In MANIFEST_ONLY mode, use direct manifest lookup (O(1)) + if (config_.mode == DiscoveryMode::MANIFEST_ONLY && manifest_manager_ && manifest_manager_->is_manifest_active()) { return manifest_manager_->get_area(id); } - // Fallback to runtime lookup + // For HYBRID and RUNTIME modes, scan the strategy's output (cached for HYBRID) auto areas = discover_areas(); for (const auto & a : areas) { if (a.id == id) { @@ -133,7 +149,7 @@ std::optional DiscoveryManager::get_area(const std::string & id) { } std::optional DiscoveryManager::get_component(const std::string & id) { - if (manifest_manager_ && manifest_manager_->is_manifest_active()) { + if (config_.mode == DiscoveryMode::MANIFEST_ONLY && manifest_manager_ && manifest_manager_->is_manifest_active()) { return manifest_manager_->get_component(id); } auto components = discover_components(); @@ -146,10 +162,9 @@ std::optional DiscoveryManager::get_component(const std::string & id) } std::optional DiscoveryManager::get_app(const std::string & id) { - if (manifest_manager_ && manifest_manager_->is_manifest_active()) { + if (config_.mode == DiscoveryMode::MANIFEST_ONLY && manifest_manager_ && manifest_manager_->is_manifest_active()) { return manifest_manager_->get_app(id); } - // Check runtime apps auto apps = discover_apps(); for (const auto & app : apps) { if (app.id == id) { @@ -160,10 +175,16 @@ std::optional DiscoveryManager::get_app(const std::string & id) { } std::optional DiscoveryManager::get_function(const std::string & id) { - if (manifest_manager_ && manifest_manager_->is_manifest_active()) { + if (config_.mode == DiscoveryMode::MANIFEST_ONLY && manifest_manager_ && manifest_manager_->is_manifest_active()) { return manifest_manager_->get_function(id); } - return std::nullopt; // No functions in runtime-only mode + auto functions = discover_functions(); + for (const auto & f : functions) { + if (f.id == id) { + return f; + } + } + return std::nullopt; } std::vector DiscoveryManager::get_subareas(const std::string & area_id) { @@ -250,7 +271,7 @@ void DiscoveryManager::set_type_introspection(TypeIntrospection * introspection) void DiscoveryManager::refresh_topic_map() { runtime_strategy_->refresh_topic_map(); if (hybrid_strategy_) { - hybrid_strategy_->refresh_linking(); + hybrid_strategy_->refresh(); } } @@ -269,7 +290,7 @@ bool DiscoveryManager::reload_manifest() { } bool result = manifest_manager_->reload_manifest(); if (result && hybrid_strategy_) { - hybrid_strategy_->refresh_linking(); + hybrid_strategy_->refresh(); } return result; } diff --git a/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp b/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp index a4988e2e..8d9a0fb0 100644 --- a/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp +++ b/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp @@ -14,134 +14,63 @@ #include "ros2_medkit_gateway/discovery/hybrid_discovery.hpp" +#include + namespace ros2_medkit_gateway { namespace discovery { -HybridDiscoveryStrategy::HybridDiscoveryStrategy(rclcpp::Node * node, ManifestManager * manifest_manager, - RuntimeDiscoveryStrategy * runtime_strategy) - : node_(node), manifest_manager_(manifest_manager), runtime_strategy_(runtime_strategy), linker_(node) { - // Perform initial linking if manifest is loaded - if (manifest_manager_ && manifest_manager_->is_manifest_active()) { - perform_linking(); - } +HybridDiscoveryStrategy::HybridDiscoveryStrategy(rclcpp::Node * node, MergePipeline pipeline) + : node_(node), pipeline_(std::move(pipeline)) { + // Initial pipeline execution + refresh(); } std::vector HybridDiscoveryStrategy::discover_areas() { std::lock_guard lock(mutex_); - - if (!manifest_manager_ || !manifest_manager_->is_manifest_active()) { - // Fallback to runtime if no manifest - return runtime_strategy_->discover_areas(); - } - - return manifest_manager_->get_areas(); + return cached_result_.areas; } std::vector HybridDiscoveryStrategy::discover_components() { std::lock_guard lock(mutex_); - - if (!manifest_manager_ || !manifest_manager_->is_manifest_active()) { - return runtime_strategy_->discover_components(); - } - - // Get manifest components - std::vector result = manifest_manager_->get_components(); - - // Get runtime components for enrichment - auto runtime_components = runtime_strategy_->discover_components(); - - // Build a map of runtime components by FQN for quick lookup - std::unordered_map runtime_map; - for (const auto & comp : runtime_components) { - runtime_map[comp.fqn] = ∁ - } - - // Enrich manifest components with runtime data if they match - for (auto & comp : result) { - auto it = runtime_map.find(comp.fqn); - if (it != runtime_map.end()) { - // Copy runtime data - comp.topics = it->second->topics; - comp.services = it->second->services; - comp.actions = it->second->actions; - } - } - - // Handle orphan nodes based on policy - auto config = manifest_manager_->get_config(); - if (config.unmanifested_nodes == ManifestConfig::UnmanifestedNodePolicy::INCLUDE_AS_ORPHAN) { - // Add orphan nodes as components - std::set manifest_fqns; - for (const auto & comp : result) { - manifest_fqns.insert(comp.fqn); - } - - for (const auto & runtime_comp : runtime_components) { - if (manifest_fqns.find(runtime_comp.fqn) == manifest_fqns.end()) { - Component orphan = runtime_comp; - orphan.source = "orphan"; - result.push_back(orphan); - } - } - } - - return result; + return cached_result_.components; } std::vector HybridDiscoveryStrategy::discover_apps() { std::lock_guard lock(mutex_); - - if (!manifest_manager_ || !manifest_manager_->is_manifest_active()) { - // No apps in runtime-only mode - return {}; - } - - // Return linked apps from last linking result - return linking_result_.linked_apps; + return cached_result_.apps; } std::vector HybridDiscoveryStrategy::discover_functions() { std::lock_guard lock(mutex_); + return cached_result_.functions; +} - if (!manifest_manager_ || !manifest_manager_->is_manifest_active()) { - // No functions in runtime-only mode - return {}; +void HybridDiscoveryStrategy::refresh() { + std::lock_guard lock(mutex_); + cached_result_ = pipeline_.execute(); + if (node_) { + RCLCPP_INFO(node_->get_logger(), "Hybrid discovery refreshed: %zu entities", cached_result_.report.total_entities); } - - return manifest_manager_->get_functions(); } -void HybridDiscoveryStrategy::refresh_linking() { +const MergeReport & HybridDiscoveryStrategy::get_merge_report() const { std::lock_guard lock(mutex_); - perform_linking(); + return cached_result_.report; } -void HybridDiscoveryStrategy::perform_linking() { - if (!manifest_manager_ || !manifest_manager_->is_manifest_active()) { - log_info("Cannot perform linking: no active manifest"); - return; - } - - // Get manifest apps - auto apps = manifest_manager_->get_apps(); - - // Get runtime node components (raw nodes, not synthetic groupings) - // Runtime linking needs individual node FQNs to match against manifest bindings - auto runtime_components = runtime_strategy_->discover_node_components(); - - // Get config for orphan policy - auto config = manifest_manager_->get_config(); - - // Perform linking - linking_result_ = linker_.link(apps, runtime_components, config); +const LinkingResult & HybridDiscoveryStrategy::get_linking_result() const { + std::lock_guard lock(mutex_); + return pipeline_.get_linking_result(); +} - log_info("Hybrid linking complete: " + linking_result_.summary()); +std::vector HybridDiscoveryStrategy::get_orphan_nodes() const { + std::lock_guard lock(mutex_); + return pipeline_.get_linking_result().orphan_nodes; } -void HybridDiscoveryStrategy::log_info(const std::string & msg) const { - if (node_) { - RCLCPP_INFO(node_->get_logger(), "%s", msg.c_str()); - } +void HybridDiscoveryStrategy::add_layer(std::unique_ptr layer) { + std::lock_guard lock(mutex_); + pipeline_.add_layer(std::move(layer)); } } // namespace discovery diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/runtime_linker.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/runtime_linker.cpp index 4f30360a..65196747 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/runtime_linker.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/runtime_linker.cpp @@ -19,24 +19,64 @@ namespace ros2_medkit_gateway { namespace discovery { +namespace { + +/// Path-segment-boundary namespace match: "/nav" matches "/nav" and "/nav/sub" but NOT "/navigation" +bool namespace_matches(const std::string & actual_ns, const std::string & expected_ns) { + if (actual_ns == expected_ns) { + return true; + } + if (actual_ns.size() > expected_ns.size() && actual_ns.compare(0, expected_ns.size(), expected_ns) == 0 && + actual_ns[expected_ns.size()] == '/') { + return true; + } + return false; +} + +/// Extract the last path segment from a FQN (e.g., "/ns/node_name" -> "node_name") +std::string extract_node_name(const std::string & fqn) { + auto pos = fqn.rfind('/'); + if (pos == std::string::npos) { + return fqn; + } + return fqn.substr(pos + 1); +} + +/// Extract namespace from a FQN (e.g., "/ns/sub/node" -> "/ns/sub", "/node" -> "/") +std::string extract_namespace(const std::string & fqn) { + auto pos = fqn.rfind('/'); + if (pos == std::string::npos || pos == 0) { + return "/"; + } + return fqn.substr(0, pos); +} + +/// Path-segment-boundary topic match: "/state" matches "/state/x" but NOT "/statement/x" +bool topic_path_matches(const std::string & topic, const std::string & topic_namespace) { + if (topic == topic_namespace) { + return true; + } + if (topic.size() > topic_namespace.size() && topic.compare(0, topic_namespace.size(), topic_namespace) == 0 && + topic[topic_namespace.size()] == '/') { + return true; + } + return false; +} + +} // namespace + RuntimeLinker::RuntimeLinker(rclcpp::Node * node) : node_(node) { } -LinkingResult RuntimeLinker::link(const std::vector & apps, const std::vector & runtime_components, +LinkingResult RuntimeLinker::link(const std::vector & manifest_apps, const std::vector & runtime_apps, const ManifestConfig & config) { LinkingResult result; - // Build a map of node FQN -> Component for quick lookup - std::unordered_map fqn_to_component; - for (const auto & comp : runtime_components) { - fqn_to_component[comp.fqn] = ∁ - } - // Track which runtime nodes have been matched std::set matched_nodes; // Process each manifest app - for (const auto & manifest_app : apps) { + for (const auto & manifest_app : manifest_apps) { App linked_app = manifest_app; // Copy linked_app.is_online = false; @@ -56,41 +96,73 @@ LinkingResult RuntimeLinker::link(const std::vector & apps, const std::vect const auto & binding = manifest_app.ros_binding.value(); bool found = false; - // Try to find matching runtime node - for (const auto & comp : runtime_components) { - // Extract node name and namespace from component - std::string node_name = comp.id; - std::string node_ns = comp.namespace_path; + // Collect candidates, excluding already-bound nodes + std::vector candidates; - if (matches_binding(binding, comp.fqn, node_name, node_ns)) { - // Match found! - linked_app.bound_fqn = comp.fqn; - linked_app.is_online = true; - enrich_app(linked_app, comp); + for (const auto & rt_app : runtime_apps) { + if (!rt_app.bound_fqn.has_value()) { + continue; + } + const auto & fqn = rt_app.bound_fqn.value(); + if (matched_nodes.count(fqn)) { + continue; // Node already bound to another app + } + auto node_name = extract_node_name(fqn); + auto node_ns = extract_namespace(fqn); + if (matches_binding(binding, fqn, node_name, node_ns)) { + candidates.push_back(&rt_app); + } else if (!binding.topic_namespace.empty() && matches_topic_namespace(binding.topic_namespace, rt_app)) { + candidates.push_back(&rt_app); + } + } - result.app_to_node[manifest_app.id] = comp.fqn; - result.node_to_app[comp.fqn] = manifest_app.id; - matched_nodes.insert(comp.fqn); - found = true; + // Check if any candidates were excluded due to binding conflicts + if (candidates.empty()) { + // Check if there WOULD have been a match without exclusivity + for (const auto & rt_app : runtime_apps) { + if (!rt_app.bound_fqn.has_value()) { + continue; + } + const auto & fqn = rt_app.bound_fqn.value(); + if (matched_nodes.count(fqn)) { + auto node_name = extract_node_name(fqn); + auto node_ns = extract_namespace(fqn); + if (matches_binding(binding, fqn, node_name, node_ns) || + (!binding.topic_namespace.empty() && matches_topic_namespace(binding.topic_namespace, rt_app))) { + result.binding_conflicts++; + result.warnings.push_back("App '" + manifest_app.id + "' cannot bind to '" + fqn + + "' - already bound to app '" + result.node_to_app[fqn] + "'"); + log_warn(result.warnings.back()); + break; + } + } + } + } - log_debug("Linked app '" + manifest_app.id + "' to node '" + comp.fqn + "'"); - break; + // Sort candidates by FQN for deterministic selection + std::sort(candidates.begin(), candidates.end(), [](const App * a, const App * b) { + return a->bound_fqn.value() < b->bound_fqn.value(); + }); + + if (!candidates.empty()) { + if (candidates.size() > 1 && binding.namespace_pattern == "*") { + result.wildcard_multi_match++; + log_warn("App '" + manifest_app.id + "' wildcard matched " + std::to_string(candidates.size()) + + " nodes, selecting '" + candidates[0]->bound_fqn.value() + "'"); } - // Try topic namespace matching - if (!binding.topic_namespace.empty() && matches_topic_namespace(binding.topic_namespace, comp)) { - linked_app.bound_fqn = comp.fqn; - linked_app.is_online = true; - enrich_app(linked_app, comp); + const auto & match = *candidates[0]; + const auto & match_fqn = match.bound_fqn.value(); + linked_app.bound_fqn = match_fqn; + linked_app.is_online = true; + enrich_app(linked_app, match); - result.app_to_node[manifest_app.id] = comp.fqn; - result.node_to_app[comp.fqn] = manifest_app.id; - matched_nodes.insert(comp.fqn); - found = true; + result.app_to_node[manifest_app.id] = match_fqn; + result.node_to_app[match_fqn] = manifest_app.id; + matched_nodes.insert(match_fqn); + found = true; - log_debug("Linked app '" + manifest_app.id + "' to node '" + comp.fqn + "' (topic namespace)"); - break; - } + log_debug("Linked app '" + manifest_app.id + "' to node '" + match_fqn + "'"); } if (!found) { @@ -101,10 +173,10 @@ LinkingResult RuntimeLinker::link(const std::vector & apps, const std::vect result.linked_apps.push_back(linked_app); } - // Find orphan nodes (runtime nodes not matching any manifest app) - for (const auto & comp : runtime_components) { - if (matched_nodes.find(comp.fqn) == matched_nodes.end()) { - result.orphan_nodes.push_back(comp.fqn); + // Find orphan nodes (runtime apps not matching any manifest app) + for (const auto & rt_app : runtime_apps) { + if (rt_app.bound_fqn.has_value() && matched_nodes.find(rt_app.bound_fqn.value()) == matched_nodes.end()) { + result.orphan_nodes.push_back(rt_app.bound_fqn.value()); } } @@ -139,17 +211,14 @@ LinkingResult RuntimeLinker::link(const std::vector & apps, const std::vect } bool RuntimeLinker::matches_binding(const App::RosBinding & binding, const std::string & node_fqn, - const std::string & node_name, const std::string & node_namespace) const { - // Check node name match + const std::string & /*node_name*/, const std::string & node_namespace) const { + // Check node name match using last FQN segment (exact match only) if (binding.node_name.empty()) { return false; } - // Node name can be simple or with subpath (e.g., "local_costmap/local_costmap") - // Check if binding.node_name matches node_name or is contained in fqn - bool name_matches = (node_name == binding.node_name) || (node_fqn.find("/" + binding.node_name) != std::string::npos); - - if (!name_matches) { + std::string actual_name = extract_node_name(node_fqn); + if (actual_name != binding.node_name) { return false; } @@ -159,45 +228,33 @@ bool RuntimeLinker::matches_binding(const App::RosBinding & binding, const std:: return true; } - // Exact namespace match - std::string expected_ns = binding.namespace_pattern; - if (expected_ns.empty()) { - expected_ns = "/"; - } - // Normalize namespaces for comparison - std::string actual_ns = node_namespace; - if (actual_ns.empty()) { - actual_ns = "/"; - } + std::string expected_ns = binding.namespace_pattern.empty() ? "/" : binding.namespace_pattern; + std::string actual_ns = node_namespace.empty() ? "/" : node_namespace; - return actual_ns == expected_ns || actual_ns.find(expected_ns) == 0; // Prefix match + // Path-segment-boundary match + return namespace_matches(actual_ns, expected_ns); } -bool RuntimeLinker::matches_topic_namespace(const std::string & topic_namespace, const Component & component) const { - // Check if any topic starts with the given namespace - for (const auto & topic : component.topics.publishes) { - if (topic.find(topic_namespace) == 0) { +bool RuntimeLinker::matches_topic_namespace(const std::string & topic_ns, const App & runtime_app) const { + // Check if any topic matches with path-segment boundary + for (const auto & topic : runtime_app.topics.publishes) { + if (topic_path_matches(topic, topic_ns)) { return true; } } - for (const auto & topic : component.topics.subscribes) { - if (topic.find(topic_namespace) == 0) { + for (const auto & topic : runtime_app.topics.subscribes) { + if (topic_path_matches(topic, topic_ns)) { return true; } } return false; } -void RuntimeLinker::enrich_app(App & app, const Component & component) { - // Copy topics - app.topics = component.topics; - - // Copy services - app.services = component.services; - - // Copy actions - app.actions = component.actions; +void RuntimeLinker::enrich_app(App & app, const App & runtime_app) { + app.topics = runtime_app.topics; + app.services = runtime_app.services; + app.actions = runtime_app.actions; } bool RuntimeLinker::is_app_online(const std::string & app_id) const { diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index 27d6efa7..39ee70d3 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -286,6 +286,11 @@ void MergePipeline::add_layer(std::unique_ptr layer) { layers_.push_back(std::move(layer)); } +void MergePipeline::set_linker(std::unique_ptr linker, const ManifestConfig & config) { + linker_ = std::move(linker); + manifest_config_ = config; +} + template std::vector MergePipeline::merge_entities(const std::vector>> & layer_entities, @@ -355,8 +360,24 @@ MergeResult MergePipeline::execute() { std::vector>> app_layers; std::vector>> function_layers; + // RuntimeLinker needs runtime-only apps (nodes as Apps, not merged with + // manifest apps). Manifest apps lack bound_fqn until linked. + std::vector runtime_apps; + for (size_t i = 0; i < layers_.size(); ++i) { auto output = layers_[i]->discover(); + + // Collect gap-fill filtering stats and runtime apps from RuntimeLayer + auto * runtime_layer = dynamic_cast(layers_[i].get()); + if (runtime_layer) { + report.filtered_by_gap_fill += runtime_layer->last_filtered_count(); + } + // Save runtime apps for the linker (before they get moved into merge). + // Check both dynamic type and layer name for testability. + if (runtime_layer || layers_[i]->name() == "runtime") { + runtime_apps = output.apps; // copy before move + } + if (!output.areas.empty()) { area_layers.emplace_back(i, std::move(output.areas)); } @@ -396,6 +417,20 @@ MergeResult MergePipeline::execute() { check_ids(result.apps, "App"); check_ids(result.functions, "Function"); + // Post-merge linking: bind manifest apps to runtime nodes (Apps, not Components) + if (linker_) { + auto linking = linker_->link(result.apps, runtime_apps, manifest_config_); + + // Replace apps with linked versions (have is_online, bound_fqn set) + result.apps = std::move(linking.linked_apps); + + // Record linking stats in report + report.total_entities = + result.areas.size() + result.components.size() + result.apps.size() + result.functions.size(); + + linking_result_ = linker_->get_last_result(); + } + RCLCPP_INFO(logger_, "MergePipeline: %zu entities from %zu layers, %zu enriched, %zu conflicts", report.total_entities, report.layers.size(), report.enriched_count, report.conflict_count); for (const auto & conflict : report.conflicts) { diff --git a/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp b/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp index 4eed8daf..3ad8f7f0 100644 --- a/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp +++ b/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp @@ -101,19 +101,13 @@ std::vector RuntimeDiscoveryStrategy::discover_areas() { } std::vector RuntimeDiscoveryStrategy::discover_components() { - // If synthetic components are enabled, use grouping logic - if (config_.create_synthetic_components) { - return discover_synthetic_components(); - } - // Default: each node = 1 component (backward compatible) - return discover_node_components(); + return discover_synthetic_components(); } -std::vector RuntimeDiscoveryStrategy::discover_node_components() { - std::vector components; +std::vector RuntimeDiscoveryStrategy::discover_apps() { + std::vector apps; // Pre-build service info map for schema lookups - // Key: service full path, Value: ServiceInfo with type info std::unordered_map service_info_map; auto all_services = discover_services(); for (const auto & svc : all_services) { @@ -121,7 +115,6 @@ std::vector RuntimeDiscoveryStrategy::discover_node_components() { } // Pre-build action info map for schema lookups - // Key: action full path, Value: ActionInfo with type info std::unordered_map action_info_map; auto all_actions = discover_actions(); for (const auto & act : all_actions) { @@ -145,62 +138,57 @@ std::vector RuntimeDiscoveryStrategy::discover_node_components() { std::string fqn = (ns == "/") ? std::string("/").append(name) : std::string(ns).append("/").append(name); - // Skip duplicate nodes - ROS 2 RMW may report same node multiple times + // Skip duplicate nodes if (seen_fqns.count(fqn) > 0) { RCLCPP_DEBUG(node_->get_logger(), "Skipping duplicate node: %s", fqn.c_str()); continue; } seen_fqns.insert(fqn); - Component comp; - comp.id = name; - comp.namespace_path = ns; - comp.fqn = fqn; - comp.area = extract_area_from_namespace(ns); + App app; + app.id = name; + app.name = name; + app.source = "heuristic"; + app.is_online = true; + app.bound_fqn = fqn; - // Use ROS 2 introspection API to get services for this specific node - // This is more accurate than grouping by parent namespace + std::string area = extract_area_from_namespace(ns); + app.component_id = derive_component_id(name, area); + + // Introspect services and actions for this node try { auto node_services = node_->get_service_names_and_types_by_node(name, ns); for (const auto & [service_path, types] : node_services) { - // Skip internal ROS2 services (parameter services, action internals, etc.) if (is_internal_service(service_path)) { continue; } - - // Use pre-built service info map to get enriched info (with schema) auto it = service_info_map.find(service_path); if (it != service_info_map.end()) { - comp.services.push_back(it->second); + app.services.push_back(it->second); } else { - // Fallback: create basic ServiceInfo ServiceInfo info; info.full_path = service_path; info.name = extract_name_from_path(service_path); info.type = types.empty() ? "" : types[0]; - comp.services.push_back(info); + app.services.push_back(info); } } - // Detect actions owned by this node by checking for /_action/send_goal services + // Detect actions by checking for /_action/send_goal services for (const auto & [service_path, types] : node_services) { const std::string action_suffix = "/_action/send_goal"; if (service_path.length() > action_suffix.length() && service_path.compare(service_path.length() - action_suffix.length(), action_suffix.length(), action_suffix) == 0) { - // Extract action path by removing /_action/send_goal suffix std::string action_path = service_path.substr(0, service_path.length() - action_suffix.length()); - - // Use pre-built action info map to get enriched info (with schema) auto it = action_info_map.find(action_path); if (it != action_info_map.end()) { - comp.actions.push_back(it->second); + app.actions.push_back(it->second); } else { - // Fallback: create basic ActionInfo ActionInfo info; info.full_path = action_path; info.name = extract_name_from_path(action_path); - comp.actions.push_back(info); + app.actions.push_back(info); } } } @@ -211,41 +199,12 @@ std::vector RuntimeDiscoveryStrategy::discover_node_components() { // Populate topics from cached map if (topic_sampler_) { - auto it = cached_topic_map_.find(comp.fqn); + auto it = cached_topic_map_.find(fqn); if (it != cached_topic_map_.end()) { - comp.topics = it->second; + app.topics = it->second; } } - components.push_back(comp); - } - - return components; -} - -std::vector RuntimeDiscoveryStrategy::discover_apps() { - std::vector apps; - auto node_components = discover_node_components(); - - for (const auto & comp : node_components) { - // Skip topic-based components (source="topic") - if (comp.source == "topic") { - continue; - } - - App app; - app.id = comp.id; - app.name = comp.id; - app.component_id = derive_component_id(comp); - app.source = "heuristic"; - app.is_online = true; - app.bound_fqn = comp.fqn; - - // Copy resources from component - app.topics = comp.topics; - app.services = comp.services; - app.actions = comp.actions; - apps.push_back(app); } @@ -588,51 +547,49 @@ bool RuntimeDiscoveryStrategy::path_belongs_to_namespace(const std::string & pat } std::vector RuntimeDiscoveryStrategy::discover_synthetic_components() { - // Group nodes by their derived component ID (based on grouping strategy) - std::map> groups; - auto node_components = discover_node_components(); + // Group runtime apps by their component_id (already derived during discover_apps) + auto apps = discover_apps(); + std::map> groups; - for (const auto & node : node_components) { - std::string group_id = derive_component_id(node); - groups[group_id].push_back(node); + for (const auto & app : apps) { + groups[app.component_id].push_back(&app); } // Create synthetic components from groups std::vector result; - for (const auto & [group_id, nodes] : groups) { + for (const auto & [group_id, group_apps] : groups) { Component comp; comp.id = group_id; comp.source = "synthetic"; comp.type = "ComponentGroup"; - // Use first node's namespace and area as representative - if (!nodes.empty()) { - comp.namespace_path = nodes[0].namespace_path; - comp.area = nodes[0].area; + // Use first app's FQN to derive namespace and area + if (!group_apps.empty() && group_apps[0]->bound_fqn.has_value()) { + const auto & fqn = group_apps[0]->bound_fqn.value(); + auto pos = fqn.rfind('/'); + comp.namespace_path = (pos == std::string::npos || pos == 0) ? "/" : fqn.substr(0, pos); + comp.area = extract_area_from_namespace(comp.namespace_path); comp.fqn = "/" + group_id; } - // Note: Topics/services are NOT aggregated here - they stay with Apps - // This is intentional: synthetic components are just groupings + // Topics/services stay with Apps - synthetic components are just groupings - RCLCPP_DEBUG(node_->get_logger(), "Created synthetic component '%s' with %zu apps", group_id.c_str(), nodes.size()); + RCLCPP_DEBUG(node_->get_logger(), "Created synthetic component '%s' with %zu apps", group_id.c_str(), + group_apps.size()); result.push_back(comp); } - RCLCPP_DEBUG(node_->get_logger(), "Discovered %zu synthetic components from %zu nodes", result.size(), - node_components.size()); + RCLCPP_DEBUG(node_->get_logger(), "Discovered %zu synthetic components from %zu nodes", result.size(), apps.size()); return result; } -std::string RuntimeDiscoveryStrategy::derive_component_id(const Component & node) { +std::string RuntimeDiscoveryStrategy::derive_component_id(const std::string & node_id, const std::string & area) { switch (config_.grouping) { case ComponentGroupingStrategy::NAMESPACE: - // Group by area (first namespace segment) - return apply_component_name_pattern(node.area); + return apply_component_name_pattern(area); case ComponentGroupingStrategy::NONE: default: - // 1:1 mapping - each node is its own component - return node.id; + return node_id; } } diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 7bfb89f0..e9b8315a 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -543,45 +543,35 @@ void GatewayNode::refresh_cache() { RCLCPP_DEBUG(get_logger(), "Refreshing entity cache..."); try { - // Refresh topic map first (rebuilds the cached map) + // Refresh topic map first (rebuilds the cached map, triggers pipeline in hybrid mode) discovery_mgr_->refresh_topic_map(); - // Discover data outside the lock to minimize lock time + // Discover entities - in HYBRID mode the pipeline merges all sources, + // in RUNTIME_ONLY mode we manually merge node + topic components auto areas = discovery_mgr_->discover_areas(); - - // Discover node-based components (standard ROS 2 nodes) - auto node_components = discovery_mgr_->discover_components(); - - // Discover topic-based components (for systems like Isaac Sim that - // publish topics without creating proper ROS 2 nodes) - auto topic_components = discovery_mgr_->discover_topic_components(); - - // Discover apps (nodes as Apps when heuristic discovery is enabled) auto apps = discovery_mgr_->discover_apps(); - - // Discover functions (from manifest in manifest_only/hybrid mode) auto functions = discovery_mgr_->discover_functions(); - // Merge both component lists std::vector all_components; - all_components.reserve(node_components.size() + topic_components.size()); - all_components.insert(all_components.end(), node_components.begin(), node_components.end()); - all_components.insert(all_components.end(), topic_components.begin(), topic_components.end()); + if (discovery_mgr_->get_mode() == DiscoveryMode::HYBRID) { + // Pipeline already merges node and topic components + all_components = discovery_mgr_->discover_components(); + } else { + auto node_components = discovery_mgr_->discover_components(); + auto topic_components = discovery_mgr_->discover_topic_components(); + all_components.reserve(node_components.size() + topic_components.size()); + all_components.insert(all_components.end(), node_components.begin(), node_components.end()); + all_components.insert(all_components.end(), topic_components.begin(), topic_components.end()); + } - // Capture sizes before move for logging + // Capture sizes for logging const size_t area_count = areas.size(); - const size_t node_component_count = node_components.size(); - const size_t topic_component_count = topic_components.size(); + const size_t component_count = all_components.size(); const size_t app_count = apps.size(); const size_t function_count = functions.size(); - // Update ThreadSafeEntityCache (primary) with copies - // This provides O(1) lookups and proper thread safety - thread_safe_cache_.update_all(areas, // copy - all_components, // copy - apps, // copy - functions // copy - ); + // Update ThreadSafeEntityCache with copies + thread_safe_cache_.update_all(areas, all_components, apps, functions); // Update topic type cache (avoids expensive ROS graph queries on /data requests) if (data_access_mgr_) { @@ -597,11 +587,8 @@ void GatewayNode::refresh_cache() { thread_safe_cache_.update_topic_types(std::move(topic_types)); } - RCLCPP_DEBUG( - get_logger(), - "Cache refreshed: %zu areas, %zu components (%zu node-based, %zu topic-based), %zu apps, %zu functions", - area_count, node_component_count + topic_component_count, node_component_count, topic_component_count, - app_count, function_count); + RCLCPP_DEBUG(get_logger(), "Cache refreshed: %zu areas, %zu components, %zu apps, %zu functions", area_count, + component_count, app_count, function_count); } catch (const std::exception & e) { RCLCPP_ERROR(get_logger(), "Failed to refresh cache: %s", e.what()); } catch (...) { diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index 224718b4..b6f1501a 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -16,11 +16,14 @@ #include "ros2_medkit_gateway/discovery/layers/manifest_layer.hpp" #include "ros2_medkit_gateway/discovery/layers/plugin_layer.hpp" #include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" +#include "ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp" #include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" #include "ros2_medkit_gateway/discovery/merge_types.hpp" #include +#include + using namespace ros2_medkit_gateway::discovery; using namespace ros2_medkit_gateway; @@ -449,3 +452,73 @@ TEST(RuntimeLayerTest, GapFillFilterBlocksApps) { auto output = layer.discover(); EXPECT_TRUE(output.apps.empty()); } + +// --- Post-merge linking tests --- + +TEST_F(MergePipelineTest, PostMergeLinkingSetsAppOnlineStatus) { + // Manifest provides app with ros_binding + App manifest_app = make_app("controller_app", "nav_comp"); + manifest_app.source = "manifest"; + App::RosBinding binding; + binding.node_name = "controller"; + binding.namespace_pattern = "/nav"; + manifest_app.ros_binding = binding; + + // Runtime provides matching app (node) + App runtime_node; + runtime_node.id = "controller"; + runtime_node.name = "controller"; + runtime_node.source = "heuristic"; + runtime_node.is_online = true; + runtime_node.bound_fqn = "/nav/controller"; + + LayerOutput manifest_out; + manifest_out.apps.push_back(manifest_app); + + LayerOutput runtime_out; + runtime_out.apps.push_back(runtime_node); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::STATUS, MergePolicy::FALLBACK}})); + pipeline_.add_layer(std::make_unique( + "runtime", runtime_out, + std::unordered_map{{FieldGroup::STATUS, MergePolicy::AUTHORITATIVE}})); + + // Enable linking + ManifestConfig manifest_config; + pipeline_.set_linker(std::make_unique(nullptr), manifest_config); + + auto result = pipeline_.execute(); + // Both apps in result (different IDs). Linker matches controller_app's binding + // to runtime controller's bound_fqn. + auto it = std::find_if(result.apps.begin(), result.apps.end(), [](const App & a) { + return a.id == "controller_app"; + }); + ASSERT_NE(it, result.apps.end()); + EXPECT_TRUE(it->is_online); + EXPECT_EQ(it->bound_fqn, "/nav/controller"); +} + +TEST_F(MergePipelineTest, PostMergeLinkingReportsOrphanNodes) { + // Runtime provides an app (node) with no matching manifest app + App orphan_node; + orphan_node.id = "orphan_node"; + orphan_node.name = "orphan_node"; + orphan_node.source = "heuristic"; + orphan_node.is_online = true; + orphan_node.bound_fqn = "/test/orphan_node"; + + LayerOutput runtime_out; + runtime_out.apps.push_back(orphan_node); + + pipeline_.add_layer(std::make_unique("runtime", runtime_out)); + + ManifestConfig manifest_config; + pipeline_.set_linker(std::make_unique(nullptr), manifest_config); + + auto result = pipeline_.execute(); + auto & linking = pipeline_.get_linking_result(); + EXPECT_FALSE(linking.orphan_nodes.empty()); +} diff --git a/src/ros2_medkit_gateway/test/test_runtime_linker.cpp b/src/ros2_medkit_gateway/test/test_runtime_linker.cpp index 7a0a1d18..1aabbdb2 100644 --- a/src/ros2_medkit_gateway/test/test_runtime_linker.cpp +++ b/src/ros2_medkit_gateway/test/test_runtime_linker.cpp @@ -40,15 +40,29 @@ class RuntimeLinkerTest : public ::testing::Test { return app; } - // Helper to create a test Component (representing a runtime node) - Component create_component(const std::string & id, const std::string & ns = "/") { - Component comp; - comp.id = id; - comp.name = id; - comp.namespace_path = ns; - comp.fqn = ns == "/" ? "/" + id : ns + "/" + id; - comp.source = "node"; - return comp; + // Helper to create a test App with topic namespace binding + App create_topic_app(const std::string & id, const std::string & topic_ns) { + App app; + app.id = id; + app.name = id; + app.source = "manifest"; + + App::RosBinding binding; + binding.topic_namespace = topic_ns; + app.ros_binding = binding; + + return app; + } + + // Helper to create a runtime App (representing a discovered ROS 2 node) + App create_runtime_app(const std::string & id, const std::string & ns = "/") { + App app; + app.id = id; + app.name = id; + app.source = "heuristic"; + app.is_online = true; + app.bound_fqn = ns == "/" ? "/" + id : ns + "/" + id; + return app; } std::unique_ptr linker_; @@ -61,9 +75,9 @@ class RuntimeLinkerTest : public ::testing::Test { TEST_F(RuntimeLinkerTest, ExactMatch_NodeNameAndNamespace) { std::vector apps = {create_app("controller_app", "controller", "/nav")}; - std::vector components = {create_component("controller", "/nav")}; + std::vector runtime_apps = {create_runtime_app("controller", "/nav")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.linked_apps.size(), 1); EXPECT_TRUE(result.linked_apps[0].is_online); @@ -76,9 +90,9 @@ TEST_F(RuntimeLinkerTest, ExactMatch_NodeNameAndNamespace) { TEST_F(RuntimeLinkerTest, ExactMatch_RootNamespace) { std::vector apps = {create_app("my_app", "my_node", "/")}; - std::vector components = {create_component("my_node", "/")}; + std::vector runtime_apps = {create_runtime_app("my_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.linked_apps.size(), 1); EXPECT_TRUE(result.linked_apps[0].is_online); @@ -87,9 +101,9 @@ TEST_F(RuntimeLinkerTest, ExactMatch_RootNamespace) { TEST_F(RuntimeLinkerTest, NoMatch_DifferentNodeName) { std::vector apps = {create_app("app1", "controller", "/nav")}; - std::vector components = {create_component("planner", "/nav")}; + std::vector runtime_apps = {create_runtime_app("planner", "/nav")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.linked_apps.size(), 1); EXPECT_FALSE(result.linked_apps[0].is_online); @@ -100,9 +114,9 @@ TEST_F(RuntimeLinkerTest, NoMatch_DifferentNodeName) { TEST_F(RuntimeLinkerTest, NoMatch_DifferentNamespace) { std::vector apps = {create_app("app1", "controller", "/navigation")}; - std::vector components = {create_component("controller", "/planning")}; + std::vector runtime_apps = {create_runtime_app("controller", "/planning")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_FALSE(result.linked_apps[0].is_online); EXPECT_EQ(result.unlinked_app_ids.size(), 1); @@ -114,12 +128,12 @@ TEST_F(RuntimeLinkerTest, NoMatch_DifferentNamespace) { TEST_F(RuntimeLinkerTest, WildcardNamespace_MatchesAny) { std::vector apps = {create_app("app1", "controller", "*")}; - std::vector components = { - create_component("controller", "/ns1"), - create_component("controller", "/ns2"), + std::vector runtime_apps = { + create_runtime_app("controller", "/ns1"), + create_runtime_app("controller", "/ns2"), }; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); // Should match the first one found @@ -130,9 +144,9 @@ TEST_F(RuntimeLinkerTest, WildcardNamespace_MatchesAny) { TEST_F(RuntimeLinkerTest, WildcardNamespace_MatchesRootNamespace) { std::vector apps = {create_app("app1", "my_node", "*")}; - std::vector components = {create_component("my_node", "/")}; + std::vector runtime_apps = {create_runtime_app("my_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); EXPECT_EQ(result.linked_apps[0].bound_fqn, "/my_node"); @@ -140,9 +154,9 @@ TEST_F(RuntimeLinkerTest, WildcardNamespace_MatchesRootNamespace) { TEST_F(RuntimeLinkerTest, WildcardNamespace_MatchesNestedNamespace) { std::vector apps = {create_app("app1", "controller", "*")}; - std::vector components = {create_component("controller", "/robot/nav/local")}; + std::vector runtime_apps = {create_runtime_app("controller", "/robot/nav/local")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); EXPECT_EQ(result.linked_apps[0].bound_fqn, "/robot/nav/local/controller"); @@ -160,13 +174,13 @@ TEST_F(RuntimeLinkerTest, TopicNamespace_MatchesByPublisher) { binding.topic_namespace = "/sensor_data"; app.ros_binding = binding; - Component comp = create_component("sensor_driver", "/"); - comp.topics.publishes = {"/sensor_data/imu", "/sensor_data/gps"}; + App rt_app = create_runtime_app("sensor_driver", "/"); + rt_app.topics.publishes = {"/sensor_data/imu", "/sensor_data/gps"}; std::vector apps = {app}; - std::vector components = {comp}; + std::vector runtime_apps = {rt_app}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); EXPECT_EQ(result.linked_apps[0].bound_fqn, "/sensor_driver"); @@ -180,13 +194,13 @@ TEST_F(RuntimeLinkerTest, TopicNamespace_MatchesBySubscriber) { binding.topic_namespace = "/cmd"; app.ros_binding = binding; - Component comp = create_component("motor_driver", "/"); - comp.topics.subscribes = {"/cmd/velocity", "/cmd/position"}; + App rt_app = create_runtime_app("motor_driver", "/"); + rt_app.topics.subscribes = {"/cmd/velocity", "/cmd/position"}; std::vector apps = {app}; - std::vector components = {comp}; + std::vector runtime_apps = {rt_app}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); } @@ -199,13 +213,13 @@ TEST_F(RuntimeLinkerTest, TopicNamespace_NoMatch) { binding.topic_namespace = "/navigation"; app.ros_binding = binding; - Component comp = create_component("sensor_driver", "/"); - comp.topics.publishes = {"/sensor/imu"}; + App rt_app = create_runtime_app("sensor_driver", "/"); + rt_app.topics.publishes = {"/sensor/imu"}; std::vector apps = {app}; - std::vector components = {comp}; + std::vector runtime_apps = {rt_app}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_FALSE(result.linked_apps[0].is_online); } @@ -216,13 +230,13 @@ TEST_F(RuntimeLinkerTest, TopicNamespace_NoMatch) { TEST_F(RuntimeLinkerTest, OrphanNodes_DetectedCorrectly) { std::vector apps = {create_app("app1", "controller", "/nav")}; - std::vector components = { - create_component("controller", "/nav"), - create_component("planner", "/nav"), - create_component("mapper", "/map"), + std::vector runtime_apps = { + create_runtime_app("controller", "/nav"), + create_runtime_app("planner", "/nav"), + create_runtime_app("mapper", "/map"), }; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.orphan_nodes.size(), 2); EXPECT_TRUE(std::find(result.orphan_nodes.begin(), result.orphan_nodes.end(), "/nav/planner") != @@ -236,12 +250,12 @@ TEST_F(RuntimeLinkerTest, OrphanNodes_AllMatched) { create_app("app1", "node1", "*"), create_app("app2", "node2", "*"), }; - std::vector components = { - create_component("node1", "/"), - create_component("node2", "/"), + std::vector runtime_apps = { + create_runtime_app("node1", "/"), + create_runtime_app("node2", "/"), }; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.orphan_nodes.empty()); EXPECT_EQ(result.app_to_node.size(), 2); @@ -255,9 +269,9 @@ TEST_F(RuntimeLinkerTest, OrphanPolicy_Error_ReportsError) { config_.unmanifested_nodes = ManifestConfig::UnmanifestedNodePolicy::ERROR; std::vector apps = {}; - std::vector components = {create_component("orphan_node", "/")}; + std::vector runtime_apps = {create_runtime_app("orphan_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.has_errors(config_.unmanifested_nodes)); EXPECT_EQ(result.orphan_nodes.size(), 1); @@ -267,9 +281,9 @@ TEST_F(RuntimeLinkerTest, OrphanPolicy_Ignore_NoError) { config_.unmanifested_nodes = ManifestConfig::UnmanifestedNodePolicy::IGNORE; std::vector apps = {}; - std::vector components = {create_component("orphan_node", "/")}; + std::vector runtime_apps = {create_runtime_app("orphan_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_FALSE(result.has_errors(config_.unmanifested_nodes)); } @@ -281,13 +295,13 @@ TEST_F(RuntimeLinkerTest, OrphanPolicy_Ignore_NoError) { TEST_F(RuntimeLinkerTest, EnrichApp_CopiesTopics) { std::vector apps = {create_app("app1", "sensor", "*")}; - Component comp = create_component("sensor", "/"); - comp.topics.publishes = {"/sensor/data", "/sensor/status"}; - comp.topics.subscribes = {"/sensor/config"}; + App rt_app = create_runtime_app("sensor", "/"); + rt_app.topics.publishes = {"/sensor/data", "/sensor/status"}; + rt_app.topics.subscribes = {"/sensor/config"}; - std::vector components = {comp}; + std::vector runtime_apps = {rt_app}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); EXPECT_EQ(result.linked_apps[0].topics.publishes.size(), 2); @@ -297,13 +311,13 @@ TEST_F(RuntimeLinkerTest, EnrichApp_CopiesTopics) { TEST_F(RuntimeLinkerTest, EnrichApp_CopiesServices) { std::vector apps = {create_app("app1", "server", "*")}; - Component comp = create_component("server", "/"); - comp.services = {{"srv1", "/srv1", "std_srvs/srv/Trigger", std::nullopt}, - {"srv2", "/srv2", "std_srvs/srv/Empty", std::nullopt}}; + App rt_app = create_runtime_app("server", "/"); + rt_app.services = {{"srv1", "/srv1", "std_srvs/srv/Trigger", std::nullopt}, + {"srv2", "/srv2", "std_srvs/srv/Empty", std::nullopt}}; - std::vector components = {comp}; + std::vector runtime_apps = {rt_app}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); EXPECT_EQ(result.linked_apps[0].services.size(), 2); @@ -312,12 +326,12 @@ TEST_F(RuntimeLinkerTest, EnrichApp_CopiesServices) { TEST_F(RuntimeLinkerTest, EnrichApp_CopiesActions) { std::vector apps = {create_app("app1", "action_server", "*")}; - Component comp = create_component("action_server", "/"); - comp.actions = {{"nav", "/nav", "nav2_msgs/action/NavigateToPose", std::nullopt}}; + App rt_app = create_runtime_app("action_server", "/"); + rt_app.actions = {{"nav", "/nav", "nav2_msgs/action/NavigateToPose", std::nullopt}}; - std::vector components = {comp}; + std::vector runtime_apps = {rt_app}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(result.linked_apps[0].is_online); EXPECT_EQ(result.linked_apps[0].actions.size(), 1); @@ -334,9 +348,9 @@ TEST_F(RuntimeLinkerTest, ExternalApp_NotLinked) { app.external = true; std::vector apps = {app}; - std::vector components = {create_component("some_node", "/")}; + std::vector runtime_apps = {create_runtime_app("some_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.linked_apps.size(), 1); EXPECT_FALSE(result.linked_apps[0].is_online); @@ -355,9 +369,9 @@ TEST_F(RuntimeLinkerTest, NoBinding_GoesToUnlinked) { // No ros_binding set std::vector apps = {app}; - std::vector components = {create_component("some_node", "/")}; + std::vector runtime_apps = {create_runtime_app("some_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_FALSE(result.linked_apps[0].is_online); EXPECT_EQ(result.unlinked_app_ids.size(), 1); @@ -371,9 +385,9 @@ TEST_F(RuntimeLinkerTest, EmptyBinding_GoesToUnlinked) { app.ros_binding = App::RosBinding{}; // Empty binding std::vector apps = {app}; - std::vector components = {create_component("some_node", "/")}; + std::vector runtime_apps = {create_runtime_app("some_node", "/")}; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_FALSE(result.linked_apps[0].is_online); EXPECT_EQ(result.unlinked_app_ids.size(), 1); @@ -388,9 +402,9 @@ TEST_F(RuntimeLinkerTest, IsAppOnline_AfterLinking) { create_app("online_app", "online_node", "*"), create_app("offline_app", "missing_node", "*"), }; - std::vector components = {create_component("online_node", "/")}; + std::vector runtime_apps = {create_runtime_app("online_node", "/")}; - linker_->link(apps, components, config_); + linker_->link(apps, runtime_apps, config_); EXPECT_TRUE(linker_->is_app_online("online_app")); EXPECT_FALSE(linker_->is_app_online("offline_app")); @@ -399,9 +413,9 @@ TEST_F(RuntimeLinkerTest, IsAppOnline_AfterLinking) { TEST_F(RuntimeLinkerTest, GetBoundNode_ReturnsCorrectFqn) { std::vector apps = {create_app("app1", "my_node", "/ns")}; - std::vector components = {create_component("my_node", "/ns")}; + std::vector runtime_apps = {create_runtime_app("my_node", "/ns")}; - linker_->link(apps, components, config_); + linker_->link(apps, runtime_apps, config_); auto bound = linker_->get_bound_node("app1"); ASSERT_TRUE(bound.has_value()); @@ -412,9 +426,9 @@ TEST_F(RuntimeLinkerTest, GetBoundNode_ReturnsCorrectFqn) { TEST_F(RuntimeLinkerTest, GetAppForNode_ReturnsCorrectId) { std::vector apps = {create_app("app1", "my_node", "/ns")}; - std::vector components = {create_component("my_node", "/ns")}; + std::vector runtime_apps = {create_runtime_app("my_node", "/ns")}; - linker_->link(apps, components, config_); + linker_->link(apps, runtime_apps, config_); auto app_id = linker_->get_app_for_node("/ns/my_node"); ASSERT_TRUE(app_id.has_value()); @@ -433,13 +447,13 @@ TEST_F(RuntimeLinkerTest, MultipleApps_AllLinked) { create_app("app2", "node2", "*"), create_app("app3", "node3", "*"), }; - std::vector components = { - create_component("node1", "/"), - create_component("node2", "/"), - create_component("node3", "/"), + std::vector runtime_apps = { + create_runtime_app("node1", "/"), + create_runtime_app("node2", "/"), + create_runtime_app("node3", "/"), }; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.linked_apps.size(), 3); for (const auto & app : result.linked_apps) { @@ -456,12 +470,12 @@ TEST_F(RuntimeLinkerTest, MultipleApps_SomeUnlinked) { create_app("app3", "node3", "*"), create_app("app4", "missing2", "*"), }; - std::vector components = { - create_component("node1", "/"), - create_component("node3", "/"), + std::vector runtime_apps = { + create_runtime_app("node1", "/"), + create_runtime_app("node3", "/"), }; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); EXPECT_EQ(result.linked_apps.size(), 4); EXPECT_EQ(result.app_to_node.size(), 2); @@ -477,12 +491,12 @@ TEST_F(RuntimeLinkerTest, ResultSummary_FormatsCorrectly) { create_app("app1", "node1", "*"), create_app("app2", "missing", "*"), }; - std::vector components = { - create_component("node1", "/"), - create_component("orphan", "/"), + std::vector runtime_apps = { + create_runtime_app("node1", "/"), + create_runtime_app("orphan", "/"), }; - auto result = linker_->link(apps, components, config_); + auto result = linker_->link(apps, runtime_apps, config_); std::string summary = result.summary(); EXPECT_TRUE(summary.find("1 linked") != std::string::npos); @@ -492,14 +506,128 @@ TEST_F(RuntimeLinkerTest, ResultSummary_FormatsCorrectly) { TEST_F(RuntimeLinkerTest, GetLastResult_ReturnsLatest) { std::vector apps = {create_app("app1", "node1", "*")}; - std::vector components = {create_component("node1", "/")}; + std::vector runtime_apps = {create_runtime_app("node1", "/")}; - linker_->link(apps, components, config_); + linker_->link(apps, runtime_apps, config_); const auto & last = linker_->get_last_result(); EXPECT_EQ(last.app_to_node.size(), 1); } +// ============================================================================= +// Namespace Matching Determinism Tests (Task 16) +// ============================================================================= + +TEST_F(RuntimeLinkerTest, NamespaceMatch_RejectsStringPrefix) { + // "/nav" should NOT match node in "/navigation" namespace + std::vector apps = {create_app("nav_app", "navigator", "/nav")}; + std::vector runtime_apps = {create_runtime_app("navigator", "/navigation")}; + + auto result = linker_->link(apps, runtime_apps, config_); + EXPECT_FALSE(result.linked_apps[0].is_online); +} + +TEST_F(RuntimeLinkerTest, NamespaceMatch_AcceptsPathPrefix) { + // "/nav" SHOULD match node in "/nav/sub" namespace + std::vector apps = {create_app("nav_app", "planner", "/nav")}; + std::vector runtime_apps = {create_runtime_app("planner", "/nav/sub")}; + + auto result = linker_->link(apps, runtime_apps, config_); + EXPECT_TRUE(result.linked_apps[0].is_online); +} + +TEST_F(RuntimeLinkerTest, NodeName_ExactLastSegmentOnly) { + // Binding for "map" should NOT match node "map_server" (FQN contains "/map") + std::vector apps = {create_app("mapper", "map", "/")}; + std::vector runtime_apps = {create_runtime_app("map_server", "/")}; + + auto result = linker_->link(apps, runtime_apps, config_); + EXPECT_FALSE(result.linked_apps[0].is_online); +} + +TEST_F(RuntimeLinkerTest, Wildcard_DeterministicMultiMatch) { + // Wildcard: two nodes match by name, deterministic winner (alphabetical FQN) + std::vector apps = {create_app("ctrl_app", "controller", "*")}; + std::vector runtime_apps = { + create_runtime_app("controller", "/beta"), + create_runtime_app("controller", "/alpha"), + }; + + auto result = linker_->link(apps, runtime_apps, config_); + EXPECT_TRUE(result.linked_apps[0].is_online); + // Deterministic: alphabetically first FQN wins + EXPECT_EQ(result.linked_apps[0].bound_fqn, "/alpha/controller"); +} + +TEST_F(RuntimeLinkerTest, TopicNamespace_RejectsStringPrefix) { + // Topic namespace "/state" should NOT match topic "/statement/data" + auto app = create_topic_app("state_app", "/state"); + App rt_app = create_runtime_app("some_node", "/"); + rt_app.topics.publishes = {"/statement/data"}; + + auto result = linker_->link({app}, {rt_app}, config_); + EXPECT_FALSE(result.linked_apps[0].is_online); +} + +TEST_F(RuntimeLinkerTest, TopicNamespace_AcceptsPathPrefix) { + // Topic namespace "/state" SHOULD match topic "/state/machine" + auto app = create_topic_app("state_app", "/state"); + App rt_app = create_runtime_app("some_node", "/"); + rt_app.topics.publishes = {"/state/machine"}; + + auto result = linker_->link({app}, {rt_app}, config_); + EXPECT_TRUE(result.linked_apps[0].is_online); +} + +// ============================================================================= +// Multi-match and Binding Conflict Tests (Task 17) +// ============================================================================= + +TEST_F(RuntimeLinkerTest, TwoAppsCompeteForSameNode) { + // Two manifest apps bind to the same runtime node + std::vector apps = { + create_app("app1", "controller", "/nav"), + create_app("app2", "controller", "/nav"), + }; + std::vector runtime_apps = {create_runtime_app("controller", "/nav")}; + + auto result = linker_->link(apps, runtime_apps, config_); + + // First app wins (insertion order = priority) + EXPECT_TRUE(result.linked_apps[0].is_online); + EXPECT_EQ(result.linked_apps[0].bound_fqn, "/nav/controller"); + + // Second app is unlinked (node already taken) + EXPECT_FALSE(result.linked_apps[1].is_online); + EXPECT_EQ(result.unlinked_app_ids.size(), 1u); + + // Conflict reported + EXPECT_GE(result.binding_conflicts, 1u); +} + +TEST_F(RuntimeLinkerTest, LinkingReportSummaryIncludesConflicts) { + std::vector apps = { + create_app("app1", "controller", "/nav"), + create_app("app2", "controller", "/nav"), + }; + std::vector runtime_apps = {create_runtime_app("controller", "/nav")}; + + auto result = linker_->link(apps, runtime_apps, config_); + auto summary = result.summary(); + EXPECT_TRUE(summary.find("conflict") != std::string::npos); +} + +TEST_F(RuntimeLinkerTest, WildcardMultiMatchCounted) { + std::vector apps = {create_app("ctrl_app", "controller", "*")}; + std::vector runtime_apps = { + create_runtime_app("controller", "/alpha"), + create_runtime_app("controller", "/beta"), + }; + + auto result = linker_->link(apps, runtime_apps, config_); + EXPECT_EQ(result.wildcard_multi_match, 1u); +} + int main(int argc, char ** argv) { testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); From 8a582005de3b7b3ab2cc95605cc46d98064968f7 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 4 Mar 2026 18:51:33 +0100 Subject: [PATCH 4/6] feat(discovery): add plugin integration, config, and /health reporting Integrate IntrospectionProviders as pipeline layers with priority ordering and batch registration. Add YAML configuration for merge pipeline gap-fill. Expose MergeReport and LinkingResult in GET /health endpoint. Pass merged entity context (IntrospectionInput) to plugin layers before discover(). Add comprehensive documentation for merge pipeline architecture, policies, gap-fill options, and merge report diagnostics. --- docs/config/discovery-options.rst | 104 +++++ docs/tutorials/manifest-discovery.rst | 73 ++- docs/tutorials/plugin-system.rst | 10 +- src/ros2_medkit_fault_manager/CMakeLists.txt | 2 + src/ros2_medkit_gateway/CMakeLists.txt | 2 + .../config/gateway_params.yaml | 20 + .../design/architecture.puml | 29 +- src/ros2_medkit_gateway/design/index.rst | 74 ++- .../discovery/discovery_layer.hpp | 9 + .../discovery/discovery_manager.hpp | 49 +- .../discovery/hybrid_discovery.hpp | 8 +- .../discovery/layers/plugin_layer.hpp | 6 +- .../discovery/layers/runtime_layer.hpp | 6 + .../discovery/merge_pipeline.hpp | 10 +- .../discovery/merge_types.hpp | 36 +- .../discovery/runtime_discovery.hpp | 4 +- .../plugins/plugin_manager.hpp | 6 + .../providers/introspection_provider.hpp | 5 +- .../src/discovery/discovery_manager.cpp | 40 +- .../src/discovery/hybrid_discovery.cpp | 14 +- .../src/discovery/layers/plugin_layer.cpp | 43 +- .../src/discovery/layers/runtime_layer.cpp | 12 +- .../src/discovery/merge_pipeline.cpp | 77 +++- .../src/discovery/runtime_discovery.cpp | 6 +- src/ros2_medkit_gateway/src/gateway_node.cpp | 33 ++ .../src/http/handlers/health_handlers.cpp | 30 +- .../src/plugins/plugin_manager.cpp | 14 + .../test/test_auth_manager.cpp | 4 +- .../test/test_health_handlers.cpp | 8 + .../test/test_merge_pipeline.cpp | 429 +++++++++++++++++- .../test/test_runtime_linker.cpp | 29 ++ .../test_scenario_discovery_hybrid.test.py | 11 + 32 files changed, 1096 insertions(+), 107 deletions(-) diff --git a/docs/config/discovery-options.rst b/docs/config/discovery-options.rst index 4d5a307f..e4261216 100644 --- a/docs/config/discovery-options.rst +++ b/docs/config/discovery-options.rst @@ -87,6 +87,98 @@ The ``min_topics_for_component`` parameter (default: 1) sets the minimum number of topics required before creating a component. This can filter out namespaces with only a few stray topics. +Merge Pipeline Options (Hybrid Mode) +------------------------------------- + +When using ``hybrid`` mode, the merge pipeline controls how entities from +different discovery layers are combined. The ``merge_pipeline`` section +configures gap-fill behavior for runtime-discovered entities. + +Gap-Fill Configuration +^^^^^^^^^^^^^^^^^^^^^^ + +In hybrid mode, the manifest is the source of truth. Runtime (heuristic) discovery +fills gaps - entities that exist at runtime but aren't in the manifest. Gap-fill +controls restrict what the runtime layer is allowed to create: + +.. code-block:: yaml + + discovery: + merge_pipeline: + gap_fill: + allow_heuristic_areas: true + allow_heuristic_components: true + allow_heuristic_apps: true + allow_heuristic_functions: false + namespace_whitelist: [] + namespace_blacklist: [] + +.. list-table:: Gap-Fill Options + :header-rows: 1 + :widths: 35 15 50 + + * - Parameter + - Default + - Description + * - ``allow_heuristic_areas`` + - ``true`` + - Allow runtime layer to create Area entities not in the manifest + * - ``allow_heuristic_components`` + - ``true`` + - Allow runtime layer to create Component entities not in the manifest + * - ``allow_heuristic_apps`` + - ``true`` + - Allow runtime layer to create App entities not in the manifest + * - ``allow_heuristic_functions`` + - ``false`` + - Allow runtime layer to create Function entities (rarely useful at runtime) + * - ``namespace_whitelist`` + - ``[]`` + - If non-empty, only allow gap-fill from these ROS 2 namespaces (Areas and Components only) + * - ``namespace_blacklist`` + - ``[]`` + - Exclude gap-fill from these ROS 2 namespaces (Areas and Components only) + +When both whitelist and blacklist are empty, all namespaces are eligible for gap-fill. +If whitelist is non-empty, only listed namespaces pass. Blacklist is always applied. + +Namespace matching uses path-segment boundaries: ``/nav`` matches ``/nav`` and ``/nav/sub`` +but NOT ``/navigation``. Both lists only filter Areas and Components (which carry +``namespace_path``). Apps and Functions are not namespace-filtered. + + +Merge Policies +^^^^^^^^^^^^^^ + +Each discovery layer declares a ``MergePolicy`` per field group. When two layers +provide the same entity (matched by ID), policies determine which values win: + +.. list-table:: Merge Policies + :header-rows: 1 + :widths: 25 75 + + * - Policy + - Description + * - ``AUTHORITATIVE`` + - This layer's value wins over lower-priority layers. + If two AUTHORITATIVE layers conflict, a warning is logged and the + higher-priority (earlier) layer wins. + * - ``ENRICHMENT`` + - Fill empty fields from this layer. Non-empty target values are preserved. + Two ENRICHMENT layers merge additively (collections are unioned). + * - ``FALLBACK`` + - Use only if no other layer provides the value. Two FALLBACK layers + merge additively (first non-empty fills gaps). + +**Built-in layer policies:** + +- **ManifestLayer** (priority 1): IDENTITY, HIERARCHY, METADATA are AUTHORITATIVE. + LIVE_DATA is ENRICHMENT (runtime topics/services take precedence). + STATUS is FALLBACK (manifest cannot know online state). +- **RuntimeLayer** (priority 2): LIVE_DATA and STATUS are AUTHORITATIVE. + METADATA is ENRICHMENT. IDENTITY and HIERARCHY are FALLBACK. +- **PluginLayer** (priority 3+): All field groups ENRICHMENT + Configuration Example --------------------- @@ -112,6 +204,16 @@ Complete YAML configuration for runtime discovery: topic_only_policy: "create_component" min_topics_for_component: 2 # Require at least 2 topics + # Note: merge_pipeline settings only apply when mode is "hybrid" + merge_pipeline: + gap_fill: + allow_heuristic_areas: true + allow_heuristic_components: true + allow_heuristic_apps: true + allow_heuristic_functions: false + namespace_whitelist: [] + namespace_blacklist: ["/rosout", "/parameter_events"] + Command Line Override --------------------- @@ -128,3 +230,5 @@ See Also - :doc:`manifest-schema` - Manifest-based configuration - :doc:`/tutorials/heuristic-apps` - Tutorial on runtime discovery +- :doc:`/tutorials/manifest-discovery` - Hybrid mode tutorial +- :doc:`/tutorials/plugin-system` - Plugin layer integration diff --git a/docs/tutorials/manifest-discovery.rst b/docs/tutorials/manifest-discovery.rst index 021be5d3..54e32391 100644 --- a/docs/tutorials/manifest-discovery.rst +++ b/docs/tutorials/manifest-discovery.rst @@ -207,16 +207,67 @@ List functions: curl http://localhost:8080/api/v1/functions -Understanding Runtime Linking ------------------------------ +Understanding Hybrid Mode +------------------------- -In hybrid mode, manifest apps are automatically linked to running ROS 2 nodes. -The linking process: +In hybrid mode, discovery uses a **merge pipeline** that combines entities from +multiple discovery layers: -1. **Discovery**: Gateway discovers running ROS 2 nodes -2. **Matching**: For each manifest app, checks ``ros_binding`` configuration -3. **Linking**: If match found, copies runtime resources (topics, services, actions) -4. **Status**: Apps with matched nodes are marked ``is_online: true`` +1. **ManifestLayer** (highest priority) - entities from the YAML manifest +2. **RuntimeLayer** - entities discovered via ROS 2 graph introspection +3. **PluginLayers** (optional) - entities from gateway plugins + +The pipeline merges entities by ID. When the same entity appears in multiple layers, +per-field-group merge policies determine which values win. See +:doc:`/config/discovery-options` for details on merge policies and gap-fill configuration. + +After merging, the **RuntimeLinker** binds manifest apps to running ROS 2 nodes: + +1. **Discovery**: All layers produce entities +2. **Merging**: Pipeline merges entities by ID, applying field-group policies +3. **Linking**: For each manifest app, checks ``ros_binding`` configuration +4. **Binding**: If match found, copies runtime resources (topics, services, actions) +5. **Status**: Apps with matched nodes are marked ``is_online: true`` + +Merge Report +~~~~~~~~~~~~ + +After each pipeline execution, the gateway produces a ``MergeReport`` available +via the health endpoint (``GET /health``). The report includes: + +- Layer names and ordering +- Total entity count, enrichment count +- Conflict details (which layers disagreed on which field groups) +- Cross-type ID collision warnings +- Gap-fill filtering statistics + +In hybrid mode, the ``GET /health`` response includes full discovery diagnostics: + +.. code-block:: json + + { + "discovery": { + "mode": "hybrid", + "strategy": "hybrid", + "pipeline": { + "layers": ["manifest", "runtime"], + "total_entities": 12, + "enriched_count": 8, + "conflict_count": 0, + "id_collisions": 0 + }, + "linking": { + "linked_count": 5, + "orphan_count": 1, + "binding_conflicts": 0, + "warnings": ["Orphan node: /unmanifested_node"] + } + } + } + + +Runtime Linking +~~~~~~~~~~~~~~~ ROS Binding Configuration ~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -360,6 +411,12 @@ in the manifest. The ``config.unmanifested_nodes`` setting controls this: - ``error``: Fail startup if orphan nodes detected - ``include_as_orphan``: Include with ``source: "orphan"`` +.. note:: + In hybrid mode with gap-fill configuration (see :doc:`/config/discovery-options`), + namespace filtering controls which runtime entities enter the pipeline. + ``unmanifested_nodes`` controls how runtime nodes that passed gap-fill + but did not match any manifest app are handled by the RuntimeLinker. + Hot Reloading ------------- diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst index a4f811ce..99528d72 100644 --- a/docs/tutorials/plugin-system.rst +++ b/docs/tutorials/plugin-system.rst @@ -10,8 +10,9 @@ Overview Plugins implement the ``GatewayPlugin`` C++ base class plus one or more typed provider interfaces: - **UpdateProvider** - software update backend (CRUD, prepare/execute, automated, status) -- **IntrospectionProvider** *(preview)* - enriches discovered entities with platform-specific metadata. - This interface is defined and can be implemented, but is not yet wired into the discovery cycle. +- **IntrospectionProvider** - enriches discovered entities with platform-specific metadata + via the merge pipeline. In hybrid mode, each IntrospectionProvider is wrapped as a + ``PluginLayer`` and added to the pipeline with ENRICHMENT merge policy. A single plugin can implement multiple provider interfaces. For example, a "systemd" plugin could provide both introspection (discover systemd units) and updates (manage service restarts). @@ -300,7 +301,10 @@ Multiple Plugins Multiple plugins can be loaded simultaneously: - **UpdateProvider**: Only one plugin's UpdateProvider is used (first in config order) -- **IntrospectionProvider**: All plugins' results are merged *(preview - not yet wired)* +- **IntrospectionProvider**: All plugins are added as PluginLayers to the merge pipeline. + Each plugin's entities are merged with ENRICHMENT policy - they fill empty fields but + never override manifest or runtime values. Plugins are added after all built-in layers, + and the pipeline is refreshed once after all plugins are registered (batch registration). - **Custom routes**: All plugins can register endpoints (use unique path prefixes) Error Handling diff --git a/src/ros2_medkit_fault_manager/CMakeLists.txt b/src/ros2_medkit_fault_manager/CMakeLists.txt index ac1e7f88..196b6488 100644 --- a/src/ros2_medkit_fault_manager/CMakeLists.txt +++ b/src/ros2_medkit_fault_manager/CMakeLists.txt @@ -192,12 +192,14 @@ if(BUILD_TESTING) TARGET test_integration TIMEOUT 60 ) + set_tests_properties(test_integration PROPERTIES LABELS "integration") add_launch_test( test/test_rosbag_integration.test.py TARGET test_rosbag_integration TIMEOUT 90 ) + set_tests_properties(test_rosbag_integration PROPERTIES LABELS "integration") endif() ament_package() diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index a41164e2..7d74a15c 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -188,6 +188,7 @@ target_precompile_headers(gateway_lib PRIVATE ) +set_target_properties(gateway_lib PROPERTIES POSITION_INDEPENDENT_CODE ON) # Gateway node executable add_executable(gateway_node src/main.cpp) @@ -497,6 +498,7 @@ if(BUILD_TESTING) test_log_manager test_log_handlers test_merge_pipeline + test_runtime_linker ) foreach(_target ${_test_targets}) target_compile_options(${_target} PRIVATE --coverage -O0 -g) diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index f85cb0ff..5b7f73c8 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -172,6 +172,26 @@ ros2_medkit_gateway: # Default: 1 (create component for any namespace with topics) min_topics_for_component: 1 + # Merge pipeline configuration (only used in hybrid mode) + # Controls how manifest, runtime, and plugin layers merge entities + merge_pipeline: + # Gap-fill: what runtime discovery can create when manifest is incomplete + gap_fill: + allow_heuristic_areas: true + allow_heuristic_components: true + allow_heuristic_apps: true + allow_heuristic_functions: false + # namespace_blacklist: ["/rosout"] + # namespace_whitelist: [] + + # Per-layer policy overrides (NOT YET IMPLEMENTED - planned for future release) + # Defaults: manifest=AUTH for identity/hierarchy/metadata, runtime=AUTH for live_data/status + # layers: + # manifest: + # live_data: "authoritative" # override: manifest topics win + # runtime: + # identity: "authoritative" # override: trust runtime names + # Authentication Configuration (REQ_INTEROP_086, REQ_INTEROP_087) # JWT-based authentication with Role-Based Access Control (RBAC) auth: diff --git a/src/ros2_medkit_gateway/design/architecture.puml b/src/ros2_medkit_gateway/design/architecture.puml index 0d5a581b..3e8ebb62 100644 --- a/src/ros2_medkit_gateway/design/architecture.puml +++ b/src/ros2_medkit_gateway/design/architecture.puml @@ -73,8 +73,29 @@ package "ros2_medkit_gateway" { class EntityCache { + areas: vector + components: vector + + apps: vector + last_update: time_point } + + class MergePipeline { + + add_layer(): void + + execute(): MergeResult + + set_linker(): void + } + + interface DiscoveryLayer <> { + + name(): string + + discover(): LayerOutput + + policy_for(FieldGroup): MergePolicy + } + + class ManifestLayer + class RuntimeLayer + class PluginLayer + + class RuntimeLinker { + + link(): LinkingResult + } } package "External Libraries" { @@ -115,6 +136,13 @@ EntityCache o-right-> Component : contains many DiscoveryManager ..> Area : creates DiscoveryManager ..> Component : creates +' MergePipeline layer architecture +MergePipeline o--> DiscoveryLayer : ordered layers +MergePipeline --> RuntimeLinker : post-merge linking +ManifestLayer .up.|> DiscoveryLayer : implements +RuntimeLayer .up.|> DiscoveryLayer : implements +PluginLayer .up.|> DiscoveryLayer : implements + ' REST Server uses HTTP library RESTServer *--> HTTPLibServer : owns @@ -123,4 +151,3 @@ Area ..> JSON : serializes to Component ..> JSON : serializes to @enduml - diff --git a/src/ros2_medkit_gateway/design/index.rst b/src/ros2_medkit_gateway/design/index.rst index 7b9886d0..9003b7e4 100644 --- a/src/ros2_medkit_gateway/design/index.rst +++ b/src/ros2_medkit_gateway/design/index.rst @@ -66,8 +66,41 @@ The following diagram shows the relationships between the main components of the } class HybridDiscoveryStrategy { - - primary_: ManifestDiscoveryStrategy - - runtime_: RuntimeDiscoveryStrategy + - pipeline_: MergePipeline + + refresh(): void + + add_layer(): void + + get_merge_report(): MergeReport + } + + class MergePipeline { + + add_layer(): void + + execute(): MergeResult + + set_linker(): void + + get_last_report(): MergeReport + - merge_entities(): vector + } + + interface DiscoveryLayer <> { + + name(): string + + discover(): LayerOutput + + policy_for(FieldGroup): MergePolicy + } + + class ManifestLayer { + - manifest_mgr_: ManifestManager* + } + + class RuntimeLayer { + - strategy_: RuntimeDiscoveryStrategy* + - gap_fill_config_: GapFillConfig + } + + class PluginLayer { + - provider_: IntrospectionProvider* + } + + class RuntimeLinker { + + link(): LinkingResult } class OperationManager { @@ -228,8 +261,15 @@ The following diagram shows the relationships between the main components of the RuntimeDiscoveryStrategy .up.|> DiscoveryStrategy : implements ManifestDiscoveryStrategy .up.|> DiscoveryStrategy : implements HybridDiscoveryStrategy .up.|> DiscoveryStrategy : implements - HybridDiscoveryStrategy --> ManifestDiscoveryStrategy : delegates - HybridDiscoveryStrategy --> RuntimeDiscoveryStrategy : delegates + HybridDiscoveryStrategy *--> MergePipeline : owns + + ' MergePipeline layer architecture + MergePipeline o--> DiscoveryLayer : ordered layers + MergePipeline --> RuntimeLinker : post-merge linking + ManifestLayer .up.|> DiscoveryLayer : implements + RuntimeLayer .up.|> DiscoveryLayer : implements + PluginLayer .up.|> DiscoveryLayer : implements + RuntimeLayer --> RuntimeDiscoveryStrategy : delegates ' REST Server uses HTTP library RESTServer *--> HTTPLibServer : owns @@ -269,9 +309,29 @@ Main Components - **ManifestDiscoveryStrategy** - Static discovery from YAML manifest - Provides stable, semantic entity IDs - Supports offline detection of failed components - - **HybridDiscoveryStrategy** - Combines manifest + runtime - - Manifest defines structure, runtime links to live nodes - - Best for production systems requiring stability + live status + - **HybridDiscoveryStrategy** - Combines manifest + runtime via MergePipeline + - Delegates to ``MergePipeline`` which orchestrates ordered discovery layers + - Supports dynamic plugin layers added at runtime + - Thread-safe: mutex protects cached results, returns by value + + **Merge Pipeline:** + + The ``MergePipeline`` is the core engine for hybrid discovery. It: + + - Maintains an ordered list of ``DiscoveryLayer`` instances (first = highest priority) + - Executes all layers, collects entities by ID, and merges them per-field-group + - Each layer declares a ``MergePolicy`` per ``FieldGroup``: AUTHORITATIVE (wins), ENRICHMENT (fills empty), FALLBACK (last resort) + - Runs ``RuntimeLinker`` post-merge to bind manifest apps to live ROS 2 nodes + - Produces a ``MergeReport`` with conflict diagnostics, enrichment counts, and ID collision detection + + **Built-in Layers:** + + - ``ManifestLayer`` - Wraps ManifestManager; IDENTITY/HIERARCHY/METADATA are AUTHORITATIVE, + LIVE_DATA is ENRICHMENT (runtime wins for topics/services), STATUS is FALLBACK + - ``RuntimeLayer`` - Wraps RuntimeDiscoveryStrategy; LIVE_DATA/STATUS are AUTHORITATIVE, + METADATA is ENRICHMENT, IDENTITY/HIERARCHY are FALLBACK. + Supports ``GapFillConfig`` to control which heuristic entities are allowed when manifest is present + - ``PluginLayer`` - Wraps IntrospectionProvider; all fields ENRICHMENT (plugins enrich, they don't override) 3. **OperationManager** - Executes ROS 2 operations (services and actions) using native APIs - Calls ROS 2 services via ``rclcpp::GenericClient`` with native serialization diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp index 22d107eb..376a62f1 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/discovery_layer.hpp @@ -27,6 +27,10 @@ #include namespace ros2_medkit_gateway { + +// Forward declaration +struct IntrospectionInput; + namespace discovery { /** @@ -58,6 +62,11 @@ class DiscoveryLayer { /// Merge policy this layer uses for the given field group virtual MergePolicy policy_for(FieldGroup group) const = 0; + + /// Provide the current discovery context (entities from previous layers). + /// Called by MergePipeline before discover(). Default no-op. + virtual void set_discovery_context(const IntrospectionInput & /*context*/) { + } }; } // 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 6a76dca1..ce564a98 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 @@ -17,7 +17,6 @@ #include "ros2_medkit_gateway/discovery/discovery_enums.hpp" #include "ros2_medkit_gateway/discovery/discovery_strategy.hpp" #include "ros2_medkit_gateway/discovery/hybrid_discovery.hpp" -#include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" #include "ros2_medkit_gateway/discovery/manifest/manifest_manager.hpp" #include "ros2_medkit_gateway/discovery/merge_types.hpp" #include "ros2_medkit_gateway/discovery/models/app.hpp" @@ -38,6 +37,7 @@ namespace ros2_medkit_gateway { // Forward declarations class NativeTopicSampler; class TypeIntrospection; +class IntrospectionProvider; /** * @brief Configuration for discovery @@ -99,6 +99,13 @@ struct DiscoveryConfig { */ int min_topics_for_component{1}; } runtime; + + /** + * @brief Merge pipeline configuration (hybrid mode only) + */ + struct MergePipelineConfig { + discovery::GapFillConfig gap_fill; + } merge_pipeline; }; /** @@ -305,19 +312,28 @@ class DiscoveryManager { // ========================================================================= /** - * @brief Get the manifest manager - * @return Pointer to manifest manager (nullptr if not using manifest) + * @brief Add a plugin layer to the merge pipeline + * + * Wraps an IntrospectionProvider as a PluginLayer and adds it to + * the pipeline. Only works in HYBRID mode. + * + * @param plugin_name Name of the plugin + * @param provider Non-owning pointer to IntrospectionProvider */ - discovery::ManifestManager * get_manifest_manager(); + void add_plugin_layer(const std::string & plugin_name, IntrospectionProvider * provider); /** - * @brief Reload manifest from file - * - * Only works if a manifest was loaded during initialize(). + * @brief Re-execute the merge pipeline (hybrid mode only) * - * @return true if reload succeeded + * Call after adding plugin layers to trigger a single pipeline refresh. + */ + void refresh_pipeline(); + + /** + * @brief Get the manifest manager + * @return Pointer to manifest manager (nullptr if not using manifest) */ - bool reload_manifest(); + discovery::ManifestManager * get_manifest_manager(); // ========================================================================= // Status @@ -337,6 +353,18 @@ class DiscoveryManager { */ std::string get_strategy_name() const; + /** + * @brief Get the last merge pipeline report (hybrid mode only) + * @return MergeReport if in hybrid mode, nullopt otherwise + */ + std::optional get_merge_report() const; + + /** + * @brief Get the last linking result (hybrid mode only) + * @return LinkingResult if in hybrid mode, nullopt otherwise + */ + std::optional get_linking_result() const; + private: /** * @brief Create and activate the appropriate strategy @@ -351,9 +379,6 @@ class DiscoveryManager { std::unique_ptr manifest_manager_; std::unique_ptr hybrid_strategy_; - // Non-owning pointer to RuntimeLayer within the pipeline (for gap-fill config) - discovery::RuntimeLayer * runtime_layer_{nullptr}; - // Active strategy pointer (points to one of the above) discovery::DiscoveryStrategy * active_strategy_{nullptr}; }; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp index d8dcc83c..75c6db43 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/hybrid_discovery.hpp @@ -60,14 +60,14 @@ class HybridDiscoveryStrategy : public DiscoveryStrategy { void refresh(); /** - * @brief Get the last merge report + * @brief Get the last merge report (returned by value for thread safety) */ - const MergeReport & get_merge_report() const; + MergeReport get_merge_report() const; /** - * @brief Get the last linking result + * @brief Get the last linking result (returned by value for thread safety) */ - const LinkingResult & get_linking_result() const; + LinkingResult get_linking_result() const; /** * @brief Get orphan nodes from last linking diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp index f93ec5ec..dbf80a9d 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/plugin_layer.hpp @@ -18,6 +18,7 @@ #include "ros2_medkit_gateway/providers/introspection_provider.hpp" #include +#include #include #include @@ -28,7 +29,7 @@ namespace discovery { /** * @brief Discovery layer wrapping an IntrospectionProvider plugin * - * Default policies: all ENRICHMENT except METADATA=AUTHORITATIVE + * Default policies: all ENRICHMENT (plugins enrich, they don't override) */ class PluginLayer : public DiscoveryLayer { public: @@ -39,6 +40,7 @@ class PluginLayer : public DiscoveryLayer { } LayerOutput discover() override; MergePolicy policy_for(FieldGroup group) const override; + void set_discovery_context(const IntrospectionInput & context) override; void set_policy(FieldGroup group, MergePolicy policy); @@ -50,8 +52,10 @@ class PluginLayer : public DiscoveryLayer { private: std::string name_; IntrospectionProvider * provider_; + rclcpp::Logger logger_; std::unordered_map policies_; std::unordered_map last_metadata_; + IntrospectionInput discovery_context_; }; } // namespace discovery diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp index 746a03d4..f641384e 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/layers/runtime_layer.hpp @@ -42,6 +42,11 @@ class RuntimeLayer : public DiscoveryLayer { void set_policy(FieldGroup group, MergePolicy policy); void set_gap_fill_config(GapFillConfig config); + /// Number of entities filtered by gap-fill config in last discover() + size_t last_filtered_count() const { + return last_filtered_count_; + } + /// Direct access to runtime services (for operation/data endpoints, not part of pipeline) std::vector discover_services(); std::vector discover_actions(); @@ -50,6 +55,7 @@ class RuntimeLayer : public DiscoveryLayer { RuntimeDiscoveryStrategy * runtime_strategy_; std::unordered_map policies_; GapFillConfig gap_fill_config_; + size_t last_filtered_count_{0}; }; } // namespace discovery diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp index 653d09aa..23feb50d 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_pipeline.hpp @@ -63,9 +63,9 @@ class MergePipeline { MergeResult execute(); /** - * @brief Get the last merge report + * @brief Get the last merge report (returned by value for thread safety) */ - const MergeReport & get_last_report() const { + MergeReport get_last_report() const { return last_report_; } @@ -77,16 +77,16 @@ class MergePipeline { void set_linker(std::unique_ptr linker, const ManifestConfig & config); /** - * @brief Get the last linking result + * @brief Get the last linking result (returned by value for thread safety) */ - const LinkingResult & get_linking_result() const { + LinkingResult get_linking_result() const { return linking_result_; } private: /// Merge a vector of entities from multiple layers by ID template - std::vector merge_entities(const std::vector>> & layer_entities, + std::vector merge_entities(std::vector>> & layer_entities, MergeReport & report); rclcpp::Logger logger_; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp index 1183af06..b2e8d0d0 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/merge_types.hpp @@ -44,6 +44,22 @@ enum class FieldGroup { METADATA ///< source, x-medkit extensions, custom fields }; +inline const char * field_group_to_string(FieldGroup fg) { + switch (fg) { + case FieldGroup::IDENTITY: + return "IDENTITY"; + case FieldGroup::HIERARCHY: + return "HIERARCHY"; + case FieldGroup::LIVE_DATA: + return "LIVE_DATA"; + case FieldGroup::STATUS: + return "STATUS"; + case FieldGroup::METADATA: + return "METADATA"; + } + return "UNKNOWN"; +} + /** * @brief Record of a merge conflict between two layers */ @@ -65,13 +81,23 @@ struct MergeReport { size_t enriched_count{0}; size_t conflict_count{0}; size_t id_collision_count{0}; + size_t filtered_by_gap_fill{0}; nlohmann::json to_json() const { + nlohmann::json conflict_list = nlohmann::json::array(); + for (const auto & c : conflicts) { + conflict_list.push_back({{"entity_id", c.entity_id}, + {"field_group", field_group_to_string(c.field_group)}, + {"winning_layer", c.winning_layer}, + {"losing_layer", c.losing_layer}}); + } return {{"layers", layers}, {"total_entities", total_entities}, {"enriched_count", enriched_count}, {"conflict_count", conflict_count}, - {"id_collisions", id_collision_count}}; + {"conflicts", conflict_list}, + {"id_collisions", id_collision_count}, + {"filtered_by_gap_fill", filtered_by_gap_fill}}; } }; @@ -92,11 +118,3 @@ struct GapFillConfig { } // namespace discovery } // namespace ros2_medkit_gateway - -// Required: C++17 does not provide std::hash for enum class types -template <> -struct std::hash { - size_t operator()(ros2_medkit_gateway::discovery::FieldGroup fg) const noexcept { - return std::hash{}(static_cast(fg)); - } -}; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp index 92631480..eb4c2e1c 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/discovery/runtime_discovery.hpp @@ -107,10 +107,12 @@ class RuntimeDiscoveryStrategy : public DiscoveryStrategy { * @brief Discover synthetic components (grouped by namespace) * * Groups runtime apps by namespace into aggregated Component entities. + * Uses provided apps to avoid re-querying the ROS 2 graph. * + * @param apps Pre-discovered apps (from discover_apps()) * @return Vector of synthetic components */ - std::vector discover_synthetic_components(); + std::vector discover_synthetic_components(const std::vector & apps); /** * @brief Discover components from topic namespaces (topic-based discovery) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp index 6e0b4347..11d7f66b 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp @@ -127,6 +127,12 @@ class PluginManager { */ std::vector get_log_observers() const; + /** + * @brief Get all introspection providers with their plugin names + * @return (plugin_name, provider) pairs for all IntrospectionProvider plugins + */ + std::vector> get_named_introspection_providers() const; + // ---- Capability queries (used by discovery handlers) ---- /// Get plugin context (for capability queries from discovery handlers) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp index 95271e3e..13275432 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp @@ -49,8 +49,9 @@ struct NewEntities { * @brief Result returned by IntrospectionProvider::introspect() */ struct IntrospectionResult { - /// Per-entity metadata enrichment. Key = entity_id. - /// Values are deep-merged into the entity's x-medkit vendor extension. + /// Per-entity metadata for plugin-internal use. Key = entity_id. + /// Plugins serve this data as SOVD vendor extension resources + /// via register_routes() and register_capability(). std::unordered_map metadata; /// New entities discovered by this provider diff --git a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp index a2837a7a..178ebbe5 100644 --- a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp +++ b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp @@ -16,6 +16,7 @@ #include "ros2_medkit_gateway/discovery/hybrid_discovery.hpp" #include "ros2_medkit_gateway/discovery/layers/manifest_layer.hpp" +#include "ros2_medkit_gateway/discovery/layers/plugin_layer.hpp" #include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" #include "ros2_medkit_gateway/discovery/manifest/runtime_linker.hpp" #include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" @@ -84,7 +85,7 @@ void DiscoveryManager::create_strategy() { pipeline.add_layer(std::make_unique(manifest_manager_.get())); auto runtime_layer = std::make_unique(runtime_strategy_.get()); - runtime_layer_ = runtime_layer.get(); + runtime_layer->set_gap_fill_config(config_.merge_pipeline.gap_fill); pipeline.add_layer(std::move(runtime_layer)); // Set up RuntimeLinker for post-merge app-to-node binding @@ -279,20 +280,23 @@ bool DiscoveryManager::is_topic_map_ready() const { return runtime_strategy_->is_topic_map_ready(); } -discovery::ManifestManager * DiscoveryManager::get_manifest_manager() { - return manifest_manager_.get(); +void DiscoveryManager::add_plugin_layer(const std::string & plugin_name, IntrospectionProvider * provider) { + if (!hybrid_strategy_) { + RCLCPP_WARN(node_->get_logger(), "Cannot add plugin layer '%s': not in hybrid mode", plugin_name.c_str()); + return; + } + hybrid_strategy_->add_layer(std::make_unique(plugin_name, provider)); + RCLCPP_INFO(node_->get_logger(), "Added plugin layer '%s' to merge pipeline", plugin_name.c_str()); } -bool DiscoveryManager::reload_manifest() { - if (!manifest_manager_) { - RCLCPP_WARN(node_->get_logger(), "No manifest manager to reload"); - return false; - } - bool result = manifest_manager_->reload_manifest(); - if (result && hybrid_strategy_) { +void DiscoveryManager::refresh_pipeline() { + if (hybrid_strategy_) { hybrid_strategy_->refresh(); } - return result; +} + +discovery::ManifestManager * DiscoveryManager::get_manifest_manager() { + return manifest_manager_.get(); } std::string DiscoveryManager::get_strategy_name() const { @@ -302,4 +306,18 @@ std::string DiscoveryManager::get_strategy_name() const { return "unknown"; } +std::optional DiscoveryManager::get_merge_report() const { + if (hybrid_strategy_) { + return hybrid_strategy_->get_merge_report(); + } + return std::nullopt; +} + +std::optional DiscoveryManager::get_linking_result() const { + if (hybrid_strategy_) { + return hybrid_strategy_->get_linking_result(); + } + return std::nullopt; +} + } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp b/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp index 8d9a0fb0..bc61d935 100644 --- a/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp +++ b/src/ros2_medkit_gateway/src/discovery/hybrid_discovery.cpp @@ -46,19 +46,23 @@ std::vector HybridDiscoveryStrategy::discover_functions() { } void HybridDiscoveryStrategy::refresh() { - std::lock_guard lock(mutex_); - cached_result_ = pipeline_.execute(); + auto new_result = pipeline_.execute(); + size_t total = new_result.report.total_entities; + { + std::lock_guard lock(mutex_); + cached_result_ = std::move(new_result); + } if (node_) { - RCLCPP_INFO(node_->get_logger(), "Hybrid discovery refreshed: %zu entities", cached_result_.report.total_entities); + RCLCPP_INFO(node_->get_logger(), "Hybrid discovery refreshed: %zu entities", total); } } -const MergeReport & HybridDiscoveryStrategy::get_merge_report() const { +MergeReport HybridDiscoveryStrategy::get_merge_report() const { std::lock_guard lock(mutex_); return cached_result_.report; } -const LinkingResult & HybridDiscoveryStrategy::get_linking_result() const { +LinkingResult HybridDiscoveryStrategy::get_linking_result() const { std::lock_guard lock(mutex_); return pipeline_.get_linking_result(); } diff --git a/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp b/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp index 3ec8f8a2..5d91b954 100644 --- a/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp +++ b/src/ros2_medkit_gateway/src/discovery/layers/plugin_layer.cpp @@ -14,18 +14,44 @@ #include "ros2_medkit_gateway/discovery/layers/plugin_layer.hpp" +#include #include namespace ros2_medkit_gateway { namespace discovery { +namespace { + +bool is_valid_entity_id(const std::string & id) { + if (id.empty() || id.size() > 256) { + return false; + } + return std::all_of(id.begin(), id.end(), [](char c) { + return std::isalnum(static_cast(c)) || c == '_' || c == '-'; + }); +} + +template +void validate_entities(std::vector & entities, const std::string & layer_name, const rclcpp::Logger & logger) { + auto it = std::remove_if(entities.begin(), entities.end(), [&](const T & e) { + if (!is_valid_entity_id(e.id)) { + RCLCPP_WARN(logger, "Plugin '%s': dropping entity with invalid ID '%s'", layer_name.c_str(), e.id.c_str()); + return true; + } + return false; + }); + entities.erase(it, entities.end()); +} + +} // namespace + PluginLayer::PluginLayer(std::string plugin_name, IntrospectionProvider * provider) - : name_(std::move(plugin_name)), provider_(provider) { + : name_(std::move(plugin_name)), provider_(provider), logger_(rclcpp::get_logger("plugin_layer." + name_)) { policies_ = {{FieldGroup::IDENTITY, MergePolicy::ENRICHMENT}, {FieldGroup::HIERARCHY, MergePolicy::ENRICHMENT}, {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}, {FieldGroup::STATUS, MergePolicy::ENRICHMENT}, - {FieldGroup::METADATA, MergePolicy::AUTHORITATIVE}}; + {FieldGroup::METADATA, MergePolicy::ENRICHMENT}}; } LayerOutput PluginLayer::discover() { @@ -34,15 +60,18 @@ LayerOutput PluginLayer::discover() { return output; } - // Build input (currently empty - pipeline will provide current entities in a future step) - IntrospectionInput input; - auto result = provider_->introspect(input); + auto result = provider_->introspect(discovery_context_); // Map new_entities to LayerOutput (no functions - plugins cannot create functions) output.areas = std::move(result.new_entities.areas); output.components = std::move(result.new_entities.components); output.apps = std::move(result.new_entities.apps); + // Validate entity IDs from plugin + validate_entities(output.areas, name_, logger_); + validate_entities(output.components, name_, logger_); + validate_entities(output.apps, name_, logger_); + // Store metadata and pass through LayerOutput for pipeline consumption last_metadata_ = result.metadata; output.entity_metadata = result.metadata; @@ -58,6 +87,10 @@ MergePolicy PluginLayer::policy_for(FieldGroup group) const { return MergePolicy::ENRICHMENT; } +void PluginLayer::set_discovery_context(const IntrospectionInput & context) { + discovery_context_ = context; +} + void PluginLayer::set_policy(FieldGroup group, MergePolicy policy) { policies_[group] = policy; } diff --git a/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp b/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp index 2842688c..925fef46 100644 --- a/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp +++ b/src/ros2_medkit_gateway/src/discovery/layers/runtime_layer.cpp @@ -15,6 +15,7 @@ #include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" #include +#include #include namespace ros2_medkit_gateway { @@ -43,14 +44,16 @@ bool is_namespace_allowed(const std::string & ns, const GapFillConfig & config) return true; } -// Filter entities with namespace_path by gap-fill config +// Filter entities with namespace_path by gap-fill config, returns count of removed entities template -void filter_by_namespace(std::vector & entities, const GapFillConfig & config) { +size_t filter_by_namespace(std::vector & entities, const GapFillConfig & config) { + size_t before = entities.size(); entities.erase(std::remove_if(entities.begin(), entities.end(), [&config](const Entity & e) { return !is_namespace_allowed(e.namespace_path, config); }), entities.end()); + return before - entities.size(); } } // namespace @@ -65,13 +68,14 @@ RuntimeLayer::RuntimeLayer(RuntimeDiscoveryStrategy * runtime_strategy) : runtim LayerOutput RuntimeLayer::discover() { LayerOutput output; + last_filtered_count_ = 0; if (!runtime_strategy_) { return output; } if (gap_fill_config_.allow_heuristic_areas) { output.areas = runtime_strategy_->discover_areas(); - filter_by_namespace(output.areas, gap_fill_config_); + last_filtered_count_ += filter_by_namespace(output.areas, gap_fill_config_); } if (gap_fill_config_.allow_heuristic_components) { @@ -82,7 +86,7 @@ LayerOutput RuntimeLayer::discover() { output.components.insert(output.components.end(), std::make_move_iterator(topic_components.begin()), std::make_move_iterator(topic_components.end())); - filter_by_namespace(output.components, gap_fill_config_); + last_filtered_count_ += filter_by_namespace(output.components, gap_fill_config_); } if (gap_fill_config_.allow_heuristic_apps) { diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index 39ee70d3..74dea0d1 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -14,6 +14,9 @@ #include "ros2_medkit_gateway/discovery/merge_pipeline.hpp" +#include "ros2_medkit_gateway/discovery/layers/runtime_layer.hpp" +#include "ros2_medkit_gateway/providers/introspection_provider.hpp" + #include #include #include @@ -59,7 +62,7 @@ MergeResolution resolve_policies(MergePolicy target_policy, MergePolicy source_p } else if (target_policy == MergePolicy::ENRICHMENT) { return {MergeWinner::BOTH, MergeWinner::BOTH, false}; // mutual enrichment } else { - return {MergeWinner::TARGET, MergeWinner::TARGET, false}; // FALLBACK vs FALLBACK + return {MergeWinner::BOTH, MergeWinner::BOTH, false}; // FALLBACK vs FALLBACK: fill gaps } } } @@ -292,9 +295,8 @@ void MergePipeline::set_linker(std::unique_ptr linker, const Mani } template -std::vector -MergePipeline::merge_entities(const std::vector>> & layer_entities, - MergeReport & report) { +std::vector MergePipeline::merge_entities(std::vector>> & layer_entities, + MergeReport & report) { // Collect all entities by ID with their layer index struct LayerEntity { size_t layer_idx; @@ -303,12 +305,13 @@ MergePipeline::merge_entities(const std::vector> by_id; std::vector insertion_order; - for (const auto & [layer_idx, entities] : layer_entities) { - for (const auto & entity : entities) { + for (auto & [layer_idx, entities] : layer_entities) { + for (auto & entity : entities) { if (by_id.find(entity.id) == by_id.end()) { insertion_order.push_back(entity.id); } - by_id[entity.id].push_back({layer_idx, entity}); + auto id = entity.id; // copy id before move + by_id[id].push_back({layer_idx, std::move(entity)}); } } @@ -365,17 +368,42 @@ MergeResult MergePipeline::execute() { std::vector runtime_apps; for (size_t i = 0; i < layers_.size(); ++i) { - auto output = layers_[i]->discover(); + // Build discovery context from entities collected so far (for plugin layers) + IntrospectionInput context; + for (const auto & [idx, entities] : area_layers) { + context.areas.insert(context.areas.end(), entities.begin(), entities.end()); + } + for (const auto & [idx, entities] : component_layers) { + context.components.insert(context.components.end(), entities.begin(), entities.end()); + } + for (const auto & [idx, entities] : app_layers) { + context.apps.insert(context.apps.end(), entities.begin(), entities.end()); + } + for (const auto & [idx, entities] : function_layers) { + context.functions.insert(context.functions.end(), entities.begin(), entities.end()); + } + layers_[i]->set_discovery_context(context); + + LayerOutput output; + try { + output = layers_[i]->discover(); + } catch (const std::exception & e) { + RCLCPP_ERROR(logger_, "Layer '%s' threw exception during discover(): %s", layers_[i]->name().c_str(), e.what()); + continue; + } catch (...) { + RCLCPP_ERROR(logger_, "Layer '%s' threw unknown exception during discover()", layers_[i]->name().c_str()); + continue; + } - // Collect gap-fill filtering stats and runtime apps from RuntimeLayer + // Collect gap-fill filtering stats from RuntimeLayer auto * runtime_layer = dynamic_cast(layers_[i].get()); if (runtime_layer) { report.filtered_by_gap_fill += runtime_layer->last_filtered_count(); } - // Save runtime apps for the linker (before they get moved into merge). - // Check both dynamic type and layer name for testability. + // Save runtime apps for the linker BEFORE they are moved into app_layers below. + // Check both dynamic type and layer name for testability with TestLayer. if (runtime_layer || layers_[i]->name() == "runtime") { - runtime_apps = output.apps; // copy before move + runtime_apps = output.apps; } if (!output.areas.empty()) { @@ -390,6 +418,8 @@ MergeResult MergePipeline::execute() { if (!output.functions.empty()) { function_layers.emplace_back(i, std::move(output.functions)); } + // entity_metadata is not consumed here - plugins serve their metadata + // as SOVD vendor extension resources via register_routes() and register_capability(). } MergeResult result; @@ -433,25 +463,32 @@ MergeResult MergePipeline::execute() { RCLCPP_INFO(logger_, "MergePipeline: %zu entities from %zu layers, %zu enriched, %zu conflicts", report.total_entities, report.layers.size(), report.enriched_count, report.conflict_count); + if (report.conflict_count > 0) { + RCLCPP_WARN(logger_, + "MergePipeline: %zu merge conflicts (higher-priority layer wins in all cases). " + "Details available via GET /health.", + report.conflict_count); + } for (const auto & conflict : report.conflicts) { - RCLCPP_WARN(logger_, "Merge conflict: entity '%s' field_group %d - '%s' wins over '%s'", conflict.entity_id.c_str(), - static_cast(conflict.field_group), conflict.winning_layer.c_str(), conflict.losing_layer.c_str()); + RCLCPP_DEBUG(logger_, "Merge conflict: entity '%s' field_group %s - '%s' wins over '%s'", + conflict.entity_id.c_str(), field_group_to_string(conflict.field_group), + conflict.winning_layer.c_str(), conflict.losing_layer.c_str()); } + last_report_ = report; result.report = std::move(report); - last_report_ = result.report; return result; } // Explicit template instantiations -template std::vector -MergePipeline::merge_entities(const std::vector>> &, MergeReport &); +template std::vector MergePipeline::merge_entities(std::vector>> &, + MergeReport &); template std::vector -MergePipeline::merge_entities(const std::vector>> &, MergeReport &); -template std::vector MergePipeline::merge_entities(const std::vector>> &, +MergePipeline::merge_entities(std::vector>> &, MergeReport &); +template std::vector MergePipeline::merge_entities(std::vector>> &, MergeReport &); template std::vector -MergePipeline::merge_entities(const std::vector>> &, MergeReport &); +MergePipeline::merge_entities(std::vector>> &, MergeReport &); } // namespace discovery } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp b/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp index 3ad8f7f0..e23e1f8b 100644 --- a/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp +++ b/src/ros2_medkit_gateway/src/discovery/runtime_discovery.cpp @@ -101,7 +101,8 @@ std::vector RuntimeDiscoveryStrategy::discover_areas() { } std::vector RuntimeDiscoveryStrategy::discover_components() { - return discover_synthetic_components(); + auto apps = discover_apps(); + return discover_synthetic_components(apps); } std::vector RuntimeDiscoveryStrategy::discover_apps() { @@ -546,9 +547,8 @@ bool RuntimeDiscoveryStrategy::path_belongs_to_namespace(const std::string & pat return remainder.find('/') == std::string::npos; } -std::vector RuntimeDiscoveryStrategy::discover_synthetic_components() { +std::vector RuntimeDiscoveryStrategy::discover_synthetic_components(const std::vector & apps) { // Group runtime apps by their component_id (already derived during discover_apps) - auto apps = discover_apps(); std::map> groups; for (const auto & app : apps) { diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index e9b8315a..6bc12dad 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -146,6 +146,14 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { declare_parameter("discovery.runtime.topic_only_policy", "create_component"); declare_parameter("discovery.runtime.min_topics_for_component", 1); + // Merge pipeline configuration (hybrid mode only) + declare_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_areas", true); + declare_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_components", true); + declare_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_apps", true); + declare_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_functions", false); + declare_parameter("discovery.merge_pipeline.gap_fill.namespace_whitelist", std::vector{}); + declare_parameter("discovery.merge_pipeline.gap_fill.namespace_blacklist", std::vector{}); + // Get parameter values server_host_ = get_parameter("server.host").as_string(); server_port_ = static_cast(get_parameter("server.port").as_int()); @@ -356,6 +364,20 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { discovery_config.runtime.min_topics_for_component = static_cast(get_parameter("discovery.runtime.min_topics_for_component").as_int()); + // Merge pipeline gap-fill configuration (hybrid mode) + discovery_config.merge_pipeline.gap_fill.allow_heuristic_areas = + get_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_areas").as_bool(); + discovery_config.merge_pipeline.gap_fill.allow_heuristic_components = + get_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_components").as_bool(); + discovery_config.merge_pipeline.gap_fill.allow_heuristic_apps = + get_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_apps").as_bool(); + discovery_config.merge_pipeline.gap_fill.allow_heuristic_functions = + get_parameter("discovery.merge_pipeline.gap_fill.allow_heuristic_functions").as_bool(); + discovery_config.merge_pipeline.gap_fill.namespace_whitelist = + get_parameter("discovery.merge_pipeline.gap_fill.namespace_whitelist").as_string_array(); + discovery_config.merge_pipeline.gap_fill.namespace_blacklist = + get_parameter("discovery.merge_pipeline.gap_fill.namespace_blacklist").as_string_array(); + if (!discovery_mgr_->initialize(discovery_config)) { RCLCPP_ERROR(get_logger(), "Failed to initialize discovery manager"); throw std::runtime_error("Discovery initialization failed"); @@ -423,6 +445,17 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { plugin_ctx_ = make_gateway_plugin_context(this, fault_mgr_.get()); plugin_mgr_->set_context(*plugin_ctx_); RCLCPP_INFO(get_logger(), "Loaded %zu plugin(s)", loaded); + + // Register IntrospectionProvider plugins as pipeline layers (hybrid mode only) + if (discovery_mgr_->get_mode() == DiscoveryMode::HYBRID) { + auto providers = plugin_mgr_->get_named_introspection_providers(); + for (auto & [name, provider] : providers) { + discovery_mgr_->add_plugin_layer(name, provider); + } + if (!providers.empty()) { + discovery_mgr_->refresh_pipeline(); + } + } } // Initialize log manager (subscribes to /rosout, delegates to plugin if available) diff --git a/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp index 4fe5ff95..04353583 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp @@ -1,4 +1,4 @@ -// Copyright 2025 bburda +// Copyright 2025-2026 bburda // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,6 +17,9 @@ #include #include "ros2_medkit_gateway/auth/auth_models.hpp" +#include "ros2_medkit_gateway/discovery/discovery_enums.hpp" +#include "ros2_medkit_gateway/discovery/discovery_manager.hpp" +#include "ros2_medkit_gateway/gateway_node.hpp" #include "ros2_medkit_gateway/http/error_codes.hpp" #include "ros2_medkit_gateway/http/http_utils.hpp" @@ -31,6 +34,31 @@ void HealthHandlers::handle_health(const httplib::Request & req, httplib::Respon try { json response = {{"status", "healthy"}, {"timestamp", std::chrono::system_clock::now().time_since_epoch().count()}}; + // Add discovery info + auto * dm = ctx_.node() ? ctx_.node()->get_discovery_manager() : nullptr; + if (dm) { + json discovery_info = {{"mode", discovery_mode_to_string(dm->get_mode())}, {"strategy", dm->get_strategy_name()}}; + + auto report = dm->get_merge_report(); + if (report) { + discovery_info["pipeline"] = report->to_json(); + } + + auto linking = dm->get_linking_result(); + if (linking) { + json linking_info; + linking_info["linked_count"] = linking->node_to_app.size(); + linking_info["orphan_count"] = linking->orphan_nodes.size(); + linking_info["binding_conflicts"] = linking->binding_conflicts; + if (!linking->warnings.empty()) { + linking_info["warnings"] = linking->warnings; + } + discovery_info["linking"] = linking_info; + } + + response["discovery"] = std::move(discovery_info); + } + HandlerContext::send_json(res, response); } catch (const std::exception & e) { HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, "Internal server error"); diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp index 3ef41c5d..e0cb5a32 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp @@ -283,6 +283,20 @@ std::vector PluginManager::get_log_observers() const { return result; } +std::vector> PluginManager::get_named_introspection_providers() const { + std::shared_lock lock(plugins_mutex_); + std::vector> result; + for (const auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + if (lp.introspection_provider) { + result.emplace_back(lp.load_result.plugin->name(), lp.introspection_provider); + } + } + return result; +} + bool PluginManager::has_plugins() const { std::shared_lock lock(plugins_mutex_); for (const auto & lp : plugins_) { diff --git a/src/ros2_medkit_gateway/test/test_auth_manager.cpp b/src/ros2_medkit_gateway/test/test_auth_manager.cpp index 33e11d51..23ee6eba 100644 --- a/src/ros2_medkit_gateway/test/test_auth_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_auth_manager.cpp @@ -578,8 +578,8 @@ TEST_F(AuthManagerTest, CleanupExpiredTokens) { auto result = manager.authenticate("test", "test"); ASSERT_TRUE(result.has_value()); - // Wait for tokens to expire - std::this_thread::sleep_for(std::chrono::seconds(2)); + // Wait for tokens to expire (3s margin for loaded systems) + std::this_thread::sleep_for(std::chrono::seconds(3)); // Cleanup should remove expired tokens size_t cleaned = manager.cleanup_expired_tokens(); diff --git a/src/ros2_medkit_gateway/test/test_health_handlers.cpp b/src/ros2_medkit_gateway/test/test_health_handlers.cpp index f92a8d68..2dc95392 100644 --- a/src/ros2_medkit_gateway/test/test_health_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_health_handlers.cpp @@ -57,6 +57,14 @@ TEST_F(HealthHandlersTest, HandleHealthResponseContainsStatusHealthy) { EXPECT_EQ(body["status"], "healthy"); } +TEST_F(HealthHandlersTest, HandleHealthNullNodeOmitsDiscovery) { + // ctx_ uses nullptr for GatewayNode, so discovery info should not be present + handlers_.handle_health(req_, res_); + auto body = json::parse(res_.body); + EXPECT_EQ(body["status"], "healthy"); + EXPECT_FALSE(body.contains("discovery")); +} + TEST_F(HealthHandlersTest, HandleHealthResponseContainsTimestamp) { handlers_.handle_health(req_, res_); auto body = json::parse(res_.body); diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index b6f1501a..cdb25bce 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -55,6 +55,7 @@ class TestLayer : public DiscoveryLayer { std::unordered_map policies_; }; +// @verifies REQ_INTEROP_003 TEST(MergeTypesTest, MergePolicyValues) { // Verify enum values exist and are distinct EXPECT_NE(static_cast(MergePolicy::AUTHORITATIVE), static_cast(MergePolicy::ENRICHMENT)); @@ -150,8 +151,16 @@ App make_app(const std::string & id, const std::string & component_id = "") { return a; } +Function make_function(const std::string & id, const std::string & name = "") { + Function f; + f.id = id; + f.name = name.empty() ? id : name; + return f; +} + } // namespace +// @verifies REQ_INTEROP_003 class MergePipelineTest : public ::testing::Test { protected: MergePipeline pipeline_; @@ -328,6 +337,7 @@ TEST_F(MergePipelineTest, CollectionFieldsUnionOnEnrichment) { // --- ManifestLayer and RuntimeLayer tests --- +// @verifies REQ_INTEROP_003 TEST(ManifestLayerTest, DefaultPolicies) { ManifestLayer layer(nullptr); EXPECT_EQ(layer.name(), "manifest"); @@ -354,6 +364,7 @@ TEST(ManifestLayerTest, DiscoverReturnsEmptyWhenNoManifest) { EXPECT_TRUE(output.functions.empty()); } +// @verifies REQ_INTEROP_003 TEST(RuntimeLayerTest, DefaultPolicies) { RuntimeLayer layer(nullptr); EXPECT_EQ(layer.name(), "runtime"); @@ -376,19 +387,23 @@ TEST(RuntimeLayerTest, DiscoverReturnsEmptyWhenNoStrategy) { class MockIntrospectionProvider : public IntrospectionProvider { public: IntrospectionResult introspect(const IntrospectionInput & input) override { - (void)input; + last_input_ = input; + introspect_called_ = true; return result_; } IntrospectionResult result_; + IntrospectionInput last_input_; + bool introspect_called_{false}; }; +// @verifies REQ_INTEROP_003 TEST(PluginLayerTest, DefaultPolicies) { auto provider = std::make_shared(); PluginLayer layer("lidar_mapper", provider.get()); EXPECT_EQ(layer.name(), "lidar_mapper"); EXPECT_EQ(layer.policy_for(FieldGroup::IDENTITY), MergePolicy::ENRICHMENT); - EXPECT_EQ(layer.policy_for(FieldGroup::METADATA), MergePolicy::AUTHORITATIVE); + EXPECT_EQ(layer.policy_for(FieldGroup::METADATA), MergePolicy::ENRICHMENT); } TEST(PluginLayerTest, MapsNewEntitiesToLayerOutput) { @@ -426,8 +441,39 @@ TEST(PluginLayerTest, DiscoverReturnsEmptyWhenNoProvider) { EXPECT_TRUE(output.entity_metadata.empty()); } +// @verifies REQ_INTEROP_003 +TEST_F(MergePipelineTest, PluginReceivesDiscoveryContext) { + // Manifest layer provides a component + LayerOutput manifest_out; + manifest_out.components.push_back(make_component("engine", "powertrain", "/powertrain")); + manifest_out.apps.push_back(make_app("engine_ecu", "engine")); + pipeline_.add_layer(std::make_unique("manifest", manifest_out)); + + // Runtime layer provides an area + LayerOutput runtime_out; + runtime_out.areas.push_back(make_area("powertrain")); + pipeline_.add_layer(std::make_unique("runtime", runtime_out)); + + // Plugin layer should see entities from manifest + runtime + auto provider = std::make_shared(); + auto plugin = std::make_unique("test_plugin", provider.get()); + pipeline_.add_layer(std::move(plugin)); + + pipeline_.execute(); + + // Plugin's introspect() should have received the context from previous layers + ASSERT_TRUE(provider->introspect_called_); + EXPECT_EQ(provider->last_input_.components.size(), 1u); + EXPECT_EQ(provider->last_input_.components[0].id, "engine"); + EXPECT_EQ(provider->last_input_.apps.size(), 1u); + EXPECT_EQ(provider->last_input_.apps[0].id, "engine_ecu"); + EXPECT_EQ(provider->last_input_.areas.size(), 1u); + EXPECT_EQ(provider->last_input_.areas[0].id, "powertrain"); +} + // --- GapFillConfig tests --- +// @verifies REQ_INTEROP_003 TEST(GapFillConfigTest, DefaultAllowsAll) { GapFillConfig config; EXPECT_TRUE(config.allow_heuristic_areas); @@ -453,6 +499,23 @@ TEST(RuntimeLayerTest, GapFillFilterBlocksApps) { EXPECT_TRUE(output.apps.empty()); } +TEST(MergeReportTest, FilteredByGapFillInJson) { + MergeReport report; + report.layers = {"manifest", "runtime"}; + report.total_entities = 5; + report.filtered_by_gap_fill = 3; + + auto j = report.to_json(); + EXPECT_EQ(j["filtered_by_gap_fill"], 3); +} + +TEST(RuntimeLayerTest, FilteredCountTracked) { + // RuntimeLayer with no strategy returns 0 filtered + RuntimeLayer layer(nullptr); + auto output = layer.discover(); + EXPECT_EQ(layer.last_filtered_count(), 0u); +} + // --- Post-merge linking tests --- TEST_F(MergePipelineTest, PostMergeLinkingSetsAppOnlineStatus) { @@ -519,6 +582,366 @@ TEST_F(MergePipelineTest, PostMergeLinkingReportsOrphanNodes) { pipeline_.set_linker(std::make_unique(nullptr), manifest_config); auto result = pipeline_.execute(); - auto & linking = pipeline_.get_linking_result(); + auto linking = pipeline_.get_linking_result(); EXPECT_FALSE(linking.orphan_nodes.empty()); } + +// --- M4: Cross-type entity ID collision detection --- + +TEST_F(MergePipelineTest, CrossTypeIdCollisionDetected) { + // Same ID used for both an Area and a Component - should detect collision + LayerOutput output; + output.areas.push_back(make_area("shared_id")); + output.components.push_back(make_component("shared_id")); + + pipeline_.add_layer(std::make_unique("layer1", output)); + + auto result = pipeline_.execute(); + EXPECT_EQ(result.report.id_collision_count, 1u); +} + +TEST_F(MergePipelineTest, SameTypeIdIsNotCollision) { + // Same ID in two layers of the same type - normal merge, not a collision + LayerOutput out1; + out1.areas.push_back(make_area("powertrain")); + LayerOutput out2; + out2.areas.push_back(make_area("powertrain")); + + pipeline_.add_layer(std::make_unique("layer1", out1)); + pipeline_.add_layer(std::make_unique("layer2", out2)); + + auto result = pipeline_.execute(); + EXPECT_EQ(result.report.id_collision_count, 0u); + EXPECT_EQ(result.areas.size(), 1u); // merged, not duplicated +} + +// --- M5: FALLBACK policy behavior --- + +TEST_F(MergePipelineTest, FallbackVsFallbackKeepsTargetWhenBothHaveData) { + // Both layers declare FALLBACK for IDENTITY, both have name set + // Higher priority target keeps its value (first non-empty) + Component comp1 = make_component("engine", "powertrain", "/powertrain"); + comp1.name = "First Layer Engine"; + + Component comp2 = make_component("engine", "powertrain", "/powertrain"); + comp2.name = "Second Layer Engine"; + + LayerOutput out1, out2; + out1.components.push_back(comp1); + out2.components.push_back(comp2); + + pipeline_.add_layer(std::make_unique( + "layer1", out1, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}})); + pipeline_.add_layer(std::make_unique( + "layer2", out2, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].name, "First Layer Engine"); + EXPECT_EQ(result.report.conflict_count, 0u); +} + +TEST_F(MergePipelineTest, FallbackVsFallbackFillsEmptyFields) { + // Target has empty description, source has it - FALLBACK should fill the gap + Component comp1 = make_component("engine", "powertrain", "/powertrain"); + comp1.description = ""; + + Component comp2 = make_component("engine", "powertrain", "/powertrain"); + comp2.description = "Engine control unit"; + + LayerOutput out1, out2; + out1.components.push_back(comp1); + out2.components.push_back(comp2); + + pipeline_.add_layer(std::make_unique( + "layer1", out1, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}})); + pipeline_.add_layer(std::make_unique( + "layer2", out2, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].description, "Engine control unit"); +} + +TEST_F(MergePipelineTest, FallbackVsEnrichmentEnrichmentWins) { + // ENRICHMENT > FALLBACK + Component comp1 = make_component("engine", "powertrain", "/powertrain"); + comp1.name = "Fallback Name"; + + Component comp2 = make_component("engine", "powertrain", "/powertrain"); + comp2.name = "Enrichment Name"; + + LayerOutput out1, out2; + out1.components.push_back(comp1); + out2.components.push_back(comp2); + + pipeline_.add_layer(std::make_unique( + "fallback", out1, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}})); + pipeline_.add_layer(std::make_unique( + "enrich", out2, std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + EXPECT_EQ(result.components[0].name, "Enrichment Name"); // ENRICHMENT wins over FALLBACK +} + +// --- M6: MergeConflict struct fields --- + +TEST_F(MergePipelineTest, MergeConflictStructPopulated) { + // AUTH vs AUTH on same entity - verify conflict fields + Component comp1 = make_component("engine", "powertrain", "/powertrain"); + comp1.name = "Manifest Engine"; + + Component comp2 = make_component("engine", "powertrain", "/powertrain"); + comp2.name = "Runtime Engine"; + + LayerOutput out1, out2; + out1.components.push_back(comp1); + out2.components.push_back(comp2); + + pipeline_.add_layer(std::make_unique( + "manifest", out1, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}})); + pipeline_.add_layer(std::make_unique( + "runtime", out2, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}})); + + auto result = pipeline_.execute(); + ASSERT_GE(result.report.conflicts.size(), 1u); + + // Find the IDENTITY conflict for "engine" + bool found = false; + for (const auto & c : result.report.conflicts) { + if (c.entity_id == "engine" && c.field_group == FieldGroup::IDENTITY) { + EXPECT_EQ(c.winning_layer, "manifest"); + EXPECT_EQ(c.losing_layer, "runtime"); + found = true; + break; + } + } + EXPECT_TRUE(found) << "Expected IDENTITY conflict for entity 'engine'"; +} + +// --- m9: MergeReport::to_json() includes conflicts --- + +TEST(MergeTypesTest, MergeReportToJsonIncludesConflicts) { + MergeReport report; + report.layers = {"manifest", "runtime"}; + report.conflict_count = 1; + report.conflicts.push_back({"engine", FieldGroup::IDENTITY, "manifest", "runtime"}); + + auto j = report.to_json(); + ASSERT_TRUE(j.contains("conflicts")); + ASSERT_EQ(j["conflicts"].size(), 1u); + EXPECT_EQ(j["conflicts"][0]["entity_id"], "engine"); + EXPECT_EQ(j["conflicts"][0]["field_group"], "IDENTITY"); + EXPECT_EQ(j["conflicts"][0]["winning_layer"], "manifest"); + EXPECT_EQ(j["conflicts"][0]["losing_layer"], "runtime"); +} + +// --- Three-layer merge (manifest + runtime + plugin) --- + +// @verifies REQ_INTEROP_003 +TEST_F(MergePipelineTest, ThreeLayerMerge_PluginEnrichesManifestEntity) { + // Manifest: component "engine" with AUTHORITATIVE identity + Component manifest_comp = make_component("engine", "powertrain", "/powertrain"); + manifest_comp.name = "Engine ECU"; + manifest_comp.description = "Main engine controller"; + + // Runtime: component "engine" with FALLBACK identity, AUTHORITATIVE live_data (topics) + Component runtime_comp = make_component("engine", "powertrain", "/powertrain"); + runtime_comp.name = "engine"; // heuristic name, should not override manifest + ServiceInfo svc; + svc.name = "get_status"; + svc.full_path = "/powertrain/get_status"; + svc.type = "std_srvs/srv/Trigger"; + runtime_comp.services.push_back(svc); + + // Plugin: component "engine" with ENRICHMENT metadata - adds vendor extension + Component plugin_comp = make_component("engine", "powertrain", "/powertrain"); + plugin_comp.description = "Plugin-enriched description"; // IDENTITY: won't override manifest AUTH + plugin_comp.source = "vendor-plugin"; // METADATA: will fill empty field + + LayerOutput manifest_out, runtime_out, plugin_out; + manifest_out.components.push_back(manifest_comp); + runtime_out.components.push_back(runtime_comp); + plugin_out.components.push_back(plugin_comp); + + pipeline_.add_layer(std::make_unique( + "manifest", manifest_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}, + {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}})); + pipeline_.add_layer(std::make_unique( + "plugin", plugin_out, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::ENRICHMENT}, + {FieldGroup::LIVE_DATA, MergePolicy::ENRICHMENT}, + {FieldGroup::METADATA, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.components.size(), 1u); + + const auto & merged = result.components[0]; + // Name from manifest (AUTHORITATIVE IDENTITY wins over plugin ENRICHMENT) + EXPECT_EQ(merged.name, "Engine ECU"); + // Description from manifest (AUTHORITATIVE beats ENRICHMENT) + EXPECT_EQ(merged.description, "Main engine controller"); + // Services from runtime (AUTHORITATIVE LIVE_DATA) + EXPECT_EQ(merged.services.size(), 1u); + // entity_source tracks the first layer that introduced this entity + EXPECT_EQ(result.report.entity_source["engine"], "manifest"); +} + +// --- App STATUS field group merge --- + +// @verifies REQ_INTEROP_003 +TEST_F(MergePipelineTest, AppStatusMerge_BoolOrSemantics) { + // Two layers provide the same App with different is_online values + App app1 = make_app("controller", "nav_comp"); + app1.is_online = false; + + App app2 = make_app("controller", "nav_comp"); + app2.is_online = true; + app2.bound_fqn = "/nav/controller"; + + LayerOutput out1, out2; + out1.apps.push_back(app1); + out2.apps.push_back(app2); + + pipeline_.add_layer(std::make_unique( + "manifest", out1, std::unordered_map{{FieldGroup::STATUS, MergePolicy::FALLBACK}})); + pipeline_.add_layer(std::make_unique( + "runtime", out2, std::unordered_map{{FieldGroup::STATUS, MergePolicy::AUTHORITATIVE}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.apps.size(), 1u); + // Runtime STATUS is AUTHORITATIVE, so is_online=true and bound_fqn set + EXPECT_TRUE(result.apps[0].is_online); + EXPECT_EQ(result.apps[0].bound_fqn, "/nav/controller"); +} + +// --- GapFillConfig namespace filtering --- +// These tests verify the namespace matching semantics used by RuntimeLayer. +// Since filter_by_namespace is internal to runtime_layer.cpp, we replicate the +// matching logic here to test the *semantics* of GapFillConfig lists. + +namespace { +// Mirrors the is_namespace_allowed() semantics in runtime_layer.cpp: +// path-segment boundary matching (ns == w || ns starts with w + "/"). +bool is_namespace_allowed(const std::string & ns, const GapFillConfig & config) { + if (!config.namespace_whitelist.empty()) { + bool found = + std::any_of(config.namespace_whitelist.begin(), config.namespace_whitelist.end(), [&ns](const std::string & w) { + return ns == w || ns.find(w + "/") == 0; + }); + if (!found) { + return false; + } + } + for (const auto & b : config.namespace_blacklist) { + if (ns == b || ns.find(b + "/") == 0) { + return false; + } + } + return true; +} +} // namespace + +// @verifies REQ_INTEROP_003 +TEST(GapFillConfigTest, NamespaceWhitelistFiltersAreas) { + GapFillConfig config; + config.namespace_whitelist = {"/robot"}; + + // Exact match passes + EXPECT_TRUE(is_namespace_allowed("/robot", config)); + // Child namespace passes (path-segment boundary) + EXPECT_TRUE(is_namespace_allowed("/robot/nav", config)); + // Different namespace blocked + EXPECT_FALSE(is_namespace_allowed("/sensor", config)); + // Prefix that is NOT a path segment boundary should be blocked + EXPECT_FALSE(is_namespace_allowed("/robotics", config)); +} + +// @verifies REQ_INTEROP_003 +TEST(GapFillConfigTest, NamespaceBlacklistFiltersAreas) { + GapFillConfig config; + config.namespace_blacklist = {"/rosout"}; + + // Not blacklisted + EXPECT_TRUE(is_namespace_allowed("/robot", config)); + // Exact match blocked + EXPECT_FALSE(is_namespace_allowed("/rosout", config)); + // Child namespace blocked (path-segment boundary) + EXPECT_FALSE(is_namespace_allowed("/rosout/sub", config)); + // Prefix that is NOT a path segment boundary should pass + EXPECT_TRUE(is_namespace_allowed("/rosoutput", config)); +} + +// --- Pipeline exception safety --- + +namespace { +class ThrowingLayer : public DiscoveryLayer { + public: + std::string name() const override { + return "throwing"; + } + LayerOutput discover() override { + throw std::runtime_error("plugin crash"); + } + MergePolicy policy_for(FieldGroup /*group*/) const override { + return MergePolicy::ENRICHMENT; + } +}; +} // namespace + +// @verifies REQ_INTEROP_003 +TEST_F(MergePipelineTest, LayerExceptionDoesNotCrashPipeline) { + // A good layer followed by a throwing layer - good layer's data should survive + LayerOutput good_output; + good_output.areas.push_back(make_area("powertrain")); + pipeline_.add_layer(std::make_unique("good", good_output)); + pipeline_.add_layer(std::make_unique()); + + auto result = pipeline_.execute(); + // Good layer's data should be present despite the throwing layer + ASSERT_EQ(result.areas.size(), 1u); + EXPECT_EQ(result.areas[0].id, "powertrain"); +} + +// @verifies REQ_DISC_MERGE_FUNCTION +TEST_F(MergePipelineTest, FunctionMerge_HostsAndIdentity) { + // Layer 1 (AUTH for IDENTITY): function with name, no hosts + Function auth_func = make_function("diagnostics", "Diagnostics Suite"); + auth_func.source = "manifest"; + + LayerOutput auth_output; + auth_output.functions.push_back(auth_func); + + // Layer 2 (ENRICHMENT): function with hosts, different name + Function enrich_func = make_function("diagnostics", "diag_runtime"); + enrich_func.hosts = {"engine_ecu", "brake_controller"}; + enrich_func.source = "runtime"; + + LayerOutput enrich_output; + enrich_output.functions.push_back(enrich_func); + + pipeline_.add_layer(std::make_unique( + "manifest", auth_output, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::AUTHORITATIVE}, + {FieldGroup::HIERARCHY, MergePolicy::ENRICHMENT}})); + + pipeline_.add_layer(std::make_unique( + "runtime", enrich_output, + std::unordered_map{{FieldGroup::IDENTITY, MergePolicy::FALLBACK}, + {FieldGroup::HIERARCHY, MergePolicy::ENRICHMENT}})); + + auto result = pipeline_.execute(); + ASSERT_EQ(result.functions.size(), 1u); + EXPECT_EQ(result.functions[0].name, "Diagnostics Suite"); // AUTH identity wins + EXPECT_EQ(result.functions[0].hosts.size(), 2u); // ENRICHMENT fills hosts + EXPECT_EQ(result.functions[0].source, "manifest"); // higher priority source +} diff --git a/src/ros2_medkit_gateway/test/test_runtime_linker.cpp b/src/ros2_medkit_gateway/test/test_runtime_linker.cpp index 1aabbdb2..2e60eae9 100644 --- a/src/ros2_medkit_gateway/test/test_runtime_linker.cpp +++ b/src/ros2_medkit_gateway/test/test_runtime_linker.cpp @@ -73,6 +73,7 @@ class RuntimeLinkerTest : public ::testing::Test { // Exact Match Tests // ============================================================================= +// @verifies REQ_INTEROP_003 TEST_F(RuntimeLinkerTest, ExactMatch_NodeNameAndNamespace) { std::vector apps = {create_app("controller_app", "controller", "/nav")}; std::vector runtime_apps = {create_runtime_app("controller", "/nav")}; @@ -288,6 +289,34 @@ TEST_F(RuntimeLinkerTest, OrphanPolicy_Ignore_NoError) { EXPECT_FALSE(result.has_errors(config_.unmanifested_nodes)); } +// @verifies REQ_INTEROP_003 +TEST_F(RuntimeLinkerTest, OrphanPolicy_Warn_NoError) { + config_.unmanifested_nodes = ManifestConfig::UnmanifestedNodePolicy::WARN; + + std::vector apps = {}; + std::vector runtime_apps = {create_runtime_app("orphan_node", "/")}; + + auto result = linker_->link(apps, runtime_apps, config_); + + // WARN logs warnings but does not fail + EXPECT_FALSE(result.has_errors(config_.unmanifested_nodes)); + EXPECT_EQ(result.orphan_nodes.size(), 1u); +} + +// @verifies REQ_INTEROP_003 +TEST_F(RuntimeLinkerTest, OrphanPolicy_IncludeAsOrphan_NoError) { + config_.unmanifested_nodes = ManifestConfig::UnmanifestedNodePolicy::INCLUDE_AS_ORPHAN; + + std::vector apps = {}; + std::vector runtime_apps = {create_runtime_app("orphan_node", "/")}; + + auto result = linker_->link(apps, runtime_apps, config_); + + // INCLUDE_AS_ORPHAN includes orphans but does not fail + EXPECT_FALSE(result.has_errors(config_.unmanifested_nodes)); + EXPECT_EQ(result.orphan_nodes.size(), 1u); +} + // ============================================================================= // App Enrichment Tests // ============================================================================= diff --git a/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_hybrid.test.py b/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_hybrid.test.py index 843962bf..8cf6e30c 100644 --- a/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_hybrid.test.py +++ b/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_hybrid.test.py @@ -435,6 +435,17 @@ def test_30_nonexistent_function(self): """404 for non-existent function.""" self.assert_entity_not_found('functions', 'nonexistent') + def test_31_health_includes_discovery_diagnostics(self): + """GET /health in hybrid mode includes discovery pipeline info.""" + data = self.get_json('/health') + self.assertIn('discovery', data) + self.assertEqual(data['discovery']['mode'], 'hybrid') + self.assertIn('pipeline', data['discovery']) + pipeline = data['discovery']['pipeline'] + self.assertIn('layers', pipeline) + self.assertIn('total_entities', pipeline) + self.assertGreater(pipeline['total_entities'], 0) + @launch_testing.post_shutdown_test() class TestShutdown(unittest.TestCase): From 0a2efed085d6c6991c00018a159cec356556f7a4 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Thu, 5 Mar 2026 07:35:58 +0100 Subject: [PATCH 5/6] test(plugins): add vendor extension demo and integration test Add VendorExtensionPlugin demo that registers custom entities and routes via the IntrospectionProvider interface. Add integration test validating plugin layer discovery, entity merging, and context passing through the merge pipeline. --- docs/tutorials/plugin-system.rst | 2 + src/ros2_medkit_gateway/design/index.rst | 4 +- .../providers/introspection_provider.hpp | 5 +- .../test/demo_nodes/test_gateway_plugin.cpp | 31 +++++ .../test_plugin_vendor_extensions.test.py | 126 ++++++++++++++++++ 5 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 src/ros2_medkit_integration_tests/test/features/test_plugin_vendor_extensions.test.py diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst index 99528d72..5f1b74c2 100644 --- a/docs/tutorials/plugin-system.rst +++ b/docs/tutorials/plugin-system.rst @@ -305,6 +305,8 @@ Multiple plugins can be loaded simultaneously: Each plugin's entities are merged with ENRICHMENT policy - they fill empty fields but never override manifest or runtime values. Plugins are added after all built-in layers, and the pipeline is refreshed once after all plugins are registered (batch registration). + The ``introspect()`` method receives an ``IntrospectionInput`` populated with all entities + from previous layers (manifest + runtime), enabling context-aware metadata and discovery. - **Custom routes**: All plugins can register endpoints (use unique path prefixes) Error Handling diff --git a/src/ros2_medkit_gateway/design/index.rst b/src/ros2_medkit_gateway/design/index.rst index 9003b7e4..120b4b50 100644 --- a/src/ros2_medkit_gateway/design/index.rst +++ b/src/ros2_medkit_gateway/design/index.rst @@ -331,7 +331,9 @@ Main Components - ``RuntimeLayer`` - Wraps RuntimeDiscoveryStrategy; LIVE_DATA/STATUS are AUTHORITATIVE, METADATA is ENRICHMENT, IDENTITY/HIERARCHY are FALLBACK. Supports ``GapFillConfig`` to control which heuristic entities are allowed when manifest is present - - ``PluginLayer`` - Wraps IntrospectionProvider; all fields ENRICHMENT (plugins enrich, they don't override) + - ``PluginLayer`` - Wraps IntrospectionProvider; all fields ENRICHMENT (plugins enrich, they don't override). + Before each layer's ``discover()`` call, the pipeline populates ``IntrospectionInput`` with entities + from all previous layers, so plugins see the current manifest + runtime entity set 3. **OperationManager** - Executes ROS 2 operations (services and actions) using native APIs - Calls ROS 2 services via ``rclcpp::GenericClient`` with native serialization diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp index 13275432..05cbfa09 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp @@ -74,9 +74,10 @@ class IntrospectionProvider { /** * @brief Core introspection method * - * Called after each discovery cycle, before EntityCache update. + * Called during each discovery cycle by the merge pipeline. + * Input contains entities from all higher-priority layers (manifest + runtime). * - * @param input Snapshot of currently discovered entities + * @param input Snapshot of entities discovered by previous layers * @return Metadata enrichments and new entities */ virtual IntrospectionResult introspect(const IntrospectionInput & input) = 0; diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp index 5f48f73f..7dbd13a5 100644 --- a/src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" +#include "ros2_medkit_gateway/plugins/plugin_context.hpp" #include "ros2_medkit_gateway/plugins/plugin_types.hpp" #include "ros2_medkit_gateway/providers/introspection_provider.hpp" #include "ros2_medkit_gateway/providers/update_provider.hpp" @@ -29,6 +30,10 @@ using namespace ros2_medkit_gateway; * * Used by test_plugin_loader to verify dlopen/dlsym-based loading, * API version checking, and extern "C" provider query functions. + * + * Also demonstrates vendor extension endpoints via register_capability() + * and entity-scoped routes. The plugin registers an "x-medkit-diagnostics" + * capability on all Components and serves diagnostic data per entity. */ class TestGatewayPlugin : public GatewayPlugin, public UpdateProvider, public IntrospectionProvider { public: @@ -40,12 +45,38 @@ class TestGatewayPlugin : public GatewayPlugin, public UpdateProvider, public In void configure(const nlohmann::json & /*config*/) override { } + void set_context(PluginContext & context) override { + ctx_ = &context; + // Register vendor extension capability for all Components + ctx_->register_capability(SovdEntityType::COMPONENT, "x-medkit-diagnostics"); + } + void register_routes(httplib::Server & server, const std::string & api_prefix) override { + // Global vendor extension endpoint server.Get((api_prefix + "/x-test/ping").c_str(), [](const httplib::Request &, httplib::Response & res) { res.set_content("pong", "text/plain"); }); + + // Entity-scoped vendor extension: GET /components/{id}/x-medkit-diagnostics + server.Get((api_prefix + R"(/components/([^/]+)/x-medkit-diagnostics)").c_str(), + [this](const httplib::Request & req, httplib::Response & res) { + auto entity_id = req.matches[1].str(); + auto entity = ctx_->validate_entity_for_route(req, res, entity_id); + if (!entity) { + return; + } + nlohmann::json data = {{"entity_id", entity->id}, + {"plugin", "test_plugin"}, + {"cpu_usage", 42.5}, + {"memory_mb", 128}, + {"uptime_seconds", 3600}}; + PluginContext::send_json(res, data); + }); } + private: + PluginContext * ctx_{nullptr}; + // --- UpdateProvider --- tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { return std::vector{}; diff --git a/src/ros2_medkit_integration_tests/test/features/test_plugin_vendor_extensions.test.py b/src/ros2_medkit_integration_tests/test/features/test_plugin_vendor_extensions.test.py new file mode 100644 index 00000000..17372882 --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/features/test_plugin_vendor_extensions.test.py @@ -0,0 +1,126 @@ +#!/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. + +"""Integration tests for plugin vendor extension endpoints. + +Validates that plugins can register per-entity-type capabilities via +register_capability() and serve entity-scoped vendor extension resources +via register_routes(). + +Uses test_gateway_plugin which registers an "x-medkit-diagnostics" +capability for all Components and serves diagnostic data at +GET /components/{id}/x-medkit-diagnostics. +""" + +import os +import unittest + +import launch_testing +import requests + +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 + + +def _get_test_plugin_path(): + """Get path to test_gateway_plugin.so.""" + from ament_index_python.packages import get_package_prefix + + pkg_prefix = get_package_prefix('ros2_medkit_gateway') + return os.path.join( + pkg_prefix, 'lib', 'ros2_medkit_gateway', 'libtest_gateway_plugin.so' + ) + + +def generate_test_description(): + """Launch gateway with test_gateway_plugin and a few demo nodes.""" + plugin_path = _get_test_plugin_path() + return create_test_launch( + demo_nodes=['temp_sensor', 'rpm_sensor'], + fault_manager=False, + gateway_params={ + 'plugins': ['test_plugin'], + 'plugins.test_plugin.path': plugin_path, + }, + ) + + +class TestPluginVendorExtensions(GatewayTestCase): + """Vendor extension endpoint tests via test_gateway_plugin.""" + + MIN_EXPECTED_APPS = 2 + REQUIRED_APPS = {'temp_sensor', 'rpm_sensor'} + + def _get_any_component_id(self): + """Get the first discovered component ID.""" + data = self.get_json('/components') + items = data.get('items', []) + self.assertGreater(len(items), 0, 'No components discovered') + return items[0]['id'] + + def test_01_vendor_extension_endpoint_returns_data(self): + """GET /components/{id}/x-medkit-diagnostics returns plugin data.""" + comp_id = self._get_any_component_id() + data = self.get_json(f'/components/{comp_id}/x-medkit-diagnostics') + self.assertEqual(data['entity_id'], comp_id) + self.assertEqual(data['plugin'], 'test_plugin') + self.assertIn('cpu_usage', data) + self.assertIn('memory_mb', data) + self.assertIn('uptime_seconds', data) + + def test_02_capabilities_include_vendor_extension(self): + """Entity capabilities include the plugin-registered capability.""" + comp_id = self._get_any_component_id() + data = self.get_json(f'/components/{comp_id}') + capabilities = data.get('capabilities', []) + cap_names = [c['name'] for c in capabilities] + self.assertIn( + 'x-medkit-diagnostics', + cap_names, + f'x-medkit-diagnostics not in capabilities: {cap_names}', + ) + # Verify href points to the correct path + diag_cap = next( + c for c in capabilities if c['name'] == 'x-medkit-diagnostics' + ) + self.assertIn(f'/components/{comp_id}/x-medkit-diagnostics', diag_cap['href']) + + def test_03_vendor_extension_nonexistent_entity_returns_404(self): + """GET /components/nonexistent/x-medkit-diagnostics returns 404.""" + r = requests.get( + f'{self.BASE_URL}/components/nonexistent-entity/x-medkit-diagnostics', + timeout=5, + ) + self.assertEqual(r.status_code, 404) + + def test_04_global_vendor_endpoint_still_works(self): + """GET /x-test/ping global endpoint still responds.""" + r = requests.get(f'{self.BASE_URL}/x-test/ping', timeout=5) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.text, 'pong') + + +@launch_testing.post_shutdown_test() +class TestShutdown(unittest.TestCase): + """Verify gateway exits cleanly.""" + + def test_exit_codes(self, proc_info): + for info in proc_info: + self.assertIn( + info.returncode, + ALLOWED_EXIT_CODES, + f'Process {info.process_name} exited with {info.returncode}', + ) From e8863425afe8f8cff40a945ae4ef8bd258c78542 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Thu, 5 Mar 2026 18:46:39 +0100 Subject: [PATCH 6/6] fix(build): enable POSITION_INDEPENDENT_CODE for MODULE targets Fix linker errors (TPOFF32, R_X86_64_PC32) when gateway_lib.a is linked into test_gateway_plugin.so (MODULE target). Remove static thread_local std::mt19937 in BulkDataStore (incompatible with initial-exec TLS model). Enable POSITION_INDEPENDENT_CODE on ros2_medkit_serialization static lib. Remove @verifies tag referencing nonexistent REQ_DISC_MERGE_FUNCTION. --- src/ros2_medkit_gateway/src/bulk_data_store.cpp | 5 +++-- src/ros2_medkit_gateway/test/test_merge_pipeline.cpp | 1 - src/ros2_medkit_serialization/CMakeLists.txt | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ros2_medkit_gateway/src/bulk_data_store.cpp b/src/ros2_medkit_gateway/src/bulk_data_store.cpp index 3b0b0842..fce7475e 100644 --- a/src/ros2_medkit_gateway/src/bulk_data_store.cpp +++ b/src/ros2_medkit_gateway/src/bulk_data_store.cpp @@ -14,7 +14,6 @@ #include "ros2_medkit_gateway/bulk_data_store.hpp" -#include #include #include #include @@ -75,7 +74,9 @@ std::string BulkDataStore::generate_id(const std::string & category) { auto ns = std::chrono::duration_cast(now.time_since_epoch()).count(); // Generate 8 hex chars from random - static thread_local std::mt19937 gen(std::random_device{}()); + // Local (not static thread_local) to avoid initial-exec TLS relocations (TPOFF32) + // that break when gateway_lib.a is linked into a shared object (test_gateway_plugin.so) + std::mt19937 gen(std::random_device{}()); std::uniform_int_distribution dist(0, 0xFFFFFFFF); uint32_t rand_val = dist(gen); diff --git a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp index cdb25bce..59312ba5 100644 --- a/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/test/test_merge_pipeline.cpp @@ -912,7 +912,6 @@ TEST_F(MergePipelineTest, LayerExceptionDoesNotCrashPipeline) { EXPECT_EQ(result.areas[0].id, "powertrain"); } -// @verifies REQ_DISC_MERGE_FUNCTION TEST_F(MergePipelineTest, FunctionMerge_HostsAndIdentity) { // Layer 1 (AUTH for IDENTITY): function with name, no hosts Function auth_func = make_function("diagnostics", "Diagnostics Suite"); diff --git a/src/ros2_medkit_serialization/CMakeLists.txt b/src/ros2_medkit_serialization/CMakeLists.txt index 47f4b5c2..8720ffd8 100644 --- a/src/ros2_medkit_serialization/CMakeLists.txt +++ b/src/ros2_medkit_serialization/CMakeLists.txt @@ -51,6 +51,10 @@ add_library(${PROJECT_NAME} src/vendored/dynmsg/yaml_utils.cpp ) +# Enable PIC so this static library can be linked into shared objects +# (e.g. test_gateway_plugin.so via gateway_lib.a) +set_target_properties(${PROJECT_NAME} PROPERTIES POSITION_INDEPENDENT_CODE ON) + target_include_directories(${PROJECT_NAME} PUBLIC $ $