-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
nrf: Implement time.time() and machine.RTC #13340
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #13340 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21200 21200
=======================================
Hits 20860 20860
Misses 340 340 ☔ View full report in Codecov by Sentry. |
|
Code size report:
|
I just see that approximately the same thing has already been proposed in #10400. It looks like it would need some updates for things that have gone on in master in the meantime; other than that, at a first glance, I don’t have a strong opinion on which is better. It has an additional check for whether the epoch offset is valid, but I haven’t spent the thought yet to understand in what scenario that is useful. |
This is an automated heads-up that we've just merged a Pull Request See #13763 A search suggests this PR might apply the STATIC macro to some C code. If it Although this is an automated message, feel free to @-reply to me directly if |
Rebased on master to deal with the STATIC removal, no other changes required. |
Thanks for the contribution. As you noted above, #10400 also added In the meantime can you please rebase on the latest master, to pick up the code formatting and LFCLK fix commits that are now merged. |
Thanks, rebased as you suggest. Not tested yet, will do that in a few days. |
Board definitions could already define BLUETOOTH_LFCLK_RC or not to choose whether to use the internal RC oscillator or an external 32kHz crystal as the source for LFCLK, but that setting was only applied when the softdevice was enabled by ble.enable(). Apply it from the start, so that the functions of the time module always have the specified accuracy and run without interruption when the softdevice is enabled. Also add an option for the third LFCLK source, synthesized from the HFCLK, by defining BLUETOOTH_LFCLK_SYNTH. Signed-off-by: Christian Walther <cwalther@gmx.ch>
A board that uses BLUETOOTH_LFCLK_SYNTH is expected to have a 32MHz crystal (Do any without one exist at all? I guess not because it's required for Bluetooth, which nRF microcontrollers are usually chosen for.), otherwise BLUETOOTH_LFCLK_RC would be more appropriate, so start the crystal oscillator together with LFCLK to have accurate timekeeping from the start. Signed-off-by: Christian Walther <cwalther@gmx.ch>
Optionally adds time() and time_ns() to the time module, as well as a machine.RTC class that only implements the datetime() method (following the example of the rp2 port), whose sole purpose is to provide the ability to set the time. Also provides the basis for enabling gmtime(), localtime(), mktime() in the time module. The nRF52 does not have a dedicated real-time clock peripheral, but timekeeping can be done by the same real-time counter that already powers the time.ticks_* and related functions. For reasonable accuracy, a suitable LFCLK source is required: The internal RC oscillator (BLUETOOTH_LFCLK_RC) by itself is insufficient, but any of the following work fine: - external 32kHz crystal (default) - synthesis from HFCLK (BLUETOOTH_LFCLK_SYNTH) when HFXO (external 32MHz crystal) is enabled - BLUETOOTH_LFCLK_RC + periodical calibration from HFXO (automatically done by the SoftDevice while enabled using ble.enable()) Boards can enable this by defining both configuration options MICROPY_PY_TIME_TICKS and MICROPY_PY_TIME_TIME_TIME_NS. Additionally, they may want to enable MICROPY_PY_TIME_GMTIME_LOCALTIME_MKTIME. This includes a generic implementation of mp_time_localtime_get() and mp_time_time_get() in terms of mp_hal_time_ns(), which could also be used by other ports. In particular by the embed port, for which I originally wrote it, noting the following: "I'm unsure where to put modtime_mphal.h, it could also be in extmod. The important thing is that for MICROPY_PY_TIME_INCLUDEFILE to work it must be at the same path in both the port build (original source tree) and the application build (micropython_embed distribution), therefore not in ports/embed/port. It is named .h, mismatching the corresponding ports/*/modtime.c, because it must not be compiled separately, which naming it .c would make harder for users of the embed port - they would need to explicitly exclude it, whereas this way they can continue to just compile all the .c files found in the micropython_embed distribution except those in lib." Signed-off-by: Christian Walther <cwalther@gmx.ch>
I have added another commit in the middle, an extension of the first one (could be squashed into it): On a board that uses HFXO could be started by the user program later, but there is no Python API for doing only that, only an arcane I considered doing it in Also rebased on master again for good measure. |
Optionally adds
time()
andtime_ns()
to thetime
module, as well as amachine.RTC
class that only implements thedatetime()
method (following the example of the rp2 port), whose sole purpose is to provide the ability to set the time. Also provides the basis for enablinggmtime()
,localtime()
,mktime()
in the time module.The nRF52 does not have a dedicated real-time clock peripheral, but timekeeping can be done by the same real-time counter that already powers the
time.ticks_*
and related functions. For reasonable accuracy, a suitable LFCLK source is required: The internal RC oscillator (BLUETOOTH_LFCLK_RC
) by itself is insufficient, but any of the following work fine:BLUETOOTH_LFCLK_SYNTH
) when HFXO (external 32MHz crystal) is enabledBLUETOOTH_LFCLK_RC
+ periodic calibration from HFXO (automatically done by the SoftDevice while enabled usingble.enable()
)Boards can enable this by defining both configuration options
MICROPY_PY_TIME_TICKS
andMICROPY_PY_TIME_TIME_TIME_NS
. Additionally, they may want to enableMICROPY_PY_TIME_GMTIME_LOCALTIME_MKTIME
. I have not enabled it on any of the existing boards, because my main use case is an external one (and I don’t have any of them to test), but can do that if desired.This includes a generic implementation of
mp_time_localtime_get()
andmp_time_time_get()
in terms ofmp_hal_time_ns()
, which could also be used by other ports. In particular by the embed port, for which I originally wrote it in #11430 (82b8679), noting the following:Tested on nRF52832 (DS-D6) and nRF52840 (ItsyBitsy).
The first two commits are #13339, included here because separating them would create a text conflict with the third commit.Thethirdfirst two commits are not strictly required, but helpful. Thefourththird commit is the main change.