ros
Add new tf_static broadcaster class which can handle more instances per node
I guess the implemented behavior should just become the default behavior for normal static TF broadcaster, as the C++ counterpart does exactly the same... Having this duality in the interface really seems wrong to me.
And, the other way round, the statically printed warning could get into to C++ counterpart, as the same limitation applies there, too.
the C++ counterpart does exactly the same
Has the same bug? Or the same fix?
It has the behavior of aggregating all received transforms and sending the aggregate every time.
https://github.com/ros/geometry2/blob/c73b5939723db078c9bbe18523230ad54f859682/tf2_ros/src/static_transform_broadcaster.cpp#L46-L62
the statically printed warning could get into to C++ counterpart, as the same limitation applies there, too.
So then the warning is not required in C++?
No, it can still happen that you e.g. compose two nodelets, each with its own static broadcaster. The warning should thus be added to C++, too.
Or, maybe, the solution with static (global) publisher object could be ported to C++, too. But I'm not 100% sure how would that play with the plugin/nodelet loading mechanism. And adding global variables isn't a nice thing, but it could actually solve a lot of headaches and make nodelets safely composable. Do you think you could work on the C++ part or should I try to grasp it?
Feel free to grasp it!
I won't put the effort in the c++ part since I haven't run into that yet :slightly_smiling_face:
Before this PR is merged, I've released a standalone version with the proposed solution for Python:
http://docs.ros.org/en/noetic/api/cras_py_common/html/cras.html#module-cras.static_transform_broadcaster from package http://wiki.ros.org/cras_py_common .
Yeah, it's a bummer that we need to release custom packages with such minor fixes because they don't get merged or reviewed :slightly_frowning_face:
I've added this as a seperate class in the hope this would be easier to accept this change. Since that does not seem the case, should we fix the original implementation?
And if we're doing aggregation there should be at least some way to also delete/remove the transforms
Not sure I agree. The standard implementation is a latched topic. Here the tf could also only be cleared by deleting the publisher . It could not be revoked. I have not seen this in practice, so why would we need that feature here?
Also it would be good to add at least a basic test
Agreed! Will do
And if we're doing aggregation there should be at least some way to also delete/remove the transforms besides unloading the python module.
As discussed in https://github.com/ros/geometry2/issues/370, deleting static TFs is not a supported feature (and will never be in ROS 1). To work around this limitation, you can disconnect the transform from the tree (which is a use-case that should be explicitly checked in the unit tests, e.g. transform body->wheel changing to nonexistent->wheel).