Time going backwards: aon_timer_get_time() [SDK 2.1.1]

[tjko]

After upgrading from SDK version 2.1.0 to 2.1.1, I noticed weird behavior with aon_timer_get_time() on RP2040 (pico_w).
Time returned seems to randomly go backwards (and forwards) 1 hour.

I have a project that displays clock (among other things) on OLED panel, and after upgrading to new SDK (2.1.1), I noticed hour digit was randomly toggling back and forth.

So I added some 'debug' output in the function that gets the current (RTC) time:

time went backwards: 1743055938 -> 1743052339 (delta=3599)
time went backwards: 1743055948 -> 1743052349 (delta=3599)
time went backwards: 1743055953 -> 1743052354 (delta=3599)
time went backwards: 1743055970 -> 1743052371 (delta=3599)
time went backwards: 1743055973 -> 1743052374 (delta=3599)
time went backwards: 1743055990 -> 1743052391 (delta=3599)
time went backwards: 1743056009 -> 1743052410 (delta=3599)
time went backwards: 1743056041 -> 1743052442 (delta=3599)
time went backwards: 1743056082 -> 1743052483 (delta=3599)
time went backwards: 1743056116 -> 1743052517 (delta=3599)
time went backwards: 1743056123 -> 1743052524 (delta=3599)
time went backwards: 1743056126 -> 1743052527 (delta=3599)
time went backwards: 1743056148 -> 1743052549 (delta=3599)
time went backwards: 1743056156 -> 1743052557 (delta=3599)

This is from the function shown below (that gets called about once a second):

bool rtc_get_time(time_t *t)
{
        static time_t old_t = 0;
        struct timespec ts;
        time_t new_t;

        if (!t || !aon_timer_is_running())
                return false;

        if (!aon_timer_get_time(&ts))
                return false;
        new_t = ts.tv_sec;

        if (old_t > new_t) {
                printf("time went backwards: %lld -> %lld (delta=%lld)\n", old_t, new_t, old_t - new_t);
        }
        old_t = new_t;

        *t = new_t;

        return true;
}

(switching back to SDK 2.1.0 seems to "fix" the issue)

[tjko]

Actually, seems like this is happening on SDK 2.1.0, but lot less frequently, it its minutes to hours between occurrences.

I left test running overnight and save couple occurrences logged:

time went backwards: 1743061269 -> 1743057670 (delta=3599)
time went backwards: 1743061703 -> 1743058104 (delta=3599)
time went backwards: 1743085016 -> 1743081417 (delta=3599)

When this happens time seems to jump forward 1 hour for about a second and then jump back....
As if this is somehow timezone (daylight savings) related, but doesn't seem aon_timer_get_time() does any timezone processing?

[peterharperuk]

It calls mktime. I wonder if that's doing some daylight saving. It might be worth zeroing the struct timespec ts; memory which might indicate a bug in datetime_to_tm?

[tjko]

I think you might be right, if I clear the timespec struct before calling aon_timer_get_time(), it would seem like issue goes away....

        memset(&ts, 0, sizeof(ts));
        if (!aon_timer_get_time(&ts))
                return false;
        new_t = ts.tv_sec;

[peterharperuk]

Good find. I guess we should be setting tm_isdst to zero. Although how it determines day light saving is on is beyond me.
Actually, we should just make sure it's not -1? Or always use -1?

[tjko]

Seems like clearing timespec before making call to aon_timer_get_time() didn't really "help", just unlucky that didn't see the issue immediately...

Looks like datetime_to_tm() leaves three fields uninitialized (tm_wday, tm_yday, tm_isdst) that likely lead to the random issues I was seeing?

This change would seem to "fix" the issue for me:

diff --git a/src/common/pico_util/datetime.c b/src/common/pico_util/datetime.c
index 130e2d5..f25bb5f 100644
--- a/src/common/pico_util/datetime.c
+++ b/src/common/pico_util/datetime.c
@@ -63,6 +63,9 @@ void datetime_to_tm(const datetime_t *dt, struct tm *tm) {
     tm->tm_hour = dt->hour;
     tm->tm_min = dt->min;
     tm->tm_sec = dt->sec;
+    tm->tm_wday = dt->dotw;
+    tm->tm_yday = -1; // FIXME
+    tm->tm_isdst = -1;
 }
 
 void tm_to_datetime(const struct tm *tm, datetime_t *dt) {

[tjko]

Looks like workaround to the issue (that avoids having to patch the SDK) is to define following pico_mktime() function (that overrides the implementation in SDK, thanks to the __weak linker directive).

/* Workaround to issue (#2374) in datetime_to_tm() ... */
time_t pico_mktime(struct tm *tm)
{
        tm->tm_isdst = -1;
        return mktime(tm);
}

Seems like this issue is only triggered when timezone is set to a timezone that has daylight savings time.

I'm getting time from NTP, so I'm setting timezone in order to convert UTC time received from NTP server to local time
(I was using following timezone "PST8PDT7,M3.2.0/2,M11.1.0/02:00:00")

I'm setting timezone as follows:

setenv("TZ", dhcp_timezone, 1);
tzset();

[kilograham]

less of a workaround and more of a fix, as tm_isdst does need to be initialized, however in this case i would put it in datetime_to_tm

[kilograham]

merged into develop