search

iotappstory / ESP-Library

Software Distribution and Management Over the Air
GNU Lesser General Public License v2.1
124 stars 35 forks source link

setClock sets the time to GMT+3 #131

Closed abaskin closed 3 years ago

abaskin commented 5 years ago

On the ESP8266 (or the ESP32 when SNTP_INT_CLOCK_UPD is true) setClock sets the time to GMT+3. It would seem better to set the time to GMT (as opposed to a random offset) and document that the time is set on the ESP8266 in begin and loop by default.

mcgurk commented 4 years ago

If I understood correctly, whole setClock()-function can be removed. I commented out everything inside setClock()-function. ESP8266 API handles timezone and DST and updates time with NTP every hour. My code:

#include <TZ.h>
configTime(TZ_Europe_Helsinki, "pool.ntp.org", "time.nist.gov");

https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/NTP-TZ-DST/NTP-TZ-DST.ino

Onno-Dirkzwager commented 3 years ago

@abaskin this is fixed with the last PR. Set to UTC. Why this had the strange +3 as default....I don't know.... If necessary you can change the defaults in the config.h file.

@mcgurk no need to erase code.... if you want you can turn this off in the config.h file https://github.com/iotappstory/ESP-Library/blob/master/src/config.h#L55

@abaskin @mcgurk the library needs to sync time so it can use BearSSL to perform httpS requests.

So if you turn the libraries version of time sync off, make sure you update the clock (now) yourself. Otherwise you will not be able to callhome for updates!

Can we close this issue?

mcgurk commented 3 years ago

Now setClock() in https://github.com/iotappstory/ESP-Library/blob/master/src/IOTAppStory.cpp calls function configTime(SNTP_INT_CLOCK_TIME_ZONE, SNTP_INT_CLOCK_TIME_OFFSET, SNTP_INT_CLOCK_SERV_1, SNTP_INT_CLOCK_SERV_2); That is old way to set time and it doesn't support e.g. DST.

Like in example https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/NTP-TZ-DST/NTP-TZ-DST.ino time should be set with current format of configTime-function: configTime(MYTZ, "pool.ntp.org"); // maybe two timeservers can be used here if needed like in old configtime format?

My timezone is part of the year +2 and other part of the year +3. https://github.com/esp8266/Arduino/blob/master/cores/esp8266/TZ.h:

define TZ_Europe_Helsinki PSTR("EET-2EEST,M3.5.0/3,M10.5.0/4")

Why this is important?: configTime handles all time tasks. It activates and updates ntp and saves timezone with dst-setting to "eeprom". Like wifi-functions, if it is called once and "eeprom" is not cleared, it works always even without calling configTime again in new sketch. If IOTAppStory calls configTime with fixed timezone and time offset, it overwrites user settings.

My proposal is that setClock uses current format for configTime function call: configTime(SNTP_INT_CLOCK_TIME_ZONE, SNTP_INT_CLOCK_SERV) and SNTP_INT_CLOCK_TIME_ZONE defaults to TZ_Etc_GMT (TZ_Etc_GMT is defined as PSTR("GMT0"), timezone 0 without DST). After that I can modify my https://github.com/iotappstory/ESP-Library/blob/master/src/config.h to define SNTP_INT_CLOCK_TIME_ZONE TZ_Europe_Helsinki

With this arrangement, localtime()-function gives me always right local time and gmtime() gives UTC-time (usage example in NTP-TZ-DST.ino).

IOTAppStory::ntpSync and SNTP_INT_CLOCK_UPD_INTERVAL also not needed. This leads to little funny arbitrary magic number in ntpSync() code: while(now < 8 3600 2) // why 16 hours, why not 16,5 hours or 123456 hours?

Library doesn't have to try update ntp-time itself. Information when clock is set (e.g. first time after reboot) can be get from callback-function (example in NTP-TZ-DST.ino).

https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/NTP-TZ-DST/NTP-TZ-DST.ino: "SNTP is updated every hour by default"

So if you turn the libraries version of time sync off, make sure you update the clock (now) yourself. Otherwise you will not be able to callhome for updates!

This is not exactly true. If configTime is ever called at least once in history and whole flash is not erased, clock is updated every hour with NTP and timezone/dst-setting are loaded from "eeprom" by ESP8266 library. Quick test example: erase whole ESP8266 flash. Do sketch where you set Wifi-settings and call configTime. Flash that sketch and check that time is updated by printing time to console every second. After that do new sketch where you just print clock to console every second (no wifi-stuff and no configTime-stuff). Flash that (only sketch, do not erase whole flash) and you see that it prints right time to console. Time can be zero at start, because sync can take some time, but that is why there is settimeofday_cb() callback function in ESP8266 library (coredecls.h).

(Now I wait in Arduino setup()-function until time_is_set() is called and continue after that to IOTAppStory-calls. That way I can be sure that time is set when IOTAPPStory-functions is called.)

Onno-Dirkzwager commented 3 years ago

@mcgurk thankyou for your detailed feedback! Ill have to dive in later and will get back to you.

Onno-Dirkzwager commented 3 years ago

@mcgurk First of thankyou for taking the time to give us your feedback in such a detailed fashion! (and editing multiple times)

That is old way to set time and it doesn't support e.g. DST.

This was originally setup for the use of BearSSL. And taken from the following examples:

This is where the old style configTime and funny arbitrary magic numbers come from. We initially did not put enough thought into the end user.... So thankyou for making this beter!

Like in example, My timezone, Why this is important?

Yep, you convinced me. Supporting DST is the way to go.

localtime()-function gives me always right local time and gmtime() gives UTC-time

Exactly what we need.

My proposal

I fully agree and have:

You can find these changes here: https://github.com/iotappstory/ESP-Library/tree/NTP-TZ-DST

(Now I wait in Arduino setup()-function until time_is_set() is called and continue after that to IOTAppStory-calls. That way I can be sure that time is set when IOTAPPStory-functions is called.)

We / our users do not want to wait on boot. Some users don't callhome on boot and want to go ahead with whatever.... So I placed wait in the callhome and configmode parts. If settimeofday_cb() already set _timeSet = true no wait is necessary.

 Calling Home
 Wait for background NTP sync
 .
 GMT0: Sat Dec 19 02:24:56 2020

 Checking for App(Sketch) updates from: https://iotappstory.com/ota/updates
 No updates necessary!
 Calling Home

 Checking for App(Sketch) updates from: https://iotappstory.com/ota/updates
 No updates necessary!

This is not exactly true. If configTime is ever called at least once in history and whole flash is not erased, clock is updated every hour with NTP and timezone/dst-setting are loaded from "eeprom" by ESP8266 library.

I'm probably missing / doing something wrong. Or understand you completely wrong. I feel like I tried everything.... including your suggestions in simple sketches (without our library) but I always end up with 1970 etc.

It would be of great benifit if it is posible to fall back on a previously stored time untill an ntp update has taken place. A lot of our starting users reboot a lot in between testing. And it seems that after a while with every reboot it takes longer and longer to ntp update. This can be very enoying... especially when going into configmode:

 bootTimes since last update: 20
 boardMode: C
*-------------------------------------------------------------------------*
 Set NTP server settings
 - GMT0
 - pool.ntp.org
*-------------------------------------------------------------------------*
 Connecting to WiFi AP
 .....
 WiFi connected

 IP Address: 192.168.2.5
 Device MAC: F4:CF:A2:D8:45:DD
 MDNS responder started: http://iasblink-661.local
*-------------------------------------------------------------------------*
 Wait for background NTP sync
 ........................................................................................................................................................................................................ ........................................................................................................................................................................................................

 C O N F I G U R A T I O N   M O D E
 STA mode. Open 192.168.2.5

In extreem cases it can take nearly 2 minutes. I could fallback on compile time(untill ntp update) as this would be sufficient enought for making httpS calls in the first year or so.... And greatly benifit the first experience trying our libary....

Any feedback is welcome! Please check my changes and thank you for your time.

https://github.com/iotappstory/ESP-Library/tree/NTP-TZ-DST

mcgurk commented 3 years ago

Thank you for your nice replies.

Here is post from arduino-forum which was my eye opener how deeply NTP is in ESP8266 libraries: https://forum.arduino.cc/index.php?topic=655222.0 But that and my example was little sidetrack and impractical, because there is no reason to try do sketch without calling configTime at all. Maybe it doesn't even work like that with current API/libraries.

I noticed too that sometimes it gets longer time to get NTP-time. I also use "change something in code - compile and upload - test - reboot"-method multiple times in a row in short time span, but NTP sync never took so long that it would be too annoying for me.

I don't know any straightforward sollution for remembering time after reboot, but if I understood correctly, ESP8266 RTC-memory survives deepsleep and reboot. Every time time_is_set-function is called, new time could be saved to RTC-memory. And if that is set to ESP8266 internal time in setup() (or in library-setup-functions), time can be only one hour off at most after reboot/deepsleep. https://www.youtube.com/watch?v=r-hEOL007nw

Onno-Dirkzwager commented 3 years ago

@mcgurk I made some more changes as this has to be compatible with the ESP32. It's not turned on by default for the ESP32 as we do not need it for certificate validation. But some of our users use it for their projects...

Their are some differences between the platforms:

We want to keep the library as free as possible from platform definitions whithin the main code. So I moved the ntp code to separate files and let the include.h decide which to include.

I don't know any straightforward sollution for remembering time after reboot, but if I understood correctly, ESP8266 RTC-memory survives deepsleep and reboot.

Yes I thought about this to as we use it for storing modes (N & C) and bootimes. I have not implemented this yet. But have included Settimeofday functions in the files mentiond above. So users can already use them if they want to. And we may decide to in the future.

Please test the NTP-TZ-DST branche and let me know if we can close this issue and add the code to the main branche.

Onno-Dirkzwager commented 3 years ago

@mcgurk added the "ESP8266 RTC-memory" solutions as we talked about. This drastically brings down the wait time in slow NTP sync situations. And lets us enter config directly instead of having to wait.

So it's to bad that something like this is not in the default libraries. But the solution we came up with works perfectly!

https://github.com/iotappstory/ESP-Library/blob/NTP-TZ-DST/src/espressif/esp8266/NtpTimeSync.cpp#L31

Thankyou for your feedback, suggestions and brainstorm session. I'm going to merge this with the master branch and will close this issue. Feel free to reopen if deemed necessary.