[bug] Shutdown during an active timer is broken
Commit Message
I am sorry I have to bring this up after a few months, i came over it and had
to realize that the new behavior is not what the user expects.
Now i am a little bit afraid that you are going to change this again.
(see vdr shutdown handling / streamdev plugin)
> How about this: if the user presses the "Power" key to turn off VDR,
> and VDR tells him/her that there is a recording going on, or a timer
> will hit in a few minutes, and the user still insists in shutting
> down VDR *now*, what could be the reason for this? IMHO the only real
> reason is that the user wants to do some maintenance, for which the
> device must be turned off (like, for instance, install some hardware
> or relocate it to a different place).
This assumption is not correct. Not anyone ist going to do some maintenance if
he shuts down his box. They are not all freaks like us ;)
In fact i found out that there are two main reasons:
1. Saving power. Well, *we* know that shutting down for some minutes isn't
really saving anything, but a user thinks "ok, its still 15 minutes to the
next recording so its save to shut it down. And the vdr isn't telling him a
warning that wakeuptime is shifted if he does so. So it's not intuitive.
A timer is a "holy kow" - it should not be modified automatically.
2. Users always think "my box will (has to) return to the same state as it was
before the recording". So they shut it down.
They do not know that there is this (hidden) feature in the vdr that lets it
shut down if he ignores the message and then keeps away his fingers from the
remote (as it is no informational message but a question).
On the other hand, the user expects that a running recording is disabled and
he says "yes" to the question to shut down anyway.
So here is my solution:
- if a recording is running and User presses "Power" tell him the box will
shutdown after the current recording
- If he presses "Power" a second time, ask if he realy wants to do this (like
before) *and* stop any running recordings (i reused the code from Udo which
work very well, thanks)
- if a timer is pending within MinEventTimeout ask if he really wants to do so
but do *not* modify any timers or wakeup times.
I think it's the task of whoever adopts vdr to a mainboard or box to program a
valid wakeup time, not the vdr itself.
Cheers,
Tim
patch:
Comments
Thiemo wrote:
> So here is my solution:
> - if a recording is running and User presses "Power" tell him the box will
> shutdown after the current recording
> - If he presses "Power" a second time, ask if he realy wants to do this (like
> before) *and* stop any running recordings (i reused the code from Udo which
> work very well, thanks)
I agree that the shutdown-not-confirmed state should be more visible to
users by some message that VDR is just waiting for background tasks to
complete before shutdown. A solution I thought of was to put up a
message after not confirming shutdown that VDR will shut down as soon as
whatever is done - maybe even keeping that message on screen all the time.
The idea of pressing power button twice is also nice, though it will
confuse scripts that send power key presses. Plus, effectively, while
you currently confirm shutdown with "power, ok", you now confirm with
"power, power". And together with the other numerous reasons for not
shutting down, this gets confusing: Use power button to override running
timers, and use ok button to override timers in a few minutes?
> - if a timer is pending within MinEventTimeout ask if he really wants to do so
> but do *not* modify any timers or wakeup times.
So you *do* want running timers to be disabled, but *not* want to ignore
timers in a few minutes?
> I think it's the task of whoever adopts vdr to a mainboard or box to program a
> valid wakeup time, not the vdr itself.
Then we can also go back to what it was before, leave all timers alone
and report wakeup in -30 minutes to the shutdown script. In the end it
doesn't matter if any timers are running when VDR is killed.
Cheers,
Udo
On Sat, Dec 09, 2006 at 05:48:47PM +0100, Udo Richter wrote:
> The idea of pressing power button twice is also nice, though it will
> confuse scripts that send power key presses. Plus, effectively, while
> you currently confirm shutdown with "power, ok", you now confirm with
> "power, power".
Furthermore, some cRemote implementations have incorrectly omitted the
kRepeat flag from repeat events in the past. As far as I remember, the
latest report of this on this list was not resolved. It was with some
RF remote control unit, as far as I remember. To be exact, it was in
the kernel input event driver in that case, but there have been bugs in
user-space drivers as well.
Users of such RCUs could accidentally send "power, power" with a single
press of the button.
> And together with the other numerous reasons for not shutting down,
> this gets confusing: Use power button to override running
> timers, and use ok button to override timers in a few minutes?
I hope you will find some logical solution. And I'm sure someone will
complain, no matter what you come up with. :-)
Marko
Am Samstag, 9. Dezember 2006 17:48 schrieb Udo Richter:
> Thiemo wrote:
> > So here is my solution:
> > - if a recording is running and User presses "Power" tell him the box
> > will shutdown after the current recording
> > - If he presses "Power" a second time, ask if he realy wants to do this
> > (like before) *and* stop any running recordings (i reused the code from
> > Udo which work very well, thanks)
>
> I agree that the shutdown-not-confirmed state should be more visible to
> users by some message that VDR is just waiting for background tasks to
> complete before shutdown. A solution I thought of was to put up a
> message after not confirming shutdown that VDR will shut down as soon as
> whatever is done - maybe even keeping that message on screen all the time.
>
> The idea of pressing power button twice is also nice, though it will
> confuse scripts that send power key presses.
No it won't. VDR still remembers the "shutdown-after-recording"-state with my
changes. The only difference here is that the user is told whats going on.
A script never knows if it has to confirm the kPower or not (except if you
would parse the timers to see if one is running). So if a script wants to
shutdown vdr regardless of its state (i.e. for some maintenance ;) ) then
sending kPower via svdrp isn't the right action at all. (one should use
"killall -1 vdr" or similar and prevent vdr from starting up again).
> Plus, effectively, while
> you currently confirm shutdown with "power, ok", you now confirm with
> "power, power". And together with the other numerous reasons for not
> shutting down, this gets confusing: Use power button to override running
> timers, and use ok button to override timers in a few minutes?
No, you still confirm with Ok, theres just one additional step before.
I suggest you try it out - thats easier than describing it here.
> > - if a timer is pending within MinEventTimeout ask if he really wants to
> > do so but do *not* modify any timers or wakeup times.
>
> So you *do* want running timers to be disabled, but *not* want to ignore
> timers in a few minutes?
exactly.
If you would alter a timer (or the wakeuptime whats the same in the end) you
would have to give a clear warning "Your timers will be shifted by xx
minutes. Are you sure".
But as i wrote in the previous post, it's not a good idea to alter a timer at
all.
> > I think it's the task of whoever adopts vdr to a mainboard or box to
> > program a valid wakeup time, not the vdr itself.
>
> Then we can also go back to what it was before, leave all timers alone
> and report wakeup in -30 minutes to the shutdown script. In the end it
> doesn't matter if any timers are running when VDR is killed.
No, it's an improvement to what is was before. Users treat negative times as
bugs. (And it *is* a bug to ask "recording in -x min" instead of "a recording
is active")
Tim
Thiemo wrote:
>> Plus, effectively, while
>> you currently confirm shutdown with "power, ok", you now confirm with
>> "power, power". And together with the other numerous reasons for not
>> shutting down, this gets confusing: Use power button to override running
>> timers, and use ok button to override timers in a few minutes?
> No, you still confirm with Ok, theres just one additional step before.
> I suggest you try it out - thats easier than describing it here.
Ok, I try to sum it up:
If there's a good reason not to shut down, then hitting power once just
switches to non-interactive mode. Hitting power twice in a short time
starts the usual confirm marathon, and you'll need at least "power,
power, ok" before anything happens.
Now what if nothing blocks shutdown? Going to non-interactive mode
doesn't make sense, since VDR will start the 5-minute-shutdown then. So
in that case VDR could power down on single power button. But is this
intuitive? Hmmm.
>> So you *do* want running timers to be disabled, but *not* want to ignore
>> timers in a few minutes?
> exactly.
> If you would alter a timer (or the wakeuptime whats the same in the end) you
> would have to give a clear warning "Your timers will be shifted by xx
> minutes. Are you sure".
> But as i wrote in the previous post, it's not a good idea to alter a timer at
> all.
In my opinion, disabling a timer *is* altering. I tend to agree that
timers shouldn't be altered, especially since the external shutdown
script may just ignore the shutdown, in which case the timers should
keep running. Also, I wouldn't bring down VDR with Interrupted=1, thats
the job of the external script too.
But at the end, the shutdown script needs some advice when to start up
again. It doesn't make sense to pass negative values, and it doesn't
make sense to pass values in a few minutes, as shutdown and reboot takes
more time. And SVDRP'ing for the next timer that is realistically
rebootable in time is a too difficult task for an external script. So
what now?
Cheers,
Udo
@@ -1012,18 +1012,46 @@
break;
}
LastActivity = 1; // not 0, see below!
- UserShutdown = true;
if (cRecordControls::Active()) {
+ if (!UserShutdown) {
+ Skins.Message(mtInfo, tr("Activated standby after current recording"));
+ UserShutdown = true;
+ break;
+ } else
if (!Interface->Confirm(tr("Recording - shut down anyway?")))
break;
- }
+ }
+ UserShutdown = true;
if (cPluginManager::Active(tr("shut down anyway?")))
break;
- if (!cRecordControls::Active()) {
+ if (cRecordControls::Active()) {
+ // Stop all running timers
+ time_t Now = time(NULL);
+ cTimer *timer = Timers.GetNextActiveTimer();
+ time_t Next = timer ? timer->StartTime() : 0;
+ while (timer && Next - Now < 0) {
+ if (timer->IsSingleEvent())
+ timer->ClrFlags(tfActive);
+ else
+ timer->Skip();
+ timer->Matches();
+
+ cTimer *nextTimer = Timers.GetNextActiveTimer();
+ time_t nextNext = nextTimer ? nextTimer->StartTime() : 0;
+ if (nextNext < Next || (nextNext == Next && nextTimer == timer)) {
+ esyslog("Loop detected while disabling running timers");
+ break;
+ }
+ Next=nextNext;
+ timer=nextTimer;
+ }
+ Timers.SetModified();
+ }
+ else {
cTimer *timer = Timers.GetNextActiveTimer();
time_t Next = timer ? timer->StartTime() : 0;
time_t Delta = timer ? Next - time(NULL) : 0;
- if (Next && Delta <= Setup.MinEventTimeout * 60) {
+ if (Next && Delta <= Setup.MinEventTimeout * 60 && !AutoShutdown) {
char *buf;
asprintf(&buf, tr("Recording in %ld minutes, shut down anyway?"), Delta / 60);
bool confirm = Interface->Confirm(buf);
@@ -1172,19 +1200,21 @@
else
LastActivity = 1;
}
+ /* This one manipulates starttime, we don't want that
if (timer && Delta < Setup.MinEventTimeout * 60 && ForceShutdown) {
Delta = Setup.MinEventTimeout * 60;
Next = Now + Delta;
timer = NULL;
dsyslog("reboot at %s", *TimeToString(Next));
}
+ */
if (!Next || Delta > Setup.MinEventTimeout * 60 || ForceShutdown) {
ForceShutdown = false;
if (timer)
dsyslog("next timer event at %s", *TimeToString(Next));
if (WatchdogTimeout > 0)
signal(SIGALRM, SIG_IGN);
- if (Interface->Confirm(tr("Press any key to cancel shutdown"), UserShutdown ? 5 : SHUTDOWNWAIT, true)) {
+ if (Interface->Confirm(tr("Activating standby"), UserShutdown ? 2 : SHUTDOWNWAIT, true)) {
cControl::Shutdown();
int Channel = timer ? timer->Channel()->Number() : 0;
const char *File = timer ? timer->File() : "";
@@ -1195,6 +1225,7 @@
isyslog("executing '%s'", cmd);
SystemExec(cmd);
free(cmd);
+ Interrupted=1; // use this to make vdr shut down without "kill -1"
LastActivity = time(NULL) - Setup.MinUserInactivity * 60 + SHUTDOWNRETRY; // try again later
}
else {
Nur in vdr-1.4.4-new: vdr.c.orig.