8000 ports/rtc: Inconsistencies between ports and the documentation. · Issue #10578 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

ports/rtc: Inconsistencies between ports and the documentation. #10578

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.

Sign up for GitHub

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

Jump to bottom
Closed
robert-hh opened this issue Jan 24, 2023 · 9 comments
Closed

ports/rtc: Inconsistencies between ports and the documentation. #10578

robert-hh opened this issue Jan 24, 2023 · 9 comments
Labels
ports Relates to multiple ports, or a new/proposed port

Comments

@robert-hh
Copy link
Contributor
robert-hh commented Jan 24, 2023 •
edited
Loading

It looks like the machine.rtc() API is different for all ports and to the documentation.

  • to start with the good part: In all ports rtc.datetime() matches the machine.RTC class documentation.
  • rtc.init() for the cc3200 works as documented
  • rtc.init() differs from the documentation in the item order of the datetime tuple for the ports esp32, samd, mimxrt.
  • no rtc.init() at the rp2 port.
  • rtc.init() at the stm32 and renesas.ra port just reinit's the RTC device and ignores the function argument.
  • rtc.now() is only available in the cc3200 and mimxrt port.
  • the documentation for rtc.datetime() is different between the machine.RTC class section and the quickref for the ports stm32, rp2, mimxrt, esp32, esp8266, renesas-ra,

So it seems as if the documentation of rtc.init() and rtc.now() only matches the cc3200 port. I can make that all consistent, but we should agree first on which class methods should be available. So a simple approach would be:

  • keep rtc.datetime() as it is now. Maybe add it to the cc3200 port. It is not consistent to CPython datetime.datetime, but that topic has been discussed several times already with the decision to keep the actual rtc.datetime format.
  • drop rtc.now() from the mimxrt port. Only keep it at the CC3200 port for legacy.
  • drop the set date/time functionality of rtc.init() and/or change at least the documentation.
  • fix the inconsistencies in the documentation.

Edit: Created PR #10607
@zzzbatmand zzzbatmand mentioned this issue Jan 25, 2023
Closed
@peterhinch
Copy link
Contributor

You've probably seen #5553. Fixing this is long overdue but it will break code.

Should the rtc.datetime tuple match the format of time.localtime? (This would break even more code, though.)

@robert-hh
Copy link
Contributor Author

The section of the documentation which you changed is the only one which matches rtc.datetime(). But all the quickref paragraphs are wrong.My preference is in the post before in the last four bullets. I do not suggest to change rtc.datetime(), since it seems the only part which is consistent over the ports, if implemented at all.

@zzzbatmand
Copy link
zzzbatmand commented Jan 25, 2023 •
edited
Loading

How about adding a rtc.localtime that follow the time.localtime format?
This way, there is a function that follows the time format, and don't break any existing code.

@robert-hh
Copy link
Contributor Author

Why? rtc.datetime() and time.localtime() use the same hardware clock, if it exists. So there is no benefit in having a rtc,localtime().

@zzzbatmand
Copy link

What I really want, is a simple way of setting the RTC using something like Unix Timestamp.
It is possible to use time.gmtime to convert seconds to a datetime format, but this don't fit with the rtc.datetime as stated.

An alternative solution is make a function like rtc.set_epoc or similar.
Then even libraries that can get the time, can be converted to epoc and used to set the RTC.

@robert-hh
Copy link
Contributor Author

See https://github.com/micropython/micropython-lib/tree/master/micropython/net/ntptime

@robert-hh
Copy link
Contributor Author
robert-hh commented Jan 25, 2023 •
edited
Loading

For the SAMD port I had made an extension for boards without RTC, that you could supply a time value as seconds since epoch as argument to time.localtime() to set the time. But that's non-standard.

@robert-hh
Copy link
Contributor Author

@dpgeorge Do you have a preference about how to proceed with reducing the inconsistencies of machine.RTC. I can at least make an initial PR to fix the documentation of rtc.datetime(), assuming that this method will not be changed.

@robert-hh robert-hh mentioned this issue Jan 28, 2023
Merged
@villeneuve
Copy link

Hi!
Correct me if I'm wrong, but I see a mistake in the doc for the rp2 RTC. We can read:
rtc.datetime((2017, 8, 23, 2, 12, 48, 0, 0)) # set a specific date and time, eg. 2017/8/23 1:12:48

The code works ok, no problem. But the comment should be read: #... eg. 2017/8/23 12:48

As per the class RTC doc, The 8-tuple has the following format: (year, month, day, weekday, hours, minutes, seconds, subseconds)

So the time set is 12:48:00 not 1:12:48

@robert-hh robert-hh mentioned this issue Oct 5, 2023
Merged
@dpgeorge dpgeorge added the ports Relates to multiple ports, or a new/proposed port label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ports Relates to multiple ports, or a new/proposed port
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@villeneuve @dpgeorge @peterhinch @robert-hh @zzzbatmand
0