search

arthurdent75 / SimpleScheduler

An Home Assistant AddOn to schedule entities on/off on weekly base
183 stars 36 forks source link

Potential for missed schedule #159

Closed hudcap closed 3 months ago

hudcap commented 3 months ago

Hi! First, thank you for this awesome integration!

I was reviewing your code, and if I understand correctly, the code checks every second to see if the current time has zero seconds, and if it does, it checks if any events are scheduled. Supposed the scheduler starts running with 59.999 seconds. Suppose the sleep plus the call for current time lasts just a drop more than a second (maybe because of a heavy cpu load). Then in the next loop, the current time may have 1.001 seconds, and any scheduled events for that minute will never run.

Am I off my mark here? If you think this is a concern, I'm happy to discuss alternatives and make a PR.

arthurdent75 commented 3 months ago

Thank you for your analysis. What you say is potentially correct. This was a concern of mine, indeed. In the first alphas (never released) I checked the first 5 seconds of the minutes. I realize with some specific tests that the while runs hundreds of times on a raspberry. So I decided to keep only the zero check. For 4 years, this never happened to me nor anyone did report this. I believe we could agree that there is no such concern.

hudcap commented 3 months ago

Thank you for such a prompt reply! In the interest of reducing the number of loops, why not sleep until the next zero second, and then skip the zero check?

arthurdent75 commented 3 months ago

I cannot foresee the length of the cycle. As an alternative, I should take the second at the end of the cycle and sleep for a complementary number of seconds. I hope you understand me when I say that I prefer not to touch something that works flawlessly😉 Even a little modification to the source code, requires at least a month of testing before releasing, so I prefer to limit this effort only for new features. By the way, I will consider your suggestion for the next release. Thank you for all your interest!

hudcap commented 3 months ago

take the second at the end of the cycle and sleep for a complementary number of seconds

I think we're saying the same thing. The sleep at the end of the loop would be for (60 minus the current number of seconds). But I completely understand your position. Thank you!