From a97460d1abfe4bdd67416e0eb814c2d644d26a1b Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sun, 10 May 2020 04:52:08 +0200 Subject: [PATCH] Make upnp update interval configurable (#35298) * Simplification of upnp component * Make update interval configurable * Description * Require minimal value of 30 * Black * Linting Co-authored-by: Paulus Schoutsen --- homeassistant/components/upnp/config_flow.py | 31 +++++-- homeassistant/components/upnp/const.py | 2 + homeassistant/components/upnp/sensor.py | 16 +++- homeassistant/components/upnp/strings.json | 3 +- tests/components/upnp/test_config_flow.py | 95 ++++++++++++++++++-- 5 files changed, 130 insertions(+), 17 deletions(-) diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index 4701f21633c..237e97b98a8 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -5,10 +5,13 @@ import voluptuous as vol from homeassistant import config_entries from homeassistant.components import ssdp +from homeassistant.const import CONF_SCAN_INTERVAL from .const import ( # pylint: disable=unused-import + CONFIG_ENTRY_SCAN_INTERVAL, CONFIG_ENTRY_ST, CONFIG_ENTRY_UDN, + DEFAULT_SCAN_INTERVAL, DISCOVERY_LOCATION, DISCOVERY_NAME, DISCOVERY_ST, @@ -54,7 +57,9 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): await self.async_set_unique_id( discovery[DISCOVERY_USN], raise_on_progress=False ) - return await self._async_create_entry_from_data(discovery) + return await self._async_create_entry_from_discovery( + discovery, user_input[CONF_SCAN_INTERVAL] + ) # Discover devices. discoveries = await Device.async_discover(self.hass) @@ -82,6 +87,9 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): for discovery in self._discoveries } ), + vol.Optional( + CONF_SCAN_INTERVAL, default=DEFAULT_SCAN_INTERVAL, + ): vol.All(vol.Coerce(int), vol.Range(min=30)), } ) return self.async_show_form(step_id="user", data_schema=data_schema,) @@ -119,7 +127,9 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): return self.async_abort(reason="no_devices_found") discovery = self._discoveries[0] - return await self._async_create_entry_from_data(discovery) + return await self._async_create_entry_from_discovery( + discovery, DEFAULT_SCAN_INTERVAL + ) async def async_step_ssdp(self, discovery_info: Mapping): """Handle a discovered UPnP/IGD device. @@ -160,11 +170,19 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): return self.async_show_form(step_id="ssdp_confirm") discovery = self._discoveries[0] - return await self._async_create_entry_from_data(discovery) + return await self._async_create_entry_from_discovery( + discovery, DEFAULT_SCAN_INTERVAL + ) - async def _async_create_entry_from_data(self, discovery: Mapping): - """Create an entry from own _data.""" - _LOGGER.debug("_async_create_entry_from_data: discovery: %s", discovery) + async def _async_create_entry_from_discovery( + self, discovery: Mapping, scan_interval + ): + """Create an entry from discovery.""" + _LOGGER.debug( + "_async_create_entry_from_data: discovery: %s, scan_interval: %s", + discovery, + scan_interval, + ) # Get name from device, if not found already. if DISCOVERY_NAME not in discovery and DISCOVERY_LOCATION in discovery: discovery[DISCOVERY_NAME] = await self._async_get_name_for_discovery( @@ -175,6 +193,7 @@ class UpnpFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): data = { CONFIG_ENTRY_UDN: discovery[DISCOVERY_UDN], CONFIG_ENTRY_ST: discovery[DISCOVERY_ST], + CONFIG_ENTRY_SCAN_INTERVAL: scan_interval, } return self.async_create_entry(title=title, data=data) diff --git a/homeassistant/components/upnp/const.py b/homeassistant/components/upnp/const.py index 1673fb4c113..eb0844e2cb0 100644 --- a/homeassistant/components/upnp/const.py +++ b/homeassistant/components/upnp/const.py @@ -23,3 +23,5 @@ DISCOVERY_UDN = "udn" DISCOVERY_USN = "usn" CONFIG_ENTRY_UDN = "udn" CONFIG_ENTRY_ST = "st" +CONFIG_ENTRY_SCAN_INTERVAL = "scan_interval" +DEFAULT_SCAN_INTERVAL = timedelta(seconds=30).seconds diff --git a/homeassistant/components/upnp/sensor.py b/homeassistant/components/upnp/sensor.py index 5c356b53c8a..14ed442407f 100644 --- a/homeassistant/components/upnp/sensor.py +++ b/homeassistant/components/upnp/sensor.py @@ -12,15 +12,17 @@ from homeassistant.helpers.update_coordinator import DataUpdateCoordinator from .const import ( BYTES_RECEIVED, BYTES_SENT, + CONFIG_ENTRY_SCAN_INTERVAL, + CONFIG_ENTRY_UDN, DATA_PACKETS, DATA_RATE_PACKETS_PER_SECOND, + DEFAULT_SCAN_INTERVAL, DOMAIN, KIBIBYTE, LOGGER as _LOGGER, PACKETS_RECEIVED, PACKETS_SENT, TIMESTAMP, - UPDATE_INTERVAL, ) from .device import Device @@ -78,21 +80,24 @@ async def async_setup_entry( ) -> None: """Set up the UPnP/IGD sensors.""" data = config_entry.data - if "udn" in data: - udn = data["udn"] + if CONFIG_ENTRY_UDN in data: + udn = data[CONFIG_ENTRY_UDN] else: # any device will do udn = list(hass.data[DOMAIN]["devices"].keys())[0] device: Device = hass.data[DOMAIN]["devices"][udn] + update_interval_sec = data.get(CONFIG_ENTRY_SCAN_INTERVAL, DEFAULT_SCAN_INTERVAL) + update_interval = timedelta(seconds=update_interval_sec) + _LOGGER.debug("update_interval: %s", update_interval) _LOGGER.debug("Adding sensors") coordinator = DataUpdateCoordinator( hass, _LOGGER, name=device.name, update_method=device.async_get_traffic_data, - update_interval=timedelta(seconds=UPDATE_INTERVAL.seconds), + update_interval=update_interval, ) await coordinator.async_refresh() @@ -117,11 +122,14 @@ class UpnpSensor(Entity): coordinator: DataUpdateCoordinator, device: Device, sensor_type: Mapping[str, str], + update_multiplier: int = 2, ) -> None: """Initialize the base sensor.""" self._coordinator = coordinator self._device = device self._sensor_type = sensor_type + self._update_counter_max = update_multiplier + self._update_counter = 0 @property def should_poll(self) -> bool: diff --git a/homeassistant/components/upnp/strings.json b/homeassistant/components/upnp/strings.json index 9f2f6978341..8936d2ec791 100644 --- a/homeassistant/components/upnp/strings.json +++ b/homeassistant/components/upnp/strings.json @@ -9,7 +9,8 @@ }, "user": { "data": { - "usn": "Device" + "usn": "Device", + "scan_interval": "Update interval (seconds, minimal 30)" } } }, diff --git a/tests/components/upnp/test_config_flow.py b/tests/components/upnp/test_config_flow.py index c6e383bae55..fc9c8fe8cd5 100644 --- a/tests/components/upnp/test_config_flow.py +++ b/tests/components/upnp/test_config_flow.py @@ -1,8 +1,15 @@ """Test UPnP/IGD config flow.""" +import pytest +import voluptuous as vol + from homeassistant import config_entries, data_entry_flow from homeassistant.components import ssdp from homeassistant.components.upnp.const import ( + CONFIG_ENTRY_SCAN_INTERVAL, + CONFIG_ENTRY_ST, + CONFIG_ENTRY_UDN, + DEFAULT_SCAN_INTERVAL, DISCOVERY_LOCATION, DISCOVERY_ST, DISCOVERY_UDN, @@ -52,8 +59,9 @@ async def test_flow_ssdp_discovery(hass: HomeAssistantType): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == mock_device.name assert result["data"] == { - "st": mock_device.device_type, - "udn": mock_device.udn, + CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL, } @@ -89,11 +97,85 @@ async def test_flow_user(hass: HomeAssistantType): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == mock_device.name assert result["data"] == { - "st": mock_device.device_type, - "udn": mock_device.udn, + CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL, } +async def test_flow_user_update_interval(hass: HomeAssistantType): + """Test config flow: discovered + configured through user with non-default scan_interval.""" + udn = "uuid:device_1" + mock_device = MockDevice(udn) + usn = f"{mock_device.udn}::{mock_device.device_type}" + scan_interval = 60 + discovery_infos = [ + { + DISCOVERY_USN: usn, + DISCOVERY_ST: mock_device.device_type, + DISCOVERY_UDN: mock_device.udn, + DISCOVERY_LOCATION: "dummy", + } + ] + + with patch.object( + Device, "async_create_device", AsyncMock(return_value=mock_device) + ), patch.object(Device, "async_discover", AsyncMock(return_value=discovery_infos)): + # Discovered via step user. + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + + # Confirmed via step user. + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={"usn": usn, CONFIG_ENTRY_SCAN_INTERVAL: scan_interval}, + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == mock_device.name + assert result["data"] == { + CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_SCAN_INTERVAL: scan_interval, + } + + +async def test_flow_user_update_interval_min_30(hass: HomeAssistantType): + """Test config flow: discovered + configured through user with non-default scan_interval.""" + udn = "uuid:device_1" + mock_device = MockDevice(udn) + usn = f"{mock_device.udn}::{mock_device.device_type}" + scan_interval = 15 + discovery_infos = [ + { + DISCOVERY_USN: usn, + DISCOVERY_ST: mock_device.device_type, + DISCOVERY_UDN: mock_device.udn, + DISCOVERY_LOCATION: "dummy", + } + ] + + with patch.object( + Device, "async_create_device", AsyncMock(return_value=mock_device) + ), patch.object(Device, "async_discover", AsyncMock(return_value=discovery_infos)): + # Discovered via step user. + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + + # Confirmed via step user. + with pytest.raises(vol.error.MultipleInvalid): + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={"usn": usn, CONFIG_ENTRY_SCAN_INTERVAL: scan_interval}, + ) + + async def test_flow_config(hass: HomeAssistantType): """Test config flow: discovered + configured through configuration.yaml.""" udn = "uuid:device_1" @@ -119,6 +201,7 @@ async def test_flow_config(hass: HomeAssistantType): assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["title"] == mock_device.name assert result["data"] == { - "st": mock_device.device_type, - "udn": mock_device.udn, + CONFIG_ENTRY_ST: mock_device.device_type, + CONFIG_ENTRY_UDN: mock_device.udn, + CONFIG_ENTRY_SCAN_INTERVAL: DEFAULT_SCAN_INTERVAL, }