diff --git a/rclcpp/src/rclcpp/timer.cpp b/rclcpp/src/rclcpp/timer.cpp index 9ece178..50df28b 100644 --- a/rclcpp/src/rclcpp/timer.cpp +++ b/rclcpp/src/rclcpp/timer.cpp @@ -61,15 +61,11 @@ TimerBase::TimerBase( rcl_clock_t * clock_handle = clock_->get_clock_handle(); { std::lock_guard clock_guard(clock_->get_clock_mutex()); - if ( - rcl_timer_init( - timer_handle_.get(), clock_handle, rcl_context.get(), period.count(), nullptr, - rcl_get_default_allocator()) != RCL_RET_OK) - { - RCUTILS_LOG_ERROR_NAMED( - "rclcpp", - "Couldn't initialize rcl timer handle: %s\n", rcl_get_error_string().str); - rcl_reset_error(); + rcl_ret_t ret = rcl_timer_init( + timer_handle_.get(), clock_handle, rcl_context.get(), period.count(), nullptr, + rcl_get_default_allocator()); + if (ret != RCL_RET_OK) { + rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't initialize rcl timer handle"); } } } diff --git a/rclcpp/test/rclcpp/test_timer.cpp b/rclcpp/test/rclcpp/test_timer.cpp index 2d9022e..7f85a1c 100644 --- a/rclcpp/test/rclcpp/test_timer.cpp +++ b/rclcpp/test/rclcpp/test_timer.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include "rcl/timer.h" @@ -151,3 +152,42 @@ TEST_F(TestTimer, test_run_cancel_timer) EXPECT_TRUE(has_timer_run.load()); EXPECT_TRUE(timer->is_canceled()); } + +TEST_F(TestTimer, test_bad_arguments) { + auto node_base = rclcpp::node_interfaces::get_node_base_interface(test_node); + auto context = node_base->get_context(); + + auto steady_clock = std::make_shared(RCL_STEADY_TIME); + + // Negative period + EXPECT_THROW( + rclcpp::GenericTimer(steady_clock, -1ms, []() {}, context), + rclcpp::exceptions::RCLInvalidArgument); + + // Very negative period + constexpr auto nanoseconds_min = std::chrono::nanoseconds::min(); + EXPECT_THROW( + rclcpp::GenericTimer( + steady_clock, nanoseconds_min, []() {}, context), + rclcpp::exceptions::RCLInvalidArgument); + + // nanoseconds max, should be ok + constexpr auto nanoseconds_max = std::chrono::nanoseconds::max(); + EXPECT_NO_THROW( + rclcpp::GenericTimer( + steady_clock, nanoseconds_max, []() {}, context)); + + // 0 duration period, should be ok + EXPECT_NO_THROW( + rclcpp::GenericTimer(steady_clock, 0ms, []() {}, context)); + + // context is null, which resorts to default + EXPECT_NO_THROW( + rclcpp::GenericTimer(steady_clock, 1ms, []() {}, nullptr)); + + // Clock is unitialized + auto unitialized_clock = std::make_shared(RCL_CLOCK_UNINITIALIZED); + EXPECT_THROW( + rclcpp::GenericTimer(unitialized_clock, 1us, []() {}, context), + rclcpp::exceptions::RCLError); +}