openatv / enigma2

openatv-gui
GNU General Public License v2.0
198 stars 317 forks source link

[Timers] BUG: Far-future conflicting timer(s) block every type of immediate recording. #2827

Open wedebe opened 1 year ago

wedebe commented 1 year ago

Describe the bug / Actual behaviour: Where a conflicting timer exists - no matter how far into the future - it becomes impossible to:

Error message Record time limited due to conflicting timer is shown.

Expected behaviour: Ability to record should be vetoed ONLY if end time reaches the time period affected by conflicting timers.

If recording DOES overlap, present the Conflicting Timers screen.

Has this issue started to happen just recently? No, this has been a problem as long as I've been using openATV.

To reproduce: Steps to reproduce the behavior:

Screenshots 20230422195252

GhostofGeeeee commented 8 months ago

Can you please update this Issue, i think there was fixes!

wedebe commented 7 months ago

Still an issue as of 2023-11-11 image timer20231111122016

timer20231111122049

>>> int(datetime.now().timestamp())
1699705391
<?xml version="1.0" ?>

<timers>
    <timer begin="1699992000" end="1699995600" marginBefore="0" eventBegin="1699992000" eventEnd="1699995600" marginAfter="0" hasEndTime="1" serviceref="1:0:19:5212:812:2:11A0000:0:0:0:" eit="446" cridSeries="crid://missing_authority/ebshd45386" cridEpisode="crid://missing_authority/1617490572" cridRecommendation="[]" repeated="0" rename_repeat="1" name="New: The Martin Lewis Money Show Live" description="Martin is back for his annual Christmas special" tags="AutoTimer" afterevent="auto" disabled="0" justplay="0" always_zap="1" descramble="1" record_ecm="0" isAutoTimer="1">
        <log code="500" time="1699403855">[AutoTimer] Try to add new timer based on AutoTimer New: The Martin Lewis Money Show Live.</log>
        <log code="509" time="1699403855">[AutoTimer] Timer start on: Tue Nov 14 20:00:00 2023</log>
        <log code="501" time="1699418255">[AutoTimer] Warning, AutoTimer New: The Martin Lewis Money Show Live messed with a timer which might not belong to it: New: The Martin Lewis Money Show Live .</log>
        <log code="501" time="1699447055">[AutoTimer] Warning, AutoTimer New: The Martin Lewis Money Show Live messed with a timer which might not belong to it: New: The Martin Lewis Money Show Live .</log>
        <log code="501" time="1699679311">[AutoTimer] Warning, AutoTimer New: The Martin Lewis Money Show Live messed with a timer which might not belong to it: New: The Martin Lewis Money Show Live .</log>
    </timer>
    <timer begin="1699995600" end="1699999200" marginBefore="0" eventBegin="1699995600" eventEnd="1699999200" marginAfter="0" hasEndTime="1" serviceref="1:0:19:50E6:80F:2:11A0000:0:0:0:" eit="1483" cridSeries="crid://missing_authority/ebshd52714" cridEpisode="crid://missing_authority/1617466423" cridRecommendation="[]" repeated="0" rename_repeat="1" name="New: Big Brother" description="A chance to catch up with everything that&apos;s happened in the house." tags="AutoTimer" afterevent="auto" disabled="0" justplay="0" always_zap="1" descramble="1" record_ecm="0" isAutoTimer="1">
        <log code="500" time="1699403854">[AutoTimer] Try to add new timer based on AutoTimer New: Big Brother.</log>
        <log code="509" time="1699403854">[AutoTimer] Timer start on: Tue Nov 14 21:00:00 2023</log>
        <log code="501" time="1699418254">[AutoTimer] Warning, AutoTimer New: Big Brother messed with a timer which might not belong to it: New: Big Brother .</log>
        <log code="501" time="1699447054">[AutoTimer] Warning, AutoTimer New: Big Brother messed with a timer which might not belong to it: New: Big Brother .</log>
        <log code="501" time="1699679309">[AutoTimer] Warning, AutoTimer New: Big Brother messed with a timer which might not belong to it: New: Big Brother .</log>
    </timer>
</timers>
GhostofGeeeee commented 7 months ago

it becomes impossible to: record current event record indefinitely record duration record with end time, no matter how long or short the intended record time.

I think this is not correct anymore!

GhostofGeeeee commented 7 months ago

I did some investigation, I know the issue by myself. It's possible to not understand the situation!

Not only InstantRecord becomes impossible! You can't add any timer. -> You need to fix the conflicts first. It's not that easy to create a conflicting timer! I can't reproduce it with AutoTimer.

wedebe commented 7 months ago

My apologies, I eventually figured out how these were happening but wasn't online to update this.

It happens most times after an image flash and settings restore. If I can, I'll try zip the epg, timers and autotimers files for debugging.

GhostofGeeeee commented 7 months ago

@wedebe It stays interesting how to reproduce, but because I know this too(min. 1 case), I try to fix. Please test(InfoBarGenerics.py) and let me know about close #2827, #3093, #3122, #3123, #3124.

jbleyel commented 7 months ago

Hi @GhostofGeeeee, what's the exact intention of the TimerSanityCheck inside of startInstantRecording? I have not finally checked this change but I think you are on the wrong track.

GhostofGeeeee commented 7 months ago

If you add a timer there is a check for conflicts and the ConflictTimerOverview get open then the recording doesnt start but you can fix the conflicts. But with Instand Recording there is no ConflictTimerOverview and so on. It looks that the code can be improved global but I don't understand setAutoincreaseEnd etc..

This is not a improvment only a fix for old conflicts on timers. I'm not sure that we need this but maybe we need bigger improvments to dont make the user confuse.

GhostofGeeeee commented 7 months ago

If you can reproduce it then this will help you first fix old timer conflicts and start instand recording else you get a message "Record time limited due to conflicting timer", no Instand recoring is started and you need to undertsand by your self the unmatched far future conflict. This is not easy!

jbleyel commented 7 months ago

We need a step by step "how to reproduce" if this is not already documented in an existing issue.

wedebe commented 7 months ago

I've created a settings backup in the hope it might help. This has a few record events which overlap, as well as actual EPG data. I'm not sure how it'll behave on a system set up for anything other than Astra 28.2ºE.

enigma2settingsbackup.tar.gz

Note

Whereas the steps below are a synthetic scenario, the ultimate issue here is that an immediate recording which spans - say, the next 45 minutes - shouldn't be prevented because of a timer that starts in 46 minutes' time (taking before margin into account). Any plugin could potentially create messed-up timers so openATV should be able to handle them gracefully.

Reproduce

Ultimately though, the steps I took to reproduce are as follows... 1) use the EPG to create a record timer for an event at a particular time in a few days' time 2) repeat step 1, with a similar start time, this time selecting a service/channel on a different transponder (or likely to be), and setting Enabled to no 3) repeat step 2 a few more times 4) restart the box (not sure if this is actually needed to save /etc/enigma2/timers.xml to disk) 5) ssh into the box 6) type init 4 and enter 7) edit /etc/enigma2/timers.xml 8) change enabled="0" to enabled="1" for each timer created in steps 2 & 3 9) save changes 10) type init 6 and enter 11) wait for box to restart 12) open record menu by pressing the Record button while watching a program (don't use EPG) 13) select Add recording (stop after current event)

Notification should appear stating Record time limited due to conflicting timer ...

Screenshots

conflicts20231123134217

conflicts20231123134420

wedebe commented 7 months ago

I understand there's a huge amount of work being done to refactor much of the openATV code. Hopefully recording will be overhauled as it seems to be something of... a mess 😬

Case in point... The comment above triggers a notification pointing to the first of the conflicting timers.

HOWEVER! On a service/channel with no EPG data (eg. on Astra28.2ºE there's - ironically - an IEPG data 1 channel), using the Record menu to Add recording (stop after current event), results in a conflict notification but this time shows the next record timer.

conflicts20231123134217

conflicts20231123134344

conflicts20231123134250

conflicts20231123134325

wedebe commented 7 months ago

Attempting to add a record timer via EPG will show the ConflictTimer Overview screen, BUT in this case probably shouldn't, because the selected event isn't affected by the conflict time range.

conflicts20231123134217

conflicts20231123141115

conflicts20231123141129

wedebe commented 7 months ago

I haven't even started looking into Zap timers 🫥

wedebe commented 7 months ago

Perhaps a prompt when a user tries to add a timer when a conflict/conflicts exist:

(i) Oops
---
There are conflicting timers, what would you like to do? (or There are conflicting timers, would you like to resolve them?)
---
[Resolve conflicts] (or [Yes])
[Not Now]
IanSav commented 7 months ago

@wedebe the timers and recording system are definitely on the list to be refactored. @jbleyel and I made a start on this a while ago but had to back down when we noted how much code has strong dependency on the current timer and recording code. We did make a good start on the refactor but started to come unstuck when we started changing and fixing functionality. There is a lot of code and plugins that depend on current variables, methods, data structures and quirks of the current code that means that fixing this code is going to be very difficult.

For the moment we try to fix bugs as they are documented but a proper refactor of all this code is not likely to be in the near future. I suspect that we will need to create a whole new timer and recording system with proper APIs. We could then use the old code as a stub to redirect all the other code and plugins into the new recording system. This is how I fixed all the issues with languages. I created International.py and then made Languages.py into a stub the did all the legacy conversions.

GhostofGeeeee commented 7 months ago

@wedebe Now we have a way to reproduce, but is there a bug? Is the way an hack? Do we need a fix?

Any plugin could potentially create messed-up timers so openATV should be able to handle them gracefully.

This is not a bug!

The way to detect(old conflicts) is before add a record.

Attempting to add a record timer via EPG will show the ConflictTimer Overview screen, BUT in this case probably shouldn't, because the selected event isn't affected by the conflict time range.

All these conflicts aren't in the correct range! You want ignore the conflict? No good feature! Here you want ignore, there you want a prompt.

Perhaps a prompt when a user tries to add a timer when a conflict/conflicts exist:

(i) Oops
---
There are conflicting timers, what would you like to do? (or There are conflicting timers, would you like to resolve them?)
---
[Resolve conflicts] (or [Yes])
[Not Now]

Do we need this prompt, if you hack the system? Else you can ignore on ConflictTimerOverview!

Was there feedback? I think No. Do we now have an documented issue? Or a long discussion of no interess(But we have 4 more equal issues!) Have a nice time!