Skip to content

Conversation

@kehanXue
Copy link
Contributor


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
Copy link
Contributor

malban commented Jul 15, 2022

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
Copy link
Contributor Author

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

@malban
Copy link
Contributor

malban commented Jul 15, 2022

@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
Copy link
Contributor

malban commented Jul 15, 2022

@kehanXue when you get a chance could you try this branch and see if it fixes the issue: #515

@SteveMacenski
Copy link
Owner

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

@kehanXue
Copy link
Contributor Author

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

@kehanXue kehanXue closed this Jul 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants