Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ros2-bridge action attempt #567

Merged
merged 6 commits into from
Jul 26, 2024
Merged

Ros2-bridge action attempt #567

merged 6 commits into from
Jul 26, 2024

Conversation

starlitxiling
Copy link
Contributor

@starlitxiling starlitxiling commented Jun 27, 2024

In this PR, the implementation of the ros2-bridge action interface and the process of generating C code through cxx are mainly completed.
This is just a communication method for action in ros2-bridge. I don’t know whether it can be used normally.

About this implementation, I think there are some problems:
In the process of using the ros2-client library to implement action, due to some interfaces, the entire processing logic can only be placed in one function, so it will contain some asynchronous operations.
But I personally think this is not a reasonable implementation. When some functions are integrated into one function, if the user encounters problems in the process of using the c++ api action, debugging will be very difficult.

Fixes #536

Copy link
Collaborator

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks a lot, looks good to me overall!

I think this is ready for merging after https://github.com/starlitxiling/dora/pull/1 .

Fix for action PR: Return data from `downcast` function
@starlitxiling
Copy link
Contributor Author

Thanks for your fix,I merge this in my repo.It looks good.

@phil-opp phil-opp merged commit c6f44a7 into dora-rs:main Jul 26, 2024
38 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.

ros2-bridge action implementation
3 participants