sys/walltime: add module for system time by benpicco · Pull Request #21343 · RIOT-OS/RIOT

benpicco · 2025-04-01T16:43:24Z

Contribution description

Currently applications use the periph_rtc API directly if they want to track wall clock time.
This leads to problems when the time is not provided by periph_rtc but some external RTC or when we want to perform actions e.g. on setting the time - then this has to be hooked up with every periph_rtc implementation.

Instead, provide a wrapper API that applications can use to obtain the current system time without accessing the RTC API directly.

This allows to write applications that are agnostic to the availability of periph_rtc and still can provide some proper timestamps.

Testing procedure

Enable the walltime module.
This will provide a walltime command to get / set the time and display the time since last boot.

Issues/PRs references

Alternative to #17416, #21366

crasbe

I like this addition a lot, some small comments and observations from testing the PR below.

Edit: For reference: I tested this with an nRF52840DK with USEMODULE+=walltime BOARD=nrf52840dk make -C tests/sys/shell flash term.

sys/walltime/shell.c

sys/include/walltime.h

sys/walltime/shell.c

sys/include/walltime.h

crasbe · 2025-04-01T20:07:06Z

sys/include/walltime.h

+ * @brief       Common functions to access the wall-clock / real time clock
+ * @{
+ * @file
+ *


It would be nice to have some documentation about what this module does and how to use it. More or less what the PR description says for example.

Currently it's a bit sparse and if you don't know what it does, it's a bit hard to get into it (IMO):

Also since there is no documentation for struct tm (I think time.h is part of the standard library?), it might be good to include a link to a reference for it.

This is the link used by drivers/periph/rtc.h: https://pubs.opengroup.org/onlinepubs/7908799/xsh/time.h.html

RIOT/drivers/include/periph/rtc.h

Lines 17 to 20 in 85dc9be

* @note

* The values used for setting and getting the time/alarm should

* conform to the `struct tm` specification.

* Compare: http://pubs.opengroup.org/onlinepubs/7908799/xsh/time.h.html

riot-ci · 2025-04-01T20:31:24Z

Murdock results

✔️ PASSED

33b8ec8 cpu/esp8266: implement cpu_woke_from_backup()

Success	Failures	Total	Runtime
10966	0	10968	10m:43s

Artifacts

Documentation preview

mguetschow

Nice, I like this! Some first thoughts on it below.

sys/include/walltime.h

sys/walltime/shell.c

sys/walltime/walltime.c

mguetschow · 2025-04-02T11:43:42Z

sys/walltime/walltime.c

+#endif
+
+static uint32_t _boottime;
+#ifdef BACKUP_RAM


Is that macro documented somewhere?

Does this count?

$ rg BACKUP_RAM -B 5 features.yaml 417-- title: RAM Related Features 418- help: These features indicate presence of special RAM regions or features 419- features: 420- - name: backup_ram 421- help: A special portion of RAM is retained during deep sleep. 422: Variables can be placed there by annotating them with the `BACKUP_RAM`

See https://doc.riot-os.org/feature-list.html#autotoc_md2404 for the output

Hum, that's indeed something, but probably nothing doxygen would find when using @ref BACKUP_RAM

sys/walltime/walltime.c

sys/include/walltime.h

sys/walltime/shell.c

sys/walltime/walltime.c

sys/include/walltime.h

sys/walltime/shell.c

fabian18 · 2025-04-07T14:26:45Z

sys/walltime/walltime.c

+
+void walltime_set(struct tm *time)
+{
+    int32_t diff = rtc_mktime(time) - walltime_get_riot(NULL);


I think the int32_t could lead to unexpected results. For example the extreme case 0 - 4294967295 becomes 1.
I would expect -4294967295
You could figure out the sign and compute the number as uint32_t higher - lower.

diff --git a/sys/walltime/walltime.c b/sys/walltime/walltime.c index 903e856a16..5171cb0770 100644 --- a/sys/walltime/walltime.c +++ b/sys/walltime/walltime.c @@ -17,6 +17,8 @@ * * @} */ +#include <stdio.h> + #include "auto_init.h" #include "auto_init_utils.h" #include "auto_init_priorities.h" @@ -96,7 +98,10 @@ time_t walltime_get_unix(uint16_t *ms) void walltime_set(struct tm *time) { - int32_t diff = rtc_mktime(time) - walltime_get_riot(NULL); + uint32_t t; + int32_t w; + int32_t diff = (t = rtc_mktime(time)) - (w = walltime_get_riot(NULL)); + printf("t %"PRIu32" - w %"PRId32" = diff %"PRId32"\n", t, w, diff); _boottime += diff; #ifdef BACKUP_RAM _boottime_bkup += diff;

> 2025-05-06 15:29:20,168 # main(): This is RIOT! (Version: 2025.04-devel-522-g90fc5e-walltime) > walltime set 4000-01-01 00:00:00 2025-05-06 15:29:23,464 # walltime set 4000-01-01 00:00:00 2025-05-06 15:29:23,480 # t 2353209856 - w 3 = diff -1941757443 2025-05-06 15:29:23,481 # time changed by -1941757443 sec, 0 ms

So when going very very very far in the future, the diff becomes negative.
I know that no one actually cares about the year 4000, but consider some user input or malformed network packet ... whatever.

sys/walltime/shell.c

sys/walltime/walltime.c

fabian18 · 2025-04-07T14:37:41Z

sys/walltime/walltime.c

+    return now - _boottime;
+}
+
+static void auto_init_uptime(void)


maybe guard with IS_USED(MODULE_AUTO_INIT_WALLTIME/UPTIME)

We always want to auto-init this module

sys/walltime/walltime.c

fabian18 · 2025-04-07T14:54:25Z

This allows to write applications that are independent of the RTC
peripheral and allows the implementation of on-time change callbacks.

How would I implement an on-time change callback?
You expect that the backend for walltime implements the RTC API, don't you?
This limits to only only one RTC API. If I want to implement my application rtc_set_time around the RTC rtc_set_time, it does not work. I would have gone with function pointers for walltime_backend->rtc_set_time and walltime_backend->rtc_get_time

benpicco · 2025-04-07T15:05:47Z

How would I implement an on-time change callback?

We can add that to the walltime module as a 2nd step, then we would only need to add it in one place instead of adding it to every RTC implementation.

I wanted to get the core API in first, but might as well already add this here.

If I want to implement my application rtc_set_time around the RTC rtc_set_time, it does not work.

Yea applications must not use the periph_rtc API if the walltime module is used to access the RTC instead - but that should be a simple change.

I would have gone with function pointers for walltime_backend->rtc_set_time and walltime_backend->rtc_get_time

There only is a single wall time/system time. I'm happy to hook up different backends like ds1307 or even dcf77 or gnss. The nice thing about the high level API is that the internals can be as complex as needed, to the user it's still just a simple get time / set time API.

sys/walltime/walltime.c

sys/include/walltime.h

benpicco · 2025-12-04T18:08:38Z

If the walltime would not use the RTC API but use a driver struct with function callbacks, then the walltime could be instanciated with either the periph RTC or the external RTC driver. Then periph RTC features could still be used.

Since there will only be a single active walltime source, we might as well make that a compile-time selection inside the walltime module.

Or a weak default implementation that can be overwritten by other backends?

maribu · 2025-12-04T18:51:33Z

+1 for the weak default implementation. That would allow us to hook in our I2C RTC into the walltime module quite easily.

benpicco · 2025-12-04T19:04:50Z

That would allow us to hook in our I2C RTC into the walltime module quite easily.

It's a shame we don't have that upstream though…

sys/walltime/fallback.c

sys/walltime/rtc.c

fabian18 · 2025-12-12T12:56:30Z

sys/walltime/fallback.c

+__attribute__((weak))
+int walltime_impl_get(struct tm *time, uint16_t *ms)
+{
+    uint32_t now = ztimer_now(ZTIMER_MSEC);


could benefit from #20494

The walltime module is good from my perspective. As the xtimer_compat was merged, it would be nice to have it here.

Hm we don't have a xtimer_now_msec(), but I could add that.

Ah I thought there is. It's a logic addition to xtimer_now_usec. It should probably favor ZTIMER_MSEC over ZTIMER_USEC.

but tbh I'm not sure how useful a non-msec based backend would be here.
ZTIMER_USEC wraps after 1h and 11 minutes.

If there is no implication of using ztimer_msec towards not using ztimer_msec then yes msec is better than usec here.
Then ztimer_msec should be added to the Makefile.dep.

ahh but only in the fallback case... I dont know ... either you squash or add xtimer_now_msec if you like

I added it to #21958

maribu · 2025-12-12T13:22:34Z

Would you mind sprinkling some SPDX-license-identifier fairy dust over the new code?

benpicco · 2025-12-18T13:20:43Z

cpu_woke_from_backup() was not implemented, doing to caused a merge conflict - let's see if CI is happy now.

github-actions bot added the Area: sys Area: System label Apr 1, 2025

benpicco force-pushed the walltime branch from c7303e5 to 128f19f Compare April 1, 2025 16:54

maribu self-requested a review April 1, 2025 17:25

crasbe reviewed Apr 1, 2025

View reviewed changes

crasbe added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 1, 2025

mguetschow reviewed Apr 2, 2025

View reviewed changes

maribu reviewed Apr 2, 2025

View reviewed changes

sys/include/walltime.h Outdated Show resolved Hide resolved

maribu reviewed Apr 2, 2025

View reviewed changes

sys/walltime/shell.c Outdated Show resolved Hide resolved

sys/walltime/shell.c Outdated Show resolved Hide resolved

kfessel reviewed Apr 3, 2025

View reviewed changes

sys/walltime/walltime.c Outdated Show resolved Hide resolved

mguetschow reviewed Apr 3, 2025

View reviewed changes

sys/include/walltime.h Outdated Show resolved Hide resolved

mguetschow reviewed Apr 3, 2025

View reviewed changes

sys/include/walltime.h Outdated Show resolved Hide resolved

benpicco requested a review from fabian18 April 7, 2025 13:51

benpicco mentioned this pull request Apr 7, 2025

periph/rtc: add rtc_pre_set_time() and rtc_post_set_time() #21366

Closed

fabian18 reviewed Apr 7, 2025

View reviewed changes

sys/walltime/walltime.c Outdated Show resolved Hide resolved

sys/walltime/walltime.c Outdated Show resolved Hide resolved

fabian18 mentioned this pull request Apr 8, 2025

sys/fmt: add {fmt,scn}_time_tm_iso8601 #21391

Merged

benpicco force-pushed the walltime branch from 873243a to 28e964b Compare April 11, 2025 17:37

benpicco requested a review from kaspar030 as a code owner April 11, 2025 17:37

benpicco requested review from MichelRottleuthner, aabadie and leandrolanzieri as code owners April 24, 2025 16:13

github-actions bot added the Area: tests Area: tests and testing framework label Apr 24, 2025

benpicco force-pushed the walltime branch 2 times, most recently from d3999cd to 90fc5e4 Compare April 24, 2025 16:57

fabian18 reviewed May 6, 2025

View reviewed changes

sys/include/walltime.h Outdated Show resolved Hide resolved

fabian18 reviewed May 6, 2025

View reviewed changes

sys/include/walltime.h Outdated Show resolved Hide resolved

crasbe added the State: waiting for author State: Action by the author of the PR is required label Sep 12, 2025

github-actions bot added the Area: build system Area: Build system label Dec 4, 2025

benpicco removed the State: waiting for author State: Action by the author of the PR is required label Dec 8, 2025

benpicco force-pushed the walltime branch from c7513f9 to 0aa70a7 Compare December 11, 2025 11:03

fabian18 reviewed Dec 12, 2025

View reviewed changes

benpicco force-pushed the walltime branch from f183ee1 to ae12661 Compare December 17, 2025 18:30

fabian18 approved these changes Dec 17, 2025

View reviewed changes

benpicco added this pull request to the merge queue Dec 17, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 17, 2025

benpicco requested a review from gschorcht as a code owner December 18, 2025 13:08

github-actions bot added Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Dec 18, 2025

benpicco added 6 commits December 18, 2025 14:09

sys/fmt: split up scn_time_tm_iso8601()

f2be55e

sys/walltime: add module for system time

379e457

sys/walltime: add RTC backend

bca13cf

sys/walltime: add shell commands

72df56a

tests/walltime: add test for the walltime module

59056eb

cpu/esp8266: implement cpu_woke_from_backup()

33b8ec8

benpicco force-pushed the walltime branch from 1039d27 to 33b8ec8 Compare December 18, 2025 13:10

benpicco enabled auto-merge December 18, 2025 13:20

benpicco added this pull request to the merge queue Dec 18, 2025

Merged via the queue into RIOT-OS:master with commit 42050c5 Dec 18, 2025
25 checks passed

benpicco deleted the walltime branch December 18, 2025 16:48

leandrolanzieri added this to the Release 2026.01 milestone Jan 13, 2026

	* @note
	* The values used for setting and getting the time/alarm should
	* conform to the `struct tm` specification.
	* Compare: http://pubs.opengroup.org/onlinepubs/7908799/xsh/time.h.html

Conversation

benpicco commented Apr 1, 2025 • 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!

crasbe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

riot-ci commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

mguetschow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fabian18 commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benpicco commented Apr 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benpicco commented Dec 4, 2025

Uh oh!

maribu commented Dec 4, 2025

Uh oh!

benpicco commented Dec 4, 2025

Uh oh!

Uh oh!

Uh oh!

benpicco commented Apr 1, 2025 •

edited

Loading

crasbe left a comment •

edited

Loading

riot-ci commented Apr 1, 2025 •

edited

Loading

fabian18 commented Apr 7, 2025 •

edited

Loading

fabian18 Dec 17, 2025 •

edited

Loading