Skip to content

Commit

Permalink
Modifies timers API to select autostart state (#1004)
Browse files Browse the repository at this point in the history 
* Modifies timers API to select autostart state

* Add test for the initial state of the timer

* Marks as deprecated the API rcl_timer_init

Signed-off-by: Voldivh <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
@Voldivh
Voldivh committed Jun 21, 2023
1 parent 230ae2f commit 43f8163
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 57 deletions.
22 changes: 20 additions & 2 deletions rcl/include/rcl/timer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ extern "C"
#include "rcl/macros.h"
#include "rcl/time.h"
#include "rcl/types.h"
#include "rcutils/logging_macros.h"
#include "rmw/rmw.h"

typedef struct rcl_timer_impl_s rcl_timer_impl_t;
Expand Down Expand Up @@ -125,8 +126,8 @@ rcl_get_zero_initialized_timer(void);
* // ... error handling
*
* rcl_timer_t timer = rcl_get_zero_initialized_timer();
* ret = rcl_timer_init(
* &timer, &clock, context, RCL_MS_TO_NS(100), my_timer_callback, allocator);
* ret = rcl_timer_init2(
* &timer, &clock, context, RCL_MS_TO_NS(100), my_timer_callback, allocator, true);
* // ... error handling, use the timer with a wait set, or poll it manually, then cleanup
* ret = rcl_timer_fini(&timer);
* // ... error handling
Expand All @@ -151,6 +152,7 @@ rcl_get_zero_initialized_timer(void);
* \param[in] period the duration between calls to the callback in nanoseconds
* \param[in] callback the user defined function to be called every period
* \param[in] allocator the allocator to use for allocations
* \param[in] autostart the state of the timer at initialization
* \return #RCL_RET_OK if the timer was initialized successfully, or
* \return #RCL_RET_INVALID_ARGUMENT if any arguments are invalid, or
* \return #RCL_RET_ALREADY_INIT if the timer was already initialized, or
Expand All @@ -160,6 +162,22 @@ rcl_get_zero_initialized_timer(void);
RCL_PUBLIC
RCL_WARN_UNUSED
rcl_ret_t
rcl_timer_init2(
rcl_timer_t * timer,
rcl_clock_t * clock,
rcl_context_t * context,
int64_t period,
const rcl_timer_callback_t callback,
rcl_allocator_t allocator,
bool autostart);

/**
* \deprecated `rcl_timer_init` implementation was removed.
* Refer to `rcl_timer_init2`.
*/
RCL_PUBLIC
RCUTILS_DEPRECATED_WITH_MSG("Call rcl_timer_init2 instead")
rcl_ret_t
rcl_timer_init(
rcl_timer_t * timer,
rcl_clock_t * clock,
Expand Down
17 changes: 16 additions & 1 deletion rcl/src/rcl/timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,21 @@ rcl_timer_init(
int64_t period,
const rcl_timer_callback_t callback,
rcl_allocator_t allocator)
{
return rcl_timer_init2(
timer, clock, context, period, callback,
allocator, true);
}

rcl_ret_t
rcl_timer_init2(
rcl_timer_t * timer,
rcl_clock_t * clock,
rcl_context_t * context,
int64_t period,
const rcl_timer_callback_t callback,
rcl_allocator_t allocator,
bool autostart)
{
RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(timer, RCL_RET_INVALID_ARGUMENT);
Expand Down Expand Up @@ -182,7 +197,7 @@ rcl_timer_init(
atomic_init(&impl.time_credit, 0);
atomic_init(&impl.last_call_time, now);
atomic_init(&impl.next_call_time, now + period);
atomic_init(&impl.canceled, false);
atomic_init(&impl.canceled, !autostart);
impl.allocator = allocator;

// Empty init on reset callback data
Expand Down
142 changes: 96 additions & 46 deletions rcl/test/rcl/test_timer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ class TestPreInitTimer : public TestTimerFixture
RCL_RET_OK,
rcl_clock_init(RCL_ROS_TIME, &clock, &allocator)) << rcl_get_error_string().str;

ret = rcl_timer_init(
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test,
rcl_get_default_allocator());
rcl_get_default_allocator(), true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
}

Expand All @@ -133,29 +133,29 @@ TEST_F(TestTimerFixture, test_timer_init_with_invalid_arguments) {
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
rcl_timer_t timer = rcl_get_zero_initialized_timer();

ret = rcl_timer_init(
nullptr, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, allocator);
ret = rcl_timer_init2(
nullptr, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, allocator, true);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();

ret = rcl_timer_init(
&timer, nullptr, this->context_ptr, RCL_MS_TO_NS(50), nullptr, allocator);
ret = rcl_timer_init2(
&timer, nullptr, this->context_ptr, RCL_MS_TO_NS(50), nullptr, allocator, true);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();

ret = rcl_timer_init(
&timer, &clock, nullptr, RCL_MS_TO_NS(50), nullptr, allocator);
ret = rcl_timer_init2(
&timer, &clock, nullptr, RCL_MS_TO_NS(50), nullptr, allocator, true);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();

ret = rcl_timer_init(
&timer, &clock, this->context_ptr, -1, nullptr, allocator);
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, -1, nullptr, allocator, true);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();

rcl_allocator_t invalid_allocator = rcutils_get_zero_initialized_allocator();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, invalid_allocator);
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, invalid_allocator, true);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
rcl_reset_error();
}
Expand All @@ -167,8 +167,8 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) {
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, 0, nullptr, allocator);
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, 0, nullptr, allocator, true);
EXPECT_EQ(RCL_RET_ERROR, ret);
rcl_reset_error();

Expand All @@ -180,8 +180,8 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) {
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
});

ret = rcl_timer_init(
&timer, &clock, this->context_ptr, 0, nullptr, allocator);
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, 0, nullptr, allocator, true);
ASSERT_EQ(RCL_RET_OK, ret);
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -233,12 +233,14 @@ TEST_F(TestTimerFixture, test_two_timers) {
rcl_timer_t timer = rcl_get_zero_initialized_timer();
rcl_timer_t timer2 = rcl_get_zero_initialized_timer();

ret = rcl_timer_init(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_timer_init(
&timer2, &clock, this->context_ptr, RCL_MS_TO_NS(1000), nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer2, &clock, this->context_ptr, RCL_MS_TO_NS(1000), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
Expand Down Expand Up @@ -301,12 +303,14 @@ TEST_F(TestTimerFixture, test_two_timers_ready_before_timeout) {
rcl_timer_t timer2 = rcl_get_zero_initialized_timer();

// Keep the first timer period low enough so that rcl_wait() doesn't timeout too early.
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(10), nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(10), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_timer_init(
&timer2, &clock, this->context_ptr, RCL_MS_TO_NS(1000), nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer2, &clock, this->context_ptr, RCL_MS_TO_NS(1000), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
Expand Down Expand Up @@ -367,8 +371,9 @@ TEST_F(TestTimerFixture, test_timer_not_ready) {

rcl_timer_t timer = rcl_get_zero_initialized_timer();

ret = rcl_timer_init(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(1000), nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(1000), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
Expand Down Expand Up @@ -415,8 +420,9 @@ TEST_F(TestTimerFixture, test_timer_overrun) {
});

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(200), nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_MS_TO_NS(200), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -470,8 +476,9 @@ TEST_F(TestTimerFixture, test_timer_with_zero_period) {
});

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, 0, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, 0, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand All @@ -492,6 +499,43 @@ TEST_F(TestTimerFixture, test_timer_with_zero_period) {
EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str;
}

TEST_F(TestTimerFixture, test_timer_init_state) {
rcl_ret_t ret;

rcl_clock_t clock;
rcl_allocator_t allocator = rcl_get_default_allocator();
ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

rcl_timer_t timer = rcl_get_zero_initialized_timer();

ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_S_TO_NS(1), nullptr, rcl_get_default_allocator(),
false);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

bool is_canceled = false;
ret = rcl_timer_is_canceled(&timer, &is_canceled);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
EXPECT_TRUE(is_canceled);

ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_S_TO_NS(1), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_ALREADY_INIT, ret) << rcl_get_error_string().str;

timer = rcl_get_zero_initialized_timer();

ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, RCL_S_TO_NS(1), nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_timer_is_canceled(&timer, &is_canceled);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
EXPECT_FALSE(is_canceled);
}

TEST_F(TestTimerFixture, test_canceled_timer) {
rcl_ret_t ret;

Expand All @@ -502,8 +546,9 @@ TEST_F(TestTimerFixture, test_canceled_timer) {

rcl_timer_t timer = rcl_get_zero_initialized_timer();

ret = rcl_timer_init(
&timer, &clock, this->context_ptr, 500, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, 500, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

ret = rcl_timer_cancel(&timer);
Expand Down Expand Up @@ -561,8 +606,9 @@ TEST_F(TestTimerFixture, test_rostime_time_until_next_call) {
ASSERT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(&clock)) << rcl_get_error_string().str;

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -600,8 +646,9 @@ TEST_F(TestTimerFixture, test_system_time_to_ros_time) {
});

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -643,8 +690,9 @@ TEST_F(TestTimerFixture, test_ros_time_to_system_time) {
ASSERT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(&clock)) << rcl_get_error_string().str;

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -689,8 +737,9 @@ TEST_F(TestTimerFixture, test_ros_time_backwards_jump) {
ASSERT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(&clock)) << rcl_get_error_string().str;

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, sec_5, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -733,8 +782,9 @@ TEST_F(TestTimerFixture, test_ros_time_wakes_wait) {
ASSERT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(&clock)) << rcl_get_error_string().str;

rcl_timer_t timer = rcl_get_zero_initialized_timer();
ret = rcl_timer_init(
&timer, &clock, this->context_ptr, sec_1, nullptr, rcl_get_default_allocator());
ret = rcl_timer_init2(
&timer, &clock, this->context_ptr, sec_1, nullptr, rcl_get_default_allocator(),
true);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
Expand Down Expand Up @@ -912,15 +962,15 @@ TEST_F(TestPreInitTimer, test_invalid_init_fini) {
rcl_timer_t timer_fail = rcl_get_zero_initialized_timer();

EXPECT_EQ(
RCL_RET_ALREADY_INIT, rcl_timer_init(
RCL_RET_ALREADY_INIT, rcl_timer_init2(
&timer, &clock, this->context_ptr, 500, nullptr,
rcl_get_default_allocator())) << rcl_get_error_string().str;
rcl_get_default_allocator(), true)) << rcl_get_error_string().str;
rcl_reset_error();

ASSERT_EQ(
RCL_RET_BAD_ALLOC, rcl_timer_init(
RCL_RET_BAD_ALLOC, rcl_timer_init2(
&timer_fail, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test,
bad_allocator)) << rcl_get_error_string().str;
bad_allocator, true)) << rcl_get_error_string().str;
rcl_reset_error();

EXPECT_EQ(RCL_RET_OK, rcl_timer_fini(nullptr)) << rcl_get_error_string().str;
Expand Down
Loading

0 comments on commit 43f8163

Please sign in to comment.