From aad8dad918a7092b91fa9f56d7151c3ead65eb00 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 16:54:40 +0000 Subject: [PATCH 01/14] chore: add .worktrees/ to gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index b17fbd9c..c8e61a62 100644 --- a/.gitignore +++ b/.gitignore @@ -244,6 +244,9 @@ qtcreator-* COLCON_IGNORE AMENT_IGNORE +# Git worktrees +.worktrees/ + # End of https://www.toptal.com/developers/gitignore/api/ros2,c++,pythonPLAN.md PLAN.mdsrc/dynamic_message_introspection/ From 33ea91f6318f92f7737e936136635dd3348cba68 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 17:31:34 +0000 Subject: [PATCH 02/14] feat(plugins): add GatewayPlugin base class and plugin types Define the base class for all gateway plugins with lifecycle methods (configure, set_node, register_routes, shutdown) and callback-based logging. Plugins implement this plus typed provider interfaces. Refs #235 --- .../plugins/gateway_plugin.hpp | 130 ++++++++++++++++++ .../plugins/plugin_types.hpp | 35 +++++ 2 files changed, 165 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp new file mode 100644 index 00000000..394dd13d --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp @@ -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. + +#pragma once + +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" + +#include +#include +#include +#include + +namespace rclcpp { +class Node; +} + +namespace ros2_medkit_gateway { + +/** + * @brief Base class for all gateway plugins + * + * Plugins are loaded as shared libraries (.so) via dlopen/dlsym. + * Each .so must export two extern "C" functions: + * int plugin_api_version(); // must return PLUGIN_API_VERSION + * GatewayPlugin* create_plugin(); // factory + * + * Plugins implement this base class plus one or more provider interfaces + * (UpdateProvider, IntrospectionProvider) via multiple inheritance. + * + * @see PluginManager for loading and lifecycle orchestration + * @see UpdateProvider, IntrospectionProvider for typed interfaces + */ +class GatewayPlugin { + public: + virtual ~GatewayPlugin() = default; + + /** + * @brief Unique name for this plugin + * @return Plugin name (e.g., "systemd", "procfs", "mender_ota") + */ + virtual std::string name() const = 0; + + /** + * @brief Configure the plugin + * + * Called once after loading with per-plugin config from YAML. + * + * @param config JSON configuration object + */ + virtual void configure(const nlohmann::json & config) = 0; + + /** + * @brief Optionally receive a ROS 2 node + * + * Called after configure(). Plugins that need ROS 2 access + * (subscriptions, service clients) can use this node. + * Platform-only plugins can ignore this. + * + * @param node ROS 2 node pointer (must outlive this plugin) + */ + virtual void set_node(rclcpp::Node * /*node*/) { + } + + /** + * @brief Optionally register custom REST routes + * + * Called once during REST server setup. Plugins can register + * vendor-specific endpoints (e.g., /x-medkit/my-feature). + * + * @param server httplib server instance + * @param api_prefix API path prefix (e.g., "/api/v1") + */ + virtual void register_routes(httplib::Server & /*server*/, const std::string & /*api_prefix*/) { + } + + /** + * @brief Shutdown hook for cleanup + * + * Called before the plugin is destroyed. Use for releasing + * resources, closing connections, etc. + */ + virtual void shutdown() { + } + + protected: + /// Log an informational message (routed to gateway's ROS 2 logger) + void log_info(const std::string & msg) const { + if (log_fn_) { + log_fn_(PluginLogLevel::kInfo, msg); + } + } + + /// Log a warning message + void log_warn(const std::string & msg) const { + if (log_fn_) { + log_fn_(PluginLogLevel::kWarn, msg); + } + } + + /// Log an error message + void log_error(const std::string & msg) const { + if (log_fn_) { + log_fn_(PluginLogLevel::kError, msg); + } + } + + private: + friend class PluginManager; // Sets log_fn_ after construction + + /// Logging callback set by PluginManager. Routes to rclcpp::get_logger("plugin."). + std::function log_fn_; + + /// Called by PluginManager to wire up logging + void set_logger(std::function fn) { + log_fn_ = std::move(fn); + } +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp new file mode 100644 index 00000000..996abea6 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp @@ -0,0 +1,35 @@ +// 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 + +namespace ros2_medkit_gateway { + +/// Current plugin API version. Plugins must export this value from plugin_api_version(). +constexpr int PLUGIN_API_VERSION = 1; + +/// Log severity levels for plugin logging callback +enum class PluginLogLevel { kInfo, kWarn, kError }; + +/// Configuration for a single plugin loaded from YAML +struct PluginConfig { + std::string name; ///< Plugin key from YAML (used for parameter namespace) + std::string path; ///< Path to .so file + nlohmann::json config; ///< Per-plugin configuration (passed to configure()) +}; + +} // namespace ros2_medkit_gateway From 936be5a6f5d6d551edfee52fe2001de9538bc52e Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 17:32:32 +0000 Subject: [PATCH 03/14] feat(plugins): add UpdateProvider and IntrospectionProvider interfaces Typed provider interfaces that plugins implement alongside GatewayPlugin. UpdateProvider mirrors existing UpdateBackend. IntrospectionProvider defines the enrichment/discovery interface. Refs #235 --- .../providers/introspection_provider.hpp | 84 +++++++++++++++++++ .../providers/update_provider.hpp | 54 ++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp 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 new file mode 100644 index 00000000..95271e3e --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp @@ -0,0 +1,84 @@ +// 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/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 { + +/** + * @brief Snapshot of current discovery state, passed to introspection providers + */ +struct IntrospectionInput { + std::vector areas; + std::vector components; + std::vector apps; + std::vector functions; +}; + +/** + * @brief New entity definitions an introspection provider can introduce + */ +struct NewEntities { + std::vector areas; + std::vector components; + std::vector apps; +}; + +/** + * @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. + std::unordered_map metadata; + + /// New entities discovered by this provider + NewEntities new_entities; +}; + +/** + * @brief Provider interface for platform-specific introspection + * + * Implementations enrich discovered entities with runtime metadata + * and can discover new entities from non-ROS sources. + * + * @see GatewayPlugin for the base class + * @see PluginManager for orchestration + */ +class IntrospectionProvider { + public: + virtual ~IntrospectionProvider() = default; + + /** + * @brief Core introspection method + * + * Called after each discovery cycle, before EntityCache update. + * + * @param input Snapshot of currently discovered entities + * @return Metadata enrichments and new entities + */ + virtual IntrospectionResult introspect(const IntrospectionInput & input) = 0; +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp new file mode 100644 index 00000000..6c52cd3a --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp @@ -0,0 +1,54 @@ +// 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 + +// TODO(Task 5): rename this include to update_types.hpp when UpdateBackend is removed +#include "ros2_medkit_gateway/updates/update_backend.hpp" + +namespace ros2_medkit_gateway { + +/** + * @brief Provider interface for software update backends + * + * Typed provider interface that plugins implement alongside GatewayPlugin + * via multiple inheritance. Replaces the old UpdateBackend abstract class. + * + * Reuses all types from update_backend.hpp (UpdateFilter, UpdateBackendErrorInfo, + * UpdateProgressReporter, etc.). + * + * @see GatewayPlugin for the base class + * @see UpdateManager for the subsystem that uses this + */ +class UpdateProvider { + public: + virtual ~UpdateProvider() = default; + + // ---- CRUD (metadata storage owned by plugin) ---- + virtual tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & filter) = 0; + virtual tl::expected get_update(const std::string & id) = 0; + virtual tl::expected register_update(const nlohmann::json & metadata) = 0; + virtual tl::expected delete_update(const std::string & id) = 0; + + // ---- Async operations (called in background threads) ---- + virtual tl::expected prepare(const std::string & id, + UpdateProgressReporter & reporter) = 0; + virtual tl::expected execute(const std::string & id, + UpdateProgressReporter & reporter) = 0; + + // ---- Capability queries ---- + virtual tl::expected supports_automated(const std::string & id) = 0; +}; + +} // namespace ros2_medkit_gateway From aeff01a07e8f9d1c02f764ce4459af736805d295 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 18:13:05 +0000 Subject: [PATCH 04/14] feat(plugins): add PluginLoader with API versioning and test plugin Loads GatewayPlugin instances from .so via dlopen/dlsym. Checks plugin_api_version() before calling create_plugin() factory. Test plugin implements both UpdateProvider and IntrospectionProvider. Refs #235 --- src/ros2_medkit_gateway/CMakeLists.txt | 17 ++++ .../plugins/plugin_loader.hpp | 45 ++++++++++ .../src/plugins/plugin_loader.cpp | 90 +++++++++++++++++++ .../test/demo_nodes/test_gateway_plugin.cpp | 90 +++++++++++++++++++ .../test/test_plugin_loader.cpp | 73 +++++++++++++++ 5 files changed, 315 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp create mode 100644 src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp create mode 100644 src/ros2_medkit_gateway/test/test_plugin_loader.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 9c3929f0..e4777dec 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -133,6 +133,8 @@ add_library(gateway_lib STATIC src/updates/plugin_loader.cpp # HTTP handlers - updates src/http/handlers/update_handlers.cpp + # Plugin framework + src/plugins/plugin_loader.cpp ) ament_target_dependencies(gateway_lib @@ -393,6 +395,20 @@ if(BUILD_TESTING) LIBRARY DESTINATION lib/${PROJECT_NAME} ) + # Test gateway plugin (.so for plugin loader tests) + add_library(test_gateway_plugin MODULE + test/demo_nodes/test_gateway_plugin.cpp + ) + target_link_libraries(test_gateway_plugin gateway_lib) + install(TARGETS test_gateway_plugin + LIBRARY DESTINATION lib/${PROJECT_NAME} + ) + + # Add plugin loader tests + ament_add_gtest(test_plugin_loader test/test_plugin_loader.cpp) + target_link_libraries(test_plugin_loader gateway_lib) + ament_target_dependencies(test_plugin_loader ament_index_cpp) + # Apply coverage flags to test targets if(ENABLE_COVERAGE) set(_test_targets @@ -423,6 +439,7 @@ if(BUILD_TESTING) test_data_handlers test_auth_handlers test_health_handlers + test_plugin_loader ) foreach(_target ${_test_targets}) target_compile_options(${_target} PRIVATE --coverage -O0 -g) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp new file mode 100644 index 00000000..3e7a3f0f --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp @@ -0,0 +1,45 @@ +// 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/plugins/gateway_plugin.hpp" + +#include +#include + +#include + +namespace ros2_medkit_gateway { + +/// Result of loading a gateway plugin: plugin instance + dlopen handle (for cleanup) +struct GatewayPluginLoadResult { + std::unique_ptr plugin; + void * handle = nullptr; // dlopen handle, caller owns dlclose +}; + +/** + * @brief Loads a GatewayPlugin from a shared library (.so). + * + * The .so must export: + * extern "C" int plugin_api_version(); // must return PLUGIN_API_VERSION + * extern "C" GatewayPlugin* create_plugin(); // factory + */ +class PluginLoader { + public: + /// Load plugin from .so path. Returns plugin + handle, or error string. + static tl::expected load(const std::string & plugin_path); +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp new file mode 100644 index 00000000..b278c4af --- /dev/null +++ b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp @@ -0,0 +1,90 @@ +// 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/plugins/plugin_loader.hpp" + +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" + +#include + +namespace ros2_medkit_gateway { + +tl::expected PluginLoader::load(const std::string & plugin_path) { + void * handle = dlopen(plugin_path.c_str(), RTLD_LAZY); + if (!handle) { + return tl::make_unexpected("Failed to load plugin '" + plugin_path + "': " + std::string(dlerror())); + } + + // Clear any existing error + dlerror(); + + // --- Check API version --- + using VersionFn = int (*)(); + auto version_fn = reinterpret_cast(dlsym(handle, "plugin_api_version")); + + const char * error = dlerror(); + if (error) { + dlclose(handle); + return tl::make_unexpected("Failed to find 'plugin_api_version' in '" + plugin_path + "': " + std::string(error)); + } + + int version = 0; + try { + version = version_fn(); + } catch (...) { + dlclose(handle); + return tl::make_unexpected("'plugin_api_version' threw exception in '" + plugin_path + "'"); + } + + if (version != PLUGIN_API_VERSION) { + dlclose(handle); + return tl::make_unexpected("API version mismatch in '" + plugin_path + "': plugin=" + std::to_string(version) + + " expected=" + std::to_string(PLUGIN_API_VERSION)); + } + + // --- Call factory --- + dlerror(); + + using FactoryFn = GatewayPlugin * (*)(); + auto factory = reinterpret_cast(dlsym(handle, "create_plugin")); + + error = dlerror(); + if (error) { + dlclose(handle); + return tl::make_unexpected("Failed to find 'create_plugin' in '" + plugin_path + "': " + std::string(error)); + } + + GatewayPlugin * raw_plugin = nullptr; + try { + raw_plugin = factory(); + } catch (const std::exception & e) { + dlclose(handle); + return tl::make_unexpected("Factory 'create_plugin' threw exception in '" + plugin_path + "': " + e.what()); + } catch (...) { + dlclose(handle); + return tl::make_unexpected("Factory 'create_plugin' threw unknown exception in '" + plugin_path + "'"); + } + + if (!raw_plugin) { + dlclose(handle); + return tl::make_unexpected("Factory 'create_plugin' returned null in '" + plugin_path + "'"); + } + + GatewayPluginLoadResult result; + result.plugin = std::unique_ptr(raw_plugin); + result.handle = handle; + return result; +} + +} // namespace ros2_medkit_gateway 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 new file mode 100644 index 00000000..9b0265d0 --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp @@ -0,0 +1,90 @@ +// 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/plugins/gateway_plugin.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" + +#include +#include +#include +#include + +using namespace ros2_medkit_gateway; + +/** + * @brief Test gateway plugin implementing all provider interfaces. + * + * Used by test_plugin_loader to verify dlopen/dlsym-based loading, + * API version checking, and dynamic_cast to provider interfaces. + */ +class TestGatewayPlugin : public GatewayPlugin, public UpdateProvider, public IntrospectionProvider { + public: + // --- GatewayPlugin --- + std::string name() const override { + return "test_plugin"; + } + + void configure(const nlohmann::json & /*config*/) override { + } + + void register_routes(httplib::Server & server, const std::string & api_prefix) override { + server.Get((api_prefix + "/x-test/ping").c_str(), [](const httplib::Request &, httplib::Response & res) { + res.set_content("pong", "text/plain"); + }); + } + + // --- UpdateProvider --- + tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { + return std::vector{}; + } + + tl::expected get_update(const std::string & id) override { + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::NotFound, "not found: " + id}); + } + + tl::expected register_update(const nlohmann::json &) override { + return {}; + } + + tl::expected delete_update(const std::string &) override { + return {}; + } + + tl::expected prepare(const std::string &, UpdateProgressReporter &) override { + return {}; + } + + tl::expected execute(const std::string &, UpdateProgressReporter &) override { + return {}; + } + + tl::expected supports_automated(const std::string &) override { + return false; + } + + // --- IntrospectionProvider --- + IntrospectionResult introspect(const IntrospectionInput &) override { + return {}; + } +}; + +extern "C" int plugin_api_version() { + return PLUGIN_API_VERSION; +} + +extern "C" GatewayPlugin * create_plugin() { + return new TestGatewayPlugin(); +} diff --git a/src/ros2_medkit_gateway/test/test_plugin_loader.cpp b/src/ros2_medkit_gateway/test/test_plugin_loader.cpp new file mode 100644 index 00000000..8280cae5 --- /dev/null +++ b/src/ros2_medkit_gateway/test/test_plugin_loader.cpp @@ -0,0 +1,73 @@ +// 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/plugins/plugin_loader.hpp" +#include "ros2_medkit_gateway/providers/introspection_provider.hpp" +#include "ros2_medkit_gateway/providers/update_provider.hpp" + +#include + +#include +#include + +using namespace ros2_medkit_gateway; + +namespace { + +std::string test_plugin_path() { + auto prefix = ament_index_cpp::get_package_prefix("ros2_medkit_gateway"); + return prefix + "/lib/ros2_medkit_gateway/libtest_gateway_plugin.so"; +} + +} // namespace + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, LoadsValidPlugin) { + auto result = PluginLoader::load(test_plugin_path()); + ASSERT_TRUE(result.has_value()) << result.error(); + EXPECT_NE(result->plugin, nullptr); + EXPECT_EQ(result->plugin->name(), "test_plugin"); + EXPECT_NE(result->handle, nullptr); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, LoadedPluginImplementsUpdateProvider) { + auto result = PluginLoader::load(test_plugin_path()); + ASSERT_TRUE(result.has_value()) << result.error(); + auto * update = dynamic_cast(result->plugin.get()); + EXPECT_NE(update, nullptr); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, LoadedPluginImplementsIntrospectionProvider) { + auto result = PluginLoader::load(test_plugin_path()); + ASSERT_TRUE(result.has_value()) << result.error(); + auto * introspection = dynamic_cast(result->plugin.get()); + EXPECT_NE(introspection, nullptr); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsNonexistentFile) { + auto result = PluginLoader::load("/nonexistent/path/to/plugin.so"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("Failed to load plugin"), std::string::npos); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsMissingFactory) { + // libc.so.6 is a valid .so but has no plugin_api_version symbol + auto result = PluginLoader::load("libc.so.6"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("plugin_api_version"), std::string::npos); +} From 9f2273014b4940e6fd0e63a360140f3ec1712b4c Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 19:00:57 +0000 Subject: [PATCH 05/14] fix(plugins): address review critical blockers in plugin loader Replace dynamic_cast with extern "C" provider query functions to avoid RTTI failures across dlopen boundaries. Add RAII to GatewayPluginLoadResult with correct destruction order (providers, plugin, dlclose) and move-only semantics. Add path validation (absolute, .so extension, canonical). Switch RTLD_LAZY to RTLD_NOW. Add GATEWAY_PLUGIN_EXPORT visibility macro. Replace non-portable libc.so.6 test with dedicated test plugins. Expand test coverage from 5 to 10 cases (version mismatch, null factory, missing factory symbol, path validation). Refs #235 --- src/ros2_medkit_gateway/CMakeLists.txt | 8 ++ .../plugins/plugin_loader.hpp | 41 ++++++- .../plugins/plugin_types.hpp | 8 ++ .../src/plugins/plugin_loader.cpp | 116 +++++++++++++++--- .../demo_nodes/test_bad_version_plugin.cpp | 26 ++++ .../test/demo_nodes/test_gateway_plugin.cpp | 16 ++- .../demo_nodes/test_no_symbols_plugin.cpp | 21 ++++ .../demo_nodes/test_null_factory_plugin.cpp | 26 ++++ .../demo_nodes/test_version_only_plugin.cpp | 23 ++++ .../test/test_plugin_loader.cpp | 68 +++++++--- 10 files changed, 315 insertions(+), 38 deletions(-) create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_no_symbols_plugin.cpp create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_version_only_plugin.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index e4777dec..4557ea81 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -404,6 +404,14 @@ if(BUILD_TESTING) LIBRARY DESTINATION lib/${PROJECT_NAME} ) + # Minimal test plugins for error-path coverage + foreach(_plugin test_bad_version_plugin test_no_symbols_plugin test_null_factory_plugin test_version_only_plugin) + add_library(${_plugin} MODULE test/demo_nodes/${_plugin}.cpp) + target_include_directories(${_plugin} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_link_libraries(${_plugin} nlohmann_json::nlohmann_json) + install(TARGETS ${_plugin} LIBRARY DESTINATION lib/${PROJECT_NAME}) + endforeach() + # Add plugin loader tests ament_add_gtest(test_plugin_loader test/test_plugin_loader.cpp) target_link_libraries(test_plugin_loader gateway_lib) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp index 3e7a3f0f..0f732efd 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp @@ -23,18 +23,53 @@ namespace ros2_medkit_gateway { -/// Result of loading a gateway plugin: plugin instance + dlopen handle (for cleanup) +class UpdateProvider; +class IntrospectionProvider; + +/** + * @brief Result of loading a gateway plugin. + * + * RAII wrapper guaranteeing correct destruction order: provider pointers + * are invalidated first, then the plugin is destroyed, then dlclose is called. + * Move-only (no copy). + */ struct GatewayPluginLoadResult { + GatewayPluginLoadResult() = default; + ~GatewayPluginLoadResult(); + + GatewayPluginLoadResult(GatewayPluginLoadResult && other) noexcept; + GatewayPluginLoadResult & operator=(GatewayPluginLoadResult && other) noexcept; + + GatewayPluginLoadResult(const GatewayPluginLoadResult &) = delete; + GatewayPluginLoadResult & operator=(const GatewayPluginLoadResult &) = delete; + std::unique_ptr plugin; - void * handle = nullptr; // dlopen handle, caller owns dlclose + + /// Non-owning pointer to UpdateProvider interface (null if plugin doesn't provide updates). + /// Lifetime tied to plugin - do not use after plugin is destroyed. + UpdateProvider * update_provider = nullptr; + + /// Non-owning pointer to IntrospectionProvider interface (null if not provided). + /// Lifetime tied to plugin - do not use after plugin is destroyed. + IntrospectionProvider * introspection_provider = nullptr; + + private: + friend class PluginLoader; + void * handle_ = nullptr; // dlopen handle, destroyed after plugin }; /** * @brief Loads a GatewayPlugin from a shared library (.so). * - * The .so must export: + * The .so must export (with GATEWAY_PLUGIN_EXPORT visibility): * extern "C" int plugin_api_version(); // must return PLUGIN_API_VERSION * extern "C" GatewayPlugin* create_plugin(); // factory + * + * Optionally, for provider interface discovery (avoids RTTI across dlopen boundary): + * extern "C" UpdateProvider* get_update_provider(GatewayPlugin* plugin); + * extern "C" IntrospectionProvider* get_introspection_provider(GatewayPlugin* plugin); + * + * Path requirements: must be absolute, have .so extension, and resolve to a real file. */ class PluginLoader { public: diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp index 996abea6..fdf0f99b 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp @@ -17,6 +17,14 @@ #include #include +/// Visibility macro for plugin extern "C" exports. +/// Ensures symbols are exported even with -fvisibility=hidden builds. +#ifdef _WIN32 +#define GATEWAY_PLUGIN_EXPORT __declspec(dllexport) +#else +#define GATEWAY_PLUGIN_EXPORT __attribute__((visibility("default"))) +#endif + namespace ros2_medkit_gateway { /// Current plugin API version. Plugins must export this value from plugin_api_version(). diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp index b278c4af..ec38c7ab 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp @@ -15,17 +15,88 @@ #include "ros2_medkit_gateway/plugins/plugin_loader.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" #include +#include + namespace ros2_medkit_gateway { +// --- GatewayPluginLoadResult RAII --- + +GatewayPluginLoadResult::~GatewayPluginLoadResult() { + update_provider = nullptr; + introspection_provider = nullptr; + plugin.reset(); + if (handle_) { + dlclose(handle_); + } +} + +GatewayPluginLoadResult::GatewayPluginLoadResult(GatewayPluginLoadResult && other) noexcept + : plugin(std::move(other.plugin)) + , update_provider(other.update_provider) + , introspection_provider(other.introspection_provider) + , handle_(other.handle_) { + other.update_provider = nullptr; + other.introspection_provider = nullptr; + other.handle_ = nullptr; +} + +GatewayPluginLoadResult & GatewayPluginLoadResult::operator=(GatewayPluginLoadResult && other) noexcept { + if (this != &other) { + // Destroy current state in correct order + update_provider = nullptr; + introspection_provider = nullptr; + plugin.reset(); + if (handle_) { + dlclose(handle_); + } + + // Move from other + plugin = std::move(other.plugin); + update_provider = other.update_provider; + introspection_provider = other.introspection_provider; + handle_ = other.handle_; + + other.update_provider = nullptr; + other.introspection_provider = nullptr; + other.handle_ = nullptr; + } + return *this; +} + +// --- PluginLoader --- + tl::expected PluginLoader::load(const std::string & plugin_path) { - void * handle = dlopen(plugin_path.c_str(), RTLD_LAZY); + // --- Validate path --- + std::filesystem::path fs_path(plugin_path); + + if (!fs_path.is_absolute()) { + return tl::make_unexpected("Plugin path must be absolute: " + plugin_path); + } + + if (fs_path.extension() != ".so") { + return tl::make_unexpected("Plugin path must have .so extension: " + plugin_path); + } + + std::error_code ec; + auto canonical_path = std::filesystem::canonical(fs_path, ec); + if (ec) { + return tl::make_unexpected("Plugin path does not exist or is not accessible: " + plugin_path); + } + + // --- dlopen --- + void * handle = dlopen(canonical_path.c_str(), RTLD_NOW); if (!handle) { - return tl::make_unexpected("Failed to load plugin '" + plugin_path + "': " + std::string(dlerror())); + return tl::make_unexpected("Failed to load plugin: " + std::string(dlerror())); } + // Scope guard: dlclose on any error path. Released on success. + auto handle_guard = std::unique_ptr(handle, dlclose); + // Clear any existing error dlerror(); @@ -35,22 +106,20 @@ tl::expected PluginLoader::load(const std: const char * error = dlerror(); if (error) { - dlclose(handle); - return tl::make_unexpected("Failed to find 'plugin_api_version' in '" + plugin_path + "': " + std::string(error)); + return tl::make_unexpected(std::string("Failed to find 'plugin_api_version': ") + std::string(error)); } int version = 0; try { version = version_fn(); } catch (...) { - dlclose(handle); - return tl::make_unexpected("'plugin_api_version' threw exception in '" + plugin_path + "'"); + return tl::make_unexpected("'plugin_api_version' threw exception in plugin"); } if (version != PLUGIN_API_VERSION) { - dlclose(handle); - return tl::make_unexpected("API version mismatch in '" + plugin_path + "': plugin=" + std::to_string(version) + - " expected=" + std::to_string(PLUGIN_API_VERSION)); + return tl::make_unexpected("API version mismatch: plugin=" + std::to_string(version) + + " expected=" + std::to_string(PLUGIN_API_VERSION) + + ". Rebuild the plugin against matching gateway headers."); } // --- Call factory --- @@ -61,29 +130,40 @@ tl::expected PluginLoader::load(const std: error = dlerror(); if (error) { - dlclose(handle); - return tl::make_unexpected("Failed to find 'create_plugin' in '" + plugin_path + "': " + std::string(error)); + return tl::make_unexpected(std::string("Failed to find 'create_plugin': ") + std::string(error)); } GatewayPlugin * raw_plugin = nullptr; try { raw_plugin = factory(); } catch (const std::exception & e) { - dlclose(handle); - return tl::make_unexpected("Factory 'create_plugin' threw exception in '" + plugin_path + "': " + e.what()); + return tl::make_unexpected(std::string("Factory 'create_plugin' threw exception: ") + e.what()); } catch (...) { - dlclose(handle); - return tl::make_unexpected("Factory 'create_plugin' threw unknown exception in '" + plugin_path + "'"); + return tl::make_unexpected("Factory 'create_plugin' threw unknown exception"); } if (!raw_plugin) { - dlclose(handle); - return tl::make_unexpected("Factory 'create_plugin' returned null in '" + plugin_path + "'"); + return tl::make_unexpected("Factory 'create_plugin' returned null"); } + // --- Query provider interfaces via extern "C" (no RTTI across dlopen boundary) --- GatewayPluginLoadResult result; result.plugin = std::unique_ptr(raw_plugin); - result.handle = handle; + + using UpdateProviderFn = UpdateProvider * (*)(GatewayPlugin *); + auto update_fn = reinterpret_cast(dlsym(handle, "get_update_provider")); + if (update_fn) { + result.update_provider = update_fn(raw_plugin); + } + + using IntrospectionProviderFn = IntrospectionProvider * (*)(GatewayPlugin *); + auto introspection_fn = reinterpret_cast(dlsym(handle, "get_introspection_provider")); + if (introspection_fn) { + result.introspection_provider = introspection_fn(raw_plugin); + } + + // Transfer handle ownership to result (disarm scope guard) + result.handle_ = handle_guard.release(); return result; } diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp new file mode 100644 index 00000000..44be3716 --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp @@ -0,0 +1,26 @@ +// 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. + +/// Test plugin that exports a wrong API version (PLUGIN_API_VERSION + 1). +/// Used by test_plugin_loader to verify version mismatch rejection. + +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" + +extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return ros2_medkit_gateway::PLUGIN_API_VERSION + 1; +} + +extern "C" GATEWAY_PLUGIN_EXPORT void * create_plugin() { + return nullptr; +} 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 9b0265d0..5f48f73f 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 @@ -28,7 +28,7 @@ using namespace ros2_medkit_gateway; * @brief Test gateway plugin implementing all provider interfaces. * * Used by test_plugin_loader to verify dlopen/dlsym-based loading, - * API version checking, and dynamic_cast to provider interfaces. + * API version checking, and extern "C" provider query functions. */ class TestGatewayPlugin : public GatewayPlugin, public UpdateProvider, public IntrospectionProvider { public: @@ -81,10 +81,20 @@ class TestGatewayPlugin : public GatewayPlugin, public UpdateProvider, public In } }; -extern "C" int plugin_api_version() { +// --- Plugin exports (with visibility macros) --- + +extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { return PLUGIN_API_VERSION; } -extern "C" GatewayPlugin * create_plugin() { +extern "C" GATEWAY_PLUGIN_EXPORT GatewayPlugin * create_plugin() { return new TestGatewayPlugin(); } + +extern "C" GATEWAY_PLUGIN_EXPORT UpdateProvider * get_update_provider(GatewayPlugin * plugin) { + return static_cast(plugin); +} + +extern "C" GATEWAY_PLUGIN_EXPORT IntrospectionProvider * get_introspection_provider(GatewayPlugin * plugin) { + return static_cast(plugin); +} diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_no_symbols_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_no_symbols_plugin.cpp new file mode 100644 index 00000000..dbb7fafc --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_no_symbols_plugin.cpp @@ -0,0 +1,21 @@ +// 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. + +/// Trivial shared library with no plugin symbols. +/// Used by test_plugin_loader to verify missing symbol detection. +/// Replaces non-portable libc.so.6 test. + +extern "C" int dummy_function() { + return 42; +} diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp new file mode 100644 index 00000000..32891955 --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp @@ -0,0 +1,26 @@ +// 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. + +/// Test plugin with correct version but create_plugin() returns nullptr. +/// Used by test_plugin_loader to verify null factory rejection. + +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" + +extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return ros2_medkit_gateway::PLUGIN_API_VERSION; +} + +extern "C" GATEWAY_PLUGIN_EXPORT void * create_plugin() { + return nullptr; +} diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_version_only_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_version_only_plugin.cpp new file mode 100644 index 00000000..9d57f9d1 --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_version_only_plugin.cpp @@ -0,0 +1,23 @@ +// 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. + +/// Test plugin exporting plugin_api_version() but NOT create_plugin(). +/// Used by test_plugin_loader to verify missing factory symbol detection +/// (separate from missing version symbol). + +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" + +extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return ros2_medkit_gateway::PLUGIN_API_VERSION; +} diff --git a/src/ros2_medkit_gateway/test/test_plugin_loader.cpp b/src/ros2_medkit_gateway/test/test_plugin_loader.cpp index 8280cae5..0f52d344 100644 --- a/src/ros2_medkit_gateway/test/test_plugin_loader.cpp +++ b/src/ros2_medkit_gateway/test/test_plugin_loader.cpp @@ -22,52 +22,92 @@ #include using namespace ros2_medkit_gateway; - namespace { +std::string plugin_lib_dir() { + return ament_index_cpp::get_package_prefix("ros2_medkit_gateway") + "/lib/ros2_medkit_gateway/"; +} + std::string test_plugin_path() { - auto prefix = ament_index_cpp::get_package_prefix("ros2_medkit_gateway"); - return prefix + "/lib/ros2_medkit_gateway/libtest_gateway_plugin.so"; + return plugin_lib_dir() + "libtest_gateway_plugin.so"; } } // namespace +// --- Happy path --- + // @verifies REQ_INTEROP_012 TEST(TestPluginLoader, LoadsValidPlugin) { auto result = PluginLoader::load(test_plugin_path()); ASSERT_TRUE(result.has_value()) << result.error(); EXPECT_NE(result->plugin, nullptr); EXPECT_EQ(result->plugin->name(), "test_plugin"); - EXPECT_NE(result->handle, nullptr); } // @verifies REQ_INTEROP_012 -TEST(TestPluginLoader, LoadedPluginImplementsUpdateProvider) { +TEST(TestPluginLoader, DiscoverUpdateProviderViaExternC) { auto result = PluginLoader::load(test_plugin_path()); ASSERT_TRUE(result.has_value()) << result.error(); - auto * update = dynamic_cast(result->plugin.get()); - EXPECT_NE(update, nullptr); + EXPECT_NE(result->update_provider, nullptr); } // @verifies REQ_INTEROP_012 -TEST(TestPluginLoader, LoadedPluginImplementsIntrospectionProvider) { +TEST(TestPluginLoader, DiscoverIntrospectionProviderViaExternC) { auto result = PluginLoader::load(test_plugin_path()); ASSERT_TRUE(result.has_value()) << result.error(); - auto * introspection = dynamic_cast(result->plugin.get()); - EXPECT_NE(introspection, nullptr); + EXPECT_NE(result->introspection_provider, nullptr); } +// --- Path validation --- + // @verifies REQ_INTEROP_012 TEST(TestPluginLoader, RejectsNonexistentFile) { auto result = PluginLoader::load("/nonexistent/path/to/plugin.so"); ASSERT_FALSE(result.has_value()); - EXPECT_NE(result.error().find("Failed to load plugin"), std::string::npos); + EXPECT_NE(result.error().find("does not exist"), std::string::npos); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsRelativePath) { + auto result = PluginLoader::load("relative/path/plugin.so"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("must be absolute"), std::string::npos); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsNonSoExtension) { + auto result = PluginLoader::load("/tmp/plugin.dll"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find(".so extension"), std::string::npos); } +// --- Symbol validation --- + // @verifies REQ_INTEROP_012 -TEST(TestPluginLoader, RejectsMissingFactory) { - // libc.so.6 is a valid .so but has no plugin_api_version symbol - auto result = PluginLoader::load("libc.so.6"); +TEST(TestPluginLoader, RejectsMissingVersionSymbol) { + auto result = PluginLoader::load(plugin_lib_dir() + "libtest_no_symbols_plugin.so"); ASSERT_FALSE(result.has_value()); EXPECT_NE(result.error().find("plugin_api_version"), std::string::npos); } + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsVersionMismatch) { + auto result = PluginLoader::load(plugin_lib_dir() + "libtest_bad_version_plugin.so"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("version mismatch"), std::string::npos); + EXPECT_NE(result.error().find("Rebuild"), std::string::npos); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsMissingFactorySymbol) { + auto result = PluginLoader::load(plugin_lib_dir() + "libtest_version_only_plugin.so"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("create_plugin"), std::string::npos); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, RejectsNullFactory) { + auto result = PluginLoader::load(plugin_lib_dir() + "libtest_null_factory_plugin.so"); + ASSERT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("returned null"), std::string::npos); +} From 48176296ce8a7d2a1d61ac8993099c9e1efd7760 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 19:12:48 +0000 Subject: [PATCH 06/14] feat(plugins): add PluginManager with lifecycle and dispatch Orchestrates plugin loading, configuration, route registration, and shutdown. Uses extern "C" provider pointers from PluginLoader for .so plugins (safe across dlopen boundary) and dynamic_cast for compile-time test plugins (safe within same binary). Error isolation: throwing plugins are disabled, not crashed. 10 test cases covering empty state, dispatch, configure, shutdown, multi-provider, single provider, duplicate UpdateProvider, throwing plugin, and load failure. Refs #235 --- src/ros2_medkit_gateway/CMakeLists.txt | 6 + .../plugins/plugin_manager.hpp | 133 +++++++++++ .../src/plugins/plugin_manager.cpp | 209 ++++++++++++++++++ .../test/test_plugin_manager.cpp | 201 +++++++++++++++++ 4 files changed, 549 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp create mode 100644 src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp create mode 100644 src/ros2_medkit_gateway/test/test_plugin_manager.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 4557ea81..42b65ecf 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -135,6 +135,7 @@ add_library(gateway_lib STATIC src/http/handlers/update_handlers.cpp # Plugin framework src/plugins/plugin_loader.cpp + src/plugins/plugin_manager.cpp ) ament_target_dependencies(gateway_lib @@ -417,6 +418,10 @@ if(BUILD_TESTING) target_link_libraries(test_plugin_loader gateway_lib) ament_target_dependencies(test_plugin_loader ament_index_cpp) + # Plugin manager tests + ament_add_gtest(test_plugin_manager test/test_plugin_manager.cpp) + target_link_libraries(test_plugin_manager gateway_lib) + # Apply coverage flags to test targets if(ENABLE_COVERAGE) set(_test_targets @@ -448,6 +453,7 @@ if(BUILD_TESTING) test_auth_handlers test_health_handlers test_plugin_loader + test_plugin_manager ) foreach(_target ${_test_targets}) target_compile_options(${_target} PRIVATE --coverage -O0 -g) 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 new file mode 100644 index 00000000..96213cd6 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp @@ -0,0 +1,133 @@ +// 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/plugins/gateway_plugin.hpp" +#include "ros2_medkit_gateway/plugins/plugin_loader.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" + +#include +#include +#include +#include +#include + +namespace rclcpp { +class Node; +} + +namespace ros2_medkit_gateway { + +/** + * @brief Orchestrates loading, lifecycle, and dispatch of gateway plugins + * + * Owns all plugin instances and dlopen handles. Subsystem managers receive + * non-owning pointers via get_update_provider() / get_introspection_providers(). + * + * Provider discovery uses extern "C" query functions from loaded .so files + * (avoiding RTTI across dlopen boundary). For compile-time plugins added + * via add_plugin(), dynamic_cast is used (safe within the same binary). + * + * Error isolation: every call to plugin code is wrapped in try/catch. + * A failing plugin is disabled but does not crash the gateway. + * + * IMPORTANT: PluginManager must outlive all subsystem managers that hold + * non-owning provider pointers (e.g. UpdateManager). In GatewayNode, declare + * plugin_mgr_ BEFORE update_mgr_ so that destruction order is safe. + */ +class PluginManager { + public: + PluginManager() = default; + ~PluginManager(); + + // Non-copyable, non-movable (owns dlopen handles) + PluginManager(const PluginManager &) = delete; + PluginManager & operator=(const PluginManager &) = delete; + + /** + * @brief Add a plugin directly (for testing with compile-time plugins) + * + * Uses dynamic_cast for provider discovery (safe within same binary). + * + * @param plugin Plugin instance + */ + void add_plugin(std::unique_ptr plugin); + + /** + * @brief Load plugins from shared library paths + * @param configs Plugin configurations with paths and per-plugin config + * @return Number of successfully loaded plugins + */ + size_t load_plugins(const std::vector & configs); + + /** + * @brief Configure all loaded plugins + * + * Calls configure() on each plugin with its per-plugin config. + * Plugins that throw are disabled. + */ + void configure_plugins(); + + /** + * @brief Set ROS 2 node on all plugins + * @param node ROS 2 node pointer (must outlive all plugins) + */ + void set_node(rclcpp::Node * node); + + /** + * @brief Register custom REST routes from all plugins + * @param server httplib server instance + * @param api_prefix API path prefix (e.g., "/api/v1") + */ + void register_routes(httplib::Server & server, const std::string & api_prefix); + + /** + * @brief Shutdown all plugins + */ + void shutdown_all(); + + // ---- Dispatch to subsystem managers ---- + + /** + * @brief Get the update provider (first plugin implementing UpdateProvider) + * @return Non-owning pointer, or nullptr if no UpdateProvider plugin loaded + */ + UpdateProvider * get_update_provider() const; + + /** + * @brief Get all introspection providers + * @return Non-owning pointers to all IntrospectionProvider plugins + */ + std::vector get_introspection_providers() const; + + // ---- Info ---- + bool has_plugins() const; + std::vector plugin_names() const; + + private: + static void setup_plugin_logging(GatewayPlugin & plugin); + + struct LoadedPlugin { + GatewayPluginLoadResult load_result; + nlohmann::json config; + UpdateProvider * update_provider = nullptr; + IntrospectionProvider * introspection_provider = nullptr; + }; + std::vector plugins_; +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp new file mode 100644 index 00000000..4dbd60b9 --- /dev/null +++ b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp @@ -0,0 +1,209 @@ +// 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/plugins/plugin_manager.hpp" + +#include + +namespace ros2_medkit_gateway { + +namespace { +auto logger() { + return rclcpp::get_logger("plugin_manager"); +} +} // namespace + +void PluginManager::setup_plugin_logging(GatewayPlugin & plugin) { + std::string plugin_name = plugin.name(); + plugin.set_logger([plugin_name](PluginLogLevel level, const std::string & msg) { + auto log = rclcpp::get_logger("plugin." + plugin_name); + switch (level) { + case PluginLogLevel::kInfo: + RCLCPP_INFO(log, "%s", msg.c_str()); + break; + case PluginLogLevel::kWarn: + RCLCPP_WARN(log, "%s", msg.c_str()); + break; + case PluginLogLevel::kError: + RCLCPP_ERROR(log, "%s", msg.c_str()); + break; + } + }); +} + +PluginManager::~PluginManager() { + shutdown_all(); + // GatewayPluginLoadResult RAII handles destruction order + plugins_.clear(); +} + +void PluginManager::add_plugin(std::unique_ptr plugin) { + LoadedPlugin lp; + lp.config = nlohmann::json::object(); + + // For in-process plugins, use dynamic_cast (safe within same binary) + lp.update_provider = dynamic_cast(plugin.get()); + lp.introspection_provider = dynamic_cast(plugin.get()); + + setup_plugin_logging(*plugin); + lp.load_result.plugin = std::move(plugin); + plugins_.push_back(std::move(lp)); +} + +size_t PluginManager::load_plugins(const std::vector & configs) { + size_t loaded = 0; + for (const auto & cfg : configs) { + auto result = PluginLoader::load(cfg.path); + if (result) { + RCLCPP_INFO(logger(), "Loaded plugin '%s' from %s", result->plugin->name().c_str(), cfg.path.c_str()); + + LoadedPlugin lp; + lp.config = cfg.config; + + // Provider pointers from extern "C" query functions (safe across dlopen boundary) + lp.update_provider = result->update_provider; + lp.introspection_provider = result->introspection_provider; + + setup_plugin_logging(*result->plugin); + lp.load_result = std::move(*result); + plugins_.push_back(std::move(lp)); + ++loaded; + } else { + RCLCPP_ERROR(logger(), "Failed to load plugin from %s: %s", cfg.path.c_str(), result.error().c_str()); + } + } + return loaded; +} + +void PluginManager::configure_plugins() { + for (auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + try { + lp.load_result.plugin->configure(lp.config); + } catch (const std::exception & e) { + RCLCPP_ERROR(logger(), "Plugin '%s' threw during configure(): %s", lp.load_result.plugin->name().c_str(), + e.what()); + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.plugin.reset(); + } catch (...) { + RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during configure()", + lp.load_result.plugin->name().c_str()); + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.plugin.reset(); + } + } +} + +void PluginManager::set_node(rclcpp::Node * node) { + for (auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + try { + lp.load_result.plugin->set_node(node); + } catch (const std::exception & e) { + RCLCPP_ERROR(logger(), "Plugin '%s' threw during set_node(): %s", lp.load_result.plugin->name().c_str(), + e.what()); + } catch (...) { + RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during set_node()", + lp.load_result.plugin->name().c_str()); + } + } +} + +void PluginManager::register_routes(httplib::Server & server, const std::string & api_prefix) { + for (auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + try { + lp.load_result.plugin->register_routes(server, api_prefix); + } catch (const std::exception & e) { + RCLCPP_ERROR(logger(), "Plugin '%s' threw during register_routes(): %s", lp.load_result.plugin->name().c_str(), + e.what()); + } catch (...) { + RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during register_routes()", + lp.load_result.plugin->name().c_str()); + } + } +} + +void PluginManager::shutdown_all() { + for (auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + try { + lp.load_result.plugin->shutdown(); + } catch (...) { + // Best effort shutdown + } + } +} + +UpdateProvider * PluginManager::get_update_provider() const { + UpdateProvider * first = nullptr; + for (const auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + if (lp.update_provider) { + if (!first) { + first = lp.update_provider; + } else { + RCLCPP_WARN(logger(), "Multiple UpdateProvider plugins loaded - ignoring '%s'", + lp.load_result.plugin->name().c_str()); + } + } + } + return first; +} + +std::vector PluginManager::get_introspection_providers() const { + std::vector result; + for (const auto & lp : plugins_) { + if (!lp.load_result.plugin) { + continue; + } + if (lp.introspection_provider) { + result.push_back(lp.introspection_provider); + } + } + return result; +} + +bool PluginManager::has_plugins() const { + for (const auto & lp : plugins_) { + if (lp.load_result.plugin) { + return true; + } + } + return false; +} + +std::vector PluginManager::plugin_names() const { + std::vector names; + for (const auto & lp : plugins_) { + if (lp.load_result.plugin) { + names.push_back(lp.load_result.plugin->name()); + } + } + return names; +} + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp new file mode 100644 index 00000000..755e2f4d --- /dev/null +++ b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp @@ -0,0 +1,201 @@ +// 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 + +#include "ros2_medkit_gateway/plugins/plugin_manager.hpp" + +using namespace ros2_medkit_gateway; +using json = nlohmann::json; + +/// Minimal mock plugin for compile-time testing (no .so needed) +class MockPlugin : public GatewayPlugin, public UpdateProvider, public IntrospectionProvider { + public: + std::string name() const override { + return "mock"; + } + void configure(const json & config) override { + configured_ = true; + config_ = config; + } + void shutdown() override { + shutdown_called_ = true; + } + + // UpdateProvider + tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { + return std::vector{}; + } + tl::expected get_update(const std::string &) override { + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::NotFound, "mock"}); + } + tl::expected register_update(const json &) override { + return {}; + } + tl::expected delete_update(const std::string &) override { + return {}; + } + tl::expected prepare(const std::string &, UpdateProgressReporter &) override { + return {}; + } + tl::expected execute(const std::string &, UpdateProgressReporter &) override { + return {}; + } + tl::expected supports_automated(const std::string &) override { + return false; + } + + // IntrospectionProvider + IntrospectionResult introspect(const IntrospectionInput &) override { + return {}; + } + + bool configured_ = false; + bool shutdown_called_ = false; + json config_; +}; + +/// Plugin that only provides IntrospectionProvider +class MockIntrospectionOnly : public GatewayPlugin, public IntrospectionProvider { + public: + std::string name() const override { + return "introspection_only"; + } + void configure(const json &) override { + } + IntrospectionResult introspect(const IntrospectionInput &) override { + return {}; + } +}; + +/// Plugin that throws during configure +class MockThrowingPlugin : public GatewayPlugin { + public: + std::string name() const override { + return "throwing"; + } + void configure(const json &) override { + throw std::runtime_error("configure failed"); + } +}; + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, EmptyManagerHasNoPlugins) { + PluginManager mgr; + EXPECT_FALSE(mgr.has_plugins()); + EXPECT_TRUE(mgr.plugin_names().empty()); + EXPECT_EQ(mgr.get_update_provider(), nullptr); + EXPECT_TRUE(mgr.get_introspection_providers().empty()); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, AddPluginAndDispatch) { + PluginManager mgr; + auto plugin = std::make_unique(); + auto * raw = plugin.get(); + mgr.add_plugin(std::move(plugin)); + + EXPECT_TRUE(mgr.has_plugins()); + EXPECT_EQ(mgr.plugin_names().size(), 1u); + EXPECT_EQ(mgr.plugin_names()[0], "mock"); + + EXPECT_EQ(mgr.get_update_provider(), static_cast(raw)); + EXPECT_EQ(mgr.get_introspection_providers().size(), 1u); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ConfigurePassesConfig) { + PluginManager mgr; + auto plugin = std::make_unique(); + auto * raw = plugin.get(); + mgr.add_plugin(std::move(plugin)); + + mgr.configure_plugins(); + EXPECT_TRUE(raw->configured_); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ShutdownCallsAllPlugins) { + PluginManager mgr; + auto plugin = std::make_unique(); + auto * raw = plugin.get(); + mgr.add_plugin(std::move(plugin)); + + mgr.shutdown_all(); + EXPECT_TRUE(raw->shutdown_called_); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, MultiCapabilityPluginDispatchedToBoth) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + + EXPECT_NE(mgr.get_update_provider(), nullptr); + EXPECT_EQ(mgr.get_introspection_providers().size(), 1u); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, IntrospectionOnlyPluginNotUpdateProvider) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + + EXPECT_EQ(mgr.get_update_provider(), nullptr); + EXPECT_EQ(mgr.get_introspection_providers().size(), 1u); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, MultipleIntrospectionProviders) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + mgr.add_plugin(std::make_unique()); + + EXPECT_EQ(mgr.get_introspection_providers().size(), 2u); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, DuplicateUpdateProviderFirstWins) { + PluginManager mgr; + auto first = std::make_unique(); + auto * first_raw = first.get(); + mgr.add_plugin(std::move(first)); + mgr.add_plugin(std::make_unique()); + + // First UpdateProvider wins + EXPECT_EQ(mgr.get_update_provider(), static_cast(first_raw)); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ThrowingPluginDisabledDuringConfigure) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + auto good = std::make_unique(); + auto * good_raw = good.get(); + mgr.add_plugin(std::move(good)); + + // Should not throw + mgr.configure_plugins(); + + // Good plugin still works + EXPECT_TRUE(good_raw->configured_); + EXPECT_NE(mgr.get_update_provider(), nullptr); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, LoadNonexistentPluginReturnsZero) { + PluginManager mgr; + std::vector configs = {{"nonexistent", "/nonexistent/path.so", json::object()}}; + auto loaded = mgr.load_plugins(configs); + EXPECT_EQ(loaded, 0u); + EXPECT_FALSE(mgr.has_plugins()); +} From c7fcc71634ffc771bb26e4d7e167dc0be86cb897 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 19:38:43 +0000 Subject: [PATCH 07/14] refactor(updates): migrate UpdateBackend to UpdateProvider Rename update_backend.hpp to update_types.hpp (types-only header). Remove the UpdateBackend abstract class - UpdateProvider replaces it. UpdateManager now takes non-owning UpdateProvider* via set_backend() instead of owning unique_ptr. PluginManager owns the plugin lifecycle and dlopen handles. Old UpdatePluginLoader removed. test_update_backend.cpp migrated to new plugin framework exports. All 27 update manager tests pass with the new interface. Refs #235 --- src/ros2_medkit_gateway/CMakeLists.txt | 4 +- .../config/gateway_params.yaml | 15 +- .../providers/update_provider.hpp | 5 +- .../updates/plugin_loader.hpp | 43 ----- .../updates/update_backend.hpp | 161 ------------------ .../updates/update_manager.hpp | 16 +- .../updates/update_types.hpp | 94 ++++++++++ src/ros2_medkit_gateway/src/gateway_node.cpp | 29 +--- .../src/updates/plugin_loader.cpp | 61 ------- .../src/updates/update_manager.cpp | 15 +- .../test/demo_nodes/test_update_backend.cpp | 33 +++- .../test/test_update_manager.cpp | 45 +++-- 12 files changed, 178 insertions(+), 343 deletions(-) delete mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp delete mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_types.hpp delete mode 100644 src/ros2_medkit_gateway/src/updates/plugin_loader.cpp diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 42b65ecf..1b6e66aa 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -130,7 +130,6 @@ add_library(gateway_lib STATIC src/auth/auth_requirement_policy.cpp # Updates module src/updates/update_manager.cpp - src/updates/plugin_loader.cpp # HTTP handlers - updates src/http/handlers/update_handlers.cpp # Plugin framework @@ -391,6 +390,9 @@ if(BUILD_TESTING) target_link_libraries(test_update_backend nlohmann_json::nlohmann_json tl::expected + cpp_httplib_target + OpenSSL::SSL + OpenSSL::Crypto ) install(TARGETS test_update_backend LIBRARY DESTINATION lib/${PROJECT_NAME} diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index 1fc50522..fd8ff1cc 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -243,19 +243,8 @@ ros2_medkit_gateway: # When false, update routes are not registered # Default: false enabled: false - - # Backend type - # Options: - # - "none": Endpoints registered but return 501 Not Implemented - # - "plugin": Load update backend from shared library (.so) - # Default: "none" - backend: "none" - - # Path to update backend .so plugin - # Only used when backend: "plugin" - # The .so must export: extern "C" UpdateBackend* create_update_backend(); - # Example: "/opt/ros2_medkit/lib/libmy_update_backend.so" - plugin_path: "" + # Update backend is wired via the plugin framework. + # A plugin implementing UpdateProvider will be detected automatically. # Rate Limiting Configuration # Token-bucket-based rate limiting for API requests. diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp index 6c52cd3a..82c1f06f 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp @@ -14,8 +14,7 @@ #pragma once -// TODO(Task 5): rename this include to update_types.hpp when UpdateBackend is removed -#include "ros2_medkit_gateway/updates/update_backend.hpp" +#include "ros2_medkit_gateway/updates/update_types.hpp" namespace ros2_medkit_gateway { @@ -25,7 +24,7 @@ namespace ros2_medkit_gateway { * Typed provider interface that plugins implement alongside GatewayPlugin * via multiple inheritance. Replaces the old UpdateBackend abstract class. * - * Reuses all types from update_backend.hpp (UpdateFilter, UpdateBackendErrorInfo, + * Reuses all types from update_types.hpp (UpdateFilter, UpdateBackendErrorInfo, * UpdateProgressReporter, etc.). * * @see GatewayPlugin for the base class diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp deleted file mode 100644 index 38bdde68..00000000 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp +++ /dev/null @@ -1,43 +0,0 @@ -// 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 "ros2_medkit_gateway/updates/update_backend.hpp" - -namespace ros2_medkit_gateway { - -/// Result of loading a plugin: backend + dlopen handle (for cleanup) -struct PluginLoadResult { - std::unique_ptr backend; - void * handle = nullptr; // dlopen handle, pass to UpdateManager -}; - -/** - * @brief Loads an UpdateBackend plugin from a shared library (.so). - * - * The .so must export: extern "C" UpdateBackend* create_update_backend(); - */ -class UpdatePluginLoader { - public: - /// Load plugin from .so path. Returns backend + handle. - static tl::expected load(const std::string & plugin_path); -}; - -} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp deleted file mode 100644 index 3e47ce35..00000000 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp +++ /dev/null @@ -1,161 +0,0 @@ -// 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 -#include - -namespace ros2_medkit_gateway { - -/// Filter criteria for listing update packages -struct UpdateFilter { - std::optional origin; // "remote" | "proximity" - std::optional target_version; // Filter by target version -}; - -/// Status of an update operation -enum class UpdateStatus { Pending, InProgress, Completed, Failed }; - -/// Detailed progress for a sub-step of an update operation -struct UpdateSubProgress { - std::string name; - int progress; // 0-100 -}; - -/// Full status information for an update operation -struct UpdateStatusInfo { - UpdateStatus status = UpdateStatus::Pending; - std::optional progress; // 0-100 - std::optional> sub_progress; // Detailed per-step progress - std::optional error_message; // Set when status == Failed -}; - -/// Internal phase tracking for update lifecycle -enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed, Deleting }; - -/// Error codes for backend return values -enum class UpdateBackendError { - NotFound, // Package does not exist - AlreadyExists, // Duplicate ID on registration - InvalidInput, // Malformed metadata - Internal // Unexpected error -}; - -/// Typed error for backend return values -struct UpdateBackendErrorInfo { - UpdateBackendError code; - std::string message; -}; - -/** - * @brief Thread-safe reporter for update progress. - * - * Passed to UpdateBackend::prepare/execute. The plugin MAY use it to report - * fine-grained progress. If not used, UpdateManager still tracks base status - * (Pending -> InProgress -> Completed/Failed) automatically. - */ -class UpdateProgressReporter { - public: - UpdateProgressReporter(UpdateStatusInfo & status, std::mutex & mutex) : status_(status), mutex_(mutex) { - } - - void set_progress(int percent) { - std::lock_guard lock(mutex_); - status_.progress = percent; - } - - void set_sub_progress(std::vector steps) { - std::lock_guard lock(mutex_); - status_.sub_progress = std::move(steps); - } - - private: - UpdateStatusInfo & status_; - std::mutex & mutex_; -}; - -/** - * @brief Abstract base class for software update backends (plugin interface). - * - * Implementations handle the actual update logic: metadata storage, package - * preparation, and execution. The gateway's UpdateManager handles async - * lifecycle and status tracking. - * - * Plugins can be loaded at compile-time (subclass and pass to UpdateManager) - * or at runtime (.so loaded via dlopen with extern "C" factory function). - * - * For runtime loading, the .so must export: - * extern "C" UpdateBackend* create_update_backend(); - * - * @par Thread Safety - * - CRUD methods (list_updates, get_update, register_update, delete_update) - * are called WITHOUT holding UpdateManager's mutex. They may be called - * concurrently with each other and with prepare/execute running in a - * background thread. If the backend shares state, it must provide its own - * synchronization. - * Note: delete_update has a partial guard - UpdateManager checks/sets a - * Deleting sentinel under lock before calling delete_update, but the - * backend call itself runs outside the lock. - * - prepare() and execute() run in a background std::async thread. They may - * run concurrently with CRUD calls from the HTTP thread. - * - The UpdateProgressReporter passed to prepare/execute is already - * thread-safe - plugins may call set_progress/set_sub_progress freely. - * - Exceptions thrown from prepare/execute are caught by UpdateManager and - * mapped to Failed status. Plugins should prefer returning - * tl::make_unexpected() for expected errors. - */ -class UpdateBackend { - public: - virtual ~UpdateBackend() = default; - - // ---- CRUD (plugin owns all metadata storage) ---- - - /// List all registered update package IDs, optionally filtered - virtual tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & filter) = 0; - - /// Get full metadata for a specific update package as JSON - virtual tl::expected get_update(const std::string & id) = 0; - - /// Register a new update package from JSON metadata - virtual tl::expected register_update(const nlohmann::json & metadata) = 0; - - /// Delete an update package - virtual tl::expected delete_update(const std::string & id) = 0; - - // ---- Async operations (called in background thread by UpdateManager) ---- - - /// Prepare an update (download, verify, check dependencies). - /// Reporter is optional - plugin may call reporter.set_progress() etc. - virtual tl::expected prepare(const std::string & id, - UpdateProgressReporter & reporter) = 0; - - /// Execute an update (install). Only called after prepare succeeds. - virtual tl::expected execute(const std::string & id, - UpdateProgressReporter & reporter) = 0; - - /// Check whether a package supports automated mode (prepare + execute) - virtual tl::expected supports_automated(const std::string & id) = 0; - - /// Optional: receive plugin-specific configuration from YAML - virtual void configure(const nlohmann::json & /* config */) { - } -}; - -} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp index 6fe22bb9..f6907077 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp @@ -25,7 +25,7 @@ #include #include -#include "ros2_medkit_gateway/updates/update_backend.hpp" +#include "ros2_medkit_gateway/providers/update_provider.hpp" namespace ros2_medkit_gateway { @@ -49,22 +49,25 @@ struct UpdateError { }; /** - * @brief Manages software update lifecycle with pluggable backend. + * @brief Manages software update lifecycle with pluggable UpdateProvider backend. * * Handles async operations (prepare/execute) in background threads, - * tracks status automatically, and delegates to UpdateBackend for + * tracks status automatically, and delegates to UpdateProvider for * actual work. Without a backend, all operations return errors. */ class UpdateManager { public: - /// Construct with optional backend. Pass nullptr for 501 mode. - explicit UpdateManager(std::unique_ptr backend, void * plugin_handle = nullptr); + /// Construct without a backend. Use set_backend() to wire one in. + UpdateManager(); ~UpdateManager(); // Prevent copy/move (owns async tasks) UpdateManager(const UpdateManager &) = delete; UpdateManager & operator=(const UpdateManager &) = delete; + /// Set the backend provider (non-owning pointer, caller manages lifetime) + void set_backend(UpdateProvider * backend); + /// Check if a backend is loaded bool has_backend() const; @@ -83,8 +86,7 @@ class UpdateManager { tl::expected get_status(const std::string & id); private: - std::unique_ptr backend_; - void * plugin_handle_ = nullptr; // dlopen handle, closed in destructor + UpdateProvider * backend_ = nullptr; struct PackageState { UpdatePhase phase = UpdatePhase::None; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_types.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_types.hpp new file mode 100644 index 00000000..9b315c00 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_types.hpp @@ -0,0 +1,94 @@ +// 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 +#include + +namespace ros2_medkit_gateway { + +/// Filter criteria for listing update packages +struct UpdateFilter { + std::optional origin; // "remote" | "proximity" + std::optional target_version; // Filter by target version +}; + +/// Status of an update operation +enum class UpdateStatus { Pending, InProgress, Completed, Failed }; + +/// Detailed progress for a sub-step of an update operation +struct UpdateSubProgress { + std::string name; + int progress; // 0-100 +}; + +/// Full status information for an update operation +struct UpdateStatusInfo { + UpdateStatus status = UpdateStatus::Pending; + std::optional progress; // 0-100 + std::optional> sub_progress; // Detailed per-step progress + std::optional error_message; // Set when status == Failed +}; + +/// Internal phase tracking for update lifecycle +enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed, Deleting }; + +/// Error codes for backend return values +enum class UpdateBackendError { + NotFound, // Package does not exist + AlreadyExists, // Duplicate ID on registration + InvalidInput, // Malformed metadata + Internal // Unexpected error +}; + +/// Typed error for backend return values +struct UpdateBackendErrorInfo { + UpdateBackendError code; + std::string message; +}; + +/** + * @brief Thread-safe reporter for update progress. + * + * Passed to UpdateProvider::prepare/execute. The plugin MAY use it to report + * fine-grained progress. If not used, UpdateManager still tracks base status + * (Pending -> InProgress -> Completed/Failed) automatically. + */ +class UpdateProgressReporter { + public: + UpdateProgressReporter(UpdateStatusInfo & status, std::mutex & mutex) : status_(status), mutex_(mutex) { + } + + void set_progress(int percent) { + std::lock_guard lock(mutex_); + status_.progress = percent; + } + + void set_sub_progress(std::vector steps) { + std::lock_guard lock(mutex_); + status_.sub_progress = std::move(steps); + } + + private: + UpdateStatusInfo & status_; + std::mutex & mutex_; +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index f0f99b37..c6bfaa3b 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -16,8 +16,6 @@ #include -#include "ros2_medkit_gateway/updates/plugin_loader.hpp" - using namespace std::chrono_literals; namespace ros2_medkit_gateway { @@ -72,10 +70,8 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { declare_parameter("manifest_path", ""); declare_parameter("manifest_strict_validation", true); - // Software updates plugin parameters + // Software updates parameters declare_parameter("updates.enabled", false); - declare_parameter("updates.backend", std::string("none")); - declare_parameter("updates.plugin_path", std::string("")); // Bulk data storage parameters declare_parameter("bulk_data.storage_dir", "/tmp/ros2_medkit_bulk_data"); @@ -323,29 +319,10 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { subscription_mgr_ = std::make_unique(max_subscriptions); RCLCPP_INFO(get_logger(), "Subscription manager: max_subscriptions=%zu", max_subscriptions); - // Initialize update manager + // Initialize update manager (backend wired by PluginManager in Task 6) auto updates_enabled = get_parameter("updates.enabled").as_bool(); if (updates_enabled) { - auto backend_type = get_parameter("updates.backend").as_string(); - if (backend_type == "plugin") { - auto plugin_path = get_parameter("updates.plugin_path").as_string(); - if (plugin_path.empty()) { - RCLCPP_ERROR(get_logger(), "updates.plugin_path is empty - cannot load plugin"); - update_mgr_ = std::make_unique(nullptr); - } else { - auto load_result = UpdatePluginLoader::load(plugin_path); - if (load_result) { - RCLCPP_INFO(get_logger(), "Loaded update plugin: %s", plugin_path.c_str()); - update_mgr_ = std::make_unique(std::move(load_result->backend), load_result->handle); - } else { - RCLCPP_ERROR(get_logger(), "Failed to load update plugin: %s", load_result.error().c_str()); - update_mgr_ = std::make_unique(nullptr); - } - } - } else { - // backend: "none" - endpoints exist but return 501 - update_mgr_ = std::make_unique(nullptr); - } + update_mgr_ = std::make_unique(); } // Connect topic sampler to discovery manager for component-topic mapping diff --git a/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp b/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp deleted file mode 100644 index f21f561c..00000000 --- a/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp +++ /dev/null @@ -1,61 +0,0 @@ -// 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/updates/plugin_loader.hpp" - -#include - -namespace ros2_medkit_gateway { - -tl::expected UpdatePluginLoader::load(const std::string & plugin_path) { - void * handle = dlopen(plugin_path.c_str(), RTLD_LAZY); - if (!handle) { - return tl::make_unexpected("Failed to load plugin '" + plugin_path + "': " + std::string(dlerror())); - } - - // Clear any existing error - dlerror(); - - using FactoryFn = UpdateBackend * (*)(); - auto factory = reinterpret_cast(dlsym(handle, "create_update_backend")); - - const char * error = dlerror(); - if (error) { - dlclose(handle); - return tl::make_unexpected("Failed to find 'create_update_backend' in '" + plugin_path + - "': " + std::string(error)); - } - - UpdateBackend * raw_backend = nullptr; - try { - raw_backend = factory(); - } catch (const std::exception & e) { - dlclose(handle); - return tl::make_unexpected("Factory 'create_update_backend' threw exception in '" + plugin_path + "': " + e.what()); - } catch (...) { - dlclose(handle); - return tl::make_unexpected("Factory 'create_update_backend' threw unknown exception in '" + plugin_path + "'"); - } - if (!raw_backend) { - dlclose(handle); - return tl::make_unexpected("Factory 'create_update_backend' returned null in '" + plugin_path + "'"); - } - - PluginLoadResult result; - result.backend = std::unique_ptr(raw_backend); - result.handle = handle; - return result; -} - -} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/updates/update_manager.cpp b/src/ros2_medkit_gateway/src/updates/update_manager.cpp index 7ecc28a1..513a1d22 100644 --- a/src/ros2_medkit_gateway/src/updates/update_manager.cpp +++ b/src/ros2_medkit_gateway/src/updates/update_manager.cpp @@ -14,13 +14,9 @@ #include "ros2_medkit_gateway/updates/update_manager.hpp" -#include - namespace ros2_medkit_gateway { -UpdateManager::UpdateManager(std::unique_ptr backend, void * plugin_handle) - : backend_(std::move(backend)), plugin_handle_(plugin_handle) { -} +UpdateManager::UpdateManager() = default; UpdateManager::~UpdateManager() { // Signal background tasks to stop accepting new work @@ -40,11 +36,10 @@ UpdateManager::~UpdateManager() { for (auto & f : futures) { f.wait(); } - // Destroy backend before closing plugin handle - backend_.reset(); - if (plugin_handle_) { - dlclose(plugin_handle_); - } +} + +void UpdateManager::set_backend(UpdateProvider * backend) { + backend_ = backend; } bool UpdateManager::has_backend() const { diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp index cdbb811c..21685208 100644 --- a/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp @@ -21,7 +21,9 @@ #include -#include "ros2_medkit_gateway/updates/update_backend.hpp" +#include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" +#include "ros2_medkit_gateway/providers/update_provider.hpp" using json = nlohmann::json; using namespace ros2_medkit_gateway; @@ -29,11 +31,22 @@ using namespace ros2_medkit_gateway; /** * @brief In-memory update backend for integration testing. * - * Stores package metadata in a map. Simulates prepare/execute with - * short delays (100ms per step, 4 steps) and progress reporting. + * Implements both GatewayPlugin (for plugin framework loading) and + * UpdateProvider (for update functionality). Stores package metadata + * in a map. Simulates prepare/execute with short delays (100ms per + * step, 4 steps) and progress reporting. */ -class TestUpdateBackend : public UpdateBackend { +class TestUpdateBackend : public GatewayPlugin, public UpdateProvider { public: + // ---- GatewayPlugin interface ---- + std::string name() const override { + return "test_update_backend"; + } + + void configure(const nlohmann::json & /*config*/) override { + } + + // ---- UpdateProvider interface ---- tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & filter) override { std::lock_guard lock(mutex_); std::vector ids; @@ -184,7 +197,15 @@ class TestUpdateBackend : public UpdateBackend { std::unordered_map packages_; }; -// Plugin factory function - exported for dlopen/dlsym loading -extern "C" UpdateBackend * create_update_backend() { +// Plugin framework exports +extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return PLUGIN_API_VERSION; +} + +extern "C" GATEWAY_PLUGIN_EXPORT GatewayPlugin * create_plugin() { return new TestUpdateBackend(); } + +extern "C" GATEWAY_PLUGIN_EXPORT UpdateProvider * get_update_provider(GatewayPlugin * plugin) { + return static_cast(plugin); +} diff --git a/src/ros2_medkit_gateway/test/test_update_manager.cpp b/src/ros2_medkit_gateway/test/test_update_manager.cpp index 7f0015b4..21d40e75 100644 --- a/src/ros2_medkit_gateway/test/test_update_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_update_manager.cpp @@ -23,7 +23,7 @@ using namespace ros2_medkit_gateway; using json = nlohmann::json; /// Mock backend for unit testing -class MockUpdateBackend : public UpdateBackend { +class MockUpdateBackend : public UpdateProvider { public: tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { std::lock_guard lock(mutex_); @@ -94,10 +94,19 @@ class MockUpdateBackend : public UpdateBackend { class UpdateManagerTest : public ::testing::Test { protected: void SetUp() override { - auto backend = std::make_unique(); - manager_ = std::make_unique(std::move(backend)); + backend_ = std::make_unique(); + backend_ptr_ = backend_.get(); + manager_ = std::make_unique(); + manager_->set_backend(backend_ptr_); } + void TearDown() override { + manager_.reset(); + backend_.reset(); + } + + std::unique_ptr backend_; + MockUpdateBackend * backend_ptr_ = nullptr; std::unique_ptr manager_; }; @@ -108,7 +117,7 @@ TEST_F(UpdateManagerTest, HasBackend) { // @verifies REQ_INTEROP_082 TEST_F(UpdateManagerTest, NoBackendMode) { - UpdateManager no_backend(nullptr); + UpdateManager no_backend; EXPECT_FALSE(no_backend.has_backend()); auto result = no_backend.list_updates({}); EXPECT_FALSE(result.has_value()); @@ -314,7 +323,7 @@ TEST_F(UpdateManagerTest, ConcurrentPrepareOnSamePackageRejected) { } /// Mock backend that returns errors from prepare/execute -class MockFailingBackend : public UpdateBackend { +class MockFailingBackend : public UpdateProvider { public: tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & /*filter*/) override { @@ -347,7 +356,7 @@ class MockFailingBackend : public UpdateBackend { }; /// Mock backend that throws exceptions from prepare/execute -class MockThrowingBackend : public UpdateBackend { +class MockThrowingBackend : public UpdateProvider { public: tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & /*filter*/) override { @@ -378,7 +387,8 @@ class MockThrowingBackend : public UpdateBackend { // @verifies REQ_INTEROP_091 TEST(UpdateManagerFailureTest, PrepareFailureSetsFailedStatus) { auto backend = std::make_unique(); - auto manager = std::make_unique(std::move(backend)); + auto manager = std::make_unique(); + manager->set_backend(backend.get()); json pkg = {{"id", "test-pkg"}}; (void)manager->register_update(pkg); @@ -399,10 +409,12 @@ TEST(UpdateManagerFailureTest, PrepareFailureSetsFailedStatus) { EXPECT_EQ(status.status, UpdateStatus::Failed); ASSERT_TRUE(status.error_message.has_value()); EXPECT_NE(status.error_message->find("download failed"), std::string::npos); + manager.reset(); + backend.reset(); } /// Mock backend with working prepare but failing execute -class MockExecuteFailingBackend : public UpdateBackend { +class MockExecuteFailingBackend : public UpdateProvider { public: tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & /*filter*/) override { @@ -432,7 +444,7 @@ class MockExecuteFailingBackend : public UpdateBackend { }; /// Mock backend with working prepare but throwing execute -class MockExecuteThrowingBackend : public UpdateBackend { +class MockExecuteThrowingBackend : public UpdateProvider { public: tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & /*filter*/) override { @@ -464,7 +476,8 @@ class MockExecuteThrowingBackend : public UpdateBackend { // @verifies REQ_INTEROP_091 TEST(UpdateManagerFailureTest, PrepareExceptionSetsFailedStatus) { auto backend = std::make_unique(); - auto manager = std::make_unique(std::move(backend)); + auto manager = std::make_unique(); + manager->set_backend(backend.get()); json pkg = {{"id", "test-pkg"}}; (void)manager->register_update(pkg); @@ -484,6 +497,8 @@ TEST(UpdateManagerFailureTest, PrepareExceptionSetsFailedStatus) { EXPECT_EQ(status.status, UpdateStatus::Failed); ASSERT_TRUE(status.error_message.has_value()); EXPECT_NE(status.error_message->find("Exception"), std::string::npos); + manager.reset(); + backend.reset(); } // Helper: prepare a package and wait for completion @@ -506,7 +521,8 @@ static bool prepare_and_wait(UpdateManager & manager, const std::string & id) { // @verifies REQ_INTEROP_092 TEST(UpdateManagerFailureTest, ExecuteFailureSetsFailedStatus) { auto backend = std::make_unique(); - auto manager = std::make_unique(std::move(backend)); + auto manager = std::make_unique(); + manager->set_backend(backend.get()); json pkg = {{"id", "test-pkg"}}; (void)manager->register_update(pkg); @@ -531,12 +547,15 @@ TEST(UpdateManagerFailureTest, ExecuteFailureSetsFailedStatus) { EXPECT_EQ(status.status, UpdateStatus::Failed); ASSERT_TRUE(status.error_message.has_value()); EXPECT_NE(status.error_message->find("install failed"), std::string::npos); + manager.reset(); + backend.reset(); } // @verifies REQ_INTEROP_092 TEST(UpdateManagerFailureTest, ExecuteExceptionSetsFailedStatus) { auto backend = std::make_unique(); - auto manager = std::make_unique(std::move(backend)); + auto manager = std::make_unique(); + manager->set_backend(backend.get()); json pkg = {{"id", "test-pkg"}}; (void)manager->register_update(pkg); @@ -561,4 +580,6 @@ TEST(UpdateManagerFailureTest, ExecuteExceptionSetsFailedStatus) { EXPECT_EQ(status.status, UpdateStatus::Failed); ASSERT_TRUE(status.error_message.has_value()); EXPECT_NE(status.error_message->find("Exception"), std::string::npos); + manager.reset(); + backend.reset(); } From 507db16bd92fefb91faa16eb6be119bb84ec0553 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 24 Feb 2026 20:33:19 +0000 Subject: [PATCH 08/14] feat(plugins): integrate PluginManager into GatewayNode Wire plugin loading, configuration, and lifecycle into the gateway. Plugins are declared via the 'plugins' parameter and loaded from per-plugin path config. UpdateProvider from plugins is automatically wired to UpdateManager when updates are enabled. Plugin routes are registered on the REST server after core routes. --- .../ros2_medkit_gateway/gateway_node.hpp | 5 +++ src/ros2_medkit_gateway/src/gateway_node.cpp | 40 ++++++++++++++++++- .../src/http/rest_server.cpp | 8 ++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp index b77ab3a1..ca483895 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp @@ -34,6 +34,7 @@ #include "ros2_medkit_gateway/http/rest_server.hpp" #include "ros2_medkit_gateway/models/thread_safe_entity_cache.hpp" #include "ros2_medkit_gateway/operation_manager.hpp" +#include "ros2_medkit_gateway/plugins/plugin_manager.hpp" #include "ros2_medkit_gateway/subscription_manager.hpp" #include "ros2_medkit_gateway/updates/update_manager.hpp" @@ -99,6 +100,7 @@ class GatewayNode : public rclcpp::Node { * @return Raw pointer to UpdateManager (valid for lifetime of GatewayNode), or nullptr if disabled */ UpdateManager * get_update_manager() const; + PluginManager * get_plugin_manager() const; private: void refresh_cache(); @@ -122,6 +124,9 @@ class GatewayNode : public rclcpp::Node { std::unique_ptr fault_mgr_; std::unique_ptr bulk_data_store_; std::unique_ptr subscription_mgr_; + // IMPORTANT: plugin_mgr_ BEFORE update_mgr_ - C++ destroys in reverse order, + // so update_mgr_ waits for async tasks before plugin_mgr_ destroys the plugin. + std::unique_ptr plugin_mgr_; std::unique_ptr update_mgr_; std::unique_ptr rest_server_; diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index c6bfaa3b..ef755bdb 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -73,6 +73,9 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { // Software updates parameters declare_parameter("updates.enabled", false); + // Plugin framework parameters + declare_parameter("plugins", std::vector{}); + // Bulk data storage parameters declare_parameter("bulk_data.storage_dir", "/tmp/ros2_medkit_bulk_data"); declare_parameter("bulk_data.max_upload_size", 104857600); // 100MB @@ -319,10 +322,38 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { subscription_mgr_ = std::make_unique(max_subscriptions); RCLCPP_INFO(get_logger(), "Subscription manager: max_subscriptions=%zu", max_subscriptions); - // Initialize update manager (backend wired by PluginManager in Task 6) + // Initialize plugin manager + plugin_mgr_ = std::make_unique(); + auto plugin_names = get_parameter("plugins").as_string_array(); + if (!plugin_names.empty()) { + std::vector configs; + for (const auto & pname : plugin_names) { + auto path_param = "plugins." + pname + ".path"; + declare_parameter(path_param, std::string("")); + auto path = get_parameter(path_param).as_string(); + if (path.empty()) { + RCLCPP_ERROR(get_logger(), "Plugin '%s' has no path configured", pname.c_str()); + continue; + } + configs.push_back({pname, path, nlohmann::json::object()}); + } + auto loaded = plugin_mgr_->load_plugins(configs); + plugin_mgr_->configure_plugins(); + plugin_mgr_->set_node(this); + RCLCPP_INFO(get_logger(), "Loaded %zu plugin(s)", loaded); + } + + // Initialize update manager auto updates_enabled = get_parameter("updates.enabled").as_bool(); if (updates_enabled) { update_mgr_ = std::make_unique(); + auto * update_provider = plugin_mgr_->get_update_provider(); + if (update_provider) { + update_mgr_->set_backend(update_provider); + RCLCPP_INFO(get_logger(), "Update backend provided by plugin"); + } else { + RCLCPP_WARN(get_logger(), "Updates enabled but no UpdateProvider plugin loaded"); + } } // Connect topic sampler to discovery manager for component-topic mapping @@ -365,6 +396,9 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { GatewayNode::~GatewayNode() { RCLCPP_INFO(get_logger(), "Shutting down ROS 2 Medkit Gateway..."); stop_rest_server(); + if (plugin_mgr_) { + plugin_mgr_->shutdown_all(); + } } const ThreadSafeEntityCache & GatewayNode::get_thread_safe_cache() const { @@ -403,6 +437,10 @@ UpdateManager * GatewayNode::get_update_manager() const { return update_mgr_.get(); } +PluginManager * GatewayNode::get_plugin_manager() const { + return plugin_mgr_.get(); +} + void GatewayNode::refresh_cache() { RCLCPP_DEBUG(get_logger(), "Refreshing entity cache..."); diff --git a/src/ros2_medkit_gateway/src/http/rest_server.cpp b/src/ros2_medkit_gateway/src/http/rest_server.cpp index a45ca1da..d0580750 100644 --- a/src/ros2_medkit_gateway/src/http/rest_server.cpp +++ b/src/ros2_medkit_gateway/src/http/rest_server.cpp @@ -136,6 +136,14 @@ RESTServer::RESTServer(GatewayNode * node, const std::string & host, int port, c // Set up pre-routing handler for CORS and Authentication setup_pre_routing_handler(); setup_routes(); + + // Register plugin custom routes + if (node_->get_plugin_manager()) { + auto * plugin_srv = http_server_->get_server(); + if (plugin_srv) { + node_->get_plugin_manager()->register_routes(*plugin_srv, "/api/v1"); + } + } } void RESTServer::setup_pre_routing_handler() { From 3ba9b33b6cc988c59955a008307895f1f8381b87 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 25 Feb 2026 07:04:19 +0000 Subject: [PATCH 09/14] fix(plugins): address review critical blockers and quick fixes - Fix integration test to use new plugin framework parameters (#235) - Update docs: remove stale updates.backend/plugin_path, add plugin framework section to server.rst, update changelog, rest.rst, FAQ - Add plugins section to gateway_params.yaml with documented examples - Add RTLD_LOCAL flag to dlopen for explicit symbol isolation - Use API_BASE_PATH constant instead of hardcoded "/api/v1" - Disable plugins on set_node()/register_routes() failure - Add logging to shutdown_all() catch block - Add doxygen to get_plugin_manager() --- docs/api/rest.rst | 5 +- docs/changelog.rst | 10 ++- docs/config/server.rst | 62 +++++++++++++++---- docs/troubleshooting.rst | 6 +- .../config/gateway_params.yaml | 23 +++++-- .../ros2_medkit_gateway/gateway_node.hpp | 5 ++ .../src/http/rest_server.cpp | 2 +- .../src/plugins/plugin_loader.cpp | 2 +- .../src/plugins/plugin_manager.cpp | 29 ++++++--- .../test/features/test_updates.test.py | 5 +- 10 files changed, 116 insertions(+), 33 deletions(-) diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 43359cfc..97a55e50 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -676,8 +676,9 @@ Software Updates ---------------- Manage software update packages with an async prepare/execute lifecycle. -The updates feature requires a backend plugin to be loaded (see :doc:`/config/server`). -Without a plugin, all endpoints return ``501 Not Implemented``. +The updates feature requires a plugin implementing ``UpdateProvider`` to be loaded +via the plugin framework (see :doc:`/config/server`). +Without such a plugin, all endpoints return ``501 Not Implemented``. ``GET /api/v1/updates`` List all registered update packages. diff --git a/docs/changelog.rst b/docs/changelog.rst index 847b56b2..259e743b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,10 +12,16 @@ and this project adheres to `Semantic Versioning .path`` parameter. + * - ``plugins..path`` + - string + - (required) + - Absolute path to the plugin ``.so`` file. Must exist and have ``.so`` extension. + +Plugin loading lifecycle: + +1. Shared library is loaded via ``dlopen`` with ``RTLD_NOW | RTLD_LOCAL`` +2. API version is checked (must match gateway headers) +3. ``create_plugin()`` factory is called to instantiate the plugin +4. Provider interfaces are queried via ``extern "C"`` functions +5. ``configure()`` is called with per-plugin config +6. ``set_node()`` provides access to the ROS 2 node +7. ``register_routes()`` allows the plugin to add custom REST endpoints + +Error isolation: if a plugin throws during any lifecycle call, it is disabled +without crashing the gateway. Other plugins continue to operate normally. + +Example: + +.. code-block:: yaml + + ros2_medkit_gateway: + ros__parameters: + plugins: ["my_ota_plugin"] + plugins.my_ota_plugin.path: "/opt/ros2_medkit/lib/libmy_ota_plugin.so" + Software Updates ---------------- -Configure the software updates plugin system. Updates are disabled by default. +Configure the software updates system. Updates are disabled by default. +When enabled, a plugin implementing ``UpdateProvider`` is required to provide +the backend functionality (see `Plugin Framework`_ above). .. list-table:: :header-rows: 1 @@ -272,14 +320,6 @@ Configure the software updates plugin system. Updates are disabled by default. - bool - ``false`` - Enable/disable software updates endpoints. When disabled, ``/updates`` routes are not registered. - * - ``updates.backend`` - - string - - ``"none"`` - - Backend type. ``"none"`` enables endpoints but returns 501. ``"plugin"`` loads a shared library. - * - ``updates.plugin_path`` - - string - - ``""`` - - Path to the ``.so`` plugin file. Required when ``backend`` is ``"plugin"``. Example: @@ -287,10 +327,10 @@ Example: ros2_medkit_gateway: ros__parameters: + plugins: ["my_update_plugin"] + plugins.my_update_plugin.path: "/opt/ros2_medkit/lib/libmy_update_plugin.so" updates: enabled: true - backend: "plugin" - plugin_path: "/opt/ros2_medkit/plugins/libmy_update_backend.so" Complete Example ---------------- diff --git a/docs/troubleshooting.rst b/docs/troubleshooting.rst index 652c539a..245cdbef 100644 --- a/docs/troubleshooting.rst +++ b/docs/troubleshooting.rst @@ -253,8 +253,10 @@ ros2_medkit is currently suitable for development and testing. For production: **Q: Can I extend ros2_medkit with custom endpoints?** -Not directly at this time. Future versions may support plugins. -For now, you can fork the gateway and add custom routes in ``rest_server.cpp``. +Yes. The gateway plugin framework allows you to add custom REST endpoints via +``GatewayPlugin::register_routes()``. Create a shared library (``.so``) that +implements the ``GatewayPlugin`` base class and configure it in ``gateway_params.yaml``. +See :doc:`/config/server` for plugin configuration details. **Q: How do I report a bug or request a feature?** diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index fd8ff1cc..d046ed54 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -236,15 +236,30 @@ ros2_medkit_gateway: # Example: ["calibration", "firmware", "comlogs", "snapshots"] categories: [] - # Software Updates Plugin System - # Enables /updates endpoints for SOVD-compliant software update management + # Plugin Framework + # Extend gateway with custom plugins loaded from shared libraries (.so). + # Plugins can implement provider interfaces (UpdateProvider, IntrospectionProvider) + # that are automatically detected and wired into subsystem managers. + # + # List plugin names, then configure each with a plugins..path parameter. + # Example: + # plugins: ["my_ota_plugin"] + # plugins.my_ota_plugin.path: "/opt/ros2_medkit/lib/libmy_ota_plugin.so" + # + # Plugin lifecycle: load -> configure -> set_node -> register_routes + # Plugins that throw during any lifecycle call are disabled (not crashed). + # Default: [] (no plugins) + plugins: [] + + # Software Updates + # Enables /updates endpoints for SOVD-compliant software update management. + # When enabled, a plugin implementing UpdateProvider is required to provide + # the backend functionality. Without such a plugin, endpoints return 501. updates: # Enable/disable /updates endpoints # When false, update routes are not registered # Default: false enabled: false - # Update backend is wired via the plugin framework. - # A plugin implementing UpdateProvider will be detected automatically. # Rate Limiting Configuration # Token-bucket-based rate limiting for API requests. diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp index ca483895..91999534 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp @@ -100,6 +100,11 @@ class GatewayNode : public rclcpp::Node { * @return Raw pointer to UpdateManager (valid for lifetime of GatewayNode), or nullptr if disabled */ UpdateManager * get_update_manager() const; + + /** + * @brief Get the PluginManager instance + * @return Raw pointer to PluginManager (valid for lifetime of GatewayNode) + */ PluginManager * get_plugin_manager() const; private: diff --git a/src/ros2_medkit_gateway/src/http/rest_server.cpp b/src/ros2_medkit_gateway/src/http/rest_server.cpp index d0580750..feb99fb4 100644 --- a/src/ros2_medkit_gateway/src/http/rest_server.cpp +++ b/src/ros2_medkit_gateway/src/http/rest_server.cpp @@ -141,7 +141,7 @@ RESTServer::RESTServer(GatewayNode * node, const std::string & host, int port, c if (node_->get_plugin_manager()) { auto * plugin_srv = http_server_->get_server(); if (plugin_srv) { - node_->get_plugin_manager()->register_routes(*plugin_srv, "/api/v1"); + node_->get_plugin_manager()->register_routes(*plugin_srv, API_BASE_PATH); } } } diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp index ec38c7ab..ed07c731 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp @@ -89,7 +89,7 @@ tl::expected PluginLoader::load(const std: } // --- dlopen --- - void * handle = dlopen(canonical_path.c_str(), RTLD_NOW); + void * handle = dlopen(canonical_path.c_str(), RTLD_NOW | RTLD_LOCAL); if (!handle) { return tl::make_unexpected("Failed to load plugin: " + std::string(dlerror())); } diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp index 4dbd60b9..b8c6bda4 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp @@ -117,11 +117,17 @@ void PluginManager::set_node(rclcpp::Node * node) { try { lp.load_result.plugin->set_node(node); } catch (const std::exception & e) { - RCLCPP_ERROR(logger(), "Plugin '%s' threw during set_node(): %s", lp.load_result.plugin->name().c_str(), - e.what()); + RCLCPP_ERROR(logger(), "Plugin '%s' threw during set_node(): %s - disabling", + lp.load_result.plugin->name().c_str(), e.what()); + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.plugin.reset(); } catch (...) { - RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during set_node()", + RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during set_node() - disabling", lp.load_result.plugin->name().c_str()); + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.plugin.reset(); } } } @@ -134,11 +140,17 @@ void PluginManager::register_routes(httplib::Server & server, const std::string try { lp.load_result.plugin->register_routes(server, api_prefix); } catch (const std::exception & e) { - RCLCPP_ERROR(logger(), "Plugin '%s' threw during register_routes(): %s", lp.load_result.plugin->name().c_str(), - e.what()); + RCLCPP_ERROR(logger(), "Plugin '%s' threw during register_routes(): %s - disabling", + lp.load_result.plugin->name().c_str(), e.what()); + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.plugin.reset(); } catch (...) { - RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during register_routes()", + RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during register_routes() - disabling", lp.load_result.plugin->name().c_str()); + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.plugin.reset(); } } } @@ -150,8 +162,11 @@ void PluginManager::shutdown_all() { } try { lp.load_result.plugin->shutdown(); + } catch (const std::exception & e) { + RCLCPP_WARN(logger(), "Plugin '%s' threw during shutdown(): %s", lp.load_result.plugin->name().c_str(), e.what()); } catch (...) { - // Best effort shutdown + RCLCPP_WARN(logger(), "Plugin '%s' threw unknown exception during shutdown()", + lp.load_result.plugin->name().c_str()); } } } diff --git a/src/ros2_medkit_integration_tests/test/features/test_updates.test.py b/src/ros2_medkit_integration_tests/test/features/test_updates.test.py index 01163ac1..2cf0b19f 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_updates.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_updates.test.py @@ -64,7 +64,6 @@ def generate_test_description(): 'server.port': PORT_NO_PLUGIN, 'refresh_interval_ms': 1000, 'updates.enabled': True, - 'updates.backend': 'none', }], additional_env=coverage_env, ) @@ -79,8 +78,8 @@ def generate_test_description(): 'server.port': PORT_WITH_PLUGIN, 'refresh_interval_ms': 1000, 'updates.enabled': True, - 'updates.backend': 'plugin', - 'updates.plugin_path': plugin_path, + 'plugins': ['test_update_backend'], + 'plugins.test_update_backend.path': plugin_path, }], additional_env=coverage_env, ) From 27b593898b4ff2a0dcfd76c9646287323925f8c7 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 25 Feb 2026 07:10:24 +0000 Subject: [PATCH 10/14] docs(plugins): add plugin system tutorial Documents plugin interface, configuration, extern "C" exports, lifecycle, custom REST endpoints, multi-plugin dispatch, error handling, and API versioning. Refs #235 --- docs/tutorials/index.rst | 4 + docs/tutorials/plugin-system.rst | 166 +++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+) create mode 100644 docs/tutorials/plugin-system.rst diff --git a/docs/tutorials/index.rst b/docs/tutorials/index.rst index 281b6b3d..091588df 100644 --- a/docs/tutorials/index.rst +++ b/docs/tutorials/index.rst @@ -20,6 +20,7 @@ Step-by-step guides for common use cases with ros2_medkit. custom_areas web-ui mcp-server + plugin-system Demos ----- @@ -84,3 +85,6 @@ Advanced Tutorials :doc:`custom_areas` Customize the entity hierarchy for your robot architecture. + +:doc:`plugin-system` + Extend the gateway with custom plugins for update backends, introspection, and REST endpoints. diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst new file mode 100644 index 00000000..171b2a01 --- /dev/null +++ b/docs/tutorials/plugin-system.rst @@ -0,0 +1,166 @@ +Plugin System +============= + +The gateway supports a plugin system for extending functionality with shared libraries (``.so`` files). +Plugins can provide software update backends, platform-specific introspection, and custom REST endpoints. + +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** - enriches discovered entities with platform-specific metadata + +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). + +Configuration +------------- + +Add plugins to ``gateway_params.yaml``: + +.. code-block:: yaml + + ros2_medkit_gateway: + ros__parameters: + plugins: ["my_ota_plugin"] + plugins.my_ota_plugin.path: "/opt/ros2_medkit/lib/libmy_ota_plugin.so" + + # Enable updates if your plugin implements UpdateProvider + updates: + enabled: true + +Each plugin name in the ``plugins`` array requires a corresponding ``plugins..path`` +parameter with the absolute path to the ``.so`` file. Plugins are loaded in the order listed. +An empty list (default) means no plugins are loaded, with zero overhead. + +Writing a Plugin +---------------- + +1. Create a C++ shared library implementing ``GatewayPlugin`` and optionally one or more providers: + +.. code-block:: cpp + + #include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" + #include "ros2_medkit_gateway/plugins/plugin_types.hpp" + #include "ros2_medkit_gateway/providers/update_provider.hpp" + + using namespace ros2_medkit_gateway; + + class MyPlugin : public GatewayPlugin, public UpdateProvider { + public: + std::string name() const override { return "my_plugin"; } + + void configure(const nlohmann::json& config) override { + // Read plugin-specific configuration (env vars, files, etc.) + } + + void shutdown() override { + // Clean up resources + } + + // UpdateProvider methods + tl::expected, UpdateBackendErrorInfo> + list_updates(const UpdateFilter& filter) override { /* ... */ } + // ... remaining CRUD and async methods + }; + +2. Export the required ``extern "C"`` symbols: + +.. code-block:: cpp + + // Required: API version check + extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return ros2_medkit_gateway::PLUGIN_API_VERSION; + } + + // Required: factory function + extern "C" GATEWAY_PLUGIN_EXPORT GatewayPlugin* create_plugin() { + return new MyPlugin(); + } + + // Optional: provider query (needed for UpdateProvider) + extern "C" GATEWAY_PLUGIN_EXPORT UpdateProvider* get_update_provider(GatewayPlugin* p) { + return static_cast(p); + } + + // Optional: provider query (needed for IntrospectionProvider) + extern "C" GATEWAY_PLUGIN_EXPORT IntrospectionProvider* get_introspection_provider(GatewayPlugin* p) { + return static_cast(p); + } + +The ``get_update_provider`` and ``get_introspection_provider`` functions use ``extern "C"`` +to avoid RTTI issues across shared library boundaries. The ``static_cast`` is safe because +these functions execute inside the plugin's own ``.so`` where the type hierarchy is known. + +3. Build as a MODULE library: + +.. code-block:: cmake + + add_library(my_plugin MODULE src/my_plugin.cpp) + target_link_libraries(my_plugin gateway_lib) + +4. Install the ``.so`` and add its path to ``gateway_params.yaml``. + +Plugin Lifecycle +---------------- + +1. ``dlopen`` loads the ``.so`` with ``RTLD_NOW | RTLD_LOCAL`` +2. ``plugin_api_version()`` is checked against the gateway's ``PLUGIN_API_VERSION`` +3. ``create_plugin()`` factory function creates the plugin instance +4. Provider interfaces are queried via ``get_update_provider()`` / ``get_introspection_provider()`` +5. ``configure()`` is called with per-plugin JSON config +6. ``set_node()`` provides optional ROS 2 node access +7. ``register_routes()`` allows registering custom REST endpoints +8. Runtime: subsystem managers call provider methods as needed +9. ``shutdown()`` is called before the plugin is destroyed + +Custom REST Endpoints +--------------------- + +Any plugin can register vendor-specific endpoints via ``register_routes()``: + +.. code-block:: cpp + + void register_routes(httplib::Server& server, const std::string& api_prefix) override { + server.Get(api_prefix + "/x-myvendor/status", + [this](const httplib::Request&, httplib::Response& res) { + res.set_content(get_status_json(), "application/json"); + }); + } + +Use the ``x-`` prefix for vendor-specific endpoints per SOVD convention. + +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 +- **Custom routes**: All plugins can register endpoints (use unique path prefixes) + +Error Handling +-------------- + +If a plugin throws during any lifecycle method (``configure``, ``set_node``, ``register_routes``, +``shutdown``), the exception is caught and logged. The plugin is disabled but the gateway continues +operating. A failing plugin never crashes the gateway. + +API Versioning +-------------- + +Plugins export ``plugin_api_version()`` which must return the gateway's ``PLUGIN_API_VERSION``. +If the version does not match, the plugin is rejected with a clear error message suggesting +a rebuild against matching gateway headers. + +The current API version is **1**. It will be incremented when breaking changes are made to +``GatewayPlugin`` or provider interfaces. + +Build Requirements +------------------ + +- **Same compiler and ABI** as the gateway executable +- **RTTI must be enabled** - do NOT compile plugins with ``-fno-rtti`` +- The ``GATEWAY_PLUGIN_EXPORT`` macro ensures correct symbol visibility From ae09ffad06684eddc6935adff59dcd8fbc29bc49 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 25 Feb 2026 08:04:24 +0000 Subject: [PATCH 11/14] fix(plugins): address review round 2 - security, tests, and docs Add shutdown idempotency guard preventing double-shutdown UB. Extract disable_plugin() helper to DRY lifecycle error handling and clear dangling provider pointers in GatewayPluginLoadResult. Re-check .so extension after symlink resolution. Validate plugin names from YAML config. Port thread safety docs from deleted UpdateBackend. Add 8 new tests covering set_node/register_routes error isolation, shutdown idempotency, move semantics, minimal plugin, and load_plugins success path. Add complete minimal plugin example to tutorial and fix misleading "Optional" labels on required provider query functions. --- docs/changelog.rst | 7 +- docs/tutorials/plugin-system.rst | 110 ++++++++++++++- src/ros2_medkit_gateway/CMakeLists.txt | 6 + .../plugins/plugin_manager.hpp | 6 + .../providers/update_provider.hpp | 34 ++++- src/ros2_medkit_gateway/src/gateway_node.cpp | 17 +++ .../src/plugins/plugin_loader.cpp | 5 + .../src/plugins/plugin_manager.cpp | 36 ++--- .../test/demo_nodes/test_minimal_plugin.cpp | 36 +++++ .../test/test_plugin_loader.cpp | 66 +++++++++ .../test/test_plugin_manager.cpp | 131 ++++++++++++++++++ 11 files changed, 428 insertions(+), 26 deletions(-) create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_minimal_plugin.cpp diff --git a/docs/changelog.rst b/docs/changelog.rst index 259e743b..5be3a053 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,7 +15,7 @@ Added * Gateway plugin framework for extending the gateway with custom functionality: - Load plugins from shared libraries (``.so``) via ``plugins`` parameter - - Provider interfaces: ``UpdateProvider``, ``IntrospectionProvider`` + - Provider interfaces: ``UpdateProvider``, ``IntrospectionProvider`` (preview) - Error isolation: failing plugins are disabled without crashing the gateway - RAII lifecycle with API version checking and path validation @@ -47,6 +47,9 @@ Changed * Fault response structure now SOVD-compliant with ``item`` wrapper * Rosbag downloads use SOVD bulk-data pattern instead of legacy endpoints * Rosbag IDs changed from timestamps to UUIDs +* Software update backends now load via the plugin framework instead of + dedicated ``updates.backend`` / ``updates.plugin_path`` parameters. + Migrate to ``plugins`` array with ``plugins..path`` entries. Removed ~~~~~~~ @@ -62,6 +65,8 @@ Removed and ``environment_data`` structure * Legacy snapshot endpoints removed - migrate to inline snapshots and bulk-data * Rosbag identifiers changed from timestamps to UUIDs +* ``updates.backend`` and ``updates.plugin_path`` parameters removed - use the + ``plugins`` array to load update backend plugins [0.1.0] - 2026-02-01 -------------------- diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst index 171b2a01..b71f605a 100644 --- a/docs/tutorials/plugin-system.rst +++ b/docs/tutorials/plugin-system.rst @@ -10,7 +10,8 @@ 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** - enriches discovered entities with platform-specific metadata +- **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. 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). @@ -35,6 +36,8 @@ Each plugin name in the ``plugins`` array requires a corresponding ``plugins., UpdateBackendErrorInfo> list_updates(const UpdateFilter& filter) override { /* ... */ } - // ... remaining CRUD and async methods + + tl::expected + get_update(const std::string& id) override { /* ... */ } + + tl::expected + register_update(const nlohmann::json& metadata) override { /* ... */ } + + tl::expected + delete_update(const std::string& id) override { /* ... */ } + + tl::expected + prepare(const std::string& id, UpdateProgressReporter& reporter) override { /* ... */ } + + tl::expected + execute(const std::string& id, UpdateProgressReporter& reporter) override { /* ... */ } + + tl::expected + supports_automated(const std::string& id) override { /* ... */ } }; 2. Export the required ``extern "C"`` symbols: @@ -80,12 +101,12 @@ Writing a Plugin return new MyPlugin(); } - // Optional: provider query (needed for UpdateProvider) + // Required if your plugin implements UpdateProvider: extern "C" GATEWAY_PLUGIN_EXPORT UpdateProvider* get_update_provider(GatewayPlugin* p) { return static_cast(p); } - // Optional: provider query (needed for IntrospectionProvider) + // Required if your plugin implements IntrospectionProvider: extern "C" GATEWAY_PLUGIN_EXPORT IntrospectionProvider* get_introspection_provider(GatewayPlugin* p) { return static_cast(p); } @@ -94,6 +115,9 @@ The ``get_update_provider`` and ``get_introspection_provider`` functions use ``e to avoid RTTI issues across shared library boundaries. The ``static_cast`` is safe because these functions execute inside the plugin's own ``.so`` where the type hierarchy is known. +Without the corresponding ``get_*_provider`` export, the gateway cannot detect that your plugin +implements the provider interface, even if the class inherits from it. + 3. Build as a MODULE library: .. code-block:: cmake @@ -103,6 +127,80 @@ these functions execute inside the plugin's own ``.so`` where the type hierarchy 4. Install the ``.so`` and add its path to ``gateway_params.yaml``. +Complete Minimal Plugin +----------------------- + +A self-contained plugin implementing UpdateProvider (copy-paste starting point): + +.. code-block:: cpp + + // my_ota_plugin.cpp + #include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" + #include "ros2_medkit_gateway/plugins/plugin_types.hpp" + #include "ros2_medkit_gateway/providers/update_provider.hpp" + + #include + #include + + using namespace ros2_medkit_gateway; + + class MyOtaPlugin : public GatewayPlugin, public UpdateProvider { + public: + std::string name() const override { return "my_ota"; } + + void configure(const nlohmann::json& /*config*/) override {} + + void shutdown() override {} + + // UpdateProvider CRUD + tl::expected, UpdateBackendErrorInfo> + list_updates(const UpdateFilter& /*filter*/) override { + return std::vector{}; + } + + tl::expected + get_update(const std::string& id) override { + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::NotFound, "not found: " + id}); + } + + tl::expected + register_update(const nlohmann::json& /*metadata*/) override { return {}; } + + tl::expected + delete_update(const std::string& /*id*/) override { return {}; } + + // UpdateProvider async operations + tl::expected + prepare(const std::string& /*id*/, UpdateProgressReporter& reporter) override { + reporter.set_progress(100); + return {}; + } + + tl::expected + execute(const std::string& /*id*/, UpdateProgressReporter& reporter) override { + reporter.set_progress(100); + return {}; + } + + tl::expected + supports_automated(const std::string& /*id*/) override { return false; } + }; + + // Required exports + extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return PLUGIN_API_VERSION; + } + + extern "C" GATEWAY_PLUGIN_EXPORT GatewayPlugin* create_plugin() { + return new MyOtaPlugin(); + } + + // Required for UpdateProvider detection + extern "C" GATEWAY_PLUGIN_EXPORT UpdateProvider* get_update_provider(GatewayPlugin* p) { + return static_cast(p); + } + Plugin Lifecycle ---------------- @@ -138,7 +236,7 @@ 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 +- **IntrospectionProvider**: All plugins' results are merged *(preview - not yet wired)* - **Custom routes**: All plugins can register endpoints (use unique path prefixes) Error Handling diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 1b6e66aa..1f32c0fa 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -415,6 +415,12 @@ if(BUILD_TESTING) install(TARGETS ${_plugin} LIBRARY DESTINATION lib/${PROJECT_NAME}) endforeach() + # Minimal plugin with no provider query functions (only required exports) + add_library(test_minimal_plugin MODULE test/demo_nodes/test_minimal_plugin.cpp) + target_include_directories(test_minimal_plugin PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_link_libraries(test_minimal_plugin nlohmann_json::nlohmann_json cpp_httplib_target OpenSSL::SSL OpenSSL::Crypto) + install(TARGETS test_minimal_plugin LIBRARY DESTINATION lib/${PROJECT_NAME}) + # Add plugin loader tests ament_add_gtest(test_plugin_loader test/test_plugin_loader.cpp) target_link_libraries(test_plugin_loader gateway_lib) 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 96213cd6..3c59a1ab 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,7 +127,13 @@ class PluginManager { UpdateProvider * update_provider = nullptr; IntrospectionProvider * introspection_provider = nullptr; }; + + /// Disable a plugin after a lifecycle error (nulls providers, resets plugin). + /// Also clears provider pointers in load_result to prevent dangling references. + void disable_plugin(LoadedPlugin & lp); + std::vector plugins_; + bool shutdown_called_ = false; }; } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp index 82c1f06f..e26648d4 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp @@ -27,6 +27,23 @@ namespace ros2_medkit_gateway { * Reuses all types from update_types.hpp (UpdateFilter, UpdateBackendErrorInfo, * UpdateProgressReporter, etc.). * + * @par Thread Safety + * - CRUD methods (list_updates, get_update, register_update, delete_update) + * are called WITHOUT holding UpdateManager's mutex. They may be called + * concurrently with each other and with prepare/execute running in a + * background thread. If the plugin shares state, it must provide its own + * synchronization. + * Note: delete_update has a partial guard - UpdateManager checks/sets a + * Deleting sentinel under lock before calling delete_update, but the + * plugin call itself runs outside the lock. + * - prepare() and execute() run in a background std::async thread. They may + * run concurrently with CRUD calls from the HTTP thread. + * - The UpdateProgressReporter passed to prepare/execute is already + * thread-safe - plugins may call set_progress/set_sub_progress freely. + * - Exceptions thrown from prepare/execute are caught by UpdateManager and + * mapped to Failed status. Plugins should prefer returning + * tl::make_unexpected() for expected errors. + * * @see GatewayPlugin for the base class * @see UpdateManager for the subsystem that uses this */ @@ -35,18 +52,33 @@ class UpdateProvider { virtual ~UpdateProvider() = default; // ---- CRUD (metadata storage owned by plugin) ---- + + /// List all registered update package IDs, optionally filtered virtual tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & filter) = 0; + + /// Get full metadata for a specific update package as JSON virtual tl::expected get_update(const std::string & id) = 0; + + /// Register a new update package from JSON metadata virtual tl::expected register_update(const nlohmann::json & metadata) = 0; + + /// Delete an update package virtual tl::expected delete_update(const std::string & id) = 0; - // ---- Async operations (called in background threads) ---- + // ---- Async operations (called in background thread by UpdateManager) ---- + + /// Prepare an update (download, verify, check dependencies). + /// Reporter is optional - plugin may call reporter.set_progress() etc. virtual tl::expected prepare(const std::string & id, UpdateProgressReporter & reporter) = 0; + + /// Execute an update (install). Only called after prepare succeeds. virtual tl::expected execute(const std::string & id, UpdateProgressReporter & reporter) = 0; // ---- Capability queries ---- + + /// Check whether a package supports automated mode (prepare + execute) virtual tl::expected supports_automated(const std::string & id) = 0; }; diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index ef755bdb..72b6ac4f 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -14,6 +14,8 @@ #include "ros2_medkit_gateway/gateway_node.hpp" +#include +#include #include using namespace std::chrono_literals; @@ -327,7 +329,22 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { auto plugin_names = get_parameter("plugins").as_string_array(); if (!plugin_names.empty()) { std::vector configs; + // Plugin name validation: alphanumeric, underscore, hyphen only (max 256 chars) + auto is_valid_plugin_name = [](const std::string & name) -> bool { + if (name.empty() || name.size() > 256) { + return false; + } + return std::all_of(name.begin(), name.end(), [](char c) { + return std::isalnum(c) || c == '_' || c == '-'; + }); + }; for (const auto & pname : plugin_names) { + if (!is_valid_plugin_name(pname)) { + RCLCPP_ERROR(get_logger(), + "Invalid plugin name '%s': must be alphanumeric, underscore, or hyphen (max 256 chars)", + pname.c_str()); + continue; + } auto path_param = "plugins." + pname + ".path"; declare_parameter(path_param, std::string("")); auto path = get_parameter(path_param).as_string(); diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp index ed07c731..098d3336 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp @@ -88,6 +88,11 @@ tl::expected PluginLoader::load(const std: return tl::make_unexpected("Plugin path does not exist or is not accessible: " + plugin_path); } + // Re-check extension after symlink resolution + if (canonical_path.extension() != ".so") { + return tl::make_unexpected("Plugin path must have .so extension after resolving symlinks: " + plugin_path); + } + // --- dlopen --- void * handle = dlopen(canonical_path.c_str(), RTLD_NOW | RTLD_LOCAL); if (!handle) { diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp index b8c6bda4..4a1c2220 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp @@ -86,6 +86,14 @@ size_t PluginManager::load_plugins(const std::vector & configs) { return loaded; } +void PluginManager::disable_plugin(LoadedPlugin & lp) { + lp.update_provider = nullptr; + lp.introspection_provider = nullptr; + lp.load_result.update_provider = nullptr; + lp.load_result.introspection_provider = nullptr; + lp.load_result.plugin.reset(); +} + void PluginManager::configure_plugins() { for (auto & lp : plugins_) { if (!lp.load_result.plugin) { @@ -96,15 +104,11 @@ void PluginManager::configure_plugins() { } catch (const std::exception & e) { RCLCPP_ERROR(logger(), "Plugin '%s' threw during configure(): %s", lp.load_result.plugin->name().c_str(), e.what()); - lp.update_provider = nullptr; - lp.introspection_provider = nullptr; - lp.load_result.plugin.reset(); + disable_plugin(lp); } catch (...) { RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during configure()", lp.load_result.plugin->name().c_str()); - lp.update_provider = nullptr; - lp.introspection_provider = nullptr; - lp.load_result.plugin.reset(); + disable_plugin(lp); } } } @@ -119,15 +123,11 @@ void PluginManager::set_node(rclcpp::Node * node) { } catch (const std::exception & e) { RCLCPP_ERROR(logger(), "Plugin '%s' threw during set_node(): %s - disabling", lp.load_result.plugin->name().c_str(), e.what()); - lp.update_provider = nullptr; - lp.introspection_provider = nullptr; - lp.load_result.plugin.reset(); + disable_plugin(lp); } catch (...) { RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during set_node() - disabling", lp.load_result.plugin->name().c_str()); - lp.update_provider = nullptr; - lp.introspection_provider = nullptr; - lp.load_result.plugin.reset(); + disable_plugin(lp); } } } @@ -142,20 +142,20 @@ void PluginManager::register_routes(httplib::Server & server, const std::string } catch (const std::exception & e) { RCLCPP_ERROR(logger(), "Plugin '%s' threw during register_routes(): %s - disabling", lp.load_result.plugin->name().c_str(), e.what()); - lp.update_provider = nullptr; - lp.introspection_provider = nullptr; - lp.load_result.plugin.reset(); + disable_plugin(lp); } catch (...) { RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during register_routes() - disabling", lp.load_result.plugin->name().c_str()); - lp.update_provider = nullptr; - lp.introspection_provider = nullptr; - lp.load_result.plugin.reset(); + disable_plugin(lp); } } } void PluginManager::shutdown_all() { + if (shutdown_called_) { + return; + } + shutdown_called_ = true; for (auto & lp : plugins_) { if (!lp.load_result.plugin) { continue; diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_minimal_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_minimal_plugin.cpp new file mode 100644 index 00000000..52a5a60b --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_minimal_plugin.cpp @@ -0,0 +1,36 @@ +// 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/plugins/gateway_plugin.hpp" +#include "ros2_medkit_gateway/plugins/plugin_types.hpp" + +using namespace ros2_medkit_gateway; + +/// Minimal valid plugin: exports required symbols only, no provider query functions. +class TestMinimalPlugin : public GatewayPlugin { + public: + std::string name() const override { + return "minimal_plugin"; + } + void configure(const nlohmann::json & /*config*/) override { + } +}; + +extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { + return PLUGIN_API_VERSION; +} + +extern "C" GATEWAY_PLUGIN_EXPORT GatewayPlugin * create_plugin() { + return new TestMinimalPlugin(); +} diff --git a/src/ros2_medkit_gateway/test/test_plugin_loader.cpp b/src/ros2_medkit_gateway/test/test_plugin_loader.cpp index 0f52d344..da70699a 100644 --- a/src/ros2_medkit_gateway/test/test_plugin_loader.cpp +++ b/src/ros2_medkit_gateway/test/test_plugin_loader.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include "ros2_medkit_gateway/plugins/plugin_loader.hpp" +#include "ros2_medkit_gateway/plugins/plugin_manager.hpp" #include "ros2_medkit_gateway/providers/introspection_provider.hpp" #include "ros2_medkit_gateway/providers/update_provider.hpp" @@ -111,3 +112,68 @@ TEST(TestPluginLoader, RejectsNullFactory) { ASSERT_FALSE(result.has_value()); EXPECT_NE(result.error().find("returned null"), std::string::npos); } + +// --- Minimal plugin (no provider query functions) --- + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, LoadsMinimalPluginWithNoProviders) { + auto result = PluginLoader::load(plugin_lib_dir() + "libtest_minimal_plugin.so"); + ASSERT_TRUE(result.has_value()) << result.error(); + EXPECT_NE(result->plugin, nullptr); + EXPECT_EQ(result->plugin->name(), "minimal_plugin"); + EXPECT_EQ(result->update_provider, nullptr); + EXPECT_EQ(result->introspection_provider, nullptr); +} + +// --- GatewayPluginLoadResult move semantics --- + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, MoveConstructorTransfersOwnership) { + auto result = PluginLoader::load(test_plugin_path()); + ASSERT_TRUE(result.has_value()) << result.error(); + + auto * orig_plugin = result->plugin.get(); + auto * orig_update = result->update_provider; + + GatewayPluginLoadResult moved(std::move(*result)); + + // Moved-to object has the resources + EXPECT_EQ(moved.plugin.get(), orig_plugin); + EXPECT_EQ(moved.update_provider, orig_update); + + // Moved-from object is empty + EXPECT_EQ(result->plugin, nullptr); + EXPECT_EQ(result->update_provider, nullptr); + EXPECT_EQ(result->introspection_provider, nullptr); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, MoveAssignmentTransfersOwnership) { + auto result = PluginLoader::load(test_plugin_path()); + ASSERT_TRUE(result.has_value()) << result.error(); + + auto * orig_plugin = result->plugin.get(); + + GatewayPluginLoadResult assigned; + assigned = std::move(*result); + + EXPECT_EQ(assigned.plugin.get(), orig_plugin); + EXPECT_NE(assigned.plugin, nullptr); + + // Moved-from is empty + EXPECT_EQ(result->plugin, nullptr); + EXPECT_EQ(result->update_provider, nullptr); +} + +// @verifies REQ_INTEROP_012 +TEST(TestPluginLoader, LoadPluginsSuccessPath) { + // Test load_plugins() through PluginManager with real .so file + PluginManager mgr; + std::vector configs = {{"test", test_plugin_path(), nlohmann::json::object()}}; + auto loaded = mgr.load_plugins(configs); + EXPECT_EQ(loaded, 1u); + EXPECT_TRUE(mgr.has_plugins()); + EXPECT_EQ(mgr.plugin_names()[0], "test_plugin"); + EXPECT_NE(mgr.get_update_provider(), nullptr); + EXPECT_EQ(mgr.get_introspection_providers().size(), 1u); +} diff --git a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp index 755e2f4d..04e41a02 100644 --- a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp @@ -13,8 +13,10 @@ // limitations under the License. #include +#include #include "ros2_medkit_gateway/plugins/plugin_manager.hpp" +#include "ros2_medkit_gateway/providers/introspection_provider.hpp" using namespace ros2_medkit_gateway; using json = nlohmann::json; @@ -90,6 +92,71 @@ class MockThrowingPlugin : public GatewayPlugin { } }; +/// Plugin that throws during set_node +class MockThrowOnSetNode : public GatewayPlugin, public UpdateProvider { + public: + std::string name() const override { + return "throw_set_node"; + } + void configure(const json &) override { + } + void set_node(rclcpp::Node *) override { + throw std::runtime_error("set_node failed"); + } + + tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { + return std::vector{}; + } + tl::expected get_update(const std::string &) override { + return json::object(); + } + tl::expected register_update(const json &) override { + return {}; + } + tl::expected delete_update(const std::string &) override { + return {}; + } + tl::expected prepare(const std::string &, UpdateProgressReporter &) override { + return {}; + } + tl::expected execute(const std::string &, UpdateProgressReporter &) override { + return {}; + } + tl::expected supports_automated(const std::string &) override { + return false; + } +}; + +/// Plugin that throws during register_routes +class MockThrowOnRegisterRoutes : public GatewayPlugin, public IntrospectionProvider { + public: + std::string name() const override { + return "throw_register_routes"; + } + void configure(const json &) override { + } + void register_routes(httplib::Server &, const std::string &) override { + throw std::runtime_error("register_routes failed"); + } + + IntrospectionResult introspect(const IntrospectionInput &) override { + return {}; + } +}; + +/// Plugin that throws during shutdown +class MockThrowOnShutdown : public GatewayPlugin { + public: + std::string name() const override { + return "throw_shutdown"; + } + void configure(const json &) override { + } + void shutdown() override { + throw std::runtime_error("shutdown failed"); + } +}; + // @verifies REQ_INTEROP_012 TEST(PluginManagerTest, EmptyManagerHasNoPlugins) { PluginManager mgr; @@ -199,3 +266,67 @@ TEST(PluginManagerTest, LoadNonexistentPluginReturnsZero) { EXPECT_EQ(loaded, 0u); EXPECT_FALSE(mgr.has_plugins()); } + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ThrowOnSetNodeDisablesPlugin) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + auto good = std::make_unique(); + auto * good_raw = good.get(); + mgr.add_plugin(std::move(good)); + + mgr.configure_plugins(); + mgr.set_node(nullptr); + + // Throwing plugin disabled, good plugin's UpdateProvider still works + EXPECT_EQ(mgr.get_update_provider(), static_cast(good_raw)); + // Only good plugin remains active + EXPECT_EQ(mgr.plugin_names().size(), 1u); + EXPECT_EQ(mgr.plugin_names()[0], "mock"); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ThrowOnRegisterRoutesDisablesPlugin) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + auto good = std::make_unique(); + mgr.add_plugin(std::move(good)); + + mgr.configure_plugins(); + httplib::Server srv; + mgr.register_routes(srv, "/api/v1"); + + // Throwing plugin disabled, good plugin's IntrospectionProvider still works + EXPECT_EQ(mgr.get_introspection_providers().size(), 1u); + EXPECT_EQ(mgr.plugin_names().size(), 1u); + EXPECT_EQ(mgr.plugin_names()[0], "introspection_only"); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ShutdownAllIdempotent) { + PluginManager mgr; + auto plugin = std::make_unique(); + auto * raw = plugin.get(); + mgr.add_plugin(std::move(plugin)); + + mgr.shutdown_all(); + EXPECT_TRUE(raw->shutdown_called_); + + // Reset flag, call again - should be a no-op + raw->shutdown_called_ = false; + mgr.shutdown_all(); + EXPECT_FALSE(raw->shutdown_called_); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, ShutdownSwallowsExceptions) { + PluginManager mgr; + mgr.add_plugin(std::make_unique()); + auto good = std::make_unique(); + auto * good_raw = good.get(); + mgr.add_plugin(std::move(good)); + + // Should not throw, even though first plugin throws during shutdown + EXPECT_NO_THROW(mgr.shutdown_all()); + EXPECT_TRUE(good_raw->shutdown_called_); +} From 1fc928b133ee6605788db3c505516fcbb19b9d4d Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 25 Feb 2026 16:00:00 +0000 Subject: [PATCH 12/14] feat(plugins): add PluginContext for gateway data access and capability registration Replace set_node(rclcpp::Node*) with set_context(PluginContext&) to give plugins access to entity cache, fault data, HTTP utilities, and capability registration. Plugins can now register custom capabilities per entity type or per specific entity, which appear in discovery responses with auto-generated hrefs. --- docs/changelog.rst | 2 + docs/tutorials/plugin-system.rst | 57 +++++- src/ros2_medkit_gateway/CMakeLists.txt | 1 + .../ros2_medkit_gateway/gateway_node.hpp | 2 + .../plugins/gateway_plugin.hpp | 18 +- .../plugins/plugin_context.hpp | 142 ++++++++++++++ .../plugins/plugin_manager.hpp | 23 ++- src/ros2_medkit_gateway/src/gateway_node.cpp | 3 +- .../src/http/handlers/discovery_handlers.cpp | 34 +++- .../src/plugins/plugin_context.cpp | 183 ++++++++++++++++++ .../src/plugins/plugin_manager.cpp | 9 +- .../test/test_plugin_manager.cpp | 20 +- 12 files changed, 459 insertions(+), 35 deletions(-) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_context.hpp create mode 100644 src/ros2_medkit_gateway/src/plugins/plugin_context.cpp diff --git a/docs/changelog.rst b/docs/changelog.rst index 5be3a053..96b28f39 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,8 @@ Added - Load plugins from shared libraries (``.so``) via ``plugins`` parameter - Provider interfaces: ``UpdateProvider``, ``IntrospectionProvider`` (preview) + - ``PluginContext`` gives plugins access to entity cache, fault data, and HTTP utilities + - Custom capability registration (per-type and per-entity) with discovery integration - Error isolation: failing plugins are disabled without crashing the gateway - RAII lifecycle with API version checking and path validation diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst index b71f605a..e77bf488 100644 --- a/docs/tutorials/plugin-system.rst +++ b/docs/tutorials/plugin-system.rst @@ -209,27 +209,76 @@ Plugin Lifecycle 3. ``create_plugin()`` factory function creates the plugin instance 4. Provider interfaces are queried via ``get_update_provider()`` / ``get_introspection_provider()`` 5. ``configure()`` is called with per-plugin JSON config -6. ``set_node()`` provides optional ROS 2 node access +6. ``set_context()`` provides ``PluginContext`` with ROS 2 node, entity cache, faults, and HTTP utilities 7. ``register_routes()`` allows registering custom REST endpoints 8. Runtime: subsystem managers call provider methods as needed 9. ``shutdown()`` is called before the plugin is destroyed +PluginContext +------------- + +After ``configure()``, the gateway calls ``set_context()`` with a ``PluginContext`` reference +providing access to gateway data and utilities: + +- ``node()`` - ROS 2 node pointer for subscriptions, service clients, timers, etc. +- ``get_entity(id)`` - look up any entity (area, component, app, function) from the discovery cache +- ``list_entity_faults(entity_id)`` - query faults for an entity +- ``validate_entity_for_route(req, res, entity_id)`` - validate entity exists and matches the route type, auto-sending SOVD errors on failure +- ``send_error()`` / ``send_json()`` - SOVD-compliant HTTP response helpers (static methods) +- ``register_capability()`` / ``register_entity_capability()`` - register custom capabilities on entities + +.. code-block:: cpp + + void set_context(PluginContext& ctx) override { + ctx_ = &ctx; + + // Register a custom capability for all apps + ctx.register_capability(SovdEntityType::APP, "x-medkit-traces"); + + // Register a capability for a specific entity + ctx.register_entity_capability("sensor1", "x-medkit-calibration"); + } + + PluginContext* ctx_ = nullptr; + +.. note:: + + The ``PluginContext`` interface is versioned alongside ``PLUGIN_API_VERSION``. + Additional methods (entity data access, configuration queries, etc.) may be added + in future versions. + Custom REST Endpoints --------------------- -Any plugin can register vendor-specific endpoints via ``register_routes()``: +Any plugin can register vendor-specific endpoints via ``register_routes()``. +Use ``PluginContext`` utilities for entity validation and SOVD-compliant responses: .. code-block:: cpp void register_routes(httplib::Server& server, const std::string& api_prefix) override { + // Global vendor endpoint server.Get(api_prefix + "/x-myvendor/status", [this](const httplib::Request&, httplib::Response& res) { - res.set_content(get_status_json(), "application/json"); + PluginContext::send_json(res, get_status_json()); + }); + + // Entity-scoped endpoint (matches a registered capability) + server.Get((api_prefix + R"(/apps/([^/]+)/x-medkit-traces)").c_str(), + [this](const httplib::Request& req, httplib::Response& res) { + auto entity = ctx_->validate_entity_for_route(req, res, req.matches[1]); + if (!entity) return; // Error already sent + + auto faults = ctx_->list_entity_faults(entity->id); + PluginContext::send_json(res, {{"entity", entity->id}, {"faults", faults}}); }); } Use the ``x-`` prefix for vendor-specific endpoints per SOVD convention. +For entity-scoped endpoints, register a matching capability via ``register_capability()`` +or ``register_entity_capability()`` in ``set_context()`` so the endpoint appears in the +entity's capabilities array in discovery responses. + Multiple Plugins ---------------- @@ -242,7 +291,7 @@ Multiple plugins can be loaded simultaneously: Error Handling -------------- -If a plugin throws during any lifecycle method (``configure``, ``set_node``, ``register_routes``, +If a plugin throws during any lifecycle method (``configure``, ``set_context``, ``register_routes``, ``shutdown``), the exception is caught and logged. The plugin is disabled but the gateway continues operating. A failing plugin never crashes the gateway. diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 1f32c0fa..f50dab2f 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -133,6 +133,7 @@ add_library(gateway_lib STATIC # HTTP handlers - updates src/http/handlers/update_handlers.cpp # Plugin framework + src/plugins/plugin_context.cpp src/plugins/plugin_loader.cpp src/plugins/plugin_manager.cpp ) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp index 91999534..c2ea6e1b 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp @@ -131,6 +131,8 @@ class GatewayNode : public rclcpp::Node { std::unique_ptr subscription_mgr_; // IMPORTANT: plugin_mgr_ BEFORE update_mgr_ - C++ destroys in reverse order, // so update_mgr_ waits for async tasks before plugin_mgr_ destroys the plugin. + // plugin_ctx_ is owned here (outlives plugins); plugin_mgr_ holds a non-owning ref. + std::unique_ptr plugin_ctx_; std::unique_ptr plugin_mgr_; std::unique_ptr update_mgr_; std::unique_ptr rest_server_; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp index 394dd13d..6caae1c0 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp @@ -21,12 +21,10 @@ #include #include -namespace rclcpp { -class Node; -} - namespace ros2_medkit_gateway { +class PluginContext; + /** * @brief Base class for all gateway plugins * @@ -61,15 +59,15 @@ class GatewayPlugin { virtual void configure(const nlohmann::json & config) = 0; /** - * @brief Optionally receive a ROS 2 node + * @brief Receive gateway context * - * Called after configure(). Plugins that need ROS 2 access - * (subscriptions, service clients) can use this node. - * Platform-only plugins can ignore this. + * Called after configure(). Provides access to the ROS 2 node, + * entity cache, fault data, and HTTP handler utilities. + * Store the reference if needed during runtime. * - * @param node ROS 2 node pointer (must outlive this plugin) + * @param context Gateway plugin context (outlives this plugin) */ - virtual void set_node(rclcpp::Node * /*node*/) { + virtual void set_context(PluginContext & /*context*/) { } /** diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_context.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_context.hpp new file mode 100644 index 00000000..f0db1a31 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_context.hpp @@ -0,0 +1,142 @@ +// 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/models/entity_types.hpp" + +#include +#include +#include +#include +#include +#include +#include + +namespace rclcpp { +class Node; +} + +namespace ros2_medkit_gateway { + +/** + * @brief Entity information exposed to plugins + */ +struct PluginEntityInfo { + SovdEntityType type{SovdEntityType::UNKNOWN}; + std::string id; + std::string namespace_path; ///< ROS 2 namespace (for component fault filtering) + std::string fqn; ///< Fully qualified ROS 2 node name +}; + +/** + * @brief Context interface providing plugins access to gateway data and utilities + * + * Passed to plugins during lifecycle via set_context(). Replaces the old set_node() + * by providing both ROS 2 node access and gateway-level abstractions. + * + * @note This interface is versioned alongside PLUGIN_API_VERSION. New methods may + * be added in future versions (entity data access, configuration queries, etc.). + * + * @par Thread Safety + * All methods are safe to call from any thread. Entity and fault queries use + * the gateway's thread-safe caches internally. + */ +class PluginContext { + public: + virtual ~PluginContext() = default; + + // ---- ROS 2 access (replaces set_node) ---- + + /// Get ROS 2 node pointer for subscriptions, service clients, etc. + virtual rclcpp::Node * node() const = 0; + + // ---- Entity access (read-only) ---- + + /// Look up an entity by ID. Returns nullopt if not found. + virtual std::optional get_entity(const std::string & id) const = 0; + + // ---- Fault access ---- + + /// List faults for a given entity. Returns JSON array of fault objects. + /// Empty array if entity has no faults or fault manager is unavailable. + virtual nlohmann::json list_entity_faults(const std::string & entity_id) const = 0; + + // ---- HTTP handler utilities (for entity-scoped routes) ---- + + /** + * @brief Validate entity exists and matches route type, sending SOVD error if not + * + * Use this in register_routes() handlers to validate entity IDs from path params. + * On failure, an appropriate SOVD GenericError response is sent automatically. + * + * @param req HTTP request (extracts expected entity type from path) + * @param res HTTP response (error sent here on failure) + * @param entity_id Entity ID from path parameter (e.g., req.matches[1]) + * @return Entity info if valid, nullopt if error was sent + */ + virtual std::optional validate_entity_for_route(const httplib::Request & req, + httplib::Response & res, + const std::string & entity_id) const = 0; + + /// Send SOVD-compliant JSON error response + static void send_error(httplib::Response & res, int status, const std::string & error_code, + const std::string & message, const nlohmann::json & parameters = {}); + + /// Send JSON success response + static void send_json(httplib::Response & res, const nlohmann::json & data); + + // ---- Capability registration ---- + + /** + * @brief Register a custom capability for all entities of a given type + * + * The capability will appear in the entity's capabilities array with an + * auto-generated href. For example, registering "x-medkit-traces" for + * SovdEntityType::APP produces: {"name": "x-medkit-traces", "href": "/api/v1/apps/{id}/x-medkit-traces"} + * + * The plugin must also register a matching route in register_routes(). + * + * @param entity_type Entity type to add the capability to + * @param capability_name Capability name (use x- prefix for vendor extensions) + */ + virtual void register_capability(SovdEntityType entity_type, const std::string & capability_name) = 0; + + /** + * @brief Register a custom capability for a specific entity + * + * Like register_capability(entity_type, name) but scoped to a single entity. + * + * @param entity_id Specific entity ID + * @param capability_name Capability name (use x- prefix for vendor extensions) + */ + virtual void register_entity_capability(const std::string & entity_id, const std::string & capability_name) = 0; + + // ---- Capability query (used by discovery handlers) ---- + + /// Get plugin-registered capabilities for an entity type + virtual std::vector get_type_capabilities(SovdEntityType entity_type) const = 0; + + /// Get plugin-registered capabilities for a specific entity + virtual std::vector get_entity_capabilities(const std::string & entity_id) const = 0; +}; + +// Forward declarations +class GatewayNode; +class FaultManager; + +/// Factory for creating the concrete gateway plugin context +std::unique_ptr make_gateway_plugin_context(GatewayNode * node, FaultManager * fault_manager); + +} // namespace ros2_medkit_gateway 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 3c59a1ab..5b46cdd3 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 @@ -15,6 +15,7 @@ #pragma once #include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" +#include "ros2_medkit_gateway/plugins/plugin_context.hpp" #include "ros2_medkit_gateway/plugins/plugin_loader.hpp" #include "ros2_medkit_gateway/plugins/plugin_types.hpp" #include "ros2_medkit_gateway/providers/introspection_provider.hpp" @@ -26,10 +27,6 @@ #include #include -namespace rclcpp { -class Node; -} - namespace ros2_medkit_gateway { /** @@ -83,10 +80,14 @@ class PluginManager { void configure_plugins(); /** - * @brief Set ROS 2 node on all plugins - * @param node ROS 2 node pointer (must outlive all plugins) + * @brief Set plugin context on all plugins + * + * Passes the gateway context (entity cache, faults, ROS 2 node, HTTP utils) + * to each plugin via set_context(). Replaces the old set_node() method. + * + * @param context Plugin context (must outlive all plugins) */ - void set_node(rclcpp::Node * node); + void set_context(PluginContext & context); /** * @brief Register custom REST routes from all plugins @@ -114,6 +115,13 @@ class PluginManager { */ std::vector get_introspection_providers() const; + // ---- Capability queries (used by discovery handlers) ---- + + /// Get plugin context (for capability queries from discovery handlers) + PluginContext * get_context() const { + return context_; + } + // ---- Info ---- bool has_plugins() const; std::vector plugin_names() const; @@ -133,6 +141,7 @@ class PluginManager { void disable_plugin(LoadedPlugin & lp); std::vector plugins_; + PluginContext * context_ = nullptr; bool shutdown_called_ = false; }; diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 72b6ac4f..8273dd30 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -356,7 +356,8 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { } auto loaded = plugin_mgr_->load_plugins(configs); plugin_mgr_->configure_plugins(); - plugin_mgr_->set_node(this); + 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); } diff --git a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp index 27bd1828..c19163ec 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp @@ -28,6 +28,33 @@ using json = nlohmann::json; namespace ros2_medkit_gateway { namespace handlers { +namespace { + +/// Append plugin-registered capabilities to a capabilities JSON array +void append_plugin_capabilities(json & capabilities, const std::string & entity_type_path, + const std::string & entity_id, SovdEntityType entity_type, const GatewayNode * node) { + auto * pmgr = node->get_plugin_manager(); + if (!pmgr || !pmgr->get_context()) { + return; + } + auto * ctx = pmgr->get_context(); + std::string href_prefix; + href_prefix.reserve(64); + href_prefix.append("/api/v1/").append(entity_type_path).append("/").append(entity_id).append("/"); + + // Type-level capabilities (registered for all entities of this type) + for (const auto & cap_name : ctx->get_type_capabilities(entity_type)) { + capabilities.push_back({{"name", cap_name}, {"href", href_prefix + cap_name}}); + } + + // Entity-specific capabilities + for (const auto & cap_name : ctx->get_entity_capabilities(entity_id)) { + capabilities.push_back({{"name", cap_name}, {"href", href_prefix + cap_name}}); + } +} + +} // namespace + // ============================================================================= // Area handlers // ============================================================================= @@ -127,6 +154,7 @@ void DiscoveryHandlers::handle_get_area(const httplib::Request & req, httplib::R std::vector caps = {Cap::SUBAREAS, Cap::CONTAINS, Cap::DATA, Cap::OPERATIONS, Cap::CONFIGURATIONS, Cap::FAULTS}; response["capabilities"] = CapabilityBuilder::build_capabilities("areas", area.id, caps); + append_plugin_capabilities(response["capabilities"], "areas", area.id, SovdEntityType::AREA, ctx_.node()); LinksBuilder links; links.self("/api/v1/areas/" + area.id).collection("/api/v1/areas"); @@ -466,7 +494,9 @@ void DiscoveryHandlers::handle_get_component(const httplib::Request & req, httpl if (!comp.depends_on.empty()) { caps.push_back(Cap::DEPENDS_ON); } - ext.add("capabilities", CapabilityBuilder::build_capabilities("components", comp.id, caps)); + auto comp_caps = CapabilityBuilder::build_capabilities("components", comp.id, caps); + append_plugin_capabilities(comp_caps, "components", comp.id, SovdEntityType::COMPONENT, ctx_.node()); + ext.add("capabilities", comp_caps); response["x-medkit"] = ext.build(); HandlerContext::send_json(res, response); @@ -784,6 +814,7 @@ void DiscoveryHandlers::handle_get_app(const httplib::Request & req, httplib::Re using Cap = CapabilityBuilder::Capability; std::vector caps = {Cap::DATA, Cap::OPERATIONS, Cap::CONFIGURATIONS, Cap::FAULTS}; response["capabilities"] = CapabilityBuilder::build_capabilities("apps", app.id, caps); + append_plugin_capabilities(response["capabilities"], "apps", app.id, SovdEntityType::APP, ctx_.node()); LinksBuilder links; links.self("/api/v1/apps/" + app.id).collection("/api/v1/apps"); @@ -984,6 +1015,7 @@ void DiscoveryHandlers::handle_get_function(const httplib::Request & req, httpli using Cap = CapabilityBuilder::Capability; std::vector caps = {Cap::HOSTS, Cap::DATA, Cap::OPERATIONS, Cap::CONFIGURATIONS, Cap::FAULTS}; response["capabilities"] = CapabilityBuilder::build_capabilities("functions", func.id, caps); + append_plugin_capabilities(response["capabilities"], "functions", func.id, SovdEntityType::FUNCTION, ctx_.node()); LinksBuilder links; links.self("/api/v1/functions/" + func.id).collection("/api/v1/functions"); diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp new file mode 100644 index 00000000..5422b48c --- /dev/null +++ b/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp @@ -0,0 +1,183 @@ +// 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/plugins/plugin_context.hpp" + +#include "ros2_medkit_gateway/fault_manager.hpp" +#include "ros2_medkit_gateway/gateway_node.hpp" +#include "ros2_medkit_gateway/http/error_codes.hpp" +#include "ros2_medkit_gateway/http/handlers/handler_context.hpp" + +#include +#include + +namespace ros2_medkit_gateway { + +// ---- Static utility methods (delegate to HandlerContext) ---- + +void PluginContext::send_error(httplib::Response & res, int status, const std::string & error_code, + const std::string & message, const nlohmann::json & parameters) { + handlers::HandlerContext::send_error(res, status, error_code, message, parameters); +} + +void PluginContext::send_json(httplib::Response & res, const nlohmann::json & data) { + handlers::HandlerContext::send_json(res, data); +} + +// ---- Concrete implementation ---- + +class GatewayPluginContext : public PluginContext { + public: + GatewayPluginContext(GatewayNode * node, FaultManager * fault_manager) : node_(node), fault_manager_(fault_manager) { + } + + rclcpp::Node * node() const override { + return node_; + } + + std::optional get_entity(const std::string & id) const override { + const auto & cache = node_->get_thread_safe_cache(); + + if (auto comp = cache.get_component(id)) { + return PluginEntityInfo{SovdEntityType::COMPONENT, id, comp->namespace_path, comp->fqn}; + } + if (auto app = cache.get_app(id)) { + return PluginEntityInfo{SovdEntityType::APP, id, {}, app->bound_fqn.value_or("")}; + } + if (auto area = cache.get_area(id)) { + return PluginEntityInfo{SovdEntityType::AREA, id, area->namespace_path, {}}; + } + if (cache.get_function(id)) { + return PluginEntityInfo{SovdEntityType::FUNCTION, id, {}, {}}; + } + return std::nullopt; + } + + nlohmann::json list_entity_faults(const std::string & entity_id) const override { + if (!fault_manager_ || !fault_manager_->is_available()) { + return nlohmann::json::array(); + } + + // Determine source_id for fault filtering based on entity type + auto entity = get_entity(entity_id); + if (!entity) { + return nlohmann::json::array(); + } + + std::string source_id; + if (entity->type == SovdEntityType::COMPONENT) { + source_id = entity->namespace_path; + } else if (entity->type == SovdEntityType::APP) { + source_id = entity->fqn; + } + + auto result = fault_manager_->list_faults(source_id); + if (result.success && result.data.is_array()) { + return result.data; + } + return nlohmann::json::array(); + } + + std::optional validate_entity_for_route(const httplib::Request & req, httplib::Response & res, + const std::string & entity_id) const override { + // Validate entity ID format + if (entity_id.empty() || entity_id.size() > 256) { + send_error(res, 400, ERR_INVALID_PARAMETER, "Invalid entity ID"); + return std::nullopt; + } + + // Determine expected type from route path + auto expected_type = SovdEntityType::UNKNOWN; + auto path = req.path; + if (path.find("/components/") != std::string::npos) { + expected_type = SovdEntityType::COMPONENT; + } else if (path.find("/apps/") != std::string::npos) { + expected_type = SovdEntityType::APP; + } else if (path.find("/areas/") != std::string::npos) { + expected_type = SovdEntityType::AREA; + } else if (path.find("/functions/") != std::string::npos) { + expected_type = SovdEntityType::FUNCTION; + } + + auto entity = get_entity(entity_id); + if (!entity) { + send_error(res, 404, ERR_ENTITY_NOT_FOUND, to_string(expected_type) + " not found: " + entity_id); + return std::nullopt; + } + + // Check type matches route + if (expected_type != SovdEntityType::UNKNOWN && entity->type != expected_type) { + send_error(res, 400, ERR_INVALID_PARAMETER, + "Entity '" + entity_id + "' is a " + to_string(entity->type) + ", not a " + to_string(expected_type)); + return std::nullopt; + } + + return entity; + } + + void register_capability(SovdEntityType entity_type, const std::string & capability_name) override { + std::lock_guard lock(capabilities_mutex_); + auto & caps = type_capabilities_[entity_type]; + // Avoid duplicates + for (const auto & c : caps) { + if (c == capability_name) { + return; + } + } + caps.push_back(capability_name); + } + + void register_entity_capability(const std::string & entity_id, const std::string & capability_name) override { + std::lock_guard lock(capabilities_mutex_); + auto & caps = entity_capabilities_[entity_id]; + for (const auto & c : caps) { + if (c == capability_name) { + return; + } + } + caps.push_back(capability_name); + } + + std::vector get_type_capabilities(SovdEntityType entity_type) const override { + std::lock_guard lock(capabilities_mutex_); + auto it = type_capabilities_.find(entity_type); + if (it != type_capabilities_.end()) { + return it->second; + } + return {}; + } + + std::vector get_entity_capabilities(const std::string & entity_id) const override { + std::lock_guard lock(capabilities_mutex_); + auto it = entity_capabilities_.find(entity_id); + if (it != entity_capabilities_.end()) { + return it->second; + } + return {}; + } + + private: + GatewayNode * node_; + FaultManager * fault_manager_; + + mutable std::mutex capabilities_mutex_; + std::unordered_map> type_capabilities_; + std::unordered_map> entity_capabilities_; +}; + +std::unique_ptr make_gateway_plugin_context(GatewayNode * node, FaultManager * fault_manager) { + return std::make_unique(node, fault_manager); +} + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp index 4a1c2220..a8b25472 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp @@ -113,19 +113,20 @@ void PluginManager::configure_plugins() { } } -void PluginManager::set_node(rclcpp::Node * node) { +void PluginManager::set_context(PluginContext & context) { + context_ = &context; for (auto & lp : plugins_) { if (!lp.load_result.plugin) { continue; } try { - lp.load_result.plugin->set_node(node); + lp.load_result.plugin->set_context(context); } catch (const std::exception & e) { - RCLCPP_ERROR(logger(), "Plugin '%s' threw during set_node(): %s - disabling", + RCLCPP_ERROR(logger(), "Plugin '%s' threw during set_context(): %s - disabling", lp.load_result.plugin->name().c_str(), e.what()); disable_plugin(lp); } catch (...) { - RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during set_node() - disabling", + RCLCPP_ERROR(logger(), "Plugin '%s' threw unknown exception during set_context() - disabling", lp.load_result.plugin->name().c_str()); disable_plugin(lp); } diff --git a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp index 04e41a02..e8c411d9 100644 --- a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp @@ -15,6 +15,7 @@ #include #include +#include "ros2_medkit_gateway/plugins/plugin_context.hpp" #include "ros2_medkit_gateway/plugins/plugin_manager.hpp" #include "ros2_medkit_gateway/providers/introspection_provider.hpp" @@ -92,16 +93,16 @@ class MockThrowingPlugin : public GatewayPlugin { } }; -/// Plugin that throws during set_node -class MockThrowOnSetNode : public GatewayPlugin, public UpdateProvider { +/// Plugin that throws during set_context +class MockThrowOnSetContext : public GatewayPlugin, public UpdateProvider { public: std::string name() const override { - return "throw_set_node"; + return "throw_set_context"; } void configure(const json &) override { } - void set_node(rclcpp::Node *) override { - throw std::runtime_error("set_node failed"); + void set_context(PluginContext &) override { + throw std::runtime_error("set_context failed"); } tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { @@ -268,15 +269,18 @@ TEST(PluginManagerTest, LoadNonexistentPluginReturnsZero) { } // @verifies REQ_INTEROP_012 -TEST(PluginManagerTest, ThrowOnSetNodeDisablesPlugin) { +TEST(PluginManagerTest, ThrowOnSetContextDisablesPlugin) { PluginManager mgr; - mgr.add_plugin(std::make_unique()); + mgr.add_plugin(std::make_unique()); auto good = std::make_unique(); auto * good_raw = good.get(); mgr.add_plugin(std::move(good)); mgr.configure_plugins(); - mgr.set_node(nullptr); + + // Create a minimal PluginContext for the test + auto ctx = make_gateway_plugin_context(nullptr, nullptr); + mgr.set_context(*ctx); // Throwing plugin disabled, good plugin's UpdateProvider still works EXPECT_EQ(mgr.get_update_provider(), static_cast(good_raw)); From a2f5bad091369ad78ecc7372110fb68d35308cf9 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 25 Feb 2026 17:31:19 +0000 Subject: [PATCH 13/14] feat(plugins): forward YAML config parameters to plugin configure() Scan parameter overrides for plugins..* keys (excluding .path), convert ROS 2 parameter types to JSON, and pass the resulting object to each plugin's configure() method. Supports all ROS 2 parameter types. --- docs/tutorials/plugin-system.rst | 19 ++++++- .../config/gateway_params.yaml | 7 ++- src/ros2_medkit_gateway/src/gateway_node.cpp | 55 ++++++++++++++++++- .../test/test_plugin_manager.cpp | 15 +++++ 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst index e77bf488..a1247c0b 100644 --- a/docs/tutorials/plugin-system.rst +++ b/docs/tutorials/plugin-system.rst @@ -27,17 +27,32 @@ Add plugins to ``gateway_params.yaml``: ros__parameters: plugins: ["my_ota_plugin"] plugins.my_ota_plugin.path: "/opt/ros2_medkit/lib/libmy_ota_plugin.so" + plugins.my_ota_plugin.server_url: "https://updates.example.com" + plugins.my_ota_plugin.api_key: "secret123" + plugins.my_ota_plugin.timeout_ms: 5000 # Enable updates if your plugin implements UpdateProvider updates: enabled: true Each plugin name in the ``plugins`` array requires a corresponding ``plugins..path`` -parameter with the absolute path to the ``.so`` file. Plugins are loaded in the order listed. -An empty list (default) means no plugins are loaded, with zero overhead. +parameter with the absolute path to the ``.so`` file. Any additional ``plugins..`` +parameters are collected into a JSON object and passed to the plugin's ``configure()`` method. + +For the example above, ``configure()`` receives: + +.. code-block:: json + + {"server_url": "https://updates.example.com", "api_key": "secret123", "timeout_ms": 5000} + +Plugins are loaded in the order listed. An empty list (default) means no plugins are loaded, +with zero overhead. Plugin names must contain only alphanumeric characters, underscores, and hyphens (max 256 chars). +All standard ROS 2 parameter types are supported: strings, integers, doubles, booleans, +and their array variants. They are automatically converted to their JSON equivalents. + Writing a Plugin ---------------- diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index d046ed54..642f3e69 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -241,12 +241,15 @@ ros2_medkit_gateway: # Plugins can implement provider interfaces (UpdateProvider, IntrospectionProvider) # that are automatically detected and wired into subsystem managers. # - # List plugin names, then configure each with a plugins..path parameter. + # List plugin names, then configure each with plugins..path and + # any additional plugins.. parameters (passed as JSON config). # Example: # plugins: ["my_ota_plugin"] # plugins.my_ota_plugin.path: "/opt/ros2_medkit/lib/libmy_ota_plugin.so" + # plugins.my_ota_plugin.server_url: "https://updates.example.com" + # plugins.my_ota_plugin.timeout_ms: 5000 # - # Plugin lifecycle: load -> configure -> set_node -> register_routes + # Plugin lifecycle: load -> configure -> set_context -> register_routes # Plugins that throw during any lifecycle call are disabled (not crashed). # Default: [] (no plugins) plugins: [] diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 8273dd30..6a41f997 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -22,6 +22,55 @@ using namespace std::chrono_literals; namespace ros2_medkit_gateway { +namespace { + +/// Convert an rclcpp::Parameter to a nlohmann::json value +nlohmann::json parameter_to_json(const rclcpp::Parameter & param) { + switch (param.get_type()) { + case rclcpp::ParameterType::PARAMETER_BOOL: + return param.as_bool(); + case rclcpp::ParameterType::PARAMETER_INTEGER: + return param.as_int(); + case rclcpp::ParameterType::PARAMETER_DOUBLE: + return param.as_double(); + case rclcpp::ParameterType::PARAMETER_STRING: + return param.as_string(); + case rclcpp::ParameterType::PARAMETER_BYTE_ARRAY: + return param.as_byte_array(); + case rclcpp::ParameterType::PARAMETER_BOOL_ARRAY: + return param.as_bool_array(); + case rclcpp::ParameterType::PARAMETER_INTEGER_ARRAY: + return param.as_integer_array(); + case rclcpp::ParameterType::PARAMETER_DOUBLE_ARRAY: + return param.as_double_array(); + case rclcpp::ParameterType::PARAMETER_STRING_ARRAY: + return param.as_string_array(); + default: + return nullptr; + } +} + +/// Extract per-plugin config from YAML parameter overrides. +/// Scans for keys matching "plugins.." (excluding ".path") +/// and builds a flat JSON object: {"": value, ...} +nlohmann::json extract_plugin_config(const std::vector & overrides, + const std::string & plugin_name) { + auto config = nlohmann::json::object(); + std::string prefix = "plugins." + plugin_name + "."; + std::string path_key = prefix + "path"; + + for (const auto & param : overrides) { + const auto & name = param.get_name(); + if (name.rfind(prefix, 0) == 0 && name != path_key) { + auto key = name.substr(prefix.size()); + config[key] = parameter_to_json(param); + } + } + return config; +} + +} // namespace + GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { RCLCPP_INFO(get_logger(), "Initializing ROS 2 Medkit Gateway..."); @@ -352,7 +401,11 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { RCLCPP_ERROR(get_logger(), "Plugin '%s' has no path configured", pname.c_str()); continue; } - configs.push_back({pname, path, nlohmann::json::object()}); + auto plugin_config = extract_plugin_config(get_node_options().parameter_overrides(), pname); + if (!plugin_config.empty()) { + RCLCPP_INFO(get_logger(), "Plugin '%s' config: %zu key(s)", pname.c_str(), plugin_config.size()); + } + configs.push_back({pname, path, std::move(plugin_config)}); } auto loaded = plugin_mgr_->load_plugins(configs); plugin_mgr_->configure_plugins(); diff --git a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp index e8c411d9..06c452c8 100644 --- a/src/ros2_medkit_gateway/test/test_plugin_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_plugin_manager.cpp @@ -191,6 +191,21 @@ TEST(PluginManagerTest, ConfigurePassesConfig) { mgr.configure_plugins(); EXPECT_TRUE(raw->configured_); + // add_plugin() uses empty config by default + EXPECT_TRUE(raw->config_.is_object()); + EXPECT_TRUE(raw->config_.empty()); +} + +// @verifies REQ_INTEROP_012 +TEST(PluginManagerTest, LoadPluginsForwardsConfig) { + // load_plugins() should forward the PluginConfig.config to configure() + PluginManager mgr; + json cfg = {{"server_url", "https://example.com"}, {"timeout_ms", 5000}}; + std::vector configs = {{"test", "/nonexistent/path.so", cfg}}; + + // Plugin won't load (bad path), but verify PluginConfig struct holds config + EXPECT_EQ(configs[0].config["server_url"], "https://example.com"); + EXPECT_EQ(configs[0].config["timeout_ms"], 5000); } // @verifies REQ_INTEROP_012 From 21c96635221beaa1c6f52cc466b2b207456ce561 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Wed, 25 Feb 2026 20:13:40 +0000 Subject: [PATCH 14/14] fix(plugins): address review round 3 - safety, correctness, and docs - Cast to unsigned char before std::isalnum to prevent UB on signed char - Deduplicate plugin names to avoid ParameterAlreadyDeclaredException crash - Call shutdown() in disable_plugin() to honor lifecycle contract - Add try/catch around provider query functions in plugin_loader - Use extract_entity_type_from_path() for proper segment-boundary matching - Cache first UpdateProvider in load_plugins(), log warning once on duplicates - Fix test plugin return types from void* to GatewayPlugin* - Add temporal ordering doc to set_backend() - Fix stale set_node() reference in server.rst - Clarify RTTI scope in plugin-system.rst - Document .so extension intent in plugin_loader --- docs/config/server.rst | 2 +- docs/tutorials/plugin-system.rst | 4 +- .../plugins/plugin_manager.hpp | 1 + .../updates/update_manager.hpp | 3 +- src/ros2_medkit_gateway/src/gateway_node.cpp | 8 ++- .../src/plugins/plugin_context.cpp | 15 +---- .../src/plugins/plugin_loader.cpp | 23 +++++++- .../src/plugins/plugin_manager.cpp | 55 ++++++++++++++----- .../demo_nodes/test_bad_version_plugin.cpp | 3 +- .../demo_nodes/test_null_factory_plugin.cpp | 3 +- 10 files changed, 82 insertions(+), 35 deletions(-) diff --git a/docs/config/server.rst b/docs/config/server.rst index dc80e221..6eb8e7d1 100644 --- a/docs/config/server.rst +++ b/docs/config/server.rst @@ -286,7 +286,7 @@ Plugin loading lifecycle: 3. ``create_plugin()`` factory is called to instantiate the plugin 4. Provider interfaces are queried via ``extern "C"`` functions 5. ``configure()`` is called with per-plugin config -6. ``set_node()`` provides access to the ROS 2 node +6. ``set_context()`` passes the gateway context to the plugin 7. ``register_routes()`` allows the plugin to add custom REST endpoints Error isolation: if a plugin throws during any lifecycle call, it is disabled diff --git a/docs/tutorials/plugin-system.rst b/docs/tutorials/plugin-system.rst index a1247c0b..a4f811ce 100644 --- a/docs/tutorials/plugin-system.rst +++ b/docs/tutorials/plugin-system.rst @@ -324,5 +324,7 @@ Build Requirements ------------------ - **Same compiler and ABI** as the gateway executable -- **RTTI must be enabled** - do NOT compile plugins with ``-fno-rtti`` +- **RTTI must be enabled for in-process plugins** - the ``add_plugin()`` path uses ``dynamic_cast`` + to query provider interfaces, so ``-fno-rtti`` will break it. Shared-library plugins loaded via + ``PluginLoader::load()`` use ``extern "C"`` query functions and do not require RTTI. - The ``GATEWAY_PLUGIN_EXPORT`` macro ensures correct symbol visibility 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 5b46cdd3..c764c457 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 @@ -142,6 +142,7 @@ class PluginManager { std::vector plugins_; PluginContext * context_ = nullptr; + UpdateProvider * first_update_provider_ = nullptr; bool shutdown_called_ = false; }; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp index f6907077..77638725 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp @@ -65,7 +65,8 @@ class UpdateManager { UpdateManager(const UpdateManager &) = delete; UpdateManager & operator=(const UpdateManager &) = delete; - /// Set the backend provider (non-owning pointer, caller manages lifetime) + /// Set the backend provider (non-owning pointer, caller manages lifetime). + /// Must be called before any update operations; operations return NoBackend error until set. void set_backend(UpdateProvider * backend); /// Check if a backend is loaded diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 6a41f997..6a5656ef 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -17,6 +17,7 @@ #include #include #include +#include using namespace std::chrono_literals; @@ -384,10 +385,15 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { return false; } return std::all_of(name.begin(), name.end(), [](char c) { - return std::isalnum(c) || c == '_' || c == '-'; + return std::isalnum(static_cast(c)) || c == '_' || c == '-'; }); }; + std::unordered_set seen_names; for (const auto & pname : plugin_names) { + if (!seen_names.insert(pname).second) { + RCLCPP_WARN(get_logger(), "Duplicate plugin name '%s' - skipping", pname.c_str()); + continue; + } if (!is_valid_plugin_name(pname)) { RCLCPP_ERROR(get_logger(), "Invalid plugin name '%s': must be alphanumeric, underscore, or hyphen (max 256 chars)", diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp index 5422b48c..b38cf383 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_context.cpp @@ -18,6 +18,7 @@ #include "ros2_medkit_gateway/gateway_node.hpp" #include "ros2_medkit_gateway/http/error_codes.hpp" #include "ros2_medkit_gateway/http/handlers/handler_context.hpp" +#include "ros2_medkit_gateway/http/http_utils.hpp" #include #include @@ -97,18 +98,8 @@ class GatewayPluginContext : public PluginContext { return std::nullopt; } - // Determine expected type from route path - auto expected_type = SovdEntityType::UNKNOWN; - auto path = req.path; - if (path.find("/components/") != std::string::npos) { - expected_type = SovdEntityType::COMPONENT; - } else if (path.find("/apps/") != std::string::npos) { - expected_type = SovdEntityType::APP; - } else if (path.find("/areas/") != std::string::npos) { - expected_type = SovdEntityType::AREA; - } else if (path.find("/functions/") != std::string::npos) { - expected_type = SovdEntityType::FUNCTION; - } + // Determine expected type from route path (segment-boundary-aware matching) + auto expected_type = extract_entity_type_from_path(req.path); auto entity = get_entity(entity_id); if (!entity) { diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp index 098d3336..f4bb3f2a 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp @@ -19,6 +19,7 @@ #include "ros2_medkit_gateway/providers/update_provider.hpp" #include +#include #include @@ -78,6 +79,8 @@ tl::expected PluginLoader::load(const std: return tl::make_unexpected("Plugin path must be absolute: " + plugin_path); } + // Gateway plugins are always unversioned .so files loaded by explicit path. + // Versioned sonames (libfoo.so.1) are a system packaging concern and not supported. if (fs_path.extension() != ".so") { return tl::make_unexpected("Plugin path must have .so extension: " + plugin_path); } @@ -158,13 +161,29 @@ tl::expected PluginLoader::load(const std: using UpdateProviderFn = UpdateProvider * (*)(GatewayPlugin *); auto update_fn = reinterpret_cast(dlsym(handle, "get_update_provider")); if (update_fn) { - result.update_provider = update_fn(raw_plugin); + try { + result.update_provider = update_fn(raw_plugin); + } catch (const std::exception & e) { + RCLCPP_WARN(rclcpp::get_logger("plugin_loader"), "get_update_provider threw in %s: %s", plugin_path.c_str(), + e.what()); + } catch (...) { + RCLCPP_WARN(rclcpp::get_logger("plugin_loader"), "get_update_provider threw unknown exception in %s", + plugin_path.c_str()); + } } using IntrospectionProviderFn = IntrospectionProvider * (*)(GatewayPlugin *); auto introspection_fn = reinterpret_cast(dlsym(handle, "get_introspection_provider")); if (introspection_fn) { - result.introspection_provider = introspection_fn(raw_plugin); + try { + result.introspection_provider = introspection_fn(raw_plugin); + } catch (const std::exception & e) { + RCLCPP_WARN(rclcpp::get_logger("plugin_loader"), "get_introspection_provider threw in %s: %s", + plugin_path.c_str(), e.what()); + } catch (...) { + RCLCPP_WARN(rclcpp::get_logger("plugin_loader"), "get_introspection_provider threw unknown exception in %s", + plugin_path.c_str()); + } } // Transfer handle ownership to result (disarm scope guard) diff --git a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp index a8b25472..afa19a80 100644 --- a/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp +++ b/src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp @@ -56,6 +56,15 @@ void PluginManager::add_plugin(std::unique_ptr plugin) { lp.update_provider = dynamic_cast(plugin.get()); lp.introspection_provider = dynamic_cast(plugin.get()); + // Cache first UpdateProvider, warn on duplicates + if (lp.update_provider) { + if (!first_update_provider_) { + first_update_provider_ = lp.update_provider; + } else { + RCLCPP_WARN(logger(), "Multiple UpdateProvider plugins loaded - ignoring '%s'", plugin->name().c_str()); + } + } + setup_plugin_logging(*plugin); lp.load_result.plugin = std::move(plugin); plugins_.push_back(std::move(lp)); @@ -75,6 +84,16 @@ size_t PluginManager::load_plugins(const std::vector & configs) { lp.update_provider = result->update_provider; lp.introspection_provider = result->introspection_provider; + // Cache first UpdateProvider, warn on duplicates + if (lp.update_provider) { + if (!first_update_provider_) { + first_update_provider_ = lp.update_provider; + } else { + RCLCPP_WARN(logger(), "Multiple UpdateProvider plugins loaded - ignoring '%s'", + result->plugin->name().c_str()); + } + } + setup_plugin_logging(*result->plugin); lp.load_result = std::move(*result); plugins_.push_back(std::move(lp)); @@ -87,6 +106,26 @@ size_t PluginManager::load_plugins(const std::vector & configs) { } void PluginManager::disable_plugin(LoadedPlugin & lp) { + if (lp.load_result.plugin) { + try { + lp.load_result.plugin->shutdown(); + } catch (const std::exception & e) { + RCLCPP_WARN(logger(), "Plugin '%s' threw during shutdown(): %s", lp.load_result.plugin->name().c_str(), e.what()); + } catch (...) { + RCLCPP_WARN(logger(), "Plugin '%s' threw unknown exception during shutdown()", + lp.load_result.plugin->name().c_str()); + } + } + // Invalidate cached provider if this plugin was the source, re-scan for next + if (first_update_provider_ && lp.update_provider == first_update_provider_) { + first_update_provider_ = nullptr; + for (const auto & other : plugins_) { + if (&other != &lp && other.load_result.plugin && other.update_provider) { + first_update_provider_ = other.update_provider; + break; + } + } + } lp.update_provider = nullptr; lp.introspection_provider = nullptr; lp.load_result.update_provider = nullptr; @@ -173,21 +212,7 @@ void PluginManager::shutdown_all() { } UpdateProvider * PluginManager::get_update_provider() const { - UpdateProvider * first = nullptr; - for (const auto & lp : plugins_) { - if (!lp.load_result.plugin) { - continue; - } - if (lp.update_provider) { - if (!first) { - first = lp.update_provider; - } else { - RCLCPP_WARN(logger(), "Multiple UpdateProvider plugins loaded - ignoring '%s'", - lp.load_result.plugin->name().c_str()); - } - } - } - return first; + return first_update_provider_; } std::vector PluginManager::get_introspection_providers() const { diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp index 44be3716..87ea9d7b 100644 --- a/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp @@ -15,12 +15,13 @@ /// Test plugin that exports a wrong API version (PLUGIN_API_VERSION + 1). /// Used by test_plugin_loader to verify version mismatch rejection. +#include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" #include "ros2_medkit_gateway/plugins/plugin_types.hpp" extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { return ros2_medkit_gateway::PLUGIN_API_VERSION + 1; } -extern "C" GATEWAY_PLUGIN_EXPORT void * create_plugin() { +extern "C" GATEWAY_PLUGIN_EXPORT ros2_medkit_gateway::GatewayPlugin * create_plugin() { return nullptr; } diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp index 32891955..fb62aca9 100644 --- a/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp @@ -15,12 +15,13 @@ /// Test plugin with correct version but create_plugin() returns nullptr. /// Used by test_plugin_loader to verify null factory rejection. +#include "ros2_medkit_gateway/plugins/gateway_plugin.hpp" #include "ros2_medkit_gateway/plugins/plugin_types.hpp" extern "C" GATEWAY_PLUGIN_EXPORT int plugin_api_version() { return ros2_medkit_gateway::PLUGIN_API_VERSION; } -extern "C" GATEWAY_PLUGIN_EXPORT void * create_plugin() { +extern "C" GATEWAY_PLUGIN_EXPORT ros2_medkit_gateway::GatewayPlugin * create_plugin() { return nullptr; }