Skip to content

Pause/resume PlayerClock#704

Merged
emersonknapp merged 5 commits intomasterfrom
emersonknapp/player-clock-pause-resume
Apr 12, 2021
Merged

Pause/resume PlayerClock#704
emersonknapp merged 5 commits intomasterfrom
emersonknapp/player-clock-pause-resume

Conversation

@emersonknapp
Copy link
Copy Markdown
Collaborator

@emersonknapp emersonknapp commented Mar 31, 2021 •
edited
Loading

Part of #696

Add set_paused and get_paused functions to PlayerClock and expose to Player.

@emersonknapp emersonknapp mentioned this pull request Mar 31, 2021
Closed
50 tasks
@emersonknapp emersonknapp changed the base branch from master to emersonknapp/player-clock March 31, 2021 21:05
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch from df6e5d8 to f0bb3cf Compare March 31, 2021 21:09
Karsten1987
Karsten1987 reviewed Mar 31, 2021
View reviewed changes
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch from 227a5e7 to e921b13 Compare March 31, 2021 21:54
@emersonknapp emersonknapp mentioned this pull request Apr 1, 2021
Merged
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock branch 2 times, most recently from 5e1012c to 20f37f3 Compare April 2, 2021 07:59
@emersonknapp emersonknapp mentioned this pull request Apr 2, 2021
Merged
1 task
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock branch from 08e530d to daadf5b Compare April 2, 2021 23:27
@delete-merged-branch delete-merged-branch bot deleted the branch master April 3, 2021 02:42
@MichaelOrlov MichaelOrlov mentioned this pull request Apr 6, 2021
Closed
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch from e921b13 to 77161c8 Compare April 6, 2021 23:43
@MichaelOrlov
Copy link
Copy Markdown
Contributor

MichaelOrlov commented Apr 6, 2021

@emersonknapp Could you please rebase your branch on top of master?
I have dependent work in #718 and it's going to be some restructure after rebase.
It would be better to rebase before writing unit tests and polishing jump implementation.

@emersonknapp emersonknapp changed the base branch from emersonknapp/player-clock to master April 6, 2021 23:51
@emersonknapp
Copy link
Copy Markdown
Collaborator Author

emersonknapp commented Apr 6, 2021

@emersonknapp Could you please rebase your branch on top of master?

I read your mind and did it 2 minutes before you asked :)

@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch 2 times, most recently from adab12f to ba1e476 Compare April 8, 2021 00:28
@emersonknapp emersonknapp changed the title [WIP] Pause/resume playback using PlayerClock Pause/resume playback using PlayerClock Apr 8, 2021
@emersonknapp emersonknapp changed the title Pause/resume playback using PlayerClock [WIP] Pause/resume playback using PlayerClock Apr 8, 2021
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch 3 times, most recently from 20c2131 to df78227 Compare April 8, 2021 21:41
Add pause/resume to PlayerClock
cdcd9a3 
Expose it on Player class as well

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch from df78227 to cdcd9a3 Compare April 9, 2021 19:07
@emersonknapp emersonknapp marked this pull request as ready for review April 9, 2021 19:34
@emersonknapp emersonknapp requested a review from a team as a code owner April 9, 2021 19:34
@emersonknapp emersonknapp requested review from Karsten1987 and piraka9011 and removed request for a team April 9, 2021 19:34
@emersonknapp emersonknapp changed the title [WIP] Pause/resume playback using PlayerClock Pause/resume playback using PlayerClock Apr 9, 2021
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch from 5325519 to 3a52671 Compare April 9, 2021 19:44
Add tests, fix functionality problems revealed in test
98079dc 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp emersonknapp force-pushed the emersonknapp/player-clock-pause-resume branch from 3a52671 to 98079dc Compare April 9, 2021 19:49
@emersonknapp emersonknapp changed the title Pause/resume playback using PlayerClock Pause/resume PlayerClock Apr 9, 2021
@emersonknapp emersonknapp mentioned this pull request Apr 9, 2021
Merged
@MichaelOrlov
Copy link
Copy Markdown
Contributor

MichaelOrlov commented Apr 10, 2021 •
edited
Loading

@emersonknapp Looks good to me.
Update: I will need to be able specify time point for snaphsot aka snapshot(time_point); when implementing jump method.
P.S. I can add it in my branch if you don't want to change it in yours.

Add new snapshot signature
71d5d68 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp
Copy link
Copy Markdown
Collaborator Author

emersonknapp commented Apr 12, 2021

I will need to be able specify time point for snaphsot aka snapshot(time_point); when implementing jump method.

I've added a new snapshot(ros_time) signature for this purpose.

Add docstring for sleep_time_while_paused param
ce77997 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
prajakta-gokhale
prajakta-gokhale approved these changes Apr 12, 2021
View reviewed changes
Separate pause tests into smaller individual tests
943a070 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp
Copy link
Copy Markdown
Collaborator Author

emersonknapp commented Apr 12, 2021

Gist: https://gist.githubusercontent.com/emersonknapp/93a1b355caa64dbd15867e203c48dcba/raw/e448dfbf5b37d9fabc6532e4c69b5961c7741736/ros2.repos
BUILD args: --packages-up-to rosbag2 rosbag2_tests rosbag2_cpp rosbag2_transport rosbag2_py
TEST args: --packages-select rosbag2 rosbag2_tests rosbag2_cpp rosbag2_transport rosbag2_py
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/8205

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@emersonknapp emersonknapp merged commit 0c2f80f into master Apr 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/player-clock-pause-resume branch April 12, 2021 18:39
david-prody
david-prody reviewed Apr 12, 2021
View reviewed changes
@emersonknapp emersonknapp mentioned this pull request Nov 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Karsten1987 Karsten1987 Awaiting requested review from Karsten1987

@piraka9011 piraka9011 Awaiting requested review from piraka9011

@anuragdmadan anuragdmadan Awaiting requested review from anuragdmadan

4 more reviewers

@zmichaels11 zmichaels11 zmichaels11 approved these changes

@david-prody david-prody david-prody left review comments

@prajakta-gokhale prajakta-gokhale prajakta-gokhale approved these changes

@jikawa-az jikawa-az jikawa-az approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@emersonknapp @MichaelOrlov @Karsten1987 @zmichaels11 @prajakta-gokhale @jikawa-az @david-prody