From 99efa4feff3ba82731fb17332d741c5c7869dcbc Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Sun, 27 Sep 2020 17:14:47 +0800 Subject: [PATCH 1/5] Improve trigger test for graph guard condition Signed-off-by: Barry Xu --- rcl/test/rcl/test_graph.cpp | 214 +++++++++++++++++++++++++----------- 1 file changed, 150 insertions(+), 64 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 5d7f2ee49..59c9fcddb 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -35,6 +35,7 @@ #include "rcutils/logging_macros.h" #include "rcutils/logging.h" +#include "rcutils/env.h" #include "test_msgs/msg/basic_types.h" #include "test_msgs/srv/basic_types.h" @@ -1223,75 +1224,160 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_query_functio 9); // number of retries } -/* Test the graph guard condition notices topic changes. +/* Test the graph guard condition notices beliow changes. + * publisher create/destroy, subscription create/destroy + * service create/destroy, client create/destroy + * Other node added/removed * * Note: this test could be impacted by other communications on the same ROS Domain. */ -TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_condition_topics) { +TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_condition_trigger_check) { rcl_ret_t ret; - // Create a thread to sleep for a time, then create a publisher, sleep more, then a subscriber, - // sleep more, destroy the subscriber, sleep more, and then destroy the publisher. - std::promise topic_changes_promise; - std::thread topic_thread( - [this, &topic_changes_promise]() { - // sleep - std::this_thread::sleep_for(std::chrono::milliseconds(200)); - // create the publisher - rcl_publisher_t pub = rcl_get_zero_initialized_publisher(); - rcl_publisher_options_t pub_ops = rcl_publisher_get_default_options(); - rcl_ret_t ret = rcl_publisher_init( - &pub, this->node_ptr, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), - "/chatter_test_graph_guard_condition_topics", &pub_ops); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - // sleep - std::this_thread::sleep_for(std::chrono::milliseconds(200)); - // create the subscription - rcl_subscription_t sub = rcl_get_zero_initialized_subscription(); - rcl_subscription_options_t sub_ops = rcl_subscription_get_default_options(); - ret = rcl_subscription_init( - &sub, this->node_ptr, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), - "/chatter_test_graph_guard_condition_topics", &sub_ops); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - // sleep - std::this_thread::sleep_for(std::chrono::milliseconds(200)); - // destroy the subscription - ret = rcl_subscription_fini(&sub, this->node_ptr); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - // sleep - std::this_thread::sleep_for(std::chrono::milliseconds(200)); - // destroy the publication - ret = rcl_publisher_fini(&pub, this->node_ptr); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - // notify that the thread is done - topic_changes_promise.set_value(true); - }); - // Wait for the graph state to change, expecting it to do so at least 4 times, - // once for each change in the topics thread. + std::chrono::nanoseconds time_to_sleep = std::chrono::milliseconds(400); + + // Test in new ROS domain + ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "66")); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + EXPECT_EQ(rcutils_set_env("ROS_DOMAIN_ID", NULL), true); + }); + + // Create new context + rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); + ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str; + }); + + rcl_context_t context = rcl_get_zero_initialized_context(); + ret = rcl_init(0, nullptr, &init_options, &context); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str; + }); + + rcl_node_t node = rcl_get_zero_initialized_node(); + rcl_node_options_t node_options = rcl_node_get_default_options(); + ret = rcl_node_init(&node, "test_graph", "", &context, &node_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str; + }); + + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_init( + &wait_set, 0, 1, 0, 0, 0, 0, &context, rcl_get_default_allocator()); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str; + }); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; const rcl_guard_condition_t * graph_guard_condition = - rcl_node_get_graph_guard_condition(this->node_ptr); - ASSERT_NE(nullptr, graph_guard_condition) << rcl_get_error_string().str; - std::shared_future future = topic_changes_promise.get_future(); - size_t graph_changes_count = 0; - // while the topic thread is not done, wait and count the graph changes - while (future.wait_for(std::chrono::seconds(0)) != std::future_status::ready) { - ret = rcl_wait_set_clear(this->wait_set_ptr); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(this->wait_set_ptr, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - std::chrono::nanoseconds time_to_sleep = std::chrono::milliseconds(400); - RCUTILS_LOG_INFO_NAMED( - ROS_PACKAGE_NAME, - "waiting up to '%s' nanoseconds for graph changes", - std::to_string(time_to_sleep.count()).c_str()); - ret = rcl_wait(this->wait_set_ptr, time_to_sleep.count()); - if (ret == RCL_RET_TIMEOUT) { - continue; - } - graph_changes_count++; - } - topic_thread.join(); - // expect at least 4 changes - ASSERT_GE(graph_changes_count, 4ul); + rcl_node_get_graph_guard_condition(&node); + + auto wait_check_func = + [&ret, &wait_set, &graph_guard_condition, &time_to_sleep](rcl_ret_t expected) { + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(expected, ret) << rcl_get_error_string().str; + }; + + wait_check_func(RCL_RET_OK); + + // Graph change since creating the publisher + rcl_publisher_t pub = rcl_get_zero_initialized_publisher(); + rcl_publisher_options_t pub_ops = rcl_publisher_get_default_options(); + ret = rcl_publisher_init( + &pub, &node, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), + "/chatter_test_graph_guard_condition_topics", &pub_ops); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since destroying the publisher + ret = rcl_publisher_fini(&pub, &node); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since creating the subscription + rcl_subscription_t sub = rcl_get_zero_initialized_subscription(); + rcl_subscription_options_t sub_ops = rcl_subscription_get_default_options(); + ret = rcl_subscription_init( + &sub, &node, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), + "/chatter_test_graph_guard_condition_topics", &sub_ops); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since destroying the subscription + ret = rcl_subscription_fini(&sub, &node); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since creating service + rcl_service_t service = rcl_get_zero_initialized_service(); + rcl_service_options_t service_options = rcl_service_get_default_options(); + ret = rcl_service_init( + &service, + &node, + ROSIDL_GET_SRV_TYPE_SUPPORT(test_msgs, srv, BasicTypes), + "test_graph_guard_condition_service", + &service_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since destroy service + ret = rcl_service_fini(&service, &node); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since creating client + rcl_client_t client = rcl_get_zero_initialized_client(); + rcl_client_options_t client_options = rcl_client_get_default_options(); + ret = rcl_client_init( + &client, + &node, + ROSIDL_GET_SRV_TYPE_SUPPORT(test_msgs, srv, BasicTypes), + "test_graph_guard_condition_service", + &client_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since destroying client + ret = rcl_client_fini(&client, &node); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since adding new node + rcl_node_t node_new = rcl_get_zero_initialized_node(); + ret = rcl_node_init(&node_new, "test_graph2", "", &context, &node_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Graph change since destroying new node + ret = rcl_node_fini(&node_new); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + wait_check_func(RCL_RET_OK); + + // Should not get graph change + wait_check_func(RCL_RET_TIMEOUT); } /* Test the rcl_service_server_is_available function. From c50e4abbc9e248349e19bfac5394a60ad51b5ebb Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Mon, 28 Sep 2020 11:22:26 +0800 Subject: [PATCH 2/5] Update codes based on comments. Signed-off-by: Barry Xu --- rcl/test/rcl/test_graph.cpp | 113 ++++++++++++++++++++++++++---------- 1 file changed, 81 insertions(+), 32 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 59c9fcddb..331a20d8d 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -1224,7 +1224,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_query_functio 9); // number of retries } -/* Test the graph guard condition notices beliow changes. +/* Test the graph guard condition notices below changes. * publisher create/destroy, subscription create/destroy * service create/destroy, client create/destroy * Other node added/removed @@ -1235,13 +1235,6 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_ret_t ret; std::chrono::nanoseconds time_to_sleep = std::chrono::milliseconds(400); - // Test in new ROS domain - ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "66")); - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - EXPECT_EQ(rcutils_set_env("ROS_DOMAIN_ID", NULL), true); - }); - // Create new context rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); @@ -1251,6 +1244,10 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str; }); + // Test in new ROS domain + ret = rcl_init_options_set_domain_id(&init_options, 66); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_context_t context = rcl_get_zero_initialized_context(); ret = rcl_init(0, nullptr, &init_options, &context); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -1273,25 +1270,22 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_wait_set_init( &wait_set, 0, 1, 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( { EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str; }); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + const rcl_guard_condition_t * graph_guard_condition = rcl_node_get_graph_guard_condition(&node); - auto wait_check_func = - [&ret, &wait_set, &graph_guard_condition, &time_to_sleep](rcl_ret_t expected) { - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(expected, ret) << rcl_get_error_string().str; - }; - - wait_check_func(RCL_RET_OK); + // Graph change since first node created + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since creating the publisher rcl_publisher_t pub = rcl_get_zero_initialized_publisher(); @@ -1301,13 +1295,23 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi "/chatter_test_graph_guard_condition_topics", &pub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since destroying the publisher ret = rcl_publisher_fini(&pub, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since creating the subscription rcl_subscription_t sub = rcl_get_zero_initialized_subscription(); @@ -1317,13 +1321,23 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi "/chatter_test_graph_guard_condition_topics", &sub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since destroying the subscription ret = rcl_subscription_fini(&sub, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since creating service rcl_service_t service = rcl_get_zero_initialized_service(); @@ -1336,13 +1350,23 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi &service_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since destroy service ret = rcl_service_fini(&service, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since creating client rcl_client_t client = rcl_get_zero_initialized_client(); @@ -1355,29 +1379,54 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi &client_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since destroying client ret = rcl_client_fini(&client, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since adding new node rcl_node_t node_new = rcl_get_zero_initialized_node(); ret = rcl_node_init(&node_new, "test_graph2", "", &context, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // Graph change since destroying new node ret = rcl_node_fini(&node_new); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - wait_check_func(RCL_RET_OK); + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - // Should not get graph change - wait_check_func(RCL_RET_TIMEOUT); + // Should not get graph change if no change + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + ASSERT_EQ(RCL_RET_TIMEOUT, ret) << rcl_get_error_string().str; } /* Test the rcl_service_server_is_available function. From 2cd69571bf8d8051bf2689a8a073a4c516fd1292 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Tue, 20 Oct 2020 10:35:28 +0800 Subject: [PATCH 3/5] Update codes based on review comments Signed-off-by: Barry Xu --- rcl/test/rcl/test_graph.cpp | 134 ++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 75 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 331a20d8d..c887ab5ff 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -35,7 +35,6 @@ #include "rcutils/logging_macros.h" #include "rcutils/logging.h" -#include "rcutils/env.h" #include "test_msgs/msg/basic_types.h" #include "test_msgs/srv/basic_types.h" @@ -1232,6 +1231,15 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_query_functio * Note: this test could be impacted by other communications on the same ROS Domain. */ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_condition_trigger_check) { +#define CHECK_GUARD_CONDITION_CHANGE(EXPECTED_RESULT) do { \ + ret = rcl_wait_set_clear(&wait_set); \ + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; \ + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); \ + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; \ + ret = rcl_wait(&wait_set, time_to_sleep.count()); \ + ASSERT_EQ(EXPECTED_RESULT, ret) << rcl_get_error_string().str; \ +} while (0) + rcl_ret_t ret; std::chrono::nanoseconds time_to_sleep = std::chrono::milliseconds(400); @@ -1245,7 +1253,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi }); // Test in new ROS domain - ret = rcl_init_options_set_domain_id(&init_options, 66); + ret = rcl_init_options_set_domain_id(&init_options, 42); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_context_t context = rcl_get_zero_initialized_context(); @@ -1259,7 +1267,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_node_t node = rcl_get_zero_initialized_node(); rcl_node_options_t node_options = rcl_node_get_default_options(); - ret = rcl_node_init(&node, "test_graph", "", &context, &node_options); + ret = rcl_node_init(&node, "test_graph", "/", &context, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { @@ -1280,12 +1288,10 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_node_get_graph_guard_condition(&node); // Graph change since first node created - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since creating the publisher rcl_publisher_t pub = rcl_get_zero_initialized_publisher(); @@ -1295,23 +1301,19 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi "/chatter_test_graph_guard_condition_topics", &pub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since destroying the publisher ret = rcl_publisher_fini(&pub, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since creating the subscription rcl_subscription_t sub = rcl_get_zero_initialized_subscription(); @@ -1321,23 +1323,19 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi "/chatter_test_graph_guard_condition_topics", &sub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since destroying the subscription ret = rcl_subscription_fini(&sub, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since creating service rcl_service_t service = rcl_get_zero_initialized_service(); @@ -1350,23 +1348,19 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi &service_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since destroy service ret = rcl_service_fini(&service, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since creating client rcl_client_t client = rcl_get_zero_initialized_client(); @@ -1379,54 +1373,44 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi &client_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since destroying client ret = rcl_client_fini(&client, &node); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since adding new node rcl_node_t node_new = rcl_get_zero_initialized_node(); ret = rcl_node_init(&node_new, "test_graph2", "", &context, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Graph change since destroying new node ret = rcl_node_fini(&node_new); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + } // Should not get graph change if no change - ret = rcl_wait_set_clear(&wait_set); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_wait(&wait_set, time_to_sleep.count()); - ASSERT_EQ(RCL_RET_TIMEOUT, ret) << rcl_get_error_string().str; + { + SCOPED_TRACE("Check guard condition change failed !"); + CHECK_GUARD_CONDITION_CHANGE(RCL_RET_TIMEOUT); + } } /* Test the rcl_service_server_is_available function. From bb2365c36bc20b29cbd67c9abecddf6840927018 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Thu, 17 Dec 2020 11:22:52 +0800 Subject: [PATCH 4/5] Change codes based on review comments. 1. change codes to use existing context 2. after no graph change condition, start to test Signed-off-by: Barry Xu --- rcl/test/rcl/test_graph.cpp | 75 ++++++++++++++----------------------- 1 file changed, 28 insertions(+), 47 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index c887ab5ff..e9e9fcfb9 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -1243,41 +1243,9 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_ret_t ret; std::chrono::nanoseconds time_to_sleep = std::chrono::milliseconds(400); - // Create new context - rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); - ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str; - }); - - // Test in new ROS domain - ret = rcl_init_options_set_domain_id(&init_options, 42); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - - rcl_context_t context = rcl_get_zero_initialized_context(); - ret = rcl_init(0, nullptr, &init_options, &context); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str; - EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str; - }); - - rcl_node_t node = rcl_get_zero_initialized_node(); - rcl_node_options_t node_options = rcl_node_get_default_options(); - ret = rcl_node_init(&node, "test_graph", "/", &context, &node_options); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str; - }); - rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_wait_set_init( - &wait_set, 0, 1, 0, 0, 0, 0, &context, rcl_get_default_allocator()); + &wait_set, 0, 1, 0, 0, 0, 0, context_ptr, rcl_get_default_allocator()); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { @@ -1285,19 +1253,31 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi }); const rcl_guard_condition_t * graph_guard_condition = - rcl_node_get_graph_guard_condition(&node); + rcl_node_get_graph_guard_condition(node_ptr); - // Graph change since first node created - { - SCOPED_TRACE("Check guard condition change failed !"); - CHECK_GUARD_CONDITION_CHANGE(RCL_RET_OK); + // Wait for no graph change condition + int idx = 0; + while (idx < 100) { + ret = rcl_wait_set_clear(&wait_set); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_wait(&wait_set, time_to_sleep.count()); + if (RCL_RET_TIMEOUT == ret) { + break; + } else { + RCUTILS_LOG_INFO_NAMED( + ROS_PACKAGE_NAME, + "waiting for no graph change condition ..."); + } } + ASSERT_NE(idx, 100); // Graph change since creating the publisher rcl_publisher_t pub = rcl_get_zero_initialized_publisher(); rcl_publisher_options_t pub_ops = rcl_publisher_get_default_options(); ret = rcl_publisher_init( - &pub, &node, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), + &pub, node_ptr, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), "/chatter_test_graph_guard_condition_topics", &pub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -1307,7 +1287,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi } // Graph change since destroying the publisher - ret = rcl_publisher_fini(&pub, &node); + ret = rcl_publisher_fini(&pub, node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; { @@ -1319,7 +1299,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_subscription_t sub = rcl_get_zero_initialized_subscription(); rcl_subscription_options_t sub_ops = rcl_subscription_get_default_options(); ret = rcl_subscription_init( - &sub, &node, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), + &sub, node_ptr, ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, BasicTypes), "/chatter_test_graph_guard_condition_topics", &sub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -1329,7 +1309,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi } // Graph change since destroying the subscription - ret = rcl_subscription_fini(&sub, &node); + ret = rcl_subscription_fini(&sub, node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; { @@ -1342,7 +1322,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_service_options_t service_options = rcl_service_get_default_options(); ret = rcl_service_init( &service, - &node, + node_ptr, ROSIDL_GET_SRV_TYPE_SUPPORT(test_msgs, srv, BasicTypes), "test_graph_guard_condition_service", &service_options); @@ -1354,7 +1334,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi } // Graph change since destroy service - ret = rcl_service_fini(&service, &node); + ret = rcl_service_fini(&service, node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; { @@ -1367,7 +1347,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi rcl_client_options_t client_options = rcl_client_get_default_options(); ret = rcl_client_init( &client, - &node, + node_ptr, ROSIDL_GET_SRV_TYPE_SUPPORT(test_msgs, srv, BasicTypes), "test_graph_guard_condition_service", &client_options); @@ -1379,7 +1359,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi } // Graph change since destroying client - ret = rcl_client_fini(&client, &node); + ret = rcl_client_fini(&client, node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; { @@ -1389,7 +1369,8 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi // Graph change since adding new node rcl_node_t node_new = rcl_get_zero_initialized_node(); - ret = rcl_node_init(&node_new, "test_graph2", "", &context, &node_options); + rcl_node_options_t node_options = rcl_node_get_default_options(); + ret = rcl_node_init(&node_new, "test_graph2", "", context_ptr, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; { From 396010b16fa12b2b52c40dffb76321f50ea767c3 Mon Sep 17 00:00:00 2001 From: Barry Xu Date: Tue, 22 Dec 2020 09:25:58 +0800 Subject: [PATCH 5/5] Fix a bug. Signed-off-by: Barry Xu --- rcl/test/rcl/test_graph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index e9e9fcfb9..5e4c0a218 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -1257,7 +1257,7 @@ TEST_F(CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION), test_graph_guard_conditi // Wait for no graph change condition int idx = 0; - while (idx < 100) { + for (; idx < 100; idx++) { ret = rcl_wait_set_clear(&wait_set); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_wait_set_add_guard_condition(&wait_set, graph_guard_condition, NULL);