ros2
Provide a LookupTransform service
We are having performance issues maintaining separate TransformBuffers in every node we spin and want to switch to tf2_ros_py.BufferClients with a central buffer running in the background instead.
Example use case:
We have a node checking if the robot "teleported" due to localization errors every 5 seconds. Using a separate Buffer + Listener, this node has to process every /tf message and takes almost 25% of a CPU core on a Ryzen 5 5600X when a lot of TF messages are swarming in.
Action vs. Service
However, the buffer server is currently implemented as an action server. @sloretz also commented:
https://github.com/ros2/geometry2/blob/141e3cb732e5c4dba744abac381c8317ffb286f0/tf2_ros_py/tf2_ros/buffer_client.py#L208-L209
Using an action server seems like a unnecessary overhead for something that is supposed to be used to reduce the computational cost of an own Buffer + Listener and also the wrong choice, given the result of the request should be either directly available or fail.
On a second node, the tf2_ros.BufferClient creates a new thread for every request to the Action client, yet still blocks the calling thread:
https://github.com/ros2/geometry2/blob/141e3cb732e5c4dba744abac381c8317ffb286f0/tf2_ros_py/tf2_ros/buffer_client.py#L239-L242
Is there a reason against just using event.wait with a timeout? (See https://docs.python.org/3/library/threading.html#threading.Event.wait ) I think this can't be the most efficient way to just get 3+4 floats from another node.
Is there a specific reason for this choice? Otherwise it would be good to (also) provide a simple service.
Is there a reason against just using event.wait with a timeout? (See https://docs.python.org/3/library/threading.html#threading.Event.wait )
Yes, there is a reason. If we use an event.wait with a timeout, that will always use a system clock. In the case of running on real hardware, this isn't a problem; the rest of the system is using a system clock as well, so it all lines up. However, in the case of simulation, you really want to be synchronized to the simulation time, not real time. Hence we really want to compute our own timeout in unblock_by_timeout, based on simulation time. That said, this actually doesn't work presently; we haven't yet figured out how to make unblock_by_timeout use ROS time. But that is the intention.
Is there a specific reason for this choice? Otherwise it would be good to (also) provide a simple service.
Yes, I think a service is a better choice here as well. The question is what we do for backwards compatibility. If we just change it, that will break anyone using the action. Still, I'd be for just switching it from an action to a service and putting a release note in place. @ros2/team, thoughts?
Thanks for the answer!
Yes, there is a reason. If we use an
event.waitwith a timeout, that will always use a system clock.
Ok, makes sense. Then we maybe we can put this reason in a comment next to the location, so anyone wondering knows why this is the case. Also, I don't know the exact overhead of spawning a new thread on every call, but maybe it makes sense to use a permanent thread for this task and just re-trigger it?
I created a very simple GetTransform service for our internal usage, and it works out fine. However, the best practice of applying retrieved transforms is still bound to the Buffer.transform() method, which wants to get a fresh transform from the buffer. Are there any plans for moving this functionality out of the Buffer? Also, there seem to be not many methods registered for this in rclpy. (Maybe this is not in the scope of this issue, though.)
We are only bothering with this, because on our machines the TransformListeners (and frankly any kind of high-frequency messaging) is eating a lot of CPU resources, probably of current DDS implementations.
I'd be for just switching it from an action to a service and putting a release note in place. @ros2/team, thoughts?
+1
Adding the service in parallel with the action seems reasonable. The feedback isn't super important for this use case. The transform method should be attached to the BufferInterface not the buffer itself so you can make a service equivalent of the client just like there's an action client.
With respect to the crossover, if we make the change of the client to use the service instead of the action. THen that user API wont' change. And the action server side could be left active and deprecated for one release cycle to give a full deprecation cycle.
hi, I'm working on a PR for this, if you have any additional guidance for the implementation, please share, otherwise we can discuss over the PR
