Skip to content

media_player.squeezebox: Do not fail in case no players are connected, in which case squeezeserver will return a result without player_loop.#7617

Merged
balloob merged 1 commit into
devfrom
molobrakos-patch-1
Jun 2, 2017
Merged

media_player.squeezebox: Do not fail in case no players are connected, in which case squeezeserver will return a result without player_loop.#7617
balloob merged 1 commit into
devfrom
molobrakos-patch-1

Conversation

@molobrakos
Copy link
Copy Markdown
Contributor

Do not fail in case no players are connected, in which case squeezeserver will return a result without player_loop.

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:

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.

Do not fail in case no players are connected, in which case squeezeserver will return a result without player_loop.
@mention-bot
Copy link
Copy Markdown

@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.

@molobrakos
Copy link
Copy Markdown
Contributor Author

Stack trace for reference:

2017-05-15 11:23:31 ERROR (MainThread) [homeassistant.components.media_player] Error while setting up platform squeezebox
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/homeassistant/helpers/entity_component.py", line 150, in _async_setup_platform
    entity_platform.async_schedule_add_entities, discovery_info                                                                                                             File "/usr/local/lib/python3.6/site-packages/homeassistant/components/media_player/squeezebox.py", line 75, in async_setup_platform
    players = yield from lms.create_players()
  File "/usr/local/lib/python3.6/site-packages/homeassistant/components/media_player/squeezebox.py", line 98, in create_players
    for players in data['players_loop']:
KeyError: 'players_loop'

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented May 17, 2017

Can you also add the available property to entity?

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented May 29, 2017

Ping

@molobrakos
Copy link
Copy Markdown
Contributor Author

I guess that should be possible ... maybe for another pr later?

@balloob balloob added this to the 0.47 milestone Jun 2, 2017
@balloob
Copy link
Copy Markdown
Member

balloob commented Jun 2, 2017

I'm fine with that for a future PR.

@balloob balloob merged commit e11ec88 into dev Jun 2, 2017
@balloob balloob deleted the molobrakos-patch-1 branch June 2, 2017 07:26
@bdurrer
Copy link
Copy Markdown
Contributor

bdurrer commented Jun 2, 2017

this solves issue #6569 (but not the flaw that the component only looks for players exactly once at startup)

@balloob balloob modified the milestones: 0.46, 0.47 Jun 3, 2017
@molobrakos
Copy link
Copy Markdown
Contributor Author

I agree it would be nice if the component checked for players more than once at startup. And also provided the available property as (suggested above).

@molobrakos
Copy link
Copy Markdown
Contributor Author

I haven't tested this (not at home), but the periodically checking of newly connected players might already work because of netdisco/discovery sending out discovery requests periodically?

@dasos
Copy link
Copy Markdown
Contributor

dasos commented Jun 5, 2017 via email

@balloob balloob mentioned this pull request Jun 16, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 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.

7 participants