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

Add config entry for Sonos + Cast #14955

Merged
merged 7 commits into from Jun 14, 2018

Conversation

@balloob
Member

balloob commented Jun 13, 2018

Description:

This PR introduces a generic Config Flow for discovered devices that do not require authentication. For example Wemo, Sonos or Cast.

The generic discovery config flow has the following characteristics:

  • Once discovered, will need to be confirmed by the user.
  • If initiated by the user, will create immediately once it can find devices
  • If a second discovery comes in, it will abort.
  • If a flow is discovered and then user initiates one, we finish immediately and abort the discovered flow
  • Only one config entry instance is allowed

To show that the generic config flow works, I've added an implementation for Sonos and Cast. I've made sure to make as little changes to the Sonos and Cast platforms and keep it backwards compatible. However, I did deprecate the config per platform and have that be moved to the component.

I'm planning to add this to Wemo next.

Breaking change: Configuring Sonos via media player platform config has been moved to the Sonos component.

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):

sonos:
  media_player:
    your_old_config: bla

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
"""Component to embed Sonos."""
import asyncio
import json
import os

This comment has been minimized.

@houndci-bot

houndci-bot Jun 13, 2018

'os' imported but unused

@@ -0,0 +1,33 @@
"""Component to embed Sonos."""
import asyncio
import json

This comment has been minimized.

@houndci-bot

houndci-bot Jun 13, 2018

'json' imported but unused

@@ -0,0 +1,33 @@
"""Component to embed Sonos."""
import asyncio

This comment has been minimized.

@houndci-bot

houndci-bot Jun 13, 2018

'asyncio' imported but unused

"""Set up Sonos from a config entry."""
def add_devices(devices, update_before_add=False):
"""Sync version of async add devices."""
run_callback_threadsafe(

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 13, 2018

Member

Why do we not use hass.add_job here?

@balloob balloob changed the title from Add config entry for Sonos to Add config entry for Sonos + Cast Jun 13, 2018

@@ -17,6 +17,7 @@
from homeassistant.core import callback
from homeassistant.helpers.dispatcher import (dispatcher_send,
async_dispatcher_connect)
from homeassistant.components.cast import DOMAIN

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 13, 2018

Member

We should not overwrite the media_player domain with the cast domain. We can import it like so:

from homeassistant.components.cast import DOMAIN as CAST_DOMAIN

This comment has been minimized.

@balloob

balloob Jun 14, 2018

Member

We're not importing the media player domain? The DOMAIN constant is only something that is mandatory for components. There is no such requirement for platforms.

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 14, 2018

Member

The cast media_player platform belongs to the media_player domain. Importing the cast domain under the same name is confusing and not correct.

This comment has been minimized.

@balloob

balloob Jun 14, 2018

Member

fixed

@balloob balloob merged commit 2c6e6c2 into dev Jun 14, 2018

6 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.006%) to 94.003%
Details

@wafflebot wafflebot bot removed the in progress label Jun 14, 2018

@balloob balloob deleted the config-entry-sonos branch Jun 14, 2018

@balloob balloob referenced this pull request Jun 22, 2018

Merged

0.72 #15088

MizterB added a commit to MizterB/home-assistant that referenced this pull request Aug 11, 2018

Add config entry for Sonos + Cast (home-assistant#14955)
* Add config entry for Sonos

* Lint

* Use add_job

* Add Cast config entry

* Lint

* Rename DOMAIN import

* Mock pychromecast in test

cyberjacob pushed a commit to cyberjacob/home-assistant that referenced this pull request Sep 4, 2018

Add config entry for Sonos + Cast (home-assistant#14955)
* Add config entry for Sonos

* Lint

* Use add_job

* Add Cast config entry

* Lint

* Rename DOMAIN import

* Mock pychromecast in test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment