Skip to content

ros action feedback and status support in cxx api of dora#1148

Merged
bobdingAI merged 3 commits intodora-rs:mainfrom
drindr:ros-action-api
Oct 26, 2025
Merged

ros action feedback and status support in cxx api of dora#1148
bobdingAI merged 3 commits intodora-rs:mainfrom
drindr:ros-action-api

Conversation

@drindr
Copy link
Copy Markdown
Contributor

@drindr drindr commented Sep 25, 2025

No description provided.

@bobdingAI bobdingAI marked this pull request as ready for review October 24, 2025 05:11
Signed-off-by: drindr <dreamchancn@qq.com>
@drindr drindr changed the title support ros action in cxx api of dora ros action feedback and status support in cxx api of dora Oct 24, 2025
@drindr
Copy link
Copy Markdown
Contributor Author

drindr commented Oct 24, 2025

I think this pr is ready to merge.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for ROS action feedback and status handling in the C++ API of dora, extending beyond the existing response-only support. The changes enable action clients to properly receive and process feedback and status updates during action execution.

Key Changes:

  • Added custom type converters for action-related message types (GoalStatus, GoalInfo, UUID, Time)
  • Separated action client event streams into three distinct channels: response, feedback, and status
  • Introduced dedicated matcher functions to identify each stream type in combined events

Reviewed Changes

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

File Description
libraries/extensions/ros2-bridge/msg-gen/src/types/message.rs Added From trait implementations for action_msgs types and builtin_interfaces::Time to enable conversion between ros2_client and FFI types
libraries/extensions/ros2-bridge/msg-gen/src/types/action.rs Refactored action client to support separate feedback and status streams alongside the existing response stream, with corresponding matcher functions

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

Comment on lines +231 to +234
tracing::warn!("rcl_interfaces::Time conversion overflow");
i32::MAX
} else if quot < (i32::MIN as i64) {
tracing::warn!("rcl_interfaces::Time conversion underflow");
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The warning messages reference 'rcl_interfaces::Time' but the actual type being converted is 'builtin_interfaces::Time' as shown in the function signature and struct name.

Copilot uses AI. Check for mistakes.
let quot_sat = if quot >= (i32::MIN as i64) {
quot as i32
} else {
tracing::warn!("rcl_interfaces::Time conversion underflow");
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

The warning messages reference 'rcl_interfaces::Time' but the actual type being converted is 'builtin_interfaces::Time' as shown in the function signature and struct name.

Copilot uses AI. Check for mistakes.
Comment on lines +300 to +302
response_id: response_id,
feedback_id: feedback_id,
status_id: status_id,
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

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

Redundant field initialization syntax. Since the variable names match the field names, these can use the shorthand syntax: response_id, feedback_id, status_id.

Suggested change
response_id: response_id,
feedback_id: feedback_id,
status_id: status_id,
response_id,
feedback_id,
status_id,

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@bobdingAI bobdingAI left a comment

Choose a reason for hiding this comment

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

ready

@bobdingAI bobdingAI merged commit 8a00b73 into dora-rs:main Oct 26, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants