Fix and improve ROS visualization

[nachovizzo]

First of all, now I can finally enjoy reading the implementation. Without the spaghetti if/else/if/else trying to guess which transformation should be applied.

My approach here was to replicate the same as we do in the Python visualizer.:

kiss-icp/python/kiss_icp/tools/visualizer.py

Line 223 in 44d8563

self.target.transform(np.linalg.inv(pose))

The new logic is as follows:

• When debugging using ROS, everything happens in an ego-centric world, from the sensor's point of view
• Since the KISS-ICP local map is on the odometric coordinate frame, we need to apply the last ego-centric pose to bring it back to the lidar frame.
• This allowed for further simplification

The only caveat is that, if we DO NOT PUBLISH to the tf tree, then only the trajectory will be on a different reference frame and would look odd in the viz. The rest is still fine. Since this is something we never used, need, we don't have in Python, I figured out that's worth disabling it by default.

The rest I think is all improvement so far

NOTE I haven't changed the implementation of KISS, just the visualization utilities for ROS

[benemer]

Since this is something we never used, need, we don't have in Python, I figured out that's worth disabling it by default.

But showing the point clouds and the full trajectory is something we can show in the Python visualizer or I am getting it wrong? Or are you talking about the default in the visualizer?

[nachovizzo]

Since this is something we never used, need, we don't have in Python, I figured out that's worth disabling it by default.

But showing the point clouds and the full trajectory is something we can show in the Python visualizer or I am getting it wrong? Or are you talking about the default in the visualizer?

You are right, it's something we can show on the Python side. Here it's always populating this infinitely wrong path which I don't think it has an application on the ROS side of things. The path can also be seen in RVIZ by just clicking in the odometry topic, which we don't have in Python

[tizianoGuadagnino]

@nachovizzo as a general comment not strictly related to the issue (which looks good as far as I saw), would it make sense to always publish the tf (a.k.a. we remove the publich_odom_tf_ flag) ?

[nachovizzo]

@nachovizzo as a general comment not strictly related to the issue (which looks good as far as I saw), would it make sense to always publish the tf (a.k.a. we remove the publich_odom_tf_ flag) ?

@tizianoGuadagnino Sadly no. No because it doesn't make sense, but rather due to a limitation of tf. Assume the transformation you want to publish is already present in the tree, and then publishing it from Kiss will create some conflicts.

I always wonder if we should go back to the hacky "alias" tf we had, just to publish a completely disconnected tree to make sure we control that.

On the other hand, we also have a strong limitation with rviz. Since, the visualization frame can't be variable, and thus is hardcoded. I'd prefer to do like we do in the python visualizer and see everything from the lidar eyes, but rviz forbid us to do so

[tizianoGuadagnino]

Ok, now it is clear why, I didn't think about that.

I have mixed feelings about the alias, although I really like the idea of having a bit more control on the tf tree, even if it requires some hacks. Maybe it is something worth discussing offline with @benemer .

Anyways, from my side, we can merge this.

[nachovizzo]

I also have mixed feelings, especially because it adds a ton of code that is virtually necessary. But all these problems are to avoid a user launching the pipeline and seeing a black screen because we couldn't guess 🔮 the frame_id from who knows which lidar is being used.

Complicated tasks to tackle in my opinion if we want to stay generic and don't force the user to read 5 pages of documentation on how to configure rviz