ros::Rate should use steady_clock

[DasRoteSkelett]

Hi!

There are some examples, that might tempt people to use ros::Rate for their (possibly) real time control loop. But, as far as I can tell, ros::Rate uses walltime clock, which may skip (NTP, someone setting Date and Time, ...). When I use ros::Rate, I am usually not interested in how much "walltime" was spend, but the real time difference.
Please consider at least leaving a clear statement in the Tutorials (I know, Timers are recommended, but they seem to use walltime as well).

Regards,

Matthias

[flixr]

You could make a PR to add a SteadyRate class that uses SteadyTime.
Otherwise use the SteadyTimer.

Also as already noted on http://wiki.ros.org/roscpp_tutorials/Tutorials/Timers you should not rely on any of those for realtime loops.

[mikepurvis]

I kind of agree with the statement though, that ros::Rate by default should use SteadyTime. That's a change which could safely be made for Melodic at least.

Blah, I did not consider use_sim_time; thank you for the reminder on that.

[flixr]

But wouldn't that be even more confusing and break a lot of existing cases?
Since then it wouldn't support the simulated time anymore...

Or do you propose to make ros::Rate still support the simulated time and use SteadyTime instead of WallTime if /use_sim_time is not true?

That again would make it inconsistent with ros::Timer, unless we change that to use SteadyTime instead of WallTime for non-sim time as well...
Which in turn is weird because ros::Time is either sim time or WallTime (and there it doesn't make sense to change it).

[DasRoteSkelett]

Hi there,

without having looked to much into ros::Time, I wonder if a rewrite towards std::chrono could be helpful. Right now, I am seeing code for WIN32, Apple... which I guess should be all the same if one uses std::chrono. Then, ros::Time, ros::Rate, ros::Timer could be templated classes, with the clock source as template argument. If you are afraid of introducing new behaviour, you could use std::chrono::system_clock as default template argument. I still see it as a bug (walltime instead of monotonic). albeit a seldom occurring one.

Regards,

Matthias

[flixr]

AFAIK ROS2 is requiring C++11 and using std::chrono, but here (in ROS1) we don't require C++11

[clalancette]

AFAIK ROS2 is requiring C++11 and using std::chrono, but here (in ROS1) we don't require C++11

As of Melodic, C++14 is being allowed everywhere (though we will not change the public core APIs to use C++14 features unless there is a very good reason to do so). So moving towards std::chrono could be an option for Melodic and newer if it doesn't affect the public API.

[wjwwood]

We could use std::chrono, and maybe we should now, but I don't see that as addressing the issue here which, as I understand it, is that ros::Rate doesn't use steady time. Also, what we're doing is basically the same thing that libc++ does, see:

https://github.com/llvm-mirror/libcxx/blob/master/src/chrono.cpp

So while we could shed some custom logic of our own, I don't think it will be magically better or anything.

---

As for ros::Rate, I think it should use sim time, as many applications rely on the rate changing in accordance with the realtime factor when playing data back with rosbag or running a simulation like gazebo.

Having a separate ros::WallRate and ros::SteadyRate which do not adhere to sim time might be useful, but the users could also implement this.