search

ros2 / rmw_zenoh

RMW for ROS 2 using Zenoh as the middleware
Apache License 2.0
144 stars 29 forks source link

[test_rclcpp] #101 test_services_cpp (test_rclcpp.TestTwoExecutables.test_services_cpp) #152

Closed Yadunund closed 2 months ago

Yadunund commented 3 months ago

Failure

[test_services_client_cpp-2] [==========] Running 5 tests from 1 test suite.
[test_services_client_cpp-2] [----------] Global test environment set-up.
[test_services_client_cpp-2] [----------] 5 tests from test_services_client
[test_services_client_cpp-2] [INFO] [1712303353.082268953] [rmw_zenoh_cpp]: Successfully connected to a Zenoh router with id 3e37a66397bacb45b5938f4ad4063a8.
[test_services_server_cpp-1] [INFO] [1712303353.086031453] [rmw_zenoh_cpp]: Successfully connected to a Zenoh router with id 3e37a66397bacb45b5938f4ad4063a8.
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_noreqid
[test_services_client_cpp-2] [       OK ] test_services_client.test_add_noreqid (109 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_reqid
[test_services_client_cpp-2] [       OK ] test_services_client.test_add_reqid (2 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_return_request
[test_services_client_cpp-2] [       OK ] test_services_client.test_return_request (5 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_two_ints_defered_cb
[test_services_client_cpp-2] [       OK ] test_services_client.test_add_two_ints_defered_cb (2 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_two_ints_defcb_with_handle
[test_services_server_cpp-1]   what():  failed to send response: Unable to find taken request. Report this bug., at /home/yadunund/ros2_rolling/src/rmw_alternative/rmw_zenoh_cpp/src/rmw_zenoh.cpp:2890, at /home/yadunund/ros2_rolling/src/ros2/rcl/rcl/src/rcl/service.c:400
Yadunund commented 3 months ago

With some debug statements it looks like the the sequence_number generated for each client is always 1 (See lines "Generated sequence_id 1 for client ..." below). Therefore the service cannot find the same sequence_number after it returns a response for the first client.

[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_noreqid
[test_services_client_cpp-2] Generated sequence_id 1 for client over /add_two_ints_noreqid
[test_services_client_cpp-2] [       OK ] test_services_client.test_add_noreqid (109 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_reqid
[test_services_client_cpp-2] Generated sequence_id 1 for client over /add_two_ints_reqid
[test_services_client_cpp-2] [       OK ] test_services_client.test_add_reqid (2 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_return_request
[test_services_client_cpp-2] Generated sequence_id 1 for client over /add_two_ints_reqid_return_request
[test_services_client_cpp-2] [       OK ] test_services_client.test_return_request (2 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_two_ints_defered_cb
[test_services_client_cpp-2] Generated sequence_id 1 for client over /add_two_ints_defered_cb
[test_services_client_cpp-2] [       OK ] test_services_client.test_add_two_ints_defered_cb (3 ms)
[test_services_client_cpp-2] [ RUN      ] test_services_client.test_add_two_ints_defcb_with_handle
[test_services_server_cpp-1] terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
[test_services_server_cpp-1]   what():  failed to send response: Unable to find taken request for sequence_number 1. Report this bug., at /home/yadunund/ros2_rolling/src/rmw_alternative/rmw_zenoh_cpp/src/rmw_zenoh.cpp:2897, at /home/yadunund/ros2_rolling/src/ros2/rcl/rcl/src/rcl/service.c:400
Yadunund commented 3 months ago

Okay so this is caused by the sequence_to_query_map_ data structure used to map sequence_number of client request and the ZenohQuery, ie this map. We're not distinguishing between which client the query came from.

Two solutions come to mind:

  1. Make sequence_number generated by the client globally unique.
  2. Update sequence_to_query_map_ such that it maps std::unordered_map<gid, std::unordered_map<sequence_number, ZenohQuery>>

@clalancette what do you think? Personally I think we 2 is easier.

clalancette commented 3 months ago

We're not distinguishing between which client the query came from.

Ah, of course.

2. Update sequence_to_query_map_ such that it maps std::unordered_map<gid, std::unordered_map<sequence_number, ZenohQuery>>

Just to be clear; the gid as the key here would be the gid that the client attaches, correct? If so, then yes, I think this is the way to go; it uses information we already have.

Yadunund commented 3 months ago

Yupp the gid the client already attaches.