From 1eda3d31f98e0c3d2bd4bdde8ad91cb2354544b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20=C5=9EEREMET?= Date: Tue, 1 Sep 2020 20:42:17 +0300 Subject: [PATCH] Apply code review on progettihwsw (#39520) --- .../components/progettihwsw/binary_sensor.py | 10 +- .../components/progettihwsw/config_flow.py | 67 +++++++------ .../components/progettihwsw/strings.json | 6 +- .../components/progettihwsw/switch.py | 9 +- .../progettihwsw/translations/en.json | 6 +- .../progettihwsw/test_config_flow.py | 97 ++++++------------- 6 files changed, 80 insertions(+), 115 deletions(-) diff --git a/homeassistant/components/progettihwsw/binary_sensor.py b/homeassistant/components/progettihwsw/binary_sensor.py index e6ab49e8e5c..1331d3abb0a 100644 --- a/homeassistant/components/progettihwsw/binary_sensor.py +++ b/homeassistant/components/progettihwsw/binary_sensor.py @@ -18,12 +18,6 @@ from .const import DEFAULT_POLLING_INTERVAL_SEC, DOMAIN _LOGGER = logging.getLogger(DOMAIN) -def setup_platform(hass, config, add_entities, discovery_info=None): - """Set the progettihwsw platform up and create sensor instances (legacy).""" - - return True - - async def async_setup_entry(hass, config_entry, async_add_entities): """Set up the binary sensors from a config entry.""" board_api = hass.data[DOMAIN][config_entry.entry_id] @@ -47,9 +41,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): for i in range(1, int(input_count) + 1): binary_sensors.append( ProgettihwswBinarySensor( - hass, coordinator, - config_entry, f"Input #{i}", setup_input(board_api, i), ) @@ -61,7 +53,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): class ProgettihwswBinarySensor(CoordinatorEntity, BinarySensorEntity): """Represent a binary sensor.""" - def __init__(self, hass, coordinator, config_entry, name, sensor: Input): + def __init__(self, coordinator, name, sensor: Input): """Set initializing values.""" super().__init__(coordinator) self._name = name diff --git a/homeassistant/components/progettihwsw/config_flow.py b/homeassistant/components/progettihwsw/config_flow.py index 9306b134dbb..e65b58b370a 100644 --- a/homeassistant/components/progettihwsw/config_flow.py +++ b/homeassistant/components/progettihwsw/config_flow.py @@ -5,7 +5,7 @@ import voluptuous as vol from homeassistant import config_entries, core, exceptions -from .const import DOMAIN # pylint: disable=unused-import +from .const import DOMAIN DATA_SCHEMA = vol.Schema( {vol.Required("host"): str, vol.Required("port", default=80): int} @@ -15,10 +15,20 @@ DATA_SCHEMA = vol.Schema( async def validate_input(hass: core.HomeAssistant, data): """Validate the user host input.""" + confs = hass.config_entries.async_entries(DOMAIN) + same_entries = [ + True + for entry in confs + if entry.data["host"] == data["host"] and entry.data["port"] == data["port"] + ] + + if same_entries: + raise ExistingEntry + api_instance = ProgettiHWSWAPI(f'{data["host"]}:{data["port"]}') is_valid = await api_instance.check_board() - if is_valid is False: + if not is_valid: raise CannotConnect return { @@ -29,43 +39,36 @@ async def validate_input(hass: core.HomeAssistant, data): } -async def validate_input_relay_modes(data): - """Validate the user input in relay modes form.""" - for mode in data.values(): - if mode not in ("bistable", "monostable"): - raise WrongInfo - - return True - - class ProgettiHWSWConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for ProgettiHWSW Automation.""" VERSION = 1 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL + def __init__(self): + """Initialize class variables.""" + self.s1_in = None + async def async_step_relay_modes(self, user_input=None): """Manage relay modes step.""" errors = {} if user_input is not None: - try: - await validate_input_relay_modes(user_input) - whole_data = user_input - whole_data.update(self.s1_in) - except WrongInfo: - errors["base"] = "wrong_info_relay_modes" - except Exception: # pylint: disable=broad-except - errors["base"] = "unknown" - else: - return self.async_create_entry( - title=whole_data["title"], data=whole_data - ) + + whole_data = user_input + whole_data.update(self.s1_in) + + return self.async_create_entry(title=whole_data["title"], data=whole_data) relay_modes_schema = {} for i in range(1, int(self.s1_in["relay_count"]) + 1): relay_modes_schema[ vol.Required(f"relay_{str(i)}", default="bistable") - ] = str + ] = vol.In( + { + "bistable": "Bistable (ON/OFF Mode)", + "monostable": "Monostable (Timer Mode)", + } + ) return self.async_show_form( step_id="relay_modes", @@ -77,17 +80,19 @@ class ProgettiHWSWConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle the initial step.""" errors = {} if user_input is not None: + try: info = await validate_input(self.hass, user_input) - user_input.update(info) - self.s1_in = ( # pylint: disable=attribute-defined-outside-init - user_input - ) - return await self.async_step_relay_modes() except CannotConnect: errors["base"] = "cannot_connect" + except ExistingEntry: + return self.async_abort(reason="already_configured") except Exception: # pylint: disable=broad-except errors["base"] = "unknown" + else: + user_input.update(info) + self.s1_in = user_input + return await self.async_step_relay_modes() return self.async_show_form( step_id="user", data_schema=DATA_SCHEMA, errors=errors @@ -100,3 +105,7 @@ class CannotConnect(exceptions.HomeAssistantError): class WrongInfo(exceptions.HomeAssistantError): """Error to indicate we cannot validate relay modes input.""" + + +class ExistingEntry(exceptions.HomeAssistantError): + """Error to indicate we cannot validate relay modes input.""" diff --git a/homeassistant/components/progettihwsw/strings.json b/homeassistant/components/progettihwsw/strings.json index 2c25433fba9..9a0c49d2cba 100644 --- a/homeassistant/components/progettihwsw/strings.json +++ b/homeassistant/components/progettihwsw/strings.json @@ -2,8 +2,10 @@ "config": { "error": { "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", - "unknown": "[%key:common::config_flow::error::unknown%]", - "wrong_info_relay_modes": "Relay mode selection must be monostable or bistable." + "unknown": "[%key:common::config_flow::error::unknown%]" + }, + "abort": { + "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" }, "step": { "user": { diff --git a/homeassistant/components/progettihwsw/switch.py b/homeassistant/components/progettihwsw/switch.py index 5dc6005908f..1d4c1de9993 100644 --- a/homeassistant/components/progettihwsw/switch.py +++ b/homeassistant/components/progettihwsw/switch.py @@ -18,11 +18,6 @@ from .const import DEFAULT_POLLING_INTERVAL_SEC, DOMAIN _LOGGER = logging.getLogger(DOMAIN) -async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): - """Set the switch platform up (legacy).""" - return True - - async def async_setup_entry(hass, config_entry, async_add_entities): """Set up the switches from a config entry.""" board_api = hass.data[DOMAIN][config_entry.entry_id] @@ -46,9 +41,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): for i in range(1, int(relay_count) + 1): switches.append( ProgettihwswSwitch( - hass, coordinator, - config_entry, f"Relay #{i}", setup_switch(board_api, i, config_entry.data[f"relay_{str(i)}"]), ) @@ -60,7 +53,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): class ProgettihwswSwitch(CoordinatorEntity, SwitchEntity): """Represent a switch entity.""" - def __init__(self, hass, coordinator, config_entry, name, switch: Relay): + def __init__(self, coordinator, name, switch: Relay): """Initialize the values.""" super().__init__(coordinator) self._switch = switch diff --git a/homeassistant/components/progettihwsw/translations/en.json b/homeassistant/components/progettihwsw/translations/en.json index 9add8609390..7ec64794c91 100644 --- a/homeassistant/components/progettihwsw/translations/en.json +++ b/homeassistant/components/progettihwsw/translations/en.json @@ -2,8 +2,10 @@ "config": { "error": { "cannot_connect": "Failed to connect", - "unknown": "Unexpected error", - "wrong_info_relay_modes": "Relay mode selection must be monostable or bistable." + "unknown": "Unknown error" + }, + "abort": { + "already_configured": "This board has already been set up." }, "step": { "relay_modes": { diff --git a/tests/components/progettihwsw/test_config_flow.py b/tests/components/progettihwsw/test_config_flow.py index 7da0ef82642..7a0dbd692c0 100644 --- a/tests/components/progettihwsw/test_config_flow.py +++ b/tests/components/progettihwsw/test_config_flow.py @@ -2,9 +2,14 @@ from homeassistant import config_entries, setup from homeassistant.components.progettihwsw.const import DOMAIN from homeassistant.const import CONF_HOST, CONF_PORT -from homeassistant.data_entry_flow import RESULT_TYPE_CREATE_ENTRY, RESULT_TYPE_FORM +from homeassistant.data_entry_flow import ( + RESULT_TYPE_ABORT, + RESULT_TYPE_CREATE_ENTRY, + RESULT_TYPE_FORM, +) from tests.async_mock import patch +from tests.common import MockConfigEntry mock_value_step_user = { "title": "1R & 1IN Board", @@ -60,35 +65,6 @@ async def test_form(hass): assert result3["data"]["relay_count"] == result3["data"]["input_count"] == 1 -async def test_form_wrong_info(hass): - """Test we handle wrong info exception.""" - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - - assert result["step_id"] == "user" - - with patch( - "homeassistant.components.progettihwsw.config_flow.ProgettiHWSWAPI.check_board", - return_value=mock_value_step_user, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], {CONF_HOST: "", CONF_PORT: 80} - ) - - assert result2["type"] == RESULT_TYPE_FORM - assert result2["step_id"] == "relay_modes" - assert result2["errors"] == {} - - result3 = await hass.config_entries.flow.async_configure( - result2["flow_id"], {"relay_1": ""} - ) - - assert result3["type"] == RESULT_TYPE_FORM - assert result3["step_id"] == "relay_modes" - assert result3["errors"] == {"base": "wrong_info_relay_modes"} - - async def test_form_cannot_connect(hass): """Test we handle unexisting board.""" result = await hass.config_entries.flow.async_init( @@ -111,6 +87,32 @@ async def test_form_cannot_connect(hass): assert result2["errors"] == {"base": "cannot_connect"} +async def test_form_existing_entry_exception(hass): + """Test we handle existing board.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + + assert result["step_id"] == "user" + + entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_HOST: "", + CONF_PORT: 80, + }, + ) + entry.add_to_hass(hass) + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_HOST: "", CONF_PORT: 80}, + ) + + assert result2["type"] == RESULT_TYPE_ABORT + assert result2["reason"] == "already_configured" + + async def test_form_user_exception(hass): """Test we handle unknown exception.""" result = await hass.config_entries.flow.async_init( @@ -131,38 +133,3 @@ async def test_form_user_exception(hass): assert result2["type"] == RESULT_TYPE_FORM assert result2["step_id"] == "user" assert result2["errors"] == {"base": "unknown"} - - -async def test_form_rm_exception(hass): - """Test we handle unknown exception on seconds step.""" - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - - assert result["step_id"] == "user" - - with patch( - "homeassistant.components.progettihwsw.config_flow.ProgettiHWSWAPI.check_board", - return_value=mock_value_step_user, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - {CONF_HOST: "", CONF_PORT: 80}, - ) - - assert result2["type"] == RESULT_TYPE_FORM - assert result2["step_id"] == "relay_modes" - assert result2["errors"] == {} - - with patch( - "homeassistant.components.progettihwsw.config_flow.validate_input_relay_modes", - side_effect=Exception, - ): - result3 = await hass.config_entries.flow.async_configure( - result2["flow_id"], - {"relay_1": "bistable"}, - ) - - assert result3["type"] == RESULT_TYPE_FORM - assert result3["step_id"] == "relay_modes" - assert result3["errors"] == {"base": "unknown"}