From bd5fae549073068e249e770fe927c15b94a8fa4c Mon Sep 17 00:00:00 2001 From: Ross Desmond <44277324+ross-desmond@users.noreply.github.com> Date: Tue, 2 Apr 2019 16:45:32 -0700 Subject: [PATCH 1/4] Add tests for unsupported qos checks on initialize Signed-off-by: Ross Desmond <44277324+ross-desmond@users.noreply.github.com> --- rcl/src/rcl/event.c | 4 +- rcl/test/rcl/test_count_matched.cpp | 2 +- rcl/test/rcl/test_events.cpp | 159 ++++++++++++++++++++++++---- 3 files changed, 141 insertions(+), 24 deletions(-) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 46e8967d1..84fd1a25e 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -83,7 +83,7 @@ rcl_publisher_event_init( } return rmw_publisher_event_init(&event->impl->rmw_handle, - publisher->impl->rmw_handle, rmw_event_type); + publisher->impl->rmw_handle, rmw_event_type); } rcl_ret_t @@ -121,7 +121,7 @@ rcl_subscription_event_init( } return rmw_subscription_event_init(&event->impl->rmw_handle, - subscription->impl->rmw_handle, rmw_event_type); + subscription->impl->rmw_handle, rmw_event_type); } rcl_ret_t diff --git a/rcl/test/rcl/test_count_matched.cpp b/rcl/test/rcl/test_count_matched.cpp index bfec02731..dbdd5c35c 100644 --- a/rcl/test/rcl/test_count_matched.cpp +++ b/rcl/test/rcl/test_count_matched.cpp @@ -194,7 +194,7 @@ TEST_F(CLASSNAME(TestCountFixture, RMW_IMPLEMENTATION), rcl_publisher_t pub = rcl_get_zero_initialized_publisher(); - rcl_publisher_options_t pub_ops; + rcl_publisher_options_t pub_ops = rcl_publisher_get_default_options(); pub_ops.qos.history = RMW_QOS_POLICY_HISTORY_KEEP_LAST; pub_ops.qos.depth = 10; pub_ops.qos.reliability = RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT; diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 36d0e7152..5373e093d 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -38,11 +38,18 @@ constexpr int DEADLINE_PERIOD_IN_MS = 1000; # define CLASSNAME(NAME, SUFFIX) NAME #endif -class CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test +bool is_opensplice = + std::string(rmw_get_implementation_identifier()).find("rmw_opensplice") == 0; + +bool is_fastrtps = + std::string(rmw_get_implementation_identifier()).find("rmw_fastrtps") == 0; + +class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test { public: void SetUp() { + is_unsupported = is_fastrtps || is_opensplice; rcl_ret_t ret; { rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); @@ -62,26 +69,57 @@ class CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test rcl_node_options_t node_options = rcl_node_get_default_options(); ret = rcl_node_init(this->node_ptr, name, "", this->context_ptr, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ts = ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, Primitives); + } + + rcl_ret_t SetupPublisher( + const rmw_time_t & deadline, + const rmw_time_t & lifespan, + const rmw_time_t & liveliness_lease_duration, + const rmw_qos_liveliness_policy_t liveliness_policy) + { + // init publisher + publisher = rcl_get_zero_initialized_publisher(); + rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); + publisher_options.qos.deadline = deadline; + publisher_options.qos.lifespan = lifespan; + publisher_options.qos.liveliness = liveliness_policy; + publisher_options.qos.liveliness_lease_duration = liveliness_lease_duration; + return rcl_publisher_init(&publisher, this->node_ptr, this->ts, this->topic, + &publisher_options); + } + + rcl_ret_t SetupSubscriber( + const rmw_time_t & deadline, + const rmw_time_t & lifespan, + const rmw_time_t & liveliness_lease_duration, + const rmw_qos_liveliness_policy_t liveliness_policy) + { + // init publisher + subscription = rcl_get_zero_initialized_subscription(); + rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); + subscription_options.qos.deadline = deadline; + subscription_options.qos.lifespan = lifespan; + subscription_options.qos.liveliness = liveliness_policy; + subscription_options.qos.liveliness_lease_duration = liveliness_lease_duration; + + return rcl_subscription_init(&subscription, this->node_ptr, this->ts, this->topic, + &subscription_options); } void SetUpPubSub( - const rcl_publisher_event_type_t pub_event_type, - const rcl_subscription_event_type_t sub_event_type) + const rcl_publisher_event_type_t & pub_event_type, + const rcl_subscription_event_type_t & sub_event_type) { rcl_ret_t ret; - const char * topic = "rcl_test_publisher_subscription_events"; - const rosidl_message_type_support_t * ts = - ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, Primitives); + rmw_time_t lifespan {0, 0}; + rmw_time_t deadline {DEADLINE_PERIOD_IN_MS / 1000, 0}; + rmw_time_t lease_duration {LIVELINESS_LEASE_DURATION_IN_MS / 1000, 0}; + rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC; // init publisher - publisher = rcl_get_zero_initialized_publisher(); - rcl_publisher_options_t publisher_options = rcl_publisher_get_default_options(); - publisher_options.qos.deadline = rmw_time_t {DEADLINE_PERIOD_IN_MS / 1000, 0}; - publisher_options.qos.liveliness = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC; - publisher_options.qos.liveliness_lease_duration = - rmw_time_t {LIVELINESS_LEASE_DURATION_IN_MS / 1000, 0}; - ret = rcl_publisher_init(&publisher, this->node_ptr, ts, topic, &publisher_options); + ret = SetupPublisher(deadline, lifespan, lease_duration, liveliness_policy); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // init publisher events @@ -90,13 +128,7 @@ class CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // init subscription - subscription = rcl_get_zero_initialized_subscription(); - rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options(); - subscription_options.qos.deadline = rmw_time_t {DEADLINE_PERIOD_IN_MS / 1000, 0}; - subscription_options.qos.liveliness = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC; - subscription_options.qos.liveliness_lease_duration = - rmw_time_t {LIVELINESS_LEASE_DURATION_IN_MS / 1000, 0}; - ret = rcl_subscription_init(&subscription, this->node_ptr, ts, topic, &subscription_options); + ret = SetupSubscriber(deadline, lifespan, lease_duration, liveliness_policy); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // init subscription event @@ -146,6 +178,9 @@ class CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test rcl_event_t publisher_event; rcl_subscription_t subscription; rcl_event_t subscription_event; + bool is_unsupported; + const char * topic = "rcl_test_publisher_subscription_events"; + const rosidl_message_type_support_t * ts; }; bool @@ -169,7 +204,7 @@ wait_for_msgs_and_events( rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); rcl_ret_t ret = rcl_wait_set_init(&wait_set, num_subscriptions, 0, 0, 0, 0, num_events, - context, rcl_get_default_allocator()); + context, rcl_get_default_allocator()); EXPECT_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); @@ -220,11 +255,87 @@ wait_for_msgs_and_events( return subscription_event_ready || publisher_event_ready; } +TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_unsupported_lifespan) { + if (is_unsupported) { + rmw_time_t deadline {0, 0}; + rmw_time_t lifespan {1, 0}; + rmw_time_t lease_duration {1, 0}; + rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC; + EXPECT_EQ(RMW_RET_ERROR, + SetupSubscriber(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized subscriber lifespan when unsupported"; + EXPECT_EQ(RMW_RET_ERROR, + SetupPublisher(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized publisher lifespan when unsupported"; + + lifespan = {0, 1}; + EXPECT_EQ(RMW_RET_ERROR, + SetupSubscriber(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized subscriber lifespan when unsupported"; + EXPECT_EQ(RMW_RET_ERROR, + SetupPublisher(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized publisher lifespan when unsupported"; + } +} + +TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_unsupported_liveliness) { + if (is_unsupported) { + rmw_time_t deadline {0, 0}; + rmw_time_t lifespan {0, 0}; + rmw_time_t lease_duration {0, 0}; + rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE; + EXPECT_EQ(RMW_RET_ERROR, + SetupSubscriber(deadline, lifespan, lease_duration, + liveliness_policy)) << + "Initialized subscriber RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE when unsupported"; + EXPECT_EQ(RMW_RET_ERROR, + SetupPublisher(deadline, lifespan, lease_duration, + liveliness_policy)) << + "Initialized publisher RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_NODE when unsupported"; + + liveliness_policy = RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC; + EXPECT_EQ(RMW_RET_ERROR, + SetupSubscriber(deadline, lifespan, lease_duration, + liveliness_policy)) << + "Initialized subscriber RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC when unsupported"; + EXPECT_EQ(RMW_RET_ERROR, + SetupPublisher(deadline, lifespan, lease_duration, + liveliness_policy)) << + "Initialized publisher RMW_QOS_POLICY_LIVELINESS_MANUAL_BY_TOPIC when unsupported"; + } +} + +TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_unsupported_unsupported_deadline) { + if (is_unsupported) { + rmw_time_t deadline; + rmw_time_t lifespan {0, 0}; + rmw_time_t lease_duration {0, 0}; + rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC; + EXPECT_EQ(RMW_RET_ERROR, + SetupSubscriber(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized subscriber deadline when unsupported"; + EXPECT_EQ(RMW_RET_ERROR, + SetupPublisher(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized publisher deadline when unsupported"; + + deadline = {0, 1}; + EXPECT_EQ(RMW_RET_ERROR, + SetupSubscriber(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized subscriber deadline when unsupported"; + EXPECT_EQ(RMW_RET_ERROR, + SetupPublisher(deadline, lifespan, lease_duration, + liveliness_policy)) << "Initialized publisher deadline when unsupported"; + } +} + /* * Basic test of publisher and subscriber liveliness events, with publisher killed */ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_liveliness_kill_pub) { + if (is_unsupported) { + return; + } // initialize publisher and subscriber SetUpPubSub(RCL_PUBLISHER_LIVELINESS_LOST, RCL_SUBSCRIPTION_LIVELINESS_CHANGED); @@ -301,6 +412,9 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_liveliness_k */ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_deadline_missed) { + if (is_unsupported) { + return; + } // initialize publisher and subscriber SetUpPubSub(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED, RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED); @@ -370,6 +484,9 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_deadline_mis */ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_no_deadline_missed) { + if (is_unsupported) { + return; + } // initialize publisher and subscriber SetUpPubSub(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED, RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED); From df16ac7fd219ced61ff9b0bdfcf2180fa8ee25cd Mon Sep 17 00:00:00 2001 From: Ross Desmond <44277324+ross-desmond@users.noreply.github.com> Date: Tue, 2 Apr 2019 17:48:35 -0700 Subject: [PATCH 2/4] Use std::chrono instead of manual conversions --- rcl/test/rcl/test_events.cpp | 48 +++++++++++++++++++----------------- rcl/test/rcl/test_timer.cpp | 2 +- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 5373e093d..78b8cc9d0 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -27,9 +27,12 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" -constexpr int DISCOVERY_WAIT_TIME_IN_MS = 1000; -constexpr int LIVELINESS_LEASE_DURATION_IN_MS = 1000; -constexpr int DEADLINE_PERIOD_IN_MS = 1000; +using std::chrono::milliseconds; +using std::chrono::seconds; + +constexpr seconds DISCOVERY_WAIT_TIME_IN_S(1); +constexpr seconds LIVELINESS_LEASE_DURATION_IN_S(1); +constexpr seconds DEADLINE_PERIOD_IN_S(1); #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX @@ -38,17 +41,16 @@ constexpr int DEADLINE_PERIOD_IN_MS = 1000; # define CLASSNAME(NAME, SUFFIX) NAME #endif -bool is_opensplice = - std::string(rmw_get_implementation_identifier()).find("rmw_opensplice") == 0; - -bool is_fastrtps = - std::string(rmw_get_implementation_identifier()).find("rmw_fastrtps") == 0; - class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test { public: void SetUp() { + const bool is_opensplice = + std::string(rmw_get_implementation_identifier()).find("rmw_opensplice") == 0; + const bool is_fastrtps = + std::string(rmw_get_implementation_identifier()).find("rmw_fastrtps") == 0; + is_unsupported = is_fastrtps || is_opensplice; rcl_ret_t ret; { @@ -107,15 +109,15 @@ class CLASSNAME (TestEventFixture, RMW_IMPLEMENTATION) : public ::testing::Test &subscription_options); } - void SetUpPubSub( + void SetupPublisherAndSubscriber( const rcl_publisher_event_type_t & pub_event_type, const rcl_subscription_event_type_t & sub_event_type) { rcl_ret_t ret; rmw_time_t lifespan {0, 0}; - rmw_time_t deadline {DEADLINE_PERIOD_IN_MS / 1000, 0}; - rmw_time_t lease_duration {LIVELINESS_LEASE_DURATION_IN_MS / 1000, 0}; + rmw_time_t deadline {DEADLINE_PERIOD_IN_S.count(), 0}; + rmw_time_t lease_duration {LIVELINESS_LEASE_DURATION_IN_S.count(), 0}; rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC; // init publisher @@ -336,14 +338,14 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_liveliness_k if (is_unsupported) { return; } - // initialize publisher and subscriber - SetUpPubSub(RCL_PUBLISHER_LIVELINESS_LOST, RCL_SUBSCRIPTION_LIVELINESS_CHANGED); + + SetupPublisherAndSubscriber(RCL_PUBLISHER_LIVELINESS_LOST, RCL_SUBSCRIPTION_LIVELINESS_CHANGED); // wait for discovery // 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(DISCOVERY_WAIT_TIME_IN_MS)); + std::this_thread::sleep_for(DISCOVERY_WAIT_TIME_IN_S); rcl_ret_t ret; @@ -365,7 +367,7 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_liveliness_k EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; // wait for lease duration to expire - std::this_thread::sleep_for(std::chrono::milliseconds(LIVELINESS_LEASE_DURATION_IN_MS + 500)); + std::this_thread::sleep_for(LIVELINESS_LEASE_DURATION_IN_S + milliseconds(500)); // wait for events bool msg_ready, subscription_event_ready, publisher_event_ready; @@ -415,14 +417,15 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_deadline_mis if (is_unsupported) { return; } - // initialize publisher and subscriber - SetUpPubSub(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED, RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED); + + SetupPublisherAndSubscriber(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED, + RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED); // wait for discovery. also adds delay to publishing of message // 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(DEADLINE_PERIOD_IN_MS + 500)); + std::this_thread::sleep_for(DEADLINE_PERIOD_IN_S + milliseconds(500)); rcl_ret_t ret; @@ -487,14 +490,15 @@ TEST_F(CLASSNAME(TestEventFixture, RMW_IMPLEMENTATION), test_pubsub_no_deadline_ if (is_unsupported) { return; } - // initialize publisher and subscriber - SetUpPubSub(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED, RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED); + + SetupPublisherAndSubscriber(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED, + RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED); // wait for discovery. also adds delay to publishing of message // 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(DEADLINE_PERIOD_IN_MS - 500)); + std::this_thread::sleep_for(DEADLINE_PERIOD_IN_S - milliseconds(500)); rcl_ret_t ret; diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 71d6f1545..3ab86508e 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -460,7 +460,7 @@ TEST_F(TestTimerFixture, test_ros_time_wakes_wait) { std::thread wait_thr([&](void) { rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, 0, context_ptr, - rcl_get_default_allocator()); + rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_EQ(RCL_RET_OK, rcl_wait_set_add_timer(&wait_set, &timer, NULL)) << From d4d89cea52b74f5b9bb8020376bf6b856e1816dc Mon Sep 17 00:00:00 2001 From: Ross Desmond <44277324+ross-desmond@users.noreply.github.com> Date: Tue, 2 Apr 2019 17:55:42 -0700 Subject: [PATCH 3/4] Reformat spacing event.c --- rcl/src/rcl/event.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 84fd1a25e..0487382d9 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -82,8 +82,10 @@ rcl_publisher_event_init( return RCL_RET_INVALID_ARGUMENT; } - return rmw_publisher_event_init(&event->impl->rmw_handle, - publisher->impl->rmw_handle, rmw_event_type); + return rmw_publisher_event_init( + &event->impl->rmw_handle, + publisher->impl->rmw_handle, + rmw_event_type); } rcl_ret_t From 78564924484dd5d49bcce434fe83be636ee8b951 Mon Sep 17 00:00:00 2001 From: Ross Desmond <44277324+ross-desmond@users.noreply.github.com> Date: Wed, 3 Apr 2019 12:13:19 -0700 Subject: [PATCH 4/4] Reformat subscription event init return event.c --- rcl/src/rcl/event.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 0487382d9..0f44638e2 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -122,8 +122,10 @@ rcl_subscription_event_init( return RCL_RET_INVALID_ARGUMENT; } - return rmw_subscription_event_init(&event->impl->rmw_handle, - subscription->impl->rmw_handle, rmw_event_type); + return rmw_subscription_event_init( + &event->impl->rmw_handle, + subscription->impl->rmw_handle, + rmw_event_type); } rcl_ret_t