-
Notifications
You must be signed in to change notification settings - Fork 7
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
use std::chrono::time_point for efitimeus_t #16
base: master
Are you sure you want to change the base?
Conversation
@@ -54,5 +59,23 @@ efitick_t getTimeNowNt(); | |||
#define US_PER_SECOND_F 1000000.0 | |||
#define US_PER_SECOND_LL 1000000LL | |||
|
|||
#define MS2US(MS_TIME) ((MS_TIME) * 1000) | |||
#define US2MS(US_TIME) ((US_TIME) / 1000) | |||
template<typename arithmetic_type> constexpr efitimeus_t USOF(const arithmetic_type& us) { |
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.
I would avoid templating the conversion functions. There are scenarios where we want/need to avoid a 64b->float or 64b divide (both are slow as there's no hardware for it).
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.
Function USOF
is actually used only in mock and unit tests. I'm not sure that we should worry about perfomance there. Maybe this header is not a good place for such stuff. But I'm not sure what place is more appropriate for functions to share between lib-time-mocks.cpp
and unit tests. Do you have any suggestion?
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.
@kifir23917 new header something with obviously test name?
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.
I've got rid of template functions and moved conversions float -> int64_t to tests
/** | ||
* 64 bit time in microseconds (1/1_000_000 of a second), since boot | ||
*/ | ||
using efitimeus_t = int64_t; | ||
using efitimeus_t = std::chrono::time_point<std::chrono::system_clock, efidurus_t>; |
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.
it probably doesn't matter, but steady_clock
is more correct
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.
changed to steady_clock
4b5e704
to
6455284
Compare
See alternative approach at FOME-Tech/fome-fw#415 |
Here you can see modifications to rusefi with use of new efitimeus_t |
0d5cbb1
to
2481d39
Compare
see rusefi/rusefi#6409