From 4c27d6b9aaae4ae9c98f76aa002d9b5c5de88619 Mon Sep 17 00:00:00 2001 From: Maciej Bieniek Date: Wed, 22 Jan 2020 20:34:11 +0100 Subject: [PATCH] Add zeroconf discovery support to Brother Printer integration (#30959) * Add zeroconf discovery support * Fix data for config_entry * Add sting for zeroconf confirm dialog * Add and fix tests * Fix pylint errors * Suggested changes * Tests * Remove unnecessary object * Add error handling * Remove unnecessary objects * Suggested change * Suggested change * Use core interfaces for tests --- .../components/brother/config_flow.py | 57 ++++++ .../components/brother/manifest.json | 1 + homeassistant/components/brother/strings.json | 8 + homeassistant/generated/zeroconf.py | 3 + tests/components/brother/test_config_flow.py | 166 +++++++++++++----- 5 files changed, 192 insertions(+), 43 deletions(-) diff --git a/homeassistant/components/brother/config_flow.py b/homeassistant/components/brother/config_flow.py index b95469977a7..e50105e0b27 100644 --- a/homeassistant/components/brother/config_flow.py +++ b/homeassistant/components/brother/config_flow.py @@ -34,6 +34,11 @@ class BrotherConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): VERSION = 1 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL + def __init__(self): + """Initialize.""" + self.brother = None + self.host = None + async def async_step_user(self, user_input=None): """Handle the initial step.""" errors = {} @@ -64,6 +69,58 @@ class BrotherConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): step_id="user", data_schema=DATA_SCHEMA, errors=errors ) + async def async_step_zeroconf(self, user_input=None): + """Handle zeroconf discovery.""" + if user_input is None: + return self.async_abort(reason="connection_error") + + if not user_input.get("name") or not user_input["name"].startswith("Brother"): + return self.async_abort(reason="not_brother_printer") + + # Hostname is format: brother.local. + self.host = user_input["hostname"].rstrip(".") + + self.brother = Brother(self.host) + try: + await self.brother.async_update() + except (ConnectionError, SnmpError, UnsupportedModel): + return self.async_abort(reason="connection_error") + + # Check if already configured + await self.async_set_unique_id(self.brother.serial.lower()) + self._abort_if_unique_id_configured() + + # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 + self.context.update( + { + "title_placeholders": { + "serial_number": self.brother.serial, + "model": self.brother.model, + } + } + ) + return await self.async_step_zeroconf_confirm() + + async def async_step_zeroconf_confirm(self, user_input=None): + """Handle a flow initiated by zeroconf.""" + if user_input is not None: + title = f"{self.brother.model} {self.brother.serial}" + # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 + return self.async_create_entry( + title=title, + data={CONF_HOST: self.host, CONF_TYPE: user_input[CONF_TYPE]}, + ) + return self.async_show_form( + step_id="zeroconf_confirm", + data_schema=vol.Schema( + {vol.Optional(CONF_TYPE, default="laser"): vol.In(PRINTER_TYPES)} + ), + description_placeholders={ + "serial_number": self.brother.serial, + "model": self.brother.model, + }, + ) + class InvalidHost(exceptions.HomeAssistantError): """Error to indicate that hostname/IP address is invalid.""" diff --git a/homeassistant/components/brother/manifest.json b/homeassistant/components/brother/manifest.json index d080ee4fd6c..e63fb9b0d7c 100644 --- a/homeassistant/components/brother/manifest.json +++ b/homeassistant/components/brother/manifest.json @@ -5,5 +5,6 @@ "dependencies": [], "codeowners": ["@bieniu"], "requirements": ["brother==0.1.4"], + "zeroconf": ["_printer._tcp.local."], "config_flow": true } diff --git a/homeassistant/components/brother/strings.json b/homeassistant/components/brother/strings.json index b636b7c0202..c14903df950 100644 --- a/homeassistant/components/brother/strings.json +++ b/homeassistant/components/brother/strings.json @@ -1,6 +1,7 @@ { "config": { "title": "Brother Printer", + "flow_title": "Brother Printer: {model} {serial_number}", "step": { "user": { "title": "Brother Printer", @@ -9,6 +10,13 @@ "host": "Printer hostname or IP address", "type": "Type of the printer" } + }, + "zeroconf_confirm": { + "description": "Do you want to add the Brother Printer {model} with serial number `{serial_number}` to Home Assistant?", + "title": "Discovered Brother Printer", + "data": { + "type": "Type of the printer" + } } }, "error": { diff --git a/homeassistant/generated/zeroconf.py b/homeassistant/generated/zeroconf.py index 9af4bde8a2a..8d3bff42d12 100644 --- a/homeassistant/generated/zeroconf.py +++ b/homeassistant/generated/zeroconf.py @@ -24,6 +24,9 @@ ZEROCONF = { "_hap._tcp.local.": [ "homekit_controller" ], + "_printer._tcp.local.": [ + "brother" + ], "_viziocast._tcp.local.": [ "vizio" ], diff --git a/tests/components/brother/test_config_flow.py b/tests/components/brother/test_config_flow.py index 5f81be7c1ea..3f07fca49f0 100644 --- a/tests/components/brother/test_config_flow.py +++ b/tests/components/brother/test_config_flow.py @@ -5,30 +5,23 @@ from asynctest import patch from brother import SnmpError, UnsupportedModel from homeassistant import data_entry_flow -from homeassistant.components.brother import config_flow from homeassistant.components.brother.const import DOMAIN -from homeassistant.const import CONF_HOST, CONF_NAME, CONF_TYPE +from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF +from homeassistant.const import CONF_HOST, CONF_TYPE -from tests.common import load_fixture +from tests.common import MockConfigEntry, load_fixture -CONFIG = { - CONF_HOST: "localhost", - CONF_NAME: "Printer", - CONF_TYPE: "laser", -} +CONFIG = {CONF_HOST: "localhost", CONF_TYPE: "laser"} async def test_show_form(hass): """Test that the form is served with no input.""" - flow = config_flow.BrotherConfigFlow() - flow.hass = hass - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "user"} + DOMAIN, context={"source": SOURCE_USER} ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "user" + assert result["step_id"] == SOURCE_USER async def test_create_entry_with_hostname(hass): @@ -37,18 +30,14 @@ async def test_create_entry_with_hostname(hass): "brother.Brother._get_data", return_value=json.loads(load_fixture("brother_printer_data.json")), ): - flow = config_flow.BrotherConfigFlow() - flow.hass = hass - flow.context = {} - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "user"}, data=CONFIG + DOMAIN, context={"source": SOURCE_USER}, data=CONFIG ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == "HL-L2340DW 0123456789" assert result["data"][CONF_HOST] == CONFIG[CONF_HOST] - assert result["data"][CONF_NAME] == CONFIG[CONF_NAME] + assert result["data"][CONF_TYPE] == CONFIG[CONF_TYPE] async def test_create_entry_with_ip_address(hass): @@ -57,31 +46,24 @@ async def test_create_entry_with_ip_address(hass): "brother.Brother._get_data", return_value=json.loads(load_fixture("brother_printer_data.json")), ): - flow = config_flow.BrotherConfigFlow() - flow.hass = hass - flow.context = {} - result = await hass.config_entries.flow.async_init( DOMAIN, - context={"source": "user"}, - data={CONF_NAME: "Name", CONF_HOST: "127.0.0.1", CONF_TYPE: "laser"}, + context={"source": SOURCE_USER}, + data={CONF_HOST: "127.0.0.1", CONF_TYPE: "laser"}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == "HL-L2340DW 0123456789" assert result["data"][CONF_HOST] == "127.0.0.1" - assert result["data"][CONF_NAME] == "Name" + assert result["data"][CONF_TYPE] == "laser" async def test_invalid_hostname(hass): """Test invalid hostname in user_input.""" - flow = config_flow.BrotherConfigFlow() - flow.hass = hass - result = await hass.config_entries.flow.async_init( DOMAIN, - context={"source": "user"}, - data={CONF_NAME: "Name", CONF_HOST: "invalid/hostname", CONF_TYPE: "laser"}, + context={"source": SOURCE_USER}, + data={CONF_HOST: "invalid/hostname", CONF_TYPE: "laser"}, ) assert result["errors"] == {CONF_HOST: "wrong_host"} @@ -90,11 +72,8 @@ async def test_invalid_hostname(hass): async def test_connection_error(hass): """Test connection to host error.""" with patch("brother.Brother._get_data", side_effect=ConnectionError()): - flow = config_flow.BrotherConfigFlow() - flow.hass = hass - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "user"}, data=CONFIG + DOMAIN, context={"source": SOURCE_USER}, data=CONFIG ) assert result["errors"] == {"base": "connection_error"} @@ -103,11 +82,8 @@ async def test_connection_error(hass): async def test_snmp_error(hass): """Test SNMP error.""" with patch("brother.Brother._get_data", side_effect=SnmpError("error")): - flow = config_flow.BrotherConfigFlow() - flow.hass = hass - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "user"}, data=CONFIG + DOMAIN, context={"source": SOURCE_USER}, data=CONFIG ) assert result["errors"] == {"base": "snmp_error"} @@ -116,12 +92,116 @@ async def test_snmp_error(hass): async def test_unsupported_model_error(hass): """Test unsupported printer model error.""" with patch("brother.Brother._get_data", side_effect=UnsupportedModel("error")): - flow = config_flow.BrotherConfigFlow() - flow.hass = hass result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": "user"}, data=CONFIG + DOMAIN, context={"source": SOURCE_USER}, data=CONFIG ) - assert result["type"] == "abort" + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "unsupported_model" + + +async def test_device_exists_abort(hass): + """Test we abort config flow if Brother printer already configured.""" + with patch( + "brother.Brother._get_data", + return_value=json.loads(load_fixture("brother_printer_data.json")), + ): + MockConfigEntry(domain=DOMAIN, unique_id="0123456789", data=CONFIG).add_to_hass( + hass + ) + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER}, data=CONFIG + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured" + + +async def test_zeroconf_no_data(hass): + """Test we abort if zeroconf provides no data.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_ZEROCONF} + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "connection_error" + + +async def test_zeroconf_not_brother_printer_error(hass): + """Test we abort zeroconf flow if printer isn't Brother.""" + with patch( + "brother.Brother._get_data", + return_value=json.loads(load_fixture("brother_printer_data.json")), + ): + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_ZEROCONF}, + data={"hostname": "example.local.", "name": "Another Printer"}, + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "not_brother_printer" + + +async def test_zeroconf_snmp_error(hass): + """Test we abort zeroconf flow on SNMP error.""" + with patch("brother.Brother._get_data", side_effect=SnmpError("error")): + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_ZEROCONF}, + data={"hostname": "example.local.", "name": "Brother Printer"}, + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "connection_error" + + +async def test_zeroconf_device_exists_abort(hass): + """Test we abort zeroconf flow if Brother printer already configured.""" + with patch( + "brother.Brother._get_data", + return_value=json.loads(load_fixture("brother_printer_data.json")), + ): + MockConfigEntry(domain=DOMAIN, unique_id="0123456789", data=CONFIG).add_to_hass( + hass + ) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_ZEROCONF}, + data={"hostname": "example.local.", "name": "Brother Printer"}, + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "already_configured" + + +async def test_zeroconf_confirm_create_entry(hass): + """Test zeroconf confirmation and create config entry.""" + with patch( + "brother.Brother._get_data", + return_value=json.loads(load_fixture("brother_printer_data.json")), + ): + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_ZEROCONF}, + data={"hostname": "example.local.", "name": "Brother Printer"}, + ) + + assert result["step_id"] == "zeroconf_confirm" + assert result["description_placeholders"]["model"] == "HL-L2340DW" + assert result["description_placeholders"]["serial_number"] == "0123456789" + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={CONF_TYPE: "laser"} + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == "HL-L2340DW 0123456789" + assert result["data"][CONF_HOST] == "example.local" + assert result["data"][CONF_TYPE] == "laser"