Fix snapcast uuid to be more unique #14925
Conversation
homeassistant
added
cla-signed
platform: media_player.snapcast
small-pr
ghost
added
|
|
||
| @property | ||
| def name(self): | ||
| """Return the name of the device.""" | ||
| return '{}{}'.format(CLIENT_PREFIX, self._client.identifier) | ||
| return '{}{}_{}'.format(CLIENT_PREFIX, self._client._server._host, self._client.identifier) |
There was a problem hiding this comment.
line too long (99 > 79 characters)
| """Return the ID of this snapcast client. | ||
| Note: Host part is needed, when using multiple snapservers | ||
| """ | ||
| return '{}{}_{}'.format(CLIENT_PREFIX, self._client._server._host, self._client.identifier) |
There was a problem hiding this comment.
line too long (99 > 79 characters)
|
|
||
| @property | ||
| def name(self): | ||
| """Return the name of the device.""" | ||
| return '{}{}'.format(GROUP_PREFIX, self._group.identifier) | ||
| return '{}{}_{}'.format(GROUP_PREFIX, self._group._server._host, self._group.identifier) |
There was a problem hiding this comment.
line too long (96 > 79 characters)
| @@ -107,12 +107,12 @@ def state(self): | |||
| @property | |||
| def unique_id(self): | |||
| """Return the ID of snapcast group.""" | |||
| return '{}{}'.format(GROUP_PREFIX, self._group.identifier) | |||
| return '{}{}_{}'.format(GROUP_PREFIX, self._group._server._host, self._group.identifier) | |||
There was a problem hiding this comment.
line too long (96 > 79 characters)
|
|
||
| @property | ||
| def name(self): | ||
| """Return the name of the device.""" | ||
| return '{}{}'.format(GROUP_PREFIX, self._group.identifier) | ||
| return '{}{}_{}'.format(GROUP_PREFIX, self._group._server._host, |
There was a problem hiding this comment.
Changing the name will change the entity_id, so that will make this a breaking change.
With the unique id set, the entity_id doesn't need to be changed. Both name and entity_id can be customized by the user.
There was a problem hiding this comment.
Hmm, I see. Reason for changing it was the same name twice in the tab.
So should I keep the previous name and make renaming inside the registry file?
There was a problem hiding this comment.
That's my suggestion. I'm guessing not all users will need to change this if they don't have multiple hosts. So it's better to not make a breaking change and still allow impacted users to set their names and IDs as they want.
Current uuid is ok when using only 1 snapserver New uuid is needed when using multiple snapserver Because the client can connect to more snapservers and then uuid based on client MAC is not enough
|
I must changed the way of getting host, port because travis does not like the access of "private" members |
|
@MartinHjelmare Isn't this a breaking change? When the unique ID changes, the entity registry will generate a new entry for it and thus a new entity ID. (side note: changing the name of an entity that implements unique_id is AFAIK not a breaking change, as the entity registry keeps entity IDs with the same unique ID static) |
|
That's true. But since the unique_id was added five days ago it hasn't been released yet, so it's not a breaking change and my statements above was based on that. |
Current uuid is ok when using only 1 snapserver New uuid is needed when using multiple snapserver Because the client can connect to more snapservers and then uuid based on client MAC is not enough
Current uuid is ok when using only 1 snapserver
New uuid is needed when using multiple snapserver
Because the client can connect to more snapservers and
then uuid based on client MAC is not enough
Description:
Related issue (if applicable): fixes #
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices: