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

[draft] Don't reset the RTC time from gettimeofday(), plus other RTC cleanups #210

Merged
merged 3 commits into from Jan 19, 2024

Conversation

multiplemonomials
Copy link
Collaborator

Summary of changes

This is a fix for the RTC issues that @sunnydaywest reported on the forums, and also as ARMmbed#15487.

As I explained in the thread, the easiest way to fix the issue is to update gettimeofday() to not reset the RTC time. It's not intuitive for getting the time to reset the time, just because you didn't call rtc_init() first.

While I was in the RTC code, I also took the opportunity to:

  • Remove code from mbed_overrides.c which is also in rtc_api.c. As far as we could tell, the code in rtc_api.c is the correct version, and it's what gets executed second so it makes sense for it to be the correct one.
  • Move target.lse_bypass option to be for all STM32 targets, as this is a useful option which should work on all STM32 targets and might be wanted on custom boards
  • Update docs for target.clock_source and target.lse_drive_load_level options

Impact of changes

Calling time() or gettimeofday() without calling rtc_init() first will no longer cause the RTC time to be reset to 0.

Migration actions required

Documentation


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[X] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


targets/targets.json5 Outdated Show resolved Hide resolved
@lefebvresam
Copy link

lefebvresam commented Jan 17, 2024

What would be the action to make time(NULL) working without calling rtc_init() first? It seems logical for me that RTC module is initialized when it's used and available and that you just can use the time(NULL) example as presented in the documention. Related to that, why you can use RTC at 4 different levels? As RTC object, RTC api, RTC HAL and default C-lib functions?

@JohnK1987
Copy link
Member

JohnK1987 commented Jan 18, 2024

From my understanding

  • RTC HAL is brand (MCU) specific HAL code in C.
  • Mbed hal is definition of how brand specific RTC HAL has to be represented in MbedOS in C
  • Mbed RTC driver is wrapper from C to C++
    You are able to use all layers in MbedOS.

After this PR the rtc_init() will be called inside of gettimeofday() and this is called inside of time(NULL) so finaly you can use it without extra call of rtc_init() or set_time().

BR, Jan

@lefebvresam
Copy link

Is this for all functionality the same? It's strange that they use a mix in the mbed documentation. For instance for SPI they use the C++ way of instantiating objects.

@lefebvresam
Copy link

I will try this solution but I'm struggeling with (never used gh before):

sam@elitebook-845:~/git/hmc20_test/mbed-os$ gh pr checkout 210
could not determine base repo: none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`

@JohnK1987
Copy link
Member

Is this for all functionality the same? It's strange that they use a mix in the mbed documentation. For instance for SPI they use the C++ way of instantiating objects.

I think yes and it is cool because Mbed can not cover every posibility and then you can combine Mbed APIs with brand specific HALs or make your own API based on Mbed HALs.

@lefebvresam
Copy link

Why this merge is blocked?

@multiplemonomials
Copy link
Collaborator Author

Huh, that is weird. I am able to merge...

@multiplemonomials multiplemonomials merged commit f42c322 into master Jan 19, 2024
9 checks passed
@multiplemonomials multiplemonomials deleted the dev/dont-reset-rtc-time-from-gettimeofday branch January 19, 2024 22:57
@multiplemonomials
Copy link
Collaborator Author

Oops looks like I forgot to remove "[draft]" from the title... oh well

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.

None yet

3 participants