Misc dds & rsutils #13704
Misc dds & rsutils #13704
Conversation
| { | ||
| namespace time | ||
| public: | ||
| stopwatch() { _start = clock::now(); } |
There was a problem hiding this comment.
Better to use initializer list
| } | ||
|
|
||
| // Block until the event is set() | ||
| // If already set, returns immediately | ||
| // The event remains set when returning: it needs to be cleared... | ||
| void wait() const | ||
| void wait() |
There was a problem hiding this comment.
Why did you remove the const? Semantically we don't expect wait to change internal event state
There was a problem hiding this comment.
Hmm I guess wait() waits for someone else to change it - even though you're expecting a change, you're no changing it yourself...
So I'll try putting back the const.
| { | ||
| std::unique_lock< std::mutex > lock( _m ); | ||
| if( ! _is_set ) | ||
| _is_set = ( std::cv_status::timeout != _cv.wait( lock, timeout ) ); | ||
| _is_set = ( std::cv_status::timeout != _cv.wait_for( lock, timeout ) ); |
There was a problem hiding this comment.
I don't think we should assign _is_set here, same way we don't assign in wait() function.
There was a problem hiding this comment.
And then the function can stay const
| @@ -90,7 +93,7 @@ class event | |||
| { | |||
| std::unique_lock< std::mutex > lock( _m ); | |||
| _is_set = false; | |||
| _is_set = ( std::cv_status::timeout != _cv.wait( lock, timeout ) ); | |||
| _is_set = ( std::cv_status::timeout != _cv.wait_for( lock, timeout ) ); | |||
There was a problem hiding this comment.
Same, don't assign _is_set here.
There was a problem hiding this comment.
OK, but note that line 95 still is needed. And this function is not const.
Separated a few commits from my upcoming PR...
The functions in timer and stopwatch has bad
constdesignations... I changed those. Sorry about the formatting...