-
Notifications
You must be signed in to change notification settings - Fork 427
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
Omnibus fixes for running tests with Connext. #2684
base: rolling
Are you sure you want to change the base?
Conversation
Pulls: #2684 |
54455b3
to
871cd7a
Compare
@clalancette i was trying to fix #2613 with #2651, but i was able to reproduce the issue after QoS depth 10. i will try your patch to make sure if that problem goes away. (i have never tried ServiceQoSProfile yet.) |
@clalancette unfortunately still fails... export RMW_IMPLEMENTATION=rmw_connextdds
colcon test --event-handlers console_direct+ --packages-select rclcpp --ctest-args -R test_service_introspection --retest-until-fail 100
...
74: Test command: /usr/bin/python3 "-u" "/root/ros2_ws/work/install/ament_cmake_test/share/ament_cmake_test/cmake/run_test.py" "/root/ros2_ws/work/build/rclcpp/test_results/rclcpp/test_service_introspection.gtest.xml" "--package-name" "rclcpp" "--output-file" "/root/ros2_ws/work/build/rclcpp/ament_cmake_gmock/test_service_introspection.txt" "--command" "/root/ros2_ws/work/build/rclcpp/test/rclcpp/test_service_introspection" "--gtest_output=xml:/root/ros2_ws/work/build/rclcpp/test_results/rclcpp/test_service_introspection.gtest.xml"
74: Working Directory: /root/ros2_ws/work/build/rclcpp/test/rclcpp
74: Test timeout computed to be: 60
74: -- run_test.py: invoking following command in '/root/ros2_ws/work/build/rclcpp/test/rclcpp':
74: - /root/ros2_ws/work/build/rclcpp/test/rclcpp/test_service_introspection --gtest_output=xml:/root/ros2_ws/work/build/rclcpp/test_results/rclcpp/test_service_introspection.gtest.xml
74: Running main() from gmock_main.cc
74: [==========] Running 3 tests from 1 test suite.
74: [----------] Global test environment set-up.
74: [----------] 3 tests from TestServiceIntrospection
74: [ RUN ] TestServiceIntrospection.service_introspection_nominal
74: RTI Connext DDS Non-commercial license is for academic, research, evaluation and personal use only. USE FOR COMMERCIAL PURPOSES IS PROHIBITED. See RTI_LICENSE.TXT for terms. Download free tools at rti.com/ncl. License issued to Non-Commercial User [email protected] For non-production use only.
74: Expires on 00-jan-00 See www.rti.com for more information.
74: [ OK ] TestServiceIntrospection.service_introspection_nominal (305 ms)
74: [ RUN ] TestServiceIntrospection.service_introspection_enable_disable_events
74: RTI Connext DDS Non-commercial license is for academic, research, evaluation and personal use only. USE FOR COMMERCIAL PURPOSES IS PROHIBITED. See RTI_LICENSE.TXT for terms. Download free tools at rti.com/ncl. License issued to Non-Commercial User [email protected] For non-production use only.
74: Expires on 00-jan-00 See www.rti.com for more information.
74: /root/ros2_ws/work/src/ros2/rclcpp/rclcpp/test/rclcpp/test_service_introspection.cpp:232: Failure
74: Expected equality of these values:
74: events.size()
74: Which is: 3
74: 4U
74: Which is: 4
74:
74: [ FAILED ] TestServiceIntrospection.service_introspection_enable_disable_events (2305 ms) |
When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here: 1. Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond. 2. Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events. 3. Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it. With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds. Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
871cd7a
to
ceb7e76
Compare
Signed-off-by: Chris Lalancette <[email protected]>
Ah, sorry, I forgot about your PR.
Good call on this. I was able to reproduce, and I found out the actual problem. What we needed to do was wait until our subscription in the test got matched with the introspection publishers coming from the service introspection. I did that in 309cd85, and it now seems to work for me. Can you give it another shot and see if that fixes it for you? |
Signed-off-by: Chris Lalancette <[email protected]>
Pulls: #2684 |
When running the tests with RTI Connext as the default RMW, some of the tests are failing. There are three different failures fixed here:
Setting the liveliness duration to a value smaller than a microsecond causes Connext to throw an error. Set it to a millisecond.
Using the SystemDefaultsQoS sets the QoS to KEEP_LAST 1. Connext is somewhat slow in this regard, so it can be the case that we are overwriting a previous service introspection event with the next one. Switch to the ServicesDefaultQoS in the test, which ensures we will not lose events.
Connext is slow to match publishers and subscriptions. Thus, when creating a subscription "on-the-fly", we should wait for the publisher to match it before expecting the subscription to actually receive data from it.
With these fixes in place, the test_client_common, test_generic_service, test_service_introspection, and test_executors tests all pass for me with rmw_connextdds.
This should fix #2613, #2611, and #2646. @Crola1702 FYI