From 34cdfc95627438ea95934a2dfcf5d2ab1b5b38cc Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Thu, 23 Apr 2020 08:42:20 +0800 Subject: [PATCH 1/8] convert sleep_for into appropriate logic Signed-off-by: Barry Xu --- rcl/test/rcl/test_service.cpp | 33 +++++++++++++++++++++---- rcl/test/rcl/test_subscription.cpp | 39 ++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 7a2db37c6..dd6477d66 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -18,6 +18,7 @@ #include #include +#include "rcl/graph.h" #include "rcl/service.h" #include "rcl/rcl.h" @@ -76,6 +77,30 @@ class CLASSNAME (TestServiceFixture, RMW_IMPLEMENTATION) : public ::testing::Tes } }; +void +wait_for_server_to_be_available( + rcl_node_t * node, + rcl_client_t * client, + size_t max_tries, + int64_t period_ms, + bool & success) +{ + size_t iteration = 0; + bool is_ready = false; + rcl_ret_t ret = RCL_RET_OK; + do { + ++iteration; + ret = rcl_service_server_is_available(node, client, &is_ready); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (is_ready) { + success = true; + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); + } while (iteration < max_tries); + success = false; +} + void wait_for_service_to_be_ready( rcl_service_t * service, @@ -165,10 +190,9 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - // TODO(wjwwood): add logic to wait for the connection to be established - // use count_services busy wait mechanism - // until then we will sleep for a short period of time - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + bool success; + wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000, success); + ASSERT_TRUE(success); // Initialize a request. test_msgs__srv__BasicTypes_Request client_request; @@ -190,7 +214,6 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) test_msgs__srv__BasicTypes_Request__fini(&client_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - bool success; wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); ASSERT_TRUE(success); diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 0cf6e42c0..83ed23e6d 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -77,6 +77,29 @@ class CLASSNAME (TestSubscriptionFixture, RMW_IMPLEMENTATION) : public ::testing } }; +void +wait_for_established_subscription( + const rcl_publisher_t * publisher, + size_t max_tries, + int64_t period_ms, + bool & success) +{ + size_t iteration = 0; + rcl_ret_t ret = RCL_RET_OK; + size_t subscription_count = 0; + do { + ++iteration; + ret = rcl_publisher_get_subscription_count(publisher, &subscription_count); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (subscription_count > 0) { + success = true; + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); + } while (iteration < max_tries); + success = false; +} + void wait_for_subscription_to_be_ready( rcl_subscription_t * subscription, @@ -175,10 +198,9 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription }); rcl_reset_error(); - // TODO(wjwwood): add logic to wait for the connection to be established - // probably using the count_subscriptions busy wait mechanism - // until then we will sleep for a short period of time - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + bool success; + wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_TRUE(success); #ifdef RMW_TIMESTAMPS_SUPPORTED rcl_time_point_value_t pre_publish_time; EXPECT_EQ( @@ -193,7 +215,6 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__BasicTypes__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - bool success; wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); ASSERT_TRUE(success); { @@ -250,10 +271,9 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - // TODO(wjwwood): add logic to wait for the connection to be established - // probably using the count_subscriptions busy wait mechanism - // until then we will sleep for a short period of time - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + bool success; + wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_TRUE(success); const char * test_string = "testing"; { test_msgs__msg__Strings msg; @@ -263,7 +283,6 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - bool success; wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); ASSERT_TRUE(success); { From cdcc49a624ff3c15a9bee98a128ac5e6f7948033 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Thu, 23 Apr 2020 14:30:12 +0800 Subject: [PATCH 2/8] Use accurate checking condition Signed-off-by: Barry Xu --- rcl/test/rcl/test_subscription.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 83ed23e6d..5d7c5ee29 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -91,7 +91,7 @@ wait_for_established_subscription( ++iteration; ret = rcl_publisher_get_subscription_count(publisher, &subscription_count); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - if (subscription_count > 0) { + if (subscription_count == 1) { success = true; return; } From 9603c7cde3a6dc737973b3c242c050a64e3e276e Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Sun, 26 Apr 2020 13:23:40 +0800 Subject: [PATCH 3/8] Fixed based on comments Signed-off-by: Barry Xu --- rcl/test/rcl/test_service.cpp | 55 +++++++++++------- rcl/test/rcl/test_subscription.cpp | 91 ++++++++++++++++++------------ 2 files changed, 88 insertions(+), 58 deletions(-) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index dd6477d66..9564960fc 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -77,7 +77,7 @@ class CLASSNAME (TestServiceFixture, RMW_IMPLEMENTATION) : public ::testing::Tes } }; -void +rcl_ret_t wait_for_server_to_be_available( rcl_node_t * node, rcl_client_t * client, @@ -88,20 +88,23 @@ wait_for_server_to_be_available( size_t iteration = 0; bool is_ready = false; rcl_ret_t ret = RCL_RET_OK; - do { + while (iteration < max_tries) { ++iteration; ret = rcl_service_server_is_available(node, client, &is_ready); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } if (is_ready) { success = true; - return; + return ret; } std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); - } while (iteration < max_tries); + } success = false; + return ret; } -void +rcl_ret_t wait_for_service_to_be_ready( rcl_service_t * service, rcl_context_t * context, @@ -112,32 +115,38 @@ wait_for_service_to_be_ready( rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, 0, context, rcl_get_default_allocator()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - rcl_ret_t ret = rcl_wait_set_fini(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); + if (ret != RCL_RET_OK) { + success = false; + return ret; + } size_t iteration = 0; - do { + while (iteration < max_tries) { ++iteration; ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } ret = rcl_wait_set_add_service(&wait_set, service, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); if (ret == RCL_RET_TIMEOUT) { continue; } - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } for (size_t i = 0; i < wait_set.size_of_services; ++i) { if (wait_set.services[i] && wait_set.services[i] == service) { success = true; - return; + return ret; } } - } while (iteration < max_tries); + } success = false; + ret = rcl_wait_set_fini(&wait_set); + return ret; } /* Basic nominal test of a service. @@ -191,7 +200,8 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) }); bool success; - wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000, success); + ret = wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); // Initialize a request. @@ -214,7 +224,8 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) test_msgs__srv__BasicTypes_Request__fini(&client_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); + ret = wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); // This scope simulates the service responding in a different context so that we can @@ -257,7 +268,9 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; test_msgs__srv__BasicTypes_Request__fini(&service_request); } - wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); + ret = wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_FALSE(success); // Initialize the response owned by the client and take the response. test_msgs__srv__BasicTypes_Response client_response; diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 5d7c5ee29..7ff4349b5 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -77,7 +77,7 @@ class CLASSNAME (TestSubscriptionFixture, RMW_IMPLEMENTATION) : public ::testing } }; -void +rcl_ret_t wait_for_established_subscription( const rcl_publisher_t * publisher, size_t max_tries, @@ -87,20 +87,24 @@ wait_for_established_subscription( size_t iteration = 0; rcl_ret_t ret = RCL_RET_OK; size_t subscription_count = 0; - do { + while (iteration < max_tries) { ++iteration; ret = rcl_publisher_get_subscription_count(publisher, &subscription_count); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + success = false; + return ret; + } if (subscription_count == 1) { success = true; - return; + return ret; } std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); - } while (iteration < max_tries); + } success = false; + return ret; } -void +rcl_ret_t wait_for_subscription_to_be_ready( rcl_subscription_t * subscription, rcl_context_t * context, @@ -111,32 +115,38 @@ wait_for_subscription_to_be_ready( rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); rcl_ret_t ret = rcl_wait_set_init(&wait_set, 1, 0, 0, 0, 0, 0, context, rcl_get_default_allocator()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - rcl_ret_t ret = rcl_wait_set_fini(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); + if (ret != RCL_RET_OK) { + success = false; + return ret; + } size_t iteration = 0; - do { + while (iteration < max_tries) { ++iteration; ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } ret = rcl_wait_set_add_subscription(&wait_set, subscription, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); if (ret == RCL_RET_TIMEOUT) { continue; } - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (ret != RCL_RET_OK) { + break; + } for (size_t i = 0; i < wait_set.size_of_subscriptions; ++i) { if (wait_set.subscriptions[i] && wait_set.subscriptions[i] == subscription) { success = true; - return; + return ret; } } - } while (iteration < max_tries); + } success = false; + ret = rcl_wait_set_fini(&wait_set); + return ret; } /* Test subscription init, fini and is_valid functions @@ -199,7 +209,8 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription rcl_reset_error(); bool success; - wait_for_established_subscription(&publisher, 10, 100, success); + ret = wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); #ifdef RMW_TIMESTAMPS_SUPPORTED rcl_time_point_value_t pre_publish_time; @@ -215,7 +226,8 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__BasicTypes__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); { test_msgs__msg__BasicTypes msg; @@ -272,7 +284,8 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); bool success; - wait_for_established_subscription(&publisher, 10, 100, success); + ret = wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); const char * test_string = "testing"; { @@ -283,7 +296,8 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); { test_msgs__msg__Strings msg; @@ -326,10 +340,10 @@ TEST_F( rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - // TODO(wjwwood): add logic to wait for the connection to be established - // probably using the count_subscriptions busy wait mechanism - // until then we will sleep for a short period of time - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + bool success; + ret = wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_TRUE(success); const char * test_string = "testing"; { test_msgs__msg__Strings msg; @@ -341,8 +355,8 @@ TEST_F( test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - bool success; - wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); auto allocator = rcutils_get_default_allocator(); { @@ -492,13 +506,16 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription }); rcl_reset_error(); - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + bool success; + ret = wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_TRUE(success); { ret = rcl_publish_serialized_message(&publisher, &serialized_msg, nullptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - bool success; - wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); { rcl_serialized_message_t serialized_msg_rcv = rmw_get_zero_initialized_serialized_message(); @@ -543,10 +560,10 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - // TODO(wjwwood): add logic to wait for the connection to be established - // probably using the count_subscriptions busy wait mechanism - // until then we will sleep for a short period of time - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + bool success; + ret = wait_for_established_subscription(&publisher, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_TRUE(success); const char * test_string = "testing"; { test_msgs__msg__Strings msg; @@ -556,8 +573,8 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - bool success; - wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_TRUE(success); { test_msgs__msg__Strings msg; From 22247e5d6ac644d9349371af9ce35c258111d891 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Sun, 26 Apr 2020 14:52:36 +0800 Subject: [PATCH 4/8] miss calling rcl_wait_set_fini Signed-off-by: Barry Xu --- rcl/test/rcl/test_service.cpp | 1 + rcl/test/rcl/test_subscription.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 9564960fc..b02746eec 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -140,6 +140,7 @@ wait_for_service_to_be_ready( for (size_t i = 0; i < wait_set.size_of_services; ++i) { if (wait_set.services[i] && wait_set.services[i] == service) { success = true; + ret = rcl_wait_set_fini(&wait_set); return ret; } } diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 7ff4349b5..83ce0d6dc 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -140,6 +140,7 @@ wait_for_subscription_to_be_ready( for (size_t i = 0; i < wait_set.size_of_subscriptions; ++i) { if (wait_set.subscriptions[i] && wait_set.subscriptions[i] == subscription) { success = true; + ret = rcl_wait_set_fini(&wait_set); return ret; } } From e202724149a0c5340df9bd7ab7b53d52b11bec52 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Wed, 29 Apr 2020 09:24:05 +0800 Subject: [PATCH 5/8] keep same logic in a common area Signed-off-by: Barry Xu --- rcl/test/CMakeLists.txt | 8 +- rcl/test/rcl/client_fixture.cpp | 87 +---------- rcl/test/rcl/common.cpp | 237 +++++++++++++++++++++++++++++ rcl/test/rcl/common.hpp | 56 +++++++ rcl/test/rcl/service_fixture.cpp | 55 +------ rcl/test/rcl/test_service.cpp | 93 +---------- rcl/test/rcl/test_subscription.cpp | 121 ++------------- 7 files changed, 315 insertions(+), 342 deletions(-) create mode 100644 rcl/test/rcl/common.cpp create mode 100644 rcl/test/rcl/common.hpp diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 261f8bd82..94abddd40 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -191,7 +191,7 @@ function(test_target_function) ) rcl_add_custom_gtest(test_service${target_suffix} - SRCS rcl/test_service.cpp + SRCS rcl/test_service.cpp rcl/common.cpp ENV ${rmw_implementation_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} LIBRARIES ${PROJECT_NAME} @@ -199,7 +199,7 @@ function(test_target_function) ) rcl_add_custom_gtest(test_subscription${target_suffix} - SRCS rcl/test_subscription.cpp + SRCS rcl/test_subscription.cpp rcl/common.cpp ENV ${rmw_implementation_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} LIBRARIES ${PROJECT_NAME} @@ -259,13 +259,13 @@ function(test_target_function) # Launch tests rcl_add_custom_executable(service_fixture${target_suffix} - SRCS rcl/service_fixture.cpp + SRCS rcl/service_fixture.cpp rcl/common.cpp LIBRARIES ${PROJECT_NAME} AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs" ) rcl_add_custom_executable(client_fixture${target_suffix} - SRCS rcl/client_fixture.cpp + SRCS rcl/client_fixture.cpp rcl/common.cpp LIBRARIES ${PROJECT_NAME} AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs" ) diff --git a/rcl/test/rcl/client_fixture.cpp b/rcl/test/rcl/client_fixture.cpp index c9cdbd2aa..ccf717fd8 100644 --- a/rcl/test/rcl/client_fixture.cpp +++ b/rcl/test/rcl/client_fixture.cpp @@ -12,11 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include -#include -#include -#include - #include "rcutils/logging_macros.h" #include "rcl/client.h" @@ -26,87 +21,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" -#include "rcl/graph.h" - -bool -wait_for_server_to_be_available( - rcl_node_t * node, - rcl_client_t * client, - size_t max_tries, - int64_t period_ms) -{ - size_t iteration = 0; - do { - ++iteration; - bool is_ready; - rcl_ret_t ret = rcl_service_server_is_available(node, client, &is_ready); - if (ret != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, - "Error in rcl_service_server_is_available: %s", - rcl_get_error_string().str); - return false; - } - if (is_ready) { - return true; - } - std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); - } while (iteration < max_tries); - return false; -} - -bool -wait_for_client_to_be_ready( - rcl_client_t * client, - rcl_context_t * context, - size_t max_tries, - int64_t period_ms) -{ - rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); - rcl_ret_t ret = - rcl_wait_set_init(&wait_set, 0, 0, 0, 1, 0, 0, context, rcl_get_default_allocator()); - if (ret != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str); - return false; - } - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str); - throw std::runtime_error("error while waiting for client"); - } - }); - size_t iteration = 0; - do { - ++iteration; - if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str); - return false; - } - if (rcl_wait_set_add_client(&wait_set, client, NULL) != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait_set_add_client: %s", rcl_get_error_string().str); - return false; - } - ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); - if (ret == RCL_RET_TIMEOUT) { - continue; - } - if (ret != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str); - return false; - } - for (size_t i = 0; i < wait_set.size_of_clients; ++i) { - if (wait_set.clients[i] && wait_set.clients[i] == client) { - return true; - } - } - } while (iteration < max_tries); - return false; -} +#include "common.hpp" int main(int argc, char ** argv) { diff --git a/rcl/test/rcl/common.cpp b/rcl/test/rcl/common.cpp new file mode 100644 index 000000000..a28650690 --- /dev/null +++ b/rcl/test/rcl/common.cpp @@ -0,0 +1,237 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "common.hpp" + +#include +#include + +#include "rcutils/logging_macros.h" + +#include "osrf_testing_tools_cpp/scope_exit.hpp" +#include "rcl/error_handling.h" +#include "rcl/graph.h" + +bool +wait_for_server_to_be_available( + rcl_node_t * node, + rcl_client_t * client, + size_t max_tries, + int64_t period_ms) +{ + size_t iteration = 0; + while (iteration < max_tries) { + ++iteration; + bool is_ready; + rcl_ret_t ret = rcl_service_server_is_available(node, client, &is_ready); + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, + "Error in rcl_service_server_is_available: %s", + rcl_get_error_string().str); + return false; + } + if (is_ready) { + return true; + } + std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); + } + return false; +} + +bool +wait_for_client_to_be_ready( + rcl_client_t * client, + rcl_context_t * context, + size_t max_tries, + int64_t period_ms) +{ + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + rcl_ret_t ret = + rcl_wait_set_init(&wait_set, 0, 0, 0, 1, 0, 0, context, rcl_get_default_allocator()); + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str); + return false; + } + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str); + throw std::runtime_error("error while waiting for client"); + } + }); + size_t iteration = 0; + while (iteration < max_tries) { + ++iteration; + if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str); + return false; + } + if (rcl_wait_set_add_client(&wait_set, client, NULL) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait_set_add_client: %s", rcl_get_error_string().str); + return false; + } + ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); + if (ret == RCL_RET_TIMEOUT) { + continue; + } + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str); + return false; + } + for (size_t i = 0; i < wait_set.size_of_clients; ++i) { + if (wait_set.clients[i] && wait_set.clients[i] == client) { + return true; + } + } + } + return false; +} + +bool +wait_for_service_to_be_ready( + rcl_service_t * service, + rcl_context_t * context, + size_t max_tries, + int64_t period_ms) +{ + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + rcl_ret_t ret = + rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, 0, context, rcl_get_default_allocator()); + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str); + return false; + } + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str); + throw std::runtime_error("error waiting for service to be ready"); + } + }); + size_t iteration = 0; + while (iteration < max_tries) { + ++iteration; + if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str); + return false; + } + if (rcl_wait_set_add_service(&wait_set, service, NULL) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait_set_add_service: %s", rcl_get_error_string().str); + return false; + } + ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); + if (ret == RCL_RET_TIMEOUT) { + continue; + } + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str); + return false; + } + for (size_t i = 0; i < wait_set.size_of_services; ++i) { + if (wait_set.services[i] && wait_set.services[i] == service) { + return true; + } + } + } + return false; +} + +bool +wait_for_established_subscription( + const rcl_publisher_t * publisher, + size_t max_tries, + int64_t period_ms) +{ + size_t iteration = 0; + rcl_ret_t ret = RCL_RET_OK; + size_t subscription_count = 0; + while (iteration < max_tries) { + ++iteration; + ret = rcl_publisher_get_subscription_count(publisher, &subscription_count); + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, + "Error in rcl_publisher_get_subscription_count: %s", + rcl_get_error_string().str); + return false; + } + if (subscription_count == 1) { + return true; + } + std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); + } + return false; +} + +bool +wait_for_subscription_to_be_ready( + rcl_subscription_t * subscription, + rcl_context_t * context, + size_t max_tries, + int64_t period_ms) +{ + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + rcl_ret_t ret = + rcl_wait_set_init(&wait_set, 1, 0, 0, 0, 0, 0, context, rcl_get_default_allocator()); + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str); + return false; + } + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str); + throw std::runtime_error("error waiting for service to be ready"); + } + }); + size_t iteration = 0; + while (iteration < max_tries) { + ++iteration; + if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str); + return false; + } + if (rcl_wait_set_add_subscription(&wait_set, subscription, NULL) != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + ROS_PACKAGE_NAME, "Error in rcl_wait_set_add_subscription: %s", rcl_get_error_string().str); + return false; + } + ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); + if (ret == RCL_RET_TIMEOUT) { + continue; + } + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str); + return false; + } + for (size_t i = 0; i < wait_set.size_of_subscriptions; ++i) { + if (wait_set.subscriptions[i] && wait_set.subscriptions[i] == subscription) { + return true; + } + } + } + return false; +} diff --git a/rcl/test/rcl/common.hpp b/rcl/test/rcl/common.hpp new file mode 100644 index 000000000..efdaf6322 --- /dev/null +++ b/rcl/test/rcl/common.hpp @@ -0,0 +1,56 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RCL__COMMON_HPP_ +#define RCL__COMMON_HPP_ + +#include "rcl/client.h" +#include "rcl/service.h" +#include "rcl/rcl.h" + +bool +wait_for_server_to_be_available( + rcl_node_t * node, + rcl_client_t * client, + size_t max_tries, + int64_t period_ms); + +bool +wait_for_client_to_be_ready( + rcl_client_t * client, + rcl_context_t * context, + size_t max_tries, + int64_t period_ms); + +bool +wait_for_service_to_be_ready( + rcl_service_t * service, + rcl_context_t * context, + size_t max_tries, + int64_t period_ms); + +bool +wait_for_established_subscription( + const rcl_publisher_t * publisher, + size_t max_tries, + int64_t period_ms); + +bool +wait_for_subscription_to_be_ready( + rcl_subscription_t * subscription, + rcl_context_t * context, + size_t max_tries, + int64_t period_ms); + +#endif // RCL__COMMON_HPP_ diff --git a/rcl/test/rcl/service_fixture.cpp b/rcl/test/rcl/service_fixture.cpp index 3bed7a14a..f6045309d 100644 --- a/rcl/test/rcl/service_fixture.cpp +++ b/rcl/test/rcl/service_fixture.cpp @@ -13,8 +13,6 @@ // limitations under the License. #include -#include -#include #include #include "rcutils/logging_macros.h" @@ -27,59 +25,8 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "common.hpp" -bool -wait_for_service_to_be_ready( - rcl_service_t * service, - rcl_context_t * context, - size_t max_tries, - int64_t period_ms) -{ - rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); - rcl_ret_t ret = - rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, 0, context, rcl_get_default_allocator()); - if (ret != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str); - return false; - } - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str); - throw std::runtime_error("error waiting for service to be ready"); - } - }); - size_t iteration = 0; - do { - ++iteration; - if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str); - return false; - } - if (rcl_wait_set_add_service(&wait_set, service, NULL) != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, "Error in wait_set_add_service: %s", rcl_get_error_string().str); - return false; - } - ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); - if (ret == RCL_RET_TIMEOUT) { - continue; - } - if (ret != RCL_RET_OK) { - RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str); - return false; - } - for (size_t i = 0; i < wait_set.size_of_services; ++i) { - if (wait_set.services[i] && wait_set.services[i] == service) { - return true; - } - } - } while (iteration < max_tries); - return false; -} int main(int argc, char ** argv) { diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index b02746eec..635fabdfa 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -14,19 +14,14 @@ #include -#include -#include -#include - -#include "rcl/graph.h" #include "rcl/service.h" - #include "rcl/rcl.h" #include "test_msgs/srv/basic_types.h" #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "common.hpp" #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX @@ -77,79 +72,6 @@ class CLASSNAME (TestServiceFixture, RMW_IMPLEMENTATION) : public ::testing::Tes } }; -rcl_ret_t -wait_for_server_to_be_available( - rcl_node_t * node, - rcl_client_t * client, - size_t max_tries, - int64_t period_ms, - bool & success) -{ - size_t iteration = 0; - bool is_ready = false; - rcl_ret_t ret = RCL_RET_OK; - while (iteration < max_tries) { - ++iteration; - ret = rcl_service_server_is_available(node, client, &is_ready); - if (ret != RCL_RET_OK) { - break; - } - if (is_ready) { - success = true; - return ret; - } - std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); - } - success = false; - return ret; -} - -rcl_ret_t -wait_for_service_to_be_ready( - rcl_service_t * service, - rcl_context_t * context, - size_t max_tries, - int64_t period_ms, - bool & success) -{ - rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); - rcl_ret_t ret = - rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, 0, context, rcl_get_default_allocator()); - if (ret != RCL_RET_OK) { - success = false; - return ret; - } - size_t iteration = 0; - while (iteration < max_tries) { - ++iteration; - ret = rcl_wait_set_clear(&wait_set); - if (ret != RCL_RET_OK) { - break; - } - ret = rcl_wait_set_add_service(&wait_set, service, NULL); - if (ret != RCL_RET_OK) { - break; - } - ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); - if (ret == RCL_RET_TIMEOUT) { - continue; - } - if (ret != RCL_RET_OK) { - break; - } - for (size_t i = 0; i < wait_set.size_of_services; ++i) { - if (wait_set.services[i] && wait_set.services[i] == service) { - success = true; - ret = rcl_wait_set_fini(&wait_set); - return ret; - } - } - } - success = false; - ret = rcl_wait_set_fini(&wait_set); - return ret; -} - /* Basic nominal test of a service. */ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) { @@ -200,10 +122,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - bool success; - ret = wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000)); // Initialize a request. test_msgs__srv__BasicTypes_Request client_request; @@ -225,9 +144,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) test_msgs__srv__BasicTypes_Request__fini(&client_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_service_to_be_ready(&service, context_ptr, 10, 100)); // This scope simulates the service responding in a different context so that we can // test take_request/send_response in a single-threaded, deterministic execution. @@ -269,9 +186,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; test_msgs__srv__BasicTypes_Request__fini(&service_request); } - ret = wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_FALSE(success); + ASSERT_FALSE(wait_for_service_to_be_ready(&service, context_ptr, 10, 100)); // Initialize the response owned by the client and take the response. test_msgs__srv__BasicTypes_Response client_response; diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 83ce0d6dc..3e01ade29 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -19,14 +19,15 @@ #include #include "rcl/subscription.h" - #include "rcl/rcl.h" + #include "test_msgs/msg/basic_types.h" #include "test_msgs/msg/strings.h" #include "rosidl_runtime_c/string_functions.h" #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "common.hpp" #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX @@ -77,79 +78,6 @@ class CLASSNAME (TestSubscriptionFixture, RMW_IMPLEMENTATION) : public ::testing } }; -rcl_ret_t -wait_for_established_subscription( - const rcl_publisher_t * publisher, - size_t max_tries, - int64_t period_ms, - bool & success) -{ - size_t iteration = 0; - rcl_ret_t ret = RCL_RET_OK; - size_t subscription_count = 0; - while (iteration < max_tries) { - ++iteration; - ret = rcl_publisher_get_subscription_count(publisher, &subscription_count); - if (ret != RCL_RET_OK) { - success = false; - return ret; - } - if (subscription_count == 1) { - success = true; - return ret; - } - std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); - } - success = false; - return ret; -} - -rcl_ret_t -wait_for_subscription_to_be_ready( - rcl_subscription_t * subscription, - rcl_context_t * context, - size_t max_tries, - int64_t period_ms, - bool & success) -{ - rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); - rcl_ret_t ret = - rcl_wait_set_init(&wait_set, 1, 0, 0, 0, 0, 0, context, rcl_get_default_allocator()); - if (ret != RCL_RET_OK) { - success = false; - return ret; - } - size_t iteration = 0; - while (iteration < max_tries) { - ++iteration; - ret = rcl_wait_set_clear(&wait_set); - if (ret != RCL_RET_OK) { - break; - } - ret = rcl_wait_set_add_subscription(&wait_set, subscription, NULL); - if (ret != RCL_RET_OK) { - break; - } - ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms)); - if (ret == RCL_RET_TIMEOUT) { - continue; - } - if (ret != RCL_RET_OK) { - break; - } - for (size_t i = 0; i < wait_set.size_of_subscriptions; ++i) { - if (wait_set.subscriptions[i] && wait_set.subscriptions[i] == subscription) { - success = true; - ret = rcl_wait_set_fini(&wait_set); - return ret; - } - } - } - success = false; - ret = rcl_wait_set_fini(&wait_set); - return ret; -} - /* Test subscription init, fini and is_valid functions */ TEST_F( @@ -209,10 +137,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription }); rcl_reset_error(); - bool success; - ret = wait_for_established_subscription(&publisher, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_established_subscription(&publisher, 10, 100)); #ifdef RMW_TIMESTAMPS_SUPPORTED rcl_time_point_value_t pre_publish_time; EXPECT_EQ( @@ -227,9 +152,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__BasicTypes__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100)); { test_msgs__msg__BasicTypes msg; test_msgs__msg__BasicTypes__init(&msg); @@ -284,10 +207,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - bool success; - ret = wait_for_established_subscription(&publisher, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_established_subscription(&publisher, 10, 100)); const char * test_string = "testing"; { test_msgs__msg__Strings msg; @@ -297,9 +217,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100)); { test_msgs__msg__Strings msg; test_msgs__msg__Strings__init(&msg); @@ -341,10 +259,7 @@ TEST_F( rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - bool success; - ret = wait_for_established_subscription(&publisher, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_established_subscription(&publisher, 10, 100)); const char * test_string = "testing"; { test_msgs__msg__Strings msg; @@ -356,9 +271,7 @@ TEST_F( test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100)); auto allocator = rcutils_get_default_allocator(); { size_t size = 1; @@ -507,17 +420,12 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription }); rcl_reset_error(); - bool success; - ret = wait_for_established_subscription(&publisher, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_established_subscription(&publisher, 10, 100)); { ret = rcl_publish_serialized_message(&publisher, &serialized_msg, nullptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100)); { rcl_serialized_message_t serialized_msg_rcv = rmw_get_zero_initialized_serialized_message(); initial_capacity_ser = 0u; @@ -561,10 +469,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription rcl_ret_t ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }); - bool success; - ret = wait_for_established_subscription(&publisher, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_established_subscription(&publisher, 10, 100)); const char * test_string = "testing"; { test_msgs__msg__Strings msg; @@ -574,9 +479,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription test_msgs__msg__Strings__fini(&msg); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - ret = wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100, success); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ASSERT_TRUE(success); + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 10, 100)); { test_msgs__msg__Strings msg; test_msgs__msg__Strings * msg_loaned; From bf65de2ca33dc9e6e77c9c1ea061153b985f8f3b Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Thu, 30 Apr 2020 08:58:26 +0800 Subject: [PATCH 6/8] add comments for helper functions Signed-off-by: Barry Xu --- rcl/test/CMakeLists.txt | 8 ++++---- rcl/test/rcl/client_fixture.cpp | 2 +- rcl/test/rcl/service_fixture.cpp | 2 +- rcl/test/rcl/test_service.cpp | 2 +- rcl/test/rcl/test_subscription.cpp | 2 +- .../{common.cpp => wait_for_entity_helpers.cpp} | 4 ++-- .../{common.hpp => wait_for_entity_helpers.hpp} | 16 +++++++++++++--- 7 files changed, 23 insertions(+), 13 deletions(-) rename rcl/test/rcl/{common.cpp => wait_for_entity_helpers.cpp} (99%) rename rcl/test/rcl/{common.hpp => wait_for_entity_helpers.hpp} (66%) diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 94abddd40..1a31a383e 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -191,7 +191,7 @@ function(test_target_function) ) rcl_add_custom_gtest(test_service${target_suffix} - SRCS rcl/test_service.cpp rcl/common.cpp + SRCS rcl/test_service.cpp rcl/wait_for_entity_helpers.cpp ENV ${rmw_implementation_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} LIBRARIES ${PROJECT_NAME} @@ -199,7 +199,7 @@ function(test_target_function) ) rcl_add_custom_gtest(test_subscription${target_suffix} - SRCS rcl/test_subscription.cpp rcl/common.cpp + SRCS rcl/test_subscription.cpp rcl/wait_for_entity_helpers.cpp ENV ${rmw_implementation_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} LIBRARIES ${PROJECT_NAME} @@ -259,13 +259,13 @@ function(test_target_function) # Launch tests rcl_add_custom_executable(service_fixture${target_suffix} - SRCS rcl/service_fixture.cpp rcl/common.cpp + SRCS rcl/service_fixture.cpp rcl/wait_for_entity_helpers.cpp LIBRARIES ${PROJECT_NAME} AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs" ) rcl_add_custom_executable(client_fixture${target_suffix} - SRCS rcl/client_fixture.cpp rcl/common.cpp + SRCS rcl/client_fixture.cpp rcl/wait_for_entity_helpers.cpp LIBRARIES ${PROJECT_NAME} AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs" ) diff --git a/rcl/test/rcl/client_fixture.cpp b/rcl/test/rcl/client_fixture.cpp index ccf717fd8..c6d035a9c 100644 --- a/rcl/test/rcl/client_fixture.cpp +++ b/rcl/test/rcl/client_fixture.cpp @@ -21,7 +21,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" -#include "common.hpp" +#include "wait_for_entity_helpers.hpp" int main(int argc, char ** argv) { diff --git a/rcl/test/rcl/service_fixture.cpp b/rcl/test/rcl/service_fixture.cpp index f6045309d..0dc7fdcd8 100644 --- a/rcl/test/rcl/service_fixture.cpp +++ b/rcl/test/rcl/service_fixture.cpp @@ -25,7 +25,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" -#include "common.hpp" +#include "wait_for_entity_helpers.hpp" int main(int argc, char ** argv) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 635fabdfa..ead37e909 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -21,7 +21,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" -#include "common.hpp" +#include "wait_for_entity_helpers.hpp" #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 3e01ade29..910775a1c 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -27,7 +27,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" -#include "common.hpp" +#include "wait_for_entity_helpers.hpp" #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX diff --git a/rcl/test/rcl/common.cpp b/rcl/test/rcl/wait_for_entity_helpers.cpp similarity index 99% rename from rcl/test/rcl/common.cpp rename to rcl/test/rcl/wait_for_entity_helpers.cpp index a28650690..a28cf7f22 100644 --- a/rcl/test/rcl/common.cpp +++ b/rcl/test/rcl/wait_for_entity_helpers.cpp @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "common.hpp" +#include "wait_for_entity_helpers.hpp" #include #include @@ -175,7 +175,7 @@ wait_for_established_subscription( rcl_get_error_string().str); return false; } - if (subscription_count == 1) { + if (subscription_count > 0) { return true; } std::this_thread::sleep_for(std::chrono::milliseconds(period_ms)); diff --git a/rcl/test/rcl/common.hpp b/rcl/test/rcl/wait_for_entity_helpers.hpp similarity index 66% rename from rcl/test/rcl/common.hpp rename to rcl/test/rcl/wait_for_entity_helpers.hpp index efdaf6322..7f2374cdd 100644 --- a/rcl/test/rcl/common.hpp +++ b/rcl/test/rcl/wait_for_entity_helpers.hpp @@ -12,13 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef RCL__COMMON_HPP_ -#define RCL__COMMON_HPP_ +#ifndef RCL__WAIT_FOR_ENTITY_HELPERS_HPP_ +#define RCL__WAIT_FOR_ENTITY_HELPERS_HPP_ #include "rcl/client.h" #include "rcl/service.h" #include "rcl/rcl.h" +/// Wait a server to be available for a client with a node +/// by a maximum trying times and a milliseconds period bool wait_for_server_to_be_available( rcl_node_t * node, @@ -26,6 +28,8 @@ wait_for_server_to_be_available( size_t max_tries, int64_t period_ms); +/// Wait a client to be ready with a context +/// by a maximum trying times and a milliseconds period bool wait_for_client_to_be_ready( rcl_client_t * client, @@ -33,6 +37,8 @@ wait_for_client_to_be_ready( size_t max_tries, int64_t period_ms); +/// Wait a service to be ready with a context +/// by a maximum trying times and a milliseconds period bool wait_for_service_to_be_ready( rcl_service_t * service, @@ -40,12 +46,16 @@ wait_for_service_to_be_ready( size_t max_tries, int64_t period_ms); +/// Wait a publisher to get one or more established subscriptions +/// by a maximum trying times and a milliseconds period bool wait_for_established_subscription( const rcl_publisher_t * publisher, size_t max_tries, int64_t period_ms); +/// Wait a subscription to be ready with a context +/// by a maximum trying times and a milliseconds period bool wait_for_subscription_to_be_ready( rcl_subscription_t * subscription, @@ -53,4 +63,4 @@ wait_for_subscription_to_be_ready( size_t max_tries, int64_t period_ms); -#endif // RCL__COMMON_HPP_ +#endif // RCL__WAIT_FOR_ENTITY_HELPERS_HPP_ From d32e4357d27a2f384aa9953c2439a5aaa3bcef1f Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Mon, 4 May 2020 15:50:13 +0800 Subject: [PATCH 7/8] Update comments for helper functions Signed-off-by: Barry Xu --- rcl/test/rcl/wait_for_entity_helpers.hpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/rcl/test/rcl/wait_for_entity_helpers.hpp b/rcl/test/rcl/wait_for_entity_helpers.hpp index 7f2374cdd..35baa645b 100644 --- a/rcl/test/rcl/wait_for_entity_helpers.hpp +++ b/rcl/test/rcl/wait_for_entity_helpers.hpp @@ -19,8 +19,8 @@ #include "rcl/service.h" #include "rcl/rcl.h" -/// Wait a server to be available for a client with a node -/// by a maximum trying times and a milliseconds period +/// Wait for a server to be available for `client`, by trying at most `max_tries` times +/// with a `period_ms` period. bool wait_for_server_to_be_available( rcl_node_t * node, @@ -28,8 +28,8 @@ wait_for_server_to_be_available( size_t max_tries, int64_t period_ms); -/// Wait a client to be ready with a context -/// by a maximum trying times and a milliseconds period +/// Wait for `client` to be ready, i.e. a response is available to be handled. +/// It's tried at most `max_tries` times with a period of `period_ms`. bool wait_for_client_to_be_ready( rcl_client_t * client, @@ -37,8 +37,8 @@ wait_for_client_to_be_ready( size_t max_tries, int64_t period_ms); -/// Wait a service to be ready with a context -/// by a maximum trying times and a milliseconds period +/// Wait for service to be ready, i.e. a request is available to be handled. +/// It's tried at most `max_tries` times with a period of `period_ms`. bool wait_for_service_to_be_ready( rcl_service_t * service, @@ -46,16 +46,16 @@ wait_for_service_to_be_ready( size_t max_tries, int64_t period_ms); -/// Wait a publisher to get one or more established subscriptions -/// by a maximum trying times and a milliseconds period +/// Wait for a publisher to get one or more established subscriptions +/// by trying at most `max_tries` times with a `period_ms` period. bool wait_for_established_subscription( const rcl_publisher_t * publisher, size_t max_tries, int64_t period_ms); -/// Wait a subscription to be ready with a context -/// by a maximum trying times and a milliseconds period +/// Wait a subscription to be ready, i.e. a message is ready to be handled, +/// by trying at least `max_tries` times with a `period_ms` period. bool wait_for_subscription_to_be_ready( rcl_subscription_t * subscription, From 5e7ac2069e48dcedc76db27766e1ad895079233a Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Wed, 13 May 2020 09:33:23 +0800 Subject: [PATCH 8/8] Fix build error for Windows Signed-off-by: Barry Xu --- rcl/test/rcl/wait_for_entity_helpers.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/rcl/test/rcl/wait_for_entity_helpers.cpp b/rcl/test/rcl/wait_for_entity_helpers.cpp index a28cf7f22..625e1ea4a 100644 --- a/rcl/test/rcl/wait_for_entity_helpers.cpp +++ b/rcl/test/rcl/wait_for_entity_helpers.cpp @@ -15,6 +15,7 @@ #include "wait_for_entity_helpers.hpp" #include +#include #include #include "rcutils/logging_macros.h"