From 1010edf4bd6a7f99322920754644e4552945028a Mon Sep 17 00:00:00 2001 From: ilan <31193909+iloveicedgreentea@users.noreply.github.com> Date: Sun, 18 Aug 2024 15:17:10 -0400 Subject: [PATCH] Add reconfigure flow to Madvr (#122477) * feat: add reconfigure * feat: add reconfigure step * fix: don't abort unique on reconfigure * fix: add success string * fix: improve reconfigure * fix: entry will never be none * fix: update ip in abort * fix: check unique id on reconfigure * feat: add test in case of new device * fix: fail reconfigure if mac changes * fix: abort instead of form * feat: use is, dont mock config flow * fix: implement comments --- homeassistant/components/madvr/config_flow.py | 145 +++++++++++------- homeassistant/components/madvr/strings.json | 20 ++- tests/components/madvr/const.py | 1 + tests/components/madvr/test_config_flow.py | 121 ++++++++++++++- 4 files changed, 227 insertions(+), 60 deletions(-) diff --git a/homeassistant/components/madvr/config_flow.py b/homeassistant/components/madvr/config_flow.py index cf43e03a68b..1ca1dd296d8 100644 --- a/homeassistant/components/madvr/config_flow.py +++ b/homeassistant/components/madvr/config_flow.py @@ -8,8 +8,9 @@ import aiohttp from madvr.madvr import HeartBeatError, Madvr import voluptuous as vol -from homeassistant.config_entries import ConfigFlow, ConfigFlowResult +from homeassistant.config_entries import ConfigEntry, ConfigFlow, ConfigFlowResult from homeassistant.const import CONF_HOST, CONF_PORT +from homeassistant.core import HomeAssistant from .const import DEFAULT_NAME, DEFAULT_PORT, DOMAIN from .errors import CannotConnect @@ -18,13 +19,8 @@ _LOGGER = logging.getLogger(__name__) STEP_USER_DATA_SCHEMA = vol.Schema( { - vol.Required( - CONF_HOST, - ): str, - vol.Required( - CONF_PORT, - default=DEFAULT_PORT, - ): int, + vol.Required(CONF_HOST): str, + vol.Required(CONF_PORT, default=DEFAULT_PORT): int, } ) @@ -36,81 +32,120 @@ class MadVRConfigFlow(ConfigFlow, domain=DOMAIN): VERSION = 1 + entry: ConfigEntry | None = None + async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Handle the initial step.""" + return await self._handle_config_step(user_input) + + async def async_step_reconfigure( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle reconfiguration of the device.""" + self.entry = self.hass.config_entries.async_get_entry(self.context["entry_id"]) + return await self.async_step_reconfigure_confirm(user_input) + + async def async_step_reconfigure_confirm( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle a reconfiguration flow initialized by the user.""" + return await self._handle_config_step(user_input, step_id="reconfigure") + + async def _handle_config_step( + self, user_input: dict[str, Any] | None = None, step_id: str = "user" + ) -> ConfigFlowResult: + """Handle the configuration step for both initial setup and reconfiguration.""" errors: dict[str, str] = {} if user_input is not None: + _LOGGER.debug("User input: %s", user_input) host = user_input[CONF_HOST] port = user_input[CONF_PORT] try: - # ensure we can connect and get the mac address from device - mac = await self._test_connection(host, port) + mac = await test_connection(self.hass, host, port) except CannotConnect: _LOGGER.error("CannotConnect error caught") errors["base"] = "cannot_connect" else: if not mac: errors["base"] = "no_mac" - if not errors: - _LOGGER.debug("MAC address found: %s", mac) - # this will prevent the user from adding the same device twice and persist the mac address - await self.async_set_unique_id(mac) - self._abort_if_unique_id_configured() + else: + _LOGGER.debug("MAC address found: %s", mac) + # abort if the detected mac differs from the one in the entry + if self.entry: + existing_mac = self.entry.unique_id + if existing_mac != mac: + _LOGGER.debug( + "MAC address changed from %s to %s", existing_mac, mac + ) + # abort + return self.async_abort(reason="set_up_new_device") - # create the entry - return self.async_create_entry( - title=DEFAULT_NAME, - data=user_input, - ) + _LOGGER.debug("Reconfiguration done") + return self.async_update_reload_and_abort( + entry=self.entry, + data={**user_input, CONF_HOST: host, CONF_PORT: port}, + reason="reconfigure_successful", + ) + # abort if already configured with same mac + await self.async_set_unique_id(mac) + self._abort_if_unique_id_configured(updates={CONF_HOST: host}) - # this will show the form or allow the user to retry if there was an error + _LOGGER.debug("Configuration successful") + return self.async_create_entry( + title=DEFAULT_NAME, + data=user_input, + ) + _LOGGER.debug("Showing form with errors: %s", errors) return self.async_show_form( - step_id="user", + step_id=step_id, data_schema=self.add_suggested_values_to_schema( STEP_USER_DATA_SCHEMA, user_input ), errors=errors, ) - async def _test_connection(self, host: str, port: int) -> str: - """Test if we can connect to the device and grab the mac.""" - madvr_client = Madvr(host=host, port=port, loop=self.hass.loop) - _LOGGER.debug("Testing connection to madVR at %s:%s", host, port) - # try to connect - try: - await asyncio.wait_for(madvr_client.open_connection(), timeout=15) - # connection can raise HeartBeatError if the device is not available or connection does not work - except (TimeoutError, aiohttp.ClientError, OSError, HeartBeatError) as err: - _LOGGER.error("Error connecting to madVR: %s", err) - raise CannotConnect from err - # check if we are connected - if not madvr_client.connected: - raise CannotConnect("Connection failed") +async def test_connection(hass: HomeAssistant, host: str, port: int) -> str: + """Test if we can connect to the device and grab the mac.""" + madvr_client = Madvr(host=host, port=port, loop=hass.loop) + _LOGGER.debug("Testing connection to madVR at %s:%s", host, port) + # try to connect + try: + await asyncio.wait_for(madvr_client.open_connection(), timeout=15) + # connection can raise HeartBeatError if the device is not available or connection does not work + except (TimeoutError, aiohttp.ClientError, OSError, HeartBeatError) as err: + _LOGGER.error("Error connecting to madVR: %s", err) + raise CannotConnect from err - # background tasks needed to capture realtime info - await madvr_client.async_add_tasks() + # check if we are connected + if not madvr_client.connected: + raise CannotConnect("Connection failed") - # wait for client to capture device info - retry_time = 15 - while not madvr_client.mac_address and retry_time > 0: - await asyncio.sleep(RETRY_INTERVAL) - retry_time -= 1 + # background tasks needed to capture realtime info + await madvr_client.async_add_tasks() - mac_address = madvr_client.mac_address - if mac_address: - _LOGGER.debug("Connected to madVR with MAC: %s", mac_address) - # close this connection because this client object will not be reused - await self._close_test_connection(madvr_client) - _LOGGER.debug("Connection test successful") - return mac_address + # wait for client to capture device info + retry_time = 15 + while not madvr_client.mac_address and retry_time > 0: + await asyncio.sleep(RETRY_INTERVAL) + retry_time -= 1 - async def _close_test_connection(self, madvr_client: Madvr) -> None: - """Close the test connection.""" - madvr_client.stop() - await madvr_client.async_cancel_tasks() - await madvr_client.close_connection() + mac_address = madvr_client.mac_address + if mac_address: + _LOGGER.debug("Connected to madVR with MAC: %s", mac_address) + # close this connection because this client object will not be reused + await close_test_connection(madvr_client) + _LOGGER.debug("Connection test successful") + return mac_address + + +async def close_test_connection(madvr_client: Madvr) -> None: + """Close the test connection.""" + _LOGGER.debug("Closing test connection") + madvr_client.stop() + await madvr_client.async_cancel_tasks() + await madvr_client.close_connection() diff --git a/homeassistant/components/madvr/strings.json b/homeassistant/components/madvr/strings.json index 3e8e786f775..b8d30be23aa 100644 --- a/homeassistant/components/madvr/strings.json +++ b/homeassistant/components/madvr/strings.json @@ -3,7 +3,19 @@ "step": { "user": { "title": "Setup madVR Envy", - "description": "Your device needs to be turned in order to add the integation. ", + "description": "Your device needs to be on in order to add the integation. ", + "data": { + "host": "[%key:common::config_flow::data::host%]", + "port": "[%key:common::config_flow::data::port%]" + }, + "data_description": { + "host": "The hostname or IP address of your madVR Envy device.", + "port": "The port your madVR Envy is listening on. In 99% of cases, leave this as the default." + } + }, + "reconfigure": { + "title": "Reconfigure madVR Envy", + "description": "Your device needs to be on in order to reconfigure the integation. ", "data": { "host": "[%key:common::config_flow::data::host%]", "port": "[%key:common::config_flow::data::port%]" @@ -15,11 +27,13 @@ } }, "abort": { - "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", + "reconfigure_successful": "[%key:common::config_flow::abort::reconfigure_successful%]" }, "error": { "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", - "no_mac": "A MAC address was not found. It required to identify the device. Please ensure your device is connectable." + "no_mac": "A MAC address was not found. It required to identify the device. Please ensure your device is connectable.", + "set_up_new_device": "A new device was detected. Please set it up as a new entity instead of reconfiguring." } }, "entity": { diff --git a/tests/components/madvr/const.py b/tests/components/madvr/const.py index 8c5e122377b..e1c5435fcbb 100644 --- a/tests/components/madvr/const.py +++ b/tests/components/madvr/const.py @@ -8,6 +8,7 @@ MOCK_CONFIG = { } MOCK_MAC = "00:11:22:33:44:55" +MOCK_MAC_NEW = "00:00:00:00:00:01" TEST_CON_ERROR = ConnectionError("Connection failed") TEST_IMP_ERROR = NotImplementedError("Not implemented") diff --git a/tests/components/madvr/test_config_flow.py b/tests/components/madvr/test_config_flow.py index 6dc84fd6b00..65eba05c802 100644 --- a/tests/components/madvr/test_config_flow.py +++ b/tests/components/madvr/test_config_flow.py @@ -6,12 +6,12 @@ from unittest.mock import AsyncMock, patch import pytest from homeassistant.components.madvr.const import DEFAULT_NAME, DOMAIN -from homeassistant.config_entries import SOURCE_USER +from homeassistant.config_entries import SOURCE_RECONFIGURE, SOURCE_USER from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType -from .const import MOCK_CONFIG, MOCK_MAC +from .const import MOCK_CONFIG, MOCK_MAC, MOCK_MAC_NEW from tests.common import MockConfigEntry @@ -126,3 +126,120 @@ async def test_duplicate( ) assert result["type"] is FlowResultType.ABORT assert result["reason"] == "already_configured" + + +async def test_reconfigure_flow( + hass: HomeAssistant, + mock_madvr_client: AsyncMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test reconfigure flow.""" + mock_config_entry.add_to_hass(hass) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_RECONFIGURE, "entry_id": mock_config_entry.entry_id}, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reconfigure" + assert result["errors"] == {} + + # define new host + new_host = "192.168.1.100" + # make sure setting port works + new_port = 44078 + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: new_host, CONF_PORT: new_port}, + ) + + # should get the abort with success result + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reconfigure_successful" + + # Verify that the config entry was updated + assert mock_config_entry.data[CONF_HOST] == new_host + assert mock_config_entry.data[CONF_PORT] == new_port + + # Verify that the connection was tested + mock_madvr_client.open_connection.assert_called() + mock_madvr_client.async_add_tasks.assert_called() + mock_madvr_client.async_cancel_tasks.assert_called() + + +async def test_reconfigure_new_device( + hass: HomeAssistant, + mock_madvr_client: AsyncMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test reconfigure flow.""" + mock_config_entry.add_to_hass(hass) + # test reconfigure with a new device (should fail) + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_RECONFIGURE, "entry_id": mock_config_entry.entry_id}, + ) + + # define new host + new_host = "192.168.1.100" + # make sure setting port works + new_port = 44078 + + # modify test_connection so it returns new_mac + mock_madvr_client.mac_address = MOCK_MAC_NEW + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: new_host, CONF_PORT: new_port}, + ) + + # unique id should remain unchanged with new device, should fail + assert mock_config_entry.unique_id == MOCK_MAC + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "set_up_new_device" + + +async def test_reconfigure_flow_errors( + hass: HomeAssistant, + mock_madvr_client: AsyncMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test error handling in reconfigure flow.""" + mock_config_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_RECONFIGURE, "entry_id": mock_config_entry.entry_id}, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "reconfigure" + + # Test CannotConnect error + mock_madvr_client.open_connection.side_effect = TimeoutError + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "192.168.1.100", CONF_PORT: 44077}, + ) + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": "cannot_connect"} + + # Test no_mac error + mock_madvr_client.open_connection.side_effect = None + mock_madvr_client.connected = True + mock_madvr_client.mac_address = None + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "192.168.1.100", CONF_PORT: 44077}, + ) + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": "no_mac"} + + # Ensure errors are recoverable + mock_madvr_client.mac_address = MOCK_MAC + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "192.168.1.100", CONF_PORT: 44077}, + ) + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reconfigure_successful"