-
Notifications
You must be signed in to change notification settings - Fork 412
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 clock state cached time to be a copy, not a reference. #2092
Conversation
That is, in cache_last_msg(), we were just taking a shared_ptr reference. While this is pretty fast, it also means that any changes to that message would be reflected internally. Instead, make a new shared pointer out of that message when it comes in, which essentially causes this to be a copy. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
LGTM wit CI! |
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.
@clalancette lgtm, let me try this fixes the flaky test since i can reproduce this issue my local environment. i want to make sure about this.
Yeah, that would be good to verify. It fixed it for me here, but I will note that |
@clalancette i tried 100 times and no failure observed on that test. thanks for the fix.
yeah, i am aware of that, i will post the issue so that we cannot miss it, lets track this with another issue. on this PR, good to go with green CI 🚀 |
Thanks for double-checking!
Sounds good. I'll open one later today. |
CI is green here, so merging this in. Thanks for the reviews. |
That is, in cache_last_msg(), we were just taking a shared_ptr reference. While this is pretty fast, it also means that any changes to that message would be reflected internally. Instead, make a new shared pointer out of that message when it comes in, which essentially causes this to be a copy.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
This should fix #2088 . FYI @fujitatomoya and @mjcarroll .