feat(gateway): accurate status read for apps and components#455
Open
bburda wants to merge 9 commits into
Open
Conversation
Add const-ref and comment-name to unnamed/by-value shared_ptr params in the GetState service callback and StubLifecycleStateReader override to fix performance-unnecessary-value-param and readability-named-parameter.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes GET /api/v1/{apps|components}/{id}/status reflect more accurate readiness signals: lifecycle-managed apps report readiness from lifecycle_msgs/srv/GetState, and local components are always ready while the gateway is reachable (remote components still rely on aggregation forwarding).
Changes:
- App status now uses lifecycle state when the app exposes both
GetStateandChangeStateservices (active→ready, otherwise/timeout →notReady), falling back tois_onlinefor unmanaged nodes. - Component status (no provider) is simplified: any reachable local component returns
readyindependent of hosted app presence. - Adds a
LifecycleStateReaderseam (core interface + ROS2 implementation), plus unit/integration tests and a new lifecycle demo node.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_integration_tests/test/features/test_lifecycle.test.py | Adds an integration test proving lifecycle-based status for an unconfigured lifecycle node. |
| src/ros2_medkit_integration_tests/ros2_medkit_test_utils/launch_helpers.py | Registers the new managed_lifecycle demo node for launch tests. |
| src/ros2_medkit_integration_tests/package.xml | Adds rclcpp_lifecycle dependency for the new demo node. |
| src/ros2_medkit_integration_tests/demo_nodes/managed_lifecycle_node.cpp | New lifecycle demo node (defaults to unconfigured) used by integration tests. |
| src/ros2_medkit_integration_tests/CMakeLists.txt | Builds/installs the managed_lifecycle demo executable and finds rclcpp_lifecycle. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/status/lifecycle_state_reader.hpp | Introduces core lifecycle reader interface + helper declarations (core-pure seam). |
| src/ros2_medkit_gateway/src/core/status/lifecycle_status_helpers.cpp | Implements lifecycle-state → SOVD-status mapping and lifecycle-service detection helper. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/ros2/status/ros2_lifecycle_state_reader.hpp | Declares ROS2 GetState-backed lifecycle reader implementation. |
| src/ros2_medkit_gateway/src/ros2/status/ros2_lifecycle_state_reader.cpp | Implements lifecycle state reads via a private node/executor and bounded timeout. |
| src/ros2_medkit_gateway/src/http/rest_server.cpp | Wires Ros2LifecycleStateReader into LifecycleHandlers at server construction. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/lifecycle_handlers.hpp | Extends LifecycleHandlers constructor to accept an optional lifecycle reader (defaulted). |
| src/ros2_medkit_gateway/src/http/handlers/lifecycle_handlers.cpp | Updates default status logic for apps/components (lifecycle-aware apps; local components always ready). |
| src/ros2_medkit_gateway/test/test_lifecycle_status_helpers.cpp | Adds unit coverage for the helper mapping and lifecycle-service detection behavior. |
| src/ros2_medkit_gateway/test/test_ros2_lifecycle_state_reader.cpp | Adds unit coverage for Ros2LifecycleStateReader against an in-process GetState service. |
| src/ros2_medkit_gateway/test/test_lifecycle_handlers.cpp | Updates handler tests for new component semantics and adds stubbed-reader tests for lifecycle apps. |
| src/ros2_medkit_gateway/CMakeLists.txt | Adds new tests/targets and lifecycle_msgs dependency; registers lifecycle reader source in gateway_ros2. |
| src/ros2_medkit_gateway/package.xml | Adds lifecycle_msgs dependency required for GetState usage. |
| src/ros2_medkit_gateway/README.md | Updates user-facing docs for the new status derivation rules. |
| src/ros2_medkit_gateway/design/lifecycle.rst | Updates design doc to reflect lifecycle-based app status and new component readiness rule. |
Comment on lines
979
to
991
| set(_test_targets | ||
| test_gateway_node | ||
| test_auth_manager | ||
| test_generic_client_compat | ||
| test_operation_manager | ||
| test_configuration_manager | ||
| test_discovery_manager | ||
| test_runtime_discovery | ||
| test_tls_config | ||
| test_fault_manager | ||
| test_error_info | ||
| test_ros2_lifecycle_state_reader | ||
| test_ros2_subscription_executor |
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
Make
GET /{apps|components}/{id}/statusreportready/notReadyfrom real signals instead ofcoarse ones. Read-only; the
PUTlifecycle transitions stay501(actuation is tracked separately).lifecycle_msgsget_stateandchange_state), the status is read from the actual lifecycle state via aGetStateservice call(
activeisready; any other state, or an unreachable/timed-out read, isnotReady). Thissubsumes liveness, since a dead node's service is gone. Plain (unmanaged) nodes keep using
App::is_online(presence in the ROS 2 graph), which is the best signal available for anunmanaged node.
readywhile the gateway is serving the request (thesubstrate is reachable), independent of how many hosted apps are online, including zero. The old
"at least one hosted app online" rule and the
host_metadataspecial-case are removed. A downhosted app is that app's own
notReady, not the component's. Remote components are unchanged(reached through aggregation forwarding).
The lifecycle read goes through a small injected
LifecycleStateReaderseam: a pure interface ingateway_core(keeps the handler unit-testable with a stub and preserves core purity) plus agateway_ros2implementation that callsGetStateon a private node and private single-threadedexecutor spun inline (mirrors the existing
ros2_fault_service_transportpattern, so it neverblocks or races the gateway executor). A
managed_lifecyclerclcpp_lifecycledemo node and anintegration test prove the path end to end.
The third
LifecycleHandlersconstructor parameter is defaulted, so existing call sites andnon-lifecycle apps are unchanged. No API break.
Issue
Type
Testing
Unit (GTest):
activetoready, other state ornullopttonotReady; the lifecycle-servicedetector requires both
get_stateandchange_state.Ros2LifecycleStateReaderagainst an in-processGetStateservice: readsactive/inactive,returns
nulloptfor an unreachable service.inactivereportsnotReadyeven though it is online(the discriminator); an active managed node reports
ready; a plain node still usesis_online.Component reads return
readyfor the host component, a zero-app component, and a component whosehosted apps are all offline.
Integration (launch_testing):
managed_lifecyclenode is launched and left unconfigured. It is present in the ROS 2 graph(so
is_onlinealone would sayready), butGET /apps/managed_lifecycle/statusreturnsnotReady, proving the status is read from the lifecycle state viaGetState. Tagged@verifies REQ_INTEROP_076.clang-tidy is clean on the changed files.
Checklist