Omnibus fixes for running tests with Connext. (backport #2684)#2690
Omnibus fixes for running tests with Connext. (backport #2684)#2690fujitatomoya merged 2 commits intojazzyfrom
Conversation
* Omnibus fixes for running tests with Connext. 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 <clalancette@gmail.com> * Fixes for executors. Signed-off-by: Chris Lalancette <clalancette@gmail.com> * One more fix for services. Signed-off-by: Chris Lalancette <clalancette@gmail.com> * More fixes for service_introspection. Signed-off-by: Chris Lalancette <clalancette@gmail.com> * More fixes for introspection. Signed-off-by: Chris Lalancette <clalancette@gmail.com> --------- Signed-off-by: Chris Lalancette <clalancette@gmail.com> (cherry picked from commit 9984197) # Conflicts: # rclcpp/test/rclcpp/executors/test_executors.cpp # rclcpp/test/rclcpp/test_generic_service.cpp
|
Cherry-pick of 9984197 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
fujitatomoya
left a comment
There was a problem hiding this comment.
@clalancette base code is a bit different, but would be better to backport this to jazzy for long term? what do you think? i can patch it to resolve conflict.
Yeah, I think it would be good to fix this in Jazzy. It should help with https://build.ros2.org/view/Jci/job/Jci__nightly-connext_ubuntu_noble_amd64/ |
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
|
Pulls: #2690
|
|
@clalancette requesting review just in case! |
clalancette
left a comment
There was a problem hiding this comment.
Looks good to me with green CI.
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
This is an automatic backport of pull request #2684 done by Mergify.