Set RTC to same timezone and daylight settings upon boot

[Niek]

This sets the RTC to the same timezone and daylight settings as the NTP config. This fixes that time(nullptr) returns the same time before and after NTP sync.

[mcspr]

Why though? configTime just enables sntp loop & timer, which will do nothing bc there is no server set (besides setting timer and wasting some time in timer func).

AFAIK, this may be different between lwip and lwip2, and lwip2 uses TimeLib-like approach and not the real RTC memory time

[Niek]

If you call time(nullptr) before NTP is synced, it will return the system clock time in wrong timezone (I think it's always China time). It looks like configTime() is the most compatible way to set the timezone of the system clock before NTP is synced, but maybe there's another way?

[mcspr]

I mean, why use it at all? It is not synced anyways using the lwip's sntp and we increase binary size too, because of the way linkage works.

BTW, have you tried with lwip2 builds? (removing -DPIO_FRAMEWORK_ARDUINO_LWIP_HIGHER_BANDWIDTH from the build flags). Would it not just return 0 there?

[mcspr]

To answer my own question, time(0) results are:

• 0 (constant) with lwip1.4 builds
• 28800 initially and ticking up every ~1s with lwip2 builds (see realtime_stamp in cores/esp8266/sntp-lwip2.cpp)

Initially I thought that there was a way to set this outside of sntp code, but does not seem like it :/

[Niek]

I tested this with LWIP 2 (PIO_FRAMEWORK_ARDUINO_LWIP2_LOW_MEMORY_LOW_FLASH). The result of time(nullptr) (on both older cores as well as staging) are:

• Current time in UTC +7h before NTP sync is completed (right after boot)
• Current time in UTC after NTP sync

The reason it's useful to get the RTC time in the correct timezone is that you don't need to wait on NTP sync to complete in order to connect a BearSSL/CA client (e.g. MQTT). In most cases the RTC will be close to synchronized with the NTP clock (for example after a reboot) so you can speed up the initial connect with a few seconds (a wrong time will results in a failing BearSSL connection).

[mcspr]

Sorry for being a bit dense, but do you mean that you have it synced between resets somehow? AFAIK it just keeps a global var and does not do network sync. configTime is a separate thing from TimeLib / NtpClientLib.

And it is not RTC per-se, since ESP8266 does not keep time there, but some internal timekeep timer thing that Core devs added.
We can, however, use additional Rtcmem slot to save timestamp there on each sync (or, better off, via ticker every second). That way, you will have up-to-date time whenever the device is rebooted

[Niek]

I understand that's it not a (battery powered) RTC in the traditional sense, but it serves the same purpose (keep clock state between reboots).

Only after the NTP sync is done, the system time (from time(nullptr), I think now() from TimeLib returns the same) will be in the correct timezone. Maybe configTime is not the best way to set the timezone, I see RTC_TZ_SET and RTC_DST_SET are used internally.

[mcspr]

If you call time(nullptr) before NTP is synced, it will return the system clock time in wrong timezone (I think it's always China time).

I am missing one thing still - how does the time from NtpClientLib gets into the time() results? Whenever time() is called in our context, it should always return seconds from boot (i.e. 0) plus timezone offset and not the real timestamp.

Looking at the routines that you mention, they are only relevant for lwip1.4 code and those RTC register writes are only triggered when sntp_time_flag is true. But, it is always false:
https://github.com/esp8266/Arduino/blob/29bedfa8429100f3ed0d5a960bd2520c9ebf15e2/tools/sdk/lwip/src/core/sntp.c#L305
https://github.com/esp8266/Arduino/blob/29bedfa8429100f3ed0d5a960bd2520c9ebf15e2/tools/sdk/lwip/src/core/sntp.c#L1167

[mcspr]

Ok. So there's one way that this may happen:
https://github.com/d-a-v/lwIP/blob/159e31b689577dbf69cf0683bbaffbd71fa5ee10/src/apps/sntp/sntp.c#L800
SNTP server list is populated by the DHCP callback, which is always enabled for LWIP2 builds:
https://github.com/esp8266/Arduino/blob/2130f3ee8c98b5b79be71031ad6cf8f9b62da18b/tools/sdk/lwip2/include/lwipopts.h#L954
First one of which you erase by using configTime "" arg, right after wifi connection finishes:
wifiConnected -> ntpLoop continues -> ntpConfigure called first time -> configTime bam

time() will return valid timestamp from the initial sync, but it will never be updated after that (if that what is really happening, this also is causing really funny issue accessing ntp server twice in a row)
Debug build of lwip + wireshark will tell for sure.

[Niek]

Wow good find, I was scratching my head why I couldn't reproduce the issue anymore at a different location - time(nullptr) suddenly always returned 0 from boot time. Apparently it depends on the DHCP server. I guess this PR can be closed?

[mcspr]

Yep, given that this is not doing the intended thing.

This does incentivize revisiting #1380 again, and maybe just dropping NTPClient when lwip2 is detected. Minimal port would be to add timezone conversion rules and minimal TimeLib compatibility layer.
There is also sntp_getserver to check if dhcp did something to the sntp server list

[Niek]

Ok closing this, I agree that #1380 would probably be the better approach

[mcspr]

btw, see esp8266/Arduino#6373
we can disable dhcp ntp with old builds already, but additional update and start time configuration depends on that PR. edit: supports is on par with ntpclientlib, only thing missing is that it will not do some additional checks that i intended to have:
http://git.savannah.nongnu.org/cgit/lwip.git/tree/src/apps/sntp/sntp.c , see SNTP_CHECK_RESPONSE checks
http://git.savannah.nongnu.org/cgit/lwip.git/tree/src/include/lwip/apps/sntp_opts.h#n111

[d-a-v]

@mcspr Why don't you PR on lwip2 repo ?

[mcspr]

I have not tested anything yet 🙃 And comments also suggest to check whether arithmetic works ok. Plus, there's also requirement to set the default time closer to today's date
("in order for the round-trip calculation to work, the difference between the local clock and the NTP server clock must not be larger than about 34 years")

And there are also some other things I wanted to try first, like customizable sntp_set_time (or esp8266/arduino settimeofday) callback a-la IDF (commit / discussion PR / forum discussion)

[d-a-v]

(Arduino has the time_is_set-like callback)

[mcspr]

Yes, but I was looking for any reason to make time_is_set cb itself overridable. The way commit above chooses to either use the default, setting time directly via settimeofday or through adjtime (which is another interesting feature). Or it can be overriden by the user app to avoid doing that at all, working as a notification of sntp success instead of system time change

[Niek]

esp8266/Arduino#6373 is now merged in core: esp8266/Arduino@ffe5476