SteveMacenski / slam_toolbox

Slam Toolbox for lifelong mapping and localization in potentially massive maps with ROS
GNU Lesser General Public License v2.1
1.68k stars 525 forks source link

Fix bug in visualize constraint edges. #514

Closed kehanXue closed 2 years ago

kehanXue commented 2 years ago

Basic Info

Info Please fill out this column
Ticket(s) this addresses
Primary OS tested on Ubuntu
Robotic platform tested on gazebo simulation of turtlebot

Description of contribution in a few bullet points

Fix bug in visualize constraint edges. The old code only add the last two edge markers in visualization.

Description of documentation updates required from your changes

N/A


Future work that may be required in bullet points

N/A

malban commented 2 years ago

I don't think this fix is correct.

The edges_marker and localization_edges_marker are type LINE_LIST, and contain all of the respective edges. Each pair of edge points is pushed onto the respective marker and they aren't cleared. Adding the marker to the marker array each iteration should be unnecessary and will hugely duplicate the edges.

If there is a bug in the visualization, it seems like it would be something else. Do you have any more details, like a before and after screenshot?

kehanXue commented 2 years ago

OK, I will give the screenshots tomorrow. I got off work today. It seems significantly different for me.

malban commented 2 years ago

@kehanXue I do see there is a bug where the edges_marker and localization_edges_marker are given the same marker id, which means one will overwrite the other in rviz. Originally they had different namespaces, so the id's didn't conflict. Maybe this is what is causing what you are seeing.

I'll make a MR to fix.

malban commented 2 years ago

@kehanXue when you get a chance could you try this branch and see if it fixes the issue: https://github.com/SteveMacenski/slam_toolbox/pull/515

SteveMacenski commented 2 years ago

515 merged. I'll leave this open in case it doesn't work for @kehanXue but I suspect it will

kehanXue commented 2 years ago

It works. Thanks. I have a misunderstanding of the MarkerArray.