AIT-Assistive-Autonomous-Systems / ros2bag_tools

Tool extensions for ros2bag cli
122 stars 34 forks source link

Add sync-timestamp filter to use first topic #17

Closed wallner90 closed 1 year ago

wallner90 commented 1 year ago

Added option to unify all written files to use the timestamp of the first given topic.

devrite commented 1 year ago

@wallner90 Just recalled that we need to check the order of topics. The synchronization filters are created as a dict (topic: filter) and whatever order comes out of it will be the order of the message, which may conflict with the order on the command line. Besides the sync_filters are first checked on presence in the meta-data of the bag, which also may have a different order.

https://github.com/Kettenhoax/ros2bag_tools/blob/18127e9a48f54b11b6a4605ae930d9efc450b047/ros2bag_tools/ros2bag_tools/filter/sync.py#L121-L122

From Python 3.7 onward dictionaries are ordered in insertion order: https://docs.python.org/3.7/tutorial/datastructures.html#dictionaries

Currently (humble onwards) only RHEL requires support for a version < 3,7.

So for less compatibility issues, I suggest first creating the Async-Filters as a list based on args.topics, then the sychronizer and then creating a dict of them (zip(args.topic, filters)) as before.

devrite commented 1 year ago

@Kettenhoax LGTM!

wallner90 commented 1 year ago

Still one change open.

wallner90 commented 1 year ago

Done!

Kettenhoax commented 1 year ago

LGTM!