ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
752 stars 911 forks source link

The `std::string Recorder::timeToStr(T ros_t)` function in the bag recorder does not use the provided input argument #2260

Open tgaspar opened 2 years ago

tgaspar commented 2 years ago

I've been working on a functionality for the rosbag recorder (forked) that allows you to split the bags according to the wall clock (e.g. split every five minutes, at 13:00:00, at 13:05:00, 13:10:00, etc).

On rare occasions, it happened that the rosbag filename was inconsistent with the selected modulo. Instead of having the name 2022-07-29-13-05-00 it had the name 2022-07-29-13-04-59.

After some digging through the recorder.cpp file, I realized that even if we see the following snippet inside the updateFilenames function

...
    if (options_.append_date)
        parts.push_back(timeToStr(ros::WallTime::now()));
...

the ros::WallTime::now() does not get used inside the timeToStr function:

template<class T>
std::string Recorder::timeToStr(T ros_t)
{
    (void)ros_t;
    std::stringstream msg;
    const boost::posix_time::ptime now=
        boost::posix_time::second_clock::local_time();
    boost::posix_time::time_facet *const f=
        new boost::posix_time::time_facet("%Y-%m-%d-%H-%M-%S");
    msg.imbue(std::locale(msg.getloc(),f));
    msg << now;
    return msg.str();
}

I see this is as the only possible reason for this kind of behavior.

This is not a bug report because it does not affect any of the functionalities of the rosbag recorder as it currently is. This is merely a question to understand the reasoning behind this implementation.