Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/service cleanup #423

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

adamsutton
Copy link
Contributor

Code to clean up old out of date services, I think this probably needs some testing before inclusion. However it will take a while to test since the timeout is 1 week.

@perexg
Copy link
Contributor

perexg commented Jun 11, 2014

Thanks for this.

I think that we should also track the successful last tune time and do not delete muxes when they were not tuned for a reason. This reminds me - actually, we have only "early fail" error code for the tuning operation, but if something bad happens later, the upper layer does not know that the tuning failed - this is the example, where this situation should be evaluated before the mux is removed.

Also, I would prefer to have the check time window configurable between 1 - 7 days, for example.

@adamsutton
Copy link
Contributor Author

@perexg actually deleting muxes is a bad idea full stop, I'm planning to remove that. Better to mark them as disabled. Else dodgy entries in the NIT can result in constant in/out.

The only reason, or at least intent, is that there is a failure to tune for N days. But you're probably right that we should be trying to track whether a tune example has been tried.

What sort of failures are you thinking about? The fact that it fails to lock etc...? I agree this does need to be better handled.

@bluzee
Copy link
Contributor

bluzee commented Jun 11, 2014

I think the Autodiscover, Autoscan etc. naming makes functions easier to understand than previous.

I switched autoclean services on for one network. The network has one Mux with 2 good services and 6 defunct services. Flipped the MUX to active, it rescanned and autoclean ran. Deleted the 2 good services and left the 6 defunct. Deleted the MUX, recreated, rescanned the 2 services. Repeated the autoclean but it deleted the two again. Perhaps I'm not using this correctly.

It's a good idea. There are a lot of factors that can mess with it though. Solar outages for example. Personally, just a delete button on the services list would suffice.

@adamsutton
Copy link
Contributor Author

@bluzee make sure you're definitely using the latest code on this branch, the original code had the time check inverted so it would have done exactly what you report.

@adamsutton
Copy link
Contributor Author

@bluzee just to clarify I squashed the commits to hide the shame ;) which was probably a bit naughty while the code is under review, but I thought I'd get away with it.

@bluzee
Copy link
Contributor

bluzee commented Jun 12, 2014

fetch bluzee fetch....

Ok. Yes now it's deleting dead services and keeping the active ones. Could be handy for sure. I can't leave it on auto as TVH has no way to know what satellite the dish is pointed at, but when I do want to rescan a satellite it's pretty simple to flip it on for the one network. I have each satellite configured on it's own network.

@adamsutton
Copy link
Contributor Author

@bluzee the fix the rotor code ;) unless we add .1 support it'll never definitely know, but if it waits long enough (define long enough) then it should be in the right position. But yeah, rotors do make things that much more complex. But it won't delete anything that's it's seen in the last N days (and @perexg is right the N needs to be configurable).

The next iteration won't delete muxes either, since that's fraught with problems, all it will do is disable things it can't tune/receive (though that probably also needs some reworking to understand what "can't tune" really means).

@bluzee
Copy link
Contributor

bluzee commented Jun 12, 2014

All that code is way above my pay grade. I'm not using a diseqc motor though either. Still turning the dish with the analog receiver. There is no way to sort that out in TVH, nor would I ever expect there to be. Main thing is it seems I can now control when MUXes scan and that is a game changer. So, unless I notice something catastrophic in the next little bit I'll vote for this code. The odds of services being deleted due to outages from solar, rain, snow etc. are probably pretty low.

One thing I noticed is channel names were erased when services were deleted but the channels were not deleted. After rescan it made it tricky to figure out which which channel to map the services back to so epg would match. Services where I had manually renamed the channel did not have the channel name erased. In this case it was nice that it didn't delete the channels because I had accidentally autocleaned the good services. Not sure what the best plan would be for the channels with autoclean working correctly. Should the channels also be deleted?

@adamsutton
Copy link
Contributor Author

@bluzee that's an interesting side effect I'd not thought about and honestly I'm not sure. The reason the name disappears is the channel has no name, but master has the concept of if no channel name is defined fetch it from one (arbitrary as to which) of the underlying services.

Now arguably if you delete all services attached to a channel you could delete the channel. However I'd be pretty pissed if TVH did that to me, as my channels all have hand crafted names, numbers and icons. Also it'll mess up any recordings tied to the channel etc... So if a service did get accidentally removed you can't simply re-associate with the channel and carry on.

However MAYBE TVH should at least do something slightly more friendly if the last service is removed and no name exists. Maybe set the name to something really obvious like:

_DELETED - $svcname

_ just so that sorting will put these at one end of list
$svcname being the name of the last service being deleted, ofc if that has no name you're not going to get much, but hey ho.

This stuff is never trivial when you consider the horrible myriad of possible configuration people might try and use with TVH! A STB would be so much simpler!

@bluzee
Copy link
Contributor

bluzee commented Jun 12, 2014

Yes, that's the thing. Creating the channel in the first place takes time. Finding an icon, finding EPG source, tagging, channel numbering. It's a process. Marking the channel somehow when the associated service gets deleted sounds like a good idea. If you really do want to delete the channel later that is very simple. Most of the time the channel still exists somewhere, perhaps a different frequency or even a different satellite. When you find the new service it can be quickly mapped to the old channel.

STB's have trickier issues with hardware. I disconnected my STB because there are now so many channels that it can't play due to odd ball codecs or encoding. If the hardware can't decode then there are no options. Also, the same satellite dish configurations still exist. No difference there.

On a fixed dish system where all muxes are constantly visible this is ideal. Even on a complex system like mine it will be helpful. This is good.

@perexg
Copy link
Contributor

perexg commented Dec 4, 2014

Just a note. I implemented really simple solution to auto-disable the services which are no longer in PAT:

89baf88

With the manual service removal (which was implemented in the meantime), I think that this solution is enough to discover the dead services.

@adamsutton
Copy link
Contributor Author

@perexg not sure if you're suggesting that in place of the auto remove stuff I've done here? It would still be nice to actually remove things? The code here works fine, I've tested a fair bit, but just not had time to push it.

@perexg
Copy link
Contributor

perexg commented Dec 4, 2014

@adamsutton : I think that both solutions/actions may co-exist. I made my code unconditional, because the service is only disabled (not removed) and the check interval is 24 hours only. The auto-remove feature maybe ok for someone, too.

@bluzee
Copy link
Contributor

bluzee commented Dec 4, 2014

I played with perexg commit a bit this morning. It disabled some services out of a MUX that don't belong there. They were scanned in by TVH while the dish was parked on a different satellite and it found a MUX on the same frequency. So, this works. I wasn't able to get it to disable any services it should not, so that is good. I really hate when features are added in unconditionally though. No way to opt out if it causes problems and there are too many configurations out there to be able to predict how some of these features may cause a problem for users.

My channels are spread across 30 plus satellites from 55W to 138W. They are constantly moving from one sat to another. They are constantly changing. Channels go off the satellite, new ones come on. Channels crypt and unencrypt. Needless to say this is a feature I'd love as long as it doesn't change anything just because it's parked on one satellite and can't see channels on the other satellites.

I have not played with adamsutton PR for a while but I can't remember any game stopping issues and I seem to recall quite liking it.

@bluzee
Copy link
Contributor

bluzee commented Dec 4, 2014

Sorry, I think hate is too strong a word for what I mean. I mean it's a cause for concern when there is a possibility a feature may break things for some users and there is no easy option to disable it.

Things are not always done to spec in the DVB world. Hiding channels by leaving them out of PAT is a common practice. I've had channels in a mux that were nothing more than a Video ES and and Audio ES.

@perexg
Copy link
Contributor

perexg commented Dec 5, 2014

@bluzee : It's good point (services without PAT). I added 'Automatic Checking' field to the services list now which offers to disable the Automatic Service checks and this field is also used to notify users that the service was disabled, because it is missing in PAT (it is handy to sort grid using this column to see all automatically disabled services together).

f20bab4

@bluzee
Copy link
Contributor

bluzee commented Dec 5, 2014

Tested and seems to be working just fine. If I run into any issues I'll let you know.

@ProfYaffle
Copy link
Member

@galeneca PLEASE. STOP. IT. You're spamming probably several hundred people with notifications while you learn to use github.

Please, set up your own repo(s) and practice on those.

@perexg perexg force-pushed the master branch 5 times, most recently from e51af23 to 792a7ca Compare April 19, 2017 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants