ros / geometry2

A set of ROS packages for keeping track of coordinate transforms.
190 stars 275 forks source link

tf_py.BufferCore not respecting cache_time argument #540

Open MechatronixX opened 2 years ago

MechatronixX commented 2 years ago

For ROS Noetic on Ubuntu 20.04 and Python 3.8.10. When loading a tf.py.BufferCore with transforms, it seems it does not respect the cache_time argument when setting it as a named argument in Python. Running the below code should generate an exception with a message along the lines of

Lookup would require extrapolation 9.999000000s into the past. Requested time 0.001000000 but the earliest data is at time 10.000000000, when looking up transform from frame [frame_b] to frame [frame_a]

The error message is consistent with an hypothesis that using the cache_time named argument makes the constructor use the default argument. The default argument is as far as I can tell from scattered sources online a duration of 10 seconds.


import geometry_msgs.msg
import rospy
import tf2_py

TIME_RANGE = 20
CACHE_DURATION = rospy.Duration(float(TIME_RANGE*2))

# This works fine:
#t = tf2_py.BufferCore(CACHE_DURATION)

# Using the cache_time named argument makes it default to a cache of 10 seconds!
t = tf2_py.BufferCore(cache_time=CACHE_DURATION)

# Generate dummy synthetic data
FRAMES = ["frame_a", "frame_b"]
for time in range(TIME_RANGE+1):
    for parent_frame, child_frame in zip(FRAMES[:-1], FRAMES[1:]):
        m = geometry_msgs.msg.TransformStamped()
        m.header.frame_id = parent_frame
        m.header.stamp = rospy.Time.from_sec(float(time))
        m.child_frame_id = child_frame
        m.transform.translation.x = 1.0
        m.transform.rotation.w = 1.0

        t.set_transform(m, "default_authority")

# Do lookups over the whole time range except for the last second, make sure it works.
for time_ms in range((TIME_RANGE-1)*1000):
    tf = t.lookup_transform_core(target_frame=FRAMES[0],
                                 source_frame=FRAMES[-1], time=rospy.Time.from_sec(time_ms/1000))
tfoote commented 1 year ago

It's read here: https://github.com/ros/geometry2/blob/5d17166159a0f9e90197f37709355ce5d757bb02/tf2_py/src/tf2_py.cpp#L182

It uses PyArg_ParseTuple and would need to be extended to use

PyArg_ParseTupleAndKeywords to be able to use the keywords as well.

If this would be valuable to you a PR would be appreciated.

MechatronixX commented 1 year ago

I see. Maybe just disabling named arguments in the concerned function would be fine for now? https://deepsource.io/blog/python-positional-only-arguments/

Not planning to create a PR currently.