RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
319 stars 70 forks source link

Error in TimeSource subscription to /clock topic #696

Closed wayneparrott closed 4 years ago

wayneparrott commented 4 years ago

Currently when ROSTime is active via parameter use_sim_time=true, TimeSource incorrectly subscribes to the relative topic clock. Expected TimeSource to subscribe to the topic /clock according to https://design.ros2.org/articles/clock_and_time.html. Note that the back-slash char is missing.

@minggangw currently test-time-source.js is sending Time msgs to the topic clock. Shouldn't the topic be /clock?

The background on this is I was attempting to incrementally step time for a node that is using the use_sim_time parameter. I used the ros2 topic pub tool to publish sim time updates to the node and the time updates where not being applied. Here are 2 example commands that failed until I changed the TimeSource topic to subscribe to /clock instead of clock.

ros2 topic pub /clock builtin_interfaces/msg/Time "{nanosec: 0, sec: 800}" -1

and

ros2 topic pub clock builtin_interfaces/msg/Time "{nanosec: 0, sec: 800}" -1
minggangw commented 4 years ago

The topic should be /clock, thanks for finding this problem and will merge the PR soon.

wayneparrott commented 4 years ago

The short term workaround for users of the rclnodejs ver 0.15.2 and earlier desiring to use simulation time is to use ros2 parameter remapping as shown below.

Example:

node <your_rclnodejs_node>.js --ros-args -p use_sim_time:=true -r clock:=/clock
minggangw commented 4 years ago

I think we could publish v0.15.3 to include the PR used to fix this issue, I will work on it, thanks!

minggangw commented 4 years ago

Latest v0.15.3 has landed the commit.