SOVD service interface: aggregate peer faults#419
Open
bburda wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns the ROS 2 SOVD service interface fault listing behavior with the HTTP API by fanning out fault queries to aggregation peers (when aggregation.enabled is on), so peer faults become visible via the ROS 2 service path as well.
Changes:
- Updated
GatewayPluginContext::list_entity_faults()to optionally fan out/api/v1/<type>/<id>/faultsto peers and merge results. - Updated
GatewayPluginContext::list_all_faults()to optionally fan out/api/v1/faultsto peers and merge results. - Added a new GTest (
test_plugin_context_aggregation) plus CMake wiring.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/ros2_medkit_gateway/src/plugins/plugin_context.cpp |
Adds aggregation-manager fan-out + merge behavior for plugin-context fault listing APIs. |
src/ros2_medkit_gateway/test/test_plugin_context_aggregation.cpp |
New unit test covering peer fan-out behavior for per-entity and global faults. |
src/ros2_medkit_gateway/CMakeLists.txt |
Registers the new test target and includes it in coverage configuration. |
mfaferek93
reviewed
Jun 15, 2026
bburda
force-pushed
the
feat/sovd-service-interface-aggregation
branch
from
8e96cff to
638d040
Compare
The ROS 2 SOVD service interface (plugin context) returned only local fault-manager faults, while the HTTP API already fans out to aggregation peers. Wire list_entity_faults and list_all_faults to the aggregation manager (fan_out_get) so a gateway with aggregation.enabled exposes peer faults over the ROS 2 service path too, consistent with the HTTP /api/v1/faults and /api/v1/<entity>/faults endpoints. - list_entity_faults: fans out /api/v1/<type>/<id>/faults to peers and merges. - list_all_faults: fans out /api/v1/faults and merges. - Aggregation is gated on get_aggregation_manager() != nullptr (no behavior change when aggregation is disabled). Extract entity -> source-FQN resolution and fault scoping into a shared neutral helper (core/faults/fault_scope) used by both the HTTP fault handlers and the plugin-context fault path. list_entity_faults now scopes its local faults to the entity's resolved App-FQN set (APP, COMPONENT, AREA, FUNCTION) exactly as the HTTP per-entity handler does, instead of returning a bare fault-manager envelope. Log partial peer fan-out and document that the ROS 2 service path queries peers without an Authorization header. Recompute list_all_faults "count" after merging peer faults so it does not undercount. Add test_plugin_context_aggregation: no-aggregation baseline, live-mock-peer fan-out, and local-fault-path coverage with a fake fault transport (scoped local fault, component/area/function resolution, local+peer merge, and a peer that fails the fan-out while local faults survive). Closes #418
bburda
force-pushed
the
feat/sovd-service-interface-aggregation
branch
from
638d040 to
86b69da
Compare
…aults
When the gateway aggregates faults from a peer (rtmaps_medkit), the
aggregated fault carries status as a SOVD DTC object:
{"aggregatedStatus": "active", "confirmedDTC": "1", ...}
list_entity_faults previously called fault_json.value("status", string{})
which throws json::type_error.302 when status is an object.
Fix: check the type before extracting. Strings are passed through as-is.
Objects use the "aggregatedStatus" field if present.
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.
Summary
The ROS 2 SOVD service interface returned only local fault-manager faults, while the HTTP API already fans out to aggregation peers. This wires
GatewayPluginContext::list_entity_faultsandlist_all_faultsto the aggregation manager (fan_out_get), mirroring the HTTP fault handler, so a gateway withaggregation.enabledexposes peer faults over the ROS 2 service path too - consistent with the HTTP/api/v1/faultsand/api/v1/<entity>/faultsendpoints.list_entity_faultsfans out/api/v1/<type>/<id>/faultsto peers and merges.list_all_faultsfans out/api/v1/faultsand merges.get_aggregation_manager() != nullptr(no behavior change when aggregation is disabled).Aggregated faults can carry
statusas a SOVD DTC object (e.g.{"aggregatedStatus": "active", ...}) rather than a plain string.handle_list_entity_faultspreviously calledvalue("status", string{}), which throwsjson::type_error.302on an object. It now checks the JSON type first: strings pass through unchanged, objects use theaggregatedStatusfield when present. Without this, a fanned-out service call over a peer with object-typed status would fail.Issue
Type
Testing
test_plugin_context_aggregation(mock peer server): per-entity and all-faults paths surface a peer fault when aggregation is enabled; local-only behavior unchanged when disabled.test_plugin_context_aggregation,test_aggregation_manager,test_aggregation_classification- all pass.colcon test --packages-select ros2_medkit_gateway- 0 failures.-Wall -Wextra -Wpedantic -Wshadow -Wconversion), zero warnings; clang-tidy clean on the changed lines.