Skip to content

Commit 448dc1e

Browse files
clalancetteAlberto Soragna
authored and
Alberto Soragna
committed
Update rcutils_steady_time_now to return the same data as std::chrono (ros2#357)
* Update rcutils_steady_time_now to return the same data as std::chrono Based on an investigation several years ago, rcutils_steady_time_now has slightly different behavior than std::chrono::now . In particular, on Linux rcutils_steady_time_now was using CLOCK_MONOTONIC_RAW, while std::chrono is using CLOCK_MONOTONIC. On macOS, rcutils_steady_time_now was using SYSTEM_CLOCK, while std::chrono was using CLOCK_MONOTONIC_RAW. Fix both of these so they match what std::chrono does, and in the case of macOS, significantly simplify the code by switching to clock_gettime. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
1 parent ca42bc9 commit 448dc1e

File tree

2 files changed

+8
-34
lines changed

2 files changed

+8
-34
lines changed

src/time_unix.c

+8-20
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,10 @@ rcutils_system_time_now(rcutils_time_point_value_t * now)
5151
RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT);
5252
struct timespec timespec_now;
5353
#if defined(__MACH__)
54-
// On OS X use clock_get_time.
55-
clock_serv_t cclock;
56-
mach_timespec_t mts;
57-
host_get_clock_service(mach_host_self(), CALENDAR_CLOCK, &cclock);
58-
clock_get_time(cclock, &mts);
59-
mach_port_deallocate(mach_task_self(), cclock);
60-
timespec_now.tv_sec = mts.tv_sec;
61-
timespec_now.tv_nsec = mts.tv_nsec;
54+
// On macOS, use clock_gettime(CLOCK_REALTIME), which matches
55+
// the clang implementation
56+
// (https://github.com/llvm/llvm-project/blob/baebe12ad0d6f514cd33e418d6504075d3e79c0a/libcxx/src/chrono.cpp)
57+
clock_gettime(CLOCK_REALTIME, &timespec_now);
6258
#else // defined(__MACH__)
6359
// Otherwise use clock_gettime.
6460
clock_gettime(CLOCK_REALTIME, &timespec_now);
@@ -78,21 +74,13 @@ rcutils_steady_time_now(rcutils_time_point_value_t * now)
7874
// If clock_gettime is available or on OS X, use a timespec.
7975
struct timespec timespec_now;
8076
#if defined(__MACH__)
81-
// On OS X use clock_get_time.
82-
clock_serv_t cclock;
83-
mach_timespec_t mts;
84-
host_get_clock_service(mach_host_self(), SYSTEM_CLOCK, &cclock);
85-
clock_get_time(cclock, &mts);
86-
mach_port_deallocate(mach_task_self(), cclock);
87-
timespec_now.tv_sec = mts.tv_sec;
88-
timespec_now.tv_nsec = mts.tv_nsec;
77+
// On macOS, use clock_gettime(CLOCK_MONOTONIC_RAW), which matches
78+
// the clang implementation
79+
// (https://github.com/llvm/llvm-project/blob/baebe12ad0d6f514cd33e418d6504075d3e79c0a/libcxx/src/chrono.cpp)
80+
clock_gettime(CLOCK_MONOTONIC_RAW, &timespec_now);
8981
#else // defined(__MACH__)
9082
// Otherwise use clock_gettime.
91-
#if defined(CLOCK_MONOTONIC_RAW)
92-
clock_gettime(CLOCK_MONOTONIC_RAW, &timespec_now);
93-
#else // defined(CLOCK_MONOTONIC_RAW)
9483
clock_gettime(CLOCK_MONOTONIC, &timespec_now);
95-
#endif // defined(CLOCK_MONOTONIC_RAW)
9684
#endif // defined(__MACH__)
9785
if (__WOULD_BE_NEGATIVE(timespec_now.tv_sec, timespec_now.tv_nsec)) {
9886
RCUTILS_SET_ERROR_MSG("unexpected negative time");

test/test_time.cpp

-14
Original file line numberDiff line numberDiff line change
@@ -193,17 +193,7 @@ TEST_F(TestTimeFixture, test_rcutils_steady_time_now) {
193193

194194
#if !defined(_WIN32)
195195

196-
// For mocking purposes
197-
#if defined(__MACH__)
198-
#include <mach/clock.h>
199-
#include <mach/mach.h>
200-
#define clock_gettime clock_get_time
201-
#endif
202-
203-
// Tests rcutils_system_time_now() and rcutils_steady_time_now() functions
204-
// when system clocks misbehave.
205196
TEST_F(TestTimeFixture, test_rcutils_with_bad_system_clocks) {
206-
#if !defined (__MACH__) // as tv_sec is an unsigned integer there
207197
{
208198
auto mock = mocking_utils::patch(
209199
"lib:rcutils", clock_gettime,
@@ -222,7 +212,6 @@ TEST_F(TestTimeFixture, test_rcutils_with_bad_system_clocks) {
222212
EXPECT_EQ(RCUTILS_RET_ERROR, ret);
223213
rcutils_reset_error();
224214
}
225-
#endif
226215
{
227216
auto mock = mocking_utils::patch(
228217
"lib:rcutils", clock_gettime,
@@ -243,9 +232,6 @@ TEST_F(TestTimeFixture, test_rcutils_with_bad_system_clocks) {
243232
}
244233
}
245234

246-
#if defined(__MACH__)
247-
#undef clock_gettime
248-
#endif
249235
#endif // !defined(_WIN32)
250236

251237
// Tests the rcutils_time_point_value_as_nanoseconds_string() function.

0 commit comments

Comments
 (0)