embassy icon indicating copy to clipboard operation
embassy copied to clipboard

Cannot use RTC functionality alongside low_power module

Open tomhampshire opened this issue 7 months ago • 6 comments
trafficstars

Using low_power requires you to relinquish control of the RTC peripheral to the executor. e.g:


    // give the RTC to the executor...
    let mut rtc = Rtc::new(p.RTC, RtcConfig::default());
    static RTC: StaticCell<Rtc> = StaticCell::new();
    let rtc = RTC.init(rtc);
    embassy_stm32::low_power::stop_with_rtc(rtc);

source

This means that the RTC can not be subsequently used for the purposes of getting/setting the datetime, as this requires mutable references.

tomhampshire avatar Apr 18 '25 17:04 tomhampshire

A suggestion from the embassy chat room was to implement a split function for the RTC - one part can be passed to the executor, and the other used for general datetime functionality (get/set)

tomhampshire avatar Apr 23 '25 10:04 tomhampshire

An excerpt from the chat:

dirbaio i'm not sure what the best fix would be the lowpower executor does require the rtc to live forever so it should prevent the user from dropping or reconfiguring it

danielb Split it in two

dirbaio yeah so much boilerplate though it'd be nice if it was "more automatic" like the other time drivers. you enable it in Cargo features then embassy_stm32::init does all the necessary init so you don't have to explicitly configure rtc, split it, give one half to the executor

tomhampshire avatar Apr 23 '25 10:04 tomhampshire

@Dirbaio what do you think of the following?

Proposal: Refactor embassy_stm32::rtc using split for Low-Power and Application Access

Problem:

The current rtc::Rtc struct combines low-power wakeup management and application-level time access (now(), set_datetime()). This creates an ownership conflict when embassy_stm32::low_power::stop_with_rtc requires exclusive control of the Rtc instance, preventing the application from simultaneously accessing time functions.

Proposed Solution:

Adopt the split pattern, common in embassy-stm32, to separate RTC functionalities:

  1. Initialisation: Rtc::new(p.RTC, config) initialises the peripheral (clocks, prescalers) and returns a single, unified Rtc instance.
  2. Splitting: Introduce a rtc.split() -> (RtcControl, RtcClock) method. This method consumes the Rtc instance.
    • RtcControl: This handle manages low-power wakeup functions and holds related state (e.g., stop_time). It's designed to be passed to stop_with_rtc.
    • RtcClock: This handle provides the application API for timekeeping (now(), set_datetime(), calibrate(), backup registers, etc.). It's retained by the application.

Implementation Details:

  • The core register access logic (handling write protection, init mode, read retries, merging v2/v3 specifics) would be centralised in internal helper functions (hal::read_registers, hal::write_registers).
  • Safety: These helpers ensure thread-safe access to the shared RTC hardware registers (pac::RTC) by using critical_section::with.
  • Both RtcControl (for its low-power functions) and RtcClock (for application functions) would use these safe internal helpers for all hardware register interactions.

tomhampshire avatar May 12 '25 16:05 tomhampshire

The API sounds good, but it's a bit of boilerplate the user would have to write.

I think it'd be even better if embassy_stm32::init would take care of everything automatically, like it does with the regular time driver. It'd init the RTC (or at least part of it) to make the low-power time driver work, then the peripheral singleton p.RTC would still be available, the "user-visible" RTC driver would let the user do everything except stuff that would disrupt the time driver.

Dirbaio avatar May 13 '25 09:05 Dirbaio

So would the intention be to still have separate RtcControl and RtcClock instances, but the former would be set up during embassy_stm32::init and stored in the time_driver::RtcDriver instance, then the latter would be accessed in the usual way (via rtc::new(rtc, rtc_config)? I'm also assuming here that the Rtc::configure isn't a necessary prerequisite for the low-power time driver, otherwise I don't see how the boilerplate code can be avoided...

tomhampshire avatar May 13 '25 09:05 tomhampshire

So would the intention be to still have separate RtcControl and RtcClock instances, but the former would be set up during embassy_stm32::init and stored in the time_driver::RtcDriver instance, then the latter would be accessed in the usual way (via rtc::new(rtc, rtc_config)?

they may or may not exist internally, it's not something the user has to see. The user would see just:

  • Just setting the right Cargo features makes the time driver be low-power.
  • There's a p.RTC singleton and a struct Rtc driver they can use for wall clock time.

I'm also assuming here that the Rtc::configure isn't a necessary prerequisite for the low-power time driver, otherwise I don't see how the boilerplate code can be avoided...

It could be added to the main embassy_stm32::Config. This way it'd have a default so no boilerplate needed, but the user could still customize it if wanted.

Dirbaio avatar May 13 '25 10:05 Dirbaio