Conversation
…to MSEC_IN_SEC (name consistency)
peternewman
left a comment
peternewman
left a comment
There was a problem hiding this comment.
Some initial comments, sorry it's taken so long work has been rather busy.
common/utils/Clock.cpp
Outdated
| * load of the system, a prior GOOD state is no guarantee for future proper | ||
| * timing. | ||
| */ | ||
| bool Sleep::CheckTimeGranularity(uint64_t wanted, uint64_t maxDeviation) { |
There was a problem hiding this comment.
This shouldn't be called Check... given it's actually usleeping. Should there not also be one that uses the defined wanting and max deviation times? Test maybe, or ideally a similar synonym.
There was a problem hiding this comment.
It's usleeping but it's only meant to be used to get a rough idea of how accurate usleep is after which further sleeps should be done with Usleep()
(which still remains a total crapshoot no matter what unless we/I find out if there is a way to trigger the program to be run in a real time fashion but afaik that is changing the kernel scheduler settings, so that would just be adding docs that "if the user wants really acurate timings they should do ....")
include/ola/Clock.h
Outdated
| int32_t MicroSeconds() const { return m_interval.MicroSeconds(); } | ||
|
|
||
| int64_t InMilliSeconds() const { return m_interval.InMilliSeconds(); } | ||
| int64_t InMicroSeconds() const { return m_interval.InMicroSeconds(); } |
There was a problem hiding this comment.
Please can you correct the CamelCase of this and it's siblings to InMicroseconds (and throughout the codebase). We'll just add a note to the NEWS file like when we renamed DMXRecieved(sic) to DMXReceived.
There was a problem hiding this comment.
Did this, should I add a note to NEWS or will you once you merge?
| } | ||
|
|
||
| void Sleep::usleep(uint32_t requested) { | ||
| timespec req; |
There was a problem hiding this comment.
This should be using the existing maths in TimeInterval rather than rewriting that code here.
There was a problem hiding this comment.
I'm not sure I follow, we have a TimeIntervalToTimeSpec function somewhere?
There was a problem hiding this comment.
(It does make sense to add such a function but I did not see any such thing listed in the header)
2 participants
Adds InMicroSeconds() as wrapper for AsInt() for naming consistency.
Adds Sleep class with implementation of usleep() and granularity detection
Renames/adds some constants for consistency and use.