-
Notifications
You must be signed in to change notification settings - Fork 162
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
Implement rcl_clock_time_started #1021
Conversation
b3c2184
to
d3a766b
Compare
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.
Looks good to me!
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.
although this change itself looks good to me, i would suggest that the following procedure.
- revert Implement validity checks for time points #1018 only, and merge this revert commit in rolling.
- then apply
rcl_clock_time_started
.
it is up to package maintainers, but highly recommended. each PR should have complete fix as less dependencies as possible.
Fair point! #1022 I've reconfigured this PR @fujitatomoya , the revert change is split off. I will merge on green CI |
3aaab01
to
78f1934
Compare
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.
lgtm with one comment
Signed-off-by: methylDragon <methylDragon@gmail.com>
7e31df0
to
51a0841
Compare
@Mergifyio backport humble |
✅ Backports have been created
|
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> (cherry picked from commit 09d86e2)
This PR reverts #1018 and implements
rcl_clock_time_started
instead, which checks if anrcl_clock_t
has started (i.e., outputs a non-zero time when queried for current time.)This was from an internal discussion that concluded that checking for time validity on a time point doesn't make sense (std::chrono's time points have no concept of validity, a time point reflecting (0, 0) is valid.) Instead, what matters is if a clock has a time point or not.