-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
setClock sets the time to GMT+3 #131
Comments
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:
https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/NTP-TZ-DST/NTP-TZ-DST.ino |
@abaskin this is fixed with the last PR. Set to UTC. @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? |
Now setClock() in https://github.com/iotappstory/ESP-Library/blob/master/src/IOTAppStory.cpp calls function 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: My timezone is part of the year +2 and other part of the year +3. Why this is important?: My proposal is that setClock uses current format for configTime function call: 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. 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:
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. (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.) |
@mcgurk thankyou for your detailed feedback! |
@mcgurk
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!
Yep, you convinced me. Supporting DST is the way to go.
Exactly what we need.
I fully agree and have:
You can find these changes here: https://github.com/iotappstory/ESP-Library/tree/NTP-TZ-DST
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.
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:
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. |
Thank you for your nice replies. Here is post from arduino-forum which was my eye opener how deeply NTP is in ESP8266 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. |
@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.
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. |
@mcgurk 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. |
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.
The text was updated successfully, but these errors were encountered: