search

AIT-Assistive-Autonomous-Systems / ros2bag_tools

Tool extensions for ros2bag cli
122 stars 34 forks source link

Add to the CLI arguments of drop #26

Closed bergercookie closed 1 year ago

bergercookie commented 1 year ago

Enable the user to pass multiple topics to ros2 bag drop -t. They can specify t hem sequentially, one after the other -t topic1 topic2 .... The user can also specify the special all - -t all in order to drop messages from all the topics in the given bag.

P.S. Thanks for this very useful tool!

bergercookie commented 1 year ago

Hi @devrite , anything to add in this PR? It's ready to merge from my side

devrite commented 1 year ago

Hi @bergercookie,

no. Just still need to test it and did not have time last week. Maybe one of my colleagues can do it before me.

devrite commented 1 year ago

Hi @bergercookie,

just for a review. It is intended that the counter is not a separate one per topic but counting both topics at the same time?

If the intention was to use a single drop what multiple drops did before we would need separate counters.

Besides I would need write access to the PR (you can allow maintainers todo that). As there were some test flake/pep errors. I already fixed those and would push them.

bergercookie commented 1 year ago

just for a review. It is intended that the counter is not a separate one per topic but counting both topics at the same time? If the intention was to use a single drop what multiple drops did before we would need separate counters.

Yes, it's a bug, thanks for catching it. I'll update the PR accordingly

Besides I would need write access to the PR (you can allow maintainers todo that). As there were some test flake/pep errors. I already fixed those and would push them.

You've already pushed successfully to my branch so I assume you already have this, right?