16/feat/get/areas/area id/components by mfaferek93 · Pull Request #20 · selfpatch/ros2_medkit

mfaferek93 · 2025-11-23T14:40:54Z

Pull Request

Description

What changed and why.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Related issues

Fixes # (if applicable)
Related to # (if applicable)

How has this been tested?

Describe the tests you ran to verify your changes.

Checklist

My code follows the project's coding style
I ran colcon build and the build succeeds without warnings
I ran colcon test and all tests pass locally
I added/updated tests where applicable
I updated documentation where applicable
My commit messages are clear and descriptive

Additional notes for reviewers

mfaferek93 · 2025-11-23T14:45:39Z

@copilot please review last commit

Copilot · 2025-11-23T14:45:47Z

@mfaferek93 I've opened a new pull request, #21, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

This PR implements two new REST API endpoints for component discovery and filtering: /components (lists all components across all areas) and /areas/{area_id}/components (lists components within a specific area). These endpoints enhance the gateway's discovery capabilities by exposing component-level information in addition to the existing area-level data.

Key Changes:

Added two new REST endpoint handlers (handle_list_components and handle_area_components) following existing patterns
Implemented comprehensive integration tests with proper error case coverage (404 for non-existent areas)
Updated documentation with detailed API reference including examples, response structures, and use cases

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/ros2_medkit_gateway/src/rest_server.cpp`	Implements two new endpoint handlers with consistent error handling and JSON serialization
`src/ros2_medkit_gateway/include/ros2_medkit_gateway/rest_server.hpp`	Adds function declarations for the two new endpoint handlers
`src/ros2_medkit_gateway/test/test_integration.test.py`	Adds three comprehensive tests covering component listing, area-filtered components, and 404 error handling; renumbers existing test from `test_21_` to `test_04_` for sequential ordering
`src/ros2_medkit_gateway/README.md`	Adds extensive API documentation with curl examples, response formats, field descriptions, and use cases for both new endpoints
`postman/collections/ros2-medkit-gateway.postman_collection.json`	Adds two new Postman requests for testing the component endpoints
`postman/README.md`	Updates quick start guide with examples for testing the new endpoints
`docs/README.md`	Updates architecture documentation to reflect the new endpoints

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Add endpoint to list components within a specific area with error handling. Changes: - Add REST handler for GET /areas/{area_id}/components with path parameter - Implement area validation and 404 error response for nonexistent areas - Filter components by area from entity cache - Add test_05_area_components_success integration test - Add test_06_area_components_nonexistent_error integration test - Update README with API reference, success/error examples, and use cases - Update Postman collection with new endpoint and testing instructions

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-23T16:44:03Z

@mfaferek93 I've opened a new pull request, #22, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bburda

LGTM!

…, backoff Write error types (#23, #24): - Change write_value return from bool to tl::expected<void, WriteErrorInfo> with specific error codes: NotConnected, TypeMismatch, AccessDenied, NodeNotFound, TransportError - Add data_type_hint parameter to skip the readValue type-probe round-trip when the caller knows the expected type from NodeMap; halves mutex hold time on writes - Callers (vendor route handlers + DataProvider::write_data + OperationProvider::execute_operation) map WriteError codes to appropriate HTTP status (400 for type mismatch, 403 for access denied, 502 for transport errors) instead of generic 502 for all failures YAML validation (#19): - Validate required fields (node_id, entity_id, data_name) per entry; skip with RCLCPP_WARN if missing - Validate data_type is one of bool/int/float/string; default to float with warning if unknown - Detect duplicate node_id entries; first wins, subsequent skipped with warning - Cap node count at 10000 to prevent DDS resource exhaustion from oversized configs - Replace all std::cerr output with RCLCPP_WARN/RCLCPP_ERROR for structured logging Poll interval clamp (#20): - Clamp poll_interval_ms to [100, 60000] in configure() so sub-100ms values no longer cause a CPU spin even without the condition_variable fix from the previous commit Exponential backoff on reconnect (#28): - Double the reconnect wait on each failed attempt, capped at 60s - Reset to configured interval on successful reconnect - Prevents network and log saturation when PLC is down for extended periods

…rams Extract shared parse_coerce_validate() helper (#3 review comment): - Value parsing (bool/int/string/float dispatch), range validation, and type coercion were duplicated in handle_plc_operations, write_data, and execute_operation. The three copies had already diverged (write_data was missing data_type_hint). Now all three call parse_coerce_validate() from an anonymous namespace, keeping them in sync. Document missing parameters (#20 review comment): - Add subscription_interval_ms to the Gateway Parameters table - Document the optional ros2_topic field in node map entries - Update poll_interval_ms description to note [100, 60000] ms clamping

…, backoff Write error types (#23, #24): - Change write_value return from bool to tl::expected<void, WriteErrorInfo> with specific error codes: NotConnected, TypeMismatch, AccessDenied, NodeNotFound, TransportError - Add data_type_hint parameter to skip the readValue type-probe round-trip when the caller knows the expected type from NodeMap; halves mutex hold time on writes - Callers (vendor route handlers + DataProvider::write_data + OperationProvider::execute_operation) map WriteError codes to appropriate HTTP status (400 for type mismatch, 403 for access denied, 502 for transport errors) instead of generic 502 for all failures YAML validation (#19): - Validate required fields (node_id, entity_id, data_name) per entry; skip with RCLCPP_WARN if missing - Validate data_type is one of bool/int/float/string; default to float with warning if unknown - Detect duplicate node_id entries; first wins, subsequent skipped with warning - Cap node count at 10000 to prevent DDS resource exhaustion from oversized configs - Replace all std::cerr output with RCLCPP_WARN/RCLCPP_ERROR for structured logging Poll interval clamp (#20): - Clamp poll_interval_ms to [100, 60000] in configure() so sub-100ms values no longer cause a CPU spin even without the condition_variable fix from the previous commit Exponential backoff on reconnect (#28): - Double the reconnect wait on each failed attempt, capped at 60s - Reset to configured interval on successful reconnect - Prevents network and log saturation when PLC is down for extended periods

…rams Extract shared parse_coerce_validate() helper (#3 review comment): - Value parsing (bool/int/string/float dispatch), range validation, and type coercion were duplicated in handle_plc_operations, write_data, and execute_operation. The three copies had already diverged (write_data was missing data_type_hint). Now all three call parse_coerce_validate() from an anonymous namespace, keeping them in sync. Document missing parameters (#20 review comment): - Add subscription_interval_ms to the Gateway Parameters table - Document the optional ros2_topic field in node map entries - Update poll_interval_ms description to note [100, 60000] ms clamping

mfaferek93 self-assigned this Nov 23, 2025

mfaferek93 added this to the Basic Read/Write & Discovery milestone Nov 23, 2025

Copilot AI mentioned this pull request Nov 23, 2025

[WIP] Add components for getting areas by area ID #21

Closed

mfaferek93 force-pushed the 16/feat/GET/areas/area_id/components branch from 1096dcd to c8eaa64 Compare November 23, 2025 14:51

bburda requested review from bburda and Copilot November 23, 2025 15:30

Copilot started reviewing on behalf of bburda November 23, 2025 15:31 View session

Copilot finished reviewing on behalf of bburda November 23, 2025 15:33

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Comment thread src/ros2_medkit_gateway/src/rest_server.cpp Outdated

Comment thread src/ros2_medkit_gateway/test/test_integration.test.py

mfaferek93 force-pushed the 16/feat/GET/areas/area_id/components branch from c8eaa64 to a286a32 Compare November 23, 2025 15:51

mfaferek93 linked an issue Nov 23, 2025 that may be closed by this pull request

Implement GET /areas/{area_id}/components endpoint to list components within a specific area #16

Closed

Update src/ros2_medkit_gateway/src/rest_server.cpp

a336761

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

mfaferek93 requested a review from Copilot November 23, 2025 16:05

Copilot started reviewing on behalf of mfaferek93 November 23, 2025 16:16 View session

Copilot finished reviewing on behalf of mfaferek93 November 23, 2025 16:18

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Comment thread src/ros2_medkit_gateway/src/rest_server.cpp

mfaferek93 requested a review from Copilot November 23, 2025 16:38

Copilot started reviewing on behalf of mfaferek93 November 23, 2025 16:39 View session

Copilot finished reviewing on behalf of mfaferek93 November 23, 2025 16:40

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Comment thread src/ros2_medkit_gateway/src/rest_server.cpp

Copilot AI mentioned this pull request Nov 23, 2025

Clarify req parameter usage in handle_area_components - no changes needed #22

Closed

mfaferek93 requested a review from Copilot November 23, 2025 16:48

Copilot started reviewing on behalf of mfaferek93 November 23, 2025 16:48 View session

Copilot finished reviewing on behalf of mfaferek93 November 23, 2025 16:50

Copilot AI reviewed Nov 23, 2025

View reviewed changes

bburda approved these changes Nov 23, 2025

View reviewed changes

bburda merged commit 754bea6 into main Nov 23, 2025
19 checks passed

mfaferek93 deleted the 16/feat/GET/areas/area_id/components branch November 26, 2025 19:56

mfaferek93 mentioned this pull request Apr 12, 2026

feat(plugins): add ros2_medkit_opcua plugin #365

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

16/feat/get/areas/area id/components#20

16/feat/get/areas/area id/components#20
bburda merged 2 commits into
mainfrom
16/feat/GET/areas/area_id/components

mfaferek93 commented Nov 23, 2025 •

edited

Loading

Uh oh!

mfaferek93 commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

bburda left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

mfaferek93 commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Description

Type of change

Related issues

How has this been tested?

Checklist

Additional notes for reviewers

Uh oh!

mfaferek93 commented Nov 23, 2025

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI commented Nov 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bburda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mfaferek93 commented Nov 23, 2025 •

edited

Loading