Add convert verb

[Kettenhoax]

Adds a convert verb to convert bags between storages and serialization formats. Tested with a conversion of rosbag2_bag_v2 to sqlite3, but I was unsure whether to add a bag to the repository for an integration test, because it would require rosbag2_bag_v2 as test dependency.

Should close #163

[Karsten1987]

Thanks for the PR. A convert tool is highly appreciated.

I am requesting changes though as I believe that this logic should not live in rosbag2_transport but can rather be handled solely with rosbag2_cpp API.

[Karsten1987]

year 2020

[Karsten1987]

same below

[Karsten1987]

for a convert verb I would consider renaming this to --input-storage to highlight the conversion from input to output.

[Karsten1987]

I am not completely sure a default for input makes sense here. The storage identifier might be guessed from the metadata.yaml if present, but otherwise this has to be specified.
It makes more sense for me to have it defaulted to sqlite3 on the output side.

[Kettenhoax]

In current master, the play and info verbs also set the input storage argument default to sqlite3, so I thought it best to keep the convention. I agree that it should be guessed, but the necessary api doesn't seem to be available in python, and IMO shouldn't be added in this PR.

I would rename it to --input-storage, remove the default and set required=True. I would also remove the short -s and not add -i to avoid confusion with -o, which is a filepath rather than storage specifier.

[togaen]

@Kettenhoax @Karsten1987 Any updates on this? It would be really great to have. I happy to help out if I can.

[Kettenhoax]

I'm still interested in merging this and have been waiting on #232, which seems to add the required Python API to implement the requested changes.

[jacobperron]

I'm also interested in this.

@Kettenhoax, #232 has been resolved. There's now a basic Python API. I'm not sure if it contains all the functionality needed for this feature, but I'm happy to help add/review PRs adding what's missing.

[jacobperron]

@Kettenhoax Friendly ping. Let me know if you have plans to work on this, otherwise, I may have some spare cycles soon to help push it forward.

[Kettenhoax]

Yes, I can work on this next few days.

@Karsten1987 or @jacobperron Looking at the current master, could you clarify if the preferred way to implement this is either

1. using the existing Reader and Writer api in rosbag_py to iterate over messages in the verb extension point, or
2. add a Converter api to rosbag_py and invoke it in ros2bag

In both cases as dependency on rosbag_py will need to be added to ros2bag.

[jacobperron]

@Kettenhoax For me, adding a converter API to rosbag2_py is the preferred option. This allows other Python applications to take advantage of the functionality without having to go through the CLI verb.

[jacobperron]

@Kettenhoax When adding the new API to rosbag2_py, it would be best to do it in a separate PR from the changes to the CLI. This will help with the review process and maintenance 🙂

[jacobperron]

@Kettenhoax Friendly ping 🙂 Any update?

[Kettenhoax]

Yes, I've started to extend rosbag2_py API by a converter. However, I've run into roadblocks when implementing compression. Currently a caller of rosbag2_py decides whether to instantiate a compression- or standard reader/writer. The way I think this API is meant to be used, a converter should be passed an already-instantiated reader and writer.

As PoC I tried to do this in the verb only, without extending rosbag2_py. Unfortunately rosbag2_py.SequentialCompressionWriter seems to lack a parameter for the compression options.

We also need a python-exposed API to determine whether a bag is compressed and instantiate the correct reader. Currently this is only done in rosbag2_transport_python.cpp.

@jacobperron It would be great if you could do the necessary rosbag2_py extensions. You can check da6c184 for how I'm trying to use rosbag2_py

[emersonknapp]

A note of interest - we have plans to consolidate the compression/standard readers/writers - having realized that the split mostly leads to code duplication that is hard to maintain. Unfortunately that work hasn't started yet - I know it would make this easier

[ZhenshengLee]

Any update?

[emersonknapp]

Closing this as stale in favor of the work going on by @gbiggs for #831 - that work will implement most if not all of this proposal.