-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
esp32/time: make utime conform to CPython #5973
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
I suspect that the 2000/1/1 Epoch was chosen because it makes seconds-since-epoch fit into 31-bit signed integers... |
Thanks for taking these steps, these are good things to discuss and try to address. But because there are lots of subtle things here, additions to the API and breaking changes, they really need to be split in to separate PRs, at least for the things that are independent like
Yes that's correct. It was engineered around the criteria that |
I believe I can switch back to the 2000/1/1 based epoch and make SNTP work. In terms of breaking this up into multiple PRs, please tell me what you'd like broken out. Right now I don't quite see which subsets are usable and testable... |
I hope it will. And I expect, that at this time micros will have an 64 bit architecture. |
Alright, back to 2000/1/1! The changes were actually much simpler than I had thought. I also pulled out all SNTP stuff. I wrote most of a modsntp to use the LwIP SNTP client but then decided that it's too much of a mess in the sense that the code has a slew of |
Can you elaborate on the idea to deprecate this? The RTC is a real peripheral on most MCUs, and you need the ability to set and query it. Some systems will have an external RTC and such things can be exposed as |
Good point. So on some systems, like the esp32, the RTC hardware just provides ticks (and moreover is managed by the RTOS). It really isn't particularly useful as machine.RTC. I believe the following 2 options make the most sense:
On systems where the RTC provides YMDHMS, like stm32, I believe the best option is to leave machine.RTC as-is and document its datetime interface correctly. This means RTC.datetime and utime have overlapping functionality but with different input/output formats. A good question is what to do about daylight savings. Setting aside the issue that there are incompatible implementations and documentation right now, it seems to me that there are two options for machine.RTC:
I personally would vote for option 1. |
As I'm implementing an sntp daemon I'm realizing there's an additional issue, which is that currently |
The mantissa size of float is not sufficient in single precision ports. |
Good point. So maybe adding a |
I added a The time_us() and settime with microseconds calls are necessary to implement an SNTP client, which I've done (in python) and it works well. Overall, this PR is pretty much ready from my POV, I just want to give it another round of testing. Given the request to break this up into multiple PRs I'll wait for further instructions... |
I would like to see the rtc beeing monotonic. however if it's bound to unix timestamp it's going to be non monotonic in the presence of leap seconds ... I don't think newlib is handling that correct. |
If you use adjtime it is monotonic as far as I can tell: https://github.com/espressif/esp-idf/blob/v4.0/components/newlib/time.c |
Just tested and providing feedback: merging this PR into latest master and/or 1.13 results in - when adjusting/setting time using e.g. NTP ( |
That's expected and happens because ntptime sets the RTC directly. With this PR the RTC is managed by esp-idf, so you need to use |
Heads-up that I've mentioned this PR in a discussion trying to pull together all of the different approaches to local time and timezone management, here: |
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 |
Closing due to inactivity (this is still in draft status and has been for a long time). The IDF has since been updated, and also the |
This PR attempts to start fixing some of the issues with
utime
andmachine.RTC
, per #5969, #5553, #5733. The overall idea is to flesh oututime
so it is (a) compatible with CPython and (b) fully-featured to manage the time and (internally) query/set the RTC. TheRTC.datetime
method would then be deprecated but left in-place for code compatibility and possibly for minimal ports that don't haveutime
(I have not looked at that aspect).What this PR currently does is in the esp32 port:
utime.localtime
andutime.mktime
to conform to the CPython standard 9-tuple (well, actually to the 9 11-tuple elements that have numeric indexes).utime.gmtime
andtime.tzset
so one can set a time zone and query UTC as well as local time. The implementation oftzset
does not conform to CPython in that it takes a zone specification whereas in CPython it takes no argument and the zone is set via the TZ environment variable.utime.settime
andutime.adjtime
as extensions WRT CPython so one can step the time or gradually adjust the time.network.SNTP
class with methods to start SNTP, stop SNTP, and get the SNTP status (this is only partially implemented in this PR and is intended to leverage the LwIP SNTP implementation).I believe that this ends up producing a clean interface to time that can be fully implemented on the bigger ports using newlib and LwIP, and that can be stripped down, e.g. without time-zone support, for smaller ports. It somewhat side-steps the incompatibilities between RTC implementations by leaving that class along for legacy use, as well as for minimal ports without
utime
.I volunteer to port this PR to the unix and stm32 ports so these are in-sync.