search

ros / roscpp_core

ros distribution sandbox
88 stars 116 forks source link

improper implementation of ros::Time::useSystemTime() #91

Closed sunnyroad closed 6 years ago

sunnyroad commented 6 years ago

https://github.com/ros/roscpp_core/blob/1110bb03024d5b698c481b41e5eeb7f4b5afbcbd/rostime/src/time.cpp#L285 may be should like this if we want to change to use SystemTime

void Time::useSystemTime()
{
  g_use_sim_time = false;
}
dirk-thomas commented 6 years ago

The function bool Time::useSystemTime() acts as a "getter" - not as a "setter" so it is fine as it is. (Maybe not named very intuitively.)

Assuming this resolves your "problem" I will go ahead and close the ticket. Please feel free to continue commenting on it.

sunnyroad commented 6 years ago

then what's the difference between bool Time::useSystemTime() and bool Time::isSystemTime() ? and which function to call if we want to change to using Systemtime, where is the "setter" function? Time::init()?

dirk-thomas commented 6 years ago

what's the difference between bool Time::useSystemTime() and bool Time::isSystemTime()?

There is none - both methods do exactly the same.

which function to call if we want to change to using Systemtime, where is the "setter" function? Time::init()?

The choice between system time and sim time is being made based on the Clock. So there is no API you expect to simply switch between them. When sim time is enabled and clock messages are received they set the time (https://github.com/ros/ros_comm/blob/6b9efd56d6882d1c017152ba11a5780a1496b30b/clients/roscpp/src/libros/init.cpp#L255-L258) which then changes the state of the clock: https://github.com/ros/roscpp_core/blob/1110bb03024d5b698c481b41e5eeb7f4b5afbcbd/rostime/src/time.cpp#L318-L324