[#110] Discovery: Improve heuristic discovery strategy #124
Conversation
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
…s with synthetic Components
Implement heuristic/runtime discovery mode (PLAN.md) that maps ROS 2 nodes
to SOVD entity hierarchy without requiring a manifest file.
Key changes:
- ROS 2 nodes are now exposed as Apps (not Components)
- Synthetic Components are created by namespace aggregation (e.g., 'powertrain', 'chassis')
- Unified entity lookup via EntityInfo/get_entity_info() for handlers
- REST routes added for /apps/{id}/faults, /apps/{id}/operations, /apps/{id}/configurations
Runtime discovery configuration (ROS parameters):
- discovery.runtime.expose_nodes_as_apps (default: true)
- discovery.runtime.create_synthetic_components (default: true)
- discovery.runtime.grouping_strategy (default: 'namespace')
- discovery.runtime.synthetic_component_name_pattern (default: '{area}')
Handler updates:
- fault_handlers: unified entity lookup for Apps and Components
- config_handlers: simplified using get_entity_info()
- operation_handlers: returns SOVD-compliant {items:[...], total_count}
- component_handlers: aggregates topics/operations from related Apps
Tests:
- Integration tests updated for new entity model (Apps vs Components)
- Discovery tests updated for synthetic component source
…ation Add configurable policies for handling topic-only namespaces in heuristic discovery mode: - TopicOnlyPolicy enum (IGNORE, CREATE_COMPONENT, CREATE_AREA_ONLY) - New gateway parameters: topic_only_policy, min_topics_for_component - discovery_enums.hpp/cpp to centralize enum definitions - Documentation in docs/config/discovery-options.rst - Integration tests in test_discovery_heuristic_apps.test.py
- Add new tutorial docs/tutorials/heuristic-apps.rst - Update tutorials/index.rst with Discovery Tutorials section - Update design diagram with Discovery Strategy classes (App, RuntimeDiscovery) - Remove invalid @verifies REQ_DISCO_* links from heuristic discovery tests
…Q_INTEROP_008)
- Add depends_on field to Component model
- Parse depends_on from manifest YAML
- Add handle_get_depends_on handler in ComponentHandlers
- Register /components/{id}/depends-on route in RestServer
- Add DEPENDS_ON to CapabilityBuilder enum
- Show depends-on capability only when component has dependencies
- Add integration tests for the endpoint
- Update demo_nodes_manifest.yaml with example dependencies
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request enhances the heuristic discovery strategy to improve how ROS 2 graph entities are mapped to SOVD entities. The PR introduces a configurable discovery system that can expose ROS 2 nodes as App entities and create synthetic Components to group them logically by namespace.
Changes:
- Adds runtime discovery configuration options (expose_nodes_as_apps, create_synthetic_components, topic_only_policy)
- Implements synthetic component grouping by namespace with Apps representing individual ROS 2 nodes
- Extends REST API handlers to be entity-agnostic (supporting both components and apps)
- Adds component dependencies support (depends-on relationship endpoint)
- Updates integration tests to reflect the new synthetic component model
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/discovery/runtime_discovery.cpp | Implements synthetic component discovery and node-to-app mapping with configurable policies |
| src/http/handlers/operation_handlers.cpp | Refactored to handle operations for both components and apps with aggregation logic |
| src/http/handlers/fault_handlers.cpp | Made entity-agnostic using unified entity lookup (EntityInfo) |
| src/http/handlers/config_handlers.cpp | Extended to support configuration endpoints for both components and apps |
| src/http/handlers/discovery/component_handlers.cpp | Added depends-on endpoint and synthetic component topic aggregation |
| src/http/rest_server.cpp | Added new REST endpoints for app operations, configurations, and faults |
| test/test_integration.test.py | Updated to test synthetic components and apps instead of node-based components |
| test/test_discovery_heuristic_apps.test.py | New comprehensive test suite for heuristic discovery features |
| include/ros2_medkit_gateway/discovery/discovery_enums.hpp | New file defining discovery mode, grouping strategy, and topic-only policy enums |
| config/gateway_params.yaml | Added extensive runtime discovery configuration options with documentation |
| docs/tutorials/heuristic-apps.rst | New tutorial explaining heuristic discovery usage and configuration |
src/ros2_medkit_gateway/src/http/handlers/discovery/component_handlers.cpp
Show resolved
Hide resolved
src/ros2_medkit_gateway/test/test_discovery_heuristic_apps.test.py
Outdated
Show resolved
Hide resolved
- Fix duplicate operations in aggregation by using unordered_set to deduplicate by full_path before building response (operation_handlers.cpp) - Add missing dependency warning: mark unresolved dependencies with 'missing: true' and log RCLCPP_WARN (component_handlers.cpp) - Use entity_type variable in debug logging instead of leaving it unused - Fix silent exception handling: add explanatory comment and logging for transient connectivity errors during startup - Update outdated test comments to clarify synthetic component IDs vs node component IDs - Fix flake8 line length errors in Python test files Addresses review comments from PR #124
- Add complete Apps folder to Postman collection with all /apps/* endpoints
- Add GET /components/{id}/depends-on endpoints to Discovery folder
- Add depends_on field to manifest-schema.rst Components section
Remove @unittest.skip from test_41 and test_42 by replacing time.sleep() calls with a new _wait_for_action_status() helper that polls the status endpoint with retry logic. Changes: - Add _wait_for_action_status() helper (15s timeout, 0.5s intervals) - test_41: poll for 'succeeded' instead of sleep(3) - test_42: poll for 'executing' before cancel, accept both 'canceling' and 'canceled' status for timing variance Fixes flaky CI failures due to action server timing variability.
Remove the legacy 'expose_nodes_as_apps' parameter that controlled whether ROS 2 nodes are exposed as Apps. Since the initial release will always expose nodes as Apps in runtime discovery mode, this toggle is unnecessary. Changes: - Remove expose_nodes_as_apps from RuntimeConfig struct - Remove expose_nodes_as_apps from DiscoveryConfig::RuntimeOptions - Remove parameter declaration and reading in gateway_node.cpp - Remove config option from gateway_params.yaml - Update documentation in heuristic-apps.rst and discovery-options.rst - Update tests to not set the removed parameter Runtime discovery now always exposes nodes as Apps (source=\"heuristic\"). This simplifies configuration and removes dead code paths.
- Add empty area/pattern validation in apply_component_name_pattern() with WARN logging and safe fallbacks - Add DEBUG logging for duplicate node detection in discover_node_components() - Refactor config_handlers.cpp to use unified get_entity_info() lookup, reducing code duplication (~80 lines) - Increase action test timeout to 30s and use shared ACTION_TIMEOUT constant - Add documentation for expected entities in integration tests
bburda
added
enhancement
bburda
added a commit
that referenced
this pull request
Jan 21, 2026
- Fix duplicate operations in aggregation by using unordered_set to deduplicate by full_path before building response (operation_handlers.cpp) - Add missing dependency warning: mark unresolved dependencies with 'missing: true' and log RCLCPP_WARN (component_handlers.cpp) - Use entity_type variable in debug logging instead of leaving it unused - Fix silent exception handling: add explanatory comment and logging for transient connectivity errors during startup - Update outdated test comments to clarify synthetic component IDs vs node component IDs - Fix flake8 line length errors in Python test files Addresses review comments from PR #124
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
Enhances the heuristic discovery strategy to improve how ROS 2 graph entities are mapped to SOVD entities. The PR introduces a configurable discovery system that can expose ROS 2 nodes as App entities and create synthetic Components to group them logically by namespace.
Changes:
Issue
Link the related issue (required):
Type
Testing
Unit & Integration tests
Checklist