search

ros2 / geometry2

A set of ROS packages for keeping track of coordinate transforms.
BSD 3-Clause "New" or "Revised" License
110 stars 193 forks source link

Create a StaticTransformListener #662

Open ayrton04 opened 3 months ago

ayrton04 commented 3 months ago

Feature request

Feature description

The TransformListener in tf2_ros subscribes to both the /tf and /tf_static topics and provides a unified transform tree. However, there are many cases where a node will only be using transforms that are known to be static, e.g., looking up transforms from a sensor frame to the body frame. In those cases, all of the overhead in maintaining the high-rate transform data as provided by the /tf topic is totally wasted. Given that many nodes use TransformListener, this can quickly add up to a lot of wasted resources in systems with large numbers of nodes.

I propose that we implement a StaticTransformListener that only subscribes to /tf_static.

Implementation considerations

We could define a base class for both TransformListener and StaticTransformListener so keep the interface consistent between the two. The aim of the design would be to not change anything about TransformListener itself from a user perspective.

If that is not possible, then if we could at least make the init method protected instead of private, it will let users override this behaviour themselves.

SteveMacenski commented 3 months ago

I would love this. @ahcorde @clalancette is this something that you'd be open to merging if it was provided in a PR?

clalancette commented 3 months ago

It seems like a reasonable feature request to me.

One other implementation idea I'll throw out is to not add a new class at all, but instead add a new parameter to the constructor/init method that chooses whether to use /tf, /tf_static, or both, with the default being both. That seems like less churn to me, but I don't have a strong opinion between that and a new class.

SteveMacenski commented 3 months ago

That seems like approval @ayrton04 :smile:

ayrton04 commented 3 months ago

If we're OK with changing the API for TransformListener, then that's great!

We'll have to think about how to add that parameter. It feels a little strange if we have a parameter that says "only subscribe to static data", but then we have to provide the QoS and subscription options for the unused /tf subscription. Maybe a new constructor overload?

On PTO, but will come back to this soon.

clalancette commented 3 months ago

If we're OK with changing the API for TransformListener, then that's great!

To be clear, we can't change any of the existing parameters or constructors; that would constitute an API break. But we can add in new parameters with default values (this is only an ABI break), or we can add in new constructors as you suggest.

scott-robotics commented 2 months ago

Would adding an optional frame_id filter for /tf to the TopicListener achieve the same goal? i.e. only process /tf frames that match a particular/specific pattern. This way, you can still connect disconnected static groups with a dynamic transform (/tf).

camera1 → base_link1 → odom ← base_link2 ← camera2