Timezone date / time functions not working properly with time_t values

[bperrybap]

The TimeLib library allows using hour(now()) to get the current hour so it would seem reasonable that local.hour(local.now()) should work as well as hour(now()) (when the local timezone is set to be the default) , but it doesn't.

Based on the above assumption that something like local.hour(local.now()), should work as it is passing in the time_t value from the local timezone object and simply attempting to get the hour in the local timezone, there appears to be an issue related to using the date / time functions like hour() minute() etc.... when a time_t value is passed in. Example: local is not the default timezone. now() - works as expected and returns the standard unix time_t local.now() - returns a munged time_t value that is offset by the current local timezone offset. local.hour() - works as expected and returns the hour in the local timezone

local.hour(local.now()) - returns the hour in UTC timezone not the hour in the local timezone. local.hour(local.now(), LOCAL_TIME) - same as above These seem incorrect, as there appears to be no way to get from a time_t value returned by the local timezone object to a local time value.

local.hour(UTC.now(), UTC_TIME) - works as expected and returns the hour in the local timezone

If you set the default timezone to local by using local.setDefault() then: now() - returns a munged time_t value that is offset by the current local timezone offset. local.hour(now()) - returns the hour in UTC timezone not the hour in the local timezone hour(now()) - returns the hour in UTC timezone not the hour in the local timezone

While these examples are very simplistic actual code may do something more like:

t = now();
// other code
hourvalue = hour(t);
minvalue = minute(t);
// or
sprintf(buf, "%02d:%02d:%02d", hour(t), minute(t), second(t));

And that doesn't work with eztime when using something other than the UTC timezone.

It seems that there should be a way to get from a time_t value returned by the timezone object to a local time value using the timezone methods.

---

I think much of these kinds of issues relate to the issues brought in in issue #10 where the time_t values returned by the timezone objects are not the actual time_t value but rather a munged value for the local timezone.

In this specific case it appears that tzTime() is always called by the various timeszone methods like hour() and when the local_or_utc value indicates the value is LOCAL_TIME, tzTime() flips it to UTC , the UTC time_t value is used and then the UTC time value is returned instead of the local timezone value. IMO, that is not the expected behavior.

While all this could be fixed to work, even when using munged time_t values, as the code is currently doing, local.hour(t) or local.hour(local.now()) could work when time_t value was talways he proper unix time_t value.

I think things would get much simpler and become consistent with the way time_t epoch values are supposed to work if all the now() functions always returned the unix time_t value which is the UTC time_t. Yes there would be some need for internally doing conversions when doing the local value conversions, but that wouldn't need to be exposed to the users. Everything should still work and be compatible with the TimeLib API.

It would also make now() lightweight and very fast, as it should be. Currently tzTime() is called 3 or more times just to get a time_t when some thing like local.now() is called which definitely adds in quite a bit of overhead that would go away. makeTime() and breakTime() should be the expensive functions that deal with all the timezone issues and should be moved to be in the Timezone class which can handle the local timezone conversions. Then, all the timezone methods like hour() would first call breakTime() which would do the needed timezone offset conversion, then pick out the needed value and return it. users could call also local.breakTime() to get all the local time values at once if they desired. makeTime() and breakTime() could be left in the ezt class and call the default timezone version just like some of the other functions.

[chaffinched]

Thanks for writing all of that. I spent a couple of hours getting utterly baffled by a sketch that both did and didn't give the correct hour value for a DST zone when using myTZ.setDefault() and I think it relates to what you've written.

Unless I've done something silly, the following statement in the manual is incorrect:

ezTime is compatible with the classic Arduino time library, and thus you can call various functions in the root namespace like hour() and minute() — without a timezone in front. They are interpreted as if passed to the default timezone.

[bitboy85]

I also need number values for hour and so on. But instead of Changing the existing functions and breaking compatibility its possible create new ones which outputs results according to the selected timezone: uint8_t hourTZ(TIME) uint8_t hourFormat12TZ(TIME) uint8_t minuteTZ(TIME) uint8_t secondTZ(TIME) uint16_t msTZ(TIME) uint8_t dayTZ(TIME) uint8_t weekdayTZ(TIME) uint8_t monthTZ(TIME) uint16_t yearTZ(TIME);

PS: getting the correct value could be achieved with myTZ.dateTime("G") but i think this will do at least 2 unnecessary conversions first int to string, and if a number is needed back from string to int.

[bperrybap]

@bitboy85 that is a terrible solution as it creates a new API that is not compatible with TimeLib. Fixing the ezTime library code to work correctly (to work the same as the TimeLib API) doesn't break compatibility with TimeLib API, it simply fixes the ezTime code to work the same as the TimeLib API. Creating new wrapper functions totally breaks compatibility with TimeLib as it means that users can't use the existing TimeLib API functions they were previously using, which I believe was the goal of ezTime. And even if you were to create those new API functions, you still can't completely work around the problem as the issue is in the underlying functions that these functions must call. There are also a few issues related to certain local time calculations which are based on calculations using the utc time_t vs the munged up local time_t so they are wrong when the local time is within the time zone offset of the transition.

Regardless of how the ezTime code is fixed, it needs to be fixed because "as is", LOTS and LOTS of existing TimeLib code is broken when trying convert over to using this library since most of basic and commonly used API functions are broken. Simply put: The functions: now(), hour(), hourFormat12(), minute(), second(), day(), month(), isPM(), isAM(), isDST() don't work correctly for anything but the utc timezone object. This is a HUGE problem for existing code that would like a simple solution to add timezone support which is the entire point of the ezTime library. IMO, the ezTime library is not usable until this is fixed since the core API functions to get the local time members do not work correctly when using something other than utc which is entire point of using a timezone library.

---

The real issue is down in the low level code. It makes some incorrect assumptions and does things backwards with respect to tracking time_t values. Things like returning the time_t values should be very fast. In this library they are slow as it does lots of expensive calculations - including DST and DOW calculations on every API call - even if just getting a time_t value. It will do these calculations more than once, including for a single call to get the time_t value. Also, some of the low level ezTime support routines make the assumption that a utc/local or local/utc flip conversion always needs to be done when they called and in some cases this is simply wrong. i.e. if you pass in local you want utc and if you pass in utc you want local. That is what is creating the problem describe in the issue above where the local time hour is wrong.

Here are two ways to fix the code work correctly.

• fix the code to always use proper time_t values as mentioned above, which would really simplify the code,
• fix the low level code to look at and always use the local_or_utc parameter.

method 1 Fixing the code to always use proper time_t values which would simplify the code and make it faster & leaner. This might break some existing TimeLib users code. But the vast majority or users and existing code would never know the difference. However the amount of existing sketch code that would break from this fix would be much less that amount of existing code that is currently broken because of the issues described earlier. For those case where using proper time_t values do happen to break existing sketch code trying to covert from TimeLib to ezTime, the sketch updates to fix it would be minimal since, for the most part, existing code was doing things that could have been done differently using the existing TimeLib API by calling a slightly different API function to do the work for them vs trying to do themselves. i.e. they were doing some things "the hard way" and could remove some of their code to instead call some TimeLib API functions to do that work instead. But again, the vast majority of existing code would never see any difference. i.e. much less existing sketch code would break from switching to actual time_t values than is currently broken by the issues in the current ezTime library.

method 2 In term of fixing the code to work correctly (i.e. fix the API to work the same as the TimeLib API works) without fixing the ezTime code to use proper time_t values, the issue is that some of the low level ezTime routines don't look at or use the local_utc parameter properly and some make the assumption that you always want the opposite of what you pass in. THIS is what creates the problem noted in the issue above as a conversion from one to the other is not always needed and THAT is what breaks returning the proper local time as noted in the issue above. This is because the low level code is making an assumption that the time should be converted from utc/local or local/utc rather than converting the time to what the timezone object has been configured for. This is wrong.

I would love to use the eztime library for my clock projects. But "as is", it is completely unusable since it cannot convert the time_t returned by now() to local elements by using functions like hour(t), min(t) etc... That is very basic functionality that simply does not work. And as pointed out above, it can fixed with or without converting the ezTime code to use proper time_t values.

[teemue]

How is tzTime() supposed to work? For example, how I convert the following to the timezone I have set as default? time_t fire = makeTime(schedule[i].H, schedule[i].M, 0, day(), month(), year());

[bperrybap]

@teemue I cannot provide an answer to how tzTime() is supposed to work because there is an underlying design flaw in the ezTime library and tzTime() is inherently a big part of how it works today and also what creates the issue. The overall main problem with the ezTime library is it is promoting a concept of a "local" time_t value and it sometimes gets confused about whether it is using this "local" time_t value which is a munged epoch based on the local timezone or a true time_t value which always is based on a single epoch. When time_t values are done properly, they are based on a single epoch, so for a given point in time, the time_t value is the same EVERYWHERE on earth as a true time_t is timezone independent. As currently implemented, some of ezTime's issues relate to the library code not passing on certain parameters/information to lower level functions around and tzTime() always making assumptions as to which way to convert a time_t value (local<->UTC).

So the real solution is to fix/correct the ezTime library to use true epoch time_t values based on a SINGLE epoch, which means that a time_t value does not change or ever need to be changed for timezones. There is no such thing as a time_t value for a timezone i.e. you track time based on a SINGLE epoch; it only gets converted to broken down local timezone values when the human wants to see the broken down components.

Fixing the code to use real time_t values can also help remove some complexity in the existing library code.

Sketch/user code that properly uses all the TimeLib functions like hour(), minute(), day(), month() or hour(time_t), minute(time_t), secon(time_t) etc..., and a slew of other macros will never know the difference as things will continue to "just work" just like with TimeLib. This is NOT the currently the case with ezTime as MANY things that work with TimeLIb do not work correctly with ezTime because of its internal confusion over the use of a "local" time_t vs a "UTC" time_t.

The issue is that ezTime made a mistake in its underlying implementation by creating this "local" time_t concept which is the wrong way to handle epoch time. Unix defined how time_t values and the associated APIs work 50 years ago. Not only is what ezTime doing incorrect with respect to time_t values, but it is very costly in terms of execution time as MANY conversions are having to be done back and forth between "local" time_t values and the actual (UTC) time_t value for simple things that would not require these conversions if real epoch time_t values were being used. But one of the biggest things that affects users of the ezTime library is the way ezTime is handling its conversions, there are many cases (as I showed above) where the helper functions return the wrong local time broken down values.

Incorrect conversions from functions like hour(time_t), minute(time_t), second(time_t) which are some of the most basic of all time tracking functions, make the library unusable.

[bperrybap]

@teemue In terms of your specific example:

time_t fire = makeTime(schedule[i].H, schedule[i].M, 0, day(), month(), year());

I've written about this a few times, here and in other issues. But here is a summary: Once ezTime is fixed use a proper time_t the the time_t value will never be a bastardized "local" time_t anymore. This means that makeTime() has to know if the values it is being passed are UTC value or local value. The easiest way to handle this would be to add the makeTime() method to the Timezone class so the user can explicitly specify which timezone the local values represent and the library code can deal with any necessary conversions accordingly. As a convenience, the library code could (should IMO) also make the global makeTime() function redirect to the appropriate Timezone class object makeTime() using defaultTZ just like so many of the other functions. So makeTime() would be redirected once setDefault() was called in the Timezone object. This offers the best of all worlds. The user can set up a Timezone object and set it as the default so all the calls work just like they do in TimeLib. and things like makeTime() "just work" since they know the local timezone and can do the any needed conversion to create the proper time_t value. And if a person wants to explicitly specify a conversion they can use the TimeZone object directly or specify UTC values using the built in UTC object. So for example once the user set up myTZ as the timezone, he calls myTZ.setDefault() Then all calls to makeTime() would assume values specified were local values for myTZ timezone. If he wants UTC he could use UTC.makeTime() likewise he could explicitly specify the timezone and use myTZ.makeTime() OR, he could even access or use other timezones. example:

time_t t = now();
int local_hour = hour(t);
int ny_hour = nyTZ.hour(t); // get hour in new york
int sf_hour = sfTZ.hour(t); // get hour in san francisco

You cannot do this with ezTime today as the code incorrectly handles the conversions.

Things can actually get much simpler and clearer when real time_t values are used for both the library code and the sketch code. This is especially true for applications that use a single timezone. From a sketch perspective, one that used to use TimeLib, all it has to do is a few upfront things, then all the exiting code should "just work". Create the TimeZone object, initialize it, then call setDefault() to use it for all the TimeLib API functions. The ezTime code would then know it needs to convert from the time_t to get local broken down values, and that any local broken value specified is a local time value based on the default timezone. All while time_t is never modified even if timezones are changed.

With the current ezTime implementation, this is definitely not the case as there are many cases where ezTime returns the incorrect local time values.

[teemue]

Thank you for your very thorough answer.

[inetBrad]

I just came across this issue. I have a need to calculate a future time and I've been cross checking my math by printing out the results with hour(). My first clue that something was amiss was when I noticed that my results were consistently off by my GMT offset.

[MassiPi]

Hello, probably i'm having the very same problem, but just to be sure (and because i'm not confident i'm doing all right :)) 1) i'm getting a timestamp for my sensors readings with makeTime(hour(), minute(), second(), day(), month(), year()) as this is not impacted by TZ anyhow. 2) when sent to the client i can easily convert the timestamp to the correct timezone. 3) trying to convert locally the timestamp to formatted time with timezone fails. I tried with myTz.dateTime(myTimestamp) but the result is always wrong. I assume this is the problem described in this topic? I fixed with myTz.dateTime(myTimestamp - myTz.getOffset()*60) did i miss anything? thanks a lot

[bperrybap]

It sounds like the problem. The basic problem is that there is a design flaw in the library. The library doesn't handle the timestamp and local conversions correctly. So depending on what calls you make and if you pass in a time_t timestamp to a function, you may or may not get the proper local time values. And ALL the calls using a time_t parameter like local.hour(t) or hour(t), fail to return the proper local time.

IMO, this makes the library completely unusable. Which is sad because I think it has the potential to be a really great and useful library.

I think the library would be great if it were refactored to use time_t values correctly as I noted previously and in issue #10 (timezones do not affect timestamps)

[rvdbreemen]

I just ran into the issue explained by you @bperrybap ... Is there any way to fix this library in your opinion?

[bperrybap]

@rvdbreemen Yes. It can be fixed and while it is big change under the hood, it isn't that difficult. The root cause of this issue is that the library is not properly handling epoch time tracking and different parts of the library are not handling the date/time conversions the same way.

The time_t value is supposed to be the number of seconds since the epoch. There is only 1 epoch and that event occurred simultaneously everywhere on the planet at the same time. A time_t value is not affected or ever modified by locality / timezone / DST rules and at any point in time it is the same value everywhere on the planet. i.e. time_t represents the number seconds since the epoch; it does not reflect a number of seconds since a given local time.

This library modifies the time_t value to reflect a pseudo value that looks like a epoch time_t value but really isn't as the value includes a local offset / local time, which is not how time_t values work. See issue #10 for a bit more on this. The fix is make time_t values work they way they are supposed to. i.e. the track the seconds since the epoch and then any local time offsets are only applied to the broken down time values (not the time_t value), when the broken down date/time values are asked for. This change would simplify some of the library code, reduce/eliminate some of the API functions and make the library code run quite a bit faster as the local time conversions would only be happening when asked for vs happening all the time on each timer tick.

There is a bit of good news in all this int that for the most part the fix for this is all under the hood and if sketches were using the API functions, most would never know the difference. Some of the sketch code for dealing with "local" time_t values (which really don't and shouldn't exist) could go away. But even if the sketch code kept it, it should still work. i.e. the library could transition to properly handling time_t values and most sketch code would never know the difference, yet all the API functions that currently don't work correctly with time_t values would now work correctly.

[rvdbreemen]

@bperrybap okay, i read issue #10 and think you are right that it is fixable. Did you go ahead and try to fix the problem with this library yourself? Or are we going to need to wait until @ropg will take up the challange?

[bperrybap]

I'm more than capable of fixing it and initially spent a few days looking how to fix it. However, I haven't taken it on. While the modifications are not difficult, they are quite significant as it affects the design of the code to the point of basically needing a re-write. If I were doing it, I would gut quite a bit of the code and rewrite a few of the main functions to shrink it down to make the code a lot simpler which it can be if using proper time_t values. I'd also add in all the needed functions, macros, data types, including a sync provider function, to make it fully Time/TimeLib library compatible. While I would try to ensure compatibility with existing ezTime users, I would be less concerned about backward compatibility with some of the far corner cases of the existing ezTime API and much more focused on full Time/TimLib compatibility to get transparent support of proper localtime & DST corrections for those users. This would offer existing Time/TimeLib users a drop in replacement library that can offer proper local time with DST conversions with only a few simple changes/additions to some time objects and no changes to the main code. I'm not sure rpog would agree with my approach or accept a pull request from me.

It is a bit sad in that I think this library has the potential to be the best time library for the Arduino environment. It can solve many issues like handling timezones, DST adjustments, y2k38 problem, and network syncing all in one library that can be compatible with the popular Ardiuno Time/ TimeLib library.

But "as is", IMO, the library should not be used as there are too many hidden bugs & issues.

I had hoped to use this for a few projects but gave up on it and am using other solutions for now.

However.... I am also running into some issues with some of the time function code in the ESP8266 and in particular the ESP32 cores that the developers of ESP32 core seem unwilling to fix. (both have y2k38 issues in their time and NTP implementation)

I want a solution that can support network time syncing and does not have a y2k38 rollover issue. So at some point I may come back to this library, fork it and fix it in my version.

[ropg]

Hey there,

Sorry to be a bit silent lately. I agree with @bperrybap about what needs to be done. A next major version is iin order, breaking corner cases is not a big deal. And I would help out getting it done quickly if it weren't for me having to get some things done related to that which have to take priority.

Related but not urgent: timezoned also needs to be standardised into something that is futureproof and run on a few more machines.

How about we make ezTime a GitHub organisation and I make @bperrybap the other maintainer? I've just done this for M5ez, one of my other projects. We then use gitter.im to talk about things, and it works well.

@bperrybap: shall we video-conference about this soon? Please reach me at rop at rop dot nl

[rvdbreemen]

@ropg @bperrybap any progress on this issue. I think it's worthwhile to consider a community effort to fix the underlying structure of the library.

[bperrybap]

I had some internet issues that took me offline for a bit, and some other personals things that consumed me. I'll reach out to rop soon to see if we can figure out a way to get things moving.

[fritsjan]

Stumbled on this problem also, what is the use of this library when now() and hour(), minute() etc is not updated with the setdefault timezone?

I hope the fix is in the make. Let me know ;-)

[imranmoezkhan]

Ran into this issue as well after spending hours struggling to understand what was going wrong while assuming the code actually did something. Might be a good idea to take the lib off public repo, or put a notice on the main repo page till a fix is in so that others don't spend hours on something that inherently doesn't work?

[rvdbreemen]

@bperrybap did you reach @ropg to discuss on helping him to fix the fundamental issues with the library?

[mugginsjm]

A bit out of my depth here but... I have been happily using hourFormat12() until this morning on the DST change and nothing changed. I had to resort to this int myHour=myTZ.dateTime("g").toInt();

Is there a better way? thanks

[cyberman54]

Wow, this seems complicated, while i assumed with this library it would be Eazy...

I need to get the current hour for time_t t in my timetone myTZ, where t is given in UTC, gathered from a GPS and corrected by leap seconds:

hour = myTZ.hour(t);

Unexpectingly, this returns 4 hours offset (myTZ = Europe/Berlin).

How can i solve this?

/cc @bperrybap