Skip to content
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 crash during merge sort of bag entries (fixes #90) #93

Merged
merged 1 commit into from
Feb 11, 2021

Conversation

MichaelGrupp
Copy link
Contributor

@MichaelGrupp MichaelGrupp commented Feb 1, 2021

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.

As noted in ros-visualization#90, the previously used function was removed in ros_comm.

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

This is very useful. Please merge asap!

@peci1
Copy link
Contributor

peci1 commented Feb 11, 2021

I can confirm this doesn't break Melodic :)

@MichaelGrupp
Copy link
Contributor Author

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

@mjeronimo mjeronimo merged commit b0e0b5e into ros-visualization:master Feb 11, 2021
@wxmerkt
Copy link

wxmerkt commented Feb 11, 2021

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

@peci1
Copy link
Contributor

peci1 commented Feb 11, 2021

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

@mjeronimo
Copy link
Contributor

mjeronimo commented Feb 11, 2021

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

Copy link
Contributor

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tkazik
Copy link

tkazik commented Feb 15, 2021

Thx for the fix! 👍
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
Copy link
Contributor Author

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

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.

None yet

6 participants