From aca52737b514fc84e2c7393c43473a158ea5b254 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 02:34:16 +0000 Subject: [PATCH 01/21] Add discovery support to synology_dsm --- .../synology_dsm/.translations/en.json | 49 +++++----- .../components/synology_dsm/config_flow.py | 96 +++++++++++++------ .../components/synology_dsm/manifest.json | 8 +- .../components/synology_dsm/strings.json | 1 + homeassistant/generated/ssdp.py | 6 ++ .../synology_dsm/test_config_flow.py | 35 ++++++- 6 files changed, 140 insertions(+), 55 deletions(-) diff --git a/homeassistant/components/synology_dsm/.translations/en.json b/homeassistant/components/synology_dsm/.translations/en.json index 327745343badac..657fc4f290f19e 100644 --- a/homeassistant/components/synology_dsm/.translations/en.json +++ b/homeassistant/components/synology_dsm/.translations/en.json @@ -1,26 +1,27 @@ { - "config": { - "abort": { - "already_configured": "Host already configured" - }, - "error": { - "login": "Login error: please check your username & password", - "unknown": "Unknown error: please retry later or an other configuration" - }, - "step": { - "user": { - "data": { - "api_version": "DSM version", - "host": "Host", - "name": "Name", - "password": "Password", - "port": "Port", - "ssl": "Use SSL/TLS to connect to your NAS", - "username": "Username" - }, - "title": "Synology DSM" - } - }, - "title": "Synology DSM" + "config": { + "title": "Synology DSM", + "flow_title": "Synology DSM {name} ({host})", + "step": { + "user": { + "title": "Synology DSM", + "data": { + "name": "Name", + "host": "Host", + "port": "Port", + "ssl": "Use SSL/TLS to connect to your NAS", + "api_version": "DSM version", + "username": "Username", + "password": "Password" + } + } + }, + "error": { + "login": "Login error: please check your username & password", + "unknown": "Unknown error: please retry later or an other configuration" + }, + "abort": { + "already_configured": "Host already configured" } -} \ No newline at end of file + } +} diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index fd23931f13f5d6..01b1b1cc55c68d 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -1,4 +1,7 @@ """Config flow to configure the Synology DSM integration.""" +import logging +from urllib.parse import urlparse + from synology_dsm import SynologyDSM from synology_dsm.api.core.utilization import SynoCoreUtilization from synology_dsm.api.dsm.information import SynoDSMInformation @@ -6,6 +9,7 @@ import voluptuous as vol from homeassistant import config_entries +from homeassistant.components import ssdp from homeassistant.const import ( CONF_API_VERSION, CONF_DISKS, @@ -27,6 +31,33 @@ ) from .const import DOMAIN # pylint: disable=unused-import +_LOGGER = logging.getLogger(__name__) + + +def _schema_with_defaults(user_input=None): + if user_input is None: + user_input = {} + + return vol.Schema( + { + vol.Optional( + CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME) + ): str, + vol.Required(CONF_HOST, default=user_input.get(CONF_HOST, "")): str, + vol.Optional(CONF_PORT, default=user_input.get(CONF_PORT, "")): str, + vol.Optional(CONF_SSL, default=user_input.get(CONF_SSL, DEFAULT_SSL)): bool, + vol.Optional( + CONF_API_VERSION, + default=user_input.get(CONF_API_VERSION, DEFAULT_DSM_VERSION), + ): vol.All( + vol.Coerce(int), + vol.In([5, 6]), # DSM versions supported by the library + ), + vol.Required(CONF_USERNAME, default=user_input.get(CONF_USERNAME, "")): str, + vol.Required(CONF_PASSWORD, default=user_input.get(CONF_PASSWORD, "")): str, + } + ) + class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow.""" @@ -34,39 +65,15 @@ class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): VERSION = 1 CONNECTION_CLASS = config_entries.CONN_CLASS_CLOUD_POLL + def __init__(self): + """Initialize the synology_dsm config flow.""" + self.discovery_schema = {} + async def _show_setup_form(self, user_input=None, errors=None): """Show the setup form to the user.""" - - if user_input is None: - user_input = {} - return self.async_show_form( step_id="user", - data_schema=vol.Schema( - { - vol.Optional( - CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME) - ): str, - vol.Required(CONF_HOST, default=user_input.get(CONF_HOST, "")): str, - vol.Optional(CONF_PORT, default=user_input.get(CONF_PORT, "")): str, - vol.Optional( - CONF_SSL, default=user_input.get(CONF_SSL, DEFAULT_SSL) - ): bool, - vol.Optional( - CONF_API_VERSION, - default=user_input.get(CONF_API_VERSION, DEFAULT_DSM_VERSION), - ): vol.All( - vol.Coerce(int), - vol.In([5, 6]), # DSM versions supported by the library - ), - vol.Required( - CONF_USERNAME, default=user_input.get(CONF_USERNAME, "") - ): str, - vol.Required( - CONF_PASSWORD, default=user_input.get(CONF_PASSWORD, "") - ): str, - } - ), + data_schema=self.discovery_schema or _schema_with_defaults(), errors=errors or {}, ) @@ -74,6 +81,8 @@ async def async_step_user(self, user_input=None): """Handle a flow initiated by the user.""" errors = {} + _LOGGER.debug("incoming user_input: %s", user_input) + if user_input is None: return await self._show_setup_form(user_input, None) @@ -137,6 +146,35 @@ async def async_step_user(self, user_input=None): return self.async_create_entry(title=host, data=config_data,) + async def async_step_ssdp(self, discovery_info): + """Handle a discovered synology_dsm.""" + parsed_url = urlparse(discovery_info[ssdp.ATTR_SSDP_LOCATION]) + friendly_name = ( + discovery_info[ssdp.ATTR_UPNP_FRIENDLY_NAME].split("(", 1)[0].strip() + ) + + if self._host_already_configured(parsed_url.hostname): + return self.async_abort(reason="already_configured") + + # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 + self.context["title_placeholders"] = { + CONF_NAME: friendly_name, + CONF_HOST: parsed_url.hostname, + } + + self.discovery_schema = _schema_with_defaults( + {CONF_HOST: parsed_url.hostname, CONF_NAME: friendly_name} + ) + + return await self.async_step_user() + async def async_step_import(self, user_input=None): """Import a config entry.""" return await self.async_step_user(user_input) + + def _host_already_configured(self, hostname): + """See if we already have a host matching user input configured.""" + existing_hosts = { + entry.data[CONF_HOST] for entry in self._async_current_entries() + } + return hostname in existing_hosts diff --git a/homeassistant/components/synology_dsm/manifest.json b/homeassistant/components/synology_dsm/manifest.json index a6d171f65283fe..e2f730ddc8d3d1 100644 --- a/homeassistant/components/synology_dsm/manifest.json +++ b/homeassistant/components/synology_dsm/manifest.json @@ -4,5 +4,11 @@ "documentation": "https://www.home-assistant.io/integrations/synology_dsm", "requirements": ["python-synology==0.5.0"], "codeowners": ["@ProtoThis", "@Quentame"], - "config_flow": true + "config_flow": true, + "ssdp": [ + { + "manufacturer": "Synology", + "deviceType": "urn:schemas-upnp-org:device:Basic:1" + } + ] } diff --git a/homeassistant/components/synology_dsm/strings.json b/homeassistant/components/synology_dsm/strings.json index b9ccf8d101026f..657fc4f290f19e 100644 --- a/homeassistant/components/synology_dsm/strings.json +++ b/homeassistant/components/synology_dsm/strings.json @@ -1,6 +1,7 @@ { "config": { "title": "Synology DSM", + "flow_title": "Synology DSM {name} ({host})", "step": { "user": { "title": "Synology DSM", diff --git a/homeassistant/generated/ssdp.py b/homeassistant/generated/ssdp.py index c9832ea2d86238..4aa8eabe9d93fc 100644 --- a/homeassistant/generated/ssdp.py +++ b/homeassistant/generated/ssdp.py @@ -70,6 +70,12 @@ "st": "urn:schemas-upnp-org:device:ZonePlayer:1" } ], + "synology_dsm": [ + { + "deviceType": "urn:schemas-upnp-org:device:Basic:1", + "manufacturer": "Synology" + } + ], "wemo": [ { "manufacturer": "Belkin International Inc." diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 664ffda7f8ea0e..e1d6ae38e0e7ba 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -4,7 +4,8 @@ import pytest -from homeassistant import data_entry_flow +from homeassistant import config_entries, data_entry_flow, setup +from homeassistant.components import ssdp from homeassistant.components.synology_dsm.const import ( CONF_VOLUMES, DEFAULT_NAME, @@ -232,3 +233,35 @@ async def test_connection_failed(hass: HomeAssistantType, service_failed: MagicM ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["errors"] == {"base": "unknown"} + + +async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): + """Test we can setup from ssdp.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", + ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", + }, + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["errors"] == {} + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD} + ) + + assert result2["type"] == "create_entry" + assert result2["title"] == "192.168.1.5" + assert result2["data"] == { + "host": "192.168.1.5", + "name": "mydsm", + "port": 5001, + "password": "password", + "ssl": True, + "username": "Home_Assistant", + } From 2059b156b1113f41a87b05112f5df50e2b11b0db Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 02:37:48 +0000 Subject: [PATCH 02/21] Remove debug --- homeassistant/components/synology_dsm/config_flow.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 01b1b1cc55c68d..f89434517fbf8a 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -81,8 +81,6 @@ async def async_step_user(self, user_input=None): """Handle a flow initiated by the user.""" errors = {} - _LOGGER.debug("incoming user_input: %s", user_input) - if user_input is None: return await self._show_setup_form(user_input, None) From ca8ba8d3a4e36e46dcbab85e67b45a984871d4d9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 15:12:13 +0000 Subject: [PATCH 03/21] black --- .../components/synology_dsm/config_flow.py | 150 +++++++++++------- .../components/synology_dsm/strings.json | 11 ++ .../synology_dsm/test_config_flow.py | 2 +- 3 files changed, 103 insertions(+), 60 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index f89434517fbf8a..e65354912aad37 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -8,7 +8,7 @@ from synology_dsm.api.storage.storage import SynoStorage import voluptuous as vol -from homeassistant import config_entries +from homeassistant import config_entries, exceptions from homeassistant.components import ssdp from homeassistant.const import ( CONF_API_VERSION, @@ -34,29 +34,33 @@ _LOGGER = logging.getLogger(__name__) -def _schema_with_defaults(user_input=None): - if user_input is None: - user_input = {} - - return vol.Schema( - { - vol.Optional( - CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME) - ): str, - vol.Required(CONF_HOST, default=user_input.get(CONF_HOST, "")): str, - vol.Optional(CONF_PORT, default=user_input.get(CONF_PORT, "")): str, - vol.Optional(CONF_SSL, default=user_input.get(CONF_SSL, DEFAULT_SSL)): bool, - vol.Optional( - CONF_API_VERSION, - default=user_input.get(CONF_API_VERSION, DEFAULT_DSM_VERSION), - ): vol.All( - vol.Coerce(int), - vol.In([5, 6]), # DSM versions supported by the library - ), - vol.Required(CONF_USERNAME, default=user_input.get(CONF_USERNAME, "")): str, - vol.Required(CONF_PASSWORD, default=user_input.get(CONF_PASSWORD, "")): str, - } - ) +def _discovery_schema_with_defaults(discovery_info=None): + return vol.Schema(_ordered_shared_schema(discovery_info)) + + +def _user_schema_with_defaults(user_input=None): + user_schema = { + vol.Optional(CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME)): str, + vol.Required(CONF_HOST, default=user_input.get(CONF_HOST, "")): str, + } + user_schema.update(_ordered_shared_schema(user_input)) + + return vol.Schema(user_schema) + + +def _ordered_shared_schema(schema_input=None): + return { + vol.Optional(CONF_PORT, default=schema_input.get(CONF_PORT, "")): str, + vol.Optional(CONF_SSL, default=schema_input.get(CONF_SSL, DEFAULT_SSL)): bool, + vol.Optional( + CONF_API_VERSION, + default=schema_input.get(CONF_API_VERSION, DEFAULT_DSM_VERSION), + ): vol.All( + vol.Coerce(int), vol.In([5, 6]), # DSM versions supported by the library + ), + vol.Required(CONF_USERNAME, default=schema_input.get(CONF_USERNAME, "")): str, + vol.Required(CONF_PASSWORD, default=schema_input.get(CONF_PASSWORD, "")): str, + } class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): @@ -67,14 +71,25 @@ class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): def __init__(self): """Initialize the synology_dsm config flow.""" - self.discovery_schema = {} + self.discovered_conf = {} async def _show_setup_form(self, user_input=None, errors=None): """Show the setup form to the user.""" + if not user_input: + user_input = {} + + if self.discovered_conf: + user_input.update(self.discovered_conf) + # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 + self.context["title_placeholders"] = self.discovered_conf + step_id = "link" + data_schema = _discovery_schema_with_defaults(user_input) + else: + step_id = "user" + data_schema = _user_schema_with_defaults(user_input) + return self.async_show_form( - step_id="user", - data_schema=self.discovery_schema or _schema_with_defaults(), - errors=errors or {}, + step_id=step_id, data_schema=data_schema, errors=errors or {}, ) async def async_step_user(self, user_input=None): @@ -84,6 +99,9 @@ async def async_step_user(self, user_input=None): if user_input is None: return await self._show_setup_form(user_input, None) + if self.discovered_conf: + user_input.update(self.discovered_conf) + name = user_input.get(CONF_NAME, DEFAULT_NAME) host = user_input[CONF_HOST] port = user_input.get(CONF_PORT) @@ -102,31 +120,20 @@ async def async_step_user(self, user_input=None): host, port, username, password, use_ssl, dsm_version=api_version, ) - if not await self.hass.async_add_executor_job(api.login): - errors[CONF_USERNAME] = "login" - return await self._show_setup_form(user_input, errors) + try: + serial = self.hass.async_add_executor_job( + self._login_and_fetch_syno_info(api) + ) + except InvalidAuth: + errors["base"] = "login" + except InvalidData: + errors["base"] = "missing_data" - information: SynoDSMInformation = await self.hass.async_add_executor_job( - getattr, api, "information" - ) - utilisation: SynoCoreUtilization = await self.hass.async_add_executor_job( - getattr, api, "utilisation" - ) - storage: SynoStorage = await self.hass.async_add_executor_job( - getattr, api, "storage" - ) - - if ( - information.serial is None - or utilisation.cpu_user_load is None - or storage.disks_ids is None - or storage.volumes_ids is None - ): - errors["base"] = "unknown" + if "base" in errors: return await self._show_setup_form(user_input, errors) # Check if already configured - await self.async_set_unique_id(information.serial) + await self.async_set_unique_id(serial) self._abort_if_unique_id_configured() config_data = { @@ -138,11 +145,30 @@ async def async_step_user(self, user_input=None): CONF_PASSWORD: password, } if user_input.get(CONF_DISKS): - config_data.update({CONF_DISKS: user_input[CONF_DISKS]}) + config_data[CONF_DISKS] = user_input[CONF_DISKS] if user_input.get(CONF_VOLUMES): - config_data.update({CONF_VOLUMES: user_input[CONF_VOLUMES]}) + config_data[CONF_VOLUMES] = user_input[CONF_VOLUMES] - return self.async_create_entry(title=host, data=config_data,) + return self.async_create_entry(title=host, data=config_data) + + def _login_and_fetch_syno_info(self, api): + """Login to the NAS and fetch basic data.""" + if not api.login(): + raise InvalidAuth + + information: SynoDSMInformation = api.information() + utilisation: SynoCoreUtilization = api.utilisation() + storage: SynoStorage = api.storage() + + if ( + information.serial is None + or utilisation.cpu_user_load is None + or storage.disks_ids is None + or storage.volumes_ids is None + ): + raise InvalidData + + return information.serial async def async_step_ssdp(self, discovery_info): """Handle a discovered synology_dsm.""" @@ -154,25 +180,31 @@ async def async_step_ssdp(self, discovery_info): if self._host_already_configured(parsed_url.hostname): return self.async_abort(reason="already_configured") - # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 - self.context["title_placeholders"] = { + self.discovered_conf = { CONF_NAME: friendly_name, CONF_HOST: parsed_url.hostname, } - - self.discovery_schema = _schema_with_defaults( - {CONF_HOST: parsed_url.hostname, CONF_NAME: friendly_name} - ) - return await self.async_step_user() async def async_step_import(self, user_input=None): """Import a config entry.""" return await self.async_step_user(user_input) + async def async_step_link(self, user_input=None): + """Link a config entry from discovery.""" + return await self.async_step_user(user_input) + def _host_already_configured(self, hostname): """See if we already have a host matching user input configured.""" existing_hosts = { entry.data[CONF_HOST] for entry in self._async_current_entries() } return hostname in existing_hosts + + +class InvalidData(exceptions.HomeAssistantError): + """Error to indicate we get invalid data from the nas.""" + + +class InvalidAuth(exceptions.HomeAssistantError): + """Error to indicate there is invalid auth.""" diff --git a/homeassistant/components/synology_dsm/strings.json b/homeassistant/components/synology_dsm/strings.json index 657fc4f290f19e..c8344a451e7e75 100644 --- a/homeassistant/components/synology_dsm/strings.json +++ b/homeassistant/components/synology_dsm/strings.json @@ -14,6 +14,17 @@ "username": "Username", "password": "Password" } + }, + "link": { + "title": "Synology DSM", + "description": "Do you want to setup {name} ({host})?", + "data": { + "port": "Port", + "ssl": "Use SSL/TLS to connect to your NAS", + "api_version": "DSM version", + "username": "Username", + "password": "Password" + } } }, "error": { diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index e1d6ae38e0e7ba..226de4261ae14e 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -248,7 +248,7 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): }, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "user" + assert result["step_id"] == "link" assert result["errors"] == {} result2 = await hass.config_entries.flow.async_configure( From aecc97261cfd90da295a6673cbee1004f3ac7d2f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 16:29:43 +0000 Subject: [PATCH 04/21] Fix mocks --- .../components/synology_dsm/config_flow.py | 14 +++++----- .../synology_dsm/test_config_flow.py | 27 ++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index e65354912aad37..9c060042e844e6 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -3,9 +3,6 @@ from urllib.parse import urlparse from synology_dsm import SynologyDSM -from synology_dsm.api.core.utilization import SynoCoreUtilization -from synology_dsm.api.dsm.information import SynoDSMInformation -from synology_dsm.api.storage.storage import SynoStorage import voluptuous as vol from homeassistant import config_entries, exceptions @@ -121,8 +118,8 @@ async def async_step_user(self, user_input=None): ) try: - serial = self.hass.async_add_executor_job( - self._login_and_fetch_syno_info(api) + serial = await self.hass.async_add_executor_job( + self._login_and_fetch_syno_info, api ) except InvalidAuth: errors["base"] = "login" @@ -156,9 +153,10 @@ def _login_and_fetch_syno_info(self, api): if not api.login(): raise InvalidAuth - information: SynoDSMInformation = api.information() - utilisation: SynoCoreUtilization = api.utilisation() - storage: SynoStorage = api.storage() + # These do i/o + information = api.information + utilisation = api.utilisation + storage = api.storage if ( information.serial is None diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 226de4261ae14e..f7252ebae27eda 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -49,9 +49,10 @@ def mock_controller_service(): "homeassistant.components.synology_dsm.config_flow.SynologyDSM" ) as service_mock: service_mock.return_value.login = Mock(return_value=True) - service_mock.return_value.information = Mock(serial=SERIAL) - service_mock.return_value.utilisation = Mock(cpu_user_load=1) - service_mock.return_value.storage = Mock(disks_ids=[], volumes_ids=[]) + service_mock.return_value.information.serial = SERIAL + service_mock.return_value.utilisation.cpu_user_load = 1 + service_mock.return_value.storage.disks_ids = [] + service_mock.return_value.storage.volumes_ids = [] yield service_mock @@ -71,10 +72,10 @@ def mock_controller_service_failed(): with patch( "homeassistant.components.synology_dsm.config_flow.SynologyDSM" ) as service_mock: - service_mock.return_value.login = Mock(return_value=True) - service_mock.return_value.information = Mock(serial=None) - service_mock.return_value.utilisation = Mock(cpu_user_load=None) - service_mock.return_value.storage = Mock(disks_ids=None, volumes_ids=None) + service_mock.return_value.information.serial = None + service_mock.return_value.utilisation.cpu_user_load = None + service_mock.return_value.storage.disks_ids = None + service_mock.return_value.storage.volumes_ids = None yield service_mock @@ -111,7 +112,7 @@ async def test_user(hass: HomeAssistantType, service: MagicMock): assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None - service.return_value.information = Mock(serial=SERIAL_2) + service.return_value.information.serial = SERIAL_2 # test without port + False SSL result = await hass.config_entries.flow.async_init( DOMAIN, @@ -157,7 +158,7 @@ async def test_import(hass: HomeAssistantType, service: MagicMock): assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None - service.return_value.information = Mock(serial=SERIAL_2) + service.return_value.information.serial = SERIAL_2 # import with all result = await hass.config_entries.flow.async_init( DOMAIN, @@ -221,10 +222,12 @@ async def test_login_failed(hass: HomeAssistantType, service_login_failed: Magic data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {CONF_USERNAME: "login"} + assert result["errors"] == {"base": "login"} -async def test_connection_failed(hass: HomeAssistantType, service_failed: MagicMock): +async def test_missing_data_after_login( + hass: HomeAssistantType, service_failed: MagicMock +): """Test when we have errors during connection.""" result = await hass.config_entries.flow.async_init( DOMAIN, @@ -232,7 +235,7 @@ async def test_connection_failed(hass: HomeAssistantType, service_failed: MagicM data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {"base": "unknown"} + assert result["errors"] == {"base": "missing_data"} async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): From 8cbf5936dee7421622b13331dab9963c58cfa7d3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 16:38:12 +0000 Subject: [PATCH 05/21] Merge strings --- .../components/synology_dsm/.translations/en.json | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/homeassistant/components/synology_dsm/.translations/en.json b/homeassistant/components/synology_dsm/.translations/en.json index 657fc4f290f19e..c8344a451e7e75 100644 --- a/homeassistant/components/synology_dsm/.translations/en.json +++ b/homeassistant/components/synology_dsm/.translations/en.json @@ -14,6 +14,17 @@ "username": "Username", "password": "Password" } + }, + "link": { + "title": "Synology DSM", + "description": "Do you want to setup {name} ({host})?", + "data": { + "port": "Port", + "ssl": "Use SSL/TLS to connect to your NAS", + "api_version": "DSM version", + "username": "Username", + "password": "Password" + } } }, "error": { From 8484b18daf51b4b0dd4b106ad96d80d8a85a8aad Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 16:42:27 +0000 Subject: [PATCH 06/21] Move placeholders --- homeassistant/components/synology_dsm/config_flow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 9c060042e844e6..fa5978572aff24 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -77,8 +77,6 @@ async def _show_setup_form(self, user_input=None, errors=None): if self.discovered_conf: user_input.update(self.discovered_conf) - # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 - self.context["title_placeholders"] = self.discovered_conf step_id = "link" data_schema = _discovery_schema_with_defaults(user_input) else: @@ -182,6 +180,8 @@ async def async_step_ssdp(self, discovery_info): CONF_NAME: friendly_name, CONF_HOST: parsed_url.hostname, } + # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 + self.context["title_placeholders"] = self.discovered_conf return await self.async_step_user() async def async_step_import(self, user_input=None): From ae108836c432ee6199336ee61e8de46931c42f23 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 16:57:54 +0000 Subject: [PATCH 07/21] add missing placeholders --- homeassistant/components/synology_dsm/config_flow.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index fa5978572aff24..85e7a7fd383c52 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -84,7 +84,10 @@ async def _show_setup_form(self, user_input=None, errors=None): data_schema = _user_schema_with_defaults(user_input) return self.async_show_form( - step_id=step_id, data_schema=data_schema, errors=errors or {}, + step_id=step_id, + data_schema=data_schema, + errors=errors or {}, + description_placeholders=self.discovered_conf or {}, ) async def async_step_user(self, user_input=None): From 46307eb60731962b2fce082ddc3eb98d443b8f6e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 17:05:23 +0000 Subject: [PATCH 08/21] reorder --- homeassistant/components/synology_dsm/.translations/en.json | 6 +++--- homeassistant/components/synology_dsm/config_flow.py | 4 ++-- homeassistant/components/synology_dsm/strings.json | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/synology_dsm/.translations/en.json b/homeassistant/components/synology_dsm/.translations/en.json index c8344a451e7e75..308ee71e58854d 100644 --- a/homeassistant/components/synology_dsm/.translations/en.json +++ b/homeassistant/components/synology_dsm/.translations/en.json @@ -8,7 +8,7 @@ "data": { "name": "Name", "host": "Host", - "port": "Port", + "port": "Port (Optional)", "ssl": "Use SSL/TLS to connect to your NAS", "api_version": "DSM version", "username": "Username", @@ -19,11 +19,11 @@ "title": "Synology DSM", "description": "Do you want to setup {name} ({host})?", "data": { - "port": "Port", "ssl": "Use SSL/TLS to connect to your NAS", "api_version": "DSM version", "username": "Username", - "password": "Password" + "password": "Password", + "port": "Port (Optional)" } } }, diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 85e7a7fd383c52..910e7c64b85193 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -47,6 +47,8 @@ def _user_schema_with_defaults(user_input=None): def _ordered_shared_schema(schema_input=None): return { + vol.Required(CONF_USERNAME, default=schema_input.get(CONF_USERNAME, "")): str, + vol.Required(CONF_PASSWORD, default=schema_input.get(CONF_PASSWORD, "")): str, vol.Optional(CONF_PORT, default=schema_input.get(CONF_PORT, "")): str, vol.Optional(CONF_SSL, default=schema_input.get(CONF_SSL, DEFAULT_SSL)): bool, vol.Optional( @@ -55,8 +57,6 @@ def _ordered_shared_schema(schema_input=None): ): vol.All( vol.Coerce(int), vol.In([5, 6]), # DSM versions supported by the library ), - vol.Required(CONF_USERNAME, default=schema_input.get(CONF_USERNAME, "")): str, - vol.Required(CONF_PASSWORD, default=schema_input.get(CONF_PASSWORD, "")): str, } diff --git a/homeassistant/components/synology_dsm/strings.json b/homeassistant/components/synology_dsm/strings.json index c8344a451e7e75..308ee71e58854d 100644 --- a/homeassistant/components/synology_dsm/strings.json +++ b/homeassistant/components/synology_dsm/strings.json @@ -8,7 +8,7 @@ "data": { "name": "Name", "host": "Host", - "port": "Port", + "port": "Port (Optional)", "ssl": "Use SSL/TLS to connect to your NAS", "api_version": "DSM version", "username": "Username", @@ -19,11 +19,11 @@ "title": "Synology DSM", "description": "Do you want to setup {name} ({host})?", "data": { - "port": "Port", "ssl": "Use SSL/TLS to connect to your NAS", "api_version": "DSM version", "username": "Username", - "password": "Password" + "password": "Password", + "port": "Port (Optional)" } } }, From 0781df0d33503b8e3a3ebd84c3f90726c308cf9a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 17:31:57 +0000 Subject: [PATCH 09/21] use constants in test --- tests/components/synology_dsm/test_config_flow.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index f7252ebae27eda..547d37a7c1e589 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -261,10 +261,10 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): assert result2["type"] == "create_entry" assert result2["title"] == "192.168.1.5" assert result2["data"] == { - "host": "192.168.1.5", - "name": "mydsm", - "port": 5001, - "password": "password", - "ssl": True, - "username": "Home_Assistant", + CONF_HOST: "192.168.1.5", + CONF_NAME: "mydsm", + CONF_PORT: 5001, + CONF_PASSWORD: "password", + CONF_SSL: True, + CONF_USERNAME: "Home_Assistant", } From 7ad9b0d529c1d2e408394ba12f29eea43bc392ee Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 17:50:08 +0000 Subject: [PATCH 10/21] Remove CONF_NAME (only displayed in discovery now) --- .../components/synology_dsm/config_flow.py | 4 ---- .../components/synology_dsm/sensor.py | 18 ++++++++---------- .../synology_dsm/test_config_flow.py | 8 -------- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 910e7c64b85193..2d69f070ec190b 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -21,7 +21,6 @@ from .const import ( CONF_VOLUMES, DEFAULT_DSM_VERSION, - DEFAULT_NAME, DEFAULT_PORT, DEFAULT_PORT_SSL, DEFAULT_SSL, @@ -37,7 +36,6 @@ def _discovery_schema_with_defaults(discovery_info=None): def _user_schema_with_defaults(user_input=None): user_schema = { - vol.Optional(CONF_NAME, default=user_input.get(CONF_NAME, DEFAULT_NAME)): str, vol.Required(CONF_HOST, default=user_input.get(CONF_HOST, "")): str, } user_schema.update(_ordered_shared_schema(user_input)) @@ -100,7 +98,6 @@ async def async_step_user(self, user_input=None): if self.discovered_conf: user_input.update(self.discovered_conf) - name = user_input.get(CONF_NAME, DEFAULT_NAME) host = user_input[CONF_HOST] port = user_input.get(CONF_PORT) username = user_input[CONF_USERNAME] @@ -135,7 +132,6 @@ async def async_step_user(self, user_input=None): self._abort_if_unique_id_configured() config_data = { - CONF_NAME: name, CONF_HOST: host, CONF_PORT: port, CONF_SSL: use_ssl, diff --git a/homeassistant/components/synology_dsm/sensor.py b/homeassistant/components/synology_dsm/sensor.py index aeefbc4989379e..9b4414332e887d 100644 --- a/homeassistant/components/synology_dsm/sensor.py +++ b/homeassistant/components/synology_dsm/sensor.py @@ -5,7 +5,6 @@ from homeassistant.const import ( ATTR_ATTRIBUTION, CONF_DISKS, - CONF_NAME, DATA_MEGABYTES, DATA_RATE_KILOBYTES_PER_SECOND, TEMP_CELSIUS, @@ -17,6 +16,7 @@ from . import SynoApi from .const import ( CONF_VOLUMES, + DEFAULT_NAME, DOMAIN, STORAGE_DISK_SENSORS, STORAGE_VOL_SENSORS, @@ -31,12 +31,11 @@ async def async_setup_entry( hass: HomeAssistantType, entry: ConfigEntry, async_add_entities ) -> None: """Set up the Synology NAS Sensor.""" - name = entry.data[CONF_NAME] api = hass.data[DOMAIN][entry.unique_id] sensors = [ - SynoNasUtilSensor(api, name, sensor_type, UTILISATION_SENSORS[sensor_type]) + SynoNasUtilSensor(api, sensor_type, UTILISATION_SENSORS[sensor_type]) for sensor_type in UTILISATION_SENSORS ] @@ -45,7 +44,7 @@ async def async_setup_entry( for volume in entry.data.get(CONF_VOLUMES, api.storage.volumes_ids): sensors += [ SynoNasStorageSensor( - api, name, sensor_type, STORAGE_VOL_SENSORS[sensor_type], volume + api, sensor_type, STORAGE_VOL_SENSORS[sensor_type], volume ) for sensor_type in STORAGE_VOL_SENSORS ] @@ -55,7 +54,7 @@ async def async_setup_entry( for disk in entry.data.get(CONF_DISKS, api.storage.disks_ids): sensors += [ SynoNasStorageSensor( - api, name, sensor_type, STORAGE_DISK_SENSORS[sensor_type], disk + api, sensor_type, STORAGE_DISK_SENSORS[sensor_type], disk ) for sensor_type in STORAGE_DISK_SENSORS ] @@ -69,7 +68,6 @@ class SynoNasSensor(Entity): def __init__( self, api: SynoApi, - name: str, sensor_type: str, sensor_info: Dict[str, str], monitored_device: str = None, @@ -77,15 +75,15 @@ def __init__( """Initialize the sensor.""" self._api = api self.sensor_type = sensor_type - self._name = f"{name} {sensor_info[0]}" + self._name = f"{DEFAULT_NAME} {self._api.information.model} {sensor_info[0]}" self._unit = sensor_info[1] self._icon = sensor_info[2] self.monitored_device = monitored_device + self._unique_id = f"{self._api.information.serial}_{sensor_info[0]}" if self.monitored_device: - self._name = f"{self._name} ({self.monitored_device})" - - self._unique_id = f"{self._api.information.serial} {self._name}" + self._name += f" ({self.monitored_device})" + self._unique_id += f"_{self.monitored_device}" self._unsub_dispatcher = None diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 547d37a7c1e589..818726f7767b22 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -8,7 +8,6 @@ from homeassistant.components import ssdp from homeassistant.components.synology_dsm.const import ( CONF_VOLUMES, - DEFAULT_NAME, DEFAULT_PORT, DEFAULT_PORT_SSL, DEFAULT_SSL, @@ -92,7 +91,6 @@ async def test_user(hass: HomeAssistantType, service: MagicMock): DOMAIN, context={"source": SOURCE_USER}, data={ - CONF_NAME: NAME, CONF_HOST: HOST, CONF_PORT: PORT, CONF_SSL: SSL, @@ -103,7 +101,6 @@ async def test_user(hass: HomeAssistantType, service: MagicMock): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["result"].unique_id == SERIAL assert result["title"] == HOST - assert result["data"][CONF_NAME] == NAME assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == PORT assert result["data"][CONF_SSL] == SSL @@ -128,7 +125,6 @@ async def test_user(hass: HomeAssistantType, service: MagicMock): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["result"].unique_id == SERIAL_2 assert result["title"] == HOST - assert result["data"][CONF_NAME] == NAME assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == DEFAULT_PORT assert not result["data"][CONF_SSL] @@ -149,7 +145,6 @@ async def test_import(hass: HomeAssistantType, service: MagicMock): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["result"].unique_id == SERIAL assert result["title"] == HOST - assert result["data"][CONF_NAME] == DEFAULT_NAME assert result["data"][CONF_HOST] == HOST assert result["data"][CONF_PORT] == DEFAULT_PORT_SSL assert result["data"][CONF_SSL] == DEFAULT_SSL @@ -164,7 +159,6 @@ async def test_import(hass: HomeAssistantType, service: MagicMock): DOMAIN, context={"source": SOURCE_IMPORT}, data={ - CONF_NAME: NAME, CONF_HOST: HOST_2, CONF_PORT: PORT, CONF_SSL: SSL, @@ -177,7 +171,6 @@ async def test_import(hass: HomeAssistantType, service: MagicMock): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["result"].unique_id == SERIAL_2 assert result["title"] == HOST_2 - assert result["data"][CONF_NAME] == NAME assert result["data"][CONF_HOST] == HOST_2 assert result["data"][CONF_PORT] == PORT assert result["data"][CONF_SSL] == SSL @@ -262,7 +255,6 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): assert result2["title"] == "192.168.1.5" assert result2["data"] == { CONF_HOST: "192.168.1.5", - CONF_NAME: "mydsm", CONF_PORT: 5001, CONF_PASSWORD: "password", CONF_SSL: True, From 379f8dc40d77a548c5451e96e9fbcd6723d784d4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 17:54:22 +0000 Subject: [PATCH 11/21] test reduction --- tests/components/synology_dsm/test_config_flow.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 818726f7767b22..8113afba8cbbf8 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -4,7 +4,7 @@ import pytest -from homeassistant import config_entries, data_entry_flow, setup +from homeassistant import data_entry_flow, setup from homeassistant.components import ssdp from homeassistant.components.synology_dsm.const import ( CONF_VOLUMES, @@ -13,7 +13,7 @@ DEFAULT_SSL, DOMAIN, ) -from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER +from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP, SOURCE_USER from homeassistant.const import ( CONF_DISKS, CONF_HOST, @@ -47,7 +47,6 @@ def mock_controller_service(): with patch( "homeassistant.components.synology_dsm.config_flow.SynologyDSM" ) as service_mock: - service_mock.return_value.login = Mock(return_value=True) service_mock.return_value.information.serial = SERIAL service_mock.return_value.utilisation.cpu_user_load = 1 service_mock.return_value.storage.disks_ids = [] @@ -237,7 +236,7 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): result = await hass.config_entries.flow.async_init( DOMAIN, - context={"source": config_entries.SOURCE_SSDP}, + context={"source": SOURCE_SSDP}, data={ ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", From 9a1216872bb1a438cf3baedcd7f82f9292c39918 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 17:58:17 +0000 Subject: [PATCH 12/21] Shorten long name --- homeassistant/components/synology_dsm/const.py | 1 + homeassistant/components/synology_dsm/sensor.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/synology_dsm/const.py b/homeassistant/components/synology_dsm/const.py index 7323413636ba06..f2434ce087b254 100644 --- a/homeassistant/components/synology_dsm/const.py +++ b/homeassistant/components/synology_dsm/const.py @@ -8,6 +8,7 @@ DOMAIN = "synology_dsm" CONF_VOLUMES = "volumes" +BASE_NAME = "Synology" DEFAULT_NAME = "Synology DSM" DEFAULT_SSL = True DEFAULT_PORT = 5000 diff --git a/homeassistant/components/synology_dsm/sensor.py b/homeassistant/components/synology_dsm/sensor.py index 9b4414332e887d..880e082711cc4a 100644 --- a/homeassistant/components/synology_dsm/sensor.py +++ b/homeassistant/components/synology_dsm/sensor.py @@ -15,8 +15,8 @@ from . import SynoApi from .const import ( + BASE_NAME, CONF_VOLUMES, - DEFAULT_NAME, DOMAIN, STORAGE_DISK_SENSORS, STORAGE_VOL_SENSORS, @@ -75,7 +75,7 @@ def __init__( """Initialize the sensor.""" self._api = api self.sensor_type = sensor_type - self._name = f"{DEFAULT_NAME} {self._api.information.model} {sensor_info[0]}" + self._name = f"{BASE_NAME} {self._api.information.model} {sensor_info[0]}" self._unit = sensor_info[1] self._icon = sensor_info[2] self.monitored_device = monitored_device From f1ddadbce641f007527c850edb09eee1b3202655 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 18:13:25 +0000 Subject: [PATCH 13/21] Use name for name but NOT for unique_id, remove CONF_NAME from yaml config --- homeassistant/components/synology_dsm/__init__.py | 4 +--- homeassistant/components/synology_dsm/const.py | 1 - homeassistant/components/synology_dsm/sensor.py | 3 +-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/synology_dsm/__init__.py b/homeassistant/components/synology_dsm/__init__.py index e2ada59ec1d884..874afb18584009 100644 --- a/homeassistant/components/synology_dsm/__init__.py +++ b/homeassistant/components/synology_dsm/__init__.py @@ -12,7 +12,6 @@ CONF_API_VERSION, CONF_DISKS, CONF_HOST, - CONF_NAME, CONF_PASSWORD, CONF_PORT, CONF_SSL, @@ -23,11 +22,10 @@ from homeassistant.helpers.event import async_track_time_interval from homeassistant.helpers.typing import HomeAssistantType -from .const import CONF_VOLUMES, DEFAULT_DSM_VERSION, DEFAULT_NAME, DEFAULT_SSL, DOMAIN +from .const import CONF_VOLUMES, DEFAULT_DSM_VERSION, DEFAULT_SSL, DOMAIN CONFIG_SCHEMA = vol.Schema( { - vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, vol.Required(CONF_HOST): cv.string, vol.Optional(CONF_PORT): cv.port, vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean, diff --git a/homeassistant/components/synology_dsm/const.py b/homeassistant/components/synology_dsm/const.py index f2434ce087b254..7323413636ba06 100644 --- a/homeassistant/components/synology_dsm/const.py +++ b/homeassistant/components/synology_dsm/const.py @@ -8,7 +8,6 @@ DOMAIN = "synology_dsm" CONF_VOLUMES = "volumes" -BASE_NAME = "Synology" DEFAULT_NAME = "Synology DSM" DEFAULT_SSL = True DEFAULT_PORT = 5000 diff --git a/homeassistant/components/synology_dsm/sensor.py b/homeassistant/components/synology_dsm/sensor.py index 880e082711cc4a..132681a777d14b 100644 --- a/homeassistant/components/synology_dsm/sensor.py +++ b/homeassistant/components/synology_dsm/sensor.py @@ -15,7 +15,6 @@ from . import SynoApi from .const import ( - BASE_NAME, CONF_VOLUMES, DOMAIN, STORAGE_DISK_SENSORS, @@ -75,7 +74,7 @@ def __init__( """Initialize the sensor.""" self._api = api self.sensor_type = sensor_type - self._name = f"{BASE_NAME} {self._api.information.model} {sensor_info[0]}" + self._name = f"{self._api.information.name} {sensor_info[0]}" self._unit = sensor_info[1] self._icon = sensor_info[2] self.monitored_device = monitored_device From 69a909a5788744a906fe5e3982ec615d70e418c4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 18:55:55 +0000 Subject: [PATCH 14/21] Use Synology for the name for now, hopefully we can use the hostname later --- homeassistant/components/synology_dsm/const.py | 1 + homeassistant/components/synology_dsm/sensor.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/synology_dsm/const.py b/homeassistant/components/synology_dsm/const.py index 7323413636ba06..f2434ce087b254 100644 --- a/homeassistant/components/synology_dsm/const.py +++ b/homeassistant/components/synology_dsm/const.py @@ -8,6 +8,7 @@ DOMAIN = "synology_dsm" CONF_VOLUMES = "volumes" +BASE_NAME = "Synology" DEFAULT_NAME = "Synology DSM" DEFAULT_SSL = True DEFAULT_PORT = 5000 diff --git a/homeassistant/components/synology_dsm/sensor.py b/homeassistant/components/synology_dsm/sensor.py index 132681a777d14b..6e5a486ab8900f 100644 --- a/homeassistant/components/synology_dsm/sensor.py +++ b/homeassistant/components/synology_dsm/sensor.py @@ -15,6 +15,7 @@ from . import SynoApi from .const import ( + BASE_NAME, CONF_VOLUMES, DOMAIN, STORAGE_DISK_SENSORS, @@ -74,7 +75,7 @@ def __init__( """Initialize the sensor.""" self._api = api self.sensor_type = sensor_type - self._name = f"{self._api.information.name} {sensor_info[0]}" + self._name = f"{BASE_NAME} {sensor_info[0]}" self._unit = sensor_info[1] self._icon = sensor_info[2] self.monitored_device = monitored_device From c51902af4dd0871da5c949a00c407918fc546ff2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 6 Apr 2020 19:23:58 +0000 Subject: [PATCH 15/21] lint --- .../components/synology_dsm/config_flow.py | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 2d69f070ec190b..c21c7365056640 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -117,7 +117,7 @@ async def async_step_user(self, user_input=None): try: serial = await self.hass.async_add_executor_job( - self._login_and_fetch_syno_info, api + _login_and_fetch_syno_info, api ) except InvalidAuth: errors["base"] = "login" @@ -145,26 +145,6 @@ async def async_step_user(self, user_input=None): return self.async_create_entry(title=host, data=config_data) - def _login_and_fetch_syno_info(self, api): - """Login to the NAS and fetch basic data.""" - if not api.login(): - raise InvalidAuth - - # These do i/o - information = api.information - utilisation = api.utilisation - storage = api.storage - - if ( - information.serial is None - or utilisation.cpu_user_load is None - or storage.disks_ids is None - or storage.volumes_ids is None - ): - raise InvalidData - - return information.serial - async def async_step_ssdp(self, discovery_info): """Handle a discovered synology_dsm.""" parsed_url = urlparse(discovery_info[ssdp.ATTR_SSDP_LOCATION]) @@ -199,6 +179,27 @@ def _host_already_configured(self, hostname): return hostname in existing_hosts +def _login_and_fetch_syno_info(api): + """Login to the NAS and fetch basic data.""" + if not api.login(): + raise InvalidAuth + + # These do i/o + information = api.information + utilisation = api.utilisation + storage = api.storage + + if ( + information.serial is None + or utilisation.cpu_user_load is None + or storage.disks_ids is None + or storage.volumes_ids is None + ): + raise InvalidData + + return information.serial + + class InvalidData(exceptions.HomeAssistantError): """Error to indicate we get invalid data from the nas.""" From d17333cd662b1e76e501ffe07632c1bf1634d92a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 7 Apr 2020 05:37:30 +0000 Subject: [PATCH 16/21] Remove name from strings --- homeassistant/components/synology_dsm/.translations/en.json | 1 - homeassistant/components/synology_dsm/strings.json | 1 - 2 files changed, 2 deletions(-) diff --git a/homeassistant/components/synology_dsm/.translations/en.json b/homeassistant/components/synology_dsm/.translations/en.json index 308ee71e58854d..77bd1250033b1f 100644 --- a/homeassistant/components/synology_dsm/.translations/en.json +++ b/homeassistant/components/synology_dsm/.translations/en.json @@ -6,7 +6,6 @@ "user": { "title": "Synology DSM", "data": { - "name": "Name", "host": "Host", "port": "Port (Optional)", "ssl": "Use SSL/TLS to connect to your NAS", diff --git a/homeassistant/components/synology_dsm/strings.json b/homeassistant/components/synology_dsm/strings.json index 308ee71e58854d..77bd1250033b1f 100644 --- a/homeassistant/components/synology_dsm/strings.json +++ b/homeassistant/components/synology_dsm/strings.json @@ -6,7 +6,6 @@ "user": { "title": "Synology DSM", "data": { - "name": "Name", "host": "Host", "port": "Port (Optional)", "ssl": "Use SSL/TLS to connect to your NAS", From adc79c14eb24122a9a2f90ec52da235fc24420b7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 7 Apr 2020 14:43:44 +0000 Subject: [PATCH 17/21] remove =None --- homeassistant/components/synology_dsm/config_flow.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index c21c7365056640..067573b00b248c 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -30,11 +30,11 @@ _LOGGER = logging.getLogger(__name__) -def _discovery_schema_with_defaults(discovery_info=None): +def _discovery_schema_with_defaults(discovery_info): return vol.Schema(_ordered_shared_schema(discovery_info)) -def _user_schema_with_defaults(user_input=None): +def _user_schema_with_defaults(user_input): user_schema = { vol.Required(CONF_HOST, default=user_input.get(CONF_HOST, "")): str, } @@ -43,7 +43,7 @@ def _user_schema_with_defaults(user_input=None): return vol.Schema(user_schema) -def _ordered_shared_schema(schema_input=None): +def _ordered_shared_schema(schema_input): return { vol.Required(CONF_USERNAME, default=schema_input.get(CONF_USERNAME, "")): str, vol.Required(CONF_PASSWORD, default=schema_input.get(CONF_PASSWORD, "")): str, From 94afb77407b10679005739c90b20af21b166502a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 7 Apr 2020 14:46:04 +0000 Subject: [PATCH 18/21] show login errors at username field --- homeassistant/components/synology_dsm/config_flow.py | 4 ++-- tests/components/synology_dsm/test_config_flow.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 067573b00b248c..f34e01d55bcc04 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -120,11 +120,11 @@ async def async_step_user(self, user_input=None): _login_and_fetch_syno_info, api ) except InvalidAuth: - errors["base"] = "login" + errors[CONF_USERNAME] = "login" except InvalidData: errors["base"] = "missing_data" - if "base" in errors: + if errors: return await self._show_setup_form(user_input, errors) # Check if already configured diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 8113afba8cbbf8..2c77ba866f624b 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -214,7 +214,7 @@ async def test_login_failed(hass: HomeAssistantType, service_login_failed: Magic data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["errors"] == {"base": "login"} + assert result["errors"] == {CONF_USERNAME: "login"} async def test_missing_data_after_login( From f038679a65b8573ce9bbeeaf77180374bca87efa Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 7 Apr 2020 09:48:40 -0500 Subject: [PATCH 19/21] Update tests/components/synology_dsm/test_config_flow.py Co-Authored-By: Quentame --- tests/components/synology_dsm/test_config_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 2c77ba866f624b..5f7d3b50c3b736 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -250,7 +250,7 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): result["flow_id"], {CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD} ) - assert result2["type"] == "create_entry" + assert result2["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result2["title"] == "192.168.1.5" assert result2["data"] == { CONF_HOST: "192.168.1.5", From a536fba39953263bea132266846516176c36fce0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 7 Apr 2020 09:48:48 -0500 Subject: [PATCH 20/21] Update tests/components/synology_dsm/test_config_flow.py Co-Authored-By: Quentame --- tests/components/synology_dsm/test_config_flow.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 5f7d3b50c3b736..27ae28aa50ea33 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -254,8 +254,8 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): assert result2["title"] == "192.168.1.5" assert result2["data"] == { CONF_HOST: "192.168.1.5", - CONF_PORT: 5001, - CONF_PASSWORD: "password", - CONF_SSL: True, - CONF_USERNAME: "Home_Assistant", + CONF_PORT: DEFAULT_PORT_SSL, + CONF_PASSWORD: PASSWORD, + CONF_SSL: DEFAULT_SSL, + CONF_USERNAME: USERNAME, } From a99e987103d066cbb059facf05409f1d44555aec Mon Sep 17 00:00:00 2001 From: Quentame Date: Tue, 7 Apr 2020 17:37:00 +0200 Subject: [PATCH 21/21] Update homeassistant/components/synology_dsm/const.py --- homeassistant/components/synology_dsm/const.py | 1 - 1 file changed, 1 deletion(-) diff --git a/homeassistant/components/synology_dsm/const.py b/homeassistant/components/synology_dsm/const.py index f2434ce087b254..dc58302fa32aed 100644 --- a/homeassistant/components/synology_dsm/const.py +++ b/homeassistant/components/synology_dsm/const.py @@ -9,7 +9,6 @@ CONF_VOLUMES = "volumes" BASE_NAME = "Synology" -DEFAULT_NAME = "Synology DSM" DEFAULT_SSL = True DEFAULT_PORT = 5000 DEFAULT_PORT_SSL = 5001