Skip to content
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.

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

Allow restoring currently running rtc (lost time on initialization) #466

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

Rutherther
Copy link
Contributor

Hi, I came across a problem when using the Rtc.
The problem is that if the microcontroller is reset, some time will be lost due to the reinitialization of the oscillator.

In order to solve this, I added methods restore_or_new, restore_or_new_lsi, restore_or_new_hse.
These methods check whether the rtc is enabled and the source is correct. If this is the case, the initialization is skipped.
After testing this, (for now, only with lse) it worked as intended, no time was lost anymore and the rtc was still counting.

What do you think about this? Are there any other things needed to be done, or should I change the behavior somehow?

@burrbull
Copy link
Member

Maybe some example of usage?

@Rutherther
Copy link
Contributor Author

Maybe some example of usage?

I could add that.

But just in case it's not clear: This should just replace the Rtc::new, and the difference is that the Rtc won't be reset, when it's already running (in case the battery is connected and it was already initialized). The previously selected frequency should remain intact as well.

Maybe I should also let the user know whether the Rtc is restored, or initialized, as that could allow for something like this:

let rtc = match Rtc::restore_or_new(...) {
  Restored(rtc) => rtc, // The rtc is restored from previous configuration
  New(rtc) => { // The rtc was just initialized, the clock source selected, period is one second
    // Initialize the rtc with desired parameters
    rtc.select_frequency(2.Hz());
    rtc
  }
};

or

let (rtc, rtc_restored) = Rtc::restore_or_new(...);
if !rtc_restored { // The rtc was just initialized, the clock source selected, period is one second
  // Initialize the rtc with desired parameters
  rtc.select_frequency(2.Hz());
}

(not sure which one makes more sense)

I imagine that "restoring" should be the dominant usecase, as users probably don't want to have their time lack behind when resetting. And in case battery is not connected, the rtc will be initialized again. I am surprised that something like this was not implemented so far, maybe people weren't using this crate for something that needed persistent clock so far?

@burrbull
Copy link
Member

You can add these examples in documentation of rtc module and/or existing example/rtc.rs

@burrbull
Copy link
Member

burrbull commented Jul 28, 2023

squash commits, please

@Rutherther
Copy link
Contributor Author

Right, sorry about that.

@burrbull
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jul 28, 2023
466: Allow restoring currently running rtc (lost time on initialization) r=burrbull a=Rutherther

Hi, I came across a problem when using the Rtc.
The problem is that if the microcontroller is reset, some time will be lost due to the reinitialization of the oscillator.

In order to solve this, I added methods restore_or_new, restore_or_new_lsi, restore_or_new_hse.
These methods check whether the rtc is enabled and the source is correct. If this is the case, the initialization is skipped.
After testing this, (for now, only with lse) it worked as intended, no time was lost anymore and the rtc was still counting.

What do you think about this? Are there any other things needed to be done, or should I change the behavior somehow?

Co-authored-by: František Boháček <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 28, 2023

Build failed:

So far the Rtc had to be reset on every
startup, leading to time lacking behind.

This has been addressed by adding method
`Rtc::restore_or_new` that allows to restore
the previous rtc, without resetting it.
In case the rtc was not running, it is initialized.

Documentation has been added and it was included
in `Rtc::new` that `Rtc::restore_or_new` should be
used for persistent time accuracy.

Example `examples/rtc.rs` has been updated to use
the new method.
@burrbull
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 28, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 3f274fe into stm32-rs:master Jul 28, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants