Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a StaticTransformListener #662

Open
ayrton04 opened this issue Mar 15, 2024 · 6 comments · May be fixed by #719
Open

Create a StaticTransformListener #662

ayrton04 opened this issue Mar 15, 2024 · 6 comments · May be fixed by #719
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ayrton04
Copy link
Contributor

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
Copy link
Contributor

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

@clalancette
Copy link
Contributor

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.

@clalancette clalancette added enhancement New feature or request help wanted Extra attention is needed labels Apr 2, 2024
@SteveMacenski
Copy link
Contributor

That seems like approval @ayrton04 😄

@ayrton04
Copy link
Contributor Author

ayrton04 commented Apr 3, 2024

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
Copy link
Contributor

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
Copy link

scott-robotics commented Apr 15, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants