Skip to content

Commit 7940311

Browse files
ivanpaunoivanpauno
authored
rclcpp::Duration constructors might be confusing to users migrating from ROS 1 (#1432)
* Deprecate Duration(rcl_duration_value_t) in favor of static Duration::from_nanoseconds(rcl_duration_value_t) Signed-off-by: Ivan Santiago Paunovic <[email protected]>
1 parent 3ae5170 commit 7940311

File tree

5 files changed

+67
-41
lines changed

5 files changed

+67
-41
lines changed

‎rclcpp/include/rclcpp/duration.hpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@ class RCLCPP_PUBLIC Duration
3838
*/
3939
Duration(int32_t seconds, uint32_t nanoseconds);
4040

41-
// This constructor matches any numeric value - ints or floats.
41+
/// Construct duration from the specified nanoseconds.
42+
[[deprecated(
43+
"Use Duration::from_nanoseconds instead or std::chrono_literals. For example:"
44+
"rclcpp::Duration::from_nanoseconds(int64_variable);"
45+
"rclcpp::Duration(0ns);")]]
4246
explicit Duration(rcl_duration_value_t nanoseconds);
4347

44-
// This constructor matches std::chrono::nanoseconds.
48+
/// Construct duration from the specified std::chrono::nanoseconds.
4549
explicit Duration(std::chrono::nanoseconds nanoseconds);
4650

4751
// This constructor matches any std::chrono value other than nanoseconds
@@ -129,6 +133,10 @@ class RCLCPP_PUBLIC Duration
129133
static Duration
130134
from_seconds(double seconds);
131135

136+
/// Create a duration object from an integer number representing nanoseconds
137+
static Duration
138+
from_nanoseconds(rcl_duration_value_t nanoseconds);
139+
132140
/// Convert Duration into a std::chrono::Duration.
133141
template<class DurationT>
134142
DurationT
@@ -143,6 +151,8 @@ class RCLCPP_PUBLIC Duration
143151

144152
private:
145153
rcl_duration_t rcl_duration_;
154+
155+
Duration() = default;
146156
};
147157

148158
} // namespace rclcpp

‎rclcpp/src/rclcpp/duration.cpp‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ Duration::Duration(int32_t seconds, uint32_t nanoseconds)
3737
rcl_duration_.nanoseconds += nanoseconds;
3838
}
3939

40-
Duration::Duration(int64_t nanoseconds)
40+
Duration::Duration(rcl_duration_value_t nanoseconds)
4141
{
4242
rcl_duration_.nanoseconds = nanoseconds;
4343
}
@@ -148,7 +148,7 @@ Duration::operator+(const rclcpp::Duration & rhs) const
148148
this->rcl_duration_.nanoseconds,
149149
rhs.rcl_duration_.nanoseconds,
150150
std::numeric_limits<rcl_duration_value_t>::max());
151-
return Duration(
151+
return Duration::from_nanoseconds(
152152
rcl_duration_.nanoseconds + rhs.rcl_duration_.nanoseconds);
153153
}
154154

@@ -177,7 +177,7 @@ Duration::operator-(const rclcpp::Duration & rhs) const
177177
rhs.rcl_duration_.nanoseconds,
178178
std::numeric_limits<rcl_duration_value_t>::max());
179179

180-
return Duration(
180+
return Duration::from_nanoseconds(
181181
rcl_duration_.nanoseconds - rhs.rcl_duration_.nanoseconds);
182182
}
183183

@@ -208,7 +208,7 @@ Duration::operator*(double scale) const
208208
scale,
209209
std::numeric_limits<rcl_duration_value_t>::max());
210210
long double scale_ld = static_cast<long double>(scale);
211-
return Duration(
211+
return Duration::from_nanoseconds(
212212
static_cast<rcl_duration_value_t>(
213213
static_cast<long double>(rcl_duration_.nanoseconds) * scale_ld));
214214
}
@@ -249,7 +249,17 @@ Duration::to_rmw_time() const
249249
Duration
250250
Duration::from_seconds(double seconds)
251251
{
252-
return Duration(static_cast<int64_t>(RCL_S_TO_NS(seconds)));
252+
Duration ret;
253+
ret.rcl_duration_.nanoseconds = static_cast<int64_t>(RCL_S_TO_NS(seconds));
254+
return ret;
255+
}
256+
257+
Duration
258+
Duration::from_nanoseconds(rcl_duration_value_t nanoseconds)
259+
{
260+
Duration ret;
261+
ret.rcl_duration_.nanoseconds = nanoseconds;
262+
return ret;
253263
}
254264

255265
} // namespace rclcpp

‎rclcpp/src/rclcpp/time.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ Time::operator-(const rclcpp::Time & rhs) const
199199
throw std::underflow_error("time subtraction leads to int64_t underflow");
200200
}
201201

202-
return Duration(rcl_time_.nanoseconds - rhs.rcl_time_.nanoseconds);
202+
return Duration::from_nanoseconds(rcl_time_.nanoseconds - rhs.rcl_time_.nanoseconds);
203203
}
204204

205205
Time

‎rclcpp/test/rclcpp/test_duration.cpp‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ TEST_F(TestDuration, operators) {
6868
TEST_F(TestDuration, chrono_overloads) {
6969
int64_t ns = 123456789l;
7070
auto chrono_ns = std::chrono::nanoseconds(ns);
71-
auto d1 = rclcpp::Duration(ns);
71+
auto d1 = rclcpp::Duration::from_nanoseconds(ns);
7272
auto d2 = rclcpp::Duration(chrono_ns);
7373
auto d3 = rclcpp::Duration(123456789ns);
7474
EXPECT_EQ(d1, d2);
@@ -85,11 +85,11 @@ TEST_F(TestDuration, chrono_overloads) {
8585
}
8686

8787
TEST_F(TestDuration, overflows) {
88-
rclcpp::Duration max(std::numeric_limits<rcl_duration_value_t>::max());
89-
rclcpp::Duration min(std::numeric_limits<rcl_duration_value_t>::min());
88+
auto max = rclcpp::Duration::from_nanoseconds(std::numeric_limits<rcl_duration_value_t>::max());
89+
auto min = rclcpp::Duration::from_nanoseconds(std::numeric_limits<rcl_duration_value_t>::min());
9090

91-
rclcpp::Duration one(1);
92-
rclcpp::Duration negative_one(-1);
91+
rclcpp::Duration one(1ns);
92+
rclcpp::Duration negative_one(-1ns);
9393

9494
EXPECT_THROW(max + one, std::overflow_error);
9595
EXPECT_THROW(min - one, std::underflow_error);
@@ -106,7 +106,7 @@ TEST_F(TestDuration, overflows) {
106106
}
107107

108108
TEST_F(TestDuration, negative_duration) {
109-
rclcpp::Duration assignable_duration = rclcpp::Duration(0) - rclcpp::Duration(5, 0);
109+
rclcpp::Duration assignable_duration = rclcpp::Duration(0ns) - rclcpp::Duration(5, 0);
110110

111111
{
112112
// avoid windows converting a literal number less than -INT_MAX to unsigned int C4146
@@ -140,22 +140,24 @@ static const int64_t ONE_SEC_IN_NS = 1000 * 1000 * 1000;
140140
static const int64_t ONE_AND_HALF_SEC_IN_NS = 3 * HALF_SEC_IN_NS;
141141

142142
TEST_F(TestDuration, from_seconds) {
143-
EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration::from_seconds(0.0));
144-
EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration::from_seconds(0));
143+
EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration::from_seconds(0.0));
144+
EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration::from_seconds(0));
145145
EXPECT_EQ(rclcpp::Duration(1, HALF_SEC_IN_NS), rclcpp::Duration::from_seconds(1.5));
146-
EXPECT_EQ(rclcpp::Duration(-ONE_AND_HALF_SEC_IN_NS), rclcpp::Duration::from_seconds(-1.5));
146+
EXPECT_EQ(
147+
rclcpp::Duration::from_nanoseconds(-ONE_AND_HALF_SEC_IN_NS),
148+
rclcpp::Duration::from_seconds(-1.5));
147149
}
148150

149151
TEST_F(TestDuration, std_chrono_constructors) {
150-
EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration(0.0s));
151-
EXPECT_EQ(rclcpp::Duration(0), rclcpp::Duration(0s));
152+
EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration(0.0s));
153+
EXPECT_EQ(rclcpp::Duration(0ns), rclcpp::Duration(0s));
152154
EXPECT_EQ(rclcpp::Duration(1, HALF_SEC_IN_NS), rclcpp::Duration(1.5s));
153155
EXPECT_EQ(rclcpp::Duration(-1, 0), rclcpp::Duration(-1s));
154156
}
155157

156158
TEST_F(TestDuration, conversions) {
157159
{
158-
const rclcpp::Duration duration(HALF_SEC_IN_NS);
160+
auto duration = rclcpp::Duration::from_nanoseconds(HALF_SEC_IN_NS);
159161
const auto duration_msg = static_cast<builtin_interfaces::msg::Duration>(duration);
160162
EXPECT_EQ(duration_msg.sec, 0);
161163
EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS);
@@ -170,7 +172,7 @@ TEST_F(TestDuration, conversions) {
170172
}
171173

172174
{
173-
const rclcpp::Duration duration(ONE_SEC_IN_NS);
175+
auto duration = rclcpp::Duration::from_nanoseconds(ONE_SEC_IN_NS);
174176
const auto duration_msg = static_cast<builtin_interfaces::msg::Duration>(duration);
175177
EXPECT_EQ(duration_msg.sec, 1);
176178
EXPECT_EQ(duration_msg.nanosec, 0u);
@@ -185,7 +187,7 @@ TEST_F(TestDuration, conversions) {
185187
}
186188

187189
{
188-
const rclcpp::Duration duration(ONE_AND_HALF_SEC_IN_NS);
190+
auto duration = rclcpp::Duration::from_nanoseconds(ONE_AND_HALF_SEC_IN_NS);
189191
auto duration_msg = static_cast<builtin_interfaces::msg::Duration>(duration);
190192
EXPECT_EQ(duration_msg.sec, 1);
191193
EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS);
@@ -200,7 +202,7 @@ TEST_F(TestDuration, conversions) {
200202
}
201203

202204
{
203-
rclcpp::Duration duration(-HALF_SEC_IN_NS);
205+
auto duration = rclcpp::Duration::from_nanoseconds(-HALF_SEC_IN_NS);
204206
auto duration_msg = static_cast<builtin_interfaces::msg::Duration>(duration);
205207
EXPECT_EQ(duration_msg.sec, -1);
206208
EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS);
@@ -213,7 +215,7 @@ TEST_F(TestDuration, conversions) {
213215
}
214216

215217
{
216-
rclcpp::Duration duration(-ONE_SEC_IN_NS);
218+
auto duration = rclcpp::Duration::from_nanoseconds(-ONE_SEC_IN_NS);
217219
auto duration_msg = static_cast<builtin_interfaces::msg::Duration>(duration);
218220
EXPECT_EQ(duration_msg.sec, -1);
219221
EXPECT_EQ(duration_msg.nanosec, 0u);
@@ -226,7 +228,7 @@ TEST_F(TestDuration, conversions) {
226228
}
227229

228230
{
229-
rclcpp::Duration duration(-ONE_AND_HALF_SEC_IN_NS);
231+
auto duration = rclcpp::Duration::from_nanoseconds(-ONE_AND_HALF_SEC_IN_NS);
230232
auto duration_msg = static_cast<builtin_interfaces::msg::Duration>(duration);
231233
EXPECT_EQ(duration_msg.sec, -2);
232234
EXPECT_EQ(duration_msg.nanosec, HALF_SEC_IN_NS);
@@ -253,9 +255,10 @@ TEST_F(TestDuration, test_some_constructors) {
253255
}
254256

255257
TEST_F(TestDuration, test_some_exceptions) {
256-
rclcpp::Duration test_duration(0u);
258+
rclcpp::Duration test_duration(0ns);
257259
RCLCPP_EXPECT_THROW_EQ(
258-
test_duration = rclcpp::Duration(INT64_MAX) - rclcpp::Duration(-1),
260+
test_duration =
261+
rclcpp::Duration::from_nanoseconds(INT64_MAX) - rclcpp::Duration(-1ns),
259262
std::overflow_error("duration subtraction leads to int64_t overflow"));
260263
RCLCPP_EXPECT_THROW_EQ(
261264
test_duration = test_duration * (std::numeric_limits<double>::infinity()),

‎rclcpp/test/rclcpp/test_time.cpp‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <gtest/gtest.h>
1616

1717
#include <algorithm>
18+
#include <chrono>
1819
#include <limits>
1920
#include <string>
2021

@@ -30,6 +31,8 @@
3031
namespace
3132
{
3233

34+
using namespace std::chrono_literals;
35+
3336
bool logical_eq(const bool a, const bool b)
3437
{
3538
return (a && b) || ((!a) && !(b));
@@ -221,7 +224,7 @@ TEST_F(TestTime, operators) {
221224
EXPECT_EQ(sub, young - old);
222225

223226
rclcpp::Time young_changed(young);
224-
young_changed -= rclcpp::Duration(old.nanoseconds());
227+
young_changed -= rclcpp::Duration::from_nanoseconds(old.nanoseconds());
225228
EXPECT_EQ(sub.nanoseconds(), young_changed.nanoseconds());
226229

227230
rclcpp::Time system_time(0, 0, RCL_SYSTEM_TIME);
@@ -320,8 +323,8 @@ TEST_F(TestTime, overflow_detectors) {
320323
TEST_F(TestTime, overflows) {
321324
rclcpp::Time max_time(std::numeric_limits<rcl_time_point_value_t>::max());
322325
rclcpp::Time min_time(std::numeric_limits<rcl_time_point_value_t>::min());
323-
rclcpp::Duration one(1);
324-
rclcpp::Duration two(2);
326+
rclcpp::Duration one(1ns);
327+
rclcpp::Duration two(2ns);
325328

326329
// Cross min/max
327330
EXPECT_THROW(max_time + one, std::overflow_error);
@@ -394,7 +397,7 @@ TEST_F(TestTime, test_assignment_operator_from_builtin_msg_time) {
394397
}
395398

396399
TEST_F(TestTime, test_sum_operator) {
397-
const rclcpp::Duration one(1);
400+
const rclcpp::Duration one(1ns);
398401
const rclcpp::Time test_time(0u);
399402
EXPECT_EQ(0u, test_time.nanoseconds());
400403

@@ -406,41 +409,41 @@ TEST_F(TestTime, test_overflow_underflow_throws) {
406409
rclcpp::Time test_time(0u);
407410

408411
RCLCPP_EXPECT_THROW_EQ(
409-
test_time = rclcpp::Time(INT64_MAX) + rclcpp::Duration(1),
412+
test_time = rclcpp::Time(INT64_MAX) + rclcpp::Duration(1ns),
410413
std::overflow_error("addition leads to int64_t overflow"));
411414
RCLCPP_EXPECT_THROW_EQ(
412-
test_time = rclcpp::Time(INT64_MIN) + rclcpp::Duration(-1),
415+
test_time = rclcpp::Time(INT64_MIN) + rclcpp::Duration(-1ns),
413416
std::underflow_error("addition leads to int64_t underflow"));
414417

415418
RCLCPP_EXPECT_THROW_EQ(
416-
test_time = rclcpp::Time(INT64_MAX) - rclcpp::Duration(-1),
419+
test_time = rclcpp::Time(INT64_MAX) - rclcpp::Duration(-1ns),
417420
std::overflow_error("time subtraction leads to int64_t overflow"));
418421
RCLCPP_EXPECT_THROW_EQ(
419-
test_time = rclcpp::Time(INT64_MIN) - rclcpp::Duration(1),
422+
test_time = rclcpp::Time(INT64_MIN) - rclcpp::Duration(1ns),
420423
std::underflow_error("time subtraction leads to int64_t underflow"));
421424

422425
test_time = rclcpp::Time(INT64_MAX);
423426
RCLCPP_EXPECT_THROW_EQ(
424-
test_time += rclcpp::Duration(1),
427+
test_time += rclcpp::Duration(1ns),
425428
std::overflow_error("addition leads to int64_t overflow"));
426429
test_time = rclcpp::Time(INT64_MIN);
427430
RCLCPP_EXPECT_THROW_EQ(
428-
test_time += rclcpp::Duration(-1),
431+
test_time += rclcpp::Duration(-1ns),
429432
std::underflow_error("addition leads to int64_t underflow"));
430433

431434
test_time = rclcpp::Time(INT64_MAX);
432435
RCLCPP_EXPECT_THROW_EQ(
433-
test_time -= rclcpp::Duration(-1),
436+
test_time -= rclcpp::Duration(-1ns),
434437
std::overflow_error("time subtraction leads to int64_t overflow"));
435438
test_time = rclcpp::Time(INT64_MIN);
436439
RCLCPP_EXPECT_THROW_EQ(
437-
test_time -= rclcpp::Duration(1),
440+
test_time -= rclcpp::Duration(1ns),
438441
std::underflow_error("time subtraction leads to int64_t underflow"));
439442

440443
RCLCPP_EXPECT_THROW_EQ(
441-
test_time = rclcpp::Duration(INT64_MAX) + rclcpp::Time(1),
444+
test_time = rclcpp::Duration::from_nanoseconds(INT64_MAX) + rclcpp::Time(1),
442445
std::overflow_error("addition leads to int64_t overflow"));
443446
RCLCPP_EXPECT_THROW_EQ(
444-
test_time = rclcpp::Duration(INT64_MIN) + rclcpp::Time(-1),
447+
test_time = rclcpp::Duration::from_nanoseconds(INT64_MIN) + rclcpp::Time(-1),
445448
std::underflow_error("addition leads to int64_t underflow"));
446449
}

0 commit comments

Comments
 (0)