search

ropg / ezTime

ezTime — pronounced "Easy Time" — is a very easy to use Arduino time and date library that provides NTP network time lookups, extensive timezone support, formatted time and date strings, user events, millisecond precision and more.
MIT License
327 stars 92 forks source link

Changing to DST at wrong time. #28

Closed mike-s123 closed 5 years ago

mike-s123 commented 5 years ago

Just trying to test out code before a real summertime change.

So, I'm doing testing by setting the TZ string to change to summertime a few minutes in the future, expecting to watch the change. But, when set slightly in the future, it changes immediately. It's not changing at the correct local time.

Right now, it's Monday, 04-Feb-2019, 23:something. (date is M2.1.1, in "Olson speak")

e.g. With the string set to "EST5EDT,M2.1.2/4:13,M11.1.0" (summertime begins at 04:13 tomorrow morning), it actually changes to DST at 23:08 local standard time, which is 5 minutes before when it should change if it were simply using UTC instead of local time! 100% repeatable, I can switch back and forth. If I set the TZ string to 5 hours and 4 minutes ahead of local time, it will change to DST. If I set it to 5 hours and 6 minutes ahead of local time, it will change to standard time.

Weird. I'm not a coder, I only play one on the Internet enough to be dangerous. But, that behavior would make sense if in the relevant routine, instead of subtracting 5 hours (the offset from UTC) from UTC time, you're subtracting 5 minutes (or something like that, my brain is starting to hurt).

ropg commented 5 years ago

I had a similar case of brain hurt trying to hunt down an issue that is probably the same thing (#15), but the swap with minutes and hours is fascinating and may be the best clue yet. I will take a look soon, there's clearly some bigger work to be done...

mike-s123 commented 5 years ago

OK, did a bit of debugging.

In Timezone::tzTime, std_offset and dst_offset are assigned minutes:

int16_t std_offset = (offset_hr < 0) ? offset_hr * 60 - offset_min : offset_hr * 60 + offset_min;

But, later they're treated as seconds. So, fixing that:

    if (local_or_utc == UTC_TIME) {
        dst_start -= std_offset * 60LL;
        dst_end -= dst_offset * 60LL;
    }

Makes things more sane, it's now changing by 5 hours instead of 5 minutes. But, the signs are also wrong... Fixing that makes it:

    if (local_or_utc == UTC_TIME) {
        dst_start += std_offset * 60LL;
        dst_end += dst_offset * 60LL;
    }

All fixed. (I think??) You'll have to verify (works for me now), and roll it in.

Edit: references to "olsen" should be changed to "olson", to respect the creator, Arthur David Olson.

ropg commented 5 years ago

Fixed by commit a81250a70c896ed295452012798e1e00d34b847f from #29 (thanks!!)