Skip to content

rcl_send_response returns RCL_RET_TIMEOUT.#1048

Merged
fujitatomoya merged 1 commit intorollingfrom
fujitatomoya/bugfix-ros2-issues-1253
Aug 18, 2023
Merged

rcl_send_response returns RCL_RET_TIMEOUT.#1048
fujitatomoya merged 1 commit intorollingfrom
fujitatomoya/bugfix-ros2-issues-1253

Conversation

@fujitatomoya
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya commented Mar 20, 2023

part of ros2/ros2#1253

fujitatomoya
fujitatomoya commented Mar 20, 2023
View reviewed changes
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Mar 20, 2023

for both rclcpp and rclpy need to check the return code, and throws exception accordingly. we would need to add exception for timeout for client interface. Behavior in client libraries need to be discussed before implementation.

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Mar 22, 2023

see ros2/rmw#350 (comment)

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Mar 22, 2023

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

audrow
audrow approved these changes Mar 27, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@audrow audrow left a comment

Choose a reason for hiding this comment

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

Looks good to me!

kghost pushed a commit to kghost-ros2/rcl that referenced this pull request Jun 16, 2023
rcl_send_response returns RCL_RET_TIMEOUT.
c41f2d5 
part of ros2/ros2#1253

We hit this problem where a broken client crashes the service executor.
With this change we can fix it in rclcpp.

Note: this is same as ros2#1048 which is
not taken attention for a while.
@kghost kghost mentioned this pull request Jun 16, 2023
Closed
kghost pushed a commit to kghost-ros2/rcl that referenced this pull request Jun 16, 2023
rcl_send_response returns RCL_RET_TIMEOUT.
0d141c2 
part of ros2/ros2#1253

We hit this problem where a broken client crashes the service executor.
With this change we can fix it in rclcpp.

Note: this is same as ros2#1048 which is
not taken attention for a while.

Signed-off-by: Zang MingJie <zealot0630@gmail.com>
kghost pushed a commit to kghost-ros2/rcl that referenced this pull request Jun 16, 2023
rcl_send_response returns RCL_RET_TIMEOUT.
0ab9550 
part of ros2/ros2#1253

We hit this problem where a broken client crashes the service executor.
With this change we can fix it in rclcpp.

Note: this is same as ros2#1048 which is
not taken attention for a while.

Signed-off-by: Zang MingJie <zealot0630@gmail.com>
kghost pushed a commit to kghost-ros2/rcl that referenced this pull request Jun 16, 2023
rcl_send_response returns RCL_RET_TIMEOUT.
ba9ceea 
part of ros2/ros2#1253

We hit this problem where a broken client crashes the service executor.
With this change we can fix it in rclcpp.

Note: this is same as ros2#1048 which is
not taken attention for a while.

Signed-off-by: Zang MingJie <zealot0630@gmail.com>
kghost added a commit to kghost-ros2/rcl that referenced this pull request Jun 16, 2023
@kghost
rcl_send_response returns RCL_RET_TIMEOUT.
d8faa66 
part of ros2/ros2#1253

We hit this problem where a broken client crashes the service executor.
With this change we can fix it in rclcpp.

Note: this is same as ros2#1048 which is not been taken attention for a
while.

Signed-off-by: Zang MingJie <zealot0630@gmail.com>
@fujitatomoya fujitatomoya mentioned this pull request Jun 16, 2023
Closed
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Jun 29, 2023

@asorbini can i ask your help here?

so what we are trying to do here, if sending response times out, we will print the warning instead of generating exception for the client libraries, so that executor can proceed the next task.

as far as i see, rmw_connextdds does not return RMW_RET_TIMEOUT via rmw_send_response, it always returns RMW_RET_ERROR if anything wrong happens? (rmw_fastrtps and rmw_cyclonedds returns RMW_RET_TIMEOUT)

https://github.com/ros2/rmw_connextdds/blob/rolling/rmw_connextdds_common/src/ndds/dds_api_ndds.cpp#L709-L716

which i think that is fine because that is implementation detail, but just in case i wanted to share this with you since the same problem will exist with rmw_connextdds to generate the exception here.

just checking the behavior of rmw_connextdds to see if my understanding is correct.

thanks in advance,
Tomoya

@fujitatomoya fujitatomoya mentioned this pull request Jun 29, 2023
Merged
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Jul 6, 2023

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

clalancette
clalancette reviewed Jul 6, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

The code here looks good to me, but likely needs a rebase to pass CI.

@fujitatomoya fujitatomoya force-pushed the fujitatomoya/bugfix-ros2-issues-1253 branch from 9d9c3e9 to 7c18664 Compare July 11, 2023 01:44
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Jul 11, 2023

@clalancette thanks for the review, i just did rebase on rolling.

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Jul 11, 2023

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Jul 14, 2023

CI:

  • Linux Build Status

windows failure is unrelated.

@clalancette clalancette mentioned this pull request Jul 20, 2023
Open
@fujitatomoya
rcl_send_response returns RCL_RET_TIMEOUT.
4374dea 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya fujitatomoya force-pushed the fujitatomoya/bugfix-ros2-issues-1253 branch from 7c18664 to 4374dea Compare August 17, 2023 05:01
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Aug 17, 2023

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@fujitatomoya fujitatomoya merged commit c7b394a into rolling Aug 18, 2023
@delete-merged-branch delete-merged-branch bot deleted the fujitatomoya/bugfix-ros2-issues-1253 branch August 18, 2023 16:14
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Aug 18, 2023 •
edited
Loading

Originally this issue was reported with Galactic which is already E.O.L. Now we can see many related issues, i think it would be better to backport these fixes to Humble/Iron as well. and there is no ABI break.

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Aug 18, 2023

@Mergifyio backport iron humble

@mergify
Copy link
Copy Markdown

mergify bot commented Aug 18, 2023 •
edited
Loading

backport iron humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Aug 18, 2023
@fujitatomoya @mergify
rcl_send_response returns RCL_RET_TIMEOUT. (#1048)
d04088e 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c7b394a)
@mergify mergify bot mentioned this pull request Aug 18, 2023
Merged
mergify bot pushed a commit that referenced this pull request Aug 18, 2023
@fujitatomoya @mergify
rcl_send_response returns RCL_RET_TIMEOUT. (#1048)
8613e29 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c7b394a)

# Conflicts:
#	rcl/src/rcl/service.c
@mergify mergify bot mentioned this pull request Aug 18, 2023
Merged
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Aug 18, 2023

lets give some soak time like a week for rolling before merging this backport PRs, then i can start the CI for PRs for Humble and Iron backport.

@fujitatomoya fujitatomoya mentioned this pull request Aug 18, 2023
Closed
fujitatomoya added a commit that referenced this pull request Sep 1, 2023
@mergify @fujitatomoya
rcl_send_response returns RCL_RET_TIMEOUT. (#1048) (#1090)
1207535 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c7b394a)

Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
fujitatomoya added a commit that referenced this pull request Sep 1, 2023
@mergify @fujitatomoya
rcl_send_response returns RCL_RET_TIMEOUT. (backport #1048) (#1091)
887e696 
* rcl_send_response returns RCL_RET_TIMEOUT. (#1048)

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c7b394a)

# Conflicts:
#	rcl/src/rcl/service.c

* fix conflict with service introspection.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

---------

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
mauropasse pushed a commit to mauropasse/rcl that referenced this pull request Oct 18, 2023
@mergify @fujitatomoya
rcl_send_response returns RCL_RET_TIMEOUT. (backport ros2#1048) (ros2…
6134721 
…#1091)

* rcl_send_response returns RCL_RET_TIMEOUT. (ros2#1048)

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c7b394a)

# Conflicts:
#	rcl/src/rcl/service.c

* fix conflict with service introspection.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

---------

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@mauropasse mauropasse mentioned this pull request Oct 18, 2023
Merged
alsora added a commit to irobot-ros/rcl that referenced this pull request Oct 18, 2023
@alsora
Merge pull request #28 from mauropasse/mauro/galactic-events-executor
59c7b07 
rcl_send_response returns RCL_RET_TIMEOUT. (backport ros2#1048) (ros2#1091)
mauropasse pushed a commit to mauropasse/rcl that referenced this pull request Oct 18, 2023
@mergify @fujitatomoya
rcl_send_response returns RCL_RET_TIMEOUT. (backport ros2#1048) (ros2…
82bebd4 
…#1091)

* rcl_send_response returns RCL_RET_TIMEOUT. (ros2#1048)

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
(cherry picked from commit c7b394a)

# Conflicts:
#	rcl/src/rcl/service.c

* fix conflict with service introspection.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>

---------

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@mauropasse mauropasse mentioned this pull request Oct 18, 2023
Merged
alsora added a commit to irobot-ros/rcl that referenced this pull request Oct 18, 2023
@alsora
Merge pull request #29 from mauropasse/mauro/irobot-humble
64503c1 
rcl_send_response returns RCL_RET_TIMEOUT. (backport ros2#1048) (ros2#1091)
@fujitatomoya fujitatomoya mentioned this pull request May 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette clalancette approved these changes

@audrow audrow audrow approved these changes

@ivanpauno ivanpauno Awaiting requested review from ivanpauno

@wjwwood wjwwood Awaiting requested review from wjwwood

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fujitatomoya @clalancette @audrow