ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2
Apache License 2.0
436 stars 283 forks source link

support services run with multiple threads #315

Open iuhilnehc-ynos opened 3 years ago

iuhilnehc-ynos commented 3 years ago

Fixes #314 Signed-off-by: Chen Lihui lihui.chen@sony.com

iuhilnehc-ynos commented 3 years ago

This patch is to:

https://github.com/iuhilnehc-ynos/ros_tutorials/blob/44666088677028f4edeb6246b9f791c489dbb653/roscpp_tutorials/add_two_ints_server/add_two_ints_server.cpp#L57-L59

running ros2 run examples_rclcpp_minimal_client client_main& ros2 run examples_rclcpp_minimal_client client_main to expect this service can serve multiple requests at the same time.

[ INFO] [1621936604.891178820]: request: x=41, y=1
[ INFO] [1621936604.891784270]:   sending back response: [42]
[ INFO] [1621936604.892629346]: request: x=41, y=1
[ INFO] [1621936604.892643574]:   sending back response: [42]
              ^^^^^^^^^//\   the timestamp of two requests are almost same.

without this patch, the log as following,

[ INFO] [1621933196.546494184]: request: x=41, y=1
[ INFO] [1621933196.546513489]:   sending back response: [42]
[ INFO] [1621933198.556169433]: request: x=41, y=1
              ^^^^^^^^^^^^^^^^^^^^^/\   2 sec after to process the next request
[ INFO] [1621933198.556201921]:   sending back response: [42]

https://github.com/iuhilnehc-ynos/ros2-examples/blob/0acb1b892f318ec7898b812d10765f538c93ebc7/rclcpp/services/minimal_service/main.cpp#L44-L46

running rosservice call /add_two_ints '{a: 3, b: 4}'& rosservice call /add_two_ints '{a: 3, b: 4}' expect to get the service log as following

[INFO] [1621936929.275491886] [minimal_service]: request: 3 + 4
[INFO] [1621936929.275632310] [minimal_service]: request: 3 + 4

without this patch, the log as following,

[INFO] [1621937408.339715576] [minimal_service]: request: 3 + 4
[INFO] [1621937410.340892268] [minimal_service]: request: 3 + 4

Notice that supporting the service with running multiple threads is for not only the one service(a custom callback group with reentrant set by user) but also multiple different services.

Could somebody help to review this PR?

iuhilnehc-ynos commented 3 years ago

The failure about The following signatures were invalid: EXPKEYSIG F42ED6FBAB17C654 Open Robotics <info@osrfoundation.org> was mentioned on https://answers.ros.org/question/379190/apt-update-signatures-were-invalid-f42ed6fbab17c654/, I'll run retest this please after the CI is fixed.

fujitatomoya commented 3 years ago

@clalancette @sloretz friendly ping.

fujitatomoya commented 1 year ago

@gbiggs you might want to take a look, this fixes https://github.com/ros2/ros1_bridge/issues/314

joelbudu commented 1 year ago

Any chance in this getting merged? @iuhilnehc-ynos @sloretz @clalancette

iuhilnehc-ynos commented 1 year ago

@joelbudu

I am sorry about not figuring out an elegant fix for this. The answer is no for this PR, at least.