-
Notifications
You must be signed in to change notification settings - Fork 258
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
Add CLI verb for burst mode of playback #980
Conversation
Signed-off-by: Geoffrey Biggs <[email protected]>
Signed-off-by: Geoffrey Biggs <[email protected]>
ros2bag/setup.py
Outdated
@@ -42,6 +42,7 @@ | |||
'info = ros2bag.verb.info:InfoVerb', | |||
'list = ros2bag.verb.list:ListVerb', | |||
'play = ros2bag.verb.play:PlayVerb', | |||
'burst = ros2bag.verb.burst:BurstVerb', |
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.
maybe alphabetize?
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.
Done in c1f02a6.
"""Burst data from a bag.""" | ||
|
||
def add_arguments(self, parser, cli_name): # noqa: D102 | ||
reader_choices = get_registered_readers() |
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.
Non-blocking - mildly wondering if this could be refactored out into common code, as a lot of it looks like copy-paste from play.py
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.
I agree that it could, but I think I'd prefer to do that in a separate PR.
Signed-off-by: Geoffrey Biggs <[email protected]>
@gbiggs Build fails on code style linter. |
Signed-off-by: Geoffrey Biggs <[email protected]>
I've corrected the linter errors. |
Running CI one more time, after fixing linter errors |
@gbiggs Sorry, now this branch has conflicts with master and can't be merged. |
Running CI after rebase and resolving merge conflicts: |
CI failure in test_play_services__rmw_fastrtps_cpp is well known issue and unrelated to the changes in this PR. |
This PR adds a new verb to the
ros2bag
CLI command,burst
. The new verb runs the burst mode of playback that was recently added torosbag2_transport
'sPlayer
object.A minor change is made to the
burst(num_messages)
API on thePlayer
object: Specifying zero for thenum_messages
argument now means to burst the entire bag, rather than burst zero messages.Something that is not working correctly and that I could use ideas for is how to make the player shut down when the burst is finished. Currently due to the way
burst()
is implemented, the player pauses after the burst and just sits there. Should we add astop()
method to thePlayer
object?