Re-enable use of tf message filter

[shiveshkhaitan]

Re enable use of TF Filter in rviz_common and rviz_default_plugins. This connects to issue#55 issue#359

[andreasholzner]

If I understand this correctly enabling tf filters does not make sure that tf is used as a transformer plugin via the TransformerGuard mechanism. This will likely fail if another transformer plugin were active.

On the other hand the ideas behind #346 were never realized since in fact tf is probably always used. Other usages of rviz without tf and ros messages will make use of completely independent display classes. So maybe it is best to revert #346 as this additional complexity seems unnecessary now and this would also remove the pointer casts from this pr. However, I am not sure how easy this can be done now -> @wjwwood .

[shiveshkhaitan]

Yes, I guess this defeats the whole purpose of #346. I will try incorporating tf filters into the plugin mechanism itself if it possible.

[Martin-Idel]

This will be very hard, I believe:

• The transformers are plugins, so which plugins exist is not known at compile time nor is it known which transformer is selected.
• The displays are likewise plugins and which plugins exist is not known at compile time. Since it depends on the message and generating custom messages is an important feature of ROS, there are likely many custom displays out there.
• Message filters however require working with a subscription, which is in the RosTopicDisplay. Since the subscription is templated, it needs to be known at compile time.

That's a problem, where I see only two possible solutions:

• Find a way to do an untemplated subscription, which will require changes to other libraries or at the very least a lot of new code (see how rosbag2 does it for normal ROS subscriptions)
• Treat the tf2 transformer as a special case: Introduce some code switching between a normal subscription and a message filter subscription based on whether the transformer is a tf2 transformer or a different transformer and declare that transformer plugins will only get a normal subscription.

I think the second solution is rather awkward and the first is quite hard.

Given that, I also believe it would be best to revert the introduction of the transformer plugin hierarchy, if @wjwwood agrees. If it's worthwhile, I could make a PR reverting it to see how difficult it is from a technical perspective.

[shiveshkhaitan]

Or can this be handled using something similar to transformer_guard?
Like the guard can check for the transformer_plugin been used. And if the plugin used is the default tf2 plugin, subscription would be done using the tf_message_filters. When using any other transformer, the normal subscription would be used.
This way the current implementation of transformer_plugins would not be affected.

[Martin-Idel]

Yes, that's essentially what I tried to convey with my second option. That will be doable, I just wonder how understandable it'll be afterwards.

[shiveshkhaitan]

@Martin-Idel @wjwwood Is there any update on the mechanism to be followed?
Should I try implementing the transform_guard approach and see how does it come up?

[wjwwood]

Sorry guys, this takes more thought and time than I have at the moment. I'll get to it as soon as I can.

[Myzhar]

Any news about this PR? Are message_filters enabled in Rviz2?

[jacobperron]

Other usages of rviz without tf and ros messages will make use of completely independent display classes.

Correct me if I'm wrong, but currently it looks like the only displays that require tf2 are TFDisplay, LaserScanDisplay, and RobotModelDisplay (the ones making use of TransformerGuard). IIUC, the other displays can be used with other transformer plugins.

---

I spoke with @wjwwood, and we agree that it would be better keep the frame transformer abstraction.
Another solution is to update tf2_ros::MessageFilter to accept an interface that RViz frame transformers can implement. In this approach, we use the transformer plugin to get message transforms and leverage tf2 for applying transforms. Updating tf2_ros::MessageFilter doesn't look like it will be straight-forward, but I can take a shot at it.

[jacobperron]

I've proposed changes to tf2 (ros2/geometry2#121) and opened #422 as a possible approach for supporting the TF message filter with any FrameTransformer.

[AndreasAZiegler]

May I ask about the current state?

[jacobperron]

May I ask about the current state?

ros2/geometry2#121 and #422 are ready for review. Once those are in, I suggest we rebase this PR to make use of the changes in order to remove the use of tf2_ros::Buffer.

[emersonknapp]

@shiveshkhaitan the above dependencies have been merged, are you still interested in following this through? I'm looking forward to this change!

[shiveshkhaitan]

Yes, I am currently working on the rebase. Should be getting it done in some time soon

[jacobperron]

Thanks for doing this!

I've done a first pass. I haven't tested it yet.

[bigdayangyu]

We rebased this PR again ontop of the latest nightly packaging job and with changing the override annotation it compiles successfully, however it segfaults when trying to launch RVIz.

[jacobperron]

Thanks @shiveshkhaitan! LGTM
I just rebased on the main branch so that CI will compile.

[jacobperron]

• Linux
• Linux-aarch64
• macOS
• Windows