search

ros-visualization / rqt_bag

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

Fix merge sort in bag export. #96

Closed MichaelGrupp closed 3 years ago

MichaelGrupp commented 3 years ago

Follow up to #93.

A bit more changes were necessary to keep track of the corresponding bag files for each entry.

Tested on Noetic with two overlapping bagfiles.

MichaelGrupp commented 3 years ago

Crap, heapq.merge has no key argument in Python 2...

MichaelGrupp commented 3 years ago

Now it should also work on ROS Melodic. Would be great if someone could confirm this.

mjeronimo commented 3 years ago

@MichaelGrupp I was unable to reproduce an issue on Melodic. I tried to create overlapping bag files, load into rqt_bag and then export to a new bag file. Can you please provide the sequence of steps to reproduce and some sample bag files, if possible? Thanks.

MichaelGrupp commented 3 years ago

The issue doesn't happen on ROS Melodic because the removal of this rosbag.bag._mergesort function was not released for Melodic, only Noetic. As far as I understand, rqt_bag is published for both Melodic and Noetic from the master branch so we need to ensure that this patch works for both Python 2 and 3, although the bug itself doesn't exist in Melodic.

mjeronimo commented 3 years ago

Ah, OK. Thanks for the info. I'll try the patch on Melodic.

mjeronimo commented 3 years ago

Works fine on Melodic.