git-developer / autosuspend

AutoSuspend-Script for Tvheadend on Linux
GNU General Public License v3.0
7 stars 9 forks source link

Setting wake timer may fail if TVHEADEND_BOOT_DELAY_SECONDS is larger than VHEADEND_IDLE_MINUTES_BEFORE_RECORDING #19

Closed orgi closed 4 years ago

orgi commented 4 years ago

First of all: Thanks for this awesome set of scripts :) I found a small issue which I wanted to share. Sometimes I faced 'time missed' events in tvheadend. Turns out that the root cause is that it sometimes (when recording is about to start in between 15 to 30 minutes from now) the rtcwake command did not work.

This is because the calculation of the wake time assumes that it would never calculate a time in the past. However as I set TVHEADEND_BOOT_DELAY_SECONDS to 1800 it would calculate a time stamp in the past in case the next recording is due in the next 15 to 30 minutes.

If I come to it I may give it a shot and do a PR.

git-developer commented 4 years ago

Thanks for your report.

Could you please check which version of autosuspend you're using? I suspect it's 2017-10-12, or at least before commit 4449202 which handles this problem. Please update to the current version 2020-05-18 and report if this solves your problem.

orgi commented 4 years ago

Will try to check. Unfortunately this will take some time to verify.

git-developer commented 4 years ago

In general, it's a good idea to set TVHEADEND_IDLE_MINUTES_BEFORE_RECORDING to a greater value than TVHEADEND_BOOT_DELAY_SECONDS. The value of boot delay is a hard requirement defined by the machine while idle minutes may be configured at will of the user. When idle minutes is set to a lower value than boot delay, the user allows the machine to shutdown even if it cannot wake up in time for the next recording or activity.

orgi commented 4 years ago

That sounds totally right to me. However I was thinking that there could be some additional checks or magic to prevent the user from doing this - or at least log a respective message.

While I currently still try to reproduce, I have to say that - checking the code - I would not think that this is resolved - as there's no check that after substracting the boot delay that the wakeup time would become negative:

https://github.com/git-developer/autosuspend/blob/15694333a3ecbdd51f38082826c2d3caf347c2cb/usr/local/sbin/autosuspend.tvheadend-functions#L176

Maybe the idle time could by default be calculated using boot delay + extra idle time? Then no additional check should be required and the user will not need to ensure consistency of those two values.

What do you think?

orgi commented 4 years ago

Now thinking again it should be resolved as the fix you mentioned in the other issue I raised will most probably already fix this one, right? That leaves me with just suggesting that the messages in the log could probably be improved little for those cases. So nothing urgent :)

Thanks again for the quick support. Really appreciate what you did here. Think I'll now be able to completely switch off my dreambox for good :)

git-developer commented 4 years ago

Now thinking again it should be resolved as the fix you mentioned in the other issue I raised will most probably already fix this one, right?

Yes, the check is here: https://github.com/git-developer/autosuspend/blob/15694333a3ecbdd51f38082826c2d3caf347c2cb/usr/local/sbin/autosuspend.main#L135-L138

That leaves me with just suggesting that the messages in the log could probably be improved little for those cases.

I just added a clarification in the configuration file. If you feel that the log message could be improved, please make a suggestion.

orgi commented 4 years ago

I think that will do - thanks :)

orgi commented 4 years ago

One more question: After switching to the new scripts the system does not seem to be able to fetch the already existing timers for setting the wake timer. New timers however were recognized. Maybe this is related to the fact that all the old timers are automatic timers while the new ones I added for testing this were all one-time timers? I'll do further investigations...

git-developer commented 4 years ago

I don't understand what you mean by automatic and one-time timers. If you feel this could be an autosuspend issue, please open a separate issue.