ros / roslisp_common

Clone of the old roslisp_common SVN repo from code.ros.org.
8 stars 14 forks source link

In TF what should the default timeout be for waiting for a transform #34

Closed gaya- closed 8 years ago

gaya- commented 8 years ago

As far as I understand, in C++ the timeout is obligatory: https://github.com/ros/geometry_experimental/blob/indigo-devel/tf2_ros/src/buffer.cpp#L117 In Python it seems to be 0 (?): https://github.com/ros/geometry_experimental/blob/indigo-devel/tf2_ros/src/tf2_ros/buffer.py#L68

In Lisp the default value is 0.0 in cl-tf2, which means wait for 0.0 seconds, and NIL in cl-tf, which means wait forever. What is the better of the two? Should we have a default value or make the parameter obligatory? Do we even want to have a wait forever option (it could potentially encourage bad coding style, you really don't want anything to wait forever)? If we do, should it be a negative number or NIL?

@airballking, @fairlight1337, @bbrieber, @gheorghelisca, @mpomarlan, ... ?

airballking commented 8 years ago

That is a good question. I have been wondering about this myself.

I think waiting for a transform forever invites trouble, and could be considered a code smell.

However, some of us have used different versions of waiting for a transform forever, in the past. I think this was because we encountered situations in which both cl-tf and cl-tf2 would not have transforms that definitely were communicated over the /tf topic. I believe these problems stopped when we fixed the (ros-time) in roslisp https://github.com/ros/roslisp/commit/68831b5f0cd15d2d1ca42b111db5860cceacbca8

I would vote for keeping the interfaces of standard ROS components as similar as possible between the various client libraries. So, if rospy uses 0.0 seconds as a default timeout, then I suggest to adopt the same for cl-tf and cl-tf2.

P.S.: It seems the roscpp version of TF also uses 0.0 seconds as a default: https://github.com/ros/geometry_experimental/blob/indigo-devel/tf2_ros%2Finclude%2Ftf2_ros%2Fbuffer_client.h#L70