-
Notifications
You must be signed in to change notification settings - Fork 914
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
keep connection header information in rosbag operations (#679, #685) #1372
keep connection header information in rosbag operations (#679, #685) #1372
Conversation
14a504a
to
51d3842
Compare
Looks like a good change; I've rebased it onto |
Remaining failure on Bionic is a known-flaky test. LGTM. |
Will this be seen in kinetic, or just melodic? |
@@ -1725,7 +1736,7 @@ def __init__(self, bag): | |||
def start_reading(self): | |||
raise NotImplementedError() | |||
|
|||
def read_messages(self, topics, start_time, end_time, connection_filter, raw): | |||
def read_messages(self, topics, start_time, end_time, connection_filter, raw, return_connection_header): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no reason to break the API signature here. The new argument should be made optional. See first commit on #1473.
This PR provides a fix for the issue that
rosbag filter
&rosbag compress
removes latching information from topic connection headers.See #679 for a nice description. This PR is based on that discussion, but follows a slightly different implementation.
You can see the difference by using this script (the script needs the patched code to access the bag headers):