periph_common/rtc: add rtc_mktime() & rtc_localtime() helper functions for RTC implementations by benpicco · Pull Request #13284 · RIOT-OS/RIOT

benpicco · 2020-02-04T15:39:47Z

Contribution description

Some RTC implementations are based on a 1 Hz RTT and use a Unix timestamp and mktime()/localtime() to convert the struct tm to a timestamp and back.
While convenient, it is not necessary to start there, but worse it will also stop working on 2038-01-19.

This introduces two new functions:

rtc_mktime()
rtc_localtime()

They function similar to mktime() and localtime_r() but forego POSIX compatibility to be used by RTC implementations where this is not needed.

Key differences are:

timestamp is unsigned
epoch starts in 2020

Since RTC is unlikely to be set to a time in the past, we can use this to easily push the Y2038 problem to the year 2156 (2156-02-07 06:28:15).

Since RTC does not need to handle dailight savings time, timezone and such, the rtc_ implementation saves ~9k compared to using the newlib functions.

Testing procedure

I only converted the stm32f1 RTC as an example.

Run tests/periph_rtc on e.g. a bluepill board.

Issues/PRs references

possible solution for #13277

maribu · 2020-02-10T13:05:56Z

Hmm, 2156 seems to be reasonablly far in the future; at least our generation can be unconcerned that we will ever face issues :-D. (Same line of thoughts our politicians seem to have when it comes to climate change...)

However, is there a use case where it would make sense to set the RTC to a point in time in the past? I would say no - but if there is, this use case will be impossible now.

benpicco · 2020-02-10T13:12:22Z

You could still re-define RIOT_EPOCH if you want to dwell in the past.
Since this is all internal to the RTC implementation, the absolute values of the RTT timer don't need to be portable, the user will only get a struct tm.

(Force-pushed because I accidentally a word)

gschorcht · 2020-02-29T16:13:08Z

@benpicco This is pretty cool. I'm wondering whether it would be better to split into a PR which extends the RTC API and a PR with the changes for cpu/stm32f1

benpicco · 2020-02-29T16:41:38Z

@gschorcht I can just drop the stm32f1 commit from this PR, I thought it might be good to have an example of how those can be useful.

Add a function to convert a time struct to an unsigned timestamp (non-UNIX).

gschorcht · 2020-02-29T16:55:21Z

@gschorcht I can just drop the stm32f1 commit from this PR

Not neccessary, I read the test instruction again 😎 It makes sense to have a platform as POC. With the changed title the PR makes sense as it is 😉

benpicco · 2020-02-29T17:08:55Z

I read the test instruction again It makes sense to have a platform as POC.

Alright! But now I converted esp32 instead.

gschorcht · 2020-02-29T17:14:58Z

OK, I tested it with your initial commit for stm32f1. It works as expected.

tests/periph_rtc/main.c

drivers/include/periph/rtc.h

gschorcht · 2020-02-29T17:21:20Z

cpu/esp32/periph/rtc.c

-    else {
-        return -1;
-    }
+    return 0;


Wow, it's much easier to use 👍 I like it.

It should also save a good chunk of memory:

master

esp32-wroom-32:

text data bss dec hex filename 69261 4624 6432 80317 139bd /home/benpicco/dev/RIOT/tests/periph_rtc/bin/esp32-wroom-32/tests_periph_rtc.el

blackpill-128kib:

text data bss dec hex filename 23416 232 2736 26384 6710 /home/benpicco/dev/RIOT/tests/periph_rtc/bin/blackpill-128kib/tests_periph_rtc.elf

this branch

esp32-wroom-32:

text data bss dec hex filename 66945 4368 6424 77737 12fa9 /home/benpicco/dev/RIOT/tests/periph_rtc/bin/esp32-wroom-32/tests_periph_rtc.elf

blackpill-128kib:

text data bss dec hex filename 14272 132 2696 17100 42cc /home/benpicco/dev/RIOT/tests/periph_rtc/bin/blackpill-128kib/tests_periph_rtc.elf

gschorcht · 2020-03-01T10:12:18Z

Does commit e1c14c6 make PR #13520 obsolete?

benpicco · 2020-03-01T14:54:30Z

Yes if it can go in with this PR there is no need for a separate PR.

gschorcht · 2020-03-02T18:54:55Z

@benpicco Since you already have PR #13520 opened it is might be better to have one platform (ESP32) as POC and for testing as part of this PR and to add the changes for the other platforms as separate changes.

Please squash, feel free to drop or leave e1c14c6 part of this PR or not.

Add function to convert a RTC timestamp (non-UNIX) to a time struct.

By using a custom EPOCH for the RTC implementation, we can extend the range of the 32 bit counter based RTC by 118 years. It also reduces the code size compared to the stdlib based POSIX functions.

benpicco · 2020-03-03T13:05:37Z

Alright, squashed & dropped the stm32f1 commit.

gschorcht

ACK

benpicco · 2020-03-03T13:19:34Z

Thank you!

benpicco added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers Area: cpu Area: CPU/MCU ports labels Feb 4, 2020

benpicco requested review from kaspar030 and maribu February 4, 2020 15:39

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 4, 2020

benpicco force-pushed the rtc_mktime branch from 66438d8 to 9208677 Compare February 4, 2020 18:04

benpicco force-pushed the rtc_mktime branch from f21f62b to 20b7c76 Compare February 10, 2020 13:07

benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 10, 2020

benpicco mentioned this pull request Feb 17, 2020

cpu/atmega_common: RTT and RTC support #8842

Closed

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 25, 2020

benpicco requested review from MrKevinWeiss and miri64 as code owners February 29, 2020 15:32

benpicco changed the title ~~cpu/stm32f1: make RTC Y2038 safe~~ cpu/stm32f1: make RTC Y2038 safe, add rtc_mktime() & rtc_localtime() Feb 29, 2020

benpicco mentioned this pull request Feb 29, 2020

drivers/rtt_rtc: add RTT based RTC implementation, enable it for cpu/cc2538, nrf5x_common #13519

Merged

benpicco added 2 commits February 29, 2020 17:42

periph_common/rtc: add rtc_mktime()

becff61

Add a function to convert a time struct to an unsigned timestamp (non-UNIX).

periph_common/rtc: handle m_mday == 0 in rtc_tm_normalize()

fbada0d

benpicco force-pushed the rtc_mktime branch from 01846bc to 0e75332 Compare February 29, 2020 16:44

benpicco changed the title ~~cpu/stm32f1: make RTC Y2038 safe, add rtc_mktime() & rtc_localtime()~~ periph_common/rtc: add rtc_mktime() & rtc_localtime() helper functions for RTC implementations Feb 29, 2020

benpicco mentioned this pull request Feb 29, 2020

cpu/stm32f1: make RTC Y2038 safe #13520

Merged

gschorcht reviewed Feb 29, 2020

View reviewed changes

tests/periph_rtc/main.c Outdated Show resolved Hide resolved

gschorcht reviewed Feb 29, 2020

View reviewed changes

gschorcht added the Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines label Feb 29, 2020

benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 29, 2020

benpicco and others added 3 commits March 3, 2020 11:01

periph_common/rtc: add rtc_localtime()

5b64ae4

Add function to convert a RTC timestamp (non-UNIX) to a time struct.

tests/periph_rtc: don't set time in the past

6581d43

cpu/esp32: rtc: use rtc_mktime() for Y2038k

1ca26ad

By using a custom EPOCH for the RTC implementation, we can extend the range of the 32 bit counter based RTC by 118 years. It also reduces the code size compared to the stdlib based POSIX functions.

benpicco force-pushed the rtc_mktime branch from 742f26f to 1ca26ad Compare March 3, 2020 10:01

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 3, 2020

gschorcht approved these changes Mar 3, 2020

View reviewed changes

gschorcht merged commit 9e37210 into RIOT-OS:master Mar 3, 2020

benpicco deleted the rtc_mktime branch March 3, 2020 13:19

leandrolanzieri added this to the Release 2020.04 milestone Mar 13, 2020

Conversation

benpicco commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

maribu commented Feb 10, 2020

Uh oh!

benpicco commented Feb 10, 2020

Uh oh!

gschorcht commented Feb 29, 2020

Uh oh!

benpicco commented Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Feb 29, 2020

Uh oh!

benpicco commented Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Feb 29, 2020

Uh oh!

Uh oh!

Uh oh!

gschorcht Feb 29, 2020

Choose a reason for hiding this comment

Uh oh!

benpicco Feb 29, 2020

Choose a reason for hiding this comment

master

esp32-wroom-32:

blackpill-128kib:

this branch

esp32-wroom-32:

blackpill-128kib:

Uh oh!

gschorcht commented Mar 1, 2020

Uh oh!

benpicco commented Mar 1, 2020

Uh oh!

gschorcht commented Mar 2, 2020

Uh oh!

benpicco commented Mar 3, 2020

Uh oh!

gschorcht left a comment

Choose a reason for hiding this comment

Uh oh!

benpicco commented Mar 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benpicco commented Feb 4, 2020 •

edited

Loading

benpicco commented Feb 29, 2020 •

edited

Loading

benpicco commented Feb 29, 2020 •

edited

Loading

`esp32-wroom-32`:

`blackpill-128kib`:

`esp32-wroom-32`:

`blackpill-128kib`: