ros-controls / ros_controllers

Generic robotic controllers to accompany ros_control
http://wiki.ros.org/ros_control
BSD 3-Clause "New" or "Revised" License
560 stars 526 forks source link

Fix uncaught exception in joint_state_controller #630

Closed rhaschke closed 7 months ago

rhaschke commented 7 months ago

Using sim time in Gazebo, the starting time is close to zero, such that time - ros::Duration(1/publish_rate_) becomes negative. This causes an exception to be thrown: "Time is out of dual 32-bit range".

bmagyar commented 7 months ago

Thank you!

Would you like a release of this or happy with it being merged for now?

rhaschke commented 7 months ago

A merge would be fine for now. Sorry for pushing broken code initially. It should work now.

01binary commented 7 months ago

Totally forgot about this. I also have a fix in https://github.com/ros-controls/ros_controllers/pull/629, but the unit tests are failing and I couldn't find a way to fix them.

We can close that pull request and delete the branch if this version of the fix gets merged instead.

One small note: I see that you fixed the issue by catching an exception instead of ensuring it doesn't occur by performing a greater-than check. This code could potentially execute in a tight (update) loop, so throwing an exception and catching it every loop iteration, in a loop that's supposed to run at a specific frequency may not be the best thing long-term.

rhaschke commented 7 months ago

@bmagyar, can you please rerun the CI tests? I think the test JointStateControllerTest.valuesOk is flaky. Locally, it works for me most of the time but occasionally fails.