From 28c65fd63263675dbc5dd65ba73d69fc6574852e Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 12 Mar 2020 16:05:51 -0300 Subject: [PATCH 01/18] Add tests for time.c module Signed-off-by: Jorge Perez --- rcl/test/rcl/test_time.cpp | 68 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 2b552ea68..d173aeed5 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -268,6 +268,18 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), specific_clock_instantiation) { EXPECT_EQ(uninitialized_clock.type, RCL_CLOCK_UNINITIALIZED) << "Expected time source of type RCL_CLOCK_UNINITIALIZED"; EXPECT_TRUE(rcutils_allocator_is_valid(&(uninitialized_clock.allocator))); + ret = rcl_clock_fini(&uninitialized_clock); + EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT) << rcl_get_error_string().str; + rcl_reset_error(); + EXPECT_EQ( + rcl_ros_clock_fini(&uninitialized_clock), RCL_RET_ERROR) << rcl_get_error_string().str; + rcl_reset_error(); + EXPECT_EQ( + rcl_steady_clock_fini(&uninitialized_clock), RCL_RET_ERROR) << rcl_get_error_string().str; + rcl_reset_error(); + EXPECT_EQ( + rcl_system_clock_fini(&uninitialized_clock), RCL_RET_ERROR) << rcl_get_error_string().str; + rcl_reset_error(); } { rcl_clock_t ros_clock; @@ -296,6 +308,12 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), specific_clock_instantiation) { ret = rcl_clock_fini(&steady_clock); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; } + { + rcl_clock_t fail_clock; + rcl_clock_type_t undefined_type = (rcl_clock_type_t) 130; + rcl_ret_t ret = rcl_clock_init(undefined_type, &fail_clock, &allocator); + EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT) << rcl_get_error_string().str; + } } TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_difference) { @@ -333,6 +351,9 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_difference) { ret = rcl_difference_times(&b, &a, &d); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_EQ(d.nanoseconds, -1000); + + b.clock_type = RCL_SYSTEM_TIME; + EXPECT_EQ(rcl_difference_times(&a, &b, &d), RCL_RET_ERROR) << rcl_get_error_string().str; } TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_difference_signed) { @@ -484,6 +505,28 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_clock_change_callbacks) { reset_callback_triggers(); } +TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_fail_set_jump_callbacks) { + rcl_clock_t fail_clock; + rcl_time_jump_t time_jump; + rcl_jump_threshold_t threshold; + threshold.min_forward.nanoseconds = -1; + threshold.min_backward.nanoseconds = 0; + + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, + rcl_clock_add_jump_callback(&fail_clock, threshold, clock_callback, &time_jump)) << + rcl_get_error_string().str; + rcl_reset_error(); + + threshold.min_forward.nanoseconds = 0; + threshold.min_backward.nanoseconds = 1; + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, + rcl_clock_add_jump_callback(&fail_clock, threshold, clock_callback, &time_jump)) << + rcl_get_error_string().str; + rcl_reset_error(); +} + TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_forward_jump_callbacks) { rcl_allocator_t allocator = rcl_get_default_allocator(); auto * ros_clock = @@ -730,3 +773,28 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), add_remove_add_jump_callback) { rcl_get_error_string().str; EXPECT_EQ(1u, clock->num_jump_callbacks); } + +TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), failed_get_now) { + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_clock_t uninitialized_clock; + rcl_time_point_value_t query_now; + rcl_ret_t ret = rcl_clock_init(RCL_CLOCK_UNINITIALIZED, &uninitialized_clock, &allocator); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + EXPECT_EQ(uninitialized_clock.type, RCL_CLOCK_UNINITIALIZED); + uninitialized_clock.get_now = NULL; + EXPECT_EQ(RCL_RET_ERROR, rcl_clock_get_now(&uninitialized_clock, &query_now)); +} + +TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), fail_override) { + rcl_clock_t ros_clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + bool result; + rcl_time_point_value_t set_point = 1000000000ull; + ASSERT_EQ(RCL_RET_OK, rcl_clock_init( + RCL_CLOCK_UNINITIALIZED, &ros_clock, &allocator)) << rcl_get_error_string().str; + + EXPECT_EQ(RCL_RET_ERROR, rcl_enable_ros_time_override(&ros_clock)); + EXPECT_EQ(RCL_RET_ERROR, rcl_disable_ros_time_override(&ros_clock)); + EXPECT_EQ(RCL_RET_ERROR, rcl_is_enabled_ros_time_override(&ros_clock, &result)); + EXPECT_EQ(RCL_RET_ERROR, rcl_set_ros_time_override(&ros_clock, set_point)); +} From 892272dfee39f2340b18c583c22c8058bf0046f8 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 16 Mar 2020 17:03:41 -0300 Subject: [PATCH 02/18] Add test for function rcl_timer_clock Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 746f575b6..d84c64f9b 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -559,3 +559,22 @@ TEST_F(TestPreInitTimer, test_timer_get_allocator) { EXPECT_EQ(NULL, rcl_timer_get_allocator(nullptr)); } + +TEST_F(TestTimerFixture, test_timer_clock) { + rcl_ret_t ret; + + rcl_clock_t * clock_impl; + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + + ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, RCL_MS_TO_NS(5), nullptr, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + EXPECT_EQ(RCL_RET_OK, rcl_timer_clock(&timer, &clock_impl)); + EXPECT_EQ(clock_impl, &clock); +} From 39e9d87c0b1ecbfd76b11f96c2a7483caac42131 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Fri, 3 Apr 2020 20:00:56 -0300 Subject: [PATCH 03/18] Fix linter style issues Signed-off-by: Jorge Perez --- rcl/test/rcl/test_time.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index d173aeed5..978de5330 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -509,6 +509,7 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_fail_set_jump_callbacks) rcl_clock_t fail_clock; rcl_time_jump_t time_jump; rcl_jump_threshold_t threshold; + threshold.on_clock_change = NULL; threshold.min_forward.nanoseconds = -1; threshold.min_backward.nanoseconds = 0; @@ -790,8 +791,9 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), fail_override) { rcl_allocator_t allocator = rcl_get_default_allocator(); bool result; rcl_time_point_value_t set_point = 1000000000ull; - ASSERT_EQ(RCL_RET_OK, rcl_clock_init( - RCL_CLOCK_UNINITIALIZED, &ros_clock, &allocator)) << rcl_get_error_string().str; + ASSERT_EQ( + RCL_RET_OK, rcl_clock_init( + RCL_CLOCK_UNINITIALIZED, &ros_clock, &allocator)) << rcl_get_error_string().str; EXPECT_EQ(RCL_RET_ERROR, rcl_enable_ros_time_override(&ros_clock)); EXPECT_EQ(RCL_RET_ERROR, rcl_disable_ros_time_override(&ros_clock)); From 1c76cfb55aa857be80d65c75f4eb2d380c4fc07e Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 7 Apr 2020 16:32:28 -0300 Subject: [PATCH 04/18] Add test for rcl_timer_call Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 50 +++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index d84c64f9b..5261226ae 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -578,3 +578,53 @@ TEST_F(TestTimerFixture, test_timer_clock) { EXPECT_EQ(RCL_RET_OK, rcl_timer_clock(&timer, &clock_impl)); EXPECT_EQ(clock_impl, &clock); } + +static uint8_t times_called = 0; +static void callback_function(rcl_timer_t * timer, int64_t last_call) +{ + (void) timer; + (void) last_call; + times_called++; +} +static rcl_timer_callback_t timer_callback_test = &callback_function; + +// To test: rcl_timer_call +TEST_F(TestTimerFixture, test_timer_call) { + rcl_ret_t ret; + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + int64_t next_call_start, next_call_end; + + ASSERT_EQ(RCL_RET_OK, + rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str; + + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, + rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_ret_t ret = rcl_timer_fini(&timer); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + + EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_start)); + ret = rcl_timer_call(&timer); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 1); + + ret = rcl_timer_call(&timer); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_timer_call(&timer); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 3); + EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); + EXPECT_GT(next_call_end, next_call_start); + + // Test timer with period 0 + // Test calling when timer is canceled prior to call +} From ecff595a8a395be84e59c4f9ab9fe6023b3ac821 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 8 Apr 2020 17:39:26 -0300 Subject: [PATCH 05/18] Add callback test and improve call test Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 47 +++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 5261226ae..81bf6a85d 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -588,16 +588,17 @@ static void callback_function(rcl_timer_t * timer, int64_t last_call) } static rcl_timer_callback_t timer_callback_test = &callback_function; -// To test: rcl_timer_call TEST_F(TestTimerFixture, test_timer_call) { rcl_ret_t ret; rcl_clock_t clock; rcl_allocator_t allocator = rcl_get_default_allocator(); rcl_timer_t timer = rcl_get_zero_initialized_timer(); int64_t next_call_start, next_call_end; + int64_t old_period = 0; - ASSERT_EQ(RCL_RET_OK, - rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str; + ASSERT_EQ( + RCL_RET_OK, + rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str; ret = rcl_timer_init( &timer, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, @@ -625,6 +626,42 @@ TEST_F(TestTimerFixture, test_timer_call) { EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); EXPECT_GT(next_call_end, next_call_start); - // Test timer with period 0 - // Test calling when timer is canceled prior to call + next_call_start = next_call_end; + ASSERT_EQ(RCL_RET_OK, rcl_timer_exchange_period(&timer, 0, &old_period)); + EXPECT_EQ(RCL_S_TO_NS(1), old_period); + ret = rcl_timer_call(&timer); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 4); + EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); + EXPECT_GT(next_call_start, next_call_end); + + EXPECT_EQ(RCL_RET_OK, rcl_timer_cancel(&timer)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_TIMER_CANCELED, rcl_timer_call(&timer)); + EXPECT_EQ(times_called, 4); +} + +TEST_F(TestTimerFixture, test_get_callback) { + rcl_ret_t ret; + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + + ASSERT_EQ( + RCL_RET_OK, + rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str; + + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, + rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_ret_t ret = rcl_timer_fini(&timer); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + + ASSERT_EQ(timer_callback_test, rcl_timer_get_callback(&timer)) << rcl_get_error_string().str; } From de73cb2ce1f30764969e03b93544b57e35dc2130 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 8 Apr 2020 18:28:45 -0300 Subject: [PATCH 06/18] Add fixture class to reuse timer init/fini code Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 83 ++++--------------------------------- 1 file changed, 7 insertions(+), 76 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 81bf6a85d..e374f28ac 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -560,68 +560,22 @@ TEST_F(TestPreInitTimer, test_timer_get_allocator) { EXPECT_EQ(NULL, rcl_timer_get_allocator(nullptr)); } -TEST_F(TestTimerFixture, test_timer_clock) { - rcl_ret_t ret; - +TEST_F(TestPreInitTimer, test_timer_clock) { rcl_clock_t * clock_impl; - rcl_clock_t clock; - rcl_allocator_t allocator = rcl_get_default_allocator(); - rcl_timer_t timer = rcl_get_zero_initialized_timer(); - - ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - - ret = rcl_timer_init( - &timer, &clock, this->context_ptr, RCL_MS_TO_NS(5), nullptr, rcl_get_default_allocator()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_EQ(RCL_RET_OK, rcl_timer_clock(&timer, &clock_impl)); EXPECT_EQ(clock_impl, &clock); } -static uint8_t times_called = 0; -static void callback_function(rcl_timer_t * timer, int64_t last_call) -{ - (void) timer; - (void) last_call; - times_called++; -} -static rcl_timer_callback_t timer_callback_test = &callback_function; - -TEST_F(TestTimerFixture, test_timer_call) { - rcl_ret_t ret; - rcl_clock_t clock; - rcl_allocator_t allocator = rcl_get_default_allocator(); - rcl_timer_t timer = rcl_get_zero_initialized_timer(); +TEST_F(TestPreInitTimer, test_timer_call) { int64_t next_call_start, next_call_end; int64_t old_period = 0; - ASSERT_EQ( - RCL_RET_OK, - rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str; - - ret = rcl_timer_init( - &timer, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, - rcl_get_default_allocator()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - ret = rcl_clock_fini(&clock); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_ret_t ret = rcl_timer_fini(&timer); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); - EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_start)); - ret = rcl_timer_call(&timer); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; EXPECT_EQ(times_called, 1); - ret = rcl_timer_call(&timer); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_timer_call(&timer); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; EXPECT_EQ(times_called, 3); EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); EXPECT_GT(next_call_end, next_call_start); @@ -629,8 +583,7 @@ TEST_F(TestTimerFixture, test_timer_call) { next_call_start = next_call_end; ASSERT_EQ(RCL_RET_OK, rcl_timer_exchange_period(&timer, 0, &old_period)); EXPECT_EQ(RCL_S_TO_NS(1), old_period); - ret = rcl_timer_call(&timer); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; EXPECT_EQ(times_called, 4); EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); EXPECT_GT(next_call_start, next_call_end); @@ -640,28 +593,6 @@ TEST_F(TestTimerFixture, test_timer_call) { EXPECT_EQ(times_called, 4); } -TEST_F(TestTimerFixture, test_get_callback) { - rcl_ret_t ret; - rcl_clock_t clock; - rcl_allocator_t allocator = rcl_get_default_allocator(); - rcl_timer_t timer = rcl_get_zero_initialized_timer(); - - ASSERT_EQ( - RCL_RET_OK, - rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str; - - ret = rcl_timer_init( - &timer, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, - rcl_get_default_allocator()); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - ret = rcl_clock_fini(&clock); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_ret_t ret = rcl_timer_fini(&timer); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - }); - +TEST_F(TestPreInitTimer, test_get_callback) { ASSERT_EQ(timer_callback_test, rcl_timer_get_callback(&timer)) << rcl_get_error_string().str; } From 4fd259f33ff14e770c75ff9fc633424e890f280c Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 8 Apr 2020 20:24:24 -0300 Subject: [PATCH 07/18] Change error code returned to match function being tested Signed-off-by: Jorge Perez --- rcl/test/rcl/test_time.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 978de5330..661d86ab3 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -269,7 +269,7 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), specific_clock_instantiation) { "Expected time source of type RCL_CLOCK_UNINITIALIZED"; EXPECT_TRUE(rcutils_allocator_is_valid(&(uninitialized_clock.allocator))); ret = rcl_clock_fini(&uninitialized_clock); - EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT) << rcl_get_error_string().str; + EXPECT_EQ(ret, RCL_RET_ERROR) << rcl_get_error_string().str; rcl_reset_error(); EXPECT_EQ( rcl_ros_clock_fini(&uninitialized_clock), RCL_RET_ERROR) << rcl_get_error_string().str; From 091fdd6da1be4079e76947644c81592295800ad1 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 8 Apr 2020 20:25:03 -0300 Subject: [PATCH 08/18] Add tests for timer_reset function Also fix other minor issues causing errors Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index e374f28ac..4a9ffdc40 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -562,13 +562,26 @@ TEST_F(TestPreInitTimer, test_timer_get_allocator) { TEST_F(TestPreInitTimer, test_timer_clock) { rcl_clock_t * clock_impl; - EXPECT_EQ(RCL_RET_OK, rcl_timer_clock(&timer, &clock_impl)); + EXPECT_EQ(RCL_RET_OK, rcl_timer_clock(&timer, &clock_impl)) << rcl_get_error_string().str; EXPECT_EQ(clock_impl, &clock); } TEST_F(TestPreInitTimer, test_timer_call) { + /* + TODO: Use better names for next_call_start and next_call_end (?) + + API does not expose the explicit time for the next call of the timer, + but provides time missing before/after next call. + I'm using this to compare the time difference between consecutive calls, to see if + the next_time variable is updated properly. + Comparing next_call_start and next_call_end is the same as + comparing missing_time_next_call1 vs missing_time_next_call2 + + Open to suggestions before merging + */ int64_t next_call_start, next_call_end; int64_t old_period = 0; + times_called = 0; EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_start)); ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; @@ -596,3 +609,24 @@ TEST_F(TestPreInitTimer, test_timer_call) { TEST_F(TestPreInitTimer, test_get_callback) { ASSERT_EQ(timer_callback_test, rcl_timer_get_callback(&timer)) << rcl_get_error_string().str; } + +TEST_F(TestPreInitTimer, test_timer_reset) { + int64_t next_call_start, next_call_end; + times_called = 0; + + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 2); + EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_start)); + + ASSERT_EQ(RCL_RET_OK, rcl_timer_reset(&timer)); + EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); + EXPECT_GT(next_call_start, next_call_end); + + ASSERT_EQ(RCL_RET_OK, rcl_timer_cancel(&timer)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_TIMER_CANCELED, rcl_timer_call(&timer)); + EXPECT_EQ(times_called, 2); + ASSERT_EQ(RCL_RET_OK, rcl_timer_reset(&timer)); + EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 3); +} From d31bedaceb68124902058c4d552c3ae2d963f15f Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 9 Apr 2020 19:07:35 -0300 Subject: [PATCH 09/18] Add tests for exchange_callback function Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 4a9ffdc40..13099022b 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -72,6 +72,12 @@ static void callback_function(rcl_timer_t * timer, int64_t last_call) (void) last_call; times_called++; } +static void callback_function_changed(rcl_timer_t * timer, int64_t last_call) +{ + (void) timer; + (void) last_call; + times_called--; +} class TestPreInitTimer : public TestTimerFixture { @@ -80,6 +86,7 @@ class TestPreInitTimer : public TestTimerFixture rcl_allocator_t allocator; rcl_timer_t timer; rcl_timer_callback_t timer_callback_test = &callback_function; + rcl_timer_callback_t timer_callback_changed = &callback_function_changed; void SetUp() override { @@ -630,3 +637,15 @@ TEST_F(TestPreInitTimer, test_timer_reset) { EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; EXPECT_EQ(times_called, 3); } + +TEST_F(TestPreInitTimer, test_timer_exchange_callback) { + times_called = 0; + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 1); + ASSERT_EQ( + timer_callback_test, rcl_timer_exchange_callback( + &timer, timer_callback_changed)) << rcl_get_error_string().str; + + ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + EXPECT_EQ(times_called, 0); +} From a55658a471d50a76f6096d95a856fe3842aad604 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 9 Apr 2020 19:17:53 -0300 Subject: [PATCH 10/18] Add null parameter test for get guard condition Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 13099022b..336815b07 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -649,3 +649,7 @@ TEST_F(TestPreInitTimer, test_timer_exchange_callback) { ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; EXPECT_EQ(times_called, 0); } + +TEST_F(TestPreInitTimer, test_invalid_get_guard) { + ASSERT_EQ(NULL, rcl_timer_get_guard_condition(nullptr)); +} From aba9022e50d370a16070296870603870d8e63d59 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 9 Apr 2020 20:16:18 -0300 Subject: [PATCH 11/18] Add allocator testing tools auxiliar file This will be handy to test BAD_ALLOCATOR conditions through the whole rcl package Signed-off-by: Jorge Perez --- rcl/test/rcl/allocator_testing_utils.h | 94 ++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 rcl/test/rcl/allocator_testing_utils.h diff --git a/rcl/test/rcl/allocator_testing_utils.h b/rcl/test/rcl/allocator_testing_utils.h new file mode 100644 index 000000000..cdc707d68 --- /dev/null +++ b/rcl/test/rcl/allocator_testing_utils.h @@ -0,0 +1,94 @@ +// 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__ALLOCATOR_TESTING_UTILS_H_ +#define RCL__ALLOCATOR_TESTING_UTILS_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include + +#include "rcutils/allocator.h" + +typedef struct __failing_allocator_state +{ + bool is_failing; +} __failing_allocator_state; + +void * +failing_malloc(size_t size, void * state) +{ + if (((__failing_allocator_state *)state)->is_failing) { + return nullptr; + } + return rcutils_get_default_allocator().allocate(size, rcutils_get_default_allocator().state); +} + +void * +failing_realloc(void * pointer, size_t size, void * state) +{ + if (((__failing_allocator_state *)state)->is_failing) { + return nullptr; + } + return rcutils_get_default_allocator().reallocate( + pointer, size, rcutils_get_default_allocator().state); +} + +void +failing_free(void * pointer, void * state) +{ + if (((__failing_allocator_state *)state)->is_failing) { + return; + } + rcutils_get_default_allocator().deallocate(pointer, rcutils_get_default_allocator().state); +} + +void * +failing_calloc(size_t number_of_elements, size_t size_of_element, void * state) +{ + if (((__failing_allocator_state *)state)->is_failing) { + return nullptr; + } + return rcutils_get_default_allocator().zero_allocate( + number_of_elements, size_of_element, rcutils_get_default_allocator().state); +} + +static inline rcutils_allocator_t +get_failing_allocator(void) +{ + static __failing_allocator_state state; + state.is_failing = true; + auto failing_allocator = rcutils_get_default_allocator(); + failing_allocator.allocate = failing_malloc; + failing_allocator.deallocate = failing_free; + failing_allocator.reallocate = failing_realloc; + failing_allocator.zero_allocate = failing_calloc; + failing_allocator.state = &state; + return failing_allocator; +} + +static inline void +set_failing_allocator_is_failing(rcutils_allocator_t & failing_allocator, bool state) +{ + ((__failing_allocator_state *)failing_allocator.state)->is_failing = state; +} + +#ifdef __cplusplus +} +#endif + +#endif // RCL__ALLOCATOR_TESTING_UTILS_H_ From 315c3a8c3303ca3e5986d1de39dfcea1fb402cde Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 9 Apr 2020 20:17:10 -0300 Subject: [PATCH 12/18] Add tests for failing ini/fini conditions Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 336815b07..8d1c7c4e9 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -23,6 +23,8 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "./allocator_testing_utils.h" + class TestTimerFixture : public ::testing::Test { public: @@ -653,3 +655,21 @@ TEST_F(TestPreInitTimer, test_timer_exchange_callback) { TEST_F(TestPreInitTimer, test_invalid_get_guard) { ASSERT_EQ(NULL, rcl_timer_get_guard_condition(nullptr)); } + +TEST_F(TestPreInitTimer, test_invalid_init_fini) { + rcl_allocator_t bad_allocator = get_failing_allocator(); + rcl_timer_t timer_fail; + + EXPECT_EQ( + RCL_RET_ALREADY_INIT, rcl_timer_init( + &timer, &clock, this->context_ptr, 500, nullptr, + rcl_get_default_allocator())) << rcl_get_error_string().str; + + timer_fail = rcl_get_zero_initialized_timer(); + ASSERT_EQ( + RCL_RET_BAD_ALLOC, rcl_timer_init( + &timer_fail, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, + bad_allocator)) << rcl_get_error_string().str; + + EXPECT_EQ(RCL_RET_OK, rcl_timer_fini(nullptr)); +} From d2a1191c3a20695204889d10ce0af880af272127 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 14 Apr 2020 11:45:25 -0300 Subject: [PATCH 13/18] Add test for get_period function Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 8d1c7c4e9..86843fcc0 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -673,3 +673,12 @@ TEST_F(TestPreInitTimer, test_invalid_init_fini) { EXPECT_EQ(RCL_RET_OK, rcl_timer_fini(nullptr)); } + +TEST_F(TestPreInitTimer, test_timer_get_period) { + int64_t period; + ASSERT_EQ(RCL_RET_OK, rcl_timer_get_period(&timer, &period)); + EXPECT_EQ(RCL_S_TO_NS(1), period); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_timer_get_period(nullptr, &period)); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_timer_get_period(&timer, nullptr)); +} From f7e90095985e8394887301242f32ff273f0be7f9 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 14 Apr 2020 11:50:05 -0300 Subject: [PATCH 14/18] Add test to get_time_since_last_call Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 86843fcc0..956714ae6 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -682,3 +682,11 @@ TEST_F(TestPreInitTimer, test_timer_get_period) { EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_timer_get_period(nullptr, &period)); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_timer_get_period(&timer, nullptr)); } + +TEST_F(TestPreInitTimer, test_time_since_last_call) { + rcl_time_point_value_t time_sice_next_call_start, time_sice_next_call_end; + + ASSERT_EQ(RCL_RET_OK, rcl_timer_get_time_since_last_call(&timer, &time_sice_next_call_start)); + ASSERT_EQ(RCL_RET_OK, rcl_timer_get_time_since_last_call(&timer, &time_sice_next_call_end)); + EXPECT_GT(time_sice_next_call_end, time_sice_next_call_start); +} From e96ff6c06e8bb455e596ff10c70bc4885f792165 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Apr 2020 10:06:03 -0300 Subject: [PATCH 15/18] Remove comment related to variable names Signed-off-by: Jorge Perez --- rcl/test/rcl/test_timer.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 956714ae6..ee6e5f41f 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -576,18 +576,6 @@ TEST_F(TestPreInitTimer, test_timer_clock) { } TEST_F(TestPreInitTimer, test_timer_call) { - /* - TODO: Use better names for next_call_start and next_call_end (?) - - API does not expose the explicit time for the next call of the timer, - but provides time missing before/after next call. - I'm using this to compare the time difference between consecutive calls, to see if - the next_time variable is updated properly. - Comparing next_call_start and next_call_end is the same as - comparing missing_time_next_call1 vs missing_time_next_call2 - - Open to suggestions before merging - */ int64_t next_call_start, next_call_end; int64_t old_period = 0; times_called = 0; From 64ac3a2c95bd6873e978415c3c05eb7e345e3405 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Apr 2020 15:20:38 -0300 Subject: [PATCH 16/18] Fix return value on test case for fini clock Signed-off-by: Jorge Perez --- rcl/test/rcl/test_time.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 661d86ab3..978de5330 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -269,7 +269,7 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), specific_clock_instantiation) { "Expected time source of type RCL_CLOCK_UNINITIALIZED"; EXPECT_TRUE(rcutils_allocator_is_valid(&(uninitialized_clock.allocator))); ret = rcl_clock_fini(&uninitialized_clock); - EXPECT_EQ(ret, RCL_RET_ERROR) << rcl_get_error_string().str; + EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT) << rcl_get_error_string().str; rcl_reset_error(); EXPECT_EQ( rcl_ros_clock_fini(&uninitialized_clock), RCL_RET_ERROR) << rcl_get_error_string().str; From 166f96424c4298351b5408362a9f4b2f50479377 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 11 May 2020 15:55:20 -0300 Subject: [PATCH 17/18] Address peer review comments Signed-off-by: Jorge Perez --- rcl/test/rcl/test_time.cpp | 4 ++++ rcl/test/rcl/test_timer.cpp | 16 +++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 978de5330..2634a8472 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -506,9 +506,13 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_clock_change_callbacks) { } TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_fail_set_jump_callbacks) { + rcl_allocator_t allocator = rcl_get_default_allocator(); rcl_clock_t fail_clock; rcl_time_jump_t time_jump; rcl_jump_threshold_t threshold; + rcl_ret_t ret = rcl_clock_init(RCL_CLOCK_UNINITIALIZED, &fail_clock, &allocator); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + threshold.on_clock_change = NULL; threshold.min_forward.nanoseconds = -1; threshold.min_backward.nanoseconds = 0; diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index ee6e5f41f..07edd5c97 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -570,13 +570,14 @@ TEST_F(TestPreInitTimer, test_timer_get_allocator) { } TEST_F(TestPreInitTimer, test_timer_clock) { - rcl_clock_t * clock_impl; + rcl_clock_t * clock_impl = nullptr; EXPECT_EQ(RCL_RET_OK, rcl_timer_clock(&timer, &clock_impl)) << rcl_get_error_string().str; EXPECT_EQ(clock_impl, &clock); } TEST_F(TestPreInitTimer, test_timer_call) { - int64_t next_call_start, next_call_end; + int64_t next_call_start = 0; + int64_t next_call_end = 0; int64_t old_period = 0; times_called = 0; @@ -608,7 +609,8 @@ TEST_F(TestPreInitTimer, test_get_callback) { } TEST_F(TestPreInitTimer, test_timer_reset) { - int64_t next_call_start, next_call_end; + int64_t next_call_start = 0; + int64_t next_call_end = 0; times_called = 0; ASSERT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; @@ -646,14 +648,13 @@ TEST_F(TestPreInitTimer, test_invalid_get_guard) { TEST_F(TestPreInitTimer, test_invalid_init_fini) { rcl_allocator_t bad_allocator = get_failing_allocator(); - rcl_timer_t timer_fail; + rcl_timer_t timer_fail = rcl_get_zero_initialized_timer(); EXPECT_EQ( RCL_RET_ALREADY_INIT, rcl_timer_init( &timer, &clock, this->context_ptr, 500, nullptr, rcl_get_default_allocator())) << rcl_get_error_string().str; - timer_fail = rcl_get_zero_initialized_timer(); ASSERT_EQ( RCL_RET_BAD_ALLOC, rcl_timer_init( &timer_fail, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, @@ -663,7 +664,7 @@ TEST_F(TestPreInitTimer, test_invalid_init_fini) { } TEST_F(TestPreInitTimer, test_timer_get_period) { - int64_t period; + int64_t period = 0; ASSERT_EQ(RCL_RET_OK, rcl_timer_get_period(&timer, &period)); EXPECT_EQ(RCL_S_TO_NS(1), period); @@ -672,7 +673,8 @@ TEST_F(TestPreInitTimer, test_timer_get_period) { } TEST_F(TestPreInitTimer, test_time_since_last_call) { - rcl_time_point_value_t time_sice_next_call_start, time_sice_next_call_end; + rcl_time_point_value_t time_sice_next_call_start = 0u; + rcl_time_point_value_t time_sice_next_call_end = 0u; ASSERT_EQ(RCL_RET_OK, rcl_timer_get_time_since_last_call(&timer, &time_sice_next_call_start)); ASSERT_EQ(RCL_RET_OK, rcl_timer_get_time_since_last_call(&timer, &time_sice_next_call_end)); From 66c90aff81f9d0bbd3c6056975e34f08689f3fd2 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 13 May 2020 12:53:11 -0300 Subject: [PATCH 18/18] Fix CLang warning, replacing NULL with false Signed-off-by: Jorge Perez --- rcl/test/rcl/test_time.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 2634a8472..f267f0871 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -513,7 +513,7 @@ TEST(CLASSNAME(rcl_time, RMW_IMPLEMENTATION), rcl_time_fail_set_jump_callbacks) rcl_ret_t ret = rcl_clock_init(RCL_CLOCK_UNINITIALIZED, &fail_clock, &allocator); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - threshold.on_clock_change = NULL; + threshold.on_clock_change = false; threshold.min_forward.nanoseconds = -1; threshold.min_backward.nanoseconds = 0;