-
Notifications
You must be signed in to change notification settings - Fork 418
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
add rcl_action_client_options when creating action client. #1133
add rcl_action_client_options when creating action client. #1133
Conversation
I guess that it needs to be rebased. |
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
18252c2
to
b4c5ebe
Compare
@ros-pull-request-builder retest this please |
source build with ros2/ros2@4eece71, i do not see the following build error reported by CI,
i do not think this is related to my fix. |
friendly ping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me, but it would be better to have a C++ based option structure.
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
We should hold for after the release of Foxy.
+1 |
merge conflicts are resolved, could we have a go for merge? |
@fujitatomoya Thanks for the patch! |
* add rcl_action_client_options for create_client. Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com> * Capitalize comments and keep the default rcl_action_client_options. Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com> * delete unnecessary default rcl_action_client_options_t. Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
fix #1118.