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

rosbag migration drops latching=1 field for latched topics. #679

Closed
elliotjo opened this issue Oct 11, 2015 · 2 comments
Closed

rosbag migration drops latching=1 field for latched topics. #679

elliotjo opened this issue Oct 11, 2015 · 2 comments

Comments

@elliotjo
Copy link

When migrating a bag with latched topics, those topics will no longer be latched.

@elliotjo
Copy link
Author

I'd be happy to fix this, but a proper fix would make a few changes to the rosbag.Bag interface so I wanted to get feedback before spending time on it. The migration tool reads messages from the old bag, migrates them, then writes them to the new bag. There is no attempt to maintain the original connection information or an interface to create connection information without digging into Bag's private members. Instead, a new connection header is created for each topic name.

To fix the issue properly, rosbag.Bag will need to be updated to provide mechanisms to receive messages with their connection info, and a way to setup a connection info and write messages associated with it.

The least intrusive fix for reading messages would be to add an optional boolean parameter to read_messages (return_connection_info=False) to request that the connection info is also returned in the tuple for each message.

A new method could be added called setup_connection() to create a new connection info when writing to a bag file where the latching and callerid properties could be specified. The method would return a unique identifier for that connection.

Finally, write_messages() would receive a new optional parameter (connection=None) where you could pass in the unique identifier obtained from setup_connection(). If no connection is specified, write() would continue the default behavior of creating/reusing a new standard connection info.

This approach should leave read_messages() and write() messages with the current behavior so that existing code doesn't break but would allow the migration tool (and any other users) to translate bags more faithfully.

@wjwwood
Copy link
Member

wjwwood commented Oct 12, 2015

@elliotjo That sounds like a fine approach. I haven't reviewed the API in detail to determine if there is a better way to achieve this, but it sounds like you have a fairly good understanding of the issue. To be clear you're referring to the Python bag API right? Also, do you need a unique identifier to communicate the setup_connection() result to write_messages() since they should match based on the topic name? For example, the C++ interface do something like this:

https://github.com/ros/ros_comm/blob/indigo-devel/tools/rosbag_storage/include/rosbag/bag.h#L482-L514

  • If the user passes nullptr for the connection, then
  • search for the connection info by topic name, but if not found then
  • create one from scratch.

So you should be able to just do setup_connection() (as you've proposed it) and then write messages on the same topic without passing any extra connection information at the same time and it will use your custom connection info. Not sure if this lines up with the Python API, because I'm mostly familiar with the C++ API.

ddimarco added a commit to ddimarco/ros_comm that referenced this issue Apr 22, 2018
…#685)

* add parameter to return/write connection header to read/write method

* update rosbag operations to use it
mikepurvis pushed a commit that referenced this issue Apr 24, 2018
* add parameter to return/write connection header to read/write method

* update rosbag operations to use it
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

No branches or pull requests

2 participants