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

Squeezebox: Keep track of already added players #7149

Merged
merged 1 commit into from
Apr 18, 2017

Conversation

molobrakos
Copy link
Contributor

@molobrakos molobrakos commented Apr 17, 2017

Description:

Keep track of already added players.
When running hass e.g. inside a docker container using the bridge network, a squeezeserver might be discovered multiple times on multiple networks.

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:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link

@molobrakos, thanks for your PR! By analyzing the history of the files in this pull request, we identified @persandstrom, @dasos and @fabaff to be potential reviewers.

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Please look to sonos and use unique_id from entity to handle that.


if players['playerid'] in [x.unique_id
for x in self.hass.data[DATA_SQUEEZEBOX]]:
_LOGGER.debug('Skipping player %s because already seen', players['playerid'])

Choose a reason for hiding this comment

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

line too long (93 > 79 characters)

@@ -108,10 +100,17 @@ def create_players(self):
data = yield from self.async_query('players', 'status')

for players in data['players_loop']:

if players['playerid'] in [x.unique_id
for x in self.hass.data[DATA_SQUEEZEBOX]]:

Choose a reason for hiding this comment

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

line too long (81 > 79 characters)

@molobrakos molobrakos changed the title Use server uuid to differentiate between existing servers. Squeezebox: Keep track of already added players Apr 18, 2017
@molobrakos molobrakos changed the title Squeezebox: Keep track of already added players WIP: Squeezebox: Keep track of already added players Apr 18, 2017
@molobrakos
Copy link
Contributor Author

molobrakos commented Apr 18, 2017

@pvizeli , thanks for pointing that out. I can see that implementing the unique_id property is enough, since at https://github.com/home-assistant/home-assistant/blob/master/homeassistant/helpers/entity_component.py#L179 there is a check to prevent an entity with the same unique_id to be added twice.

Let me know if it still is preferred to locally keep track of already added entities inside the platform (e.g. storing in hass.data), and check before passing to add_devices (the typical use case will only have one server with unique players anyway)

@molobrakos molobrakos changed the title WIP: Squeezebox: Keep track of already added players Squeezebox: Keep track of already added players Apr 18, 2017
@pvizeli pvizeli merged commit 1e758ed into home-assistant:dev Apr 18, 2017
@balloob balloob mentioned this pull request Apr 21, 2017
@molobrakos molobrakos deleted the squeezebox-fix branch June 9, 2017 15:31
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants