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

Fix pause snapshot behavior and add regression test #730

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Apr 12, 2021

The test fails without the change - the snapshot behavior was just not correct. It would update ros_now on resume - so a paused clock would go, e.g. 1, 2, 3, 4, 4, 4, 4, 8, 9, 10 instead of 1, 2, 3, 4, 4, 4, 4, 5, 6, 7

@emersonknapp emersonknapp requested a review from a team as a code owner April 12, 2021 23:14
@emersonknapp emersonknapp requested review from mjeronimo and prajakta-gokhale and removed request for a team April 12, 2021 23:14
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
Copy link

@david-prody david-prody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now

@emersonknapp
Copy link
Collaborator Author

Gist: https://gist.githubusercontent.com/emersonknapp/9fbe44201d17ecc4eeff40ea50c4a860/raw/af9ad57628753bb0ed782c4465c8ef3635c0a809/ros2.repos
BUILD args: --packages-up-to rosbag2_cpp
TEST args: --packages-select rosbag2_cpp
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/8209

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

@emersonknapp emersonknapp merged commit 9d42372 into master Apr 13, 2021
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/pause-additional-test branch April 13, 2021 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants