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 rosbag's internal _read_message API #1444

Closed
wants to merge 2 commits into from

Conversation

lalten
Copy link

@lalten lalten commented Jun 26, 2018

This PR fixes the internal rosbag API used by rqt_bag, which was broken with PR #1372

This closes #1432 and ros-visualization/rqt_bag#27

@lalten lalten closed this Jun 26, 2018
@lalten lalten reopened this Jun 26, 2018
@lalten
Copy link
Author

lalten commented Jun 28, 2018

@dirk-thomas I don't understand why the checks failed. It seems like it doesn't have to do anything with the code. Is the Github API is broken?

@VictorLamoine
Copy link
Contributor

VictorLamoine commented Jul 10, 2018

@dirk-thomas Sorry to ping but this issue is quite annoying, it forces to compile ros_comm from sources on every machine where you need rqt_bag

Either this merge request or #1445 does the job.

@VictorLamoine
Copy link
Contributor

VictorLamoine commented Jul 26, 2018

@dirk-thomas @tfoote See previous message
I guess you guys are at the beach 🏖️ 🌞

@dirk-thomas
Copy link
Member

After reviewing the original (already merged) patch (#1372) as well as both proposed fixes (#1445 and this one) I actually thing both should be merged. Additional the API compatibility which was broken in the merged patch should be restored.

I created #1473 with all fixes combined.

@dirk-thomas dirk-thomas added the bug label Aug 3, 2018
@dirk-thomas
Copy link
Member

dirk-thomas commented Aug 3, 2018

Closing in favor of #1473.

@dirk-thomas dirk-thomas closed this Aug 3, 2018
@dirk-thomas
Copy link
Member

Thank you for contributing the patch - the commits are part of the combined PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rosbag - _read_message in bag.py throws exception
3 participants