-
Notifications
You must be signed in to change notification settings - Fork 52
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
Correctly clean up widgets in plot_view layout #69
Conversation
Iterating over the widgets in the layout in forward order when attempting to delete all of the widgets in a layout resulted in some widgets not getting deleted properly. Instead, they should be interated over in reverse order so that the indices for each widget do not change and all are deleted. Fixes #5 Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
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.
Hmm, I am still seeing doubles after the fix.
I followed the original issue to reproduce the problem.
For example, if I View > Plot, close the plot, then View > Plot again, I see the previous plot (with the blue line) and the configure panel behind the new plot.
The more times I close it, the more duplicates I will see when I View > Plot again.
That's the behavior without the fix. The fix worked fine for me on Ubuntu 20.04 with Noetic. I opened one terminal and sourced /opt/ros/noetic/setup.bash. Another terminal and sourced the same, then built the 'mjeronimo/fix-dups-in-plot-view' branch of rqt_bag using colcon and then sourced its install/local_setup.bash. Next, opened a ROS1 bag file, 2020-10-05-14-41-57.bag. Displayed the plot view for the cmd_vel topic. Closed plot view and opened it again. In the rqt_bag version with Noetic, it displayed the artifacts. In the fix-dups branch, it does not. Can you share what you did to see if I can reproduce? |
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.
Works for me now. I must have forgotten to source or something previously.
Just one comment.
Iterating over the widgets in the layout in forward order when attempting to delete all of the widgets in a layout resulted in some widgets not getting deleted properly. Instead, they should be interated over in reverse order so that the indices for each widget do not change and all are deleted. Fixes #5 Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
Iterating over the widgets in the layout in forward order when attempting to delete all of the widgets in a layout resulted in some widgets not getting deleted properly. Instead, they should be interated over in reverse order so that the indices for each widget do not change and all are deleted. Fixes #5 Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
Iterating over the widgets in the layout in forward order when
attempting to delete all of the widgets in a layout resulted in
some widgets not getting deleted properly. Instead, they should
be traversed in reverse order so that the index for each
widget does not change and all are deleted.
Fixes #5
Signed-off-by: Michael Jeronimo michael.jeronimo@openrobotics.org