bperrybap commented 6 years ago

Platform

Hardware: [ESP-12]
Core Version: [2.4.1]
Development Env: [Arduino IDE]
Operating System: [Linux Mint 17.3]

Settings in IDE

Module: [Wemos D1]
Flash Mode: [qio|dio|other]
Flash Size: [4MB/1MB]
lwip Variant: [v1.4|v2 Lower Memory|Higher Bandwidth]
Reset Method: [ck|nodemcu]
Flash Frequency: [40Mhz]
CPU Frequency: [80Mhz|160MHz]
Upload Using: [OTA|SERIAL]
Upload Speed: [115200|other] (serial upload only)

Problem Description

The are some serious issues with how timezone offsets are being handled in various time functions. There was mention of some issues in the discussion about the NTP-TZ-DST.ino demo sketch in issue #3835 That issue mentioned issues with gmtime() and localtime() but the issues are much more serious. Here is the behavior I've seen - using the NTP-TZ-DST.ino demo sketch with a few tweaks to see the behavior of gettimeofday() as well. The results were the same regardless of whether NTP was used or the "fake" RTC mode was used.

The time_t returned by time() does not equal the tv_sec value returned by gettimeofday() They should always be the same value.
gettimeofday() in the tv_sec member returned, is always supposed to return the number of seconds since the epoch without any consideration or alteration based on local timezone. This is working correctly.
time() is always supposed to return the number of seconds since the epoch without any consideration or alteration based on local timezone. This is not working correctly time() is returning a time_t that has been adjusted/modified and is offset by the specified timezone information. This is incorrect. time() has modified that actual time_t timestamp it returns. time_t timestamp values are not be modified for the local time. Any correction for local time should be handled by localtime() or ctime()
gmtime() and localtime() are currently generating the same broken down time elements. This is incorrect. They both take the time_t value and break down it into its components based on the time_t value provided with no adjustments for timezone. The issue is actually with localtime() localtime() is behaving just like gmtime() and is not adjusting the time for the timezone offsets. This is easily be demonstrated by handing both localtime() and gmtime() a zero value time_t. Both will generate the identical GMT time at the epoch.

These issues have likely been masked/hidden since many users that are setting timezone information are then calling localtime() with the time_t value from time(). While the fields created by localtime() when handed a time_t from time() will be correct, the time adjustment is being done incorrectly since the time_t was adjusted by time() rather than localtime() doing the timezone adjustment.

localtime() will fail to work correctly when handed the tv_sec value from gettimeofday() (it will show GMT) gmtime() will fail to work correctly when handed the time_t from time() (it will show local time since time() is returning an incorrect time_t value)

I'm not sure how this can be fixed since it appears to be an issue in code outside of this source tree. It seems like until it is fixed, users using the time code with timezone offsets need to know and understand the issues. Until it gets fixed, perhaps some notes and big warnings could be placed in the comments in the NTP-TZ-DST.ino demo sketch

5chufti commented 6 years ago

nice to see someone else barking up that tree ... using it like this should get you going

  configTime(0, 0, "pool.ntp.org");
  setenv("TZ", "CET-1CEST,M3.5.0,M10.5.0/3", 3);
  tzset();

and give correct values for gmtime() and localtime() with real dst, but only using core >= 2.4 and lwip2.0. For the sake of backward compatibility they ..... you allready found out.

bperrybap commented 6 years ago

@5chufti Not sure what you mean by this:

For the sake of backward compatibility they ..... you allready found out.

It seems that there is some odd behavior in the timezone handling code that needs to be documented and, IMO, corrected. And the NTP-TZ-DST.ino demo sketch disparately needs some updates to properly handle timezones (which it isn't doing now) and to deal with the existing code along with some clear comments about explaining how to properly use the functions for timezones and how to avoid using the misbehaving functions and to avoid modifying/corrupting the actual timestamp. A big issue is that the example sketch is not doing things correctly and it also needs lots of information in the comments to help people understand how the library code works and in some cases how it works incorrectly. (I'd be happy to submit a PR to update this sketch, with an updated version that actually works, with lots of comments explaining what works and what is broken, if people agree it would be useful)

Your example works to correct gmtime() and localtime() when NTP is being used, so I had to experiment a bit for the RTC case. And I really dug into the code in time.c and sntp-lwip2.c The "fake RTC" build of the example sketch (not using ntp) can also be made to work properly as well by not using the timezone fields and using the TZ variable.

The sntp-lwip2.c code is a bit twisted up and appears to have some issues. The main one being that it appears that the way sntp-lwip2.c is using timezone information is breaking some of the libc functions like time(), gmtime(), localtime(), and gettimeofday() by adjusting the actual time_t timestamp by the timezone offset. It seems like libc and and code in sntp-lwip2.c are both handling timezone offsets, but the code in sntp-lwip2.c is not handling timezone offsets correctly in all cases.

Also, if you call gettimeofday() and pass in a timezone pointer to get the current timezone offset, it will always be zero even when using the TZ timezone variable which is not correct.

It does appear that if you avoid setting/using any of the timezone offset information used by settimeofday(), configtime(), and sntp_set_timezone() to keep sntp-lwip2.c from messing with the timestamp, things mostly work, other than gettimeofday() can't get the current timezone offset.

Not sure how to get this stuff fixed.

5chufti commented 6 years ago

no sense in fixing things broken by design. the two values for timezone and dst are just offsets that are senslessly applied to the system time, without any TZ rules. So I ask: if one has to do the dst desicion before handing over the offset, why have two offsets at all? "system time" should allways be utc, so no need for providing offsets, just provide function to set ntp servers and/or initial time from hw-clock; the rest should be done via standard c functions.

bperrybap commented 6 years ago

I don't follow. I couldn't tell if you were referring to the esp8266 Arduino core code, or the unix time functions dealing with timezones and dst in general.

The system time stamp returned as a time_t by functions like time() should always be utc with no offset or adjustment of the local timezone - if it isn't utc, then something is broken or something is being done/used incorrectly.

You also have to take into consideration that setting the time from an RTC may not be just a one time thing. It may need be periodic, particularly in an embedded environment as the RTC may be much more accurate at time keeping than the local system that has no network connectivity.

IMO, timezone rules and the current timezone offset/dst are not the same thing and serve very different purposes. A timezone rule is optional and is how you can programmatically determine the current timezone offset and dst setting. Once timezone and dst are configured, those two can be easily (and always) be applied to the utc time_t "system-time" time stamp whenever local time information in broken down form is requested/needed. If there is no need or use of local time values, then timezone rules, offsets, dst, don't matter as they won't come into play. i.e. a timezone rule may be expensive while simple offsets and dst adjustments are simple. It could be that the timezone rule is only run periodically like on the hour to re-calculate current offset and dst setting. Or perhaps the timezone is manually set by an operator and the dst setting is set by a cron job that runs twice a year to set it appropriately - in that case no timezone rule is used and no calculation is done, dst is set/un-set when necessary.

There can be an issue initially setting the system time from an RTC clock, since the RTC may not be set to utc but rather local time. In that situation a conversion from broken down local time to a utc time_t has to be done somewhere. But for that situation, I tend to use mktime() to create a time_t which has a local time offset, then offset it by the proper amount to get to a proper utc time_t and then use it.

I have never seen a need to ever have an offset time_t value like what is being done in sntp-lwip2.c although I have seen many people incorrectly use time_t values offset to local time instead of using proper timeezone offset and dst settings especially in cases on embedded systems where the time libraries didn't include timezone support - like the popular Arduino Timelib library.

devyte commented 6 years ago

@5chufti do you mean the Arduino time handling design is broken in general, or our specific core code for handling time is broken? You both seem to have a firm grasp of how the time and timezone code should behave. How about investigating a rework and making a PR?

bperrybap commented 6 years ago

I don't believe the design is broken - at least not the way the unix/BSD/linux time function APIs are defined. It does seem like some of the code in time.c and sntp-lwip2.c is not behaving properly.

What I've seen is that some code attempts to adjust for timezone offsets by mucking with the actual time_t timestamp. It shouldn't be done that way. While that can make things like localtime() appear to work, in reality it is doing the local time correction incorrectly so that something else breaks. For example, the 3rd party Arduino Time/TimeLib library (which is not this code) has no support for timezone at all. So many users just set the internal time_t time to represent the local time rather than utc. i.e. they create a time_t value that represents their local time rather than utc. For most users that works just fine since the time_t is never used for anything but working locally with local time. However, it is not using the correct time_t timestamp, so if the time_t value from were used as a timestamp or used in anyway with anything external to the device, it can create problems.

In the case of the sntp-lwip2.c code, when using a timezone offset, it modifies the actual timestamp. The issue then becomes that various time API calls return and use different timestamp values - that should all be the same. Like in the case of time() returning a different timestamp than gettimofday() - which is wrong. The C library functions have no knowledge of this timezone offset information in sntp-lwip2.c so they think there is no timezone offset which causes gmtime() and locatltime() to show the same time. Since the atual timestamp was modified by the timezone offset, gmtime() shows local time rather than utc. And if using the TZ variable, the libC functions will know about the proper offset, but time() in time.c doesn't since it getting the timestamp from the sntp-lwip2.c code which has its own timezone offset information and has mucked with the timestamp to offset it by the timezone offset.

I'd be more than happy to propose a re-work, but to make sure it is all correct, I also need to see the libC library code for the time functions. Is that source code available somewhere? All I saw in the espressif github tree I found were .a archive files and no actual source code.

I think if done carefully it could preserve compatibility with existing code that uses the timezone offsets provided by the various functions sntp-lwip2.c. This is because I believe that most users will be setting the timezone and then using the time_t value from time() to pass to localtime() not ever realizing that once you set timezone information in the sntp-lwip2.c functions, the time_t values you are getting from time() are incorrect since they are getting proper looking information from locatltime().

There does seem to be a bit of issue with the way the dst offset is handled, and I'd have to look much closer at that in sntp-lwip2.c along with the libC code make sure they all play nice together.

The main thing is does anybody know where I can find the source code to the libC time functions supplied in the esp8266 core?

igrr commented 6 years ago

@bperrybap newlib code used is here: https://github.com/igrr/newlib-xtensa/.

The issue you describe is explained as follows:

time function is implemented inside the core here, which is incorrect. time defined in newlib should be used. The core should provide implementation of _gettimeofday_r, and use time implementation from newlib (which calls _gettimeofday_r (source here)). However, time implementation in newlib has been disabled here (yes, at that time NonOS SDK provided its own time function, so i didn't have a better solution). The fix is pretty obvious — revert change in newlib, rebuild it, remove time implementation from the core. Result: time and gettimeofday will agree.

igrr commented 6 years ago

Another thing, the time support in the core (_gettimeofday_r) should not be calling into sntp_ functions to ask for the current time. The core should be doing timekeeping on its own, and sntp library should call settimeofday or adjtime when needed to adjust the time.

bperrybap commented 6 years ago

@igrr Thank you for the links. I believe that there are other issues related to tm_isdst and dst offset handling in the sntp-lwip2.c code. I'll go through all the code in detail and see if I can figure that out as well.

devyte commented 6 years ago

@bperrybap great! Thank you for picking this up. You may also want to look at the example, in case you haven't done so.

bperrybap commented 6 years ago

@devyte The NTP-TZ-DST.ino example is the one I've been playing with and I've mentioned a few times that needs a bit of polishing and cleaning up and additional informational comments added.

A how it gets updated will depend on the final updates done to the time.c , sntp-lwip2.c code, and core library code since if those updates fix the issues, the example sketch won't need work arounds and added comments describing the issues and how to work around them.

RudyFiero commented 6 years ago

I had struggled with the NTP-TZ-DST.ino. It took me some time to figure out that most of it has nothing to do with setting the time. Some things that seem to be relevant but no explanation as to the need or purpose. ESP.eraseConfig(); I have no idea why this is there. So do I

This example shows how to read and set time,

So why is the following included? It has nothing to do with time that can be set. Sure it is good to know, as most things are, but it isn't relevant to the stated purpose.

// for testing purpose:
extern "C" int clock_gettime(clockid_t unused, struct timespec *tp);

And this really confused me.

  // human readable
  Serial.print(" ctime:(UTC+");
  //  **Serial.print((uint32_t)(TZ * 60 + DST_MN));**  // TZ*60 + DST_minutes
  //  Serial.print("mn) ");
  Serial.print(ctime(&now));

I couldn't understand what the meaning of the large number. Large number? Yes, if TZ is negative (-6 for me), and using (uint32_t) produces a large positive number. It should have been (int32_t).

When I saw the example program I felt it was exactly what I needed. I want to have a hardware real time clock as a backup when the internet is not available. No external time libraries needed, perfect. And I wanted sub-second timestamps, and it is all here. But it has been a lot of work trying to figure out what in the example that I don't need. (I'm not a programmer)

Okay, rant over. I'm thrilled that this is going to be looked at. I can't wait to see the result.

d-a-v commented 6 years ago

@RudyFiero I originally made this "exhaustive" example to check time functions along with updated in NTP code (lwip2's sntp) and several fixes where brought (not only by me).

This example shows how to read and set time,

So why is the following included? It has nothing to do with time that can be set extern "C" int clock_gettime(clockid_t unused, struct timespec *tp);

It has to do with reading time with standard and implemented libc functions, as mentioned.

and using (uint32_t) produces a large positive number. It should have been (int32_t).

Very true. I live in UTC+1 and I missed this, thanks for spotting.

@5chufti We could have patched lwip1.4 to get the same behaviour. lwip1.4 is already a patched version from espressif and I was not sure it be wise to go that risky way at that time. Risky because lwip1.4 is supposed to get updates from espressif.

@bperrybap Thanks for getting into this. Any help to improve / comment this example sketch is welcome. Maybe this sketch needs to be splitted into several pieces for clarity. The core deserves to become more accurate with time handling. The hard points will be at least to synchronize with newlib's time() fix, to keep backward compatibility with configTime() and maybe with lwip1.4's way of handling time.

Also, as @5chufti told, the sketch (or another one) lacks the tzset() example which is very handy. I use for instance "CET-1CEST,M3.5.0,M10.5.0/3" (which is CEST) and it accurately follows TZ. We should have a script that import these data to some defines and show them to users in an example as @igrr suggested in #1679(comment) (source of all these changes).

RudyFiero commented 6 years ago

The problem I had was that I was not familiar with time routines in general. Millis of course. But date/time I was not very familiar with. I didn't know what belonged with who, or what belonged together. It took a while before I figured out there were independent methods for getting time. But I still couldn't figure out who was in charge. Some more comments would have helped.

I'm not a programmer. I design electronics for a living, and have mostly had other people who did the firmware/software. But now I'm trying to do my own stuff at home. And my brain ain't as young as it used to be. Its harder now to deal with a lot of abstraction and complexity.

Booli commented 6 years ago

How do you set the time manually with the new time core?

Scenario: I connect to a MQTT server, which sends me a unixtime stamp every so often. I want to set the "internal clock" to that time stamp, but I can't figure out how to do it. Must be simple right?

I would guess something like settimeofday(unixtimestamp)

RudyFiero commented 6 years ago

You are right. Look at the NTP-TZ-DST.ino example sketch. https://github.com/esp8266/Arduino/blob/master/libraries/esp8266/examples/NTP-TZ-DST/NTP-TZ-DST.ino

define NTP0_OR_LOCAL1 1 // 0:use NTP 1:fake external RTC

When set to local it uses

define RTC_TEST 1510592825 // 1510592825 = Monday 13 November 2017 17:07:05 UTC

Booli commented 6 years ago

Yeah I just moved that into a function

void SetTime(time_t time_stamp) {
  timeval tv = { time_stamp, 0 };
  timezone tz = { 0, 0 };
  settimeofday(&tv, &tz);
}

But my time(NULL) stays 0, do I need to be running anything other that 2.4.1 core?

I checked with using the NTP version of the sketch, with configTime, and then it works. But I want to use the RTC/provide my own timestamp.

RudyFiero commented 6 years ago

Your function works for me. Are you sure you are passing it something other than 0?

I'm using 2.4.1 on the computer I tested your function.

Booli commented 6 years ago

Tried the following, but to no avail:

/*
  Minimal time set test
*/

#include <time.h>                       // time() ctime()
#include <sys/time.h>                   // struct timeval
#include <coredecls.h>                  // settimeofday_cb()

////////////////////////////////////////////////////////
#define TZ              1       // (utc+) TZ in hours
#define DST_MN          60      // use 60mn for summer time in some countries

#define RTC_TEST     1510592825 // 1510592825 = Monday 13 November 2017 17:07:05 UTC
////////////////////////////////////////////////////////

#define TZ_MN           ((TZ)*60)
#define TZ_SEC          ((TZ)*3600)

timeval cbtime;         // time set in callback
bool cbtime_set = false;

void time_is_set(void) {
  gettimeofday(&cbtime, NULL);
  cbtime_set = true;
  Serial.println("------------------ settimeofday() was called ------------------");
}

void setup() {
  Serial.begin(19200);
  settimeofday_cb(time_is_set);

  time_t rtc = RTC_TEST;
  timeval tv = { rtc, 0 };
  timezone tz = { TZ_MN + DST_MN, 0 };
  settimeofday(&tv, &tz);

}

timeval tv;
time_t now;

void loop() {

  gettimeofday(&tv, nullptr);
  now = time(nullptr);

  // EPOCH+tz+dst
  Serial.print(" time:");
  Serial.print((uint32_t)now);
  Serial.println(gettimeofday(&tv, nullptr));

  // simple drifting loop
  delay(500);
}

outputting:

 time:00
 time:00
 time:00
 time:00
 time:00
 time:00
 time:00
 time:00

And also not hitting the time_is_set callback I think.

RudyFiero commented 6 years ago

I took your code and ran it as is. This was my output.

⸮b⸮SK⸮mBB⸮⸮⸮⸮⸮qGa⸮⸮R⸮aEa⸮⸮⸮⸮------------------ settimeofday() was called ------------------
 time:15106000250
 time:15106000250
 time:15106000260
 time:15106000260
 time:15106000270
 time:15106000270
 time:15106000280
 time:15106000280
 time:15106000290
 time:15106000290
 time:15106000300
 time:15106000300
 time:15106000310
 time:15106000310
 time:15106000320
 time:15106000320
 time:15106000330
 time:15106000330
 time:15106000340
 time:15106000340

klaasdc commented 6 years ago

I think I run into this using core 2.4.2, but I'm not sure if I'm doing something wrong or if it is a bug:

I am using a DS3231 RTC to have a reliable timesource before NTP can take over. Inspired by the NTP-TZ-DST example, I do this as follows:

time_t rtc = rtcTimeNow.Epoch64Time();
timeval tv = { rtc, 0 };
float dst = localDst ? 60 : 0;
timezone tz = { localUtz*60.0 + dst, 0};
settimeofday(&tv, &tz);

This seems to work fine, and the internal clock reports a correct time.

Some time later, the network becomes available and the NTP client acquires time and sets the internal clock. When that happens, I periodically write the internal clock back to the RTC using the provided system callback function. Inside of it, I put:

time_t curTime = time(nullptr); //Should give UTC time but does not?
RtcDateTime newTime = RtcDateTime(0);
newTime.InitWithEpoch64Time(curTime);
Rtc.SetDateTime(newTime);

Because of the call to time(), this should set the RTC to UTC, so on the next system boot the time is read from RTC, and time-zone adjusted to give correct internal clock...

But in my case, the time is now ahead, until the NTP acquires the time again. As described by the OP, the call to time() gives a local time instead?

So I now work around by calling gettimeofday(), which gives the correct UTC, and then get its time_t

//WORKAROUND: Instead of calling time(), to get UTC, use gettimeofday()
timeval tv;
gettimeofday(&tv, nullptr);
time_t curTime = tv.tv_sec;

I'm worried that I'm making a mess of the time handling... And next thing is verifying DST support...

bperrybap commented 6 years ago

The timezone offset stuff is really broken and is not usable. You want to avoid it and disable it. You don't need it anyway to get proper timezone support including DST support. The key is you must disable all the timezone offsets by explicitly setting them to zero, and then use the TZ string support.

Example code:

RTC-Clock-MinExample.ino.zip --- bill

NOTE/UPDATE for those that see this and would like an example. There is a better updated example later in this issue discussion, here: https://github.com/esp8266/Arduino/issues/4637#issuecomment-703048715 --- bill

klaasdc commented 6 years ago

The timezone offset stuff is really broken and is not usable. You want to avoid it and disable it. You don't need it anyway to get proper timezone support including DST support. The key is you must disable all the timezone offsets by explicitly setting them to zero, and then use the TZ string support.

Example code:

RTC-Clock-MinExample.ino.zip --- bill

That saved me a lot of time, thank you! I think your example should replace the one that is currently supplied with the Arduino core.

AmrutaCh commented 6 years ago

The timezone offset stuff is really broken and is not usable. You want to avoid it and disable it. You don't need it anyway to get proper timezone support including DST support. The key is you must disable all the timezone offsets by explicitly setting them to zero, and then use the TZ string support.

Example code:

RTC-Clock-MinExample.ino.zip --- bill

Thanks for the solution. If settimeofday is called from any other cpp file it gives compilation errors.

In file included from sketch\module_ntp.h:16:0,
                 from sketch\module_ntp.cpp:1:

C:\Users\Amruta\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266/coredecls.h:19:24: error: variable or field 'tune_timeshift64' declared void
 void tune_timeshift64 (uint64_t now_us);
                        ^
C:\Users\Amruta\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266/coredecls.h:19:24: error: 'uint64_t' was not declared in this scope

sketch\module_ntp.cpp: In function 'void setup_NTP()':
module_ntp.cpp:35: error: 'settimeofday' was not declared in this scope
     settimeofday(&tv, &tz);
                          ^
exit status 1
'settimeofday' was not declared in this scope

error: 'uint64_t' was not declared in this scope can be fixed by adding #include "os_type.h" to coredecls.h. (I know its not very wise to change SDK files but I couldn't find any other solution)

Couldn't find the solution for 'settimeofday' was not declared in this scope Any suggestions on how to fix this?

Test Code: ntpTest.zip

Environment:

Arduino 1.8.5
ESP8266 SDK 2.4.2

gojimmypi commented 5 years ago

fwiw - I found that some named time zones work better than others. I've had pretty good success with the ones here (e..g. "CET-1CEST-2" instead of "Europe/Paris") like this:

        configTime(0, 0, "pool.ntp.org", "time.nist.gov"); // see https://github.com/esp8266/Arduino/issues/4749#issuecomment-390822737

        // Starting in 2007, most of the United States and Canada observe DST from
        // the second Sunday in March to the first Sunday in November.
        // example setting Pacific Time:
        // setenv("TZ", "PST8PDT,M3.2.0/02:00:00,M11.1.0/02:00:00", 1); // see https://users.pja.edu.pl/~jms/qnx/help/watcom/clibref/global_data.html
        //                         | month 3, second sunday at 2:00AM
        //                                        | Month 11 - first Sunday, at 2:00am  
        // Mm.n.d
        //   The dth day(0 <= d <= 6) of week n of month m of the year(1 <= n <= 5, 1 <= m <= 12, where 
        //     week 5 means "the last d day in month m", which may occur in the fourth or fifth week).
        //     Week 1 is the first week in which the dth day occurs.Day zero is Sunday.

        // this one is off by an hour:
        // setenv("TZ", "Europe/Paris,M3.5.0/02:00:00,M10.5.0/02:00:00", 1); // see https://users.pja.edu.pl/~jms/qnx/help/watcom/clibref/global_data.html
        //                             | month 3, last Sunday at 2:00AM
        //                                             | month 10 - last Sunday, at 2:00am  
        //                                                                | overwrite = 1
        // 

        // this one seems to work better!
        setenv("TZ", "CET-1CEST-2,M3.5.0/02:00:00,M10.5.0/03:00:00", 1);
        //                         | month 3, last Sunday at 2:00AM
        //                                         | month 10, last Sunday, at 2:00am  
        //                                                            | overwrite = 1
        tzset();
        Serial.print("Waiting for time(nullptr).");
        i = 0;
        while (!time(nullptr)) {
            Serial.print(".");
            delay(1000);
            i++;
            if (i > MAX_TIME_RETRY) {
                Serial.println("Gave up waiting for time(nullptr) to have a valid value.");
                break;
            }
        }

Note that I'm using ESP32, only adding the named timezone reference here that others may find useful.

minida28 commented 5 years ago

AFAIK using the timezone name or ID such as Europe/Paris and pass it to setenv() is incorrect; that might explain why you got wrong result. You must use specified POSIX timezone format, for example CET-1CEST-2 for Paris in Europe or NPT-05:45 for Kathmandu in Asia, etc.

IBM has good article in explaining POSIX timezone format. https://www.ibm.com/developerworks/aix/library/au-aix-posix/index.html.

Someone has created a GitHub project to maintain posix time zone strings DB. https://github.com/nayarsystems/posix_tz_db

And @d-a-v has made a nice small utility for esp8266 based on the above project here. https://github.com/d-a-v/EspGoodies/blob/master/src/utility/TZ.h

Personally I put the the DB file (zone.csv or zone.json) in SPIFFS and parse it manually if needed.

On top of that, I think there is still an issue with the this setenv() and posix format stuffs:

If I pass the posix timezone string in quoted form such as <+0630>-6:30 or <+0545>-5:45, I'm still getting wrong result. I thought this was some nonstandard extension conflicting with standard handling of the TZ variable, but in fact it's documented in current POSIX as a quoted form. http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

At the moment I workaround that either by removing all the quoted form timezone strings or pick the ones I need and edit them manually.

fwiw - I found that some named time zones work better than others. I've had pretty good success with the ones here (e..g. "CET-1CEST-2" instead of "Europe/Paris") like this:

        configTime(0, 0, "pool.ntp.org", "time.nist.gov"); // see https://github.com/esp8266/Arduino/issues/4749#issuecomment-390822737

        // Starting in 2007, most of the United States and Canada observe DST from
        // the second Sunday in March to the first Sunday in November.
        // example setting Pacific Time:
        // setenv("TZ", "PST8PDT,M3.2.0/02:00:00,M11.1.0/02:00:00", 1); // see https://users.pja.edu.pl/~jms/qnx/help/watcom/clibref/global_data.html
        //                         | month 3, second sunday at 2:00AM
        //                                        | Month 11 - first Sunday, at 2:00am  
        // Mm.n.d
        //   The dth day(0 <= d <= 6) of week n of month m of the year(1 <= n <= 5, 1 <= m <= 12, where 
        //     week 5 means "the last d day in month m", which may occur in the fourth or fifth week).
        //     Week 1 is the first week in which the dth day occurs.Day zero is Sunday.

        // this one is off by an hour:
        // setenv("TZ", "Europe/Paris,M3.5.0/02:00:00,M10.5.0/02:00:00", 1); // see https://users.pja.edu.pl/~jms/qnx/help/watcom/clibref/global_data.html
        //                             | month 3, last Sunday at 2:00AM
        //                                             | month 10 - last Sunday, at 2:00am  
        //                                                                | overwrite = 1
        // 

        // this one seems to work better!
        setenv("TZ", "CET-1CEST-2,M3.5.0/02:00:00,M10.5.0/03:00:00", 1);
        //                         | month 3, last Sunday at 2:00AM
        //                                         | month 10, last Sunday, at 2:00am  
        //                                                            | overwrite = 1
        tzset();
        Serial.print("Waiting for time(nullptr).");
        i = 0;
        while (!time(nullptr)) {
            Serial.print(".");
            delay(1000);
            i++;
            if (i > MAX_TIME_RETRY) {
                Serial.println("Gave up waiting for time(nullptr) to have a valid value.");
                break;
            }
        }

Note that I'm using ESP32, only adding the named timezone reference here that others may find useful.

6373 brings in all timezone definitions.

NTP-TZ-DST example is updated, and core API extended.

the time_is_set() callback function is very sensitive. I tried to add a short beep so I would hear when the NTP update happened and got Exception 9

This callback has to be considered as if it is an ISR, but the example in #6373 uses now a scheduled function in which yield(), delay(), print() and anything else are allowed (the former NTP-TZ-DST example was incorrectly using print() in the callback).

6273 brings only additions and do not change the core API (harmless for external libraries like @bxparks's).

d-a-v commented 5 years ago

or how to call sntp.sync() to force a second NTP update

You can try

#include <lwip/apps/sntp.h> // sntp_stop() sntp_init()

RudyFiero commented 5 years ago

but the example in #6373 uses now a scheduled function in which yield(), delay(), print() and anything else are allowed (the former NTP-TZ-DST example was incorrectly using print() in the callback).

Can you post a better example?

d-a-v commented 5 years ago

You mean, newer than the updated one proposed within the PR ?

RudyFiero commented 5 years ago

You said that the NTP-TZ-DST example was incorrectly using print() in the callback). And I wanted to see what a correctly updated file would be. Above your link #6373 showed the output but I don't know what you have done to produce that output.

Or any example that you feel would be better to show how to use the functions. A lot of us are not programmers, and we can use all the help we can get. It often isn't obvious by reading the libraries.

d-a-v commented 5 years ago

Right, the output demo of the updated example using scheduled functions is now proposed in https://github.com/esp8266/Arduino/pull/6373#issuecomment-527236204. Source code of this example is there. Please let me know if something is unclear.

RudyFiero commented 5 years ago

I tried to compile the new example code but the included file TZ.h is missing. Where can I get this file? I didn't find it in this repository. I looked at https://www.iana.org/time-zones but I didn't get any further.

I think I found it.

I finally realized that the code is only at the pull request stage. (after getting compile errors) I guess I will have to wait a while.

d-a-v commented 5 years ago

To try a PR I use this script:

#!/bin/sh
test -z "$2" && { echo "syntax: $0 #PR someName"; exit 1; }
set -ex
git checkout master
git branch -D pr-$1-$2
git fetch origin pull/$1/head:pr-$1-$2
git checkout pr-$1-$2
echo "current branch:"
git branch | grep pr-$1-$2
echo "use 'git checkout master' when done"

It works on any github repository. For #6373 you would

$ PR 6373 iana

and restart arduino IDE.

Tech-TX commented 5 years ago

or how to call sntp.sync() to force a second NTP update

You can try
#include <lwip/apps/sntp.h> // sntp_stop() sntp_init()

Using sntp_stop() sntp_init() changed the behavior in 2.5.2 (stable) from integer multiples of 15 seconds for the next sntp update to (some random time within the next minute). I haven't beat that up too much to find out the worst-case update time. However, there's a different way to force an sntp update: every time I reconfigure/reconnect WiFi it always causes an sntp update, regardless of the setting of NTP_UPDATE_DELAY. I'd #defined that to 12 hours as I don't care if the system clock drifts a little.

In my code, I do initial configurations in setup() (including sntp) and then inside loop() the first thing I do is wake up and reconfigure the modem, as I put it in modem sleep at the bottom of loop() and then delay(25 minutes) for lower power. By the time I need it (seconds after loop() begins), the system clock is always re-sync'd to NTP. I haven't figured out yet which part of the WiFi reconfiguration is causing the sntp update, but I've seen it happen on every 30 minute loop() overnight.

This isn't a problem, merely an observation. :-) I do a full modem reconfigure + OTA setup in loop(), even though the modem was configured and connected in setup() without OTA when I got the first sntp update.

d-a-v commented 5 years ago

SNTP default renew interval is one hour and it hasn't changed (can't be changed without lwIP recompilation).

To remember: When the dhcp server knows about sntp servers, a sntp request is issued at every WiFi reconnection and also at every dhcp lease renewal (edit: I need to double check again this second reason) Check the lease time on your dhcp server.

bperrybap commented 5 years ago

SNTP default renew interval is one hour and it hasn't changed (can't be changed without lwIP recompilation).

So maybe its time to finally support sntp_set_update_delay() ? :-) https://github.com/esp8266/Arduino/issues/5938

d-a-v commented 5 years ago

Looks like, from #5938, we still have https://github.com/d-a-v/esp82xx-nonos-linklayer/pull/32 pending.

Tech-TX commented 5 years ago

SNTP default renew interval is one hour and it hasn't changed (can't be changed without lwIP recompilation).

Ahhh, you're absolutely correct. I missed the warning when I compiled it: (warning: "SNTP_UPDATE_DELAY" redefined [enabled by default]). I saw the #ifndef in sntp.c and thought I could override it, and didn't know lwip2 was a binary blob.

To remember: When the dhcp server knows about sntp servers, a sntp request is issued at every WiFi reconnection and also at every dhcp lease renewal (edit: I need to double check again this second reason) Check the lease time on your dhcp server.

Adding extra SNTP updates when I reconnect WiFi or the DHCP lease renews doesn't cause me any trouble, I was only trying to minimize updates I didn't need... it's always a good idea to be a polite 'netizen when possible. :-) Thanks for your time and efforts!

Rob58329 commented 4 years ago

I am aware this is an old tread, but I have also noticed an error in the ESP8266 “time.h” functions related to the “_daylight” variable. (This with the latest “https://github.com/esp8266/Arduino” as at 22Feb2020.) - Update: My Bad - this was caused by an error on my part - I have detailed the actual behavour below for if anyone else is interested.

"_daylight" appears to be a boolean variable used to tell "localtime_r" and "localtime" to add 1 hour to the time to allow for Daylight Saving (but "_daylight=true" is ONLY acted on between specific dates).

In the ESP8266, it appears to always be set to true by "configTime(x,y,"pool.ntp.org",NULL,NULL);" (irrespective of what x and y are).

If no valid "setenv(..); tzset();" is used, then this "_daylight=true" variable appears to cause "localtime_r" and "localtime" to add 1 hour onto the displayed time between 8March 31Oct(inclusive). (ie the US/Canada daylight saving dates).

However, if a valid "setenv(..); tzset();" is set, then the "setenv/tzset" command will adjust the "_daylight" variable according to what the "TZ" string sets, AND seems to also adjust the dates between which "localtime_r" and "localtime" will act on a "_daylight=true" setting.

Thus for the ESP8266, if you wanted the US/Canada daylight saving dates to be used (with for example an GMT+8hr shift), you could just use: configTime(3600*8,0,"pool.ntp.org",NULL,NULL); and NOT set "TZ", and this would still automatically adjust for daylight saving between 8March and 31Oct.

However, if you want different daylight saving dates, then using a valid "setenv/tzset" command such as: setenv("TZ", "PST8PDT,M3.2.0,M11.1.0", 1); tzset(); or setenv("TZ", "GMT0BST,M3.5.0/1,M10.5.0",1); tzset(); which will automatically reset the "_daylight" variable AND set the dates between which the "localtime_r" and "localtime" will act on a "_daylight=true" setting.

(My error was using an incorrect "TZ" string which was therefore being ignored, which then caused the ESP8266 to revert to the US/Canada daylight saving dates. - If you have a correct "TZ" string, then manually changing "_daylight" is not required.)

(For the ESP32, the "configTime(..)" command does not set "_daylight=true", so never causes a daylight saving hour to be added if no valid "TZ" string is available.)

My updated test-program is as follows:

void setup() {
  Serial.begin(74880); Serial.println();
  time_t now;
  tm tm_struct; 
  //now=1583582400; // The ESP8266 correctly reports this as "Sat Mar  7 12:00:00 2020"
  now=1583668800; // This is "Sun Mar  8 12:00:00 2020", but if using US/Canada daylight saving reports as "Sun Mar  8 13:00:00 2020"

  Serial.print("tzname="); Serial.print(*tzname); Serial.print(", timezone="); Serial.print(_timezone); Serial.print(", _daylight="); Serial.println(_daylight);

  localtime_r(&now, &tm_struct); 
  Serial.print("Time Stamp #1="); Serial.print(asctime(&tm_struct)); //  Gives correct "Time Stamp #1=Sun Mar  8 12:00:00 2020"
  Serial.print("tzname="); Serial.print(*tzname); Serial.print(", timezone="); Serial.print(_timezone); Serial.print(", _daylight="); Serial.println(_daylight);

  Serial.println("\nDoing configTime()");
  configTime(0,0,"pool.ntp.org",NULL,NULL); // this appears to always set "_daylight=1"
  Serial.print("tzname="); Serial.print(*tzname); Serial.print(", timezone="); Serial.print(_timezone); Serial.print(", _daylight="); Serial.println(_daylight);

  // Serial.println("Manually resetting '_daylight'"); _daylight=0; // if the correct "TZ" string is set, it is not necessary to manually adjust the " _daylight" variable. 

  Serial.println("\nDoing setenv()/tzset()");
  setenv("TZ", "GMT0",1) ;tzset();  // dont use the invalid "setenv("TZ", "UTC", 1); tzset();"
  // if you comment out the above line, the ESP8266 will revert to US/Canada daylight saving dates.
  Serial.print("tzname="); Serial.print(*tzname); Serial.print(", timezone="); Serial.print(_timezone); Serial.print(", _daylight="); Serial.println(_daylight);

  localtime_r(&now, &tm_struct);
  Serial.print("\nTime Stamp #2="); Serial.print(asctime(&tm_struct));
  tm* tm_struct2 =localtime(&now);
  Serial.print("Time Stamp #3="); Serial.print(asctime(tm_struct2)); 
}

void loop() {}

bperrybap commented 4 years ago

@Rob58329 that sounds like a new/different bug from this one. I would create a new issue for it.

osresearch commented 4 years ago

Using the RTC-Clock-MinExample.ino with arduino 1.8.13 and esp8266 2.7.4, my board prints the same value for localtime() as gmtime().

Based on the suggestion in https://github.com/esp8266/Arduino/issues/4637#issuecomment-449664822, I changed the TZ string to add the explicit offset on the DST version (from "CST+6CDT,M3.2.0/2,M11.1.0/2" to "CST+6CDT+5,M3.2.0/2,M11.1.0/2"), which fixes it sometimes, but not on every boot and sometimes it would switch back to only GMT values.

Changing it to use the new <TZ.h> and calling configTime(TZ_Europe_Amsterdam, "pool.ntp.org") seems to work correctly. So if you've ended up here because you're looking for setenv("TZ",...) values, try this instead.

bperrybap commented 4 years ago

@osresearch Yep. Something definitely broke. I've not used that exact code sequence in the RTC-Clock-MinExample.ino example for a while. My other code was still working. I tracked it down. It appears that you must call configTime() before you call setenv() and tzset() and you must pass a NULL to settimeofday(tv, tz) for the tz pointer

Attached is an updated example that also has better comments and demonstrates how to use the local time functions with a TZ environment variable with: RTC only, NTP only, RTC and NTP. NTP-RTC-MinExample.ino.zip

bobcroft commented 3 years ago

Hi Bill, thanks for posting the latest mini example code (3 October) It appears that code is for the ESP8266, can it be used on an ES32? Or, if not in its current form, what changes would be needed please? I have been trying to use the EZtime library but I cannot get it to work reliably on the ESP processors. The ESP example of simple time works well but doesn't include DST. I want to be able to use the struct fields for hours, minutes and seconds for various timing functions. I have found the EZtime library good for printing fancy strings but not much else since the minute / second changes detection cannot be relied upon, it also appears the library has been abandoned since there are no recent updates. If you are aware of any code to use NPT and DST on an ESP32 I would greatly appreciate a pointer to it.

d-a-v commented 3 years ago

From what I can read in @bperrybap 's example, only the settimeofday_cb (time_is_set); call is specific to this core. The rest is newlib and should work on esp32.

edit configTime should be the same for the two cores.

vortigont commented 3 years ago

@d-a-v actually configTime() for esp32 is a little different and confusing, configTzTime() is the right equivalent to esp8266 configTime().

d-a-v commented 3 years ago

That's right. configTzTime() was recently added in esp8266 arduino core to maintain an API compatibility. Should we or should we not update the examples ?

esp8266 / Arduino

Serious issues with timezone support in time functions #4637

Platform

Settings in IDE

Problem Description

define NTP0_OR_LOCAL1 1 // 0:use NTP 1:fake external RTC

define RTC_TEST 1510592825 // 1510592825 = Monday 13 November 2017 17:07:05 UTC

6373 brings in all timezone definitions.

6273 brings only additions and do not change the core API (harmless for external libraries like @bxparks's).