-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add precise_time module #839
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
Conversation
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.
Thank you for the early PR! I've added a couple notes
//| Returns an always increasing value of time with an unknown reference | ||
//| point. Only use it to compare against other values from `monotonic`. | ||
//| Unlike time.monotonic, which is a float, the precision of this does not | ||
//| degrade over time. |
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.
What units is this?
#include <stdint.h> | ||
|
||
extern uint63_t common_hal_time_monotonic(void) | ||
extern void common_hal_time_delay_ms(uint32_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.
You'll need semicolons here.
|
Manually tested build at f1ea5d3 |
Some of the Travis builds are failing with
|
The new common-hal routines need to be implemented (or dummied up) for the esp8266 and nrf52 builds as well. |
This is going to be great! |
@margaret please mention us when this is ready for another review. |
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.
see comments
//| :rtype: int | ||
//| | ||
STATIC mp_obj_t precise_time_monotonic(void) { | ||
return mp_obj_new_int_from_uint(common_hal_precise_time_monotonic()); |
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 have not been in the loop on this, so @tannewt and @margaret please let me know if you've discussed this already).
mp_obj_new_int_from_uint()
will silently truncate the common_hal_precise_time_monotonic()
value when it overflows 2^32. 2^32 msecs is about 49.7 days. I could imagine a long-lived sensing program that hits that limit. If the incoming common_hal_precise_time_monotonic()
is > 2^32 you could call mp_obj_new_int_from_ull()
, which takes a long long
(which is 64 bits).
Note that on non-longint builds, py/objint.c
is used, and mp_obj_new_int_from_ull()
throws OverflowError
no matter what it's passed (even the arg would fit in a small int). On longint impls, py/objint_mpz.c
is used. So the value needs to be tested and one or the other needs to be called depending on the size.
I think the behavior on non-longint impls should probably be documented: precise_time.time_monotonic()
will raise OverflowError
when the board has been running for 2^30 msecs or more (about 12.4 days)
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.
We hadn't discussed how to deal with overflow, but these suggestions sound good. I'll work on these items later today.
//| | ||
//| :param int seconds: the time to sleep in milliseconds | ||
//| | ||
STATIC mp_obj_t precise_time_sleep(mp_obj_t milliseconds_o) { |
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.
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.
That makes sense. I'll rename it to sleep_ms
.
ports/nrf/mpconfigport.h
Outdated
@@ -221,6 +221,7 @@ extern const struct _mp_obj_module_t pulseio_module; | |||
extern const struct _mp_obj_module_t busio_module; | |||
extern const struct _mp_obj_module_t board_module; | |||
extern const struct _mp_obj_module_t os_module; | |||
extern const struct _mp_obj_module_t precise_time; |
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.
This gave an error on the nrf build. I think you meant precise_time_module
on line 224 here.
./mpconfigport.h:275:62: error: 'precise_time_module' undeclared here (not in a function); did you mean 'precise_time'?
{ MP_OBJ_NEW_QSTR (MP_QSTR_precise_time ), (mp_obj_t)&precise_time_module }, \
^
../../py/objmodule.c:233:5: note: in expansion of macro 'MICROPY_PORT_BUILTIN_MODULES'
MICROPY_PORT_BUILTIN_MODULES
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
make: *** [build-feather52-s132/py/objmodule.o] Error 1
I restarted the failed jobs and they ran fine. |
I'd hate to do all this work and end up with something that can't keep precise time indefinitely without needing to use longints. 12.4 days to OverflowError on a non-express board is not a great outcome. 12.4 days until your carefully crafted code becomes markedly slower because it switched to 'long's isn't that hot either. To that end, I have two general ideas. Option 1, return a preallocated list of two ints, for example [seconds,microseconds] or [seconds,milliseconds]. Now you're good for 2^30 seconds of runtime, but arithmetic on time values becomes more complicated and needs to allocate lists to do it too. Option 2, operate in modulo mode, such as modulo 2^30, so that the the monotonic time simply wraps after that many milliseconds. You need some helper functions for comparing times (e.g., time 0 has to be treated as less than 2^30-1) and adding times (e.g., adding 30 seconds to 2^30-1 seconds has to wrap), but at least it's all on the efficient-to-store integer type. But you can't say anything about time differences greater than 2^30 milliseconds. These options are pretty half-baked, but maybe there's a good idea in here waiting to be extracted and implemented. |
@margaret I think the simplest thing is to not support This macro can test for it:
|
I revisited this again, and found this interesting new accepted PEP, to be implemented in Python 3.7: https://www.python.org/dev/peps/pep-0564/. See below. The new functions defined in this PEP return integer nanosecond values. We could implement some of these: we still have a wraparound or overflow problem, but using these names allows compatibility with regular Python. We could still have a single Quoting from PEP 564:
|
Note that 2^30 nanoseconds is about 1 second, so on a non-longint built, the new PEP 564 functions aren't really helpful at all. |
It would be really nice to have a function which efficiently returned an int with ms or ns resolution. PEP 564 looks like it would be nice as it is CPython compatible. My only real concern is how efficient is it going to be. Ultimately there is a bunch of cases were basically you want to keep comparing the current time to some known point in time; i.e. have you exceeded the timestamp of your next event time. This is a really common use case. When the events are coming fast and furious, it would be nice if it was highly efficient. I'm inclined to think in addition to the functions which go ahead and allocate at int object (at considerable expense) it might be nice to have one which just compares your previously allocated int to the current tick count (in some useful unit). I'm just musing there. If that is too painful, then the PEP 564 looks best to me, as it is standard-y. It requires some kind of big int support, but unless you want the handstands that utime when through to handle rollover (and things like utimeq), you need big int support anyway. (or at least 60-odd bit support). So the real decisions, IMO, would be PEP 564 + big int vs some kind of utime system where we get time which rolls-over (and all that entails). |
Interest in this topic has revived recently. I did a little more looking and thinking. I think people rarely need msec resolution over weeks, but they need time keeping that doesn't wrap around or that is precise enough to measure relatively short intervals. There are no builtin timing mechanisms that start counting at zero when they're instantiated.
|
@margaret Thanks again for starting this work. I'm closing this PR because we just merged in |
Changes
This adds the
precise_time
module which exposes system ticks as an int.I'm pretty much just going off of
shared-bindings/time/
and https://github.com/adafruit/circuitpython/blob/master/docs/common_hal.mdI basically copy-pasted the
common-hal/time/__init__.*
tocommon-hal/precise_time/__init__.*
. Not sure if there's a way to get theshared_bindings/precise_time
files to use the ones fromcommon-hal/time/
instead.Testing
See comments. Manually tested only on gemma m0 board (that is the board I have).
Status 👀
Addressing review comments.
Edit 2018-06-01: planning to finish this this weekend 🐌
Issue
#519 – More accurate time reporting