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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text Search timer type and other fixes #4

Closed
wants to merge 7 commits into from

Conversation

metaron-uk
Copy link

Here is the promised 'TEXT_SEARCH' commit and several other small bugfixes.
As it won't merge automatically, I guess this means we've been at the same part of the code again 馃榾
I've been a bit more structured this time, so each different 'fix' has a separate commit. Feel free to cherry pick if you like the look of something.
I'll take a look at your latest changes and re-base tomorrow.

@janbar
Copy link
Owner

janbar commented Jun 28, 2015

hmmm, too many different things in the same PR. It will be too complex to merge this.
Else, i think it is not needed to add field isFullTextSearch in struct TimerEntry because the timer type contains already info. I thought you want to add a new type for keywords search. Title search is already implemented for daily, weekly, one and all. So it would be better and more clear this timer type provides only keyword search. Like that no need to show button "full text search" and the code will be more clear avoiding confusion with other types, specialy with "Record all".

@janbar
Copy link
Owner

janbar commented Jun 28, 2015

So before changing anything, please rebase your branch on mine. You will have to resolve some conflicts with my last fixes. Then i will try to cherry pick one by one. Thanks @metaron-uk

If LastRecorded and NextRecording are both 0
fall back to the default rule.StartTime/rule.EndTime
Affects all recording rule TimerTypes
(TIMER_TYPE_RECORD_SERIES wasn't previously included)
Also adds additional debug statements to work out what's going on
Fixes conversion of 'Title' to 'Keyword' searches using UI radio button
@metaron-uk
Copy link
Author

@janbar
I've incorporated your comments (I hope!) rebased to latest series_recordings and added two extra commits which I was working on last night.
Hopefully this won't break any existing rules and will make debugging easier.

There are still broken corners (for some reason duplicate match isn't changing or defaulting properly in Text Search) which I am still investigating, but these can be fixed later.

I was thinking about EPG info. I don't understand why it is necessarily to re-associate a rule to EPG info after initial creation:

  1. For 'older' rules there won't be any EPG info available to re-connect to
  2. Upcoming recordings will provide the necessary icons in the EPG grid.
  3. I've managed to schedule recordings from Mythweb for years without worrying about EPG past the initial creation. EPG helps you set up the rule, but then the rule becomes stand-alone based on the information it contains. It's then the backend scheduler's job to associate it with EPG entries.
  4. If the Timer settings dialog presents options (like channel / time) then the user will be expecting these to over-ride any EPG settings - otherwise, why have them?

I believe EPG info should be used during timer creation when it is the master data. After that point, the rule stored in the back end should be regarded as the master data. If this philosophy was adopted, I think the timer types and code in MythScheduleHelper would become much simper and easier to maintain. The user will also be much more likely to be able to use kodi as a frontend alongside mythfrontend and mythweb, which I believe a significant proportion of the userbase (including myself) do.

@metaron-uk
Copy link
Author

Regarding the conflict caused by having 'Title Search' in both the Text Search and other rules, I think we have different opinions. I will move the discussion to the forum as other users (@nickr, @MikeB2013 etc...) will probably also have valid opinions.

@janbar
Copy link
Owner

janbar commented Jun 29, 2015

About EPG i pushed change to retrieve EPG info using selected channel and time instead the broadcast id. But never i mix EPG data with random channel or time. So if user select an other channel the EPG data will be retrieved from selected channel: the effect is the same as selecting the show in the guide. In all cases entry.chanid is never used to build rule for an epg-based timer.

@janbar
Copy link
Owner

janbar commented Jun 30, 2015

In fact, i never used ProgramID to attach timer or upcoming with an event. The major reason is this data is not filled by most of xmltv filler. So the unified solution is to use channel/starttime and that works perfectly with the addon since we have published it in 2012. Also since i linked the addon with cppmyth, we no more hack the myth database and use exclusively RESTFull service. There never you will find any end-point in the API to find event by ProgramID and for the same good reason.
So, upcomings are not really linked with event. In real word i make a broadcast number containing first 16 bits of chanid and part of number of minutes since epoch modulo 2 power 16. This number is unique for a period -22 , +22 days in the events calendar. Obviously i push all events to pvr manager using same algorithm. Finally pvr manager (kodi) can do the match using this provided magic number.

@janbar
Copy link
Owner

janbar commented Jun 30, 2015

@metaron-uk , I cherry picked your fixes, and inspired from your proposal i pushed commits to add 2 new timer type: SEARCH_KEYWORD and SEARCH_PEOPLE. All of them seem to run great and hopefully they would resolve most of your requirements. Let me know. Thanks for your work.

@janbar
Copy link
Owner

janbar commented Jun 30, 2015

Please let me know in the thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants