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

Rosbag timestamp changes. #456

Closed

Conversation

rodericktaylor
Copy link

Didn't compile against rolling, had to change this to get it to compile.

@wep21
Copy link
Collaborator

wep21 commented May 23, 2024

@rodericktaylor There is another member named recv_timestamp. Is using send_timestamp here valid? cc @Ryanf55

@wep21 wep21 requested review from Ryanf55 and wep21 May 23, 2024 16:53
Copy link
Collaborator

@wep21 wep21 left a comment

Choose a reason for hiding this comment

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

send_timestamp falls back to recv_timestamp when it's not available, so the change looks good.

Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

LGTM. Rolling looks broken, but I'll merge this once I can figure out how to get it running again. Thanks for the contribution!

@wep21
Copy link
Collaborator

wep21 commented May 26, 2024

@rodericktaylor Thank you for your contribution. Your commit is merged in another PR. Is it alright to close this PR? cc @Ryanf55

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jul 19, 2024

Closing.

@Ryanf55 Ryanf55 closed this Jul 19, 2024
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.

None yet

3 participants