search

biddster / node-red-contrib-schedex

Scheduler for node-red which allows you to enter on/off times as 24hr clock (e.g. 01:10) or suncalc events (e.g. goldenHour). It also allows you to offset times and randomise the time within the offset.
22 stars 17 forks source link

Daylight saving not working #56

Closed ualex73 closed 4 years ago

ualex73 commented 4 years ago

This weekend (Sunday) we had the daylight saving in Europe and schedex still was using the original time (18:xx) instead of the new time (17:xx) for sunset. After a restart of nodered it was working again, but this seems to be a bug in schedex?

vondruska commented 4 years ago

I just ran into this in the US today. Skimming the source code, it seems like schedex "caches" the on/off times for the next execution. In that case, it would take an on and off execution one time before it schedules the next event with the correct time zone. This caching architecture seems pretty integral. Maybe using UTC internally would solve this problem? If I get a free minute, I'll play with it.

drmibell commented 4 years ago

This is the same difficulty I described in connection with #11. As @biddster says in the case of a power cut, schedex seems to schedule the next event before updating its clock.

biddster commented 4 years ago

The way Schedex works is very simple. On startup, it calculates which of the on and off is the next event and sets a timeout. When that timeout fires, we calculate the next event and set another timeout. And so on. It's not really caching anything.

Schedex doesn't try to work out If the clock changes between timeouts due to daylight saving etc. For all of the use cases I have, it's not really an issue. But you folks are almost certainly using it differently.

Happy to accept a PR if someone wants to fix this.

ualex73 commented 4 years ago

@biddster Thanks for the explanation. I don't know Java to 'fix' this, so I will implement a crontab script which restarts NodeRED after Daylight savings change.

biddster commented 4 years ago

Thanks for raising this bug and prompting me to look into this. I've found the problem and it's a trivial fix. Surprised I didn't notice it sooner.

Just working up unit tests to prove it's fixed.

drmibell commented 4 years ago

Thanks for looking into this. There must be lots of folks who schedule sun events and don't notice a glitch a couple of times a year. I suspected something to do with how suncalc is called but never could track it down.

biddster commented 4 years ago

Fixed in 1.6.3

biddster commented 4 years ago

Reopening as my fix broke offsets.

biddster commented 4 years ago

Another attempt at fixing in 1.6.6