Fix calculating starttime of repeated timers with start date

Message ID 44721EAE.8040200@gmx.de
State New
Headers

Commit Message

Udo Richter May 22, 2006, 8:27 p.m. UTC
  Udo Richter wrote:
> The original patch did not attempt to fix Skip(), it just fixes the 
> wrong calculation of StartTime(). Skip() could be fixed by this 
> additional patch: (untested!)

Well, that was really untested. This one *is* tested:



Cheers,

Udo
  

Comments

C.Y.M May 22, 2006, 10:43 p.m. UTC | #1
Udo Richter wrote:
> Udo Richter wrote:
>> The original patch did not attempt to fix Skip(), it just fixes the
>> wrong calculation of StartTime(). Skip() could be fixed by this
>> additional patch: (untested!)
> 
> Well, that was really untested. This one *is* tested:
> 

Thanks a lot for looking at this problem, Udo. So, is it my understanding that
Enigma does not need to change if we use the original "starttime" patch you
posted in conjunction with this other new one?

Best Regards.
  
Udo Richter May 22, 2006, 11:09 p.m. UTC | #2
C.Y.M wrote:
> Thanks a lot for looking at this problem, Udo. So, is it my understanding that
> Enigma does not need to change if we use the original "starttime" patch you
> posted in conjunction with this other new one?

The two patches (my original and my last) fix minor issues with VDR. The 
bogus Enigma loop may terminate again with these two patches, but I 
wouldn't bet on it. You can try it if you want.

Rewriting the Enigma loop as I suggested would be a good idea anyway, to 
make it a bit more robust.

Cheers,

Udo
  
Andreas Brugger May 23, 2006, 4:37 p.m. UTC | #3
Udo Richter schrieb:

> C.Y.M wrote:
>
>> Thanks a lot for looking at this problem, Udo. So, is it my 
>> understanding that
>> Enigma does not need to change if we use the original "starttime" 
>> patch you
>> posted in conjunction with this other new one?
>
>
> The two patches (my original and my last) fix minor issues with VDR. 
> The bogus Enigma loop may terminate again with these two patches, but 
> I wouldn't bet on it. You can try it if you want.
>
> Rewriting the Enigma loop as I suggested would be a good idea anyway, 
> to make it a bit more robust.

C.Y.M. maybe you could test this fix and if it works I will include it 
into the next version of Enigma.

Udo, do you know any side-effect of scripts or something like that if 
the directory is renamed? I didn't test this patch due to some concerns 
that there maybe some unknown side-effects ...
  
Udo Richter May 23, 2006, 6:07 p.m. UTC | #4
Andreas Brugger wrote:
> Udo, do you know any side-effect of scripts or something like that if 
> the directory is renamed? 

ummmm... what?
Sorry, don't get it. My patches have nothing to do with file names or 
directory names.
And if you're hunting Enigma bugs, sorry, my VDR doesn't have the OSD 
memory to run Enigma.

Cheers,

Udo
  
Andreas Brugger May 23, 2006, 8:05 p.m. UTC | #5
Udo Richter schrieb:

> Andreas Brugger wrote:
>
>> Udo, do you know any side-effect of scripts or something like that if 
>> the directory is renamed? 
>
>
> ummmm... what?
> Sorry, don't get it. My patches have nothing to do with file names or 
> directory names.

Ah sorry, that was an other patch that changed the directory-name.

> And if you're hunting Enigma bugs, sorry, my VDR doesn't have the OSD 
> memory to run Enigma.

BTW, you don't need a modded card to use Enigma. That is an old myth. ;-)
The OSD-size is limited though. You have to use a max. height of 470 
pixels or so.
  
Klaus Schmidinger May 25, 2006, 12:42 p.m. UTC | #6
Udo Richter wrote:
> Udo Richter wrote:
>> The original patch did not attempt to fix Skip(), it just fixes the 
>> wrong calculation of StartTime(). Skip() could be fixed by this 
>> additional patch: (untested!)
> 
> Well, that was really untested. This one *is* tested:
> 
> --- timers.c.orig    2006-05-20 18:50:49.000000000 +0200
> +++ timers.c    2006-05-22 22:19:02.134688704 +0200
> @@ -560,6 +560,9 @@
>  void cTimer::Skip(void)
>  {
>    day = IncDay(SetTime(StartTime(), 0), 1);
> +  startTime = 0;
> +  day = SetTime(StartTime(), 0);
> +  startTime = 0;
>    SetEvent(NULL);
>  }

I'm afraid this might have a nasty side effect.
Imagine the following scenario:

- a weekly timer that records on Mondays at 20:00
- at Monday, 19:00 the user decides to skip today's recording,
   so he presses the red button on the timer
- some time later he decides to make this timer also record
   on Tuesdays, so he edits the timer and sets the day flag for
   Tuesday by pressing '2'

The way it is now, the timer would record as the user
expects it to. With your patch he would also have to manually
set the "first day" correctly, otherwise the additional Tuesday
recording wouldn't take place.

I think I prefer to leave cTimer::Skip() as it is, and only
adopt your original patch.

Klaus
  
Udo Richter May 25, 2006, 1:22 p.m. UTC | #7
Klaus Schmidinger wrote:
>> Udo Richter wrote:
>>> The original patch did not attempt to fix Skip(), it just fixes the 
>>> wrong calculation of StartTime(). Skip() could be fixed by this 
>>> additional patch: (untested!)
>>
> I'm afraid this might have a nasty side effect.
> Imagine the following scenario:
> 
> - a weekly timer that records on Mondays at 20:00
> - at Monday, 19:00 the user decides to skip today's recording,
>   so he presses the red button on the timer
> - some time later he decides to make this timer also record
>   on Tuesdays, so he edits the timer and sets the day flag for
>   Tuesday by pressing '2'

I have no problem if Skip() continues to mean 'skip next recording' and 
sets day to one day later. With the fixed calculation of StartTime(), 
its not a big difference in user experience, and using Skip() to 
calculate future timers isn't the main goal here. (and can be done by 
either looping Skip() or smart use of Matches(t))

However, I would suggest to keep at least one startTime = 0; in Skip(), 
so the cached startTime is properly invalidated.

Cheers,

Udo
  
Klaus Schmidinger May 25, 2006, 2:39 p.m. UTC | #8
Udo Richter wrote:
> Klaus Schmidinger wrote:
>>> Udo Richter wrote:
>>>> The original patch did not attempt to fix Skip(), it just fixes the 
>>>> wrong calculation of StartTime(). Skip() could be fixed by this 
>>>> additional patch: (untested!)
>>>
>> I'm afraid this might have a nasty side effect.
>> Imagine the following scenario:
>>
>> - a weekly timer that records on Mondays at 20:00
>> - at Monday, 19:00 the user decides to skip today's recording,
>>   so he presses the red button on the timer
>> - some time later he decides to make this timer also record
>>   on Tuesdays, so he edits the timer and sets the day flag for
>>   Tuesday by pressing '2'
> 
> I have no problem if Skip() continues to mean 'skip next recording' and 
> sets day to one day later. With the fixed calculation of StartTime(), 
> its not a big difference in user experience, and using Skip() to 
> calculate future timers isn't the main goal here. (and can be done by 
> either looping Skip() or smart use of Matches(t))
> 
> However, I would suggest to keep at least one startTime = 0; in Skip(), 
> so the cached startTime is properly invalidated.

Ok, that certainly makes sense.

Klaus
  
C.Y.M May 28, 2006, 5:46 a.m. UTC | #9
>
>
> C.Y.M. maybe you could test this fix and if it works I will include it
> into the next version of Enigma.



What is the fix? I see the section of the code that  Udo is talking about,
but I am not really sure how it is supposed to be.

Best Regards.
  
Andreas Brugger May 28, 2006, 3:23 p.m. UTC | #10
Stone schrieb:

>
>
>
>     C.Y.M. maybe you could test this fix and if it works I will include it
>     into the next version of Enigma. 
>
>
>
> What is the fix? I see the section of the code that  Udo is talking 
> about, but I am not really sure how it is supposed to be.
>
> Best Regards.


You should change a part of void cText2SkinStatus::UpdateEvents(void) in 
status.c so it looks like that:

> if (!dummy.IsSingleEvent()) // add 4 additional rep. timer
> {
>     int j = 0;
>     do
>     {
>         j++; // just to avoid a endless loop
>         dummy.Skip();
>         dummy.Matches(); // Refresh start- and end-time
>     } while (!dummy.DayMatches(dummy.StartTime()) && (j < 7));
> }

This works here. Also with the official release of Klaus.
  

Patch

--- timers.c.orig	2006-05-20 18:50:49.000000000 +0200
+++ timers.c	2006-05-22 22:19:02.134688704 +0200
@@ -560,6 +560,9 @@ 
  void cTimer::Skip(void)
  {
    day = IncDay(SetTime(StartTime(), 0), 1);
+  startTime = 0;
+  day = SetTime(StartTime(), 0);
+  startTime = 0;
    SetEvent(NULL);
  }