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

Delay option for ros2 bag play #786

Closed
hayato-m126 opened this issue Jun 22, 2021 · 10 comments
Closed

Delay option for ros2 bag play #786

hayato-m126 opened this issue Jun 22, 2021 · 10 comments
Labels
enhancement New feature or request

Comments

@hayato-m126
Copy link

hayato-m126 commented Jun 22, 2021

Feature request

Add a commandline option --delay, which sleeps for the specified amount of time before starting to publish messages.

This delay should also happen in between loops, when looping playback.

Original Ticket

I want to use the delay option of bag play that was implemented in ROS1, but it seems that it is not implemented in ROS2.
Is there any reason why it is not implemented in ROS2?

TierIV implemented the delay option in the forked repository below, if we submit a pull request to this official repository, will it be accepted?

https://github.com/tier4/rosbag2/pull/3/files

@emersonknapp emersonknapp added the enhancement New feature or request label Jun 22, 2021
@emersonknapp
Copy link
Collaborator

Is there any reason why it is not implemented in ROS2?

No reason, just hasn't been built

if we submit a pull request to this official repository, will it be accepted?

Assuming we iron out anything that comes up during code review, yes!

Note: i've edited your original ticket (leaving all the original context) to lay out the feature request. Please check that it matches what you have in mind.

@hayato-m126
Copy link
Author

Thank you for reply.
The edited issue is as I intended.

@MichaelOrlov
Copy link
Contributor

@hayato-m126 Could you please clarify what a use cases for usage this delay API?

@hayato-m126
Copy link
Author

hayato-m126 commented Jul 2, 2021

@MichaelOrlov
The delay option is assumed to be used when calling ros bag play from ros launch.
When ros bag play is launched with multiple nodes such as sensing, perception, localization, etc., sometimes data be output before each node is fully initialized, so this option is used to delay data playback.

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jul 2, 2021

@hayato-m126 I would recomend to implement wait_for_matched API for Player class instead for your cases.

@hayato-m126
Copy link
Author

@MichaelOrlov
Do you mean to implement an option to wait for playback based on generic conditions, not just time?
If so, what are the possible use cases for using conditions other than time?

Since delay is an option that existed in ROS1, Tier4 simply implemented the same functionality in the pull request associated with this issue.

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jul 3, 2021

@hayato-m126 I understand that you are trying to repeat functionality from ROS1.
However there are exists possibility to wait for matched number of subscribers for publisher rather than just blind delay.
i.e. Player will not start playback until publisher will discover number of specified subscribers. It means until subscribers nodes will be initialized and visible for publisher via discovery.
We have implemented some similar functionality for unit tests. Please refer to the wait_for_matched in PublicationManager

However usage of the wait_for_matched API for Player class via ros launcher might require to pass many parameters like number of matching nodes per topic and timeout. Maybe need to consider to pass them via some config file.
I would assume that wait_for_matched would fully replace delay feature.

@hayato-m126
Copy link
Author

@MichaelOrlov
I read the unit test code you showed me.

I understand that wait_for_matched is suitable for the purpose of waiting for the subscriber to be ready, although the way of specifying the condition is a bit complicated.

Thank you for your advice.

@hayato-m126
Copy link
Author

I close this issue because the linked PR has been merged.

@yinhaosen
Copy link

@hayato-m126 I understand that you are trying to repeat functionality from ROS1. However there are exists possibility to wait for matched number of subscribers for publisher rather than just blind delay. i.e. Player will not start playback until publisher will discover number of specified subscribers. It means until subscribers nodes will be initialized and visible for publisher via discovery. We have implemented some similar functionality for unit tests. Please refer to the wait_for_matched in PublicationManager

However usage of the wait_for_matched API for Player class via ros launcher might require to pass many parameters like number of matching nodes per topic and timeout. Maybe need to consider to pass them via some config file. I would assume that wait_for_matched would fully replace delay feature.

Does "blind delay" here mean users usually don't know how much time needed to wait?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants