diff --git a/homeassistant/components/synology_dsm/__init__.py b/homeassistant/components/synology_dsm/__init__.py index 3fbed6955d9..9431cb7b1c9 100644 --- a/homeassistant/components/synology_dsm/__init__.py +++ b/homeassistant/components/synology_dsm/__init__.py @@ -11,6 +11,7 @@ from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import ( CONF_DISKS, CONF_HOST, + CONF_MAC, CONF_PASSWORD, CONF_PORT, CONF_SSL, @@ -77,6 +78,13 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry): hass.data.setdefault(DOMAIN, {}) hass.data[DOMAIN][entry.unique_id] = api + # For SSDP compat + if not entry.data.get(CONF_MAC): + network = await hass.async_add_executor_job(getattr, api.dsm, "network") + hass.config_entries.async_update_entry( + entry, data={**entry.data, CONF_MAC: network.macs} + ) + hass.async_create_task( hass.config_entries.async_forward_entry_setup(entry, "sensor") ) @@ -115,7 +123,7 @@ class SynoApi: self._device_token = device_token self.temp_unit = temp_unit - self._dsm: SynologyDSM = None + self.dsm: SynologyDSM = None self.information: SynoDSMInformation = None self.utilisation: SynoCoreUtilization = None self.storage: SynoStorage = None @@ -129,7 +137,7 @@ class SynoApi: async def async_setup(self): """Start interacting with the NAS.""" - self._dsm = SynologyDSM( + self.dsm = SynologyDSM( self._host, self._port, self._username, @@ -147,9 +155,9 @@ class SynoApi: def _fetch_device_configuration(self): """Fetch initial device config.""" - self.information = self._dsm.information - self.utilisation = self._dsm.utilisation - self.storage = self._dsm.storage + self.information = self.dsm.information + self.utilisation = self.dsm.utilisation + self.storage = self.dsm.storage async def async_unload(self): """Stop interacting with the NAS and prepare for removal from hass.""" @@ -157,5 +165,5 @@ class SynoApi: async def update(self, now=None): """Update function for updating API information.""" - await self._hass.async_add_executor_job(self._dsm.update) + await self._hass.async_add_executor_job(self.dsm.update) async_dispatcher_send(self._hass, self.signal_sensor_update) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 4b09e516451..c3d15aff2fd 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -17,6 +17,7 @@ from homeassistant.components import ssdp from homeassistant.const import ( CONF_DISKS, CONF_HOST, + CONF_MAC, CONF_NAME, CONF_PASSWORD, CONF_PORT, @@ -145,6 +146,7 @@ class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): CONF_SSL: use_ssl, CONF_USERNAME: username, CONF_PASSWORD: password, + CONF_MAC: api.network.macs, } if otp_code: config_data["device_token"] = api.device_token @@ -162,16 +164,11 @@ class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): discovery_info[ssdp.ATTR_UPNP_FRIENDLY_NAME].split("(", 1)[0].strip() ) - if self._host_already_configured(parsed_url.hostname): + # Synology NAS can broadcast on multiple IP addresses, since they can be connected to multiple ethernets. + # The serial of the NAS is actually its MAC address. + if self._mac_already_configured(discovery_info[ssdp.ATTR_UPNP_SERIAL].upper()): return self.async_abort(reason="already_configured") - if ssdp.ATTR_UPNP_SERIAL in discovery_info: - # Synology can broadcast on multiple IP addresses - await self.async_set_unique_id( - discovery_info[ssdp.ATTR_UPNP_SERIAL].upper() - ) - self._abort_if_unique_id_configured() - self.discovered_conf = { CONF_NAME: friendly_name, CONF_HOST: parsed_url.hostname, @@ -205,12 +202,14 @@ class SynologyDSMFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): 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 + def _mac_already_configured(self, mac): + """See if we already have configured a NAS with this MAC address.""" + existing_macs = [ + mac.replace("-", "") + for entry in self._async_current_entries() + for mac in entry.data.get(CONF_MAC, []) + ] + return mac in existing_macs def _login_and_fetch_syno_info(api, otp_code): @@ -221,10 +220,11 @@ def _login_and_fetch_syno_info(api, otp_code): storage = api.storage if ( - api.information.serial is None + not api.information.serial or utilisation.cpu_user_load is None - or storage.disks_ids is None - or storage.volumes_ids is None + or not storage.disks_ids + or not storage.volumes_ids + or not api.network.macs ): raise InvalidData diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 66f752ffaf4..795348d900c 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -25,6 +25,7 @@ from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP, SOURCE_USER from homeassistant.const import ( CONF_DISKS, CONF_HOST, + CONF_MAC, CONF_PASSWORD, CONF_PORT, CONF_SSL, @@ -47,6 +48,8 @@ USERNAME = "Home_Assistant" PASSWORD = "password" DEVICE_TOKEN = "Dév!cè_T0k€ñ" +MACS = ["00-11-32-XX-XX-59", "00-11-32-XX-XX-5A"] + @pytest.fixture(name="service") def mock_controller_service(): @@ -56,8 +59,9 @@ def mock_controller_service(): ) as service_mock: 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 = [] + service_mock.return_value.storage.disks_ids = ["sda", "sdb", "sdc"] + service_mock.return_value.storage.volumes_ids = ["volume_1"] + service_mock.return_value.network.macs = MACS yield service_mock @@ -72,8 +76,9 @@ def mock_controller_service_2sa(): ) 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 = [] + service_mock.return_value.storage.disks_ids = ["sda", "sdb", "sdc"] + service_mock.return_value.storage.volumes_ids = ["volume_1"] + service_mock.return_value.network.macs = MACS yield service_mock @@ -85,8 +90,9 @@ def mock_controller_service_failed(): ) as service_mock: 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 + service_mock.return_value.storage.disks_ids = [] + service_mock.return_value.storage.volumes_ids = [] + service_mock.return_value.network.macs = [] yield service_mock @@ -118,6 +124,7 @@ async def test_user(hass: HomeAssistantType, service: MagicMock): assert result["data"][CONF_SSL] == SSL assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD + assert result["data"][CONF_MAC] == MACS assert result["data"].get("device_token") is None assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None @@ -142,6 +149,7 @@ async def test_user(hass: HomeAssistantType, service: MagicMock): assert not result["data"][CONF_SSL] assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD + assert result["data"][CONF_MAC] == MACS assert result["data"].get("device_token") is None assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None @@ -183,6 +191,7 @@ async def test_user_2sa(hass: HomeAssistantType, service_2sa: MagicMock): assert result["data"][CONF_SSL] == DEFAULT_SSL assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD + assert result["data"][CONF_MAC] == MACS assert result["data"].get("device_token") == DEVICE_TOKEN assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None @@ -204,6 +213,7 @@ async def test_import(hass: HomeAssistantType, service: MagicMock): assert result["data"][CONF_SSL] == DEFAULT_SSL assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD + assert result["data"][CONF_MAC] == MACS assert result["data"].get("device_token") is None assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None @@ -231,6 +241,7 @@ async def test_import(hass: HomeAssistantType, service: MagicMock): assert result["data"][CONF_SSL] == SSL assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD + assert result["data"][CONF_MAC] == MACS assert result["data"].get("device_token") is None assert result["data"][CONF_DISKS] == ["sda", "sdb", "sdc"] assert result["data"][CONF_VOLUMES] == ["volume_1"] @@ -329,8 +340,13 @@ async def test_form_ssdp_already_configured( MockConfigEntry( domain=DOMAIN, - data={CONF_HOST: HOST, CONF_USERNAME: USERNAME, CONF_PASSWORD: PASSWORD}, - unique_id=SERIAL.upper(), + data={ + CONF_HOST: HOST, + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + CONF_MAC: MACS, + }, + unique_id=SERIAL, ).add_to_hass(hass) result = await hass.config_entries.flow.async_init( @@ -339,7 +355,7 @@ async def test_form_ssdp_already_configured( data={ ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", - ssdp.ATTR_UPNP_SERIAL: SERIAL, + ssdp.ATTR_UPNP_SERIAL: "001132XXXX59", # Existing in MACS[0], but SSDP does not have `-` }, ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT @@ -355,7 +371,7 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): data={ ssdp.ATTR_SSDP_LOCATION: "http://192.168.1.5:5000", ssdp.ATTR_UPNP_FRIENDLY_NAME: "mydsm", - ssdp.ATTR_UPNP_SERIAL: SERIAL, + ssdp.ATTR_UPNP_SERIAL: "001132XXXX99", # MAC address, but SSDP does not have `-` }, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM @@ -373,6 +389,7 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock): assert result["data"][CONF_SSL] == DEFAULT_SSL assert result["data"][CONF_USERNAME] == USERNAME assert result["data"][CONF_PASSWORD] == PASSWORD + assert result["data"][CONF_MAC] == MACS assert result["data"].get("device_token") is None assert result["data"].get(CONF_DISKS) is None assert result["data"].get(CONF_VOLUMES) is None