-
Notifications
You must be signed in to change notification settings - Fork 256
Add support for free radio stations #552
Comments
This sounds like it should be a separate method from You say this is for getting a stream URL from a radio station using a free account. Are you sure this is only for free accounts and not just a way to get a stream URL for station tracks in general? Either way, it would be better to name the call and the protocol class appropriately to better signify what it does. You didn't add I'm curious as to why a change in the Unfortunately, my phone is on Nougat right now, so I can't investigate the call myself at the moment : ( Looking forward to hearing more about this call, though. |
Thanks! I've been super busy lately, but I hope to have a chance to look at this over the weekend. |
I have commented out the check because the track_id starting with 'T' can be returned to free accounts in this case.
They come in the station dict, example code:
The mobile client uses the same 'radio/editstation' call used by create_station for free radios, I just wanted to use the same call. So, you think creating a new get_stream_url_free and get_free_station is the better aproach? |
I understood why you commented out the check. My point was that having to do that is a clue that it should probably be a separate method. Another is that you had to create a separate class in the protocol layer for it. I think the main reason you're re-using The thing is, if this is for streaming from a station, station should be in the name. As much as I can suss out right now, probably the better naming is
But do you need to change anything there to use it? You don't actually change anything but some parameter values and returning the full station dict instead of just the ID. Can you not get the station tracks by doing as I said before: using Simon will give his opinion after he looks it over. |
Just tested this myself. It is only present for free accounts.
You can, in fact, get the station tracks by the method I've described. But, you never clearly said why you changed it. It is so you can get the session token, right? A better way to do this might be to add a
I'm pretty sure these should be |
Looking forward to this inclusion, thanks a lot! 👍 |
Nice work figuring this out! I'm +1 to thebigmunch's suggested changes. Listing them out, I think it's:
Want to open a PR with the changes? |
A note: schemas should probably be updated even though there are no exceptions due to #492. There will have to be a station track schema that can copy the track schema and adds The I could probably bang this out sometime this week if you're not comfortable with the changes needed. |
@foreverguest are you ok with @thebigmunch implementing the changes? |
Sure, I would give a shot working on the changes, but I'm a little busy at the moment. |
@thebigmunch are you able to proceed with the modifications? |
If anyone is able to proceed with the modifications it would be appreciated! 😸 |
Sorry, @NonaSuomy, I was so busy last week I forgot to respond. I definitely didn't have the time : P I'm not sure when I'll have some extra time again to do it either. |
Ok, did the changes you asked: |
Just from a cursory glance, you don't need With that there needs to be a note in the docstring that the method is for free accounts only and point subscribed users to the And the new methods need to be added to the mobile client docs index. And some tests need to be added. I'm on my phone, so I haven't looked at the info method yet. |
Cool, this is looking pretty good. Thanks! I agree there's a few odds and ends to clean up, but it's probably a good time to open a PR with the existing changes. We could either discuss the suggestions there or decide to merge the meat of it and fix up the rest later (which I've got time for this weekend). |
Probably I'm missing something, I tried a version replacing |
Hmm, I assumed that the song ID wouldn't be needed for the call on Google's end. Seems odd but wouldn't be surprised. I tried using your changes as-is on my free account, but I kept getting a 403. May just be too early in the morning. Looking forward to testing it out more later. I'd still move |
From playing around quickly, I didn't notice I wasn't getting a redirect from this call like the other stream calls. We instead getting back a JSON response with a |
For the curious, the keys I'm seeing in the response are:
Some of these make me wonder if the other stream calls might be changed in the future. But it could also just be Google's servers being stupid. Or it could just be Google being inconsistent again. |
Well, they did tack on the Songza service they bought to their music service, so likely inconsistency between the two. |
Free account station streaming has always been there. The Songza stuff is what are called Situations in Google Music. Situations were implemented as stations, but they don't have any relation to this. Edit: This response almost seems more like an internal thing to me. |
Hopefully, it gets figured out! |
@foreverguest are you able to submit the Pull Request as @simon-weber was requesting => |
Thanks; I cleaned up a few things and merged it! I'd still like to add tests before cutting a release with this in it, but the folks waiting on support can go ahead and try integrating off the develop branch. |
@foreverguest and @simon-weber keep up the great work! Please post back when it is in release as well. Thank you! |
@simon-weber is this in the release now? |
Yup, in 11.0.0. Sorry that I forgot to clean this up. |
Thank you! |
Hi,
I have added support for free radios on the Kodi addon adding the missing calls in gmusicapi and changing others, please check fork below:
develop...foreverguest:develop
But this changes the data returned by create_station function, is that ok?
Do you suggest another approach?
The text was updated successfully, but these errors were encountered: