Skip to content

Add tests for 'best available' QoS policies#501

Merged
jacobperron merged 3 commits intomasterfrom
jacob/qos_best_available
May 3, 2022
Merged

Add tests for 'best available' QoS policies#501
jacobperron merged 3 commits intomasterfrom
jacob/qos_best_available

Conversation

@jacobperron
Copy link
Copy Markdown
Member

@jacobperron jacobperron commented Apr 20, 2022

Testing feature added in ros2/rmw#320 for all supported rmw implementations.

Test creating a subscription and publisher with 'best available' policies and confirm the actual
QoS policies are what we expect (adapting to an existing endpoint).

@jacobperron
Add tests for 'best available' QoS policies
f5c9ed7 
Testing feature added in ros2/rmw#320 for all supported rmw implementations.

Test creating a subscription and publisher with 'best available' policies and confirm the actual
QoS policies are what we expect (adapting to an existing endpoint).

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron requested a review from j-rivero April 20, 2022 01:33
@jacobperron
Copy link
Copy Markdown
Member Author

jacobperron commented Apr 20, 2022

Right now the feature is only implemented for rmw_fastrtps_cpp and rmw_fastrtps_dynamic_cpp. These tests won't pass for rmw_cyclonedds or rmw_connextdds until we add support for this feature to them.

@jacobperron jacobperron mentioned this pull request Apr 20, 2022
Merged
@j-rivero
Copy link
Copy Markdown

j-rivero commented Apr 22, 2022

After implementing the support for connext and cyclone, my local system is displaying some problems with fastrtps:

12: [ RUN      ] QosRclcppTestFixture__rmw_fastrtps_cpp.test_best_available_policies_subscription
12: [INFO] [1650639533.386204448] [publisher]: created publisher publisher /test_best_available_subscription
12:·                                                                             
12: [INFO] [1650639533.389540572] [subscriber]: created subscriber subscriber /test_best_available_subscription
12:·                                                                             
12: >>> [rcutils|error_handling.c:108] rcutils_set_error_state()                 
12: This error state is being overwritten:                                       
12:·                                                                             
12:   'Unknown QoS durability policy, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rmw_fastrtps/rmw_fastrtps_shared_cpp/src/qos.cpp:75'
12:·                                                                             
12: with this new error message:                                                 
12:·                                                                             
12:   'create_publisher() failed setting topic QoS, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rmw_fastrtps/rmw_fastrtps_cpp/src/subscription.cpp:223'
12:·                                                                             
12: rcutils_reset_error() should be called after error handling to avoid this.   
12: <<<                                                                          
12:·                                                                             
12: >>> [rcutils|error_handling.c:108] rcutils_set_error_state()                 
12: This error state is being overwritten:                                       
12:·                                                                             
12:   'create_publisher() failed setting topic QoS, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rmw_fastrtps/rmw_fastrtps_cpp/src/subscription.cpp:223, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rcl/rcl/src/rcl/subscr
12:·                                                                             
12: with this new error message:                                                 
12:·                                                                             
12:   'invalid allocator, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rcl/rcl/src/rcl/subscription.c:218'
12:·                                                                             
12: rcutils_reset_error() should be called after error handling to avoid this.   
12: <<<                                                                          
12: invalid allocator, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rcl/rcl/src/rcl/subscription.c:218
12: unknown file: Failure                                                        
12: C++ exception with description "could not create subscription: invalid allocator, at /home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/rcl/rcl/src/rcl/subscription.c:218" thrown in the test body.
12: [  FAILED  ] QosRclcppTestFixture__rmw_fastrtps_cpp.test_best_available_policies_subscription (15 ms)

Might be related to the new check in https://github.com/ros2/rmw_fastrtps/pull/598/files#diff-718816e7845a1f3a980129f39e3146e30d65eec32f452c1c47de051bf42717d1R74 ?

Uncrustify is also reporting to need a patch, although seems not very nice to me:

--- test/test_best_available.cpp
+++ test/test_best_available.cpp
@@ -37,5 +37,5 @@
-    .best_effort()
-    .transient_local()
-    .deadline(publisher_deadline)
-    .liveliness(rclcpp::LivelinessPolicy::Automatic)
-    .liveliness_lease_duration(publisher_liveliness_lease_duration);
+  .best_effort()
+  .transient_local()
+  .deadline(publisher_deadline)
+  .liveliness(rclcpp::LivelinessPolicy::Automatic)
+  .liveliness_lease_duration(publisher_liveliness_lease_duration);
@@ -89,5 +89,5 @@
-    .best_effort()
-    .durability_volatile()
-    .deadline(subscription_deadline)
-    .liveliness(rclcpp::LivelinessPolicy::ManualByTopic)
-    .liveliness_lease_duration(subscription_liveliness_lease_duration);
+  .best_effort()
+  .durability_volatile()
+  .deadline(subscription_deadline)
+  .liveliness(rclcpp::LivelinessPolicy::ManualByTopic)
+  .liveliness_lease_duration(subscription_liveliness_lease_duration);

@jacobperron
Uncrustify
110d999 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Copy Markdown
Member Author

jacobperron commented Apr 25, 2022

@j-rivero I'm not able to reproduce the test failure you are seeing. I can run the test for all rmw implementations without error. Double check that you have all of the connected PRs checkout out and that they are up-to-date 🤷

@jacobperron
Copy link
Copy Markdown
Member Author

jacobperron commented Apr 26, 2022 •
edited
Loading

I've added a couple tests to check that services and clients can be created with best available policies: 4accd1a

@jacobperron
Add tests for services and clients
4accd1a 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron force-pushed the jacob/qos_best_available branch from 52de9e3 to 4accd1a Compare April 26, 2022 00:41
@jacobperron jacobperron merged commit a493f18 into master May 3, 2022
@delete-merged-branch delete-merged-branch bot deleted the jacob/qos_best_available branch May 3, 2022 17:35
@Blast545 Blast545 mentioned this pull request May 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wjwwood wjwwood wjwwood approved these changes

@j-rivero j-rivero Awaiting requested review from j-rivero

@ivanpauno ivanpauno Awaiting requested review from ivanpauno

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

@jacobperron @j-rivero @wjwwood