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

Spotify albums don't appears with JSON interface #5

Closed
neoflex opened this issue Aug 13, 2011 · 13 comments
Closed

Spotify albums don't appears with JSON interface #5

neoflex opened this issue Aug 13, 2011 · 13 comments

Comments

@neoflex
Copy link
Collaborator

neoflex commented Aug 13, 2011

I tried to fix this but it's not trivial (for me). The JSON methods are based on the use of an integer album id and as spotify albums are not identified by a int id ....
I made a dirty fix modifying also the default web interface but modifying the client is not a good solution because I am still not able to use iphone remote apps for instance.

Do you have any idea how to solve this id problem? Maybe with a way to keep somewhere a matching between a unique integer and the spotify uri.

@akezeke
Copy link
Owner

akezeke commented Aug 13, 2011

I have not even thought about the json and web and all that, I guess that in the future when this is an addon this will be solved by the addon system. But until then if its not that much work we should definitely hack in some support.

I will take a look at it, it might be so that the apps are using the database directly so we need to do some ugly hack in there. If you have time and solves it please let me know. I will focus on getting the thing a little more stable before digging into this.

@neoflex
Copy link
Collaborator Author

neoflex commented Aug 15, 2011

Yes, I got something working for the webinterface by modifying the JSON methods changing the type of the album id from int to string but obviously this is not a very good solution because existing client won't work. The only solution to get it compatible with existing clients that I can see would be to keep on the server side a mapping between spotify ids and an unique int identifier.

@akezeke
Copy link
Owner

akezeke commented Aug 16, 2011

I agree, I should absolutely use a map with integers to put as path nodes instead. That will be much better and solve some other problems as well. I will take a look at this later on, I'm still working on the basics and a windows port is comming soon, but after that :)

@neoflex
Copy link
Collaborator Author

neoflex commented Aug 16, 2011

Great, good luck with that. Tell me if I can help.

@akezeke
Copy link
Owner

akezeke commented Aug 29, 2011

Hi neoflex,

I have decided not to do the mappings now, maybe never so if you could share your hack that enables JSON access to the spotify items I would be very glad.

I don't have the time to work with this right now so any help is appreciated.

Please create a pull request or just e-mail me the code and I´ll add it.

/David

@neoflex
Copy link
Collaborator Author

neoflex commented Aug 29, 2011

Hi David,

Sure, I can send you the code but I don't think you should merge it for everyone because it breaks the compatibility with existing JSON clients. This means that the webinterface will work because I modified it but other client such as an iphone App won't work anymore, even for non spotify albums.
Maybe I can do something to at least make it work for non-spotify albums for unmodified clients, I'll have a look.

How much time do you think it would take to add the integer-spotifyURI mapping?

neoflex

@akezeke
Copy link
Owner

akezeke commented Aug 29, 2011

Hi,

Ok, that is not a good idea to break all other clients I suppose.

The reason I hesitate to create the integer-spotifyURI mapping is because that is not how it will work when this becomes an addon, so the solution is only temporary and needs to be reverted at a later state.

But most code is prepared and all that is left is the actual implementation of the mapping. I guess it could be coded in say 4 hours or something like that, maybe faster. But now I don't have the time, maybe in a couple of weeks.

I will take a look at it and see what I can do when some spare time comes, it sure would be nice to get this feature going!

@neoflex
Copy link
Collaborator Author

neoflex commented Oct 19, 2011

Ok, I finally found some time to redo this normally without breaking the compatibility with other JSON client. I've sent you a PR, let me know what your think ;)

@akezeke
Copy link
Owner

akezeke commented Oct 19, 2011

Great work!

I can browse the albums but no tracks are visible, can you take a look at it? I have very limited time at the moment.

I have merged your changes and I will add you as a contributer so that you can do the checkins directly if you want.

Maybe something broke with the latest upstream merge.

@akezeke akezeke closed this as completed Oct 19, 2011
@akezeke akezeke reopened this Oct 19, 2011
@neoflex
Copy link
Collaborator Author

neoflex commented Oct 20, 2011

I couldn't reproduce the problem. What browser are you using ? just to be sure it's not just a javascript problem.
Thanks for the access to your repository ;)

@akezeke
Copy link
Owner

akezeke commented Oct 20, 2011

I have tried in both firefox and chrome, same behavior, when I click on an album the spinner starts and never stops, no tracks is shown.

I will take a closer look at it at a later time!

@neoflex
Copy link
Collaborator Author

neoflex commented Oct 21, 2011

I've try to push a fix blindly. Not really confident though ;)

@akezeke
Copy link
Owner

akezeke commented Oct 21, 2011

Great!

Now it works :)
Can you please inform the users in the official thread at the xbmc forum of this new feature?

Thank your for your help on this!

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

No branches or pull requests

2 participants