Remove chrono from the RTC API

[robertbastian]

Motivations

The Rtc::set_current_time and Rtc::current_time methods use chrono::NaiveDateTime. This is not a timestamp type, it is a civil time type, so it exposes concepts like "day" in a place where these concepts should not exist. chrono uses DateTime<Utc> as its timestamp type, but even that is flawed because you can still ask it which day it is (you get the response for London).

Unfortunately Rust lacks a standard library timestamp type, but I don't think esp-hal should pull in chrono just to use a single (flawed) type. I'm trying to migrate my chrono usage to jiff at the moment (which does have a good Timestamp type), but I have to keep chrono around to use the RTC.

Solution

Use an i64 of nanoseconds since the UNIX epoch instead of NaiveDateTime, so that users can choose their own datetime library.

Alternatives

• Use jiff::Timestamp

• Use chrono::DateTime<Utc>, which is chrono's kinda-timestamp time (it allows converting into UNIX seconds).

Additional context

[MabezDev]

This is not a timestamp type

I disagree here, a type can be a timestamp, and also understand "civil" time concepts. chrono::DateTime<Utc> itself, is just a chrono::NaiveDateTime with an extra field to store the time zone offset. Imo the NaiveDateTime makes sense here, and do note that it can be converted to i64 UNIX Epoch too: https://docs.rs/chrono/latest/chrono/struct.NaiveDateTime.html#method.timestamp which you can use to convert the the type of your choosing.

What are you actually trying to do? It sounds more like you want to compare Instant's to get Duration's, in which case we have the time module for this.

[robertbastian]

I disagree here, a type can be a timestamp, and also understand "civil" time concepts.

Yes, that's chrono's DateTime type. All call sites of this need to immediately call .and_utc() to be able to do anything with it. So why not return DateTime<Utc> directly?

Imo the NaiveDateTime makes sense here, and do note that it can be converted to i64 UNIX Epoch too

This method is deprecated for the exact reasons I'm lining out here.

[MabezDev]

Ah sorry I missed the deprecation notice. In which case, I think we'd accept a PR changing the type to chrono::DateTime<Utc>. jiff looks interesting (and simpler), but I'd rather not depend on more <1.0 public dependencies.

[robertbastian]

I think ideally this would just return an i64, and not be opinionated about which datetime library to use.

[robertbastian]

Also, jiff is aiming for a 1.0 release this summer, which is probably before this component stabilises.

[MabezDev]

I think ideally this would just return an i64, and not be opinionated about which datetime library to use.

I'm fine with adding a _raw API that just returns a i64, but I'd like there to still be a way to get a time date type.

Also, jiff is aiming for a 1.0 release this summer, which is probably before this component stabilises.

Do you have any source for that? I just took a closer look at jiff, and it's maintainer, which gives me a bit more hope for the library. I think if we have a source on a planned jiff 1.0 release, I'd be happy to try it out in esp-hal. I agree that it's likely going to be a while before we stabilize Rtc, so it seems like a good place to try it out.

[robertbastian]

https://github.com/BurntSushi/jiff/blob/12ca7aaba77894e3fcdd96e97b61abc4b0266bb5/CHANGELOG.md?plain=1#L73-L75

[MabezDev]

Thanks! I think we'd accept a PR to use jiff instead. Would you mind opening one?

[BurntSushi]

As the author of Jiff, and looking at #3200, I'm with @robertbastian here that just returning a Unix timestamp as a i64 to nanosecond precision (assuming you don't care about timestamps outside of 1677-09-21T00:12:43.145224192Z to 2262-04-11T23:47:16.854775807Z) is probably the right call here. The API surface is so tiny that bringing in a public dependency for it seems like it's probably not worth it. You can always suggest using Jiff and even provide a doc example doing so by depending on Jiff via a dev-dependency.

[robertbastian]

Probably should be u64 actually, as that's what the RTC registers store, and we don't need to support setting the time to before the UNIX epoch.

[JurajSadel]

I believe this is resolved by #3200, closing.