diff mbox

Fix shutdown

Message ID 44593276.4080107@gmx.de
State New
Headers

Commit Message

Udo Richter May 3, 2006, 10:45 p.m. UTC 
  Udo Richter wrote:
> Dominique Simon wrote:
>> I noticed that VDR does not warn anymore if you press the power button 
>> (on the remote) and there is a timer within the next X minutes. 
> 
> One way to fix this would be to always ask, and skip the && 
> !ForceShutdown before asking "Recording in %ld minutes, shut down 
> anyway?". This would result in up to three confirms on shutdown: Running 
> recording, plugin activity and soon recording. For better consistency, 
> its probably a good idea to move this question into the kPower handler, 
> where the other two confirms reside.

The attached patch moves the "Recording in %ld minutes, shut down 
anyway?" confirmation into the kPower handler, where all the other 
shutdown confirms reside. I *hope* that this time it doesn't cause yet 
another regression. ;)

Cheers,

Udo

Comments

Dominique Simon May 4, 2006, 8:14 p.m. UTC | #1 
Am 04.05.2006 um 00:45 schrieb Udo Richter:

> The attached patch moves the "Recording in %ld minutes, shut down  
> anyway?" confirmation into the kPower handler, where all the other  
> shutdown confirms reside. I *hope* that this time it doesn't cause  
> yet another regression. ;)

Thanks! I'll attach the patch to my VDR 1.4 and will report back!

Ciao, Dominique
Dominique Simon May 5, 2006, 9:29 a.m. UTC | #2 
Am 04.05.2006 um 00:45 schrieb Udo Richter:

> The attached patch moves the "Recording in %ld minutes, shut down  
> anyway?" confirmation into the kPower handler, where all the other  
> shutdown confirms reside. I *hope* that this time it doesn't cause  
> yet another regression. ;)

It works as expected by now. Shutdown is interrupted if a timer is  
near or a timer is running, good work!

Ciao, Dominique
Helmut Auer May 6, 2006, 2:28 p.m. UTC | #3 
Udo Richter schrieb:
> Udo Richter wrote:
>> Dominique Simon wrote:
>>> I noticed that VDR does not warn anymore if you press the power button 
>>> (on the remote) and there is a timer within the next X minutes. 
>> One way to fix this would be to always ask, and skip the && 
>> !ForceShutdown before asking "Recording in %ld minutes, shut down 
>> anyway?". This would result in up to three confirms on shutdown: Running 
>> recording, plugin activity and soon recording. For better consistency, 
>> its probably a good idea to move this question into the kPower handler, 
>> where the other two confirms reside.
> 
> The attached patch moves the "Recording in %ld minutes, shut down 
> anyway?" confirmation into the kPower handler, where all the other 
> shutdown confirms reside. I *hope* that this time it doesn't cause yet 
> another regression. ;)
> 
Hello,

Without testing it, I would guess, that this leads to another problem:
If the shutdown is caused by user inactiviy vdr will shutdown even if a recording will be within the next xxx minutes.

I have posted another fix here:
http://www.htpc-forum.de/forum/index.php?act=ST&f=12&t=2387&st=0&#entry15177
Udo Richter May 6, 2006, 7:21 p.m. UTC | #4 
Helmut Auer wrote:
> Hello,
> 
> Without testing it, I would guess, that this leads to another problem:
> If the shutdown is caused by user inactiviy vdr will shutdown even if a 
> recording will be within the next xxx minutes.

No, the moved part just covers the "Recording in %ld minutes, shut down 
anyway?" UI question, and this question only appears on manual shutdown.

The following block in the code covers automatic shutdown:

   if (!Next || Delta > Setup.MinEventTimeout * 60 || ForceShutdown)

> I have posted another fix here:
> http://www.htpc-forum.de/forum/index.php?act=ST&f=12&t=2387&st=0&#entry15177 

Since you query cPluginManager::Active() twice (no comment on style...) 
to differentiate between no activity and confirmed shutdown, you're 
setting UserShutdown=true and ForceShutdown=false the old way if no 
questions were asked. So why do you need to modify the "Recording in..." 
block at all?

Cheers,

Udo
Helmut Auer May 6, 2006, 8:06 p.m. UTC | #5 
Hi
>>
>> Without testing it, I would guess, that this leads to another problem:
>> If the shutdown is caused by user inactiviy vdr will shutdown even if a 
>> recording will be within the next xxx minutes.
> 
> No, the moved part just covers the "Recording in %ld minutes, shut down 
> anyway?" UI question, and this question only appears on manual shutdown.
> 
You're right :-)

> The following block in the code covers automatic shutdown:
> 
>    if (!Next || Delta > Setup.MinEventTimeout * 60 || ForceShutdown)
> 
>> I have posted another fix here:
>> http://www.htpc-forum.de/forum/index.php?act=ST&f=12&t=2387&st=0&#entry15177 
> 
> Since you query cPluginManager::Active() twice (no comment on style...) 
 >
If you don't want to comment then be quiet - what you've written is a comment !
But to call it twice is the only way to differentiate between no active plugin active and a user interaction.

 > So why do you need to modify the "Recording in..."
> block at all?
There's no need, but the code is clearer in my eyes.
Anyway, if you talk about style you should recode the whole shutdown part, because it wouldn't pass any code review ;)

Cio
Helmut
Udo Richter May 6, 2006, 9:03 p.m. UTC | #6 
Helmut Auer wrote:
>> Since you query cPluginManager::Active() twice (no comment on style...) 
> If you don't want to comment then be quiet - what you've written is a 
> comment !

Yeah, sorry for for doing a NULL comment. ;)

> But to call it twice is the only way to differentiate between no active 
> plugin active and a user interaction.

Calling it twice is creative, but not very elegant. Active() could be 
modified, but I didn't want to go that far. In the end, I liked the idea 
being asked separately for each reason not to shut down, so there's no 
need to know how forced the shutdown was.

> Anyway, if you talk about style you should recode the whole shutdown 
> part, because it wouldn't pass any code review ;)

The shutdown part, and lots of things in the main loop, but 
unfortunately 1.4.0 is supposed to be a stable version. ;)

Cheers,

Udo
Klaus Schmidinger May 6, 2006, 9:31 p.m. UTC | #7 
Udo Richter wrote:
> Helmut Auer wrote:
>>> Since you query cPluginManager::Active() twice (no comment on style...) 
>> If you don't want to comment then be quiet - what you've written is a 
>> comment !
> 
> Yeah, sorry for for doing a NULL comment. ;)
> 
>> But to call it twice is the only way to differentiate between no 
>> active plugin active and a user interaction.
> 
> Calling it twice is creative, but not very elegant. Active() could be 
> modified, but I didn't want to go that far. In the end, I liked the idea 
> being asked separately for each reason not to shut down, so there's no 
> need to know how forced the shutdown was.
> 
>> Anyway, if you talk about style you should recode the whole shutdown 
>> part, because it wouldn't pass any code review ;)
> 
> The shutdown part, and lots of things in the main loop, but 
> unfortunately 1.4.0 is supposed to be a stable version. ;)

Please let's focus on the actual problem ;-)

Klau
Helmut Auer May 6, 2006, 10:09 p.m. UTC | #8 
Hi,
> 
>> But to call it twice is the only way to differentiate between no active 
>> plugin active and a user interaction.
> 
> Calling it twice is creative, but not very elegant. Active() could be 
> modified, but I didn't want to go that far. In the end, I liked the idea 
> being asked separately for each reason not to shut down, so there's no 
> need to know how forced the shutdown was.
> 
That's also correct, but if the variable ForceShutdown exists,
then it should be set whenever the user commits a question, and not only on some questions :)

Anyway I think both patches will fix the actual problem so it doesn't matter which one to take ..
diff mbox

Patch

--- vdr-1.4.0-orig/vdr.c	2006-05-04 00:07:45.000000000 +0200
+++ vdr-1.4.0/vdr.c	2006-05-04 00:22:43.052229240 +0200
@@ -970,7 +970,7 @@ 
                   }
                break;
           // Power off:
-          case kPower:
+          case kPower: {
                isyslog("Power button pressed");
                DELETE_MENU;
                if (!Shutdown) {
@@ -985,8 +985,21 @@ 
                   }
                if (cPluginManager::Active(tr("shut down anyway?")))
                   break;
+                  
+               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) {
+                  char *buf;
+                  asprintf(&buf, tr("Recording in %ld minutes, shut down anyway?"), Delta / 60);
+                  bool confirm=Interface->Confirm(buf);
+                  free(buf);
+                  if (!confirm) break;
+                  }
+               
                ForceShutdown = true;
                break;
+               }
           default: break;
           }
         Interact = Menu ? Menu : cControl::Control(); // might have been closed in the mean time
@@ -1121,15 +1134,6 @@ 
                     else
                        LastActivity = 1;
                     }
-                 if (UserShutdown && Next && Delta <= Setup.MinEventTimeout * 60 && !ForceShutdown) {
-                    char *buf;
-                    asprintf(&buf, tr("Recording in %ld minutes, shut down anyway?"), Delta / 60);
-                    if (Interface->Confirm(buf))
-                       ForceShutdown = true;
-                    else
-                       UserShutdown = false;
-                    free(buf);
-                    }
                  if (!Next || Delta > Setup.MinEventTimeout * 60 || ForceShutdown) {
                     ForceShutdown = false;
                     if (timer)