Allow setting RTC time

[amsam0]

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

• I have updated existing examples or added new ones (if applicable).
• I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
• My changes were added to the CHANGELOG.md in the proper section.
• My changes are in accordance to the esp-rs API guidelines

Extra:

• I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

esp-idf has gettimeofday (see here) and settimeofday (see here) functions, allowing the user to get and set the internal RTC time. While esp-hal allows getting the RTC time, it does not allow setting it.

esp-idf sets the RTC time by storing the boot time (see here for getting, here for setting, here for esp32s3 register info). It calculates the current time by adding the boot time and the RTC time. Therefore, you can set the current time by setting the boot time to the new current time minus the RTC time. esp-idf also does some more advanced things like calculating the offset between RTC and systimer and adjusting the boot time, which I haven't taken the time to understand or implement since the current implementation is good enough for my needs.

This PR contains a basic port of esp-idf's boot time usage in the Rtc struct. It adds set_time_us, set_time_ms, get_rtc_time_us, get_rtc_time_us, and renames get_time_raw to get_rtc_time_raw. It should support all esp32 chips.

To avoid overflows when adding the boot time and the rtc time, the boot time also wraps around u64::MAX.

There are also some traits in the rtcc crate that may be worth implementing.

Testing

This PR has been tested on a esp32s3.

[MabezDev]

This would be a nice addition, do you plan on finishing this up?

[amsam0]

Absolutely, but first I'd like to know what the preference is for general API (for instance, should the boot time functions be public?) and for implementing the rtcc traits. There is also the "boot time adjustment" and rtc offset to consider, but I don't really think they're needed.

[amsam0]

I've deprecated Rtc::get_time_raw, added the missing get_time functions, added documentation, updated the rtc_time example, updated the changelog, and checked to ensure that RTC set supports all chips. While I only have an esp32s3 to test with, this PR should be ready for merging, unless further changes are wanted.

[Szybet]

Oh, cool that you have checks for different devices
Yea, it doesn't compile for the c6 (Or any at all?), gonna look for a solution

[Szybet]

For the c6 I think only this was needed:

        #[cfg(any(esp32h2))]
        let rtc_cntl = unsafe { &*LP_TIMER::ptr() };
        #[cfg(any(esp32c6))]
        let rtc_cntl = unsafe { &*LP_AON::ptr() };

using LP_AON instead of LP_TIMER

Some resources:
https://github.com/espressif/esp-idf/blob/8e4454b285ad39881c5bf3f440d8be617a2f18a8/components/esp_rom/esp32c6/include/esp32c6/rom/rtc.h#L60-L61
https://github.com/espressif/esp-idf/blob/8e4454b285ad39881c5bf3f440d8be617a2f18a8/components/newlib/port/esp_time_impl.c#L110-L121

Full commit here:
dragonnn#1

[amsam0]

Yes, that's correct. I saw LP_AON in the register headers but assumed it was the same as LP_TIMER, especially since LP_TIMER is used for the other RTC stuff. I'll add a fix when I have some time

[Szybet]

Additionally I tested it on esp32c6 with the latest changes, works good

[amsam0]

Chrono sounds good and in my own personal usage of this PR, is what I'm converting the current_time to anyways. I'll implement that when I have some time (pun not intended).

Although, it is important to consider that embassy has their own DateTime struct and simply provides a From implementation for chrono, and that some people may be reluctant to add chrono as a dependency.

[amsam0]

current_time and set_current_time now use chrono's NaiveDateTime. I'm guessing that the preference will be to avoid adding chrono as a dependency and instead use our own DateTime type like embassy, but chrono is definitely a simpler approach.

Currently, set_current_time will panic if the NaiveDateTime is before the Unix epoch. Wrapping should be possible but it's a little more complicated so I didn't implement it (and even if it was implemented, I don't know why anyone would want to set the time to before the Unix epoch).

[amsam0]

Any chance of further review? It's been more than a month and it would be great to get this merged. I can update the example and fix merge conflicts if the API seems fine.

[MabezDev]

LGTM API-wise, I don't see the problem in directly depending on chrono, versus having our own types here.

[amsam0]

Changelog and examples are updated, so this PR should be ready to be merged

[bjoernQ]

There is some overlap now between rtc_cntl::Rtc::current_time and time::current_time - not something that should be addressed here - just mentioning

[MabezDev]

Hmm good point, but I would somewhat argue that we should change time::current_time to something else, probably time::now, given that it returns an instant (not wall clock time)?

Imo Rtc::current_time fits quite well for wall clock time.

[bjoernQ]

Hmm good point, but I would somewhat argue that we should change time::current_time to something else, probably time::now, given that it returns an instant (not wall clock time)?

Imo Rtc::current_time fits quite well for wall clock time.

👍 Sure - I'd vote for time::uptime() or similar - even the docs say "system uptime"

[bjoernQ]

Thanks - probably you should revert fe84468 since it's already done and merged in #2091

[bjoernQ]

After a rebase this should be fine I guess

[amsam0]

The PR has been rebased, and some basic info has been added to the migration guide

[bjoernQ]

LGTM

[MabezDev]

Thank you!