Add return_connection_header=False arg

[efernandez]

Add return_connection_header=False arg to seek_and_read_message_data_record, for BagReady102.

Otherwise we get this error (e.g. using rqt_bag):

  File ".../lib/python2.7/dist-packages/rosbag/bag.py", line 1299, in _read_message
    return self._reader.seek_and_read_message_data_record(position, raw)
    TypeError: seek_and_read_message_data_record() takes exactly 4 arguments (3 given)

This bug was introduced by @ddimarco in 752cf1f, that addressed #1372

rqt_bag is impacted by this. I can't see the contents of any message.

@mikepurvis for review, since he merged it and should know better the context of the original change.

[mikepurvis]

I see the problem— thanks for tracking this down. Yeah, it definitely should have been a default value param.

LGTM.

[efernandez]

The debian stretch test failed on the sub_pub, unrelated test for this PR, and for other recent ones: http://build.ros.org/job/Mpr_ds__ros_comm__debian_stretch_amd64/

I wonder if this could be merged despite of that failure. @dirk-thomas

BTW I don't really understand why the BagReady102 class is being used. I thought that the bags I have (generated recently with melodic-devel) would be a newer version (200, I think).

[efernandez]

I didn't see #1444 when I created this, which provides a slightly different fix.

@mikepurvis @dirk-thomas I guess you can choose one to merge and close the other.

[VictorLamoine]

@efernandez ros-visualization/rqt_bag#27, just in case you didn't see the issue

[efernandez]

Thanks @VictorLamoine , this addresses ros-visualization/rqt_bag#27

Just waiting for review.

[VictorLamoine]

@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 #1444 does the job.

[DLu]

👍

[dirk-thomas]

After reviewing the original (already merged) patch (#1372) as well as both proposed fixes (#1444 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]

Closing in favor of #1473.

[dirk-thomas]

Thank you for contributing the patch - the commit is part of the combined PR.