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

Adds SoCo.get_sonos_playlists and MLSonosPlaylist #114

Merged
merged 3 commits into from May 25, 2014

Conversation

Projects
None yet
4 participants
@petteraas
Copy link
Member

petteraas commented Apr 28, 2014

based on comments in #113

to test it

lists = sonos.get_sonos_playlists()
print("search_type: %s number_retured: %s update_id: %s total_matches: %d" % (
 lists.get('search_type'),
 lists.get('number_retured'),
 lists.get('update_id'),
 lists.get('total_matches')))

for idx, playlist in enumerate(lists.get('item_list'), 1):
    print("idx: %d, playlist: %s" % (idx, playlist))

I'm not sure yet why the unit-test is failing.

def test_mlsonosplaylist():
"""Test the MLSonosPlaylist class"""
# Set the tests up
uri = 'file:///jffs/settings/savedqueues.rsq#13 title: Koop'

This comment has been minimized.

Copy link
@petteraas

petteraas Apr 28, 2014

Author Member

I got this from

print("uri: %s title: %s" % (uri, title))

in MLSonsPlayList, but as the test is failing, I guess I've misunderstood some step here!

@petteraas

This comment has been minimized.

Copy link
Member Author

petteraas commented Apr 28, 2014

A starting point for use in socos https://github.com/petteraas/socos/tree/get_sonos_playlists

My thinking is that we can have a 'list_sonos_playlists' and something like 'add_sonos_playlist %d' which adds the specified to current queue based on index from 'list_sonos_playlists'. A bit like how 'list' and 'set' works today, but this is as you can see far from finished yet.

@KennethNielsen

This comment has been minimized.

Copy link
Member

KennethNielsen commented May 2, 2014

Hi @petteraas. I'd like to review this but I have been a little busy lately. I'll see if I can get around to it this weekend.

@petteraas

This comment has been minimized.

Copy link
Member Author

petteraas commented May 2, 2014

@KennethNielsen no worries, do it when you find the time!

After seeing your SoCo/socos#26 I will probably not continue on https://github.com/petteraas/socos/tree/get_sonos_playlists , but instead incorporate it in that new MusicLibrary class.

@KennethNielsen

This comment has been minimized.

Copy link
Member

KennethNielsen commented May 4, 2014

Hi @petteraas

So I found some time to look at this. I assume really, that we should be able to add these Sonos playlists to the queue with the normal add_to_queue method, provided we can get the MLSonosPlaylist to generate its own meta_data.

Currently this fails like so:

>>> soco_instance.add_to_queue(sonos_playlist)
 Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
   File "soco/core.py", line 1082, in add_to_queue
     raise AttributeError(message)
 AttributeError: queueable_item has no attribute didl_metadata

which is really an error on my part in the add_to_queue method. Because if we try:

 >>> sonos_playlist.didl_metadata
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "soco/data_structures.py", line 254, in didl_metadata
    'DIDL Metadata cannot be created when item_id returns None '
soco.exceptions.CannotCreateDIDLMetadata: DIDL Metadata cannot be created when item_id returns None (most likely because uri is not set)

we can see that it does indeed have a didl_metadata property, but it fails to generate the metadata, which is why add_to_queue reports it as not having any. We should fix that.

In any case it also gives as a clue as to why it fails. It fails because item_id returns None. The id is used in the metadata and is, for all music library items, extracted from the URI that it gets from the XML when we fetch the items.

From MusicLibraryItem we can see that:

    @property
    def item_id(self):  # pylint: disable=C0103
        """Return the id.

        The id is extracted as the part of the URI after the first # character.
        For the few music library types where that is not correct, this method
        should be overwritten.
        """
        out = self.content['uri']
        try:
            out = out[out.index('#') + 1:]
        except ValueError:
            out = None
        return out

and I can see that you have indeed overwritten it, with what appears to be a copy of the way it is overwritten in some of the other music library items:

    @property
    def item_id(self): #pylint: disable=C0103
        """ Returns the id. """
        out = self.content['uri']
        if 'x-file-cids' in out:
            out = out.replace('x-file-cifs', 'S')
        else:
            out = None
        return out

and that returns None because there is no 'x-file-cids' in the uri: 'file:///jffs/settings/savedqueues.rsq#0', so what you need to do is to figure out how to generate the ID from the URI (for which you might need wireshark) or you could simple guess. It might not need to be overwritten at all, it may be that the ID for the Sonos queue quite simply is a 0, which we would get with the item_id method from MusicLibraryItem. Looking at the raw XML that we get from the search however:

 <ns0:container xmlns:dc="http://purl.org/dc/elements/1.1/"
    xmlns:ns0="urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/"
    xmlns:ns2="urn:schemas-upnp-org:metadata-1-0/upnp/"
    id="SQ:0" parentID="SQ:" restricted="true">
  <dc:title>Test</dc:title>
  <ns0:res protocolInfo="file:*:audio/mpegurl:*">file:///jffs/settings/savedqueues.rsq#0</ns0:res>
  <ns2:class>object.container.playlistContainer</ns2:class>
 </ns0:container>

it appears that for this type of item, there is actually an ID property with the value "SQ:0" so it appears that the ID should be created by adding 'SQ:' with the number after # in the URI.

Lets try and get this working first and then we can look at the tests.

@KennethNielsen

This comment has been minimized.

Copy link
Member

KennethNielsen commented May 4, 2014

Ok, so this isn't very nice now that I put it all up there for you to try, but I couldn't help myself from trying. It appears that if you overwrite with:

    @property
    def item_id(self): #pylint: disable=C0103
        """ Returns the id. """
        queue_number = super(MLSonosPlaylist, self).item_id
        return 'SQ:' + queue_number

then add_to_queue on the object works.

I should say, that I can see now, that there, dispite my best efforts, might be some holes in the documentation. I would appreciate your input afterwards on where I might improve it.

@petteraas

This comment has been minimized.

Copy link
Member Author

petteraas commented May 4, 2014

@KennethNielsen thanx for the input!

I tested again now, both your version of item_id and not overwriting at all seems to work. For now I'll just remove my local version of item_id, as the version in MusicLibrary appears to be enough.

@petteraas

This comment has been minimized.

Copy link
Member Author

petteraas commented May 4, 2014

now this works.

response = sonos.get_sonos_playlists()

for idx, playlist in enumerate(response.get('item_list'), 1):
    print("idx: %d, playlist: %s" % (idx, playlist))
    sonos.add_to_queue(playlist)
@stefankoegl

This comment has been minimized.

Copy link
Member

stefankoegl commented May 22, 2014

Looks good :)

@KennethNielsen

This comment has been minimized.

Copy link
Member

KennethNielsen commented May 22, 2014

Should we have a look at the tests as well? I know the data structure tests are not the most well written tests, so I don't mind lending a hand if you still have problems.

@DPH

This comment has been minimized.

Copy link
Member

DPH commented May 22, 2014

+1 works well

@petteraas

This comment has been minimized.

Copy link
Member Author

petteraas commented May 22, 2014

@KennethNielsen was there a specific thing you thought about regarding the tests? As they are working now!

ps: squashed it into fewer commits.

@KennethNielsen

This comment has been minimized.

Copy link
Member

KennethNielsen commented May 23, 2014

Ahh, no. I just wasn't aware that the tests worked now.

@petteraas

This comment has been minimized.

Copy link
Member Author

petteraas commented May 23, 2014

@KennethNielsen ok! Any other input, or can it be merged as is now?

@stefankoegl

This comment has been minimized.

Copy link
Member

stefankoegl commented May 23, 2014

+1

@petteraas petteraas referenced this pull request May 24, 2014

Closed

Release 0.8 #143

@stefankoegl

This comment has been minimized.

Copy link
Member

stefankoegl commented May 25, 2014

Merged!

stefankoegl added a commit that referenced this pull request May 25, 2014

Merge pull request #114 from petteraas/get_sonos_playlists
Adds SoCo.get_sonos_playlists and MLSonosPlaylist

@stefankoegl stefankoegl merged commit 47b81bb into SoCo:master May 25, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@KennethNielsen KennethNielsen added this to the 0.8 milestone May 25, 2014

@KennethNielsen

This comment has been minimized.

Copy link
Member

KennethNielsen commented May 25, 2014

I've added the 0.8 milestone to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.