geometry2 icon indicating copy to clipboard operation
geometry2 copied to clipboard

Missing type checks at Python/C++ interface

Open tpet opened this issue 9 years ago • 2 comments

There seems to be missing "type" checks at the Python/C++ interface causing quite unpredictable behavior if the inputs are not as expected. Namely, in tf2_ros.Buffer.set_transform it is not checked that the fields on the rotation and translation objects actually exist.

E.g., running the code below (rotation and translation mismatched)

import tf2_ros
from geometry_msgs.msg import TransformStamped, Vector3, Quaternion

if __name__ == '__main__':
    t = TransformStamped()
    t.header.frame_id = 'parent'
    t.child_frame_id = 'child'
    t.transform.rotation = Vector3()
    t.transform.translation = Quaternion()

    tf2_buf = tf2_ros.Buffer(None, False)
    tf2_buf.set_transform(t, 'default_authority')
    tf2_buf.lookup_transform('child', 'parent', t.header.stamp)

completes fine but leads to a silent exception in the background:

Exception TypeError: 'bad argument type for built-in operation' in <module 'threading' from '/usr/lib/python2.7/threading.pyc'> ignored

In more complicated scenarios it usually fails somewhere with a misleading error message. I think the checks should be more strict.

tpet avatar Mar 11 '16 16:03 tpet

The relevant lines seems be https://github.com/ros/geometry_experimental/blob/indigo-devel/tf2_py/src/tf2_py.cpp#L395 https://github.com/ros/geometry_experimental/blob/indigo-devel/tf2_py/src/tf2_py.cpp#L426 https://github.com/ros/geometry/blob/indigo-devel/tf/src/pytf.cpp#L410

tpet avatar Mar 11 '16 17:03 tpet

That seems like a reasonable improvement to make the API safer. If you have time to make a PR that would help make this happen sooner.

tfoote avatar Mar 11 '16 18:03 tfoote