google / sagetv

SageTV is a cross-platform networked DVR and media management system
http://forums.sagetv.com/
Apache License 2.0
265 stars 174 forks source link

Integrated support for Schedules Direct EPG #161

Closed enternoescape closed 7 years ago

enternoescape commented 7 years ago

This is one of the barriers to entry for new SageTV users. Schedules Direct needs to be a first class citizen fully integrated into SageTV. It needs to be selectable and configurable within the UI.

I have been looking at doing this over the past few weeks and I think it would be best to build everything from the ground up instead of using the plugin. Re-writing this EPG service will result in it being written as a piece of the core, using all that is available internally and not something that is somewhat disconnected. We should also be able to eliminate some library dependencies that we would otherwise need to account for to use the plugin code.

For the JSON parsing, I propose that we use Gson since it's owned by Google already and a compatible license. I will just take the parts we need and change the root class path to sage.epg.sd.gson so it doesn't conflict with any plugins. I would leave the Gson code otherwise unchanged so we could upgrade it if the need were to arise without any serious complications. The path for the schedules direct classes would be sage.epg.sd.

My first goal would be to get everything working as well as the current integrated solution along with UI support. After that is committed, I would take a look at ways we can incorporate additional data provided by SD that SageTV may not currently have a use for, but would provide an opportunity for new options/features.

I propose that we add the following option to the EPG Lineup Setup Wizard. Use Schedules Direct Guide Data with this Source

1) The next screen would ask for your username if this is your first time, if this is not your first time skip to step 2.

Please enter your username for Schedules Direct. <text box>

The next screen would ask for your password if this is your first time. Passwords are stored as a sha1 hash in hex somewhere other than Sage.properties.

Please enter your password for Schedules Direct. <masked text box>

2) If this is not your first time, you are presented with the option to use your existing username and password or change them. Selecting to change then takes you back to 1 with the username already populated in case you just wanted to change your password.

Use Existing Username and Password Change Username and Password

3) If authentication fails, you will see a message to that effect, then be taken to 2. You are not forced to change the saved username and password, but of course you should probably enter everything again. If the server is offline, you will see a message indicating this and your username and password will be saved, but you will be unable to proceed and will also be taken to 2. If everything goes well, you proceed on.

Authentication with Schedules Direct failed. Please check your username and password. Schedules Direct not available right now. Please try again in 30 minutes.

4) The API will then query for your existing lineups and allow you to select from what you already have or add a new one. If no lineups currently exist, you are taken to step 5 directly. When removing, it will check to see if any other capture device is using the lineup you are about to remove and will warn you about this fact.

Please select an existing lineup or add a new lineup.

Lineup 1 > prompt if you want to select the lineup or remove it. If you select, you're done. Lineup 2 > prompt if you want to select the lineup or remove it. If you select, you're done. Lineup 3 > prompt if you want to select the lineup or remove it. If you select, you're done.

Add a New Lineup > go to step 4 if you have never added a new lineup. If you have gone through steps 5 through 7 before, you will be shown a dialog asking if you would like to select the same region, country, postal code again which takes you directly to 8 or if you want to select something else, you'll go to step 5.

Would you like to use the previously entered Region, Country and Postal Code (North America, United States, 12345)? Use the Postal Code 12345 > go to step 8. Select a different Region, Country and Postal Code > go to step 5.

The select or remove dialog would be: Select this Lineup Remove this Lineup

5) Get the list of regions and countries from SD. List regions to select from, selecting one takes you to step 6.

Please select your region. North America Europe Latin America etc...

6) List countries for region selected in the previous step. Selecting one takes you to step 7.

Please select your country. United States Canada

7) Query for postal code including an example.

Please enter your postal code. (Ex. 12345) <text box>

After entering the postal code, it will be checked against the regex provided from SD to ensure it's valid. If it's not valid, an error message will appear. Otherwise, we proceed to step 7.

You have entered an invalid postal code.

8) The API will now query for all of the lineups in the provided postal code and present you with a list. At this point, your region, country and postal code have been saved so your last selections can be used to skip these steps for adding the a second lineup. Making a selection here adds the lineup to your account and you're done.

Please select your cable or satellite provider (press a key to jump to lineups starting with that letter): Provider 1 Provider 2 Provider 3

There will of course be more to discuss, but I just wanted to get a few ideas out there in case anyone has better ideas on how the UI should flow for this feature or if any of my ideas will be harder than I think to implement and what might be a better alternative. For those who do not know, Schedules Direct stores your lineup selections on their server and the UI suggestions I posted here are based on this fact.

stuckless commented 7 years ago

Personally, I'm ok if you want to include gson jar, as is... even it does conflict with plugins. We'll sort out the conflicts, if they arise... like we did with lucene.

CraziFuzzy commented 7 years ago

While we're at it (touching the EPG source wizard), however, we should probably make appropriate similar changes to the google source as well (primarily, the step to be able to enter the key from within the UI).

enternoescape commented 7 years ago

@CraziFuzzy I like that idea, I'll do that too. I'll add that to my while in there list.

enternoescape commented 7 years ago

@stuckless I know it came up in the forums that we could just change the class path to remove any issues with plugins. I really would prefer to just use the jar, but as you know, the way we load plugins will always create problems. If maybe if a way to work around this problem might be in the works over the next year or so, I should just use the jar instead.

CraziFuzzy commented 7 years ago

@enternoescape The gson jar issue is not that serious - as @stuckless said, we can most likely just fix the few plugins that are affected by it, and roll gson into the core (the same way lucene was rolled in). I certianly don't feel it's inappropriate to include google/gson in google/sagetv.

In reality, all that likely needs to be 'fixed' in the affected plugins, is the repository itself and remove the dependency on gson - since it will already be there in the classpath. The v9 repository is not store in space we can manipulate, and newer versions there will override the v7 repository downloaded from google's server.

Narflex commented 7 years ago

@enternoescape Thanks for digging into this. Personally, I'd be for putting the GSON code into the SageTV core like you proposed...it just avoids creating more issues for the time being and the less things to put people off, the better.

I do think it's best that you actually make a class that extends EPGDataSource like WarlockRipper does. I'm not sure if you intended to do that or not based on what you wrote; but that'll make it the cleanest integration. You definitely don't need to use the EPG plugin mechanism.

The UI flow looks a little complex...but it sounds like that is needed based off the way SD works; it also looks like this will get international support, which is great.

Definitely go over the data mappings for converting everything in the SD database into what SageTV wants with me. There's a fair amount of complexity to how some of the advanced correlations work, and hopefully SD has the data to take advantage of all of that (I looked over the API somewhat, but I didn't find everything I was looking for in there, but saw some good things). There's also fields we have in there for storing the image data which looks like it should map cleanly to the URLs they are using for image hosting as well (they use the same filenames and path structures that mirror Tribune's actual FTP site which is what we did on Fiber and I left that code in there hoping it would be useful for that same purpose).

On Wed, Jul 27, 2016 at 10:07 AM, Christopher Piper < notifications@github.com> wrote:

@enternoescape https://github.com/enternoescape The gson jar issue is not that serious - as @stuckless https://github.com/stuckless said, we can most likely just fix the few plugins that are affected by it, and roll gson into the core (the same way lucene was rolled in). I certianly don't feel it's inappropriate to include google/gson in google/sagetv.

In reality, all that likely needs to be 'fixed' in the affected plugins, is the repository itself and remove the dependency on gson - since it will already be there in the classpath. The v9 repository is not store in space we can manipulate, and newer versions there will override the v7 repository downloaded from google's server.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-235652850, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDH9YjsCV1cxS2fAtoR44gxYffnplks5qZ5BvgaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 7 years ago

@Narflex I didn't mention it, but yes, I am going to extend EPGDataSource just like WarlockRipper. I know the UI parts seem a little complex, but they are modeled around what SD has to offer and I want this to support international, otherwise we would be missing a whole other market completely. SD also supports alternative guide data languages which I will provide "hooks" for that we could probably tie directly to the language selected when setting up SageTV, but I'll focus a little harder on that at a later date.

I am already a little into this; I was spending my time between the last commit and your comments working on this. :) I just finished writing tests for the authentication parts and getting the available services with regions and countries last night. I'm working on making sure the most basic parts of communication with SD are reasonably solid and efficient, then I'll be moving on to the harder parts involving matching things up with what SageTV can use. I'm sure I'll have plenty of questions. :)

rkulagowski commented 7 years ago

There are very few things which are hardcoded in the API as far as countries, regions, etc. Please don't hardcode something like that because it just means that users would need to recompile if we add a new region.

Also, not even the number of lineups is fixed; the default is 4 but that can be adjusted at the server if a user has a legitimate reason for more lineups. That's not to say that you need to be able to handle 254 lineups, but don't assume that it's capped at 4.

enternoescape commented 7 years ago

I am not hard-coding the countries. I intended to try to correlate them with what SageTV has already. The way I have the coded right now, they will always be downloaded when adding a new lineup. Maybe it would be better leave it a separate option.

I didn't assume any such thing about the lineups being limited. My UI mock-up is just that, it will essentially be a list that could be "unlimited."

rkulagowski commented 7 years ago

Another thing is that stationIDs aren't integers - they're strings. stationIDs in NZ and AU start with NZ or AU as appropriate due to a namespace collision with existing data.

Narflex commented 7 years ago

When the stationIDs are not just integers, do they have a consistent AU or NZ prefix for that whole lineup? Currently SageTV stores all stationID values as integers.

Jeff Kardatzke Sent from my Android

On Jul 27, 2016 6:50 PM, "Robert Kulagowski" notifications@github.com wrote:

Another thing is that stationIDs aren't integers - they're strings. stationIDs in NZ and AU start with NZ or AU as appropriate due to a namespace collision with existing data.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-235774940, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDB-5th_ki-tec3GSDO6CHbBleH2Pks5qaArugaJpZM4JVuyx .

rkulagowski commented 7 years ago

If the plan is to strip AU or NZ to turn the string to an integer, please track that as a boolean somewhere, because when you request the schedule you'll need to add it back in the request back to the API.

Narflex commented 7 years ago

I was thinking we would just set a stationID prefix value for the lineup and then append/prefix that when it makes requests to SD. I'm not concerned about them using cross country lineups and having conflicts in the internal DB.

Jeff Kardatzke Sent from my Android

On Jul 27, 2016 7:51 PM, "Robert Kulagowski" notifications@github.com wrote:

If the plan is to strip AU or NZ to turn the string to an integer, please track that as a boolean somewhere, because when you request the schedule you'll need to add it back in the request back to the API.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-235786525, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDBkRUGAeWoUXHx-UM_WWXXCqPq7hks5qaBkpgaJpZM4JVuyx .

rkulagowski commented 7 years ago

Not all stations will necessarily always require the prefix, and the transition from requiring the prefix to not requiring it may not be a "flash cut" and may be implemented on a per-stationID basis.

Narflex commented 7 years ago

Ok, that's what I was wondering. We can just use some higher order bits in the integer then which is beyond the reach of the stationID values that Tribune uses to indicate the two prefixes.

Jeff Kardatzke Sent from my Android

On Jul 27, 2016 8:01 PM, "Robert Kulagowski" notifications@github.com wrote:

Not all stations will necessarily always require the prefix, and the transition from requiring the prefix to not requiring it may not be a "flash cut" and may be implemented on a per-stationID basis.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-235787802, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDFl8jgmrdaKZJfCi5Qi0GRp4jU1cks5qaBuSgaJpZM4JVuyx .

CraziFuzzy commented 7 years ago

Well, I think the number is always a 32-bit INTEGER in sage, and it's not like sage needs to have the exact same 28-bit range that tribune uses, as long as the SD class will do the translation between our modified 32-bit INTEGER and the STRING that SD needs.

@rkulagowski can you give an example of a lineup that has these prefixes?

enternoescape commented 7 years ago

@CraziFuzzy I removed my comment because it didn't make any sense to me in hind-sight. I was thinking about bits and bytes and well, you can see how two characters would basically be a full integer.

enternoescape commented 7 years ago

It might be ok to use flags at first, but what do we do when more prefixes are introduced. We might have some unexpected issues suddenly requiring more patch-work.

rkulagowski commented 7 years ago

GET https://json.schedulesdirect.org/20141201/headends?country=aus

(or nzl as appropriate)

and specify a token in the header like most of the rest of the API; that will return the lineups, and from there you can do a call like:

GET https://json.schedulesdirect.org/20141201/lineups/AUS-NSW0073-DEFAULT

to pull the lineup for Sydney.

Narflex commented 7 years ago

Currently, Tribune doesn't go over 1,00,000 for station ID values. It used to be limited to 100,000....and it may still be, I don't remember exactly. That fits into 20 bits. I don't foresee this going much higher ever, because this is a slowly growing value...new stations are not added at any decent rate.

We don't want to use the top bit, since station IDs less than 10000 have a certain meaning in SageTV (so no negative values). I was also thinking we'd just use 2 bits for this (and we have only 3 values to represent currently), and could extend it downward to use more later if more prefixes arise...but I don't think this special case would go much further than this. So that leaves us with 29 bits, which is about half a billion...we'll never hit that with stations. It also gives us 9 more bits for prefixes if we ever need that.

And in the end...if we ever do actually run out of bits...we could just change the DB to use Strings instead, but I'd rather avoid that overhead until it is actually truly needed.

On Thu, Jul 28, 2016 at 8:14 AM, Joseph Shuttlesworth < notifications@github.com> wrote:

It might be ok to use flags at first, but what do we do when more prefixes are introduced. We might have some unexpected issues suddenly requiring more patch-work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-235926017, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDH2X7_JLYF8mCC4rRHNnoMWlPtGlks5qaMdJgaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

rkulagowski commented 7 years ago

Gracenote is over 100000 stationIDs now.

Schedules Direct has custom channels and to avoid collision with Gracenote we have offsets that put stationIDs into the millions (if you're treating it as an integer and not a string)

{
    "stationID": "3021810",
    "name": "SD-QVC +1",
    "callsign": "SD-QVCUKP1",
    "broadcastLanguage": [
        "en-GB"
    ],
    "descriptionLanguage": [
        "en-GB"
    ],
    "logo": {
        "URL": "https://s3.amazonaws.com/schedulesdirect/assets/stationLogos/s21810_h3_aa.png",
        "height": 270,
        "width": 360,
        "md5": "68994cb1f824161057af64"
    }
}
Narflex commented 7 years ago

Robert, what's the range you use for the custom stationIDs for SD?

On Thu, Jul 28, 2016 at 9:33 AM, Robert Kulagowski <notifications@github.com

wrote:

Gracenote is over 100000 stationIDs now.

Schedules Direct has custom channels and to avoid collision with Gracenote we have offsets that put stationIDs into the millions (if you're treating it as an integer and not a string)

{ "stationID": "3021810", "name": "SD-QVC +1", "callsign": "SD-QVCUKP1", "broadcastLanguage": [ "en-GB" ], "descriptionLanguage": [ "en-GB" ], "logo": { "URL": "https://s3.amazonaws.com/schedulesdirect/assets/stationLogos/s21810_h3_aa.png", "height": 270, "width": 360, "md5": "68994cb1f824161057af64" } }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-235950457, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDJxwVbJ09F0QaabhDy67q-0ZT0VVks5qaNnNgaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

CraziFuzzy commented 7 years ago

So are the prefixes always the country code?

enternoescape commented 7 years ago

@CraziFuzzy That's a good question. That could leave us with only needing one boolean that indicates that there's a prefix and the string value of the prefix would be different depending on the country code.

enternoescape commented 7 years ago

@rkulagowski I do have a question about the "available services." The developer guide says that services could change and to check that URL to make sure the service you want is there so you can provide appropriate choices to the end user. I'm curious what alternative option you would have the end user select from if COUNTRIES was unavailable or is that not one of the services the note is talking about.

CraziFuzzy commented 7 years ago

I can certainly see more services being added - but I can't see them removing COUNTRIES - so I'm thinking for now, for the purposes of this feature, we can essentially hard-code that part.

enternoescape commented 7 years ago

I still have the code check to make sure it's available, before we use it, but basically you're not going to be able to proceed so long as that service shows that it's unavailable. I'm pretty much following the guide to the letter.

CraziFuzzy commented 7 years ago

certainly check - i was simply stating that you don't need to handle future additions to the services for our usage at this time.

enternoescape commented 7 years ago

@Narflex Even though I'm not going to use the plugin route to create the Schedules Direct EPG source, there appears to only be one EPG source allowed at a time. Would you have me make the UI only allow the built in or Schedules Direct to be used at one time (and changing between the two would require you to remove the lineups from all capture devices)?

Narflex commented 7 years ago

It allows for multiple types of sources at once. What did you see which made you think it wouldn't work?

Jeff Kardatzke Sent from my Android

On Aug 6, 2016 7:59 AM, "Joseph Shuttlesworth" notifications@github.com wrote:

@Narflex https://github.com/Narflex Even though I'm not going to use the plugin route to create the Schedules Direct EPG source, there appears to only be one EPG source allowed at a time. Would you have me make the UI only allow the built in or Schedules Direct to be used at one time (and changing between the two would require you to remove the lineups from all capture devices)?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238027414, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDO8LwM2S0rLia2HRpqwU_XkZ6VkLks5qdKFngaJpZM4JVuyx .

CraziFuzzy commented 7 years ago

I also had the impression that only one was able to be used at a time. I didn't see any way on an EPG update for it to process them from different sources on a per-lineup basis, or even a way to identify what source a given lineup is from.

Narflex commented 7 years ago

The EPG class goes through each of the EPGDataSource implementations and calls their expand methods so they can each update their own way via a subclass. This was already possible to use something like TVTV for one source , Tribune for another and then data scanning for another one.

The only constraint that needs to be dealt with is the UI only has APIs right now for one method. But another param could just be added to those few calls to specify which source type to use for creation and list of lineups.

Before we had separate API calls for TVTV altogether, but it had more specialization for logging in and other stuff....which I think SD has as well from your prior notes, so maybe another set of API calls for SD setup would be fine too. I don't foresee adding more types anytime in the near future.

These all link into different CaptureDeviceInput methods via the provider ID.

Does it make more sense now?

Jeff Kardatzke Sent from my Android

On Aug 6, 2016 2:21 PM, "Christopher Piper" notifications@github.com wrote:

I also had the impression that only one was able to be used at a time. I didn't see any way on an EPG update for it to process them from different sources on a per-lineup basis.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238049330, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDDDb5FKhNfphCoypJdel1_-DCi2qks5qdPq_gaJpZM4JVuyx .

CraziFuzzy commented 7 years ago

Okay, I can see that now - I do think there are places in EPG that call warlockripper directly, though, which now after looking further, only do so if the source isn't defined as from a different class. This sort of leaves the question of whether we should make the whole thing more agnostic and treat WarlockRipper just like any other potential class.

Regarding needing login information and such, we might think of adding a GetProperties (or something like that) call to the DataSource class, so each individual source class can define what properties it needs set. Even Warlock could be made to take advantage of this by allowing the license key to be entered from here. Then the UI can have a Source config page that is built from these properties.

Narflex commented 7 years ago

I think moving towards making things more generic is fine. And the idea about specific props for a data source sounds good too.

But I don't think we need to go overboard with making it too generic because as I mentioned before, I don't think it's likely we will be adding more data sources beyond these two.

Jeff Kardatzke Sent from my Android

On Aug 6, 2016 4:20 PM, "Christopher Piper" notifications@github.com wrote:

Okay, I can see that now - I do think there are places in EPG that call warlockripper directly, though, which now after looking further, only do so if the source isn't defined as from a different class. This sort of leaves the question of whether we should make the whole thing more agnostic and treat WarlockRipper just like any other potential class.

Regarding needing login information and such, we might think of adding a GetProperties (or something like that) call to the DataSource class, so each individual source class can define what properties it needs set. Even Warlock could be made to take advantage of this by allowing the license key to be entered from here. Then the UI can have a Source config page that is built from these properties.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238054039, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDOsxCvfdYqQJ1nkgmbCvjK7ewW43ks5qdRbVgaJpZM4JVuyx .

enternoescape commented 7 years ago

@Narflex You spelled it out better. A lot of observations were based on looking at how the API as it currently is, is using WarlockRipper very directly in several places. I was thinking I would need to either create API methods just for Schedules Direct or expand the way EPG implementations are accessed by the API. I think I'm going just add new methods to the API specific to SD.

One of the things I already have added to the API is a method to accept a username and password and if they can get a valid token from SD, then they are saved. The proposal to add parameters would turn this one step into at least a 3 step process since I don't just want you to enter a username and password, then find out later in the wizard when they are needed that they don't work or possibly save a bad username and password and lock your account out. Also I want the username and password to be somewhere other than Sage.properties because that file gets posted sometimes. I would like to return a helpful message if they don't work in case for example SD is not available at the moment so you're not given the impression that you provided the wrong password when that's not the problem.

CraziFuzzy commented 7 years ago

SD will still return different responses for a wrong password than it will for a service being down - that should be able to be handled in the SD class just fine, and can hook into system messages to report an invalid password. I had mentioned the general DataSource config properties as a method of also fixing the issues in WarlockRipper (with key entry) at the same time. We really should have as a standing goal that all configuration is done IN the SageTV UI.

The storage of the key in the .properties file, I agree, is a problem - though that said, the current plugin does so. It would be just as easy for API functioned to be created that generally store data in a private location (be it registry key, environment variable, or a separate file) as it would be to create a one-off solution for Schedules Direct - so I still think the general approach would be more advantageous.

enternoescape commented 7 years ago

I know it has many different messages that it can return. I'm talking about initial configuration. When you're going through the wizard, I would rather get immediate feedback vs. noticing a system message. I already have plans on how to handle problems with the service once you're already using it and utilizing the message system to communicate when there are problems the user should probably know about.

I am usually very much pro-generification, but after working a little with the studio editor, I can already tell this is going to be a fair amount of work and as such I really just want to focus on getting one thing out the door. I get what you're saying and I think I can meet everyone half-way here. I'll put everything in place to migrate WarlockRipper, to a more generic type of access and the Schedules Direct part that I'm working on will already use it, but someone else will need to do it and it will be a completely separate issue.

Narflex commented 7 years ago

I have no problems doing the other updates required for the warlock side of things. :-)

Jeff Kardatzke Sent from my Android

On Aug 7, 2016 4:18 PM, "Joseph Shuttlesworth" notifications@github.com wrote:

I know it has many different messages that it can return. I'm talking about initial configuration. When you're going through the wizard, I would rather get immediate feedback vs. noticing a system message. I already have plans on how to handle problems with the service once you're already using it and utilizing the message system to communicate when there are problems the user should probably know about.

I am usually very much pro-generification, but after working a little with the studio editor, I can already tell this is going to be a fair amount of work and as such I really just want to focus on getting one thing out the door. I get what you're saying and I think I can meet everyone half-way here. I'll put everything in place to migrate WarlockRipper, to a more generic type of access and the Schedules Direct part that I'm working on will already use it, but someone else will need to do it and it will be a completely separate issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238115087, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDMyCQyN9ibNMFXIDggvYoOsZzIQ_ks5qdmfZgaJpZM4JVuyx .

enternoescape commented 7 years ago

I imagine those changes will be far more trivial for you. :)

Narflex commented 7 years ago

Yeah, I still have the entire SageTV code base lodged inside my brain. :-) The STV, not as much, because that became Andy's baby....but I still know much of it intimately from what I wrote before he joined.

Jeff Kardatzke Sent from my Android

On Aug 7, 2016 4:52 PM, "Joseph Shuttlesworth" notifications@github.com wrote:

I imagine those changes will be far more trivial for you. :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238117071, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDL2bdUquinElcyPxv0KKESoTeIcDks5qdm-kgaJpZM4JVuyx .

Narflex commented 7 years ago

FYI, I've made a change to the EPG server so that it'll return a different response if it disconnects someone for an invalid license key. I'd like to use a similar mechanism between both EPG sources for reporting issues and here's my plan, let me know if this goes fine with what you were going to do...or both can be different, fine either way. :)

  1. If this happens as part of a normal EPG update (i.e. not part of setup), then it will post a system message indicating "Invalid EPG License Key", which will have a Red warning level
  2. Update the API calls GetLocalMarketsFromEPGServer and GetLineupsForZipCodeFromEPGServer so they each return an Object rather than String[]; and they'll return "INVALIDKEY" if there's a bad license key or no key at all, and return normally otherwise.

And as you are also in the midst of doing changes to this code...I'd like to avoid introducing conflicts (in the core or the STV); so let me know if you'd rather I wait to do those changes or not.

enternoescape commented 7 years ago

That change is not a problem at all. I didn't know the STV can cast an object. I just assumed that it is mostly just working with primitives and Strings. I'm comfortable enough with git that I can pull in your changes and merge them with my own in the respective classes before I commit. The only thing that would be hard to merge would be the STV itself. I haven't got a very good start in the STV just yet with life getting a little busy suddenly, so that probably wouldn't be a problem anyway.

Narflex commented 7 years ago

And I can help you with any STV merging..it's actually pretty easy. You just autogenerate an STVI based on yours vs. the one in GitHub that you started with. Then import that STVI into the one I end up creating and we should be on the same page...although if we are changing the exact same code, then it may get confused. :)

On Tue, Aug 9, 2016 at 5:20 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

That change is not a problem at all. I didn't know the STV can cast an object. I just assumed that it is mostly just working with primitives and Strings. I'm comfortable enough with git that I can pull in your changes and merge them with my own in the respective classes before I commit. The only thing that would be hard to merge would be the STV itself. I haven't got a very good start in the STV just yet with life getting a little busy suddenly, so that probably wouldn't be a problem anyway.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238732103, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDNRYxpRkfzmMvMxDQyhA-tLCUkKiks5qeRlZgaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 7 years ago

I think I recall doing something similar back in the v6 days when I made my own changes and/or imported community mods, but I didn't explore it nearly as much as I have now. :) There's something more satisfying to me about making a change that will be in there from a default installation.

Narflex commented 7 years ago

And to address your comment about typing in the Studio...the Studio has no real typing, I spent a lot of time making that work. :) Whenever an argument is needed for a normal API call or a Java reflected one; it asks to convert it to the specified type. Generally, this is easy for Strings/primitives and also for many database objects (such as java.io.File->MediaFile or Airing->Show or MediaFile->Airing and every other way that ever came up that has a 1:1 or N:1 mapping).

The trickier part is for reflected functions when they are overloaded...but I won't bore you with all those details, they are in Catbert.java if you're curious. :)

On Tue, Aug 9, 2016 at 5:40 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

I think I recall doing something similar back in the v6 days when I made my own changes and/or imported community mods, but I didn't explore it nearly as much as I have now. :) There's something more satisfying to me about making a change that will be in there from a default installation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238734834, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDI5v0_c_E4Z5SbPp5MLe8vhWvG7Vks5qeR3lgaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

Narflex commented 7 years ago

And apparently the GetLocalMarketsFromEPGServer call is no longer used...I'm not going to remove it from the API, but figured I'd mention it since it surprised me. I guess we made the EPG server aware of broadcast markets by zip code awhile back and that had slipped my mind. :)

On Wed, Aug 10, 2016 at 11:31 AM, Jeffrey Kardatzke jkardatzke@google.com wrote:

And to address your comment about typing in the Studio...the Studio has no real typing, I spent a lot of time making that work. :) Whenever an argument is needed for a normal API call or a Java reflected one; it asks to convert it to the specified type. Generally, this is easy for Strings/primitives and also for many database objects (such as java.io.File->MediaFile or Airing->Show or MediaFile->Airing and every other way that ever came up that has a 1:1 or N:1 mapping).

The trickier part is for reflected functions when they are overloaded...but I won't bore you with all those details, they are in Catbert.java if you're curious. :)

On Tue, Aug 9, 2016 at 5:40 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

I think I recall doing something similar back in the v6 days when I made my own changes and/or imported community mods, but I didn't explore it nearly as much as I have now. :) There's something more satisfying to me about making a change that will be in there from a default installation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-238734834, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDI5v0_c_E4Z5SbPp5MLe8vhWvG7Vks5qeR3lgaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 7 years ago

I noticed. :)

Narflex commented 7 years ago

OK, those changes from me are merged now. Feel free to look them over and let me know if you spot anything wrong...but I tested all the cases so it should be fine.

On Wed, Aug 10, 2016 at 4:29 PM, Joseph Shuttlesworth < notifications@github.com> wrote:

I noticed. :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/sagetv/issues/161#issuecomment-239035544, or mute the thread https://github.com/notifications/unsubscribe-auth/ANEIDOwSPvIidxHwbpeowv5xFVX_Hi7tks5qel69gaJpZM4JVuyx .

Jeffrey Kardatzke jkardatzke@google.com Google, Inc.

enternoescape commented 7 years ago

I like the use of EPGServerException. I was creating a similar exception to generate messages related to problems with Schedules Direct. I can easily align my code with this exception instead for consistency.

enternoescape commented 7 years ago

It's unclear to me how IOUtils.writeStringToFile(f, s) does something different between WriteStringToLocalFile() and WriteStringToFile() in the API. Unless I'm missing a critical detail, they are using the exact same functions, yet one is expected to write to the server's file system and the other is expected to write to the client's file system. I would imagine this would not work well if someone was using the Windows client on another machine.