From 463d949ee013b5285f96adda3f0b4acfc968c02b Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Wed, 22 Jan 2020 04:13:35 -0500 Subject: [PATCH] Add zeroconf discovery support for vizio integration (#30949) * add missing tests * readd removed test * add zeroconf discovery support for vizio integration * no mock_coro_func needed * add reasonable timeout and don't log exceptions from pyvizio due to timeout * add test to test options update and bump pyvizio to avoid timeout issues * update requirements_* * fix gaps in coverage * change return hint for async_setup_entry * use source variables instead of strings * only get unique ID if about to create entry * update based on review * Revert "update based on review" This reverts commit 0d612a90eb7d02c92061f902973e527267e3110a. * f-string * fix last review * revert cleanup changes to simplify PR * remove unnecessary ConfigFlow object variables to simplify logic * revert cleanup changes to make review easier, noted for future cleanup * revert cleanup changes to make review easier, noted for future cleanup * move zeroconf service type constant to test module --- homeassistant/components/vizio/config_flow.py | 70 +++++++++++---- homeassistant/components/vizio/const.py | 1 + homeassistant/components/vizio/manifest.json | 5 +- .../components/vizio/media_player.py | 4 +- homeassistant/components/vizio/strings.json | 3 +- homeassistant/generated/zeroconf.py | 3 + requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/vizio/test_config_flow.py | 85 +++++++++++++++++-- 9 files changed, 144 insertions(+), 31 deletions(-) diff --git a/homeassistant/components/vizio/config_flow.py b/homeassistant/components/vizio/config_flow.py index 560b01df83a..a02ae22a46b 100644 --- a/homeassistant/components/vizio/config_flow.py +++ b/homeassistant/components/vizio/config_flow.py @@ -2,7 +2,7 @@ import logging from typing import Any, Dict -from pyvizio import VizioAsync +from pyvizio import VizioAsync, async_guess_device_type import voluptuous as vol from homeassistant import config_entries @@ -13,6 +13,8 @@ from homeassistant.const import ( CONF_DEVICE_CLASS, CONF_HOST, CONF_NAME, + CONF_PORT, + CONF_TYPE, ) from homeassistant.core import callback @@ -64,6 +66,7 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Initialize config flow.""" self.import_schema = None self.user_schema = None + self._must_show_form = None async def async_step_user( self, user_input: Dict[str, Any] = None @@ -101,24 +104,31 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): errors["base"] = "tv_needs_token" if not errors: - unique_id = await VizioAsync.get_unique_id( - user_input[CONF_HOST], - user_input.get(CONF_ACCESS_TOKEN), - user_input[CONF_DEVICE_CLASS], - ) - - # Abort flow if existing component with same unique ID matches new config entry - if await self.async_set_unique_id( - unique_id=unique_id, raise_on_progress=True - ): - return self.async_abort( - reason="already_setup_with_diff_host_and_name" + # Skip validating config and creating entry if form must be shown + if self._must_show_form: + self._must_show_form = False + else: + # Abort flow if existing entry with same unique ID matches new config entry. + # Since name and host check have already passed, if an entry already exists, + # It is likely a reconfigured device. + unique_id = await VizioAsync.get_unique_id( + user_input[CONF_HOST], + user_input.get(CONF_ACCESS_TOKEN), + user_input[CONF_DEVICE_CLASS], ) - return self.async_create_entry( - title=user_input[CONF_NAME], data=user_input - ) + if await self.async_set_unique_id( + unique_id=unique_id, raise_on_progress=True + ): + return self.async_abort( + reason="already_setup_with_diff_host_and_name" + ) + return self.async_create_entry( + title=user_input[CONF_NAME], data=user_input + ) + + # Use user_input params as default values for schema if user_input is non-empty, otherwise use default schema schema = self.user_schema or self.import_schema or _config_flow_schema({}) return self.async_show_form(step_id="user", data_schema=schema, errors=errors) @@ -153,6 +163,34 @@ class VizioConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return await self.async_step_user(user_input=import_config) + async def async_step_zeroconf( + self, discovery_info: Dict[str, Any] = None + ) -> Dict[str, Any]: + """Handle zeroconf discovery.""" + + discovery_info[ + CONF_HOST + ] = f"{discovery_info[CONF_HOST]}:{discovery_info[CONF_PORT]}" + + # Check if new config entry matches any existing config entries and abort if so + for entry in self.hass.config_entries.async_entries(DOMAIN): + if entry.data[CONF_HOST] == discovery_info[CONF_HOST]: + return self.async_abort(reason="already_setup") + + # Set default name to discovered device name by stripping zeroconf service + # (`type`) from `name` + num_chars_to_strip = len(discovery_info[CONF_TYPE]) + 1 + discovery_info[CONF_NAME] = discovery_info[CONF_NAME][:-num_chars_to_strip] + + discovery_info[CONF_DEVICE_CLASS] = await async_guess_device_type( + discovery_info[CONF_HOST] + ) + + # Form must be shown after discovery so user can confirm/update configuration before ConfigEntry creation. + self._must_show_form = True + + return await self.async_step_user(user_input=discovery_info) + class VizioOptionsConfigFlow(config_entries.OptionsFlow): """Handle Transmission client options.""" diff --git a/homeassistant/components/vizio/const.py b/homeassistant/components/vizio/const.py index 0345b0c9992..92fb37c153e 100644 --- a/homeassistant/components/vizio/const.py +++ b/homeassistant/components/vizio/const.py @@ -30,6 +30,7 @@ CONF_VOLUME_STEP = "volume_step" DEFAULT_DEVICE_CLASS = DEVICE_CLASS_TV DEFAULT_NAME = "Vizio SmartCast" +DEFAULT_TIMEOUT = 8 DEFAULT_VOLUME_STEP = 1 DEVICE_ID = "pyvizio" diff --git a/homeassistant/components/vizio/manifest.json b/homeassistant/components/vizio/manifest.json index b5ea057a33d..ea1162540cf 100644 --- a/homeassistant/components/vizio/manifest.json +++ b/homeassistant/components/vizio/manifest.json @@ -2,8 +2,9 @@ "domain": "vizio", "name": "Vizio SmartCast TV", "documentation": "https://www.home-assistant.io/integrations/vizio", - "requirements": ["pyvizio==0.1.1"], + "requirements": ["pyvizio==0.1.4"], "dependencies": [], "codeowners": ["@raman325"], - "config_flow": true + "config_flow": true, + "zeroconf": ["_viziocast._tcp.local."] } diff --git a/homeassistant/components/vizio/media_player.py b/homeassistant/components/vizio/media_player.py index 76bb476317e..d143c4232bf 100644 --- a/homeassistant/components/vizio/media_player.py +++ b/homeassistant/components/vizio/media_player.py @@ -26,6 +26,7 @@ from homeassistant.helpers.typing import HomeAssistantType from .const import ( CONF_VOLUME_STEP, + DEFAULT_TIMEOUT, DEFAULT_VOLUME_STEP, DEVICE_ID, DOMAIN, @@ -46,7 +47,7 @@ async def async_setup_entry( hass: HomeAssistantType, config_entry: ConfigEntry, async_add_entities: Callable[[List[Entity], bool], None], -) -> bool: +) -> None: """Set up a Vizio media player entry.""" host = config_entry.data[CONF_HOST] token = config_entry.data.get(CONF_ACCESS_TOKEN) @@ -69,6 +70,7 @@ async def async_setup_entry( token, VIZIO_DEVICE_CLASSES[device_class], session=async_get_clientsession(hass, False), + timeout=DEFAULT_TIMEOUT, ) if not await device.can_connect(): diff --git a/homeassistant/components/vizio/strings.json b/homeassistant/components/vizio/strings.json index a6367cb3c8f..305e49d56f8 100644 --- a/homeassistant/components/vizio/strings.json +++ b/homeassistant/components/vizio/strings.json @@ -30,8 +30,7 @@ "init": { "title": "Update Vizo SmartCast Options", "data": { - "volume_step": "Volume Step Size", - "timeout": "API Request Timeout (seconds)" + "volume_step": "Volume Step Size" } } } diff --git a/homeassistant/generated/zeroconf.py b/homeassistant/generated/zeroconf.py index eceb2ee3fd5..9af4bde8a2a 100644 --- a/homeassistant/generated/zeroconf.py +++ b/homeassistant/generated/zeroconf.py @@ -24,6 +24,9 @@ ZEROCONF = { "_hap._tcp.local.": [ "homekit_controller" ], + "_viziocast._tcp.local.": [ + "vizio" + ], "_wled._tcp.local.": [ "wled" ] diff --git a/requirements_all.txt b/requirements_all.txt index f0e1848c76e..ef2c85373e6 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1690,7 +1690,7 @@ pyversasense==0.0.6 pyvesync==1.1.0 # homeassistant.components.vizio -pyvizio==0.1.1 +pyvizio==0.1.4 # homeassistant.components.velux pyvlx==0.2.12 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 594e8622e70..7ed5c31ca49 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -558,7 +558,7 @@ pyvera==0.3.7 pyvesync==1.1.0 # homeassistant.components.vizio -pyvizio==0.1.1 +pyvizio==0.1.4 # homeassistant.components.html5 pywebpush==1.9.2 diff --git a/tests/components/vizio/test_config_flow.py b/tests/components/vizio/test_config_flow.py index 4dbb375c3fe..196ef35469d 100644 --- a/tests/components/vizio/test_config_flow.py +++ b/tests/components/vizio/test_config_flow.py @@ -14,12 +14,14 @@ from homeassistant.components.vizio.const import ( DOMAIN, VIZIO_SCHEMA, ) -from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER +from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER, SOURCE_ZEROCONF from homeassistant.const import ( CONF_ACCESS_TOKEN, CONF_DEVICE_CLASS, CONF_HOST, CONF_NAME, + CONF_PORT, + CONF_TYPE, ) from homeassistant.helpers.typing import HomeAssistantType @@ -62,6 +64,19 @@ MOCK_SPEAKER_CONFIG = { CONF_DEVICE_CLASS: DEVICE_CLASS_SPEAKER, } +VIZIO_ZEROCONF_SERVICE_TYPE = "_viziocast._tcp.local." +ZEROCONF_NAME = f"{NAME}.{VIZIO_ZEROCONF_SERVICE_TYPE}" +ZEROCONF_HOST = HOST.split(":")[0] +ZEROCONF_PORT = HOST.split(":")[1] + +MOCK_ZEROCONF_ENTRY = { + CONF_TYPE: VIZIO_ZEROCONF_SERVICE_TYPE, + CONF_NAME: ZEROCONF_NAME, + CONF_HOST: ZEROCONF_HOST, + CONF_PORT: ZEROCONF_PORT, + "properties": {"name": "SB4031-D5"}, +} + @pytest.fixture(name="vizio_connect") def vizio_connect_fixture(): @@ -93,6 +108,16 @@ def vizio_bypass_update_fixture(): yield +@pytest.fixture(name="vizio_guess_device_type") +def vizio_guess_device_type_fixture(): + """Mock vizio async_guess_device_type function.""" + with patch( + "homeassistant.components.vizio.config_flow.async_guess_device_type", + return_value="speaker", + ): + yield + + @pytest.fixture(name="vizio_cant_connect") def vizio_cant_connect_fixture(): """Mock vizio device cant connect.""" @@ -175,7 +200,7 @@ async def test_options_flow(hass: HomeAssistantType) -> None: assert result["step_id"] == "init" result = await hass.config_entries.options.async_configure( - result["flow_id"], user_input={CONF_VOLUME_STEP: VOLUME_STEP}, + result["flow_id"], user_input={CONF_VOLUME_STEP: VOLUME_STEP} ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY @@ -188,9 +213,7 @@ async def test_user_host_already_configured( ) -> None: """Test host is already configured during user setup.""" entry = MockConfigEntry( - domain=DOMAIN, - data=MOCK_SPEAKER_CONFIG, - options={CONF_VOLUME_STEP: VOLUME_STEP}, + domain=DOMAIN, data=MOCK_SPEAKER_CONFIG, options={CONF_VOLUME_STEP: VOLUME_STEP} ) entry.add_to_hass(hass) fail_entry = MOCK_SPEAKER_CONFIG.copy() @@ -216,9 +239,7 @@ async def test_user_name_already_configured( ) -> None: """Test name is already configured during user setup.""" entry = MockConfigEntry( - domain=DOMAIN, - data=MOCK_SPEAKER_CONFIG, - options={CONF_VOLUME_STEP: VOLUME_STEP}, + domain=DOMAIN, data=MOCK_SPEAKER_CONFIG, options={CONF_VOLUME_STEP: VOLUME_STEP} ) entry.add_to_hass(hass) @@ -385,3 +406,51 @@ async def test_import_flow_update_options( hass.config_entries.async_get_entry(entry_id).options[CONF_VOLUME_STEP] == VOLUME_STEP + 1 ) + + +async def test_zeroconf_flow( + hass: HomeAssistantType, vizio_connect, vizio_bypass_setup, vizio_guess_device_type +) -> None: + """Test zeroconf config flow.""" + discovery_info = MOCK_ZEROCONF_ENTRY.copy() + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_ZEROCONF}, data=discovery_info + ) + + # Form should always show even if all required properties are discovered + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + + # Apply discovery updates to entry to mimick when user hits submit without changing + # defaults which were set from discovery parameters + user_input = result["data_schema"](discovery_info) + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input=user_input + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == NAME + assert result["data"][CONF_HOST] == HOST + assert result["data"][CONF_NAME] == NAME + assert result["data"][CONF_DEVICE_CLASS] == DEVICE_CLASS_SPEAKER + + +async def test_zeroconf_flow_already_configured( + hass: HomeAssistantType, vizio_connect, vizio_bypass_setup +) -> None: + """Test entity is already configured during zeroconf setup.""" + entry = MockConfigEntry( + domain=DOMAIN, data=MOCK_SPEAKER_CONFIG, options={CONF_VOLUME_STEP: VOLUME_STEP} + ) + entry.add_to_hass(hass) + + # Try rediscovering same device + discovery_info = MOCK_ZEROCONF_ENTRY.copy() + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_ZEROCONF}, data=discovery_info + ) + + # Flow should abort because device is already setup + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_setup"