add MonotonicTime for use with timeouts #55
Conversation
|
@wjwwood @mikaelarguedas What do you think about adding a new time class like This would already make it much easier to compute time deltas that should not be affected by NTP, PTP, etc adjusting the system time. |
| #endif | ||
| #else | ||
| ros_walltime(sec, nsec); | ||
| #endif |
There was a problem hiding this comment.
This would be a great place to use std::chrono::steady_clock or std::chrono::high_resolution_clock when it returns true for is_steady, since they're portable.
However, I'm not sure that would be acceptable within roscpp_core since it's a C++11 only feature.
| @@ -256,8 +256,41 @@ namespace ros | |||
| static bool isSystemTime() { return true; } | |||
| }; | |||
|
|
|||
| /** | |||
| * \brief Time representation. Always monotonic-clock time. | |||
There was a problem hiding this comment.
It should be noted here that it is unaffected by ROS time, just like "wall" time is not.
| struct timeval timeofday; | ||
| gettimeofday(&timeofday,NULL); | ||
| sec = timeofday.tv_sec; | ||
| nsec = timeofday.tv_usec * 1000; |
There was a problem hiding this comment.
It is possible to avoid this on macOS (also other BSD systems which don't have clock_gettime), but I'm hesitant to ask for it since the existing wall time system doesn't use it.
I've implemented something that is monotonic and works on Linux, and macOS for ROS 2:
There's also something for Windows:
You're welcome to steal those implementations if you like.
There was a problem hiding this comment.
Thanks...
On a sidenote:
I'm not convinced that it is a good idea to use the CLOCK_MONOTONIC_RAW (even if its available) for steady time. IMHO CLOCK_MONOTONIC is always what we want here, namely it doesn't jump, but the clock drift is adjusted...
There was a problem hiding this comment.
Ok, it's been too long since I've looked at it to respond intelligently 😄, but I'll review the use of CLOCK_MONOTONIC_RAW. Thanks!
| sec = start.tv_sec; | ||
| nsec = start.tv_nsec; | ||
| #else | ||
| // what to do if clock_gettime is not available??? |
There was a problem hiding this comment.
I wouldn't merge this with this code path in here, since I'm pretty sure that it isn't monotonic.
There was a problem hiding this comment.
Correct. I would actually just leave the case when clock_gettime is not available out completely, as I don't know of any way to get a monotonic clock otherwise and this should not be the case on any actually used Linux versions anymore...
|
closing in favor of #57 |
Add a new MonotonicTime which uses CLOCK_MONOTONIC to prevent issues with time jumps.
If
clock_gettimeis not available it will fall back to normal wall time... but that shouldn't be the case on many Linux systems anymore.Didn't look at the Windows implementation so far, also just returns wall time there.
See ros/bond_core#16 and ros/nodelet_core#35 for discussions why this could be useful.