Skip to content

Added unique_id property back to Yamaha media_player #12373

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.

Sign up for GitHub

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

Closed
Petro31 wants to merge 1 commit into from
+5 −0
Closed

Added unique_id property back to Yamaha media_player #12373

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions homeassistant/components/media_player/yamaha.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ def __init__(self, name, receiver, source_ignore,
self._name = name
self._zone = receiver.zone

@property
def unique_id(self):
"""Return an unique ID."""
return '{0}:{1}'.format(self.receiver.ctrl_url, self._zone)

Copy link
Copy Markdown

@houndci-bot houndci-bot Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line contains whitespace

Copy link
Copy Markdown
Contributor

@bachya bachya Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is self._zone a user-configurable string (i.e., something that could change through, say, the Yamaha app)? The objective is to not have anything user-configurable in the unique ID that could change down the road while still representing the same thing.

Copy link
Copy Markdown
Contributor Author

@Petro31 Petro31 Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not appear to, it appears as if it comes from rxv.RXV() call which returns a receiver object. After tracing through the rxv code, it appears to grab the self._zone from the xml that is requested from the device. When I get home, I can verify this.

Copy link
Copy Markdown
Contributor Author

@Petro31 Petro31 Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to check the yamaha app as well.

Copy link
Copy Markdown
Contributor

@emlove emlove Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the reason it was removed was that it's based on the URL. A unique ID needs to be based on something that won't change for the device, such as a serial number or mac address. I'd see if something like that is available from the API that could be used instead.

Copy link
Copy Markdown
Contributor Author

@Petro31 Petro31 Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. For time constraints, I only 'rolled back' the changes. When i get home I can find a better way to handle the unique_id. I may have been too hasty with this PR.

Copy link
Copy Markdown
Contributor

@sdague sdague Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self._zone is not user configurable, it comes from the device.

As far as I can see in both the rxv code, and poking at the XML API for the Yamaha unit, there is nothing available at the rxv layer for uniqueness. That's all operating at the url level (which there was already code to ignore by url devices previously discovered).

The only existing path I'm seeing to get an actual unique id is data that's available in the upnp discovery packet. We don't require everyone to go through that path, so that wouldn't work if discovery was disabled. I don't see any imperative path to get that.

Newer receivers have a more REST like API, but for those of us with older ones losing multizone because there is not a guarunteed way to generate stable uuid would be really sad.

Copy link
Copy Markdown
Contributor

@emlove emlove Feb 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd try and investigate the underlying bug. Not having a unique ID shouldn't affect the behavior of the actual platform code at all.

def update(self):
"""Get the latest details from the device."""
self._play_status = self.receiver.play_status()
Expand Down