Skip to content

Commit 89acca5

Browse files
committed
fix: route separator bug, stale docs, integration test regression
- Fix missing '/' between api_prefix and route pattern in PluginManager::register_routes (produced /api/v1apps/... instead of /api/v1/apps/...) - Fix integration test expecting plaintext 'pong' after demo plugin was changed to return JSON - Update plugin-system.rst, server.rst, troubleshooting.rst, and linux_introspection design doc to reference new get_routes() API instead of removed register_routes() - Update documented API version from 4 to 5 - Remove dead code get_all_route_info() (zero callers) - Add unit test verifying PluginManager route wrapping end-to-end
1 parent 02490fe commit 89acca5

12 files changed

Lines changed: 7201 additions & 66 deletions

File tree

docs/config/server.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ Plugin loading lifecycle:
564564
4. Provider interfaces are queried via ``extern "C"`` functions
565565
5. ``configure()`` is called with per-plugin config
566566
6. ``set_context()`` passes the gateway context to the plugin
567-
7. ``register_routes()`` allows the plugin to add custom REST endpoints
567+
7. ``get_routes()`` returns custom REST endpoint definitions as ``vector<PluginRoute>``
568568

569569
Error isolation: if a plugin throws during any lifecycle call, it is disabled
570570
without crashing the gateway. Other plugins continue to operate normally.

docs/troubleshooting.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ ros2_medkit is currently suitable for development and testing. For production:
254254
**Q: Can I extend ros2_medkit with custom endpoints?**
255255

256256
Yes. The gateway plugin framework allows you to add custom REST endpoints via
257-
``GatewayPlugin::register_routes()``. Create a shared library (``.so``) that
257+
``GatewayPlugin::get_routes()``. Create a shared library (``.so``) that
258258
implements the ``GatewayPlugin`` base class and configure it in ``gateway_params.yaml``.
259259
See :doc:`/config/server` for plugin configuration details.
260260

docs/tutorials/plugin-system.rst

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ A self-contained plugin implementing UpdateProvider (copy-paste starting point):
157157
#include "ros2_medkit_gateway/plugins/plugin_types.hpp"
158158
#include "ros2_medkit_gateway/providers/update_provider.hpp"
159159
160-
#include <httplib.h>
161160
#include <nlohmann/json.hpp>
162161
163162
using namespace ros2_medkit_gateway;
@@ -228,7 +227,7 @@ Plugin Lifecycle
228227
4. Provider interfaces are queried via ``get_update_provider()`` / ``get_introspection_provider()`` / ``get_log_provider()`` / ``get_script_provider()``
229228
5. ``configure()`` is called with per-plugin JSON config
230229
6. ``set_context()`` provides ``PluginContext`` with ROS 2 node, entity cache, faults, and HTTP utilities
231-
7. ``register_routes()`` allows registering custom REST endpoints
230+
7. ``get_routes()`` returns custom REST endpoint definitions as ``vector<PluginRoute>``
232231
8. Runtime: subsystem managers call provider methods as needed
233232
9. ``shutdown()`` is called before the plugin is destroyed
234233

@@ -242,8 +241,14 @@ providing access to gateway data and utilities:
242241
- ``get_entity(id)`` - look up any entity (area, component, app, function) from the discovery cache
243242
- ``list_entity_faults(entity_id)`` - query faults for an entity
244243
- ``validate_entity_for_route(req, res, entity_id)`` - validate entity exists and matches the route type, auto-sending SOVD errors on failure
245-
- ``send_error()`` / ``send_json()`` - SOVD-compliant HTTP response helpers (static methods)
246244
- ``register_capability()`` / ``register_entity_capability()`` - register custom capabilities on entities
245+
246+
.. note::
247+
248+
SOVD-compliant HTTP response helpers (``send_json()``, ``send_error()``) are instance
249+
methods on ``PluginResponse``, not static methods on ``PluginContext``. Use
250+
``res.send_json(data)`` and ``res.send_error(status, code, msg)`` inside route handlers.
251+
247252
- ``check_lock(entity_id, client_id, collection)`` - verify lock access before mutating operations; returns ``LockAccessResult`` with ``allowed`` flag and denial details
248253
- ``acquire_lock()`` / ``release_lock()`` - acquire and release entity locks with optional scope and TTL
249254
- ``get_entity_snapshot()`` - returns an ``IntrospectionInput`` populated from the current entity cache
@@ -302,10 +307,10 @@ for the lower-level registry API.
302307
still required because ``plugin_api_version()`` must return the current version
303308
(exact-match check).
304309

305-
PluginContext API (v4)
310+
PluginContext API (v5)
306311
----------------------
307312

308-
Version 4 of the plugin API introduced several new methods on ``PluginContext``.
313+
Version 5 of the plugin API introduced several new methods on ``PluginContext``.
309314
These methods have default no-op implementations, so existing plugins continue to
310315
compile without changes (though a rebuild is required to match the new
311316
``PLUGIN_API_VERSION``).
@@ -320,7 +325,7 @@ should call this before proceeding:
320325
321326
auto result = ctx_->check_lock(entity_id, client_id, "configurations");
322327
if (!result.allowed) {
323-
PluginContext::send_error(res, 409, result.denied_code, result.denied_reason);
328+
res.send_error(409, result.denied_code, result.denied_reason);
324329
return;
325330
}
326331
@@ -378,30 +383,36 @@ API described in `Cyclic Subscription Extensions`_.
378383
Custom REST Endpoints
379384
---------------------
380385

381-
Any plugin can register vendor-specific endpoints via ``register_routes()``.
382-
Use ``PluginContext`` utilities for entity validation and SOVD-compliant responses:
386+
Any plugin can expose vendor-specific endpoints by overriding ``get_routes()``, which
387+
returns a ``vector<PluginRoute>``. Each route specifies an HTTP method, a URL pattern
388+
relative to the API prefix (no leading slash), and a handler. Use ``PluginRequest`` and
389+
``PluginResponse`` for path parameters and SOVD-compliant responses:
383390

384391
.. code-block:: cpp
385392
386-
void register_routes(httplib::Server& server, const std::string& api_prefix) override {
387-
// Global vendor endpoint
388-
server.Get(api_prefix + "/x-myvendor/status",
389-
[this](const httplib::Request&, httplib::Response& res) {
390-
PluginContext::send_json(res, get_status_json());
391-
});
392-
393-
// Entity-scoped endpoint (matches a registered capability)
394-
server.Get((api_prefix + R"(/apps/([^/]+)/x-medkit-traces)").c_str(),
395-
[this](const httplib::Request& req, httplib::Response& res) {
396-
auto entity = ctx_->validate_entity_for_route(req, res, req.matches[1]);
397-
if (!entity) return; // Error already sent
398-
399-
auto faults = ctx_->list_entity_faults(entity->id);
400-
PluginContext::send_json(res, {{"entity", entity->id}, {"faults", faults}});
401-
});
393+
std::vector<PluginRoute> get_routes() override {
394+
return {
395+
// Global vendor endpoint
396+
{"GET", "x-myvendor/status",
397+
[this](const PluginRequest& /*req*/, PluginResponse& res) {
398+
res.send_json(get_status_json());
399+
}},
400+
401+
// Entity-scoped endpoint (matches a registered capability)
402+
{"GET", R"(apps/([^/]+)/x-medkit-traces)",
403+
[this](const PluginRequest& req, PluginResponse& res) {
404+
auto entity_id = req.path_param(1);
405+
auto entity = ctx_->validate_entity_for_route(req, res, entity_id);
406+
if (!entity) return; // Error already sent
407+
408+
auto faults = ctx_->list_entity_faults(entity->id);
409+
res.send_json({{"entity", entity->id}, {"faults", faults}});
410+
}},
411+
};
402412
}
403413
404-
Use the ``x-`` prefix for vendor-specific endpoints per SOVD convention.
414+
Use the ``x-`` prefix for vendor-specific endpoints per SOVD convention. Patterns are
415+
relative to the API prefix and must not include a leading slash.
405416

406417
For entity-scoped endpoints, register a matching capability via ``register_capability()``
407418
or ``register_entity_capability()`` in ``set_context()`` so the endpoint appears in the
@@ -750,7 +761,7 @@ Alternatively, simply do not install the ``ros2_medkit_graph_provider`` package
750761
Error Handling
751762
--------------
752763

753-
If a plugin throws during any lifecycle method (``configure``, ``set_context``, ``register_routes``,
764+
If a plugin throws during any lifecycle method (``configure``, ``set_context``, ``get_routes``,
754765
``shutdown``), the exception is caught and logged. The plugin is disabled but the gateway continues
755766
operating. A failing plugin never crashes the gateway.
756767

@@ -761,7 +772,7 @@ Plugins export ``plugin_api_version()`` which must return the gateway's ``PLUGIN
761772
If the version does not match, the plugin is rejected with a clear error message suggesting
762773
a rebuild against matching gateway headers.
763774

764-
The current API version is **4**. It is incremented when the ``PluginContext`` vtable changes
775+
The current API version is **5**. It is incremented when the ``PluginContext`` vtable changes
765776
or breaking changes are made to ``GatewayPlugin`` or provider interfaces.
766777

767778
Build Requirements

src/ros2_medkit_discovery_plugins/ros2_medkit_linux_introspection/design/index.rst

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ The following diagram shows the three plugins and their shared infrastructure.
3232
+name(): string
3333
+configure(config): void
3434
+set_context(ctx): void
35-
+register_routes(server, prefix): void
35+
+get_routes(): vector~PluginRoute~
3636
}
3737

3838
interface IntrospectionProvider {
@@ -43,8 +43,6 @@ The following diagram shows the three plugins and their shared infrastructure.
4343
+register_capability()
4444
+validate_entity_for_route()
4545
+get_child_apps()
46-
+send_json()
47-
+send_error()
4846
}
4947
}
5048

src/ros2_medkit_gateway/CMakeLists.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,9 @@ endif()
778778
# Export include directories for downstream packages (plugins)
779779
install(DIRECTORY include/ DESTINATION include)
780780
install(DIRECTORY src/vendored/tl_expected/include/ DESTINATION include/ros2_medkit_gateway/vendored)
781-
install(FILES src/vendored/cpp_httplib/httplib.h DESTINATION include/ros2_medkit_gateway/vendored)
781+
# Note: vendored httplib.h is NOT installed to the export dir. It is only used
782+
# internally by the gateway via VENDORED_DIR cmake parameter. Plugins no longer
783+
# need httplib - they use PluginRequest/PluginResponse wrappers instead.
782784
ament_export_include_directories(include include/ros2_medkit_gateway/vendored)
783785

784786
ament_package()

src/ros2_medkit_gateway/include/ros2_medkit_gateway/plugins/plugin_manager.hpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,6 @@ class PluginManager {
161161
return context_;
162162
}
163163

164-
// ---- Route descriptions (for handle_root auto-registration) ----
165-
166-
/**
167-
* @brief Collect route info (method, pattern) from all active plugins
168-
* @return Combined (method, pattern) pairs from all plugins
169-
*/
170-
std::vector<std::pair<std::string, std::string>> get_all_route_info() const;
171-
172164
// ---- Info ----
173165
bool has_plugins() const;
174166
std::vector<std::string> plugin_names() const;

src/ros2_medkit_gateway/src/plugins/plugin_manager.cpp

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ void PluginManager::register_routes(void * server_ptr, const std::string & api_p
273273
try {
274274
auto routes = lp.load_result.plugin->get_routes();
275275
for (auto & route : routes) {
276-
std::string full_pattern = api_prefix + route.pattern;
276+
std::string full_pattern = api_prefix + "/" + route.pattern;
277277
auto handler_fn = route.handler; // capture by value for lambda
278278
auto httplib_handler = [handler_fn](const httplib::Request & req, httplib::Response & res) {
279279
PluginRequest plugin_req(&req);
@@ -381,26 +381,6 @@ std::vector<std::pair<std::string, IntrospectionProvider *>> PluginManager::get_
381381
return result;
382382
}
383383

384-
std::vector<std::pair<std::string, std::string>> PluginManager::get_all_route_info() const {
385-
std::shared_lock<std::shared_mutex> lock(plugins_mutex_);
386-
std::vector<std::pair<std::string, std::string>> all;
387-
for (const auto & lp : plugins_) {
388-
if (!lp.load_result.plugin) {
389-
continue;
390-
}
391-
try {
392-
auto routes = lp.load_result.plugin->get_routes();
393-
for (const auto & route : routes) {
394-
all.emplace_back(route.method, route.pattern);
395-
}
396-
} catch (const std::exception & e) {
397-
RCLCPP_ERROR(rclcpp::get_logger("plugin_manager"), "Plugin '%s' threw in get_routes(): %s",
398-
lp.load_result.plugin->name().c_str(), e.what());
399-
}
400-
}
401-
return all;
402-
}
403-
404384
bool PluginManager::has_plugins() const {
405385
std::shared_lock<std::shared_mutex> lock(plugins_mutex_);
406386
for (const auto & lp : plugins_) {

0 commit comments

Comments
 (0)