ROS 2 Time vs std::chrono

We were investigating if we should use ROS 2 Time or std::chrono for our product. Note that our use case is a highly safety critical real-time system influencing the priority of the arguments.

Upon closer inspection, I found that ROS 2 Time is largely redundant to std::chrono.
All functionality of ROS 2 simulated time can also be achieved by providing a custom std::chrono clock encapsulating the rclcpp::Clock which would look like this (modulo some shared ptr shenanigans I omitted):

struct ros2_clock
{
  typedef std::chrono::nanoseconds 				duration;
  typedef duration::rep	  				rep;
  typedef duration::period	  				period;
  typedef std::chrono::time_point<ros2_clock, duration> 	time_point;

  static constexpr bool is_steady = false;

  static time_point
  now() noexcept {
    return time_point(duration(clock.now().nanoseconds()));
  }


  static Clock clock;
};

So from my point of view it is hard to justify using ROS 2 Time as:

  1. rclcpp uses std::chrono::Duration already.
  2. std::chrono is safer as it is not possible to mix durations from different clocks.
  3. std::chrono allows a custom time representation allowing to create overflow and underflow safe time operations.
  4. With sticking to a single time library there is no confusion on which to use when and no performance degradation from type conversion.
  5. std::chrono is the C++ standard many people are already familiar with.

So given our use cases, are there arguments speaking for using ROS 2 Time in our code instead of std::chrono?

I totally agree.
+1 for using modern C++ and avoid redundancy.

1 Like

ROS2 time is probably there to support in an unified manner the different client libraries.

Also to provide simulation in lockstep and at a different rate from 1:1

These are the first reasons that come to my mind.

2 Likes

Is it important for your product to support being used by two nodes in the same process at the same time?

If rclcpp used std::chrono then there could only be one clock per clock type per process. If a process with two nodes starts and a user sets use_sim_time to true on one node and false on the other then what time should ros2_clock::now() return?

@sloretz: You can wrap the clock provided by the node again in a std::chrono clock wrapper and you would have simulation time available.:

struct ros2_node_clock
{
  ros2_node_clock(std::shared_ptr<Clock> clock) : m_clock(clock) {}
  typedef std::chrono::nanoseconds 				duration;
  typedef duration::rep	  				rep;
  typedef duration::period	  				period;
  typedef std::chrono::time_point<ros2_clock, duration> 	time_point;

  static constexpr bool is_steady = false;

  static time_point
  now() noexcept {
    return time_point(duration(clock->now().nanoseconds()));
  }


  std::shared_ptr<Clock> m_clock;
};

ros2_node_clock rclcpp::None::get_std_clock() {
   return ros2_node_clock(get_clock());
}

The additional safety of not mixing clocks is lost in this case, but ROS 2 time is also not safe in this regard and not likely solvable.

If I understand correctly, I think the issue is that the std::chrono clock requirements don’t allow now() to be a non-static member function. ros2_node_clock::now() has to be static and could not access m_clock.

A while ago I did try prototyping a clock that conformed to the std::chrono requirements except with a non-static member now(). Not all C++ API’s allow this. For example std::thread::sleep_until() tries to call clock::now() on the time point passed into it. I gave up because whether or not an API calls clock::now() in its implementation is undefined behavior.

1 Like

@sloretz: Oh, that is a really good point! Thank you!

They should have designed std::chrono as they fixed std::allocators. :slight_smile:

This has the interesting implication that ROS 2 time can never be converted to std::chrono time and used with the standard library as it is not determinable from which clock the time came.

I will check if it is possible to create a std::chrono clock which at least works on a global level such that an error is produces when someone tries to switch a node to a different time than some global setting.

Why? From my perspective the time point returned from ROS Time must be compatible with the system clock since that’s what is returned when no simulated time is used.

I don’t see any reason why we could not have a conversion function between a ROS Time instance and a chrono system_clock::time_point instance.

I don’t think that’s what @sloretz was after, it might be totally correct to have one node using sim time and the other not, I see no reason to couple it to a global state.

Also, you have to consider the case where two nodes exist and only one has the subscription to the /clock topic and then that node is destroyed. Now the second node still exists and is expecting sim time but has no clock subscription.

These cases are why we have a non-global and non-static clock class.

@wjwwood: Thank you for your input!

I checked the code more closely and it is actually encoded in rclcpp::Time which clock was used. This is great! But if it set to RCL_ROS_TIME I have no information about the state of the clock which was used to create this rclcpp::Time, right? Maybe it was simulated time, maybe something else. Should rclcpp::Time not have a pointer to the associated rclcpp::Clock?

Maybe I am missing a feature in ROS 2 time which solves this issue but I am unable to find it.

I am also unsure about the solution how would one implement the conversion from rclcpp::Time to std::time_point.

??? to_time_point(rclcpp::Time time) {
  if(time.get_clock() == RCL_SYSTEM_TIME) {
    return std::chrono::system_clock::time_point(time.nanoseconds()ns)
  }
  else if(time.get_clock() == RCL_STEADY_TIME) {
    return std::chrono::steady_clock::time_point(time.nanoseconds()ns)
  }
  else {
    //ERROR not supported.
  }
}

One could use boost::variant or extend the standard library but those are surely not optimal solutions.

So this is where my expectation for ROS Time and what is actually implemented right now has diverged.

My expectation is that ROS Time would not be able to store or provide Steady time, and instead that would be a separate type or just be std::chrono::steady_clock instead.

I did bring up this issue at the time, but I think the refactor to address my concerns was tabled for later, but unfortunately I don’t see an issue on ros2/rclcpp tracking the problem either. @tfoote do you know if this is tracked somewhere? Perhaps on a meta issue or something?

Either way it is stated this way in the design document:

SteadyTime will be typed differently than the interchangable SystemTime and ROSTime . This is because SystemTime and ROSTime have a common base class with runtime checks that they are valid to compare against each other. However SteadyTime is never comparable to SystemTime or ROSTime so it would actually have a separate implementation of the same API. Thus you could get protection from misusing them at compile time (in compiled languages) instead of only catching it at runtime.

– from: Clock and Time


If you assume that this change is made, then ROS Time (i.e. rclcpp::Time) can only either be from the system time or from a custom time source. However, the custom time source must provide a time that is “compatible” with system time. For me that means it uses the same epoch (i.e. unix epoch) and it follows the same behaviors like handling leap seconds. Put another way a time stamp from system time and the same timestamp from a custom time source should match when converted to a date time (yyyy-mm-dd-…).

So with that assumption, the conversion function would look like this:

std::chrono::system_clock::time_point
to_time_point(const rclcpp::Time & time)
{
  return std::chrono::system_clock::time_point(std::chrono::nanoseconds(time.nanoseconds()));
}
1 Like

I don’t think that this ended up being tracked because there was a desire to not grow the backlog with enhancement tickets for known things to do. We specifically did leave it in the design document as a specific choice. And since it’s something open that might be able to be picked up by someone else it would probably be worth us opening an issue to track it.