Add new interface for time conversion#145
Conversation
27b4266 to
f4f6ee9
Compare
f4f6ee9 to
42fe666
Compare
aprotyas
left a comment
There was a problem hiding this comment.
Suggesting some inline fixes that should help address the problems with CI.
|
Please help to re-run CI. |
|
@ivanpauno friendly ping |
|
@ivanpauno Updated based on your comments. |
|
This looks like it needs a rebase, then CI and I think we can get it in. |
include/rcpputils/time.hpp
Outdated
| // Casting to a double representation might lose precision and allow the check below to succeed | ||
| // but the actual cast to nanoseconds fail. Using 1 DurationT worth of nanoseconds less than max | ||
| constexpr auto maximum_safe_cast_ns = | ||
| std::chrono::nanoseconds::max() - std::chrono::duration<DurationRepT, DurationT>(1); |
There was a problem hiding this comment.
Found a bug.
std::chrono::duration<DurationRepT, DurationT>(1) depend on user input. This leads maximum_safe_cast_ns isn't fixed value. I will use std::chrono::nanoseconds(1) instead.
There was a problem hiding this comment.
I thought this was not a bug, it is intentionally based on user template unit. if i am not mistaken...
There was a problem hiding this comment.
If depend on user template unit, there is a below problem.
For the same time, std::chrono::minutes(2562047*60) doesn't throw exception, but std::chrono::hours(2562047) can show exception.
Sorry, wrong place to reply. |
6164e5c to
a9f9d67
Compare
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
a9f9d67 to
e9f73af
Compare
|
Do rebase |
Add a new interface
rcpputils::rcutils_duration_cast.About this new interface, it is mentioned at ros2/rclcpp#1662 (ros2/rclcpp#1662 (comment))