Provide a more robust solution for getting the current time

[Dominaezzz]

The current implementations for getting the time (since boot?) are:

• https://github.com/esp-rs/esp-hal/blob/v0.19.0/esp-hal/src/time.rs
• https://github.com/esp-rs/esp-hal/blob/v0.19.0/esp-hal/src/timer/systimer.rs#L139-L152

These implementations unfortunately have a couple issues:

• They unsafely (and unsoundly I guess) acquire access to the TIMG0/SYSTIMER peripherals to read the counters in them.
• They do so assuming the peripherals have been enabled and pulled out of reset.
• They also assume that the user won't reset them (again) by recreating the driver (See https://github.com/esp-rs/esp-hal/pull/1893#issuecomment-2273769315).
• They also assume that the user won't change the counter values in the peripherals (which is a perfectly legitimate thing to do).

I'm not sure what the solution is yet besides simply removing these APIs, but if getting the time without direct access to a timer peripheral is that desirable, then the hal user needs to explicitly create a timer driver and give up access to one of its counters somehow, something along the lines of esp_hal_embassy::init.

Perhaps this feature doesn't belong directly in hals and should be exposed by 3rd party libraries similar to embassy_time_driver and critical_section.

Or maybe this could be like the esp_hal_embassy feature where SoftwareInterrupt<3> is inaccessible. Though this doesn't solve the enable/reset problem.

[bjoernQ]

There are definitely things to improve here.

... if getting the time without direct access to a timer peripheral is that desirable ...

I think it is

They do so assuming the peripherals have been enabled and pulled out of reset.

The HAL should/could (and currently does?) guarantee that (ok - we don't enable them on boot if they are not, yet)

They also assume that the user won't reset them (again) by recreating the driver

I guess that is something the drivers need to take care of. If we really need to reset those, the driver should take care to save and restore the counters

They also assume that the user won't change the counter values in the peripherals

I think we don't offer an API for that? - if a user uses direct register manipulation for that, all our assumptions are invalid, I guess (in general true for all drivers)

[Dominaezzz]

They also assume that the user won't change the counter values in the peripherals

I think we don't offer an API for that? - if a user uses direct register manipulation for that, all our assumptions are invalid, I guess (in general true for all drivers)

I guess this is the main issue. As far as the user is concerned, if they have access to the TIMG0 or SYSTIMER peripheral via peripherals.TIMG0 or peripherals.SYSTIMER, then they have full ownership of the peripheral and are free to tinker with registers. The hal breaks this rule in these APIs to get the current time.

... if getting the time without direct access to a timer peripheral is that desirable ...

I think it is

Yeah I figured. Short term, it's probably best to have a esp_hal::time::init(....) that just takes an erased timer. This solves all the problems I mentioned.

[Dominaezzz]

In addition to the global time setup, it'd be a good idea to setup an interrupt to keep track of the timer wrapping around as well (Both SYSTIMER and TIMG can do this at least), the we can have a current time that only increments forward and doesn't wrap around.

[MabezDev]

it'd be a good idea to setup an interrupt to keep track of the timer wrapping around as well (Both SYSTIMER and TIMG can do this at least)

Unless I'm misunderstanding what part is wrapping around here, the smallest wrap around possible is ~7years, the highest being 36558 years on the esp32 - is this something real world applications really have to consider?

[Dominaezzz]

it'd be a good idea to setup an interrupt to keep track of the timer wrapping around as well (Both SYSTIMER and TIMG can do this at least)

Unless I'm misunderstanding what part is wrapping around here, the smallest wrap around possible is ~7years, the highest being 36558 years on the esp32 - is this something real world applications really have to consider?

Ah, I was under the impression that the wrap around number was much smaller for some reason. 36558 years is plenty.

[MabezDev]

I think the only remaining point to address here is

They do so assuming the peripherals have been enabled and pulled out of reset.

For systimer especially, it's assumed it's running

[MabezDev]

I think this will be addressed if we ever support another bootloader. We'll find out pretty fast if systimer etc isn't running. Closing this for now.