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

Fix for missing audioitem.linein DIDL data structure #700

Closed
wants to merge 2 commits into from
Closed

Fix for missing audioitem.linein DIDL data structure #700

wants to merge 2 commits into from

Conversation

tagdara
Copy link

@tagdara tagdara commented Jan 12, 2020

I've recently started using the Line-in on my Sonos Connect, and Soco crashes with an unknown data structure type. It claims to be a subset of AudioItem, so I simply copied over that object and tacked on the linein designator.

This is another case where SoCo should probably just roll-up to the parent object when encountering subclasses rather than maintaining a hardline on DIDL compliance since that is clearly not a priority for Sonos (see my previous comments around lack of ProtocolID and other stuff being addressed by quirks).

But since there are fewer examples of the . subclassing than # subclassing and line-in is a common feature across several Sonos devices, it makes sense to target this one directly.

@coveralls
Copy link

coveralls commented Jan 12, 2020

Coverage Status

Coverage increased (+0.08%) to 57.507% when pulling 0b49c01 on tagdara:master into e23a162 on SoCo:master.

@KennethNielsen
Copy link
Member

@tagdera I think maybe we discussed this before. I care mainly about DIDL compliance due to it being easier to maintain, but since these kind of problem are quite a few of the ones we receive I have been toying with the idea and reducing the data_structures implementation to just the ones in the spec and then generating the rest of them on request and flat extensions on their super class.

It takes a little while, because I want to setup tests for all the out of spec classes that I will remove to ensure against regressions. If you want to give it a jab, let me know. Otherwise I will proceed with it.

@KennethNielsen
Copy link
Member

@tagdara I've tried to address this with #713. Please test and review. BTW in our code there was references to .# syntax for subclasses being observed, compared to the normal . syntax, but have you ever seen one where just # was used?

@tagdara
Copy link
Author

tagdara commented Apr 14, 2020

There was at least one case where I saw a # being used by itself from Spotify, but perhaps not in exactly the same way. I have this in my local monkeypatched SoCo that I have been testing before I submitted another PR. I believe it was the Spotify Daily Mixes that were causing this issue for me.

class DidlPlaylistContainerPlaylistItem(DidlPlaylistContainer):

    # Cheese created by Spotify errors on 2/8
    """Class that represents a Sonos tracklist."""
    item_class = 'object.container.playlistContainer#playlistItem'

@KennethNielsen
Copy link
Member

Thanks.

I merged #713 which now hopefully takes care of all these vendor specific classes, so I will close this one. Let me know of you still have issues.

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

3 participants