Skip to content
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

time is messed-up (RTC and utime) #5969

Open
tve opened this issue Apr 25, 2020 · 21 comments
Open

time is messed-up (RTC and utime) #5969

tve opened this issue Apr 25, 2020 · 21 comments

Comments

@tve
Copy link
Contributor

tve commented Apr 25, 2020

Background: I'm trying to enable lwip's sntp client on the esp32 and it's not going so well... The reason is the EPOCH. The world thinks it's 1970, MP thinks it's 2000. Oops.

I looked at the machine.RTC mess, utime, #5733 and #5553. On an unconnected MP board, such as a pyboard perhaps with a GPS module the existing RTC and utime make a lot of sense. It's very little code, the RTC can be set via pyboard over USB or by app code using the GPS. Thereafter the app can get the time.

On a connected board, such as esp or pybd, the existing RTC and utime make much less sense. The lwip sntp client does what you'd expect it to do and steps or slews the RTC (your choice). There's a good set of std C functions to manipulate time (gettimeofday, mktime, localtime, etc). The machine.RTC module can of course be useful if Wifi is off, etc, but it's kind'a duplicated code. The utime localtime and mktime are a hazard given the they use a different epoch than lwip. On the pybd all this is not a huge issue 'cause there's no SNTP...

I'm not sure how to fix this without spending a lot of work with an uncertain outcome... It seems easier to just write a dynamically loadable native module for the esp32 that uses the lwip sntp and std C time functions and forget about RTC and utime. Thoughts?

@dhylands
Copy link
Contributor

dhylands commented Apr 25, 2020 via email

@tve
Copy link
Contributor Author

tve commented Apr 25, 2020

I understand that there is a constant difference. However, with SNTP enabled the RTC gets set based on 1970 and then calls to, for example, utime.localtime() return nonsense. So yes, I could do something like utime.localtime(utime.time()+delta), but I call that "pretty messed up"...

I also have a dynamically loaded native module that skips RTC and utime and just uses the esp-idf functions to handle sntp and time and completely disregards RTC and utime, but that doesn't make these less messed-up...

@davehylands
Copy link
Contributor

I don't understand. The RTC gets sets using individual fields (year, month, day, hour, minute, second) and not the number of seconds from some epoch. So once it's set you can use utime.time.time() to get the current time and utime.localtime to convert that back into broken out fields.

There might be a timzone difference, but this is also just a constant time difference that you can add/subtract from utime.time().

If you're using SNTP then the RTC time will wind up being on GMT (at least this is what happens on the leboris port) and you'll need to add/subtract your timezone to convert into local time since not of the micropython ports have a notion of timezone.

@tve
Copy link
Contributor Author

tve commented Apr 25, 2020

The LwIP SNTP client sets the RTC on the esp32 directly, not by calling MicroPython's RTC.datetime.

@davehylands
Copy link
Contributor

Right - this is what the leboris port does as well. This typically means that the RTC is in GMT instead of localtime, so you need to adjust the time returned by utime.time() by the number of seconds that your timezone is.

If you wanted to make a patch, then having adding a function which sets/gets a timezone adjustment would be worthwhile (at least in my opinion) and would mean that you wouldn't need to do it in your own code.

@tve
Copy link
Contributor Author

tve commented Apr 25, 2020

Right - this is what the leboris port does as well. This typically means that the RTC is in GMT instead of localtime,

Nope. LwIP SNTP sets the RTC to UTC with an epoch of 1970.

@dpgeorge
Copy link
Member

dpgeorge commented May 3, 2020

The reason is the EPOCH. The world thinks it's 1970, MP thinks it's 2000. Oops.

I would consider changing the MicroPython-epoch to the standard 1970 one.... this is a pretty significant difference from CPython (and the unix world) and has pretty far reaching consequences. And since time in general is hard to get right, this makes it unnecessarily harder.

My opinion now (as opposed to 6 years ago when time.time() was added to the stm port: d6cbbc5) is that, since it's called time.time(), it should return the same logical value as CPython, ie Epoch of 1970. If it can't do that then the function should not exist.

On systems with only 32-bit float it cannot return a float because floats can't represent the number with 1 second accuracy. So such systems would return a big-int. Code would then work "as expected", except you don't get sub-second accuracy. (Although there's an argument that if it can return sub-second accuracy then it's not working as expected from CPython sa shouldn't exist at all, but I think it's an ok compromise to return a big-int if 64-bit floats are not available.)

On systems without big-ints enabled, they cannot include a sensible implementation of time.time().

Note: currently the way to make time.time() compatible with 1970 Epoch is to adjust it by the correct amount (seconds between 1970 and 2000), and this is how the ntptime.py module works. But such an adjustment value is already a big-int, so you necessarily have a requirement on big-int support in the firmware, and also memory allocation for the big-int and its arithmetic operations. So having time.time() already return the value with the 1970 Epoch I think is acceptable from a requires-big-int-and-memory-allocation point of view.

For getting sub-second, absolute time values, @tve suggests in #5973 to add time.time_us() which returns a big-int. I think this is a good idea. Even adding time.time_ns() would also be useful since some things can provide and use such fine resolution.

But then remains the question about getting absolute time values without memory allocation. One way to do that would be an in-place RTC.datetime() method to which you pass a pre-allocated list which the method then populates with year/month/day/etc. An alternative to get the number of seconds since the Epoch of 1970 without memory allocation would be to return 2x ints, a "high" part and a "low" part, but that suffers from eventual overflow of the high part, and isn't very convenient (eg for performing arithmetic, or printing).

To make up for the loss of time.time() which returns seconds without allocating memory, we could add ticks_s() which returns a wrapping value like the other ticks functions.

In summary:

  • time.time() provides seconds since the standard 1970 Epoch, but may allocate memory and may not provide sub-second resolution
  • time.time_us() provides sub-second resolution since the 1970 Epoch, but may allocate memory
  • time.ticks_s() provides a wrapping (relative) time value that is guaranteed not to allocate

(Also, I think all non-CPython functions like ticks_s() and time_us() should be in a separate module, eg ticks).

All the above is a big breaking change so would need to be decided upon and executed carefully.

@peterhinch
Copy link
Contributor

(Also, I think all non-CPython functions like ticks_s() and time_us() should be in a separate module, eg ticks).

If that includes ticks_ms, ticks_us and ticks_diff it will cause utter mayhem breaking a large proportion of existing code. 👎

@tve
Copy link
Contributor Author

tve commented May 3, 2020

[sorry for the wall of text, sadly time is complicated...]

I don't know whether you took a look at #5973 since I reverted back to the 2000 Epoch. It is much less painful than I expected, just a couple of delta constants sprinkled in. The NTP code is similarly a non-issue. The delta between 1970 and 2000 doesn't even appear because a delta between the NTP Epoch (which is not 1970) and the system Epoch is needed so that takes care of everything.

I see two reasons to switch to a 1970 epoch. There's the compatibility with CPython and there's the issue that any short-int solution will overflow in less than 20 years. I haven't encountered the compatibility issue and thus don't have a good sense for the use-cases that hit it. Also, it seems to me that Python does not mandate a specific Epoch, e.g. the CPython docs for time.time() state:

On Windows and most Unix systems, the epoch is January 1, 1970, 00:00:00 (UTC) and leap seconds are not counted towards the time in seconds since the epoch. This is commonly referred to as Unix time. To find out what the epoch is on a given platform, look at gmtime(0).

I find the overflow issue still a bit more theoretical than practical, but it leaves a definitely bad taste behind. Can you perhaps elaborate on what makes you reconsider the change you describe?

Overall it seems to me that it's really important that ticks_ms() does not allocate, but I'm not sure why it's so important that time() doesn't allocate. Time should be used for human interaction, timestamps, and timing on the order of seconds or more. For short term timing, ticks should be used. In any case, the two use different time-bases: ticks always have the same frequency affected only by crystal accuracy and thermal effects; time sometimes runs faster and sometimes runs slower depending on drift WRT NTP or GPS and depending on fun stuff like leap seconds. So over short time-spans ticks are much more accurate and stable, while over long time-spans time is more accurate.

In the context of the above, I'm not sure about "I think all non-CPython functions like ticks_s() and time_us() should be in a separate module, eg ticks." I would rephrase this as "all functions that measure ticks (the vibrations of a crystal oscillator) should be in a separate module from those measuring time, as in unix time which is closely related to but different from NTP time, GPS time, and UTC time." I'm not sure I see the real benefit of such a module split, though.

BTW, if there is a hard requirement for non-allocating time() calls, another option would be to add an optional since parameter: time.time() returns seconds since the system epoch, while time.time(since=<some value>) returns seconds since . This lets application pick their own epoch.

You mention using RTC.datetime to perhaps address the allocation issue. I must say that I really dislike the idea because it mixes responsibilities. An application should either (a) use the RTC as a peripheral, manage it, and refrain from using system time, or (b) use system time and let that manage the RTC peripheral as it sees fit. I.e. you cannot use system time and also place requirements on how it is to manage the time-keeping peripheral.

In terms of going forward, I'm using my PR #5973 as-is in my own fork 'cause I need the functionality to make SNTP work in a sane manner. I'm happy to prepare a different PR that implements, for example, the big-int version you outline. In my mind there are two big issues here:

  1. when can such a breaking change be made, e.g. is that going to be tied up in a long discussion about what MP v2.0 is?
  2. who will implement ports other than esp32, stm32, and unix (which I'm willing to handle)?

@chuckbook
Copy link

I opt for a pragmatic solution:

  • machine.RTC maintains a counter in NSSE format based on the RTC HW. (NSSE: ns since epoch)
  • time.time() returns 64bit float representation of NSSE. This is a 64bit float if available, otherwise a (long) integer at second granularity.
  • machine.RTC.adjust() allows gradual modification of NSSE at variable speed.
  • handling TZ & DST would be nice, but I'm afraid it would be far to bloated to implement full localtime functionality. A poor-man's variant might be feasible, specifying offsets in seconds as hours would be too coarse.
  • All existing ticks* methods maintain their behavior.
  • Provide adequate _into methods either implicit or explicit.
  • ticks*_into methods derive their range from destination parameter.
  • Make epoch parameter configurable.

An incomplete list of pros & cons:
Pro:

  • Maintain compatibility with current functionality.
  • Enable countermeasures against year 2038.
  • Allow handling of leap seconds.
  • Usually RTC oscillator is the most precise clock in a low-cost/embedded system.
  • Precise timestamps without extensive overhead.

Con:

  • Ugly semantics
  • More overhead
  • Inadequate for systems lacking reasonable RTC.

@tve
Copy link
Contributor Author

tve commented May 3, 2020

  • machine.RTC maintains a counter in NSSE format based on the RTC HW. (NSSE: ns since epoch)

The RTC hardware on stm32 does not allow NSSE, it consists of BCD counters for YMD-HMS.sss

  • time.time() returns 64bit float representation of NSSE. This is a 64bit float if available, otherwise a (long) integer at second granularity.

NTP and adjtime really need sub-second resolution to get sub-second sync

  • machine.RTC.adjust() allows gradual modification of NSSE at variable speed.

The hardware may not support that, e.g. esp32, esp8266.

  • handling TZ & DST would be nice, but I'm afraid it would be far to bloated to implement full localtime functionality. A poor-man's variant might be feasible, specifying offsets in seconds as hours would be too coarse.

The ESP-IDF already contains the code, you're already paying for it.

  • Provide adequate _into methods either implicit or explicit.

Can you elaborate?

@peterhinch
Copy link
Contributor

I think @tve is right about system time vs RTC. RTC represents a peripheral. A piece of hardware which in some cases is more accurate than system clock, in others (ESP8266) is lousy. Some users install accurate RTC devices - another reason to see them as peripherals.

It would be good if there were an interface spec for an AbstractRTC. I can also see merit in a means of monotonically adjusting the system clock from an AbstractRTC and vice versa.

@tve
Copy link
Contributor Author

tve commented May 3, 2020

It would be good if there were an interface spec for an AbstractRTC.

I think the biggest issue here is that there are two fundamentally different types of hardware devices. There are clock tick counters and there are calendar counters. The STM32 RTC and that DS13xx (forget the exact number) fundamentally count years, months, days, hours, seconds, fractions. Getting a seconds-since-epoch requires going through the equivalent of mktime. The esp and nrf RTC fundamentally count clock ticks, i.e. seconds-since-epoch and getting YMD-HMS requires going through the equivalent of gmtime. So at some level, you would really need a machine.Date peripheral or a machine.Clock peripheral depending on port...

@chuckbook
Copy link

Let's distinguish between RTC as peripheral and RTC as a virtual device. In my point of view the current RTC module as used in MP already implements a virtual device. I agree that naming is quite confusing and I don't care how the virtual RTC is called as long as it provides a monotonic 'real-time' with sufficient precision, resolution and value range. A 64bit counter can do a pretty good job.
The API should allow easy to use integer timestamps in several resolutions and ranges that can be used within IRQ service routines. This can be done by providing dedicated methods like timestamp_ms_into(dest) or by extending existing methods like ticks_ms() to accept an optional dest parameter.
Of course a reasonable method to synchronize the 64bit counter to other sources (NTP, mains frequency, 1PPS, RTC peripherals, DCF-77 etc.) is required.
AFAIK Damien already did a prove of concept that allows at least SYSCLK resolution using STM32 RTC.
Re: TZ/DST Handling
I strongly recommend not to include this within MP as long as no compact and flexible solution can be provided that supports at least fractional offsets and multiple DST levels. An adequate (frozen) Python module would be a better place for this feature.

@hardkrash
Copy link

I feel that time.monotonic with a returned float would cover most use cases for timing with less than second resolution. It is my most missed function from the standard time module. I have started making an async task that maintains a monotonic clock built using ticks_ms.

For time.time allocating memory it seems this would be most problematic in an interrupt context; this is probably where ticks_xx and the support functions probably make most sense. Making time.time a special case in an interrupt seems to fit with other restrictions in the interrupt space. e.g. A rule that says only ticks_xx are safe in interrupts, avoid other timekeeping functions from utime

@dhalbert
Copy link

dhalbert commented May 8, 2020

We have had time.monotonic() return a float in CircuitPython for years, because we have a policy of being a subset of CPython. The main issue has been the decreasing accuracy over a long period due to the limited size of the floating-point mantissa. For more precise timing, we added .monotonic_ns(), but that needs longints and causes allocations.

Note this issue about avoiding double-precision floating-point when computing a float time, which can sneak in by accident and bloat the firmware: adafruit#343.

Other CircuitPython discussions that may be of interest:
adafruit#519
adafruit#839

@tve
Copy link
Contributor Author

tve commented May 8, 2020

We have had time.monotonic() return a float in CircuitPython for years
For more precise timing, we added .monotonic_ns()

Thanks for the pointers, I need to get into the habit of checking CP when fixing stuff!

Question: other than the type of the return value, is this really ticks without roll-over? I.e., which time-base does it use? The python docs don't mention the difference between processor clock ticks and real-time...

@dhalbert
Copy link

dhalbert commented May 8, 2020

Question: other than the type of the return value, is this really ticks without roll-over? I.e., which time-base does it use? The python docs don't mention the difference between processor clock ticks and real-time...

Right, no rollover. We were using SysTick for timekeeping, and generating an interrupt on each ms and incrementing a ticks_ms timer. We had reports of it losing timein some cases (maybe due to lost interrupts), which wasn't good, but hadn't tracked down that problem yet. However, we just recently started using whatever RTC is on the chip to do timekeeping, so that problem is somewhat moot.

@tve
Copy link
Contributor Author

tve commented May 8, 2020

If you're using the RTC, how do you keep time.monotonic actually monotonic when the real-time clock needs to be adjusted?

@dhalbert
Copy link

dhalbert commented May 8, 2020

If you're using the RTC, how do you keep time.monotonic actually monotonic when the real-time clock needs to be adjusted?

On atmel and nrf, the RTC is a counter that is not reset. We just keep an offset. On STM, there may be a bug, since the RTC is a real calendar clock. We should keep an offset if we don't.

@dpgeorge
Copy link
Member

This is a good discussion but there are lots of points to follow... I'm still of the opinion that time.time() should be made compatible with CPython and return seconds since the 1970 Epoch (either 64-bit float or big-int). That's a concrete change that can be made.

It might also be useful to add time.monotonic_ns(), per CPython specs. It would return a big-int.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants