From 8e2e64e82a33389bfe57ea6781f39acade7a5ff3 Mon Sep 17 00:00:00 2001 From: Karsten Knese Date: Thu, 24 Aug 2017 15:21:01 -0700 Subject: [PATCH] freeing Time members in destructor, adding copy constructor / assignment operator (#362) * copy constructor for fixing windows debug * remove debug prints * style * correctly free resources in destructor * correct copy and assignment operators * explicit call to copy constructor --- rclcpp/include/rclcpp/time.hpp | 7 +++++++ rclcpp/src/rclcpp/time.cpp | 18 ++++++++++++++++++ rclcpp/test/test_time.cpp | 14 ++++++++++++-- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/rclcpp/include/rclcpp/time.hpp b/rclcpp/include/rclcpp/time.hpp index 7e9b99a..5941e64 100644 --- a/rclcpp/include/rclcpp/time.hpp +++ b/rclcpp/include/rclcpp/time.hpp @@ -38,6 +38,9 @@ public: RCLCPP_PUBLIC explicit Time(uint64_t nanoseconds, rcl_time_source_type_t clock = RCL_SYSTEM_TIME); + RCLCPP_PUBLIC + Time(const Time & rhs); + RCLCPP_PUBLIC Time(const builtin_interfaces::msg::Time & time_msg); // NOLINT @@ -47,6 +50,10 @@ public: RCLCPP_PUBLIC operator builtin_interfaces::msg::Time() const; + RCLCPP_PUBLIC + void + operator=(const Time & rhs); + RCLCPP_PUBLIC void operator=(const builtin_interfaces::msg::Time & time_msg); diff --git a/rclcpp/src/rclcpp/time.cpp b/rclcpp/src/rclcpp/time.cpp index 102052d..ad44758 100644 --- a/rclcpp/src/rclcpp/time.cpp +++ b/rclcpp/src/rclcpp/time.cpp @@ -102,6 +102,13 @@ Time::Time(uint64_t nanoseconds, rcl_time_source_type_t clock) rcl_time_.nanoseconds = nanoseconds; } +Time::Time(const Time & rhs) +: rcl_time_source_(init_time_source(rhs.rcl_time_source_.type)), + rcl_time_(init_time_point(rcl_time_source_)) +{ + rcl_time_.nanoseconds = rhs.rcl_time_.nanoseconds; +} + Time::Time(const builtin_interfaces::msg::Time & time_msg) // NOLINT : rcl_time_source_(init_time_source(RCL_SYSTEM_TIME)), rcl_time_(init_time_point(rcl_time_source_)) @@ -116,6 +123,9 @@ Time::Time(const builtin_interfaces::msg::Time & time_msg) // NOLINT Time::~Time() { + if (rcl_time_source_fini(&rcl_time_source_) != RCL_RET_OK) { + RCUTILS_LOG_FATAL("failed to reclaim rcl_time_source_t in destructor of rclcpp::Time") + } if (rcl_time_point_fini(&rcl_time_) != RCL_RET_OK) { RCUTILS_LOG_FATAL("failed to reclaim rcl_time_point_t in destructor of rclcpp::Time") } @@ -129,6 +139,14 @@ Time::operator builtin_interfaces::msg::Time() const return msg_time; } +void +Time::operator=(const Time & rhs) +{ + rcl_time_source_ = init_time_source(rhs.rcl_time_source_.type); + rcl_time_ = init_time_point(rcl_time_source_); + rcl_time_.nanoseconds = rhs.rcl_time_.nanoseconds; +} + void Time::operator=(const builtin_interfaces::msg::Time & time_msg) { diff --git a/rclcpp/test/test_time.cpp b/rclcpp/test/test_time.cpp index fd447ea..9f47f12 100644 --- a/rclcpp/test/test_time.cpp +++ b/rclcpp/test/test_time.cpp @@ -102,8 +102,8 @@ TEST(TestTime, operators) { EXPECT_EQ(sub.nanoseconds(), young.nanoseconds() - old.nanoseconds()); EXPECT_EQ(sub, young - old); - rclcpp::Time system_time(1, 0, RCL_SYSTEM_TIME); - rclcpp::Time steady_time(2, 0, RCL_STEADY_TIME); + rclcpp::Time system_time(0, 0, RCL_SYSTEM_TIME); + rclcpp::Time steady_time(0, 0, RCL_STEADY_TIME); EXPECT_ANY_THROW((void)(system_time == steady_time)); EXPECT_ANY_THROW((void)(system_time <= steady_time)); @@ -123,6 +123,16 @@ TEST(TestTime, operators) { EXPECT_ANY_THROW((void)(now > later)); EXPECT_ANY_THROW((void)(now + later)); EXPECT_ANY_THROW((void)(now - later)); + + for (auto time_source : {RCL_ROS_TIME, RCL_SYSTEM_TIME, RCL_STEADY_TIME}) { + rclcpp::Time time = rclcpp::Time(0, 0, time_source); + rclcpp::Time copy_constructor_time(time); + rclcpp::Time assignment_op_time = rclcpp::Time(1, 0, time_source); + assignment_op_time = time; + + EXPECT_TRUE(time == copy_constructor_time); + EXPECT_TRUE(time == assignment_op_time); + } } TEST(TestTime, overflows) {