Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,27 @@ Multi-package colcon workspace under `src/`:

## Architecture

The package is organised in two named layers under `src/ros2_medkit_gateway/`:

- **`include/.../core/`** + **`src/core/`** - middleware-neutral business logic. Compiles into the `gateway_core` STATIC library with no ROS dependencies. Hosts the HTTP routing, request handlers, JWT auth, fault model, peer aggregation, manifest parser, entity cache, and the neutral managers (lock, bulk-data, subscription, script, update, plugin).
- The remainder of `src/` - ROS adapter layer. Compiles into `gateway_ros2` (links `gateway_core` publicly). Hosts `gateway_node` (rclcpp::Node), the ROS-coupled managers (data access, operation, configuration, fault facade, log, trigger), `runtime_discovery`, `native_topic_sampler`, and ROS-specific provider implementations.

```
GatewayNode (src/ros2_medkit_gateway/src/gateway_node.cpp)
GatewayNode (src/main.cpp + src/gateway_node.cpp) [gateway_ros2]
├── DiscoveryManager - Entity discovery (runtime / manifest / hybrid)
│ └── EntityCache - Thread-safe in-memory entity store
│ └── EntityCache - Thread-safe in-memory entity store [core]
├── DataAccessManager - Topic sampling and publishing
├── OperationManager - Service calls and action goals
├── ConfigurationManager - ROS 2 parameter CRUD
├── FaultManager - Fault query/clear, SSE streams (delegates to fault_manager package)
├── LogManager - /rosout ring buffer + plugin delegation
├── PluginManager - Dynamic plugin loading (.so), provider interfaces
└── RESTServer - cpp-httplib HTTP server (separate thread)
├── PluginManager - Dynamic plugin loading (.so), provider interfaces [core]
└── RESTServer - cpp-httplib HTTP server (separate thread) [core]
└── HandlerContext - Shared validation, entity lookup, error/JSON helpers
```

The `gateway_core_purity` linter (greps for ROS includes under `core/`) and the `test_gateway_core_smoke` link-time test (compiles a TU against `gateway_core` alone) enforce the contract that the neutral layer carries no ROS coupling.

### Entity Model (SOVD-aligned)

Defined in `include/ros2_medkit_gateway/models/`:
Expand Down Expand Up @@ -67,14 +74,14 @@ Each entity type supports specific collections:
- **C++17** with `-Wall -Wextra -Wpedantic -Wshadow -Wconversion`
- **Formatting**: Google-based clang-format, 120 cols, 2-space indent, middle pointer alignment (`Type * ptr`)
- **Headers**: `#pragma once`, Apache 2.0 copyright (year 2026 for new files)
- **Namespace**: `ros2_medkit_gateway`
- **Namespace**: `medkit::` for code under `core/`, `medkit::` (via the `ros2_medkit_gateway = medkit` alias in `include/ros2_medkit_gateway/compat.hpp`) for everything else. Existing code referencing `ros2_medkit_gateway::` keeps compiling via the alias during the migration.
- **Error handling**: `tl::expected<T, std::string>` for fallible operations - NOT exceptions in handlers
- **Thread safety**: Always get fresh `EntityCache` via `ctx_.node()->get_thread_safe_cache()` (in handlers)
- **Header-only libs**: nlohmann::json, tl::expected, jwt-cpp, cpp-httplib

## Handler Pattern (follow for ALL new handlers)

Handlers live in `src/http/handlers/`, one file per group. All handlers take `HandlerContext&` in constructor.
Handlers live under `src/core/http/handlers/` (most groups - they are middleware-neutral) and `src/http/handlers/` (handlers that pull rclcpp transitively, currently `handler_context`, `fault_handlers`, `sse_fault_handler`, `cyclic_subscription_handlers`). One file per group. All handlers take `HandlerContext&` in constructor.

```cpp
void ExampleHandlers::handle_request(const httplib::Request & req, httplib::Response & res) {
Expand Down Expand Up @@ -148,7 +155,7 @@ Handler classes: `HealthHandlers`, `DiscoveryHandlers`, `DataHandlers`, `Operati

## Build System

**Static library pattern**: `gateway_lib` bundles all source for shared use between executable and tests.
**Static library pattern**: two layered targets - `gateway_core` (sources under `src/core/`, no ROS dependencies, links cpp-httplib + nlohmann/json + yaml-cpp + tl::expected + jwt-cpp + OpenSSL + SQLite + dl) and `gateway_ros2` (the remaining ROS-adapter sources, links `gateway_core` publicly via `medkit_target_dependencies()`). The `gateway_node` executable and existing test targets link `gateway_ros2` so they transitively get both layers. The link-only smoke test `test_gateway_core_smoke` deliberately links `gateway_core` plus GTest with no `ament_target_dependencies` to enforce the neutral contract.

**CMake modules** in `cmake/`:
- `ROS2MedkitCcache.cmake` - auto-detect ccache
Expand Down Expand Up @@ -211,9 +218,13 @@ Plugins provide custom backends via provider interfaces:

| Provider | Interface | Purpose |
|----------|-----------|---------|
| `LogProvider` | `providers/log_provider.hpp` | Custom log backends |
| `UpdateProvider` | `providers/update_provider.hpp` | Software update management |
| `IntrospectionProvider` | `providers/introspection_provider.hpp` | Custom entity introspection |
| `DataProvider` | `core/providers/data_provider.hpp` | Per-entity data resource backend |
| `OperationProvider` | `core/providers/operation_provider.hpp` | Per-entity operation backend |
| `FaultProvider` | `core/providers/fault_provider.hpp` | Per-entity fault backend |
| `LogProvider` | `core/providers/log_provider.hpp` | Custom log backends |
| `UpdateProvider` | `core/providers/update_provider.hpp` | Software update management |
| `ScriptProvider` | `core/providers/script_provider.hpp` | Custom script execution backends |
| `IntrospectionProvider` | `core/providers/introspection_provider.hpp` | Custom entity introspection |

Plugins loaded as `.so` files with `extern "C"` factory. Provider interfaces return typed C++ structs (not JSON) - manager layer handles serialization. Plugin failures are caught and isolated.

Expand Down Expand Up @@ -263,8 +274,8 @@ When reviewing pull requests, apply these rules to the diff. Flag violations as
Every code change must include corresponding tests. A feature without tests is not complete.

**Unit tests (C++ GTest)** - required for all new logic:
- **Flag** new or modified files in `src/http/handlers/` without a corresponding `test/test_*_handlers.cpp` in the diff. Handler unit tests verify request validation, error responses, and edge cases without ROS 2 runtime.
- **Flag** new or modified manager classes (`src/*.cpp`) without corresponding `test/test_*.cpp`. Manager tests verify business logic in isolation.
- **Flag** new or modified files in `src/core/http/handlers/` or `src/http/handlers/` without a corresponding `test/test_*_handlers.cpp` in the diff. Handler unit tests verify request validation, error responses, and edge cases without ROS 2 runtime.
- **Flag** new or modified manager classes (`src/core/managers/*.cpp` or `src/*.cpp`) without corresponding `test/test_*.cpp`. Manager tests verify business logic in isolation.
- **Flag** new utility functions or static methods without unit tests covering their behavior.

**Integration tests (Python launch_testing)** - required for all endpoint changes:
Expand All @@ -280,8 +291,9 @@ Every code change must include corresponding tests. A feature without tests is n

### Build & CMake

- **Flag** new `.cpp` source files that aren't added to the `gateway_lib` STATIC library target in CMakeLists.txt.
- **Flag** new test targets that don't link to `gateway_lib`.
- **Flag** new `.cpp` source files under `src/core/` that aren't picked up by `gateway_core`'s `GLOB_RECURSE` source list, or new ROS-adapter `.cpp` files that aren't added to the `gateway_ros2` STATIC library target in CMakeLists.txt.
- **Flag** any `#include <rclcpp/...>` or message-package include in a file under `core/` - the layer must stay middleware-neutral. The `gateway_core_purity` linter and `test_gateway_core_smoke` link test enforce this; do not paper over their failures.
- **Flag** new test targets that don't link to `gateway_ros2` (the default), unless the test is intentionally a core-only smoke test linking `gateway_core` directly.
- **Flag** use of `ament_target_dependencies()` - use `medkit_target_dependencies()` instead (removed in Rolling).
- **Flag** direct `find_package(yaml-cpp)` or `find_package(httplib)` - use `medkit_find_yaml_cpp()` / `medkit_find_cpp_httplib()` for multi-distro compatibility.

Expand Down
2 changes: 1 addition & 1 deletion docs/api/warning_codes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Codes are stable machine-readable identifiers: renaming a code is a
breaking change for downstream consumers that key on the string.

The canonical list of codes is maintained in
``src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/warning_codes.hpp``;
``src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/warning_codes.hpp``;
this page mirrors it for API consumers.

.. list-table::
Expand Down
8 changes: 4 additions & 4 deletions docs/tutorials/plugin-system.rst
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Writing a Plugin

#include "ros2_medkit_gateway/plugins/gateway_plugin.hpp"
#include "ros2_medkit_gateway/plugins/plugin_types.hpp"
#include "ros2_medkit_gateway/updates/update_provider.hpp"
#include "ros2_medkit_gateway/core/providers/update_provider.hpp"

using namespace ros2_medkit_gateway;

Expand Down Expand Up @@ -148,7 +148,7 @@ implements the provider interface, even if the class inherits from it.
.. code-block:: cmake

add_library(my_plugin MODULE src/my_plugin.cpp)
target_link_libraries(my_plugin gateway_lib)
target_link_libraries(my_plugin gateway_ros2)

Comment on lines +151 to 152
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs now instruct plugin authors to target_link_libraries(my_plugin gateway_ros2), but in-tree plugin packages build MODULE libraries with unresolved symbols that resolve from the host process (see their use of -Wl,--unresolved-symbols=ignore-all) and do not link against the gateway static libraries. Linking a plugin directly to gateway_ros2 could also accidentally embed gateway objects into the plugin .so. Please update the tutorial to match the actual/expected build pattern (headers via medkit_target_dependencies, allow unresolved symbols, and avoid linking the static gateway libraries unless there is a specific reason).

Suggested change
target_link_libraries(my_plugin gateway_ros2)
# Pull in include directories / package dependencies for the gateway plugin APIs.
# Plugins typically do not link against the gateway static libraries directly.
medkit_target_dependencies(my_plugin
ros2_medkit_gateway
)
# Allow symbols provided by the host gateway process to remain unresolved here.
target_link_options(my_plugin PRIVATE
"-Wl,--unresolved-symbols=ignore-all"
)
# Avoid target_link_libraries(my_plugin gateway_ros2) unless you have a specific
# reason to do so and understand the implications for the resulting plugin .so.

Copilot uses AI. Check for mistakes.
4. Install the ``.so`` and add its path to ``gateway_params.yaml``.

Expand All @@ -162,7 +162,7 @@ A self-contained plugin implementing UpdateProvider (copy-paste starting point):
// my_ota_plugin.cpp
#include "ros2_medkit_gateway/plugins/gateway_plugin.hpp"
#include "ros2_medkit_gateway/plugins/plugin_types.hpp"
#include "ros2_medkit_gateway/updates/update_provider.hpp"
#include "ros2_medkit_gateway/core/providers/update_provider.hpp"

#include <nlohmann/json.hpp>

Expand Down Expand Up @@ -566,7 +566,7 @@ execute, and control executions:
.. code-block:: cpp

#include "ros2_medkit_gateway/plugins/gateway_plugin.hpp"
#include "ros2_medkit_gateway/scripts/script_provider.hpp"
#include "ros2_medkit_gateway/core/providers/script_provider.hpp"

using namespace ros2_medkit_gateway;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extern "C" int plugin_api_version();
extern "C" GatewayPlugin * create_plugin();
extern "C" IntrospectionProvider * get_introspection_provider(GatewayPlugin * plugin);

// Stubs for PluginRequest/PluginResponse (implemented in gateway_lib, not linked into tests)
// Stubs for PluginRequest/PluginResponse (implemented in gateway_core, not linked into tests)
namespace ros2_medkit_gateway {
PluginRequest::PluginRequest(const void * impl) : impl_(impl) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ find_package(diagnostic_msgs REQUIRED)
find_package(nlohmann_json REQUIRED)

# MODULE target: loaded via dlopen at runtime by PluginManager.
# Symbols from gateway_lib are resolved from the host process at runtime.
# Symbols from gateway_ros2 (and gateway_core transitively) are resolved
# from the host process at runtime.
# We only need headers at compile time (via medkit_target_dependencies).
add_library(topic_beacon_plugin MODULE
src/topic_beacon_plugin.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ extern "C" int plugin_api_version();
extern "C" GatewayPlugin * create_plugin();
extern "C" IntrospectionProvider * get_introspection_provider(GatewayPlugin * plugin);

// Stubs for PluginRequest/PluginResponse (implemented in gateway_lib, not linked into tests)
// Stubs for PluginRequest/PluginResponse (implemented in gateway_core, not linked into tests)
namespace ros2_medkit_gateway {
PluginRequest::PluginRequest(const void * impl) : impl_(impl) {
}
Expand Down
Loading
Loading