diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index 81253fb95b..26d7e69362 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -25,7 +25,11 @@ if(NOT CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 14) endif() if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual) + # About -Wno-sign-conversion: With Clang, -Wconversion implies -Wsign-conversion. There are a number of + # implicit sign conversions in rclcpp and gtest.cc, see https://ci.ros2.org/job/ci_osx/9265/. + # Hence disabling -Wsign-conversion for now until all those have eventually been fixed. + # (from https://github.com/ros2/rclcpp/pull/1188#issuecomment-650229140) + add_compile_options(-Wall -Wextra -Wconversion -Wno-sign-conversion -Wpedantic -Wnon-virtual-dtor -Woverloaded-virtual) endif() set(${PROJECT_NAME}_SRCS diff --git a/rclcpp/include/rclcpp/duration.hpp b/rclcpp/include/rclcpp/duration.hpp index 00aba81e78..49125ec722 100644 --- a/rclcpp/include/rclcpp/duration.hpp +++ b/rclcpp/include/rclcpp/duration.hpp @@ -72,7 +72,7 @@ class RCLCPP_PUBLIC Duration operator=(const Duration & rhs); Duration & - operator=(const builtin_interfaces::msg::Duration & Duration_msg); + operator=(const builtin_interfaces::msg::Duration & duration_msg); bool operator==(const rclcpp::Duration & rhs) const; diff --git a/rclcpp/include/rclcpp/time.hpp b/rclcpp/include/rclcpp/time.hpp index 6782393b4c..af41bef435 100644 --- a/rclcpp/include/rclcpp/time.hpp +++ b/rclcpp/include/rclcpp/time.hpp @@ -48,10 +48,10 @@ class Time /// Time constructor /** * \param nanoseconds since time epoch - * \param clock clock type + * \param clock_type clock type */ RCLCPP_PUBLIC - explicit Time(int64_t nanoseconds = 0, rcl_clock_type_t clock = RCL_SYSTEM_TIME); + explicit Time(int64_t nanoseconds = 0, rcl_clock_type_t clock_type = RCL_SYSTEM_TIME); /// Copy constructor RCLCPP_PUBLIC @@ -60,13 +60,13 @@ class Time /// Time constructor /** * \param time_msg builtin_interfaces time message to copy - * \param ros_time clock type + * \param clock_type clock type * \throws std::runtime_error if seconds are negative */ RCLCPP_PUBLIC Time( const builtin_interfaces::msg::Time & time_msg, - rcl_clock_type_t ros_time = RCL_ROS_TIME); + rcl_clock_type_t clock_type = RCL_ROS_TIME); /// Time constructor /** @@ -90,6 +90,12 @@ class Time Time & operator=(const Time & rhs); + /** + * Assign Time from a builtin_interfaces::msg::Time instance. + * The clock_type will be reset to RCL_ROS_TIME. + * Equivalent to *this = Time(time_msg, RCL_ROS_TIME). + * \throws std::runtime_error if seconds are negative + */ RCLCPP_PUBLIC Time & operator=(const builtin_interfaces::msg::Time & time_msg); diff --git a/rclcpp/src/rclcpp/duration.cpp b/rclcpp/src/rclcpp/duration.cpp index cbe904b679..47faa0d611 100644 --- a/rclcpp/src/rclcpp/duration.cpp +++ b/rclcpp/src/rclcpp/duration.cpp @@ -47,17 +47,14 @@ Duration::Duration(std::chrono::nanoseconds nanoseconds) rcl_duration_.nanoseconds = nanoseconds.count(); } -Duration::Duration(const Duration & rhs) -{ - rcl_duration_.nanoseconds = rhs.rcl_duration_.nanoseconds; -} +Duration::Duration(const Duration & rhs) = default; Duration::Duration( const builtin_interfaces::msg::Duration & duration_msg) { rcl_duration_.nanoseconds = - static_cast(RCL_S_TO_NS(duration_msg.sec)); - rcl_duration_.nanoseconds += duration_msg.nanosec; + RCL_S_TO_NS(static_cast(duration_msg.sec)); + rcl_duration_.nanoseconds += static_cast(duration_msg.nanosec); } Duration::Duration(const rcl_duration_t & duration) @@ -69,24 +66,25 @@ Duration::Duration(const rcl_duration_t & duration) Duration::operator builtin_interfaces::msg::Duration() const { builtin_interfaces::msg::Duration msg_duration; - msg_duration.sec = static_cast(RCL_NS_TO_S(rcl_duration_.nanoseconds)); - msg_duration.nanosec = - static_cast(rcl_duration_.nanoseconds % (1000 * 1000 * 1000)); + constexpr rcl_duration_value_t kDivisor = RCL_S_TO_NS(1); + const auto result = std::div(rcl_duration_.nanoseconds, kDivisor); + if (result.rem >= 0) { + msg_duration.sec = static_cast(result.quot); + msg_duration.nanosec = static_cast(result.rem); + } else { + msg_duration.sec = static_cast(result.quot - 1); + msg_duration.nanosec = static_cast(kDivisor + result.rem); + } return msg_duration; } Duration & -Duration::operator=(const Duration & rhs) -{ - rcl_duration_.nanoseconds = rhs.rcl_duration_.nanoseconds; - return *this; -} +Duration::operator=(const Duration & rhs) = default; Duration & Duration::operator=(const builtin_interfaces::msg::Duration & duration_msg) { - rcl_duration_.nanoseconds = RCL_S_TO_NS(static_cast(duration_msg.sec)); - rcl_duration_.nanoseconds += duration_msg.nanosec; + *this = Duration(duration_msg); return *this; } @@ -236,6 +234,10 @@ Duration::seconds() const rmw_time_t Duration::to_rmw_time() const { + if (rcl_duration_.nanoseconds < 0) { + throw std::runtime_error("rmw_time_t cannot be negative"); + } + // reuse conversion logic from msg creation builtin_interfaces::msg::Duration msg = *this; rmw_time_t result; diff --git a/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp b/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp index f5d0aa7e64..412fba46c5 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp @@ -159,7 +159,7 @@ __lockless_has_parameter( // see https://en.cppreference.com/w/cpp/types/numeric_limits/epsilon RCLCPP_LOCAL bool -__are_doubles_equal(double x, double y, size_t ulp = 100) +__are_doubles_equal(double x, double y, double ulp = 100.0) { return std::abs(x - y) <= std::numeric_limits::epsilon() * std::abs(x + y) * ulp; } diff --git a/rclcpp/src/rclcpp/node_options.cpp b/rclcpp/src/rclcpp/node_options.cpp index cac10e6685..43c401828f 100644 --- a/rclcpp/src/rclcpp/node_options.cpp +++ b/rclcpp/src/rclcpp/node_options.cpp @@ -338,7 +338,7 @@ NodeOptions::get_domain_id_from_env() const _dupenv_s(&ros_domain_id, &ros_domain_id_size, env_var); #endif if (ros_domain_id) { - uint32_t number = strtoul(ros_domain_id, NULL, 0); + uint32_t number = static_cast(strtoul(ros_domain_id, NULL, 0)); if (number == (std::numeric_limits::max)()) { #ifdef _WIN32 // free the ros_domain_id before throwing, if getenv was used on Windows diff --git a/rclcpp/src/rclcpp/time.cpp b/rclcpp/src/rclcpp/time.cpp index 63289161ee..e9d633e046 100644 --- a/rclcpp/src/rclcpp/time.cpp +++ b/rclcpp/src/rclcpp/time.cpp @@ -63,17 +63,13 @@ Time::Time(int64_t nanoseconds, rcl_clock_type_t clock_type) rcl_time_.nanoseconds = nanoseconds; } -Time::Time(const Time & rhs) -: rcl_time_(rhs.rcl_time_) -{ - rcl_time_.nanoseconds = rhs.rcl_time_.nanoseconds; -} +Time::Time(const Time & rhs) = default; Time::Time( const builtin_interfaces::msg::Time & time_msg, - rcl_clock_type_t ros_time) + rcl_clock_type_t clock_type) +: rcl_time_(init_time_point(clock_type)) { - rcl_time_ = init_time_point(ros_time); if (time_msg.sec < 0) { throw std::runtime_error("cannot store a negative time point in rclcpp::Time"); } @@ -95,31 +91,25 @@ Time::~Time() Time::operator builtin_interfaces::msg::Time() const { builtin_interfaces::msg::Time msg_time; - msg_time.sec = static_cast(RCL_NS_TO_S(rcl_time_.nanoseconds)); - msg_time.nanosec = static_cast(rcl_time_.nanoseconds % (1000 * 1000 * 1000)); + constexpr rcl_time_point_value_t kRemainder = RCL_S_TO_NS(1); + const auto result = std::div(rcl_time_.nanoseconds, kRemainder); + if (result.rem >= 0) { + msg_time.sec = static_cast(result.quot); + msg_time.nanosec = static_cast(result.rem); + } else { + msg_time.sec = static_cast(result.quot - 1); + msg_time.nanosec = static_cast(kRemainder + result.rem); + } return msg_time; } Time & -Time::operator=(const Time & rhs) -{ - rcl_time_ = rhs.rcl_time_; - return *this; -} +Time::operator=(const Time & rhs) = default; Time & Time::operator=(const builtin_interfaces::msg::Time & time_msg) { - if (time_msg.sec < 0) { - throw std::runtime_error("cannot store a negative time point in rclcpp::Time"); - } - - - rcl_clock_type_t ros_time = RCL_ROS_TIME; - rcl_time_ = init_time_point(ros_time); // TODO(tfoote) hard coded ROS here - - rcl_time_.nanoseconds = RCL_S_TO_NS(static_cast(time_msg.sec)); - rcl_time_.nanoseconds += time_msg.nanosec; + *this = Time(time_msg); return *this; } diff --git a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp index bfe2b25457..b6fd2c3fda 100644 --- a/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp +++ b/rclcpp/test/rclcpp/executors/test_multi_threaded_executor.cpp @@ -83,7 +83,7 @@ TEST_F(TestMultiThreadedExecutor, timer_over_take) { { std::lock_guard lock(last_mutex); - double diff = std::abs((now - last).nanoseconds()) / 1.0e9; + double diff = static_cast(std::abs((now - last).nanoseconds())) / 1.0e9; last = now; if (diff < PERIOD - TOLERANCE) { diff --git a/rclcpp/test/rclcpp/test_duration.cpp b/rclcpp/test/rclcpp/test_duration.cpp index 202358402a..e09295a5bb 100644 --- a/rclcpp/test/rclcpp/test_duration.cpp +++ b/rclcpp/test/rclcpp/test_duration.cpp @@ -32,11 +32,7 @@ class TestDuration : public ::testing::Test { }; -// TEST(TestDuration, conversions) { -// TODO(tfoote) Implement conversion methods -// } - -TEST(TestDuration, operators) { +TEST_F(TestDuration, operators) { rclcpp::Duration old(1, 0); rclcpp::Duration young(2, 0); @@ -68,7 +64,7 @@ TEST(TestDuration, operators) { EXPECT_TRUE(time == assignment_op_duration); } -TEST(TestDuration, chrono_overloads) { +TEST_F(TestDuration, chrono_overloads) { int64_t ns = 123456789l; auto chrono_ns = std::chrono::nanoseconds(ns); auto d1 = rclcpp::Duration(ns); @@ -87,7 +83,7 @@ TEST(TestDuration, chrono_overloads) { EXPECT_EQ(chrono_float_seconds, d5.to_chrono()); } -TEST(TestDuration, overflows) { +TEST_F(TestDuration, overflows) { rclcpp::Duration max(std::numeric_limits::max()); rclcpp::Duration min(std::numeric_limits::min()); @@ -108,7 +104,7 @@ TEST(TestDuration, overflows) { EXPECT_THROW(base_d_neg * 4, std::underflow_error); } -TEST(TestDuration, negative_duration) { +TEST_F(TestDuration, negative_duration) { rclcpp::Duration assignable_duration = rclcpp::Duration(0) - rclcpp::Duration(5, 0); { @@ -131,7 +127,7 @@ TEST(TestDuration, negative_duration) { } } -TEST(TestDuration, maximum_duration) { +TEST_F(TestDuration, maximum_duration) { rclcpp::Duration max_duration = rclcpp::Duration::max(); rclcpp::Duration max(std::numeric_limits::max(), 999999999); @@ -139,18 +135,105 @@ TEST(TestDuration, maximum_duration) { } static const int64_t HALF_SEC_IN_NS = 500 * 1000 * 1000; +static const int64_t ONE_SEC_IN_NS = 1000 * 1000 * 1000; static const int64_t ONE_AND_HALF_SEC_IN_NS = 3 * HALF_SEC_IN_NS; -TEST(TestDuration, from_seconds) { +TEST_F(TestDuration, from_seconds) { EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration::from_seconds(0.0)); EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration::from_seconds(0)); EXPECT_EQ(rclcpp::Duration(1, HALF_SEC_IN_NS), rclcpp::Duration::from_seconds(1.5)); EXPECT_EQ(rclcpp::Duration(-ONE_AND_HALF_SEC_IN_NS), rclcpp::Duration::from_seconds(-1.5)); } -TEST(TestDuration, std_chrono_constructors) { +TEST_F(TestDuration, std_chrono_constructors) { EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration(0.0s)); EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration(0s)); EXPECT_EQ(rclcpp::Duration(1, HALF_SEC_IN_NS), rclcpp::Duration(1.5s)); EXPECT_EQ(rclcpp::Duration(-1, 0), rclcpp::Duration(-1s)); } + +TEST_F(TestDuration, conversions) { + { + const rclcpp::Duration duration(HALF_SEC_IN_NS); + const auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, 0); + EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS); + EXPECT_EQ(rclcpp::Duration(duration_msg).nanoseconds(), HALF_SEC_IN_NS); + + const auto rmw_time = duration.to_rmw_time(); + EXPECT_EQ(rmw_time.sec, 0u); + EXPECT_EQ(rmw_time.nsec, static_cast(HALF_SEC_IN_NS)); + + const auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), HALF_SEC_IN_NS); + } + + { + const rclcpp::Duration duration(ONE_SEC_IN_NS); + const auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, 1); + EXPECT_EQ(duration_msg.nanosec, 0u); + EXPECT_EQ(rclcpp::Duration(duration_msg).nanoseconds(), ONE_SEC_IN_NS); + + const auto rmw_time = duration.to_rmw_time(); + EXPECT_EQ(rmw_time.sec, 1u); + EXPECT_EQ(rmw_time.nsec, 0u); + + const auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), ONE_SEC_IN_NS); + } + + { + const rclcpp::Duration duration(ONE_AND_HALF_SEC_IN_NS); + auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, 1); + EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS); + EXPECT_EQ(rclcpp::Duration(duration_msg).nanoseconds(), ONE_AND_HALF_SEC_IN_NS); + + auto rmw_time = duration.to_rmw_time(); + EXPECT_EQ(rmw_time.sec, 1u); + EXPECT_EQ(rmw_time.nsec, static_cast(HALF_SEC_IN_NS)); + + auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), ONE_AND_HALF_SEC_IN_NS); + } + + { + rclcpp::Duration duration(-HALF_SEC_IN_NS); + auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, -1); + EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS); + EXPECT_EQ(rclcpp::Duration(duration_msg).nanoseconds(), -HALF_SEC_IN_NS); + + EXPECT_THROW(duration.to_rmw_time(), std::runtime_error); + + auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), -HALF_SEC_IN_NS); + } + + { + rclcpp::Duration duration(-ONE_SEC_IN_NS); + auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, -1); + EXPECT_EQ(duration_msg.nanosec, 0u); + EXPECT_EQ(rclcpp::Duration(duration_msg).nanoseconds(), -ONE_SEC_IN_NS); + + EXPECT_THROW(duration.to_rmw_time(), std::runtime_error); + + auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), -ONE_SEC_IN_NS); + } + + { + rclcpp::Duration duration(-ONE_AND_HALF_SEC_IN_NS); + auto duration_msg = static_cast(duration); + EXPECT_EQ(duration_msg.sec, -2); + EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS); + EXPECT_EQ(rclcpp::Duration(duration_msg).nanoseconds(), -ONE_AND_HALF_SEC_IN_NS); + + EXPECT_THROW(duration.to_rmw_time(), std::runtime_error); + + auto chrono_duration = duration.to_chrono(); + EXPECT_EQ(chrono_duration.count(), -ONE_AND_HALF_SEC_IN_NS); + } +} diff --git a/rclcpp/test/rclcpp/test_parameter.cpp b/rclcpp/test/rclcpp/test_parameter.cpp index 876fbb4e93..f7244e4488 100644 --- a/rclcpp/test/rclcpp/test_parameter.cpp +++ b/rclcpp/test/rclcpp/test_parameter.cpp @@ -32,7 +32,7 @@ class TestParameter : public ::testing::Test } }; -TEST(TestParameter, not_set_variant) { +TEST_F(TestParameter, not_set_variant) { // Direct instantiation rclcpp::Parameter not_set_variant; EXPECT_EQ(rclcpp::PARAMETER_NOT_SET, not_set_variant.get_type()); @@ -58,7 +58,7 @@ TEST(TestParameter, not_set_variant) { rclcpp::Parameter::from_parameter_msg(not_set_param).get_type()); } -TEST(TestParameter, bool_variant) { +TEST_F(TestParameter, bool_variant) { // Direct instantiation rclcpp::Parameter bool_variant_true("bool_param", true); EXPECT_EQ("bool_param", bool_variant_true.get_name()); @@ -116,7 +116,7 @@ TEST(TestParameter, bool_variant) { bool_variant_false.get_value_message().type); } -TEST(TestParameter, integer_variant) { +TEST_F(TestParameter, integer_variant) { const int TEST_VALUE {42}; // Direct instantiation @@ -164,7 +164,7 @@ TEST(TestParameter, integer_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, long_integer_variant) { +TEST_F(TestParameter, long_integer_variant) { const int64_t TEST_VALUE {std::numeric_limits::max()}; // Direct instantiation @@ -212,7 +212,7 @@ TEST(TestParameter, long_integer_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, float_variant) { +TEST_F(TestParameter, float_variant) { const float TEST_VALUE {42.0f}; // Direct instantiation @@ -260,7 +260,7 @@ TEST(TestParameter, float_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, double_variant) { +TEST_F(TestParameter, double_variant) { const double TEST_VALUE {-42.1}; // Direct instantiation @@ -308,7 +308,7 @@ TEST(TestParameter, double_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, string_variant) { +TEST_F(TestParameter, string_variant) { const std::string TEST_VALUE {"ROS2"}; // Direct instantiation @@ -354,7 +354,7 @@ TEST(TestParameter, string_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, byte_array_variant) { +TEST_F(TestParameter, byte_array_variant) { const std::vector TEST_VALUE {0x52, 0x4f, 0x53, 0x32}; // Direct instantiation @@ -402,7 +402,7 @@ TEST(TestParameter, byte_array_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, bool_array_variant) { +TEST_F(TestParameter, bool_array_variant) { const std::vector TEST_VALUE {false, true, true, false, false, true}; // Direct instantiation @@ -450,7 +450,7 @@ TEST(TestParameter, bool_array_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, integer_array_variant) { +TEST_F(TestParameter, integer_array_variant) { const std::vector TEST_VALUE {42, -99, std::numeric_limits::max(), std::numeric_limits::lowest(), 0}; @@ -529,7 +529,7 @@ TEST(TestParameter, integer_array_variant) { rcl_interfaces::msg::ParameterType::PARAMETER_INTEGER_ARRAY); } -TEST(TestParameter, long_integer_array_variant) { +TEST_F(TestParameter, long_integer_array_variant) { const std::vector TEST_VALUE {42, -99, std::numeric_limits::max(), std::numeric_limits::lowest(), 0}; @@ -583,7 +583,7 @@ TEST(TestParameter, long_integer_array_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, float_array_variant) { +TEST_F(TestParameter, float_array_variant) { const std::vector TEST_VALUE {42.1f, -99.1f, std::numeric_limits::max(), std::numeric_limits::lowest(), 0.1f}; @@ -662,7 +662,7 @@ TEST(TestParameter, float_array_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, double_array_variant) { +TEST_F(TestParameter, double_array_variant) { const std::vector TEST_VALUE {42.1, -99.1, std::numeric_limits::max(), std::numeric_limits::lowest(), 0.1}; @@ -716,7 +716,7 @@ TEST(TestParameter, double_array_variant) { from_msg.get_value_message().type); } -TEST(TestParameter, string_array_variant) { +TEST_F(TestParameter, string_array_variant) { const std::vector TEST_VALUE {"R", "O", "S2"}; // Direct instantiation diff --git a/rclcpp/test/rclcpp/test_time.cpp b/rclcpp/test/rclcpp/test_time.cpp index 830c0319e7..ac7f8e0a80 100644 --- a/rclcpp/test/rclcpp/test_time.cpp +++ b/rclcpp/test/rclcpp/test_time.cpp @@ -45,7 +45,7 @@ class TestTime : public ::testing::Test } }; -TEST(TestTime, clock_type_access) { +TEST_F(TestTime, clock_type_access) { rclcpp::Clock ros_clock(RCL_ROS_TIME); EXPECT_EQ(RCL_ROS_TIME, ros_clock.get_clock_type()); @@ -57,7 +57,7 @@ TEST(TestTime, clock_type_access) { } // Check that the clock may go out of the scope before the jump callback without leading in UB. -TEST(TestTime, clock_jump_callback_destruction_order) { +TEST_F(TestTime, clock_jump_callback_destruction_order) { rclcpp::JumpHandler::SharedPtr handler; { rclcpp::Clock ros_clock(RCL_ROS_TIME); @@ -68,7 +68,7 @@ TEST(TestTime, clock_jump_callback_destruction_order) { } } -TEST(TestTime, time_sources) { +TEST_F(TestTime, time_sources) { using builtin_interfaces::msg::Time; rclcpp::Clock ros_clock(RCL_ROS_TIME); Time ros_now = ros_clock.now(); @@ -86,44 +86,124 @@ TEST(TestTime, time_sources) { EXPECT_NE(0u, steady_now.nanosec); } -TEST(TestTime, conversions) { +static const int64_t HALF_SEC_IN_NS = 500 * 1000 * 1000; +static const int64_t ONE_SEC_IN_NS = 1000 * 1000 * 1000; +static const int64_t ONE_AND_HALF_SEC_IN_NS = 3 * HALF_SEC_IN_NS; + +TEST_F(TestTime, conversions) { rclcpp::Clock system_clock(RCL_SYSTEM_TIME); - rclcpp::Time now = system_clock.now(); - builtin_interfaces::msg::Time now_msg = now; + { + rclcpp::Time now = system_clock.now(); + builtin_interfaces::msg::Time now_msg = now; + + rclcpp::Time now_again = now_msg; + EXPECT_EQ(now.nanoseconds(), now_again.nanoseconds()); + } - rclcpp::Time now_again = now_msg; - EXPECT_EQ(now.nanoseconds(), now_again.nanoseconds()); + { + rclcpp::Time positive_time = rclcpp::Time(12345, 67890u); + + builtin_interfaces::msg::Time msg = positive_time; + EXPECT_EQ(msg.sec, 12345); + EXPECT_EQ(msg.nanosec, 67890u); + + rclcpp::Time time = msg; + EXPECT_EQ(time.nanoseconds(), positive_time.nanoseconds()); + EXPECT_EQ( + RCL_S_TO_NS(static_cast(msg.sec)) + static_cast(msg.nanosec), + time.nanoseconds()); + EXPECT_EQ(static_cast(msg.sec), RCL_NS_TO_S(time.nanoseconds())); + } + + // throw on construction/assignment of negative times + { + builtin_interfaces::msg::Time negative_time_msg; + negative_time_msg.sec = -1; + negative_time_msg.nanosec = 1; + + EXPECT_ANY_THROW( + { + rclcpp::Time negative_time = negative_time_msg; + }); + + EXPECT_ANY_THROW(rclcpp::Time(-1, 1)); + + EXPECT_ANY_THROW( + { + rclcpp::Time assignment(1, 2); + assignment = negative_time_msg; + }); + } - builtin_interfaces::msg::Time msg; - msg.sec = 12345; - msg.nanosec = 67890; + { + const rclcpp::Time time(HALF_SEC_IN_NS); + const auto time_msg = static_cast(time); + EXPECT_EQ(time_msg.sec, 0); + EXPECT_EQ(time_msg.nanosec, HALF_SEC_IN_NS); + EXPECT_EQ(rclcpp::Time(time_msg).nanoseconds(), HALF_SEC_IN_NS); + } - rclcpp::Time time = msg; - EXPECT_EQ( - RCL_S_TO_NS(static_cast(msg.sec)) + static_cast(msg.nanosec), - time.nanoseconds()); - EXPECT_EQ(static_cast(msg.sec), RCL_NS_TO_S(time.nanoseconds())); + { + const rclcpp::Time time(ONE_SEC_IN_NS); + const auto time_msg = static_cast(time); + EXPECT_EQ(time_msg.sec, 1); + EXPECT_EQ(time_msg.nanosec, 0u); + EXPECT_EQ(rclcpp::Time(time_msg).nanoseconds(), ONE_SEC_IN_NS); + } - builtin_interfaces::msg::Time negative_time_msg; - negative_time_msg.sec = -1; - negative_time_msg.nanosec = 1; + { + const rclcpp::Time time(ONE_AND_HALF_SEC_IN_NS); + auto time_msg = static_cast(time); + EXPECT_EQ(time_msg.sec, 1); + EXPECT_EQ(time_msg.nanosec, HALF_SEC_IN_NS); + EXPECT_EQ(rclcpp::Time(time_msg).nanoseconds(), ONE_AND_HALF_SEC_IN_NS); + } - EXPECT_ANY_THROW( { - rclcpp::Time negative_time = negative_time_msg; - }); + // Can rclcpp::Time be negative or not? The following constructor works: + rclcpp::Time time(-HALF_SEC_IN_NS); + auto time_msg = static_cast(time); + EXPECT_EQ(time_msg.sec, -1); + EXPECT_EQ(time_msg.nanosec, HALF_SEC_IN_NS); + + // The opposite conversion throws... + EXPECT_ANY_THROW( + { + rclcpp::Time negative_time(time_msg); + }); + } - EXPECT_ANY_THROW(rclcpp::Time(-1, 1)); + { + // Can rclcpp::Time be negative or not? The following constructor works: + rclcpp::Time time(-ONE_SEC_IN_NS); + auto time_msg = static_cast(time); + EXPECT_EQ(time_msg.sec, -1); + EXPECT_EQ(time_msg.nanosec, 0u); + + // The opposite conversion throws... + EXPECT_ANY_THROW( + { + rclcpp::Time negative_time(time_msg); + }); + } - EXPECT_ANY_THROW( { - rclcpp::Time assignment(1, 2); - assignment = negative_time_msg; - }); + // Can rclcpp::Time be negative or not? The following constructor works: + rclcpp::Time time(-ONE_AND_HALF_SEC_IN_NS); + auto time_msg = static_cast(time); + EXPECT_EQ(time_msg.sec, -2); + EXPECT_EQ(time_msg.nanosec, HALF_SEC_IN_NS); + + // The opposite conversion throws... + EXPECT_ANY_THROW( + { + rclcpp::Time negative_time(time_msg); + }); + } } -TEST(TestTime, operators) { +TEST_F(TestTime, operators) { rclcpp::Time old(1, 0); rclcpp::Time young(2, 0); @@ -178,7 +258,7 @@ TEST(TestTime, operators) { } } -TEST(TestTime, overflow_detectors) { +TEST_F(TestTime, overflow_detectors) { ///////////////////////////////////////////////////////////////////////////// // Test logical_eq call first: EXPECT_TRUE(logical_eq(false, false)); @@ -198,8 +278,8 @@ TEST(TestTime, overflow_detectors) { // 256 * 256 = 64K total loops, should be pretty fast on everything for (big_type_t y = min_val; y <= max_val; ++y) { for (big_type_t x = min_val; x <= max_val; ++x) { - const big_type_t sum = x + y; - const big_type_t diff = x - y; + const big_type_t sum = static_cast(x + y); + const big_type_t diff = static_cast(x - y); const bool add_will_overflow = rclcpp::add_will_overflow(test_type_t(x), test_type_t(y)); @@ -235,7 +315,7 @@ TEST(TestTime, overflow_detectors) { EXPECT_TRUE(rclcpp::sub_will_underflow(INT64_MIN, 1)); } -TEST(TestTime, overflows) { +TEST_F(TestTime, overflows) { rclcpp::Time max_time(std::numeric_limits::max()); rclcpp::Time min_time(std::numeric_limits::min()); rclcpp::Duration one(1); @@ -267,7 +347,7 @@ TEST(TestTime, overflows) { EXPECT_NO_THROW(one_time - two_time); } -TEST(TestTime, seconds) { +TEST_F(TestTime, seconds) { EXPECT_DOUBLE_EQ(0.0, rclcpp::Time(0, 0).seconds()); EXPECT_DOUBLE_EQ(4.5, rclcpp::Time(4, 500000000).seconds()); EXPECT_DOUBLE_EQ(2.5, rclcpp::Time(0, 2500000000).seconds());