search

ros-visualization / rqt_bag

http://wiki.ros.org/rqt_bag
29 stars 53 forks source link

Fix crash during merge sort of bag entries (fixes #90) #93

Closed MichaelGrupp closed 3 years ago

MichaelGrupp commented 3 years ago

As noted in #90, the previously used function was removed in ros_comm for Noetic.

This makes rqt_bag usable again on ROS Noetic and is compatible with Python 2 distros.

peterpedron commented 3 years ago

This is very useful. Please merge asap!

peci1 commented 3 years ago

I can confirm this doesn't break Melodic :)

MichaelGrupp commented 3 years ago

@mjeronimo or @mabelzhang, can you please take a look?

wxmerkt commented 3 years ago

Thank you! :-) Could we get a patch-release with this fix please?

peci1 commented 3 years ago

I'd suggest releasing after #94 is merged, too...

mjeronimo commented 3 years ago

Yes, I can release ASAP with both #93 and #94. Thanks for the fixes. I tried this on Kinetic as well.

tkazik commented 3 years ago

Thx for the fix! :+1: Would it make sense to also replace _mergesort here? It is just a few lines below and using the same function would be a bit more consistent. (Actually I think that someone eventually would run into a bug, if the function get_entries_with_bags() was be called.) What do you think?

MichaelGrupp commented 3 years ago

@tkazik Yes, the save functionality is also broken. I created #96 as a patch.