Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a unified shared-library plugin framework for ros2_medkit_gateway, replacing the previous update-backend-only plugin mechanism and enabling plugins to provide update backends, introspection (preview), and custom REST routes.
Changes:
- Added core plugin ABI + loader/manager (
GatewayPlugin,PluginLoader,PluginManager) with lifecycle orchestration and error isolation. - Migrated software updates from
UpdateBackend/UpdatePluginLoadertoUpdateProviderintegrated via the plugin framework. - Added extensive unit + integration test coverage and updated docs/config/changelog for the new
pluginsparameter model.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_integration_tests/test/features/test_updates.test.py | Updates integration test configuration to use plugins parameters. |
| src/ros2_medkit_gateway/test/test_update_manager.cpp | Adapts UpdateManager unit tests to non-owning UpdateProvider* wiring. |
| src/ros2_medkit_gateway/test/test_plugin_manager.cpp | New unit tests for PluginManager lifecycle, dispatch, and isolation. |
| src/ros2_medkit_gateway/test/test_plugin_loader.cpp | New unit tests for dlopen loader, ABI checks, and error paths. |
| src/ros2_medkit_gateway/test/demo_nodes/test_version_only_plugin.cpp | Test fixture plugin exporting only plugin_api_version(). |
| src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp | Converts integration-test backend to a plugin (GatewayPlugin + UpdateProvider). |
| src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp | Test fixture plugin with null factory return. |
| src/ros2_medkit_gateway/test/demo_nodes/test_no_symbols_plugin.cpp | Test fixture shared library with no required plugin symbols. |
| src/ros2_medkit_gateway/test/demo_nodes/test_minimal_plugin.cpp | Test fixture minimal valid plugin (required exports only). |
| src/ros2_medkit_gateway/test/demo_nodes/test_gateway_plugin.cpp | Test fixture full-feature plugin (providers + custom route). |
| src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp | Test fixture plugin with API version mismatch. |
| src/ros2_medkit_gateway/src/updates/update_manager.cpp | Removes dlopen ownership; adds set_backend(UpdateProvider*). |
| src/ros2_medkit_gateway/src/updates/plugin_loader.cpp | Removes legacy updates-only plugin loader. |
| src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp | New plugin lifecycle orchestration, dispatch, and isolation logic. |
| src/ros2_medkit_gateway/src/plugins/plugin_loader.cpp | New dlopen/dlsym loader with path validation + API versioning. |
| src/ros2_medkit_gateway/src/http/rest_server.cpp | Registers plugin-provided routes during REST server setup. |
| src/ros2_medkit_gateway/src/gateway_node.cpp | Adds plugin parameter handling + loads plugins; wires updates via PluginManager. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_types.hpp | New header containing update-related shared types. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp | Migrates UpdateManager to UpdateProvider interface. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp | Removes legacy UpdateBackend interface header. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp | Removes legacy updates-only plugin loader header. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/update_provider.hpp | New provider interface replacing UpdateBackend. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/providers/introspection_provider.hpp | New provider interface for platform introspection (preview). |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_types.hpp | Defines plugin ABI constants/types and export visibility macro. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp | Declares PluginManager API and dispatch semantics. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_loader.hpp | Declares plugin loader + RAII load result contract. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/gateway_plugin.hpp | Defines the base plugin lifecycle interface + logging callback. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp | Exposes PluginManager accessor; enforces member destruction order. |
| src/ros2_medkit_gateway/config/gateway_params.yaml | Updates configuration to plugins array and removes updates backend params. |
| src/ros2_medkit_gateway/CMakeLists.txt | Wires new plugin sources; adds plugin .so fixtures and new tests. |
| docs/tutorials/plugin-system.rst | New tutorial documenting the plugin system and ABI. |
| docs/tutorials/index.rst | Adds plugin tutorial to Sphinx toctree. |
| docs/troubleshooting.rst | Updates guidance to reflect plugin-based endpoint extension. |
| docs/config/server.rst | Documents plugin framework parameters and lifecycle. |
| docs/changelog.rst | Documents new plugin framework and breaking config changes. |
| docs/api/rest.rst | Updates updates API docs to reference plugin framework + UpdateProvider. |
| .gitignore | Adds .worktrees/ ignore entry. |
src/ros2_medkit_gateway/test/demo_nodes/test_null_factory_plugin.cpp
Outdated
Show resolved
Hide resolved
src/ros2_medkit_gateway/test/demo_nodes/test_bad_version_plugin.cpp
Outdated
Show resolved
Hide resolved
mfaferek93
reviewed
Feb 25, 2026
src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp
Show resolved
Hide resolved
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
Typed provider interfaces that plugins implement alongside GatewayPlugin. UpdateProvider mirrors existing UpdateBackend. IntrospectionProvider defines the enrichment/discovery interface. Refs #235
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
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
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
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<UpdateBackend>. 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
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.
- 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()
Documents plugin interface, configuration, extern "C" exports, lifecycle, custom REST endpoints, multi-plugin dispatch, error handling, and API versioning. Refs #235
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.
…ty 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.
Scan parameter overrides for plugins.<name>.* 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.
- 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
e51e8d7 to
21c9663
Compare
mfaferek93
approved these changes
Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Add a plugin framework for extending the gateway with custom functionality via shared libraries (
.sofiles). Plugins can provide software update backends, platform-specific introspection (preview), and custom REST endpoints.Key design decisions:
extern "C"provider query functions instead ofdynamic_castacross dlopen boundary (RTLD_LOCALmakes RTTI unreliable)GatewayPluginLoadResultwith correct destruction ordering (providers -> plugin -> dlclose)UpdateProvider*inUpdateManager(ownership centralized inPluginManager)What changed:
GatewayPluginbase class with lifecycle methods + logging APIUpdateProviderandIntrospectionProvidertyped provider interfacesPluginLoaderwith path validation, API version checking, andRTLD_NOW | RTLD_LOCALPluginManagerorchestrating loading, lifecycle, error isolation, and dispatchUpdateBackendreplaced byUpdateProvider(non-owning pointer, dependency inversion)GatewayNodeintegration with config-driven plugin loadingIssue
Type
Testing
Unit tests (28 new):
test_plugin_loader(14 tests): happy path, path validation, symbol validation, minimal plugin (no providers), move semantics (ctor + assignment),load_pluginssuccess pathtest_plugin_manager(14 tests): lifecycle, dispatch, multi-capability, error isolation for configure/set_node/register_routes, shutdown idempotency, shutdown exception swallowingTest fixtures (6
.soplugins):test_gateway_plugin- full-featured plugin (both providers + custom route)test_minimal_plugin- required exports only, no provider query functionstest_bad_version_plugin,test_no_symbols_plugin,test_null_factory_plugin,test_version_only_plugin- error path coverageIntegration tests:
test_updates.test.pyupdated to use newpluginsconfig array instead ofupdates.backend/updates.plugin_pathFull suite: 1623 tests, 0 failures
Checklist
Breaking Changes
updates.backendandupdates.plugin_pathparameters removed - usepluginsarray withplugins.<name>.pathentriesUpdateBackendclass replaced byUpdateProviderinterface (same method signatures, different base class)