ros_comm
ros_comm copied to clipboard
ros
Should rosbag play advertise /clock or /namespace/clock when using a namespace?
As I was putting together a launch file to replay a bag file in a namespace, I noticed that the --clock option caused the clock to be published to /namespace/clock instead of /clock.
Is that expected? A quick fix is to add <remap from="clock" to="/clock"/>, but should that be necessary?
Seems like changing "clock" to "/clock" here should resolve the issue. https://github.com/ros/ros_comm/blob/bf56b4022d836c8ce63e0abf2474ade32f9a8f64/tools/rosbag/src/player.cpp#L771
If someone does indeed want the clock to be published in a namespace, remap could be used. I think this would be a special case and normally users would expect the clock to publish to /clock.
Any reason I shouldn't open a pull request with that fix?
normally users would expect the clock to publish to /clock.
agree on this, that is what we would expect likely.
Any reason I shouldn't open a pull request with that fix?
i would not think of any...
TBH, if you push the rosbag play node into a namespace, I would expect the clock topic to assume that namespace (perhaps just from experience).
I think we should avoid "fixing" it in an already released distro since it could break expected behavior for anyone relying on it.
To avoid breaking existing clients, you could simply retain the namespaced topic while adding the correct global topic. The namespaced topic can later be removed at an appropriate time. If we have agreement I can take a look at implementing the PR. I imagine it won't be hard.
The question still stands: is it "correct" to default to the root namespace /clock or allow it to be namespaced? The only documentation I found on the matter is http://wiki.ros.org/Clock, which seems to imply it should use the root namespace.
In any case, since there's no planned distribution following Noetic and there's a workaround (using a remap), I'd rather not introduce solutions moving towards a new behavior at this time.
The answer to the original question, at least, is unambiguous: Sim time is supposed to use the global /clock, as it says in the documentation. The /use_sim_time param (which is also global) causes nodes to subscribe to /clock regardless of their own namespacing. rosbag's behavior is a bug.
It looks like they've done it correctly in ROS2, at least.