-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix map display for moving TF frame #483
Conversation
Instead of the current time, use Time(0) to get the latest available transform as a fallback. This is the same logic that is applied in RViz from ROS 1. Resolves #332 Signed-off-by: Jacob Perron <jacob@openrobotics.org>
I guess using Time(0) should be the same in all displays? If that is true, a quick search reveals that this is also wrong in the axes display (my bad) and all pointcloud displays (see pointcloud common). The background is that when we ported most of the displays, the time API wasn't finished and we mostly tested with static publishers. Should this be done in a different ticket or do you prefer to do it here? |
I also noticed these other occurrences. I was going to investigate further to see if I can reproduce the same issue with the other displays before proceeding. I can create a ticket for (potentially) fixing the other displays. |
I've opened up a follow-up #484 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks for looking into it!
Instead of the current time, use Time(0) to get the latest available transform as a fallback. This is the same logic that is applied in RViz from ROS 1. Resolves #332 Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Instead of the current time, use Time(0) to get the latest available transform as a fallback. This is the same logic that is applied in RViz from ROS 1. Resolves #332 Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Instead of the current time, use Time(0) to get the latest available transform as a fallback.
This is the same logic that is applied in RViz from ROS 1.
Resolves #332
Although we are using TF message_filters to deliver the map to the display, the display itself is doing a transformation:
rviz/rviz_default_plugins/src/rviz_default_plugins/displays/map/map_display.cpp
Lines 576 to 579 in 045227a
And even though it is using the current time (just a few lines before), it is off by less than a second causing the extrapolation exception. I noticed the ROS 1 implementation is slightly different, defaulting to the latest available transform by passing Time(0) (the related PR is ros-visualization/rviz#1066):
https://github.com/ayrton04/rviz/blob/8f482f494ee4031ed04b3065a9755710b61176bf/src/rviz/default_plugin/map_display.cpp#L719-L720