From 80f98b9ea21646e991ce50851af53cb8a0e2ddd4 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Thu, 16 Aug 2018 22:14:13 +0200 Subject: [PATCH 01/24] Fix message "Updating dlna_dmr media_player took longer than ..." --- homeassistant/components/media_player/dlna_dmr.py | 4 ++-- requirements_all.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/media_player/dlna_dmr.py b/homeassistant/components/media_player/dlna_dmr.py index 9b6beb83341..c40e3ed0ca9 100644 --- a/homeassistant/components/media_player/dlna_dmr.py +++ b/homeassistant/components/media_player/dlna_dmr.py @@ -35,7 +35,7 @@ from homeassistant.util import get_local_ip DLNA_DMR_DATA = 'dlna_dmr' REQUIREMENTS = [ - 'async-upnp-client==0.12.2', + 'async-upnp-client==0.12.3', ] DEFAULT_NAME = 'DLNA Digital Media Renderer' @@ -126,7 +126,7 @@ async def async_setup_platform(hass: HomeAssistant, name = config.get(CONF_NAME) elif discovery_info is not None: url = discovery_info['ssdp_description'] - name = discovery_info['name'] + name = discovery_info.get('name') if DLNA_DMR_DATA not in hass.data: hass.data[DLNA_DMR_DATA] = {} diff --git a/requirements_all.txt b/requirements_all.txt index 894e9ef1706..ed12752a934 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -138,7 +138,7 @@ apns2==0.3.0 asterisk_mbox==0.4.0 # homeassistant.components.media_player.dlna_dmr -async-upnp-client==0.12.2 +async-upnp-client==0.12.3 # homeassistant.components.light.avion # avion==0.7 From 71d00062e594e0305de259885227509349c33495 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Fri, 17 Aug 2018 21:25:08 +0200 Subject: [PATCH 02/24] Upgrade to async_upnp_client 0.12.4 --- homeassistant/components/media_player/dlna_dmr.py | 2 +- requirements_all.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/media_player/dlna_dmr.py b/homeassistant/components/media_player/dlna_dmr.py index c40e3ed0ca9..ebb1ab8d383 100644 --- a/homeassistant/components/media_player/dlna_dmr.py +++ b/homeassistant/components/media_player/dlna_dmr.py @@ -35,7 +35,7 @@ from homeassistant.util import get_local_ip DLNA_DMR_DATA = 'dlna_dmr' REQUIREMENTS = [ - 'async-upnp-client==0.12.3', + 'async-upnp-client==0.12.4', ] DEFAULT_NAME = 'DLNA Digital Media Renderer' diff --git a/requirements_all.txt b/requirements_all.txt index ed12752a934..9f0fe83b5ac 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -138,7 +138,7 @@ apns2==0.3.0 asterisk_mbox==0.4.0 # homeassistant.components.media_player.dlna_dmr -async-upnp-client==0.12.3 +async-upnp-client==0.12.4 # homeassistant.components.light.avion # avion==0.7 From 839b58c6001fd026af9c4b947e9dd3259d70e64a Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Fri, 17 Aug 2018 21:28:29 +0200 Subject: [PATCH 03/24] Working on IGD --- homeassistant/components/discovery.py | 1 + homeassistant/components/igd/__init__.py | 187 ++++++++++++++++++ homeassistant/components/igd/config_flow.py | 103 ++++++++++ homeassistant/components/igd/const.py | 5 + homeassistant/components/igd/strings.json | 26 +++ .../components/sensor/{upnp.py => igd.py} | 21 +- homeassistant/components/upnp.py | 140 ------------- homeassistant/config_entries.py | 2 + .../components/{test_upnp.py => test_igd.py} | 10 +- 9 files changed, 339 insertions(+), 156 deletions(-) create mode 100644 homeassistant/components/igd/__init__.py create mode 100644 homeassistant/components/igd/config_flow.py create mode 100644 homeassistant/components/igd/const.py create mode 100644 homeassistant/components/igd/strings.json rename homeassistant/components/sensor/{upnp.py => igd.py} (77%) delete mode 100644 homeassistant/components/upnp.py rename tests/components/{test_upnp.py => test_igd.py} (94%) diff --git a/homeassistant/components/discovery.py b/homeassistant/components/discovery.py index b400d1d8885..d686b114095 100644 --- a/homeassistant/components/discovery.py +++ b/homeassistant/components/discovery.py @@ -49,6 +49,7 @@ CONFIG_ENTRY_HANDLERS = { 'google_cast': 'cast', SERVICE_HUE: 'hue', 'sonos': 'sonos', + 'igd': 'igd', } SERVICE_HANDLERS = { diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py new file mode 100644 index 00000000000..18aa10e391d --- /dev/null +++ b/homeassistant/components/igd/__init__.py @@ -0,0 +1,187 @@ +""" +Will open a port in your router for Home Assistant and provide statistics. + +For more details about this component, please refer to the documentation at +https://home-assistant.io/components/upnp/ +""" +from ipaddress import ip_address +import aiohttp +import asyncio + +import voluptuous as vol + +from homeassistant import config_entries, data_entry_flow +from homeassistant.const import ( + CONF_URL, +) +from homeassistant.exceptions import PlatformNotReady +from homeassistant.helpers import config_validation as cv +from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.util import get_local_ip + +from .config_flow import configured_hosts +from .const import DOMAIN +from .const import LOGGER as _LOGGER + +_LOGGER.warning('Loading IGD') + + +REQUIREMENTS = ['async-upnp-client==0.12.3'] +DEPENDENCIES = ['http', 'api'] + +CONF_LOCAL_IP = 'local_ip' +CONF_ENABLE_PORT_MAPPING = 'port_mapping' +CONF_PORTS = 'ports' +CONF_UNITS = 'unit' +CONF_HASS = 'hass' + +NOTIFICATION_ID = 'igd_notification' +NOTIFICATION_TITLE = 'UPnP/IGD Setup' + +IP_SERVICE = 'urn:schemas-upnp-org:service:WANIPConnection:1' # XXX TODO: remove this + +UNITS = { + "Bytes": 1, + "KBytes": 1024, + "MBytes": 1024**2, + "GBytes": 1024**3, +} + +CONFIG_SCHEMA = vol.Schema({ + DOMAIN: vol.Schema({ + vol.Required(CONF_URL): cv.url, + vol.Optional(CONF_ENABLE_PORT_MAPPING, default=True): cv.boolean, + vol.Optional(CONF_UNITS, default="MBytes"): vol.In(UNITS), + vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), + vol.Optional(CONF_PORTS): + vol.Schema({vol.Any(CONF_HASS, cv.positive_int): cv.positive_int}) + }), +}, extra=vol.ALLOW_EXTRA) + + +async def async_setup(hass, config, *args, **kwargs): + """Register a port mapping for Home Assistant via UPnP.""" + conf = config.get(DOMAIN) + if conf is None: + conf = {} + + hass.data[DOMAIN] = {} + configured = configured_hosts(hass) + _LOGGER.debug('Config: %s', config) + _LOGGER.debug('configured: %s', configured) + + igds = [] + if not igds: + return True + + for igd_conf in igds: + hass.async_add_job(hass.config_entries.flow.async_init( + DOMAIN, context={'source': config_entries.SOURCE_IMPORT}, + data={ + 'ssdp_url': igd_conf['ssdp_url'], + } + )) + + return True + + # if host is None: + # host = get_local_ip() + # + # if host == '127.0.0.1': + # _LOGGER.error( + # 'Unable to determine local IP. Add it to your configuration.') + # return False + # + # url = config.get(CONF_URL) + # + # # build requester + # from async_upnp_client.aiohttp import AiohttpSessionRequester + # session = async_get_clientsession(hass) + # requester = AiohttpSessionRequester(session, True) + # + # # create upnp device + # from async_upnp_client import UpnpFactory + # factory = UpnpFactory(requester, disable_state_variable_validation=True) + # try: + # upnp_device = await factory.async_create_device(url) + # except (asyncio.TimeoutError, aiohttp.ClientError): + # raise PlatformNotReady() + # + # # wrap with IgdDevice + # from async_upnp_client.igd import IgdDevice + # igd_device = IgdDevice(upnp_device, None) + # hass.data[DATA_IGD]['device'] = igd_device + # + # # sensors + # unit = config.get(CONF_UNITS) + # hass.async_create_task(discovery.async_load_platform( + # hass, 'sensor', DOMAIN, {'unit': unit}, config)) + # + # # port mapping + # port_mapping = config.get(CONF_ENABLE_PORT_MAPPING) + # if not port_mapping: + # return True + # + # # determine ports + # internal_port = hass.http.server_port + # ports = config.get(CONF_PORTS) + # if ports is None: + # ports = {CONF_HASS: internal_port} + # + # registered = [] + # async def register_port_mappings(event): + # """(Re-)register the port mapping.""" + # from async_upnp_client import UpnpError + # for internal, external in ports.items(): + # if internal == CONF_HASS: + # internal = internal_port + # try: + # await igd_device.async_add_port_mapping(remote_host=None, + # external_port=external, + # protocol='TCP', + # internal_port=internal, + # internal_client=ip_address(host), + # enabled=True, + # description='Home Assistant', + # lease_duration=None) + # registered.append(external) + # _LOGGER.debug("external %s -> %s @ %s", external, internal, host) + # except UpnpError as error: + # _LOGGER.error(error) + # hass.components.persistent_notification.create( + # 'ERROR: TCP port {} is already mapped in your router.' + # '
Please disable port_mapping in the upnp ' + # 'configuration section.
' + # 'You will need to restart hass after fixing.' + # ''.format(external), + # title=NOTIFICATION_TITLE, + # notification_id=NOTIFICATION_ID) + # + # async def deregister_port_mappings(event): + # """De-register the port mapping.""" + # tasks = [igd_device.async_delete_port_mapping(remote_host=None, + # external_port=external, + # protocol='TCP') + # for external in registered] + # if tasks: + # await asyncio.wait(tasks) + # + # await register_port_mappings(None) + # hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, deregister_port_mappings) + # + # return True + + +async def async_setup_entry(hass, entry): + """Set up a bridge from a config entry.""" + _LOGGER.debug('async_setup_entry, title: %s, data: %s', entry.title, entry.data) + + # port mapping? + # sensors + + return True + +async def async_unload_entry(hass, entry): + """Unload a config entry.""" + + diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py new file mode 100644 index 00000000000..2eb1aae5b80 --- /dev/null +++ b/homeassistant/components/igd/config_flow.py @@ -0,0 +1,103 @@ +"""Config flow for IGD.""" +from homeassistant import config_entries, data_entry_flow +from homeassistant.core import callback +from homeassistant.helpers import config_validation as cv + +import voluptuous as vol + +from .const import DOMAIN +from .const import LOGGER as _LOGGER + + +@callback +def configured_hosts(hass): + """Return a set of the configured hosts.""" + return set(entry.data['ssdp_url'] + for entry in hass.config_entries.async_entries(DOMAIN)) + + +async def _get_igd_device(hass, ssdp_url): + """.""" + # build requester + from async_upnp_client.aiohttp import AiohttpSessionRequester + session = async_get_clientsession(hass) + requester = AiohttpSessionRequester(session, True) + + # create upnp device + from async_upnp_client import UpnpFactory + factory = UpnpFactory(requester, disable_state_variable_validation=True) + try: + upnp_device = await factory.async_create_device(ssdp_url) + except (asyncio.TimeoutError, aiohttp.ClientError): + raise PlatformNotReady() + + # wrap with IgdDevice + from async_upnp_client.igd import IgdDevice + igd_device = IgdDevice(upnp_device, None) + return igd_device + + +@config_entries.HANDLERS.register(DOMAIN) +class IgdFlowHandler(data_entry_flow.FlowHandler): + """Handle a Hue config flow.""" + + VERSION = 1 + + # def __init__(self): + # """Initializer.""" + # self.host = None + + # flow: 1. detection/user adding + # 2. question: port forward? sensors? + # 3. add it! + + async def async_step_user(self, user_input=None): + _LOGGER.debug('async_step_user: %s', user_input) + return await self.async_abort(reason='todo') + + async def async_step_discovery(self, discovery_info): + """Handle a discovered IGD. + + This flow is triggered by the discovery component. It will check if the + host is already configured and delegate to the import step if not. + """ + _LOGGER.debug('async_step_discovery: %s', discovery_info) + + ssdp_url = discovery_info['ssdp_description'] + return await self.async_step_options({ + 'ssdp_url': ssdp_url, + }) + + async def async_step_options(self, user_options): + """.""" + _LOGGER.debug('async_step_options: %s', user_options) + if user_options and \ + 'sensors' in user_options and \ + 'port_forward' in user_options: + return await self.async_step_import(user_options) + + return self.async_show_form( + step_id='options', + data_schema=vol.Schema({ + vol.Required('sensors'): cv.boolean, + vol.Required('port_forward'): cv.boolean, + # vol.Optional('ssdp_url', default=user_options['ssdp_url']): cv.url, + }) + ) + + async def async_step_import(self, import_info): + """Import a IGD as new entry.""" + _LOGGER.debug('async_step_import: %s', import_info) + + ssdp_url = import_info['ssdp_url'] + try: + igd_device = await _get_igd_device(self.hass, ssdp_url) # try it to see if it works + except: + pass + return self.async_create_entry( + title=igd_device.name, + data={ + 'ssdp_url': ssdp_url, + 'udn': igd_device.udn, + } + ) \ No newline at end of file diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/igd/const.py new file mode 100644 index 00000000000..a933497bd3e --- /dev/null +++ b/homeassistant/components/igd/const.py @@ -0,0 +1,5 @@ +"""Constants for the IGD component.""" +import logging + +DOMAIN = 'igd' +LOGGER = logging.getLogger('homeassistant.components.igd') diff --git a/homeassistant/components/igd/strings.json b/homeassistant/components/igd/strings.json new file mode 100644 index 00000000000..b88ed5c7968 --- /dev/null +++ b/homeassistant/components/igd/strings.json @@ -0,0 +1,26 @@ +{ + "config": { + "title": "IGD", + "step": { + "options": { + "title": "Extra configuration options for the IGD", + "data":{ + "sensors": "Add traffic in/out sensors", + "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!", + "ssdp_ur": "SSDP URL", + } + }, + "import": { + "title": "Link with IGD", + "description": "Setup the IGD" + } + }, + "error": { + }, + "abort": { + "already_configured": "IGD is already configured", + "no_igds": "No IGDs discovered", + "todo": "TODO" + } + } +} \ No newline at end of file diff --git a/homeassistant/components/sensor/upnp.py b/homeassistant/components/sensor/igd.py similarity index 77% rename from homeassistant/components/sensor/upnp.py rename to homeassistant/components/sensor/igd.py index 07b63553fcb..53692a3432f 100644 --- a/homeassistant/components/sensor/upnp.py +++ b/homeassistant/components/sensor/igd.py @@ -6,12 +6,12 @@ https://home-assistant.io/components/sensor.upnp/ """ import logging -from homeassistant.components.upnp import DATA_UPNP, UNITS, CIC_SERVICE +from homeassistant.components.igd import DATA_IGD, UNITS from homeassistant.helpers.entity import Entity _LOGGER = logging.getLogger(__name__) -DEPENDENCIES = ['upnp'] +DEPENDENCIES = ['igd'] BYTES_RECEIVED = 1 BYTES_SENT = 2 @@ -33,20 +33,19 @@ async def async_setup_platform(hass, config, async_add_devices, if discovery_info is None: return - device = hass.data[DATA_UPNP] - service = device.find_first_service(CIC_SERVICE) + device = hass.data[DATA_IGD]['device'] unit = discovery_info['unit'] async_add_devices([ - IGDSensor(service, t, unit if SENSOR_TYPES[t][1] else '#') + IGDSensor(device, t, unit if SENSOR_TYPES[t][1] else '#') for t in SENSOR_TYPES], True) class IGDSensor(Entity): """Representation of a UPnP IGD sensor.""" - def __init__(self, service, sensor_type, unit=None): + def __init__(self, device, sensor_type, unit=None): """Initialize the IGD sensor.""" - self._service = service + self._device = device self.type = sensor_type self.unit = unit self.unit_factor = UNITS[unit] if unit in UNITS else 1 @@ -78,10 +77,10 @@ class IGDSensor(Entity): async def async_update(self): """Get the latest information from the IGD.""" if self.type == BYTES_RECEIVED: - self._state = await self._service.get_total_bytes_received() + self._state = await self._device.async_get_total_bytes_received() elif self.type == BYTES_SENT: - self._state = await self._service.get_total_bytes_sent() + self._state = await self._device.async_get_total_bytes_sent() elif self.type == PACKETS_RECEIVED: - self._state = await self._service.get_total_packets_received() + self._state = await self._device.async_get_total_packets_received() elif self.type == PACKETS_SENT: - self._state = await self._service.get_total_packets_sent() + self._state = await self._device.async_get_total_packets_sent() diff --git a/homeassistant/components/upnp.py b/homeassistant/components/upnp.py deleted file mode 100644 index b4fe9d3fce9..00000000000 --- a/homeassistant/components/upnp.py +++ /dev/null @@ -1,140 +0,0 @@ -""" -Will open a port in your router for Home Assistant and provide statistics. - -For more details about this component, please refer to the documentation at -https://home-assistant.io/components/upnp/ -""" -from ipaddress import ip_address -import logging -import asyncio - -import voluptuous as vol - -from homeassistant.const import (EVENT_HOMEASSISTANT_STOP) -from homeassistant.helpers import config_validation as cv -from homeassistant.helpers import discovery -from homeassistant.util import get_local_ip - -REQUIREMENTS = ['pyupnp-async==0.1.0.2'] -DEPENDENCIES = ['http'] - -_LOGGER = logging.getLogger(__name__) - -DEPENDENCIES = ['api'] -DOMAIN = 'upnp' - -DATA_UPNP = 'upnp_device' - -CONF_LOCAL_IP = 'local_ip' -CONF_ENABLE_PORT_MAPPING = 'port_mapping' -CONF_PORTS = 'ports' -CONF_UNITS = 'unit' -CONF_HASS = 'hass' - -NOTIFICATION_ID = 'upnp_notification' -NOTIFICATION_TITLE = 'UPnP Setup' - -IGD_DEVICE = 'urn:schemas-upnp-org:device:InternetGatewayDevice:1' -PPP_SERVICE = 'urn:schemas-upnp-org:service:WANPPPConnection:1' -IP_SERVICE = 'urn:schemas-upnp-org:service:WANIPConnection:1' -CIC_SERVICE = 'urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1' - -UNITS = { - "Bytes": 1, - "KBytes": 1024, - "MBytes": 1024**2, - "GBytes": 1024**3, -} - -CONFIG_SCHEMA = vol.Schema({ - DOMAIN: vol.Schema({ - vol.Optional(CONF_ENABLE_PORT_MAPPING, default=True): cv.boolean, - vol.Optional(CONF_UNITS, default="MBytes"): vol.In(UNITS), - vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), - vol.Optional(CONF_PORTS): - vol.Schema({vol.Any(CONF_HASS, cv.positive_int): cv.positive_int}) - }), -}, extra=vol.ALLOW_EXTRA) - - -async def async_setup(hass, config): - """Register a port mapping for Home Assistant via UPnP.""" - config = config[DOMAIN] - host = config.get(CONF_LOCAL_IP) - - if host is None: - host = get_local_ip() - - if host == '127.0.0.1': - _LOGGER.error( - 'Unable to determine local IP. Add it to your configuration.') - return False - - import pyupnp_async - from pyupnp_async.error import UpnpSoapError - - service = None - resp = await pyupnp_async.msearch_first(search_target=IGD_DEVICE) - if not resp: - return False - - try: - device = await resp.get_device() - hass.data[DATA_UPNP] = device - for _service in device.services: - if _service['serviceType'] == PPP_SERVICE: - service = device.find_first_service(PPP_SERVICE) - if _service['serviceType'] == IP_SERVICE: - service = device.find_first_service(IP_SERVICE) - if _service['serviceType'] == CIC_SERVICE: - unit = config.get(CONF_UNITS) - hass.async_create_task(discovery.async_load_platform( - hass, 'sensor', DOMAIN, {'unit': unit}, config)) - except UpnpSoapError as error: - _LOGGER.error(error) - return False - - if not service: - _LOGGER.warning("Could not find any UPnP IGD") - return False - - port_mapping = config.get(CONF_ENABLE_PORT_MAPPING) - if not port_mapping: - return True - - internal_port = hass.http.server_port - - ports = config.get(CONF_PORTS) - if ports is None: - ports = {CONF_HASS: internal_port} - - registered = [] - for internal, external in ports.items(): - if internal == CONF_HASS: - internal = internal_port - try: - await service.add_port_mapping(internal, external, host, 'TCP', - desc='Home Assistant') - registered.append(external) - _LOGGER.debug("external %s -> %s @ %s", external, internal, host) - except UpnpSoapError as error: - _LOGGER.error(error) - hass.components.persistent_notification.create( - 'ERROR: tcp port {} is already mapped in your router.' - '
Please disable port_mapping in the upnp ' - 'configuration section.
' - 'You will need to restart hass after fixing.' - ''.format(external), - title=NOTIFICATION_TITLE, - notification_id=NOTIFICATION_ID) - - async def deregister_port(event): - """De-register the UPnP port mapping.""" - tasks = [service.delete_port_mapping(external, 'TCP') - for external in registered] - if tasks: - await asyncio.wait(tasks) - - hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, deregister_port) - - return True diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index b2e8389e449..f3b04f64e05 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -142,6 +142,7 @@ FLOWS = [ 'nest', 'sonos', 'zone', + 'igd', ] @@ -414,6 +415,7 @@ class ConfigEntries: Handler key is the domain of the component that we want to setup. """ component = getattr(self.hass.components, handler_key) + _LOGGER.debug('Handler key: %s', handler_key) handler = HANDLERS.get(handler_key) if handler is None: diff --git a/tests/components/test_upnp.py b/tests/components/test_igd.py similarity index 94% rename from tests/components/test_upnp.py rename to tests/components/test_igd.py index 4956b8a6278..87f992267c6 100644 --- a/tests/components/test_upnp.py +++ b/tests/components/test_igd.py @@ -6,7 +6,7 @@ import pytest from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.setup import async_setup_component -from homeassistant.components.upnp import IP_SERVICE, DATA_UPNP +from homeassistant.components.igd import IP_SERVICE, DATA_IGD class MockService(MagicMock): @@ -107,7 +107,7 @@ async def test_setup_succeeds_if_specify_ip(hass, mock_msearch_first): }) assert result - mock_service = hass.data[DATA_UPNP].peep_first_service() + mock_service = hass.data[DATA_IGD].peep_first_service() assert len(mock_service.mock_add_port_mapping.mock_calls) == 1 mock_service.mock_add_port_mapping.assert_called_once_with( 8123, 8123, '192.168.0.10', 'TCP', desc='Home Assistant') @@ -122,7 +122,7 @@ async def test_no_config_maps_hass_local_to_remote_port(hass, }) assert result - mock_service = hass.data[DATA_UPNP].peep_first_service() + mock_service = hass.data[DATA_IGD].peep_first_service() assert len(mock_service.mock_add_port_mapping.mock_calls) == 1 mock_service.mock_add_port_mapping.assert_called_once_with( 8123, 8123, '192.168.0.10', 'TCP', desc='Home Assistant') @@ -141,7 +141,7 @@ async def test_map_hass_to_remote_port(hass, }) assert result - mock_service = hass.data[DATA_UPNP].peep_first_service() + mock_service = hass.data[DATA_IGD].peep_first_service() assert len(mock_service.mock_add_port_mapping.mock_calls) == 1 mock_service.mock_add_port_mapping.assert_called_once_with( 8123, 1000, '192.168.0.10', 'TCP', desc='Home Assistant') @@ -162,7 +162,7 @@ async def test_map_internal_to_remote_ports(hass, }) assert result - mock_service = hass.data[DATA_UPNP].peep_first_service() + mock_service = hass.data[DATA_IGD].peep_first_service() assert len(mock_service.mock_add_port_mapping.mock_calls) == 2 mock_service.mock_add_port_mapping.assert_any_call( From 1eac6408f5b708188b94984195f0494c1306b36d Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Wed, 29 Aug 2018 21:19:04 +0200 Subject: [PATCH 04/24] Working on IGD --- .../components/igd/.translations/en.json | 29 ++ .../components/igd/.translations/nl.json | 29 ++ homeassistant/components/igd/__init__.py | 273 ++++++++++-------- homeassistant/components/igd/config_flow.py | 164 ++++++----- homeassistant/components/igd/const.py | 2 + homeassistant/components/igd/strings.json | 17 +- homeassistant/components/sensor/igd.py | 122 ++++++-- tests/components/igd/__init__.py | 1 + tests/components/igd/test_config_flow.py | 165 +++++++++++ tests/components/igd/test_init.py | 41 +++ tests/components/test_igd.py | 183 ------------ 11 files changed, 637 insertions(+), 389 deletions(-) create mode 100644 homeassistant/components/igd/.translations/en.json create mode 100644 homeassistant/components/igd/.translations/nl.json create mode 100644 tests/components/igd/__init__.py create mode 100644 tests/components/igd/test_config_flow.py create mode 100644 tests/components/igd/test_init.py delete mode 100644 tests/components/test_igd.py diff --git a/homeassistant/components/igd/.translations/en.json b/homeassistant/components/igd/.translations/en.json new file mode 100644 index 00000000000..bd0d2a9b7c0 --- /dev/null +++ b/homeassistant/components/igd/.translations/en.json @@ -0,0 +1,29 @@ +{ + "config": { + "title": "IGD", + "step": { + "init": { + "title": "IGD" + }, + "user": { + "title": "Configuration options for the IGD", + "data":{ + "sensors": "Add traffic in/out sensors", + "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!", + "ssdp_url": "SSDP URL", + "udn": "UDN", + "name": "Name" + } + } + }, + "error": { + }, + "abort": { + "no_devices_discovered": "No IGDs discovered", + "already_configured": "IGD is already configured", + "no_sensors_or_port_forward": "Enable at least sensors or Port forward", + "no_igds": "No IGDs discovered", + "todo": "TODO" + } + } +} \ No newline at end of file diff --git a/homeassistant/components/igd/.translations/nl.json b/homeassistant/components/igd/.translations/nl.json new file mode 100644 index 00000000000..06f9122678c --- /dev/null +++ b/homeassistant/components/igd/.translations/nl.json @@ -0,0 +1,29 @@ +{ + "config": { + "title": "IGD", + "step": { + "init": { + "title": "IGD" + }, + "user": { + "title": "Extra configuratie options voor IGD", + "data":{ + "sensors": "Verkeer in/out sensors", + "port_forward": "Maak port-forward voor Home Assistant\nZet dit alleen aan wanneer uw Home Assistant een wachtwoord heeft!", + "ssdp_url": "SSDP URL", + "udn": "UDN", + "name": "Naam" + } + } + }, + "error": { + }, + "abort": { + "no_devices_discovered": "Geen IGDs gevonden", + "already_configured": "IGD is reeds geconfigureerd", + "no_sensors_or_port_forward": "Kies ten minste sensors of Port forward", + "no_igds": "Geen IGDs gevonden", + "todo": "TODO" + } + } +} \ No newline at end of file diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 18aa10e391d..b932a4f8055 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -4,30 +4,49 @@ Will open a port in your router for Home Assistant and provide statistics. For more details about this component, please refer to the documentation at https://home-assistant.io/components/upnp/ """ +# XXX TODO: +# + flow: +# + discovery +# + adding device +# + removing device +# - configured: +# - adding +# - sensors: +# + adding +# + handle overflow +# - removing +# - port forward: +# - adding +# - removing +# - shutdown + + +from ipaddress import IPv4Address from ipaddress import ip_address import aiohttp import asyncio import voluptuous as vol -from homeassistant import config_entries, data_entry_flow -from homeassistant.const import ( - CONF_URL, -) +from homeassistant import config_entries +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import EVENT_HOMEASSISTANT_STOP +from homeassistant.const import CONF_URL from homeassistant.exceptions import PlatformNotReady from homeassistant.helpers import config_validation as cv +from homeassistant.helpers import discovery from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.typing import HomeAssistantType from homeassistant.util import get_local_ip -from .config_flow import configured_hosts +from .config_flow import configured_udns +from .const import CONF_PORT_FORWARD, CONF_SENSORS from .const import DOMAIN from .const import LOGGER as _LOGGER -_LOGGER.warning('Loading IGD') - -REQUIREMENTS = ['async-upnp-client==0.12.3'] -DEPENDENCIES = ['http', 'api'] +REQUIREMENTS = ['async-upnp-client==0.12.4'] +DEPENDENCIES = ['http'] CONF_LOCAL_IP = 'local_ip' CONF_ENABLE_PORT_MAPPING = 'port_mapping' @@ -38,8 +57,6 @@ CONF_HASS = 'hass' NOTIFICATION_ID = 'igd_notification' NOTIFICATION_TITLE = 'UPnP/IGD Setup' -IP_SERVICE = 'urn:schemas-upnp-org:service:WANIPConnection:1' # XXX TODO: remove this - UNITS = { "Bytes": 1, "KBytes": 1024, @@ -59,129 +76,157 @@ CONFIG_SCHEMA = vol.Schema({ }, extra=vol.ALLOW_EXTRA) -async def async_setup(hass, config, *args, **kwargs): - """Register a port mapping for Home Assistant via UPnP.""" - conf = config.get(DOMAIN) - if conf is None: - conf = {} +async def _async_create_igd_device(hass: HomeAssistantType, ssdp_description: str): + """.""" + # build requester + from async_upnp_client.aiohttp import AiohttpSessionRequester + session = async_get_clientsession(hass) + requester = AiohttpSessionRequester(session, True) - hass.data[DOMAIN] = {} - configured = configured_hosts(hass) - _LOGGER.debug('Config: %s', config) + # create upnp device + from async_upnp_client import UpnpFactory + factory = UpnpFactory(requester, disable_state_variable_validation=True) + try: + upnp_device = await factory.async_create_device(ssdp_description) + except (asyncio.TimeoutError, aiohttp.ClientError): + raise PlatformNotReady() + + # wrap with IgdDevice + from async_upnp_client.igd import IgdDevice + igd_device = IgdDevice(upnp_device, None) + return igd_device + + +def _store_device(hass: HomeAssistantType, udn, igd_device): + """Store an igd_device by udn.""" + hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) + hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) + hass.data[DOMAIN]['devices'][udn] = igd_device + + +def _get_device(hass: HomeAssistantType, udn): + """Get an igd_device by udn.""" + hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) + hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) + return hass.data[DOMAIN]['devices'][udn] + + +async def _async_create_port_forward(hass: HomeAssistantType, igd_device): + """Create a port forward.""" + _LOGGER.debug('Creating port forward: %s', igd_device) + + # determine local ip, ensure sane IP + local_ip = get_local_ip() + if local_ip == '127.0.0.1': + _LOGGER.warning('Could not create port forward, our IP is 127.0.0.1') + return False + local_ip = IPv4Address(local_ip) + + # create port mapping + port = hass.http.server_port + await igd_device.async_add_port_mapping(remote_host=None, + external_port=port, + protocol='TCP', + internal_port=port, + internal_client=local_ip, + enabled=True, + description="Home Assistant", + lease_duration=None) + + return True + + +async def _async_remove_port_forward(hass: HomeAssistantType, igd_device): + """Remove a port forward.""" + _LOGGER.debug('Removing port forward: %s', igd_device) + + # remove port mapping + port = hass.http.server_port + await igd_device.async_remove_port_mapping(remote_host=None, + external_port=port, + protocol='TCP') + + +# config +async def async_setup(hass: HomeAssistantType, config): + """Register a port mapping for Home Assistant via UPnP.""" + _LOGGER.debug('async_setup: config: %s', config) + conf = config.get(DOMAIN, {}) + + hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) + configured = configured_udns(hass) _LOGGER.debug('configured: %s', configured) - igds = [] - if not igds: - return True + # if no ssdp given: take any discovered - by flow - IGD entry + # if none discovered, raise PlatformNotReady + # if ssdp given: use the SSDP + igds = [] # XXX TODO for igd_conf in igds: hass.async_add_job(hass.config_entries.flow.async_init( DOMAIN, context={'source': config_entries.SOURCE_IMPORT}, data={ - 'ssdp_url': igd_conf['ssdp_url'], + 'ssdp_description': igd_conf['ssdp_description'], } )) return True - # if host is None: - # host = get_local_ip() - # - # if host == '127.0.0.1': - # _LOGGER.error( - # 'Unable to determine local IP. Add it to your configuration.') - # return False - # - # url = config.get(CONF_URL) - # - # # build requester - # from async_upnp_client.aiohttp import AiohttpSessionRequester - # session = async_get_clientsession(hass) - # requester = AiohttpSessionRequester(session, True) - # - # # create upnp device - # from async_upnp_client import UpnpFactory - # factory = UpnpFactory(requester, disable_state_variable_validation=True) - # try: - # upnp_device = await factory.async_create_device(url) - # except (asyncio.TimeoutError, aiohttp.ClientError): - # raise PlatformNotReady() - # - # # wrap with IgdDevice - # from async_upnp_client.igd import IgdDevice - # igd_device = IgdDevice(upnp_device, None) - # hass.data[DATA_IGD]['device'] = igd_device - # - # # sensors - # unit = config.get(CONF_UNITS) - # hass.async_create_task(discovery.async_load_platform( - # hass, 'sensor', DOMAIN, {'unit': unit}, config)) - # - # # port mapping - # port_mapping = config.get(CONF_ENABLE_PORT_MAPPING) - # if not port_mapping: - # return True - # - # # determine ports - # internal_port = hass.http.server_port - # ports = config.get(CONF_PORTS) - # if ports is None: - # ports = {CONF_HASS: internal_port} - # - # registered = [] - # async def register_port_mappings(event): - # """(Re-)register the port mapping.""" - # from async_upnp_client import UpnpError - # for internal, external in ports.items(): - # if internal == CONF_HASS: - # internal = internal_port - # try: - # await igd_device.async_add_port_mapping(remote_host=None, - # external_port=external, - # protocol='TCP', - # internal_port=internal, - # internal_client=ip_address(host), - # enabled=True, - # description='Home Assistant', - # lease_duration=None) - # registered.append(external) - # _LOGGER.debug("external %s -> %s @ %s", external, internal, host) - # except UpnpError as error: - # _LOGGER.error(error) - # hass.components.persistent_notification.create( - # 'ERROR: TCP port {} is already mapped in your router.' - # '
Please disable port_mapping in the upnp ' - # 'configuration section.
' - # 'You will need to restart hass after fixing.' - # ''.format(external), - # title=NOTIFICATION_TITLE, - # notification_id=NOTIFICATION_ID) - # - # async def deregister_port_mappings(event): - # """De-register the port mapping.""" - # tasks = [igd_device.async_delete_port_mapping(remote_host=None, - # external_port=external, - # protocol='TCP') - # for external in registered] - # if tasks: - # await asyncio.wait(tasks) - # - # await register_port_mappings(None) - # hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, deregister_port_mappings) - # - # return True - -async def async_setup_entry(hass, entry): +# config flow +async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Set up a bridge from a config entry.""" - _LOGGER.debug('async_setup_entry, title: %s, data: %s', entry.title, entry.data) + _LOGGER.debug('async_setup_entry: title: %s, data: %s', config_entry.title, config_entry.data) + + data = config_entry.data + ssdp_description = data['ssdp_description'] + + # build IGD device + try: + igd_device = await _async_create_igd_device(hass, ssdp_description) + except (asyncio.TimeoutError, aiohttp.ClientError): + raise PlatformNotReady() + + _store_device(hass, igd_device.udn, igd_device) + + # port forward + if data.get(CONF_PORT_FORWARD): + await _async_create_port_forward(hass, igd_device) - # port mapping? # sensors + if data.get(CONF_SENSORS): + discovery_info = { + 'unit': 'MBytes', + 'udn': data['udn'], + } + hass_config = config_entry.data + hass.async_create_task(discovery.async_load_platform( + hass, 'sensor', DOMAIN, discovery_info, hass_config)) + + async def unload_entry(event): + """Unload entry on quit.""" + await async_unload_entry(hass, config_entry) + + hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, unload_entry) return True -async def async_unload_entry(hass, entry): + +async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Unload a config entry.""" + _LOGGER.debug('async_unload_entry: title: %s, data: %s', config_entry.title, config_entry.data) + data = config_entry.data + udn = data['udn'] + igd_device = _get_device(hass, udn) + # port forward + if data.get(CONF_PORT_FORWARD): + _LOGGER.debug('Removing port forward for: %s', igd_device) + _async_remove_port_forward(hass, igd_device) + # sensors + if data.get(CONF_SENSORS): + # XXX TODO: remove sensors + pass + + return True diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index 2eb1aae5b80..9ccbe79a35f 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -1,40 +1,19 @@ """Config flow for IGD.""" from homeassistant import config_entries, data_entry_flow from homeassistant.core import callback -from homeassistant.helpers import config_validation as cv import voluptuous as vol from .const import DOMAIN from .const import LOGGER as _LOGGER - @callback -def configured_hosts(hass): - """Return a set of the configured hosts.""" - return set(entry.data['ssdp_url'] - for entry in hass.config_entries.async_entries(DOMAIN)) - - -async def _get_igd_device(hass, ssdp_url): - """.""" - # build requester - from async_upnp_client.aiohttp import AiohttpSessionRequester - session = async_get_clientsession(hass) - requester = AiohttpSessionRequester(session, True) - - # create upnp device - from async_upnp_client import UpnpFactory - factory = UpnpFactory(requester, disable_state_variable_validation=True) - try: - upnp_device = await factory.async_create_device(ssdp_url) - except (asyncio.TimeoutError, aiohttp.ClientError): - raise PlatformNotReady() - - # wrap with IgdDevice - from async_upnp_client.igd import IgdDevice - igd_device = IgdDevice(upnp_device, None) - return igd_device +def configured_udns(hass): + """Get all configured UDNs.""" + return [ + entry.data['udn'] + for entry in hass.config_entries.async_entries(DOMAIN) + ] @config_entries.HANDLERS.register(DOMAIN) @@ -43,61 +22,114 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): VERSION = 1 - # def __init__(self): - # """Initializer.""" - # self.host = None + def __init__(self): + """Initializer.""" + pass - # flow: 1. detection/user adding - # 2. question: port forward? sensors? - # 3. add it! + @property + def _discovereds(self): + """Get all discovered entries.""" + if DOMAIN not in self.hass.data: + _LOGGER.debug('DOMAIN not in hass.data') + if 'discovered' not in self.hass.data.get(DOMAIN, {}): + _LOGGER.debug('discovered not in hass.data[DOMAIN]') - async def async_step_user(self, user_input=None): - _LOGGER.debug('async_step_user: %s', user_input) - return await self.async_abort(reason='todo') + return self.hass.data.get(DOMAIN, {}).get('discovered', {}) + + def _store_discovery_info(self, discovery_info): + """Add discovery info.""" + udn = discovery_info['udn'] + if DOMAIN not in self.hass.data: + _LOGGER.debug('DOMAIN not in hass.data') + self.hass.data[DOMAIN] = self.hass.data.get(DOMAIN, {}) + if 'discovered' not in self.hass.data[DOMAIN]: + _LOGGER.debug('Creating new discovered: %s', self.hass.data[DOMAIN]) + self.hass.data[DOMAIN]['discovered'] = self.hass.data[DOMAIN].get('discovered', {}) + self.hass.data[DOMAIN]['discovered'][udn] = discovery_info async def async_step_discovery(self, discovery_info): - """Handle a discovered IGD. + """ + Handle a discovered IGD. This flow is triggered by the discovery component. It will check if the host is already configured and delegate to the import step if not. """ - _LOGGER.debug('async_step_discovery: %s', discovery_info) + _LOGGER.debug('async_step_discovery %s: %s', id(self), discovery_info) - ssdp_url = discovery_info['ssdp_description'] - return await self.async_step_options({ - 'ssdp_url': ssdp_url, - }) + # ensure not already discovered/configured + udn = discovery_info['udn'] + if udn in configured_udns(self.hass): + _LOGGER.debug('Already configured: %s', discovery_info) + return self.async_abort(reason='already_configured') - async def async_step_options(self, user_options): - """.""" - _LOGGER.debug('async_step_options: %s', user_options) - if user_options and \ - 'sensors' in user_options and \ - 'port_forward' in user_options: - return await self.async_step_import(user_options) + # store discovered device + self._store_discovery_info(discovery_info) + + # abort --> not showing up in discovered things + # return self.async_abort(reason='user_input_required') + + # user -> showing up in discovered things + return await self.async_step_user() + + async def async_step_user(self, user_input=None): + """Manual set up.""" + _LOGGER.debug('async_step_user %s: %s', id(self), user_input) + + # if user input given, handle it + user_input = user_input or {} + if 'igd_host' in user_input: + if not user_input['sensors'] and not user_input['port_forward']: + _LOGGER.debug('Aborting, no sensors and no portforward') + return self.async_abort(reason='no_sensors_or_port_forward') + + configured_hosts = [ + entry['host'] + for entry in self._discovereds.values() + if entry['udn'] in configured_udns(self.hass) + ] + if user_input['igd_host'] in configured_hosts: + return self.async_abort(reason='already_configured') + + return await self._async_save(user_input) + + # let user choose from all discovered IGDs + _LOGGER.debug('Discovered devices: %s', self._discovereds) + igd_hosts = [ + entry['host'] + for entry in self._discovereds.values() + if entry['udn'] not in configured_udns(self.hass) + ] + if not igd_hosts: + return self.async_abort(reason='no_devices_discovered') return self.async_show_form( - step_id='options', + step_id='user', data_schema=vol.Schema({ - vol.Required('sensors'): cv.boolean, - vol.Required('port_forward'): cv.boolean, - # vol.Optional('ssdp_url', default=user_options['ssdp_url']): cv.url, + vol.Required('igd_host'): vol.In(igd_hosts), + vol.Required('sensors'): bool, + vol.Required('port_forward'): bool, }) ) - async def async_step_import(self, import_info): - """Import a IGD as new entry.""" - _LOGGER.debug('async_step_import: %s', import_info) + async def _async_save(self, import_info): + """Store IGD as new entry.""" + _LOGGER.debug('async_step_import %s: %s', id(self), import_info) + + # ensure we know the host + igd_host = import_info['igd_host'] + discovery_infos = [info + for info in self._discovereds.values() + if info['host'] == igd_host] + if not discovery_infos: + return self.async_abort(reason='host_not_found') + discovery_info = discovery_infos[0] - ssdp_url = import_info['ssdp_url'] - try: - igd_device = await _get_igd_device(self.hass, ssdp_url) # try it to see if it works - except: - pass return self.async_create_entry( - title=igd_device.name, + title=discovery_info['name'], data={ - 'ssdp_url': ssdp_url, - 'udn': igd_device.udn, - } - ) \ No newline at end of file + 'ssdp_description': discovery_info['ssdp_description'], + 'udn': discovery_info['udn'], + 'sensors': import_info['sensors'], + 'port_forward': import_info['port_forward'], + }, + ) diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/igd/const.py index a933497bd3e..d1d92f7ccb5 100644 --- a/homeassistant/components/igd/const.py +++ b/homeassistant/components/igd/const.py @@ -3,3 +3,5 @@ import logging DOMAIN = 'igd' LOGGER = logging.getLogger('homeassistant.components.igd') +CONF_PORT_FORWARD = 'port_forward' +CONF_SENSORS = 'sensors' diff --git a/homeassistant/components/igd/strings.json b/homeassistant/components/igd/strings.json index b88ed5c7968..bd0d2a9b7c0 100644 --- a/homeassistant/components/igd/strings.json +++ b/homeassistant/components/igd/strings.json @@ -2,23 +2,26 @@ "config": { "title": "IGD", "step": { - "options": { - "title": "Extra configuration options for the IGD", + "init": { + "title": "IGD" + }, + "user": { + "title": "Configuration options for the IGD", "data":{ "sensors": "Add traffic in/out sensors", "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!", - "ssdp_ur": "SSDP URL", + "ssdp_url": "SSDP URL", + "udn": "UDN", + "name": "Name" } - }, - "import": { - "title": "Link with IGD", - "description": "Setup the IGD" } }, "error": { }, "abort": { + "no_devices_discovered": "No IGDs discovered", "already_configured": "IGD is already configured", + "no_sensors_or_port_forward": "Enable at least sensors or Port forward", "no_igds": "No IGDs discovered", "todo": "TODO" } diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/igd.py index 53692a3432f..1d0402520df 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/igd.py @@ -6,26 +6,29 @@ https://home-assistant.io/components/sensor.upnp/ """ import logging -from homeassistant.components.igd import DATA_IGD, UNITS +from homeassistant.components import history +from homeassistant.components.igd import DOMAIN, UNITS from homeassistant.helpers.entity import Entity _LOGGER = logging.getLogger(__name__) -DEPENDENCIES = ['igd'] +DEPENDENCIES = ['igd', 'history'] -BYTES_RECEIVED = 1 -BYTES_SENT = 2 -PACKETS_RECEIVED = 3 -PACKETS_SENT = 4 +BYTES_RECEIVED = 'bytes_received' +BYTES_SENT = 'bytes_sent' +PACKETS_RECEIVED = 'packets_received' +PACKETS_SENT = 'packets_sent' # sensor_type: [friendly_name, convert_unit, icon] SENSOR_TYPES = { - BYTES_RECEIVED: ['received bytes', True, 'mdi:server-network'], - BYTES_SENT: ['sent bytes', True, 'mdi:server-network'], - PACKETS_RECEIVED: ['packets received', False, 'mdi:server-network'], - PACKETS_SENT: ['packets sent', False, 'mdi:server-network'], + BYTES_RECEIVED: ['bytes received', True, 'mdi:server-network', float], + BYTES_SENT: ['bytes sent', True, 'mdi:server-network', float], + PACKETS_RECEIVED: ['packets received', False, 'mdi:server-network', int], + PACKETS_SENT: ['packets sent', False, 'mdi:server-network', int], } +OVERFLOW_AT = 2**32 + async def async_setup_platform(hass, config, async_add_devices, discovery_info=None): @@ -33,11 +36,12 @@ async def async_setup_platform(hass, config, async_add_devices, if discovery_info is None: return - device = hass.data[DATA_IGD]['device'] + udn = discovery_info['udn'] + device = hass.data[DOMAIN]['devices'][udn] unit = discovery_info['unit'] async_add_devices([ IGDSensor(device, t, unit if SENSOR_TYPES[t][1] else '#') - for t in SENSOR_TYPES], True) + for t in SENSOR_TYPES]) class IGDSensor(Entity): @@ -51,6 +55,7 @@ class IGDSensor(Entity): self.unit_factor = UNITS[unit] if unit in UNITS else 1 self._name = 'IGD {}'.format(SENSOR_TYPES[sensor_type][0]) self._state = None + self._last_value = None @property def name(self): @@ -60,9 +65,14 @@ class IGDSensor(Entity): @property def state(self): """Return the state of the device.""" - if self._state: - return format(float(self._state) / self.unit_factor, '.1f') - return self._state + if self._state is None: + return None + + coercer = SENSOR_TYPES[self.type][3] + if coercer == int: + return format(self._state) + + return format(self._state / self.unit_factor, '.1f') @property def icon(self): @@ -76,11 +86,85 @@ class IGDSensor(Entity): async def async_update(self): """Get the latest information from the IGD.""" + new_value = 0 if self.type == BYTES_RECEIVED: - self._state = await self._device.async_get_total_bytes_received() + new_value = await self._device.async_get_total_bytes_received() elif self.type == BYTES_SENT: - self._state = await self._device.async_get_total_bytes_sent() + new_value = await self._device.async_get_total_bytes_sent() elif self.type == PACKETS_RECEIVED: - self._state = await self._device.async_get_total_packets_received() + new_value = await self._device.async_get_total_packets_received() elif self.type == PACKETS_SENT: - self._state = await self._device.async_get_total_packets_sent() + new_value = await self._device.async_get_total_packets_sent() + + self._handle_new_value(new_value) + + # _LOGGER.debug('Removing self: %s', self) + # await self.async_remove() # XXX TODO: does not remove from the UI + + @property + def _last_state(self): + """Get the last state reported to hass.""" + states = history.get_last_state_changes(self.hass, 2, self.entity_id) + entity_states = [ + state for state in states[self.entity_id] + if state.state != 'unknown'] + _LOGGER.debug('%s: entity_states: %s', self.entity_id, entity_states) + if not entity_states: + return None + + return entity_states[0] + + @property + def _last_value_from_state(self): + """Get the last value reported to hass.""" + last_state = self._last_state + if not last_state: + _LOGGER.debug('%s: No last state', self.entity_id) + return None + + coercer = SENSOR_TYPES[self.type][3] + try: + state = coercer(float(last_state.state)) * self.unit_factor + except ValueError: + _LOGGER.debug('%s: value error, coercer: %s, state: %s', self.entity_id, coercer, last_state.state) + raise + state = coercer(0.0) + + return state + + def _handle_new_value(self, new_value): + _LOGGER.debug('%s: handle_new_value: state: %s, new_value: %s, last_value: %s', + self.entity_id, self._state, new_value, self._last_value) + + # ❯❯❯ upnp-client --debug --pprint --device http://192.168.178.1/RootDevice.xml call-action WANCIFC/GetTotalBytesReceived + if self.entity_id is None: + # don't know our entity ID yet, do nothing but store value + self._last_value = new_value + return + + if self._last_value is None: + self._last_value = new_value + + if self._state is None: + # try to get the state from history + self._state = self._last_value_from_state or 0 + + _LOGGER.debug('%s: state: %s, last_value: %s', + self.entity_id, self._state, self._last_value) + + # calculate new state + if self._last_value <= new_value: + diff = new_value - self._last_value + else: + # handle overflow + diff = OVERFLOW_AT - self._last_value + if new_value >= 0: + diff += new_value + else: + # some devices don't overflow and start at 0, but somewhere to -2**32 + diff += new_value - -OVERFLOW_AT + + self._state += diff + self._last_value = new_value + _LOGGER.debug('%s: diff: %s, state: %s, last_value: %s', + self.entity_id, diff, self._state, self._last_value) diff --git a/tests/components/igd/__init__.py b/tests/components/igd/__init__.py new file mode 100644 index 00000000000..d6985e0544b --- /dev/null +++ b/tests/components/igd/__init__.py @@ -0,0 +1 @@ +"""Tests for the IGD component.""" \ No newline at end of file diff --git a/tests/components/igd/test_config_flow.py b/tests/components/igd/test_config_flow.py new file mode 100644 index 00000000000..2fe2c60b19d --- /dev/null +++ b/tests/components/igd/test_config_flow.py @@ -0,0 +1,165 @@ +"""Tests for IGD config flow.""" + +from homeassistant.components import igd + +from tests.common import MockConfigEntry + + +async def test_flow_none_discovered(hass): + """Test no device discovered flow.""" + flow = igd.config_flow.IgdFlowHandler() + flow.hass = hass + + result = await flow.async_step_user() + assert result['type'] == 'abort' + assert result['reason'] == 'no_devices_discovered' + + +async def test_flow_already_configured(hass): + """Test device already configured flow.""" + flow = igd.config_flow.IgdFlowHandler() + flow.hass = hass + + # discovered device + udn = 'uuid:device_1' + hass.data[igd.DOMAIN] = { + 'discovered': { + udn: { + 'host': '192.168.1.1', + 'udn': udn, + }, + }, + } + + # configured entry + MockConfigEntry(domain=igd.DOMAIN, data={ + 'udn': udn, + 'host': '192.168.1.1', + }).add_to_hass(hass) + + result = await flow.async_step_user({ + 'igd_host': '192.168.1.1', + 'sensors': True, + 'port_forward': False, + }) + assert result['type'] == 'abort' + assert result['reason'] == 'already_configured' + + +async def test_flow_no_sensors_no_port_forward(hass): + """Test single device, no sensors, no port_forward.""" + flow = igd.config_flow.IgdFlowHandler() + flow.hass = hass + + # discovered device + udn = 'uuid:device_1' + hass.data[igd.DOMAIN] = { + 'discovered': { + udn: { + 'host': '192.168.1.1', + 'udn': udn, + }, + }, + } + + # configured entry + MockConfigEntry(domain=igd.DOMAIN, data={ + 'udn': udn, + 'host': '192.168.1.1', + }).add_to_hass(hass) + + result = await flow.async_step_user({ + 'igd_host': '192.168.1.1', + 'sensors': False, + 'port_forward': False, + }) + assert result['type'] == 'abort' + assert result['reason'] == 'no_sensors_or_port_forward' + + +async def test_flow_discovered_form(hass): + """Test single device discovered, show form flow.""" + flow = igd.config_flow.IgdFlowHandler() + flow.hass = hass + + # discovered device + udn = 'uuid:device_1' + hass.data[igd.DOMAIN] = { + 'discovered': { + udn: { + 'host': '192.168.1.1', + 'udn': udn, + }, + }, + } + + result = await flow.async_step_user() + assert result['type'] == 'form' + assert result['step_id'] == 'user' + + +async def test_flow_two_discovered_form(hass): + """Test single device discovered, show form flow.""" + flow = igd.config_flow.IgdFlowHandler() + flow.hass = hass + + # discovered device + udn_1 = 'uuid:device_1' + udn_2 = 'uuid:device_2' + hass.data[igd.DOMAIN] = { + 'discovered': { + udn_1: { + 'host': '192.168.1.1', + 'udn': udn_1, + }, + udn_2: { + 'host': '192.168.2.1', + 'udn': udn_2, + }, + }, + } + + result = await flow.async_step_user() + assert result['type'] == 'form' + assert result['step_id'] == 'user' + assert result['data_schema']({ + 'igd_host': '192.168.1.1', + 'sensors': True, + 'port_forward': False, + }) + assert result['data_schema']({ + 'igd_host': '192.168.2.1', + 'sensors': True, + 'port_forward': False, + }) + + +async def test_config_entry_created(hass): + flow = igd.config_flow.IgdFlowHandler() + flow.hass = hass + + # discovered device + udn = 'uuid:device_1' + hass.data[igd.DOMAIN] = { + 'discovered': { + udn: { + 'name': 'Test device 1', + 'host': '192.168.1.1', + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': udn, + }, + }, + } + + result = await flow.async_step_user({ + 'igd_host': '192.168.1.1', + 'sensors': True, + 'port_forward': False, + }) + assert result['data'] == { + 'port_forward': False, + 'sensors': True, + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': 'uuid:device_1', + } + assert result['title'] == 'Test device 1' diff --git a/tests/components/igd/test_init.py b/tests/components/igd/test_init.py new file mode 100644 index 00000000000..4405cc10999 --- /dev/null +++ b/tests/components/igd/test_init.py @@ -0,0 +1,41 @@ +"""Test IGD setup process.""" + +from unittest.mock import patch, MagicMock + +from homeassistant.setup import async_setup_component +from homeassistant.components import igd +from homeassistant.const import EVENT_HOMEASSISTANT_STOP + +from tests.common import MockConfigEntry +from tests.common import mock_coro + + +async def test_async_setup_entry_port_forward_created(hass): + """Test async_setup_entry.""" + + udn = 'uuid:device_1' + entry = MockConfigEntry(domain=igd.DOMAIN, data={ + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': udn, + 'sensors': False, + 'port_forward': True, + }) + + # ensure hass.http is available + await async_setup_component(hass, 'igd') + + mock_igd_device = MagicMock() + mock_igd_device.udn = udn + mock_igd_device.async_add_port_mapping.return_value = mock_coro() + mock_igd_device.async_remove_port_mapping.return_value = mock_coro() + with patch.object(igd, '_async_create_igd_device') as mock_create_device: + mock_create_device.return_value = mock_coro(return_value=mock_igd_device) + with patch('homeassistant.components.igd.get_local_ip', return_value='192.168.1.10'): + assert await igd.async_setup_entry(hass, entry) is True + + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) + await hass.async_block_till_done() + + assert hass.data[igd.DOMAIN]['devices'][udn] == mock_igd_device + assert len(mock_igd_device.async_add_port_mapping.mock_calls) > 0 + assert len(mock_igd_device.async_delete_port_mapping.mock_calls) > 0 diff --git a/tests/components/test_igd.py b/tests/components/test_igd.py deleted file mode 100644 index eb42b3127f6..00000000000 --- a/tests/components/test_igd.py +++ /dev/null @@ -1,183 +0,0 @@ -"""Test the UPNP component.""" -from collections import OrderedDict -from unittest.mock import patch, MagicMock - -import pytest - -from homeassistant.const import EVENT_HOMEASSISTANT_STOP -from homeassistant.setup import async_setup_component -from homeassistant.components.igd import IP_SERVICE, DATA_IGD - - -class MockService(MagicMock): - """Mock upnp IP service.""" - - async def add_port_mapping(self, *args, **kwargs): - """Original function.""" - self.mock_add_port_mapping(*args, **kwargs) - - async def delete_port_mapping(self, *args, **kwargs): - """Original function.""" - self.mock_delete_port_mapping(*args, **kwargs) - - -class MockDevice(MagicMock): - """Mock upnp device.""" - - def find_first_service(self, *args, **kwargs): - """Original function.""" - self._service = MockService() - return self._service - - def peep_first_service(self): - """Access Mock first service.""" - return self._service - - -class MockResp(MagicMock): - """Mock upnp msearch response.""" - - async def get_device(self, *args, **kwargs): - """Original function.""" - device = MockDevice() - service = {'serviceType': IP_SERVICE} - device.services = [service] - return device - - -@pytest.fixture -def mock_msearch_first(*args, **kwargs): - """Wrap async mock msearch_first.""" - async def async_mock_msearch_first(*args, **kwargs): - """Mock msearch_first.""" - return MockResp(*args, **kwargs) - - with patch('pyupnp_async.msearch_first', new=async_mock_msearch_first): - yield - - -@pytest.fixture -def mock_async_exception(*args, **kwargs): - """Wrap async mock exception.""" - async def async_mock_exception(*args, **kwargs): - return Exception - - with patch('pyupnp_async.msearch_first', new=async_mock_exception): - yield - - -@pytest.fixture -def mock_local_ip(): - """Mock get_local_ip.""" - with patch('homeassistant.components.upnp.get_local_ip', - return_value='192.168.0.10'): - yield - - -async def test_setup_fail_if_no_ip(hass): - """Test setup fails if we can't find a local IP.""" - with patch('homeassistant.components.upnp.get_local_ip', - return_value='127.0.0.1'): - result = await async_setup_component(hass, 'upnp', { - 'upnp': {} - }) - - assert not result - - -async def test_setup_fail_if_cannot_select_igd(hass, - mock_local_ip, - mock_async_exception): - """Test setup fails if we can't find an UPnP IGD.""" - result = await async_setup_component(hass, 'upnp', { - 'upnp': {} - }) - - assert not result - - -async def test_setup_succeeds_if_specify_ip(hass, mock_msearch_first): - """Test setup succeeds if we specify IP and can't find a local IP.""" - with patch('homeassistant.components.upnp.get_local_ip', - return_value='127.0.0.1'): - result = await async_setup_component(hass, 'upnp', { - 'upnp': { - 'local_ip': '192.168.0.10', - 'port_mapping': 'True' - } - }) - - assert result - mock_service = hass.data[DATA_IGD].peep_first_service() - assert len(mock_service.mock_add_port_mapping.mock_calls) == 1 - mock_service.mock_add_port_mapping.assert_called_once_with( - 8123, 8123, '192.168.0.10', 'TCP', desc='Home Assistant') - - -async def test_no_config_maps_hass_local_to_remote_port(hass, - mock_local_ip, - mock_msearch_first): - """Test by default we map local to remote port.""" - result = await async_setup_component(hass, 'upnp', { - 'upnp': { - 'port_mapping': 'True' - } - }) - - assert result - mock_service = hass.data[DATA_IGD].peep_first_service() - assert len(mock_service.mock_add_port_mapping.mock_calls) == 1 - mock_service.mock_add_port_mapping.assert_called_once_with( - 8123, 8123, '192.168.0.10', 'TCP', desc='Home Assistant') - - -async def test_map_hass_to_remote_port(hass, - mock_local_ip, - mock_msearch_first): - """Test mapping hass to remote port.""" - result = await async_setup_component(hass, 'upnp', { - 'upnp': { - 'port_mapping': 'True', - 'ports': { - 'hass': 1000 - } - } - }) - - assert result - mock_service = hass.data[DATA_IGD].peep_first_service() - assert len(mock_service.mock_add_port_mapping.mock_calls) == 1 - mock_service.mock_add_port_mapping.assert_called_once_with( - 8123, 1000, '192.168.0.10', 'TCP', desc='Home Assistant') - - -async def test_map_internal_to_remote_ports(hass, - mock_local_ip, - mock_msearch_first): - """Test mapping local to remote ports.""" - ports = OrderedDict() - ports['hass'] = 1000 - ports[1883] = 3883 - - result = await async_setup_component(hass, 'upnp', { - 'upnp': { - 'port_mapping': 'True', - 'ports': ports - } - }) - - assert result - mock_service = hass.data[DATA_IGD].peep_first_service() - assert len(mock_service.mock_add_port_mapping.mock_calls) == 2 - - mock_service.mock_add_port_mapping.assert_any_call( - 8123, 1000, '192.168.0.10', 'TCP', desc='Home Assistant') - mock_service.mock_add_port_mapping.assert_any_call( - 1883, 3883, '192.168.0.10', 'TCP', desc='Home Assistant') - - hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) - await hass.async_block_till_done() - assert len(mock_service.mock_delete_port_mapping.mock_calls) == 2 - - mock_service.mock_delete_port_mapping.assert_any_call(1000, 'TCP') - mock_service.mock_delete_port_mapping.assert_any_call(3883, 'TCP') From 20879726b0b4fe37af926ca1423c5f7cd43cb986 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Thu, 30 Aug 2018 16:38:43 +0200 Subject: [PATCH 05/24] Working on IGD --- homeassistant/components/igd/__init__.py | 147 ++++++++++---------- homeassistant/components/igd/config_flow.py | 60 ++++---- homeassistant/components/igd/const.py | 4 +- homeassistant/components/sensor/igd.py | 18 +-- requirements_all.txt | 4 +- requirements_test_all.txt | 3 - tests/components/igd/__init__.py | 2 +- tests/components/igd/test_config_flow.py | 84 +++++++++-- tests/components/igd/test_init.py | 104 +++++++++++++- 9 files changed, 284 insertions(+), 142 deletions(-) diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index b932a4f8055..191b8c1fb72 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -4,52 +4,34 @@ Will open a port in your router for Home Assistant and provide statistics. For more details about this component, please refer to the documentation at https://home-assistant.io/components/upnp/ """ -# XXX TODO: -# + flow: -# + discovery -# + adding device -# + removing device -# - configured: -# - adding -# - sensors: -# + adding -# + handle overflow -# - removing -# - port forward: -# - adding -# - removing -# - shutdown +import asyncio from ipaddress import IPv4Address from ipaddress import ip_address -import aiohttp -import asyncio +import aiohttp import voluptuous as vol -from homeassistant import config_entries from homeassistant.config_entries import ConfigEntry from homeassistant.const import EVENT_HOMEASSISTANT_STOP -from homeassistant.const import CONF_URL from homeassistant.exceptions import PlatformNotReady from homeassistant.helpers import config_validation as cv from homeassistant.helpers import discovery from homeassistant.helpers.aiohttp_client import async_get_clientsession -from homeassistant.helpers.typing import HomeAssistantType +from homeassistant.helpers.typing import ConfigType, HomeAssistantType from homeassistant.util import get_local_ip +from homeassistant.components.discovery import DOMAIN as DISCOVERY_DOMAIN -from .config_flow import configured_udns -from .const import CONF_PORT_FORWARD, CONF_SENSORS +from .const import CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS from .const import DOMAIN from .const import LOGGER as _LOGGER REQUIREMENTS = ['async-upnp-client==0.12.4'] -DEPENDENCIES = ['http'] +DEPENDENCIES = ['http'] # ,'discovery'] CONF_LOCAL_IP = 'local_ip' -CONF_ENABLE_PORT_MAPPING = 'port_mapping' CONF_PORTS = 'ports' CONF_UNITS = 'unit' CONF_HASS = 'hass' @@ -66,17 +48,16 @@ UNITS = { CONFIG_SCHEMA = vol.Schema({ DOMAIN: vol.Schema({ - vol.Required(CONF_URL): cv.url, - vol.Optional(CONF_ENABLE_PORT_MAPPING, default=True): cv.boolean, - vol.Optional(CONF_UNITS, default="MBytes"): vol.In(UNITS), + vol.Optional(CONF_ENABLE_PORT_MAPPING, default=False): cv.boolean, + vol.Optional(CONF_ENABLE_SENSORS, default=True): cv.boolean, vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), - vol.Optional(CONF_PORTS): - vol.Schema({vol.Any(CONF_HASS, cv.positive_int): cv.positive_int}) + vol.Optional(CONF_UNITS, default="MBytes"): vol.In(UNITS), }), }, extra=vol.ALLOW_EXTRA) -async def _async_create_igd_device(hass: HomeAssistantType, ssdp_description: str): +async def _async_create_igd_device(hass: HomeAssistantType, + ssdp_description: str): """.""" # build requester from async_upnp_client.aiohttp import AiohttpSessionRequester @@ -111,19 +92,22 @@ def _get_device(hass: HomeAssistantType, udn): return hass.data[DOMAIN]['devices'][udn] -async def _async_create_port_forward(hass: HomeAssistantType, igd_device): - """Create a port forward.""" - _LOGGER.debug('Creating port forward: %s', igd_device) - +async def _async_add_port_mapping(hass: HomeAssistantType, + igd_device, + local_ip=None): + """Create a port mapping.""" # determine local ip, ensure sane IP - local_ip = get_local_ip() + if local_ip is None: + local_ip = get_local_ip() + if local_ip == '127.0.0.1': - _LOGGER.warning('Could not create port forward, our IP is 127.0.0.1') + _LOGGER.warning('Could not create port mapping, our IP is 127.0.0.1') return False local_ip = IPv4Address(local_ip) # create port mapping port = hass.http.server_port + _LOGGER.debug('Creating port mapping %s:%s:%s (TCP)', port, local_ip, port) await igd_device.async_add_port_mapping(remote_host=None, external_port=port, protocol='TCP', @@ -136,48 +120,52 @@ async def _async_create_port_forward(hass: HomeAssistantType, igd_device): return True -async def _async_remove_port_forward(hass: HomeAssistantType, igd_device): - """Remove a port forward.""" - _LOGGER.debug('Removing port forward: %s', igd_device) - - # remove port mapping +async def _async_delete_port_mapping(hass: HomeAssistantType, igd_device): + """Remove a port mapping.""" port = hass.http.server_port - await igd_device.async_remove_port_mapping(remote_host=None, + await igd_device.async_delete_port_mapping(remote_host=None, external_port=port, protocol='TCP') # config -async def async_setup(hass: HomeAssistantType, config): +async def async_setup(hass: HomeAssistantType, config: ConfigType): """Register a port mapping for Home Assistant via UPnP.""" - _LOGGER.debug('async_setup: config: %s', config) - conf = config.get(DOMAIN, {}) + # defaults + hass.data[DOMAIN] = { + 'auto_config': { + 'active': False, + 'port_forward': False, + 'sensors': False, + } + } - hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) - configured = configured_udns(hass) - _LOGGER.debug('configured: %s', configured) + # ensure sane config + if DOMAIN not in config: + return False - # if no ssdp given: take any discovered - by flow - IGD entry - # if none discovered, raise PlatformNotReady - # if ssdp given: use the SSDP + if DISCOVERY_DOMAIN not in config: + _LOGGER.warning('IGD needs discovery, please enable it') + return False - igds = [] # XXX TODO - for igd_conf in igds: - hass.async_add_job(hass.config_entries.flow.async_init( - DOMAIN, context={'source': config_entries.SOURCE_IMPORT}, - data={ - 'ssdp_description': igd_conf['ssdp_description'], - } - )) + igd_config = config[DOMAIN] + if CONF_LOCAL_IP in igd_config: + hass.data[DOMAIN]['local_ip'] = igd_config[CONF_LOCAL_IP] + + hass.data[DOMAIN]['auto_config'] = { + 'active': True, + 'port_forward': igd_config[CONF_ENABLE_PORT_MAPPING], + 'sensors': igd_config[CONF_ENABLE_SENSORS], + } + _LOGGER.debug('Enabled auto_config: %s', hass.data[DOMAIN]['auto_config']) return True # config flow -async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): +async def async_setup_entry(hass: HomeAssistantType, + config_entry: ConfigEntry): """Set up a bridge from a config entry.""" - _LOGGER.debug('async_setup_entry: title: %s, data: %s', config_entry.title, config_entry.data) - data = config_entry.data ssdp_description = data['ssdp_description'] @@ -189,44 +177,49 @@ async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): _store_device(hass, igd_device.udn, igd_device) - # port forward - if data.get(CONF_PORT_FORWARD): - await _async_create_port_forward(hass, igd_device) + # port mapping + if data.get(CONF_ENABLE_PORT_MAPPING): + local_ip = hass.data[DOMAIN].get('local_ip') + await _async_add_port_mapping(hass, igd_device, local_ip=local_ip) # sensors - if data.get(CONF_SENSORS): + if data.get(CONF_ENABLE_SENSORS): discovery_info = { 'unit': 'MBytes', 'udn': data['udn'], } hass_config = config_entry.data - hass.async_create_task(discovery.async_load_platform( - hass, 'sensor', DOMAIN, discovery_info, hass_config)) + hass.async_create_task( + discovery.async_load_platform( + hass, 'sensor', DOMAIN, discovery_info, hass_config)) async def unload_entry(event): """Unload entry on quit.""" await async_unload_entry(hass, config_entry) - hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, unload_entry) return True -async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): +async def async_unload_entry(hass: HomeAssistantType, + config_entry: ConfigEntry): """Unload a config entry.""" - _LOGGER.debug('async_unload_entry: title: %s, data: %s', config_entry.title, config_entry.data) data = config_entry.data udn = data['udn'] - igd_device = _get_device(hass, udn) - # port forward - if data.get(CONF_PORT_FORWARD): - _LOGGER.debug('Removing port forward for: %s', igd_device) - _async_remove_port_forward(hass, igd_device) + igd_device = _get_device(hass, udn) + if igd_device is None: + return True + + # port mapping + if data.get(CONF_ENABLE_PORT_MAPPING): + await _async_delete_port_mapping(hass, igd_device) # sensors - if data.get(CONF_SENSORS): + if data.get(CONF_ENABLE_SENSORS): # XXX TODO: remove sensors pass + _store_device(hass, udn, None) + return True diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index 9ccbe79a35f..be6cf9f5f93 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -1,11 +1,12 @@ """Config flow for IGD.""" +import voluptuous as vol + from homeassistant import config_entries, data_entry_flow from homeassistant.core import callback -import voluptuous as vol - +from .const import CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS from .const import DOMAIN -from .const import LOGGER as _LOGGER + @callback def configured_udns(hass): @@ -29,24 +30,23 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): @property def _discovereds(self): """Get all discovered entries.""" - if DOMAIN not in self.hass.data: - _LOGGER.debug('DOMAIN not in hass.data') - if 'discovered' not in self.hass.data.get(DOMAIN, {}): - _LOGGER.debug('discovered not in hass.data[DOMAIN]') - return self.hass.data.get(DOMAIN, {}).get('discovered', {}) def _store_discovery_info(self, discovery_info): """Add discovery info.""" udn = discovery_info['udn'] - if DOMAIN not in self.hass.data: - _LOGGER.debug('DOMAIN not in hass.data') self.hass.data[DOMAIN] = self.hass.data.get(DOMAIN, {}) - if 'discovered' not in self.hass.data[DOMAIN]: - _LOGGER.debug('Creating new discovered: %s', self.hass.data[DOMAIN]) - self.hass.data[DOMAIN]['discovered'] = self.hass.data[DOMAIN].get('discovered', {}) + self.hass.data[DOMAIN]['discovered'] = \ + self.hass.data[DOMAIN].get('discovered', {}) self.hass.data[DOMAIN]['discovered'][udn] = discovery_info + def _auto_config_settings(self): + """Check if auto_config has been enabled.""" + self.hass.data[DOMAIN] = self.hass.data.get(DOMAIN, {}) + return self.hass.data[DOMAIN].get('auto_config', { + 'active': False, + }) + async def async_step_discovery(self, discovery_info): """ Handle a discovered IGD. @@ -54,32 +54,33 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): This flow is triggered by the discovery component. It will check if the host is already configured and delegate to the import step if not. """ - _LOGGER.debug('async_step_discovery %s: %s', id(self), discovery_info) - # ensure not already discovered/configured udn = discovery_info['udn'] if udn in configured_udns(self.hass): - _LOGGER.debug('Already configured: %s', discovery_info) return self.async_abort(reason='already_configured') # store discovered device self._store_discovery_info(discovery_info) - # abort --> not showing up in discovered things - # return self.async_abort(reason='user_input_required') + # auto config? + auto_config = self._auto_config_settings() + if auto_config['active']: + import_info = { + 'igd_host': discovery_info['host'], + 'sensors': auto_config['sensors'], + 'port_forward': auto_config['port_forward'], + } + + return await self._async_save_entry(import_info) - # user -> showing up in discovered things return await self.async_step_user() async def async_step_user(self, user_input=None): """Manual set up.""" - _LOGGER.debug('async_step_user %s: %s', id(self), user_input) - # if user input given, handle it user_input = user_input or {} if 'igd_host' in user_input: if not user_input['sensors'] and not user_input['port_forward']: - _LOGGER.debug('Aborting, no sensors and no portforward') return self.async_abort(reason='no_sensors_or_port_forward') configured_hosts = [ @@ -90,10 +91,9 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): if user_input['igd_host'] in configured_hosts: return self.async_abort(reason='already_configured') - return await self._async_save(user_input) + return await self._async_save_entry(user_input) # let user choose from all discovered IGDs - _LOGGER.debug('Discovered devices: %s', self._discovereds) igd_hosts = [ entry['host'] for entry in self._discovereds.values() @@ -111,10 +111,12 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): }) ) - async def _async_save(self, import_info): - """Store IGD as new entry.""" - _LOGGER.debug('async_step_import %s: %s', id(self), import_info) + async def async_step_import(self, import_info): + """Import a new IGD as a config entry.""" + return await self._async_save_entry(import_info) + async def _async_save_entry(self, import_info): + """Store IGD as new entry.""" # ensure we know the host igd_host = import_info['igd_host'] discovery_infos = [info @@ -129,7 +131,7 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): data={ 'ssdp_description': discovery_info['ssdp_description'], 'udn': discovery_info['udn'], - 'sensors': import_info['sensors'], - 'port_forward': import_info['port_forward'], + CONF_ENABLE_SENSORS: import_info['sensors'], + CONF_ENABLE_PORT_MAPPING: import_info['port_forward'], }, ) diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/igd/const.py index d1d92f7ccb5..c849e627757 100644 --- a/homeassistant/components/igd/const.py +++ b/homeassistant/components/igd/const.py @@ -3,5 +3,5 @@ import logging DOMAIN = 'igd' LOGGER = logging.getLogger('homeassistant.components.igd') -CONF_PORT_FORWARD = 'port_forward' -CONF_SENSORS = 'sensors' +CONF_ENABLE_PORT_MAPPING = 'port_forward' +CONF_ENABLE_SENSORS = 'sensors' diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/igd.py index 1d0402520df..edd936600e3 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/igd.py @@ -1,15 +1,17 @@ """ -Support for UPnP Sensors (IGD). +Support for IGD Sensors. For more details about this platform, please refer to the documentation at -https://home-assistant.io/components/sensor.upnp/ +https://home-assistant.io/components/sensor.igd/ """ +# pylint: disable=invalid-name import logging from homeassistant.components import history from homeassistant.components.igd import DOMAIN, UNITS from homeassistant.helpers.entity import Entity + _LOGGER = logging.getLogger(__name__) DEPENDENCIES = ['igd', 'history'] @@ -98,9 +100,6 @@ class IGDSensor(Entity): self._handle_new_value(new_value) - # _LOGGER.debug('Removing self: %s', self) - # await self.async_remove() # XXX TODO: does not remove from the UI - @property def _last_state(self): """Get the last state reported to hass.""" @@ -126,17 +125,11 @@ class IGDSensor(Entity): try: state = coercer(float(last_state.state)) * self.unit_factor except ValueError: - _LOGGER.debug('%s: value error, coercer: %s, state: %s', self.entity_id, coercer, last_state.state) - raise state = coercer(0.0) return state def _handle_new_value(self, new_value): - _LOGGER.debug('%s: handle_new_value: state: %s, new_value: %s, last_value: %s', - self.entity_id, self._state, new_value, self._last_value) - - # ❯❯❯ upnp-client --debug --pprint --device http://192.168.178.1/RootDevice.xml call-action WANCIFC/GetTotalBytesReceived if self.entity_id is None: # don't know our entity ID yet, do nothing but store value self._last_value = new_value @@ -161,7 +154,8 @@ class IGDSensor(Entity): if new_value >= 0: diff += new_value else: - # some devices don't overflow and start at 0, but somewhere to -2**32 + # some devices don't overflow and start at 0, + # but somewhere to -2**32 diff += new_value - -OVERFLOW_AT self._state += diff diff --git a/requirements_all.txt b/requirements_all.txt index ef89fb096da..97336bbae50 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -141,6 +141,7 @@ apns2==0.3.0 # homeassistant.components.asterisk_mbox asterisk_mbox==0.4.0 +# homeassistant.components.igd # homeassistant.components.media_player.dlna_dmr async-upnp-client==0.12.4 @@ -1183,9 +1184,6 @@ pytrafikverket==0.1.5.8 # homeassistant.components.device_tracker.unifi pyunifi==2.13 -# homeassistant.components.upnp -pyupnp-async==0.1.1.1 - # homeassistant.components.binary_sensor.uptimerobot pyuptimerobot==0.0.5 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 2415d661e29..bba6b5ca46a 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -177,9 +177,6 @@ pytradfri[async]==5.5.1 # homeassistant.components.device_tracker.unifi pyunifi==2.13 -# homeassistant.components.upnp -pyupnp-async==0.1.1.1 - # homeassistant.components.notify.html5 pywebpush==1.6.0 diff --git a/tests/components/igd/__init__.py b/tests/components/igd/__init__.py index d6985e0544b..4fcc4167e5b 100644 --- a/tests/components/igd/__init__.py +++ b/tests/components/igd/__init__.py @@ -1 +1 @@ -"""Tests for the IGD component.""" \ No newline at end of file +"""Tests for the IGD component.""" diff --git a/tests/components/igd/test_config_flow.py b/tests/components/igd/test_config_flow.py index 2fe2c60b19d..5530e172dae 100644 --- a/tests/components/igd/test_config_flow.py +++ b/tests/components/igd/test_config_flow.py @@ -1,13 +1,14 @@ """Tests for IGD config flow.""" from homeassistant.components import igd +from homeassistant.components.igd import config_flow as igd_config_flow from tests.common import MockConfigEntry async def test_flow_none_discovered(hass): """Test no device discovered flow.""" - flow = igd.config_flow.IgdFlowHandler() + flow = igd_config_flow.IgdFlowHandler() flow.hass = hass result = await flow.async_step_user() @@ -17,7 +18,7 @@ async def test_flow_none_discovered(hass): async def test_flow_already_configured(hass): """Test device already configured flow.""" - flow = igd.config_flow.IgdFlowHandler() + flow = igd_config_flow.IgdFlowHandler() flow.hass = hass # discovered device @@ -48,7 +49,7 @@ async def test_flow_already_configured(hass): async def test_flow_no_sensors_no_port_forward(hass): """Test single device, no sensors, no port_forward.""" - flow = igd.config_flow.IgdFlowHandler() + flow = igd_config_flow.IgdFlowHandler() flow.hass = hass # discovered device @@ -79,7 +80,7 @@ async def test_flow_no_sensors_no_port_forward(hass): async def test_flow_discovered_form(hass): """Test single device discovered, show form flow.""" - flow = igd.config_flow.IgdFlowHandler() + flow = igd_config_flow.IgdFlowHandler() flow.hass = hass # discovered device @@ -100,7 +101,7 @@ async def test_flow_discovered_form(hass): async def test_flow_two_discovered_form(hass): """Test single device discovered, show form flow.""" - flow = igd.config_flow.IgdFlowHandler() + flow = igd_config_flow.IgdFlowHandler() flow.hass = hass # discovered device @@ -135,18 +136,18 @@ async def test_flow_two_discovered_form(hass): async def test_config_entry_created(hass): - flow = igd.config_flow.IgdFlowHandler() + """Test config entry is created.""" + flow = igd_config_flow.IgdFlowHandler() flow.hass = hass # discovered device - udn = 'uuid:device_1' hass.data[igd.DOMAIN] = { 'discovered': { - udn: { + 'uuid:device_1': { 'name': 'Test device 1', 'host': '192.168.1.1', 'ssdp_description': 'http://192.168.1.1/desc.xml', - 'udn': udn, + 'udn': 'uuid:device_1', }, }, } @@ -156,6 +157,7 @@ async def test_config_entry_created(hass): 'sensors': True, 'port_forward': False, }) + assert result['type'] == 'create_entry' assert result['data'] == { 'port_forward': False, 'sensors': True, @@ -163,3 +165,67 @@ async def test_config_entry_created(hass): 'udn': 'uuid:device_1', } assert result['title'] == 'Test device 1' + + +async def test_flow_discovery_auto_config_sensors(hass): + """Test creation of device with auto_config.""" + flow = igd_config_flow.IgdFlowHandler() + flow.hass = hass + + # auto_config active + hass.data[igd.DOMAIN] = { + 'auto_config': { + 'active': True, + 'port_forward': False, + 'sensors': True, + }, + } + + # discovered device + result = await flow.async_step_discovery({ + 'name': 'Test device 1', + 'host': '192.168.1.1', + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': 'uuid:device_1', + }) + + assert result['type'] == 'create_entry' + assert result['data'] == { + 'port_forward': False, + 'sensors': True, + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': 'uuid:device_1', + } + assert result['title'] == 'Test device 1' + + +async def test_flow_discovery_auto_config_sensors_port_forward(hass): + """Test creation of device with auto_config, with port forward.""" + flow = igd_config_flow.IgdFlowHandler() + flow.hass = hass + + # auto_config active, with port_forward + hass.data[igd.DOMAIN] = { + 'auto_config': { + 'active': True, + 'port_forward': True, + 'sensors': True, + }, + } + + # discovered device + result = await flow.async_step_discovery({ + 'name': 'Test device 1', + 'host': '192.168.1.1', + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': 'uuid:device_1', + }) + + assert result['type'] == 'create_entry' + assert result['data'] == { + 'port_forward': True, + 'sensors': True, + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': 'uuid:device_1', + } + assert result['title'] == 'Test device 1' diff --git a/tests/components/igd/test_init.py b/tests/components/igd/test_init.py index 4405cc10999..285755c687f 100644 --- a/tests/components/igd/test_init.py +++ b/tests/components/igd/test_init.py @@ -10,9 +10,96 @@ from tests.common import MockConfigEntry from tests.common import mock_coro -async def test_async_setup_entry_port_forward_created(hass): - """Test async_setup_entry.""" +async def test_async_setup_no_auto_config(hass): + """Test async_setup.""" + # setup component, enable auto_config + await async_setup_component(hass, 'igd') + assert hass.data[igd.DOMAIN]['auto_config'] == { + 'active': False, + 'port_forward': False, + 'sensors': False, + } + + +async def test_async_setup_auto_config(hass): + """Test async_setup.""" + # setup component, enable auto_config + await async_setup_component(hass, 'igd', {'igd': {}, 'discovery': {}}) + + assert hass.data[igd.DOMAIN]['auto_config'] == { + 'active': True, + 'port_forward': False, + 'sensors': True, + } + + +async def test_async_setup_auto_config_port_forward(hass): + """Test async_setup.""" + # setup component, enable auto_config + await async_setup_component(hass, 'igd', { + 'igd': {'port_forward': True}, + 'discovery': {}}) + + assert hass.data[igd.DOMAIN]['auto_config'] == { + 'active': True, + 'port_forward': True, + 'sensors': True, + } + + +async def test_async_setup_auto_config_no_sensors(hass): + """Test async_setup.""" + # setup component, enable auto_config + await async_setup_component(hass, 'igd', { + 'igd': {'sensors': False}, + 'discovery': {}}) + + assert hass.data[igd.DOMAIN]['auto_config'] == { + 'active': True, + 'port_forward': False, + 'sensors': False, + } + + +async def test_async_setup_entry_default(hass): + """Test async_setup_entry.""" + udn = 'uuid:device_1' + entry = MockConfigEntry(domain=igd.DOMAIN, data={ + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'udn': udn, + 'sensors': True, + 'port_forward': False, + }) + + # ensure hass.http is available + await async_setup_component(hass, 'igd') + + # mock async_upnp_client.igd.IgdDevice + mock_igd_device = MagicMock() + mock_igd_device.udn = udn + mock_igd_device.async_add_port_mapping.return_value = mock_coro() + mock_igd_device.async_delete_port_mapping.return_value = mock_coro() + with patch.object(igd, '_async_create_igd_device') as mock_create_device: + mock_create_device.return_value = mock_coro( + return_value=mock_igd_device) + with patch('homeassistant.components.igd.get_local_ip', + return_value='192.168.1.10'): + assert await igd.async_setup_entry(hass, entry) is True + + # ensure device is stored/used + assert hass.data[igd.DOMAIN]['devices'][udn] == mock_igd_device + + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) + await hass.async_block_till_done() + + assert hass.data[igd.DOMAIN]['devices'][udn] is None + assert len(mock_igd_device.async_add_port_mapping.mock_calls) == 0 + assert len(mock_igd_device.async_delete_port_mapping.mock_calls) == 0 + + +async def test_async_setup_entry_port_forward(hass): + """Test async_setup_entry.""" udn = 'uuid:device_1' entry = MockConfigEntry(domain=igd.DOMAIN, data={ 'ssdp_description': 'http://192.168.1.1/desc.xml', @@ -27,15 +114,20 @@ async def test_async_setup_entry_port_forward_created(hass): mock_igd_device = MagicMock() mock_igd_device.udn = udn mock_igd_device.async_add_port_mapping.return_value = mock_coro() - mock_igd_device.async_remove_port_mapping.return_value = mock_coro() + mock_igd_device.async_delete_port_mapping.return_value = mock_coro() with patch.object(igd, '_async_create_igd_device') as mock_create_device: - mock_create_device.return_value = mock_coro(return_value=mock_igd_device) - with patch('homeassistant.components.igd.get_local_ip', return_value='192.168.1.10'): + mock_create_device.return_value = mock_coro( + return_value=mock_igd_device) + with patch('homeassistant.components.igd.get_local_ip', + return_value='192.168.1.10'): assert await igd.async_setup_entry(hass, entry) is True + # ensure device is stored/used + assert hass.data[igd.DOMAIN]['devices'][udn] == mock_igd_device + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done() - assert hass.data[igd.DOMAIN]['devices'][udn] == mock_igd_device + assert hass.data[igd.DOMAIN]['devices'][udn] is None assert len(mock_igd_device.async_add_port_mapping.mock_calls) > 0 assert len(mock_igd_device.async_delete_port_mapping.mock_calls) > 0 From 38318ed15ff3886c7e734cd7c922261367cad91e Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sat, 1 Sep 2018 10:09:22 +0200 Subject: [PATCH 06/24] Ensure config_flow is imported --- homeassistant/components/igd/__init__.py | 1 + homeassistant/config_entries.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 191b8c1fb72..305ac993b87 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -26,6 +26,7 @@ from homeassistant.components.discovery import DOMAIN as DISCOVERY_DOMAIN from .const import CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS from .const import DOMAIN from .const import LOGGER as _LOGGER +import homeassistant.components.igd.config_flow # register the handler REQUIREMENTS = ['async-upnp-client==0.12.4'] diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index a52484b6941..93c904d3b91 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -414,7 +414,6 @@ class ConfigEntries: Handler key is the domain of the component that we want to set up. """ component = getattr(self.hass.components, handler_key) - _LOGGER.debug('Handler key: %s', handler_key) handler = HANDLERS.get(handler_key) if handler is None: From c9e34e236da147d206ce66ec5d181269e622212d Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sat, 1 Sep 2018 10:14:21 +0200 Subject: [PATCH 07/24] Prevent error message when component is actually ok --- homeassistant/components/igd/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 305ac993b87..4f2e8dc7e71 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -143,7 +143,7 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): # ensure sane config if DOMAIN not in config: - return False + return True if DISCOVERY_DOMAIN not in config: _LOGGER.warning('IGD needs discovery, please enable it') From 50f63ed4c5f5fdd2ab247b4a7a2ef39d34d6a856 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sat, 1 Sep 2018 18:13:45 +0200 Subject: [PATCH 08/24] Fix not being able to re-add IGD when removed --- .coveragerc | 2 +- .../components/igd/.translations/en.json | 8 +- .../components/igd/.translations/nl.json | 8 +- homeassistant/components/igd/__init__.py | 26 +- homeassistant/components/igd/config_flow.py | 33 +- homeassistant/components/igd/strings.json | 8 +- homeassistant/components/sensor/igd.py | 290 ++++++++++++------ tests/components/igd/test_config_flow.py | 16 +- 8 files changed, 245 insertions(+), 146 deletions(-) diff --git a/.coveragerc b/.coveragerc index 39c31e4e40b..078b03f21d2 100644 --- a/.coveragerc +++ b/.coveragerc @@ -674,6 +674,7 @@ omit = homeassistant/components/sensor/haveibeenpwned.py homeassistant/components/sensor/hp_ilo.py homeassistant/components/sensor/htu21d.py + homeassistant/components/sensor/igd.py homeassistant/components/sensor/imap_email_content.py homeassistant/components/sensor/imap.py homeassistant/components/sensor/influxdb.py @@ -757,7 +758,6 @@ omit = homeassistant/components/sensor/travisci.py homeassistant/components/sensor/twitch.py homeassistant/components/sensor/uber.py - homeassistant/components/sensor/upnp.py homeassistant/components/sensor/ups.py homeassistant/components/sensor/uscis.py homeassistant/components/sensor/vasttrafik.py diff --git a/homeassistant/components/igd/.translations/en.json b/homeassistant/components/igd/.translations/en.json index bd0d2a9b7c0..e3b55a9dfd3 100644 --- a/homeassistant/components/igd/.translations/en.json +++ b/homeassistant/components/igd/.translations/en.json @@ -8,11 +8,9 @@ "user": { "title": "Configuration options for the IGD", "data":{ - "sensors": "Add traffic in/out sensors", - "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!", - "ssdp_url": "SSDP URL", - "udn": "UDN", - "name": "Name" + "igd": "IGD", + "sensors": "Add traffic sensors", + "port_forward": "Enable port forward for Home Assistant
Only enable this when your Home Assistant is password protected!" } } }, diff --git a/homeassistant/components/igd/.translations/nl.json b/homeassistant/components/igd/.translations/nl.json index 06f9122678c..2cd5e6b1651 100644 --- a/homeassistant/components/igd/.translations/nl.json +++ b/homeassistant/components/igd/.translations/nl.json @@ -8,11 +8,9 @@ "user": { "title": "Extra configuratie options voor IGD", "data":{ - "sensors": "Verkeer in/out sensors", - "port_forward": "Maak port-forward voor Home Assistant\nZet dit alleen aan wanneer uw Home Assistant een wachtwoord heeft!", - "ssdp_url": "SSDP URL", - "udn": "UDN", - "name": "Naam" + "igd": "IGD", + "sensors": "Verkeer sensors toevoegen", + "port_forward": "Maak port-forward voor Home Assistant
Zet dit alleen aan wanneer uw Home Assistant een wachtwoord heeft!" } } }, diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 4f2e8dc7e71..44c625ed87c 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -34,25 +34,16 @@ DEPENDENCIES = ['http'] # ,'discovery'] CONF_LOCAL_IP = 'local_ip' CONF_PORTS = 'ports' -CONF_UNITS = 'unit' CONF_HASS = 'hass' NOTIFICATION_ID = 'igd_notification' NOTIFICATION_TITLE = 'UPnP/IGD Setup' -UNITS = { - "Bytes": 1, - "KBytes": 1024, - "MBytes": 1024**2, - "GBytes": 1024**3, -} - CONFIG_SCHEMA = vol.Schema({ DOMAIN: vol.Schema({ vol.Optional(CONF_ENABLE_PORT_MAPPING, default=False): cv.boolean, vol.Optional(CONF_ENABLE_SENSORS, default=True): cv.boolean, vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), - vol.Optional(CONF_UNITS, default="MBytes"): vol.In(UNITS), }), }, extra=vol.ALLOW_EXTRA) @@ -133,13 +124,13 @@ async def _async_delete_port_mapping(hass: HomeAssistantType, igd_device): async def async_setup(hass: HomeAssistantType, config: ConfigType): """Register a port mapping for Home Assistant via UPnP.""" # defaults - hass.data[DOMAIN] = { - 'auto_config': { + hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) + if 'auto_config' not in hass.data[DOMAIN]: + hass.data[DOMAIN]['auto_config'] = { 'active': False, 'port_forward': False, 'sensors': False, } - } # ensure sane config if DOMAIN not in config: @@ -167,6 +158,8 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Set up a bridge from a config entry.""" + _LOGGER.debug('async_setup_entry: %s, %s', config_entry, config_entry.entry_id) + data = config_entry.data ssdp_description = data['ssdp_description'] @@ -186,7 +179,6 @@ async def async_setup_entry(hass: HomeAssistantType, # sensors if data.get(CONF_ENABLE_SENSORS): discovery_info = { - 'unit': 'MBytes', 'udn': data['udn'], } hass_config = config_entry.data @@ -205,6 +197,10 @@ async def async_setup_entry(hass: HomeAssistantType, async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Unload a config entry.""" + _LOGGER.debug('async_unload_entry: %s, entry_id: %s, data: %s', config_entry, config_entry.entry_id, config_entry.data) + for entry in hass.config_entries._entries: + _LOGGER.debug('%s: %s: %s', entry, entry.entry_id, entry.data) + data = config_entry.data udn = data['udn'] @@ -221,6 +217,10 @@ async def async_unload_entry(hass: HomeAssistantType, # XXX TODO: remove sensors pass + # clear stored device _store_device(hass, udn, None) + # XXX TODO: remove config entry + #await hass.config_entries.async_remove(config_entry.entry_id) + return True diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index be6cf9f5f93..6269e92f9c3 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -6,6 +6,7 @@ from homeassistant.core import callback from .const import CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS from .const import DOMAIN +from .const import LOGGER as _LOGGER @callback @@ -60,13 +61,15 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): return self.async_abort(reason='already_configured') # store discovered device + discovery_info['friendly_name'] = \ + '{} ({})'.format(discovery_info['host'], discovery_info['name']) self._store_discovery_info(discovery_info) # auto config? auto_config = self._auto_config_settings() if auto_config['active']: import_info = { - 'igd_host': discovery_info['host'], + 'name': discovery_info['friendly_name'], 'sensors': auto_config['sensors'], 'port_forward': auto_config['port_forward'], } @@ -79,35 +82,37 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): """Manual set up.""" # if user input given, handle it user_input = user_input or {} - if 'igd_host' in user_input: + if 'name' in user_input: if not user_input['sensors'] and not user_input['port_forward']: return self.async_abort(reason='no_sensors_or_port_forward') - configured_hosts = [ - entry['host'] + # ensure nto already configured + configured_igds = [ + entry['friendly_name'] for entry in self._discovereds.values() if entry['udn'] in configured_udns(self.hass) ] - if user_input['igd_host'] in configured_hosts: + _LOGGER.debug('Configured IGDs: %s', configured_igds) + if user_input['name'] in configured_igds: return self.async_abort(reason='already_configured') return await self._async_save_entry(user_input) - # let user choose from all discovered IGDs - igd_hosts = [ - entry['host'] + # let user choose from all discovered, non-configured, IGDs + names = [ + entry['friendly_name'] for entry in self._discovereds.values() if entry['udn'] not in configured_udns(self.hass) ] - if not igd_hosts: + if not names: return self.async_abort(reason='no_devices_discovered') return self.async_show_form( step_id='user', data_schema=vol.Schema({ - vol.Required('igd_host'): vol.In(igd_hosts), - vol.Required('sensors'): bool, - vol.Required('port_forward'): bool, + vol.Required('name'): vol.In(names), + vol.Optional('sensors', default=False): bool, + vol.Optional('port_forward', default=False): bool, }) ) @@ -118,10 +123,10 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): async def _async_save_entry(self, import_info): """Store IGD as new entry.""" # ensure we know the host - igd_host = import_info['igd_host'] + name = import_info['name'] discovery_infos = [info for info in self._discovereds.values() - if info['host'] == igd_host] + if info['friendly_name'] == name] if not discovery_infos: return self.async_abort(reason='host_not_found') discovery_info = discovery_infos[0] diff --git a/homeassistant/components/igd/strings.json b/homeassistant/components/igd/strings.json index bd0d2a9b7c0..e3b55a9dfd3 100644 --- a/homeassistant/components/igd/strings.json +++ b/homeassistant/components/igd/strings.json @@ -8,11 +8,9 @@ "user": { "title": "Configuration options for the IGD", "data":{ - "sensors": "Add traffic in/out sensors", - "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!", - "ssdp_url": "SSDP URL", - "udn": "UDN", - "name": "Name" + "igd": "IGD", + "sensors": "Add traffic sensors", + "port_forward": "Enable port forward for Home Assistant
Only enable this when your Home Assistant is password protected!" } } }, diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/igd.py index edd936600e3..63cbdff89fc 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/igd.py @@ -5,10 +5,10 @@ For more details about this platform, please refer to the documentation at https://home-assistant.io/components/sensor.igd/ """ # pylint: disable=invalid-name +from datetime import datetime import logging -from homeassistant.components import history -from homeassistant.components.igd import DOMAIN, UNITS +from homeassistant.components.igd import DOMAIN from homeassistant.helpers.entity import Entity @@ -21,15 +21,28 @@ BYTES_SENT = 'bytes_sent' PACKETS_RECEIVED = 'packets_received' PACKETS_SENT = 'packets_sent' -# sensor_type: [friendly_name, convert_unit, icon] SENSOR_TYPES = { - BYTES_RECEIVED: ['bytes received', True, 'mdi:server-network', float], - BYTES_SENT: ['bytes sent', True, 'mdi:server-network', float], - PACKETS_RECEIVED: ['packets received', False, 'mdi:server-network', int], - PACKETS_SENT: ['packets sent', False, 'mdi:server-network', int], + BYTES_RECEIVED: { + 'name': 'bytes received', + 'unit': 'bytes', + }, + BYTES_SENT: { + 'name': 'bytes sent', + 'unit': 'bytes', + }, + PACKETS_RECEIVED: { + 'name': 'packets received', + 'unit': '#', + }, + PACKETS_SENT: { + 'name': 'packets sent', + 'unit': '#', + }, } -OVERFLOW_AT = 2**32 +IN = 'received' +OUT = 'sent' +KBYTE = 1024 async def async_setup_platform(hass, config, async_add_devices, @@ -39,126 +52,207 @@ async def async_setup_platform(hass, config, async_add_devices, return udn = discovery_info['udn'] - device = hass.data[DOMAIN]['devices'][udn] - unit = discovery_info['unit'] + igd_device = hass.data[DOMAIN]['devices'][udn] + + # raw sensors async_add_devices([ - IGDSensor(device, t, unit if SENSOR_TYPES[t][1] else '#') - for t in SENSOR_TYPES]) + RawIGDSensor(igd_device, name, sensor_type) + for name, sensor_type in SENSOR_TYPES.items()], + True + ) + + # current traffic reporting + async_add_devices( + [ + KBytePerSecondIGDSensor(igd_device, IN), + KBytePerSecondIGDSensor(igd_device, OUT), + PacketsPerSecondIGDSensor(igd_device, IN), + PacketsPerSecondIGDSensor(igd_device, OUT), + ], True + ) -class IGDSensor(Entity): +class RawIGDSensor(Entity): """Representation of a UPnP IGD sensor.""" - def __init__(self, device, sensor_type, unit=None): + def __init__(self, device, sensor_type_name, sensor_type): """Initialize the IGD sensor.""" self._device = device - self.type = sensor_type - self.unit = unit - self.unit_factor = UNITS[unit] if unit in UNITS else 1 - self._name = 'IGD {}'.format(SENSOR_TYPES[sensor_type][0]) + self._type_name = sensor_type_name + self._type = sensor_type + self._name = 'IGD {}'.format(sensor_type['name']) self._state = None - self._last_value = None @property - def name(self): + def name(self) -> str: """Return the name of the sensor.""" return self._name @property - def state(self): + def state(self) -> str: """Return the state of the device.""" - if self._state is None: - return None - - coercer = SENSOR_TYPES[self.type][3] - if coercer == int: - return format(self._state) - - return format(self._state / self.unit_factor, '.1f') + return self._state @property - def icon(self): + def icon(self) -> str: """Icon to use in the frontend, if any.""" - return SENSOR_TYPES[self.type][2] + return 'mdi:server-network' @property - def unit_of_measurement(self): + def unit_of_measurement(self) -> str: """Return the unit of measurement of this entity, if any.""" - return self.unit + return self._type['unit'] async def async_update(self): """Get the latest information from the IGD.""" - new_value = 0 - if self.type == BYTES_RECEIVED: + _LOGGER.debug('%s: async_update', self) + if self._type_name == BYTES_RECEIVED: + self._state = await self._device.async_get_total_bytes_received() + elif self._type_name == BYTES_SENT: + self._state = await self._device.async_get_total_bytes_sent() + elif self._type_name == PACKETS_RECEIVED: + self._state = await self._device.async_get_total_packets_received() + elif self._type_name == PACKETS_SENT: + self._state = await self._device.async_get_total_packets_sent() + + +class KBytePerSecondIGDSensor(Entity): + """Representation of a KBytes Sent/Received per second sensor.""" + + def __init__(self, device, direction): + """Initializer.""" + self._device = device + self._direction = direction + + self._last_value = None + self._last_update_time = None + self._state = None + + @property + def unique_id(self) -> str: + """Return an unique ID.""" + return '{}:kbytes_{}'.format(self._device.udn, self._direction) + + @property + def name(self) -> str: + """Return the name of the sensor.""" + return '{} kbytes/sec {}'.format(self._device.name, + self._direction) + + @property + def state(self): + """Return the state of the device.""" + return self._state + + @property + def icon(self) -> str: + """Icon to use in the frontend, if any.""" + return 'mdi:server-network' + + @property + def unit_of_measurement(self) -> str: + """Return the unit of measurement of this entity, if any.""" + return 'kbytes/sec' + + def _is_overflowed(self, new_value) -> bool: + """Check if value has overflowed.""" + return new_value < self._last_value + + async def async_update(self): + """Get the latest information from the IGD.""" + _LOGGER.debug('%s: async_update', self) + + if self._direction == IN: new_value = await self._device.async_get_total_bytes_received() - elif self.type == BYTES_SENT: + else: new_value = await self._device.async_get_total_bytes_sent() - elif self.type == PACKETS_RECEIVED: - new_value = await self._device.async_get_total_packets_received() - elif self.type == PACKETS_SENT: - new_value = await self._device.async_get_total_packets_sent() - - self._handle_new_value(new_value) - - @property - def _last_state(self): - """Get the last state reported to hass.""" - states = history.get_last_state_changes(self.hass, 2, self.entity_id) - entity_states = [ - state for state in states[self.entity_id] - if state.state != 'unknown'] - _LOGGER.debug('%s: entity_states: %s', self.entity_id, entity_states) - if not entity_states: - return None - - return entity_states[0] - - @property - def _last_value_from_state(self): - """Get the last value reported to hass.""" - last_state = self._last_state - if not last_state: - _LOGGER.debug('%s: No last state', self.entity_id) - return None - - coercer = SENSOR_TYPES[self.type][3] - try: - state = coercer(float(last_state.state)) * self.unit_factor - except ValueError: - state = coercer(0.0) - - return state - - def _handle_new_value(self, new_value): - if self.entity_id is None: - # don't know our entity ID yet, do nothing but store value - self._last_value = new_value - return if self._last_value is None: self._last_value = new_value + self._last_update_time = datetime.now() + return - if self._state is None: - # try to get the state from history - self._state = self._last_value_from_state or 0 - - _LOGGER.debug('%s: state: %s, last_value: %s', - self.entity_id, self._state, self._last_value) - - # calculate new state - if self._last_value <= new_value: - diff = new_value - self._last_value + now = datetime.now() + if self._is_overflowed(new_value): + _LOGGER.debug('%s: Overflow: old value: %s, new value: %s', + self, self._last_value, new_value) + self._state = None # temporarily report nothing else: - # handle overflow - diff = OVERFLOW_AT - self._last_value - if new_value >= 0: - diff += new_value - else: - # some devices don't overflow and start at 0, - # but somewhere to -2**32 - diff += new_value - -OVERFLOW_AT + delta_time = (now - self._last_update_time).seconds + delta_value = new_value - self._last_value + value = (delta_value / delta_time) / KBYTE + self._state = format(float(value), '.1f') - self._state += diff self._last_value = new_value - _LOGGER.debug('%s: diff: %s, state: %s, last_value: %s', - self.entity_id, diff, self._state, self._last_value) + self._last_update_time = now + + +class PacketsPerSecondIGDSensor(Entity): + """Representation of a Packets Sent/Received per second sensor.""" + + def __init__(self, device, direction): + """Initializer.""" + self._device = device + self._direction = direction + + self._last_value = None + self._last_update_time = None + self._state = None + + @property + def unique_id(self) -> str: + """Return an unique ID.""" + return '{}:packets_{}'.format(self._device.udn, self._direction) + + @property + def name(self) -> str: + """Return the name of the sensor.""" + return '{} packets/sec {}'.format(self._device.name, + self._direction) + + @property + def state(self): + """Return the state of the device.""" + return self._state + + @property + def icon(self) -> str: + """Icon to use in the frontend, if any.""" + return 'mdi:server-network' + + @property + def unit_of_measurement(self) -> str: + """Return the unit of measurement of this entity, if any.""" + return '#/sec' + + def _is_overflowed(self, new_value) -> bool: + """Check if value has overflowed.""" + return new_value < self._last_value + + async def async_update(self): + """Get the latest information from the IGD.""" + _LOGGER.debug('%s: async_update', self) + + if self._direction == IN: + new_value = await self._device.async_get_total_bytes_received() + else: + new_value = await self._device.async_get_total_bytes_sent() + + if self._last_value is None: + self._last_value = new_value + self._last_update_time = datetime.now() + return + + now = datetime.now() + if self._is_overflowed(new_value): + _LOGGER.debug('%s: Overflow: old value: %s, new value: %s', + self, self._last_value, new_value) + self._state = None # temporarily report nothing + else: + delta_time = (now - self._last_update_time).seconds + delta_value = new_value - self._last_value + value = delta_value / delta_time + self._state = format(float(value), '.1f') + + self._last_value = new_value + self._last_update_time = now diff --git a/tests/components/igd/test_config_flow.py b/tests/components/igd/test_config_flow.py index 5530e172dae..4fc103c0772 100644 --- a/tests/components/igd/test_config_flow.py +++ b/tests/components/igd/test_config_flow.py @@ -26,6 +26,7 @@ async def test_flow_already_configured(hass): hass.data[igd.DOMAIN] = { 'discovered': { udn: { + 'friendly_name': '192.168.1.1 (Test device)', 'host': '192.168.1.1', 'udn': udn, }, @@ -39,7 +40,7 @@ async def test_flow_already_configured(hass): }).add_to_hass(hass) result = await flow.async_step_user({ - 'igd_host': '192.168.1.1', + 'name': '192.168.1.1 (Test device)', 'sensors': True, 'port_forward': False, }) @@ -57,6 +58,7 @@ async def test_flow_no_sensors_no_port_forward(hass): hass.data[igd.DOMAIN] = { 'discovered': { udn: { + 'friendly_name': '192.168.1.1 (Test device)', 'host': '192.168.1.1', 'udn': udn, }, @@ -70,7 +72,7 @@ async def test_flow_no_sensors_no_port_forward(hass): }).add_to_hass(hass) result = await flow.async_step_user({ - 'igd_host': '192.168.1.1', + 'name': '192.168.1.1 (Test device)', 'sensors': False, 'port_forward': False, }) @@ -88,6 +90,7 @@ async def test_flow_discovered_form(hass): hass.data[igd.DOMAIN] = { 'discovered': { udn: { + 'friendly_name': '192.168.1.1 (Test device)', 'host': '192.168.1.1', 'udn': udn, }, @@ -110,10 +113,12 @@ async def test_flow_two_discovered_form(hass): hass.data[igd.DOMAIN] = { 'discovered': { udn_1: { + 'friendly_name': '192.168.1.1 (Test device)', 'host': '192.168.1.1', 'udn': udn_1, }, udn_2: { + 'friendly_name': '192.168.2.1 (Test device)', 'host': '192.168.2.1', 'udn': udn_2, }, @@ -124,12 +129,12 @@ async def test_flow_two_discovered_form(hass): assert result['type'] == 'form' assert result['step_id'] == 'user' assert result['data_schema']({ - 'igd_host': '192.168.1.1', + 'name': '192.168.1.1 (Test device)', 'sensors': True, 'port_forward': False, }) assert result['data_schema']({ - 'igd_host': '192.168.2.1', + 'name': '192.168.1.1 (Test device)', 'sensors': True, 'port_forward': False, }) @@ -144,6 +149,7 @@ async def test_config_entry_created(hass): hass.data[igd.DOMAIN] = { 'discovered': { 'uuid:device_1': { + 'friendly_name': '192.168.1.1 (Test device)', 'name': 'Test device 1', 'host': '192.168.1.1', 'ssdp_description': 'http://192.168.1.1/desc.xml', @@ -153,7 +159,7 @@ async def test_config_entry_created(hass): } result = await flow.async_step_user({ - 'igd_host': '192.168.1.1', + 'name': '192.168.1.1 (Test device)', 'sensors': True, 'port_forward': False, }) From 8bec4a55d1de3475d7db1a1dd38e85a796d811a1 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sat, 1 Sep 2018 21:20:15 +0200 Subject: [PATCH 09/24] Working on igd --- CODEOWNERS | 1 - .../components/igd/.translations/en.json | 7 +- .../components/igd/.translations/nl.json | 7 +- homeassistant/components/igd/__init__.py | 60 ++-- homeassistant/components/igd/config_flow.py | 75 +++-- homeassistant/components/igd/const.py | 16 ++ homeassistant/components/igd/strings.json | 7 +- homeassistant/components/sensor/igd.py | 257 ++++++++---------- tests/components/igd/test_config_flow.py | 3 + tests/components/igd/test_init.py | 4 +- 10 files changed, 206 insertions(+), 231 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index b86e09a6b72..9015e0fb44c 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -80,7 +80,6 @@ homeassistant/components/sensor/sma.py @kellerza homeassistant/components/sensor/sql.py @dgomes homeassistant/components/sensor/sytadin.py @gautric homeassistant/components/sensor/tibber.py @danielhiversen -homeassistant/components/sensor/upnp.py @dgomes homeassistant/components/sensor/waqi.py @andrey-git homeassistant/components/switch/tplink.py @rytilahti homeassistant/components/vacuum/roomba.py @pschmitt diff --git a/homeassistant/components/igd/.translations/en.json b/homeassistant/components/igd/.translations/en.json index e3b55a9dfd3..3466b6cfa93 100644 --- a/homeassistant/components/igd/.translations/en.json +++ b/homeassistant/components/igd/.translations/en.json @@ -10,7 +10,7 @@ "data":{ "igd": "IGD", "sensors": "Add traffic sensors", - "port_forward": "Enable port forward for Home Assistant
Only enable this when your Home Assistant is password protected!" + "port_forward": "Enable port forward for Home Assistant" } } }, @@ -20,8 +20,7 @@ "no_devices_discovered": "No IGDs discovered", "already_configured": "IGD is already configured", "no_sensors_or_port_forward": "Enable at least sensors or Port forward", - "no_igds": "No IGDs discovered", - "todo": "TODO" + "no_igds": "No IGDs discovered" } } -} \ No newline at end of file +} diff --git a/homeassistant/components/igd/.translations/nl.json b/homeassistant/components/igd/.translations/nl.json index 2cd5e6b1651..5a42f6c3bea 100644 --- a/homeassistant/components/igd/.translations/nl.json +++ b/homeassistant/components/igd/.translations/nl.json @@ -10,7 +10,7 @@ "data":{ "igd": "IGD", "sensors": "Verkeer sensors toevoegen", - "port_forward": "Maak port-forward voor Home Assistant
Zet dit alleen aan wanneer uw Home Assistant een wachtwoord heeft!" + "port_forward": "Maak port-forward voor Home Assistant" } } }, @@ -20,8 +20,7 @@ "no_devices_discovered": "Geen IGDs gevonden", "already_configured": "IGD is reeds geconfigureerd", "no_sensors_or_port_forward": "Kies ten minste sensors of Port forward", - "no_igds": "Geen IGDs gevonden", - "todo": "TODO" + "no_igds": "Geen IGDs gevonden" } } -} \ No newline at end of file +} diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 44c625ed87c..d9c6ad1408c 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -2,10 +2,9 @@ Will open a port in your router for Home Assistant and provide statistics. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/upnp/ +https://home-assistant.io/components/igd/ """ - import asyncio from ipaddress import IPv4Address from ipaddress import ip_address @@ -22,19 +21,22 @@ from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.typing import ConfigType, HomeAssistantType from homeassistant.util import get_local_ip from homeassistant.components.discovery import DOMAIN as DISCOVERY_DOMAIN +import homeassistant.components.igd.config_flow # noqa: 401 -from .const import CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS +from .const import ( + CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS, + CONF_UDN, CONF_SSDP_DESCRIPTION +) from .const import DOMAIN from .const import LOGGER as _LOGGER -import homeassistant.components.igd.config_flow # register the handler +from .const import ensure_domain_data REQUIREMENTS = ['async-upnp-client==0.12.4'] -DEPENDENCIES = ['http'] # ,'discovery'] +DEPENDENCIES = ['http'] CONF_LOCAL_IP = 'local_ip' CONF_PORTS = 'ports' -CONF_HASS = 'hass' NOTIFICATION_ID = 'igd_notification' NOTIFICATION_TITLE = 'UPnP/IGD Setup' @@ -72,16 +74,15 @@ async def _async_create_igd_device(hass: HomeAssistantType, def _store_device(hass: HomeAssistantType, udn, igd_device): """Store an igd_device by udn.""" - hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) - hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) - hass.data[DOMAIN]['devices'][udn] = igd_device + if igd_device is not None: + hass.data[DOMAIN]['devices'][udn] = igd_device + elif udn in hass.data[DOMAIN]['devices']: + del hass.data[DOMAIN]['devices'][udn] def _get_device(hass: HomeAssistantType, udn): """Get an igd_device by udn.""" - hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) - hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) - return hass.data[DOMAIN]['devices'][udn] + return hass.data[DOMAIN]['devices'].get(udn) async def _async_add_port_mapping(hass: HomeAssistantType, @@ -123,14 +124,7 @@ async def _async_delete_port_mapping(hass: HomeAssistantType, igd_device): # config async def async_setup(hass: HomeAssistantType, config: ConfigType): """Register a port mapping for Home Assistant via UPnP.""" - # defaults - hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) - if 'auto_config' not in hass.data[DOMAIN]: - hass.data[DOMAIN]['auto_config'] = { - 'active': False, - 'port_forward': False, - 'sensors': False, - } + ensure_domain_data(hass) # ensure sane config if DOMAIN not in config: @@ -158,12 +152,11 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Set up a bridge from a config entry.""" - _LOGGER.debug('async_setup_entry: %s, %s', config_entry, config_entry.entry_id) - + ensure_domain_data(hass) data = config_entry.data - ssdp_description = data['ssdp_description'] # build IGD device + ssdp_description = data[CONF_SSDP_DESCRIPTION] try: igd_device = await _async_create_igd_device(hass, ssdp_description) except (asyncio.TimeoutError, aiohttp.ClientError): @@ -179,12 +172,11 @@ async def async_setup_entry(hass: HomeAssistantType, # sensors if data.get(CONF_ENABLE_SENSORS): discovery_info = { - 'udn': data['udn'], + 'udn': data[CONF_UDN], } hass_config = config_entry.data - hass.async_create_task( - discovery.async_load_platform( - hass, 'sensor', DOMAIN, discovery_info, hass_config)) + hass.async_create_task(discovery.async_load_platform( + hass, 'sensor', DOMAIN, discovery_info, hass_config)) async def unload_entry(event): """Unload entry on quit.""" @@ -197,12 +189,8 @@ async def async_setup_entry(hass: HomeAssistantType, async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Unload a config entry.""" - _LOGGER.debug('async_unload_entry: %s, entry_id: %s, data: %s', config_entry, config_entry.entry_id, config_entry.data) - for entry in hass.config_entries._entries: - _LOGGER.debug('%s: %s: %s', entry, entry.entry_id, entry.data) - data = config_entry.data - udn = data['udn'] + udn = data[CONF_UDN] igd_device = _get_device(hass, udn) if igd_device is None: @@ -213,14 +201,10 @@ async def async_unload_entry(hass: HomeAssistantType, await _async_delete_port_mapping(hass, igd_device) # sensors - if data.get(CONF_ENABLE_SENSORS): - # XXX TODO: remove sensors - pass + for sensor in hass.data[DOMAIN]['sensors'].get(udn, []): + await sensor.async_remove() # clear stored device _store_device(hass, udn, None) - # XXX TODO: remove config entry - #await hass.config_entries.async_remove(config_entry.entry_id) - return True diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index 6269e92f9c3..c66241d3b78 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -1,21 +1,15 @@ """Config flow for IGD.""" import voluptuous as vol -from homeassistant import config_entries, data_entry_flow -from homeassistant.core import callback +from homeassistant import config_entries +from homeassistant import data_entry_flow -from .const import CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS +from .const import ( + CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS, + CONF_SSDP_DESCRIPTION, CONF_UDN +) from .const import DOMAIN -from .const import LOGGER as _LOGGER - - -@callback -def configured_udns(hass): - """Get all configured UDNs.""" - return [ - entry.data['udn'] - for entry in hass.config_entries.async_entries(DOMAIN) - ] +from .const import ensure_domain_data @config_entries.HANDLERS.register(DOMAIN) @@ -24,29 +18,29 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): VERSION = 1 - def __init__(self): - """Initializer.""" - pass + @property + def _configured_igds(self): + """Get all configured IGDs.""" + return { + entry.data[CONF_UDN]: { + 'udn': entry.data[CONF_UDN], + } + for entry in self.hass.config_entries.async_entries(DOMAIN) + } @property - def _discovereds(self): + def _discovered_igds(self): """Get all discovered entries.""" - return self.hass.data.get(DOMAIN, {}).get('discovered', {}) + return self.hass.data[DOMAIN]['discovered'] def _store_discovery_info(self, discovery_info): """Add discovery info.""" udn = discovery_info['udn'] - self.hass.data[DOMAIN] = self.hass.data.get(DOMAIN, {}) - self.hass.data[DOMAIN]['discovered'] = \ - self.hass.data[DOMAIN].get('discovered', {}) self.hass.data[DOMAIN]['discovered'][udn] = discovery_info def _auto_config_settings(self): """Check if auto_config has been enabled.""" - self.hass.data[DOMAIN] = self.hass.data.get(DOMAIN, {}) - return self.hass.data[DOMAIN].get('auto_config', { - 'active': False, - }) + return self.hass.data[DOMAIN]['auto_config'] async def async_step_discovery(self, discovery_info): """ @@ -55,16 +49,18 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): This flow is triggered by the discovery component. It will check if the host is already configured and delegate to the import step if not. """ - # ensure not already discovered/configured - udn = discovery_info['udn'] - if udn in configured_udns(self.hass): - return self.async_abort(reason='already_configured') + ensure_domain_data(self.hass) # store discovered device discovery_info['friendly_name'] = \ '{} ({})'.format(discovery_info['host'], discovery_info['name']) self._store_discovery_info(discovery_info) + # ensure not already discovered/configured + udn = discovery_info['udn'] + if udn in self._configured_igds: + return self.async_abort(reason='already_configured') + # auto config? auto_config = self._auto_config_settings() if auto_config['active']: @@ -86,14 +82,13 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): if not user_input['sensors'] and not user_input['port_forward']: return self.async_abort(reason='no_sensors_or_port_forward') - # ensure nto already configured - configured_igds = [ + # ensure not already configured + configured_names = [ entry['friendly_name'] - for entry in self._discovereds.values() - if entry['udn'] in configured_udns(self.hass) + for udn, entry in self._discovered_igds.items() + if udn in self._configured_igds ] - _LOGGER.debug('Configured IGDs: %s', configured_igds) - if user_input['name'] in configured_igds: + if user_input['name'] in configured_names: return self.async_abort(reason='already_configured') return await self._async_save_entry(user_input) @@ -101,8 +96,8 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): # let user choose from all discovered, non-configured, IGDs names = [ entry['friendly_name'] - for entry in self._discovereds.values() - if entry['udn'] not in configured_udns(self.hass) + for udn, entry in self._discovered_igds.items() + if udn not in self._configured_igds ] if not names: return self.async_abort(reason='no_devices_discovered') @@ -125,7 +120,7 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): # ensure we know the host name = import_info['name'] discovery_infos = [info - for info in self._discovereds.values() + for info in self._discovered_igds.values() if info['friendly_name'] == name] if not discovery_infos: return self.async_abort(reason='host_not_found') @@ -134,8 +129,8 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): return self.async_create_entry( title=discovery_info['name'], data={ - 'ssdp_description': discovery_info['ssdp_description'], - 'udn': discovery_info['udn'], + CONF_SSDP_DESCRIPTION: discovery_info['ssdp_description'], + CONF_UDN: discovery_info['udn'], CONF_ENABLE_SENSORS: import_info['sensors'], CONF_ENABLE_PORT_MAPPING: import_info['port_forward'], }, diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/igd/const.py index c849e627757..58092cdf38f 100644 --- a/homeassistant/components/igd/const.py +++ b/homeassistant/components/igd/const.py @@ -1,7 +1,23 @@ """Constants for the IGD component.""" import logging + DOMAIN = 'igd' LOGGER = logging.getLogger('homeassistant.components.igd') CONF_ENABLE_PORT_MAPPING = 'port_forward' CONF_ENABLE_SENSORS = 'sensors' +CONF_UDN = 'udn' +CONF_SSDP_DESCRIPTION = 'ssdp_description' + + +def ensure_domain_data(hass): + """Ensure hass.data is filled properly.""" + hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) + hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) + hass.data[DOMAIN]['sensors'] = hass.data[DOMAIN].get('sensors', {}) + hass.data[DOMAIN]['discovered'] = hass.data[DOMAIN].get('discovered', {}) + hass.data[DOMAIN]['auto_config'] = hass.data[DOMAIN].get('auto_config', { + 'active': False, + 'port_forward': False, + 'sensors': False, + }) diff --git a/homeassistant/components/igd/strings.json b/homeassistant/components/igd/strings.json index e3b55a9dfd3..3466b6cfa93 100644 --- a/homeassistant/components/igd/strings.json +++ b/homeassistant/components/igd/strings.json @@ -10,7 +10,7 @@ "data":{ "igd": "IGD", "sensors": "Add traffic sensors", - "port_forward": "Enable port forward for Home Assistant
Only enable this when your Home Assistant is password protected!" + "port_forward": "Enable port forward for Home Assistant" } } }, @@ -20,8 +20,7 @@ "no_devices_discovered": "No IGDs discovered", "already_configured": "IGD is already configured", "no_sensors_or_port_forward": "Enable at least sensors or Port forward", - "no_igds": "No IGDs discovered", - "todo": "TODO" + "no_igds": "No IGDs discovered" } } -} \ No newline at end of file +} diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/igd.py index 63cbdff89fc..cb0ceb752b0 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/igd.py @@ -32,11 +32,11 @@ SENSOR_TYPES = { }, PACKETS_RECEIVED: { 'name': 'packets received', - 'unit': '#', + 'unit': 'packets', }, PACKETS_SENT: { 'name': 'packets sent', - 'unit': '#', + 'unit': 'packets', }, } @@ -54,22 +54,20 @@ async def async_setup_platform(hass, config, async_add_devices, udn = discovery_info['udn'] igd_device = hass.data[DOMAIN]['devices'][udn] - # raw sensors - async_add_devices([ + # raw sensors + per-second sensors + sensors = [ RawIGDSensor(igd_device, name, sensor_type) - for name, sensor_type in SENSOR_TYPES.items()], - True - ) - - # current traffic reporting - async_add_devices( - [ - KBytePerSecondIGDSensor(igd_device, IN), - KBytePerSecondIGDSensor(igd_device, OUT), - PacketsPerSecondIGDSensor(igd_device, IN), - PacketsPerSecondIGDSensor(igd_device, OUT), - ], True - ) + for name, sensor_type in SENSOR_TYPES.items() + ] + sensors += [ + KBytePerSecondIGDSensor(igd_device, IN), + KBytePerSecondIGDSensor(igd_device, OUT), + PacketsPerSecondIGDSensor(igd_device, IN), + PacketsPerSecondIGDSensor(igd_device, OUT), + ] + hass.data[DOMAIN]['sensors'][udn] = sensors + async_add_devices(sensors, True) + return True class RawIGDSensor(Entity): @@ -80,7 +78,7 @@ class RawIGDSensor(Entity): self._device = device self._type_name = sensor_type_name self._type = sensor_type - self._name = 'IGD {}'.format(sensor_type['name']) + self._name = '{} {}'.format(device.name, sensor_type['name']) self._state = None @property @@ -88,6 +86,11 @@ class RawIGDSensor(Entity): """Return the name of the sensor.""" return self._name + @property + def unique_id(self) -> str: + """Return an unique ID.""" + return '{}_{}'.format(self._device.udn, self._type_name) + @property def state(self) -> str: """Return the state of the device.""" @@ -116,143 +119,121 @@ class RawIGDSensor(Entity): self._state = await self._device.async_get_total_packets_sent() -class KBytePerSecondIGDSensor(Entity): +class PerSecondIGDSensor(Entity): + """Abstract representation of a X Sent/Received per second sensor.""" + + def __init__(self, device, direction): + """Initializer.""" + self._device = device + self._direction = direction + + self._state = None + self._last_value = None + self._last_update_time = None + + @property + def unit(self) -> str: + """Unit we are measuring in.""" + raise NotImplementedError() + + @property + def _async_fetch_value(self): + """Fetch a value from the IGD.""" + raise NotImplementedError() + + @property + def unique_id(self) -> str: + """Return an unique ID.""" + return '{}_{}/sec_{}'.format(self._device.udn, + self.unit, + self._direction) + + @property + def name(self) -> str: + """Return the name of the sensor.""" + return '{} {}/sec {}'.format(self._device.name, + self.unit, + self._direction) + + @property + def icon(self) -> str: + """Icon to use in the frontend, if any.""" + return 'mdi:server-network' + + @property + def unit_of_measurement(self) -> str: + """Return the unit of measurement of this entity, if any.""" + return '{}/sec'.format(self.unit) + + def _is_overflowed(self, new_value) -> bool: + """Check if value has overflowed.""" + return new_value < self._last_value + + async def async_update(self): + """Get the latest information from the IGD.""" + _LOGGER.debug('%s: async_update', self) + new_value = await self._async_fetch_value() + + if self._last_value is None: + self._last_value = new_value + self._last_update_time = datetime.now() + return + + now = datetime.now() + if self._is_overflowed(new_value): + self._state = None # temporarily report nothing + else: + delta_time = (now - self._last_update_time).seconds + delta_value = new_value - self._last_value + self._state = (delta_value / delta_time) + + self._last_value = new_value + self._last_update_time = now + + +class KBytePerSecondIGDSensor(PerSecondIGDSensor): """Representation of a KBytes Sent/Received per second sensor.""" - def __init__(self, device, direction): - """Initializer.""" - self._device = device - self._direction = direction - - self._last_value = None - self._last_update_time = None - self._state = None - @property - def unique_id(self) -> str: - """Return an unique ID.""" - return '{}:kbytes_{}'.format(self._device.udn, self._direction) + def unit(self) -> str: + """Unit we are measuring in.""" + return 'kbyte' - @property - def name(self) -> str: - """Return the name of the sensor.""" - return '{} kbytes/sec {}'.format(self._device.name, - self._direction) + async def _async_fetch_value(self) -> float: + """""" + if self._direction == IN: + return await self._device.async_get_total_bytes_received() + + return await self._device.async_get_total_bytes_sent() @property def state(self): """Return the state of the device.""" - return self._state + if self._state is None: + return None - @property - def icon(self) -> str: - """Icon to use in the frontend, if any.""" - return 'mdi:server-network' - - @property - def unit_of_measurement(self) -> str: - """Return the unit of measurement of this entity, if any.""" - return 'kbytes/sec' - - def _is_overflowed(self, new_value) -> bool: - """Check if value has overflowed.""" - return new_value < self._last_value - - async def async_update(self): - """Get the latest information from the IGD.""" - _LOGGER.debug('%s: async_update', self) - - if self._direction == IN: - new_value = await self._device.async_get_total_bytes_received() - else: - new_value = await self._device.async_get_total_bytes_sent() - - if self._last_value is None: - self._last_value = new_value - self._last_update_time = datetime.now() - return - - now = datetime.now() - if self._is_overflowed(new_value): - _LOGGER.debug('%s: Overflow: old value: %s, new value: %s', - self, self._last_value, new_value) - self._state = None # temporarily report nothing - else: - delta_time = (now - self._last_update_time).seconds - delta_value = new_value - self._last_value - value = (delta_value / delta_time) / KBYTE - self._state = format(float(value), '.1f') - - self._last_value = new_value - self._last_update_time = now + return format(float(self._state / KBYTE), '.1f') -class PacketsPerSecondIGDSensor(Entity): +class PacketsPerSecondIGDSensor(PerSecondIGDSensor): """Representation of a Packets Sent/Received per second sensor.""" - def __init__(self, device, direction): - """Initializer.""" - self._device = device - self._direction = direction - - self._last_value = None - self._last_update_time = None - self._state = None - @property - def unique_id(self) -> str: - """Return an unique ID.""" - return '{}:packets_{}'.format(self._device.udn, self._direction) + def unit(self) -> str: + """Unit we are measuring in.""" + return 'packets' - @property - def name(self) -> str: - """Return the name of the sensor.""" - return '{} packets/sec {}'.format(self._device.name, - self._direction) + async def _async_fetch_value(self) -> float: + """""" + if self._direction == IN: + return await self._device.async_get_total_packets_received() + + return await self._device.async_get_total_packets_sent() @property def state(self): """Return the state of the device.""" - return self._state + if self._state is None: + return None - @property - def icon(self) -> str: - """Icon to use in the frontend, if any.""" - return 'mdi:server-network' - - @property - def unit_of_measurement(self) -> str: - """Return the unit of measurement of this entity, if any.""" - return '#/sec' - - def _is_overflowed(self, new_value) -> bool: - """Check if value has overflowed.""" - return new_value < self._last_value - - async def async_update(self): - """Get the latest information from the IGD.""" - _LOGGER.debug('%s: async_update', self) - - if self._direction == IN: - new_value = await self._device.async_get_total_bytes_received() - else: - new_value = await self._device.async_get_total_bytes_sent() - - if self._last_value is None: - self._last_value = new_value - self._last_update_time = datetime.now() - return - - now = datetime.now() - if self._is_overflowed(new_value): - _LOGGER.debug('%s: Overflow: old value: %s, new value: %s', - self, self._last_value, new_value) - self._state = None # temporarily report nothing - else: - delta_time = (now - self._last_update_time).seconds - delta_value = new_value - self._last_value - value = delta_value / delta_time - self._state = format(float(value), '.1f') - - self._last_value = new_value - self._last_update_time = now + return format(float(self._state), '.1f') diff --git a/tests/components/igd/test_config_flow.py b/tests/components/igd/test_config_flow.py index 4fc103c0772..e02d5630e96 100644 --- a/tests/components/igd/test_config_flow.py +++ b/tests/components/igd/test_config_flow.py @@ -10,6 +10,9 @@ async def test_flow_none_discovered(hass): """Test no device discovered flow.""" flow = igd_config_flow.IgdFlowHandler() flow.hass = hass + hass.data[igd.DOMAIN] = { + 'discovered': {} + } result = await flow.async_step_user() assert result['type'] == 'abort' diff --git a/tests/components/igd/test_init.py b/tests/components/igd/test_init.py index 285755c687f..cd3f5324bb4 100644 --- a/tests/components/igd/test_init.py +++ b/tests/components/igd/test_init.py @@ -93,7 +93,7 @@ async def test_async_setup_entry_default(hass): hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done() - assert hass.data[igd.DOMAIN]['devices'][udn] is None + assert udn not in hass.data[igd.DOMAIN]['devices'] assert len(mock_igd_device.async_add_port_mapping.mock_calls) == 0 assert len(mock_igd_device.async_delete_port_mapping.mock_calls) == 0 @@ -128,6 +128,6 @@ async def test_async_setup_entry_port_forward(hass): hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done() - assert hass.data[igd.DOMAIN]['devices'][udn] is None + assert udn not in hass.data[igd.DOMAIN]['devices'] assert len(mock_igd_device.async_add_port_mapping.mock_calls) > 0 assert len(mock_igd_device.async_delete_port_mapping.mock_calls) > 0 From a4a175440b23c85382abd59e1963b9e5e994ea29 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sun, 2 Sep 2018 16:40:39 +0200 Subject: [PATCH 10/24] Move ensure_domain_data --- homeassistant/components/igd/__init__.py | 3 +-- homeassistant/components/igd/config_flow.py | 14 +++++++++++++- homeassistant/components/igd/const.py | 13 ------------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index d9c6ad1408c..37445350fa6 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -21,7 +21,6 @@ from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.typing import ConfigType, HomeAssistantType from homeassistant.util import get_local_ip from homeassistant.components.discovery import DOMAIN as DISCOVERY_DOMAIN -import homeassistant.components.igd.config_flow # noqa: 401 from .const import ( CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS, @@ -29,7 +28,7 @@ from .const import ( ) from .const import DOMAIN from .const import LOGGER as _LOGGER -from .const import ensure_domain_data +from .config_flow import ensure_domain_data REQUIREMENTS = ['async-upnp-client==0.12.4'] diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index c66241d3b78..6f0cec12b5b 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -9,7 +9,19 @@ from .const import ( CONF_SSDP_DESCRIPTION, CONF_UDN ) from .const import DOMAIN -from .const import ensure_domain_data + + +def ensure_domain_data(hass): + """Ensure hass.data is filled properly.""" + hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) + hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) + hass.data[DOMAIN]['sensors'] = hass.data[DOMAIN].get('sensors', {}) + hass.data[DOMAIN]['discovered'] = hass.data[DOMAIN].get('discovered', {}) + hass.data[DOMAIN]['auto_config'] = hass.data[DOMAIN].get('auto_config', { + 'active': False, + 'port_forward': False, + 'sensors': False, + }) @config_entries.HANDLERS.register(DOMAIN) diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/igd/const.py index 58092cdf38f..47ff9f11074 100644 --- a/homeassistant/components/igd/const.py +++ b/homeassistant/components/igd/const.py @@ -8,16 +8,3 @@ CONF_ENABLE_PORT_MAPPING = 'port_forward' CONF_ENABLE_SENSORS = 'sensors' CONF_UDN = 'udn' CONF_SSDP_DESCRIPTION = 'ssdp_description' - - -def ensure_domain_data(hass): - """Ensure hass.data is filled properly.""" - hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) - hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) - hass.data[DOMAIN]['sensors'] = hass.data[DOMAIN].get('sensors', {}) - hass.data[DOMAIN]['discovered'] = hass.data[DOMAIN].get('discovered', {}) - hass.data[DOMAIN]['auto_config'] = hass.data[DOMAIN].get('auto_config', { - 'active': False, - 'port_forward': False, - 'sensors': False, - }) From 85c40d29ee72c487c0fb288c31d54b9c8a8928d9 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sun, 2 Sep 2018 17:03:31 +0200 Subject: [PATCH 11/24] Linting --- homeassistant/components/sensor/igd.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/igd.py index cb0ceb752b0..908e465aedb 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/igd.py @@ -133,7 +133,7 @@ class PerSecondIGDSensor(Entity): @property def unit(self) -> str: - """Unit we are measuring in.""" + """Get unit we are measuring in.""" raise NotImplementedError() @property @@ -196,11 +196,11 @@ class KBytePerSecondIGDSensor(PerSecondIGDSensor): @property def unit(self) -> str: - """Unit we are measuring in.""" + """Get unit we are measuring in.""" return 'kbyte' async def _async_fetch_value(self) -> float: - """""" + """Fetch value from device.""" if self._direction == IN: return await self._device.async_get_total_bytes_received() @@ -220,11 +220,11 @@ class PacketsPerSecondIGDSensor(PerSecondIGDSensor): @property def unit(self) -> str: - """Unit we are measuring in.""" + """Get unit we are measuring in.""" return 'packets' async def _async_fetch_value(self) -> float: - """""" + """Fetch value from device.""" if self._direction == IN: return await self._device.async_get_total_packets_received() From f4a54e2483aa803e0d91d97c459f77c160643b1d Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Mon, 3 Sep 2018 21:16:45 +0200 Subject: [PATCH 12/24] Changes after review --- homeassistant/components/igd/__init__.py | 36 ++++++++++++++---------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 37445350fa6..4f7bc9606ac 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -51,7 +51,7 @@ CONFIG_SCHEMA = vol.Schema({ async def _async_create_igd_device(hass: HomeAssistantType, ssdp_description: str): - """.""" + """Create IGD device.""" # build requester from async_upnp_client.aiohttp import AiohttpSessionRequester session = async_get_clientsession(hass) @@ -94,30 +94,36 @@ async def _async_add_port_mapping(hass: HomeAssistantType, if local_ip == '127.0.0.1': _LOGGER.warning('Could not create port mapping, our IP is 127.0.0.1') - return False + return local_ip = IPv4Address(local_ip) # create port mapping + from async_upnp_client import UpnpError port = hass.http.server_port _LOGGER.debug('Creating port mapping %s:%s:%s (TCP)', port, local_ip, port) - await igd_device.async_add_port_mapping(remote_host=None, - external_port=port, - protocol='TCP', - internal_port=port, - internal_client=local_ip, - enabled=True, - description="Home Assistant", - lease_duration=None) - - return True + try: + await igd_device.async_add_port_mapping(remote_host=None, + external_port=port, + protocol='TCP', + internal_port=port, + internal_client=local_ip, + enabled=True, + description="Home Assistant", + lease_duration=None) + except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError): + _LOGGER.warning('Could not add port mapping') async def _async_delete_port_mapping(hass: HomeAssistantType, igd_device): """Remove a port mapping.""" + from async_upnp_client import UpnpError port = hass.http.server_port - await igd_device.async_delete_port_mapping(remote_host=None, - external_port=port, - protocol='TCP') + try: + await igd_device.async_delete_port_mapping(remote_host=None, + external_port=port, + protocol='TCP') + except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError): + _LOGGER.warning('Could not delete port mapping') # config From 6433f2e2e315b9787eb7e80a4ea8aa5f453d239e Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Sat, 8 Sep 2018 00:11:23 +0200 Subject: [PATCH 13/24] Working on igd --- homeassistant/components/igd/__init__.py | 128 +++++-------------- homeassistant/components/igd/config_flow.py | 1 + homeassistant/components/igd/const.py | 9 +- homeassistant/components/igd/device.py | 131 ++++++++++++++++++++ homeassistant/components/sensor/igd.py | 16 ++- tests/components/igd/test_init.py | 104 +++++++++++----- 6 files changed, 255 insertions(+), 134 deletions(-) create mode 100644 homeassistant/components/igd/device.py diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 4f7bc9606ac..118cd82be12 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -6,7 +6,6 @@ https://home-assistant.io/components/igd/ """ import asyncio -from ipaddress import IPv4Address from ipaddress import ip_address import aiohttp @@ -17,26 +16,24 @@ from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.exceptions import PlatformNotReady from homeassistant.helpers import config_validation as cv from homeassistant.helpers import discovery -from homeassistant.helpers.aiohttp_client import async_get_clientsession -from homeassistant.helpers.typing import ConfigType, HomeAssistantType -from homeassistant.util import get_local_ip +from homeassistant.helpers.typing import ConfigType +from homeassistant.helpers.typing import HomeAssistantType from homeassistant.components.discovery import DOMAIN as DISCOVERY_DOMAIN from .const import ( CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS, - CONF_UDN, CONF_SSDP_DESCRIPTION + CONF_HASS, CONF_LOCAL_IP, CONF_PORTS, + CONF_UDN, CONF_SSDP_DESCRIPTION, ) from .const import DOMAIN from .const import LOGGER as _LOGGER from .config_flow import ensure_domain_data +from .device import Device REQUIREMENTS = ['async-upnp-client==0.12.4'] DEPENDENCIES = ['http'] -CONF_LOCAL_IP = 'local_ip' -CONF_PORTS = 'ports' - NOTIFICATION_ID = 'igd_notification' NOTIFICATION_TITLE = 'UPnP/IGD Setup' @@ -45,90 +42,16 @@ CONFIG_SCHEMA = vol.Schema({ vol.Optional(CONF_ENABLE_PORT_MAPPING, default=False): cv.boolean, vol.Optional(CONF_ENABLE_SENSORS, default=True): cv.boolean, vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), + vol.Optional(CONF_PORTS): + vol.Schema({vol.Any(CONF_HASS, cv.positive_int): cv.positive_int}) }), }, extra=vol.ALLOW_EXTRA) -async def _async_create_igd_device(hass: HomeAssistantType, - ssdp_description: str): - """Create IGD device.""" - # build requester - from async_upnp_client.aiohttp import AiohttpSessionRequester - session = async_get_clientsession(hass) - requester = AiohttpSessionRequester(session, True) - - # create upnp device - from async_upnp_client import UpnpFactory - factory = UpnpFactory(requester, disable_state_variable_validation=True) - try: - upnp_device = await factory.async_create_device(ssdp_description) - except (asyncio.TimeoutError, aiohttp.ClientError): - raise PlatformNotReady() - - # wrap with IgdDevice - from async_upnp_client.igd import IgdDevice - igd_device = IgdDevice(upnp_device, None) - return igd_device - - -def _store_device(hass: HomeAssistantType, udn, igd_device): - """Store an igd_device by udn.""" - if igd_device is not None: - hass.data[DOMAIN]['devices'][udn] = igd_device - elif udn in hass.data[DOMAIN]['devices']: - del hass.data[DOMAIN]['devices'][udn] - - -def _get_device(hass: HomeAssistantType, udn): - """Get an igd_device by udn.""" - return hass.data[DOMAIN]['devices'].get(udn) - - -async def _async_add_port_mapping(hass: HomeAssistantType, - igd_device, - local_ip=None): - """Create a port mapping.""" - # determine local ip, ensure sane IP - if local_ip is None: - local_ip = get_local_ip() - - if local_ip == '127.0.0.1': - _LOGGER.warning('Could not create port mapping, our IP is 127.0.0.1') - return - local_ip = IPv4Address(local_ip) - - # create port mapping - from async_upnp_client import UpnpError - port = hass.http.server_port - _LOGGER.debug('Creating port mapping %s:%s:%s (TCP)', port, local_ip, port) - try: - await igd_device.async_add_port_mapping(remote_host=None, - external_port=port, - protocol='TCP', - internal_port=port, - internal_client=local_ip, - enabled=True, - description="Home Assistant", - lease_duration=None) - except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError): - _LOGGER.warning('Could not add port mapping') - - -async def _async_delete_port_mapping(hass: HomeAssistantType, igd_device): - """Remove a port mapping.""" - from async_upnp_client import UpnpError - port = hass.http.server_port - try: - await igd_device.async_delete_port_mapping(remote_host=None, - external_port=port, - protocol='TCP') - except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError): - _LOGGER.warning('Could not delete port mapping') - - # config async def async_setup(hass: HomeAssistantType, config: ConfigType): """Register a port mapping for Home Assistant via UPnP.""" + _LOGGER.debug('async_setup: %s', config.get(DOMAIN)) ensure_domain_data(hass) # ensure sane config @@ -143,12 +66,22 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): if CONF_LOCAL_IP in igd_config: hass.data[DOMAIN]['local_ip'] = igd_config[CONF_LOCAL_IP] + # determine ports + ports = {} + if CONF_PORTS in igd_config: + ports = igd_config[CONF_PORTS] + + if CONF_HASS in ports: + internal_port = hass.http.server_port + ports[internal_port] = ports[CONF_HASS] + del ports[CONF_HASS] + hass.data[DOMAIN]['auto_config'] = { 'active': True, 'port_forward': igd_config[CONF_ENABLE_PORT_MAPPING], + 'ports': ports, 'sensors': igd_config[CONF_ENABLE_SENSORS], } - _LOGGER.debug('Enabled auto_config: %s', hass.data[DOMAIN]['auto_config']) return True @@ -157,27 +90,31 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Set up a bridge from a config entry.""" + _LOGGER.debug('async_setup_entry: %s', config_entry.data) ensure_domain_data(hass) data = config_entry.data # build IGD device ssdp_description = data[CONF_SSDP_DESCRIPTION] try: - igd_device = await _async_create_igd_device(hass, ssdp_description) + device = await Device.async_create_device(hass, ssdp_description) except (asyncio.TimeoutError, aiohttp.ClientError): raise PlatformNotReady() - _store_device(hass, igd_device.udn, igd_device) + hass.data[DOMAIN]['devices'][device.udn] = device # port mapping if data.get(CONF_ENABLE_PORT_MAPPING): local_ip = hass.data[DOMAIN].get('local_ip') - await _async_add_port_mapping(hass, igd_device, local_ip=local_ip) + ports = hass.data[DOMAIN]['auto_config']['ports'] + _LOGGER.debug('Enabling port mappings: %s', ports) + await device.async_add_port_mappings(ports, local_ip=local_ip) # sensors if data.get(CONF_ENABLE_SENSORS): + _LOGGER.debug('Enabling sensors') discovery_info = { - 'udn': data[CONF_UDN], + 'udn': device.udn, } hass_config = config_entry.data hass.async_create_task(discovery.async_load_platform( @@ -194,22 +131,25 @@ async def async_setup_entry(hass: HomeAssistantType, async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Unload a config entry.""" + _LOGGER.debug('async_unload_entry: %s', config_entry.data) data = config_entry.data udn = data[CONF_UDN] - igd_device = _get_device(hass, udn) - if igd_device is None: + if udn not in hass.data[DOMAIN]['devices']: return True + device = hass.data[DOMAIN]['devices'][udn] # port mapping if data.get(CONF_ENABLE_PORT_MAPPING): - await _async_delete_port_mapping(hass, igd_device) + _LOGGER.debug('Deleting port mappings') + await device.async_delete_port_mappings() # sensors for sensor in hass.data[DOMAIN]['sensors'].get(udn, []): + _LOGGER.debug('Deleting sensor: %s', sensor) await sensor.async_remove() # clear stored device - _store_device(hass, udn, None) + del hass.data[DOMAIN]['devices'][udn] return True diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index 6f0cec12b5b..7af6c683ed9 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -21,6 +21,7 @@ def ensure_domain_data(hass): 'active': False, 'port_forward': False, 'sensors': False, + 'ports': {}, }) diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/igd/const.py index 47ff9f11074..8ba774447da 100644 --- a/homeassistant/components/igd/const.py +++ b/homeassistant/components/igd/const.py @@ -2,9 +2,12 @@ import logging -DOMAIN = 'igd' -LOGGER = logging.getLogger('homeassistant.components.igd') CONF_ENABLE_PORT_MAPPING = 'port_forward' CONF_ENABLE_SENSORS = 'sensors' -CONF_UDN = 'udn' +CONF_HASS = 'hass' +CONF_LOCAL_IP = 'local_ip' +CONF_PORTS = 'ports' CONF_SSDP_DESCRIPTION = 'ssdp_description' +CONF_UDN = 'udn' +DOMAIN = 'igd' +LOGGER = logging.getLogger('homeassistant.components.igd') diff --git a/homeassistant/components/igd/device.py b/homeassistant/components/igd/device.py new file mode 100644 index 00000000000..af16623e4b2 --- /dev/null +++ b/homeassistant/components/igd/device.py @@ -0,0 +1,131 @@ +"""Hass representation of an IGD.""" + +import asyncio +from ipaddress import IPv4Address + +import aiohttp + +from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.typing import HomeAssistantType +from homeassistant.util import get_local_ip + +from .const import LOGGER as _LOGGER + + +class Device: + """Hass representation of an IGD.""" + + def __init__(self, igd_device): + """Initializer.""" + self._igd_device = igd_device + self._mapped_ports = [] + + @classmethod + async def async_create_device(cls, + hass: HomeAssistantType, + ssdp_description: str): + """Create IGD device.""" + # build async_upnp_client requester + from async_upnp_client.aiohttp import AiohttpSessionRequester + session = async_get_clientsession(hass) + requester = AiohttpSessionRequester(session, True) + + # create async_upnp_client device + from async_upnp_client import UpnpFactory + factory = UpnpFactory(requester, + disable_state_variable_validation=True) + upnp_device = await factory.async_create_device(ssdp_description) + + # wrap with async_upnp_client IgdDevice + from async_upnp_client.igd import IgdDevice + igd_device = IgdDevice(upnp_device, None) + + return Device(igd_device) + + @property + def udn(self): + """Get the UDN.""" + return self._igd_device.udn + + @property + def name(self): + """Get the name.""" + return self._igd_device.name + + async def async_add_port_mappings(self, ports, local_ip=None): + """Add port mappings.""" + # determine local ip, ensure sane IP + if local_ip is None: + local_ip = get_local_ip() + + if local_ip == '127.0.0.1': + _LOGGER.error( + 'Could not create port mapping, our IP is 127.0.0.1') + local_ip = IPv4Address(local_ip) + + # create port mappings + for external_port, internal_port in ports.items(): + await self._async_add_port_mapping(external_port, + local_ip, + internal_port) + self._mapped_ports.append(external_port) + + async def _async_add_port_mapping(self, + external_port, + local_ip, + internal_port): + """Add a port mapping.""" + # create port mapping + from async_upnp_client import UpnpError + _LOGGER.info('Creating port mapping %s:%s:%s (TCP)', + external_port, local_ip, internal_port) + try: + await self._igd_device.async_add_port_mapping( + remote_host=None, + external_port=external_port, + protocol='TCP', + internal_port=internal_port, + internal_client=local_ip, + enabled=True, + description="Home Assistant", + lease_duration=None) + + self._mapped_ports.append(external_port) + except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError): + _LOGGER.error('Could not add port mapping: %s:%s:%s', + external_port, local_ip, internal_port) + + async def async_delete_port_mappings(self): + """Remove a port mapping.""" + for port in self._mapped_ports: + await self._async_delete_port_mapping(port) + + async def _async_delete_port_mapping(self, external_port): + """Remove a port mapping.""" + from async_upnp_client import UpnpError + try: + await self._igd_device.async_delete_port_mapping( + remote_host=None, + external_port=external_port, + protocol='TCP') + + self._mapped_ports.remove(external_port) + except (asyncio.TimeoutError, aiohttp.ClientError, UpnpError): + _LOGGER.error('Could not delete port mapping') + + async def async_get_total_bytes_received(self): + """Get total bytes received.""" + return await self._igd_device.async_get_total_bytes_received() + + async def async_get_total_bytes_sent(self): + """Get total bytes sent.""" + return await self._igd_device.async_get_total_bytes_sent() + + async def async_get_total_packets_received(self): + """Get total packets received.""" + # pylint: disable=invalid-name + return await self._igd_device.async_get_total_packets_received() + + async def async_get_total_packets_sent(self): + """Get total packets sent.""" + return await self._igd_device.async_get_total_packets_sent() diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/igd.py index 908e465aedb..ff93af89f34 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/igd.py @@ -14,7 +14,7 @@ from homeassistant.helpers.entity import Entity _LOGGER = logging.getLogger(__name__) -DEPENDENCIES = ['igd', 'history'] +DEPENDENCIES = ['igd'] BYTES_RECEIVED = 'bytes_received' BYTES_SENT = 'bytes_sent' @@ -52,18 +52,18 @@ async def async_setup_platform(hass, config, async_add_devices, return udn = discovery_info['udn'] - igd_device = hass.data[DOMAIN]['devices'][udn] + device = hass.data[DOMAIN]['devices'][udn] # raw sensors + per-second sensors sensors = [ - RawIGDSensor(igd_device, name, sensor_type) + RawIGDSensor(device, name, sensor_type) for name, sensor_type in SENSOR_TYPES.items() ] sensors += [ - KBytePerSecondIGDSensor(igd_device, IN), - KBytePerSecondIGDSensor(igd_device, OUT), - PacketsPerSecondIGDSensor(igd_device, IN), - PacketsPerSecondIGDSensor(igd_device, OUT), + KBytePerSecondIGDSensor(device, IN), + KBytePerSecondIGDSensor(device, OUT), + PacketsPerSecondIGDSensor(device, IN), + PacketsPerSecondIGDSensor(device, OUT), ] hass.data[DOMAIN]['sensors'][udn] = sensors async_add_devices(sensors, True) @@ -108,7 +108,6 @@ class RawIGDSensor(Entity): async def async_update(self): """Get the latest information from the IGD.""" - _LOGGER.debug('%s: async_update', self) if self._type_name == BYTES_RECEIVED: self._state = await self._device.async_get_total_bytes_received() elif self._type_name == BYTES_SENT: @@ -171,7 +170,6 @@ class PerSecondIGDSensor(Entity): async def async_update(self): """Get the latest information from the IGD.""" - _LOGGER.debug('%s: async_update', self) new_value = await self._async_fetch_value() if self._last_value is None: diff --git a/tests/components/igd/test_init.py b/tests/components/igd/test_init.py index cd3f5324bb4..336935710d8 100644 --- a/tests/components/igd/test_init.py +++ b/tests/components/igd/test_init.py @@ -1,15 +1,51 @@ """Test IGD setup process.""" +from ipaddress import ip_address from unittest.mock import patch, MagicMock from homeassistant.setup import async_setup_component from homeassistant.components import igd +from homeassistant.components.igd.device import Device from homeassistant.const import EVENT_HOMEASSISTANT_STOP from tests.common import MockConfigEntry from tests.common import mock_coro +class MockDevice(Device): + """Mock device for Device.""" + + def __init__(self, udn): + """Initializer.""" + super().__init__(None) + self._udn = udn + self.added_port_mappings = [] + self.removed_port_mappings = [] + + @classmethod + async def async_create_device(cls, hass, ssdp_description): + """Return self.""" + return cls() + + @property + def udn(self): + """Get the UDN.""" + return self._udn + + async def _async_add_port_mapping(self, + external_port, + local_ip, + internal_port): + """Add a port mapping.""" + entry = [external_port, local_ip, internal_port] + self.added_port_mappings.append(entry) + + async def _async_delete_port_mapping(self, external_port): + """Remove a port mapping.""" + entry = external_port + self.removed_port_mappings.append(entry) + + async def test_async_setup_no_auto_config(hass): """Test async_setup.""" # setup component, enable auto_config @@ -18,6 +54,7 @@ async def test_async_setup_no_auto_config(hass): assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': False, 'port_forward': False, + 'ports': {}, 'sensors': False, } @@ -30,6 +67,7 @@ async def test_async_setup_auto_config(hass): assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': False, + 'ports': {}, 'sensors': True, } @@ -38,12 +76,13 @@ async def test_async_setup_auto_config_port_forward(hass): """Test async_setup.""" # setup component, enable auto_config await async_setup_component(hass, 'igd', { - 'igd': {'port_forward': True}, + 'igd': {'port_forward': True, 'ports': {8123: 8123}}, 'discovery': {}}) assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': True, + 'ports': {8123: 8123}, 'sensors': True, } @@ -58,6 +97,7 @@ async def test_async_setup_auto_config_no_sensors(hass): assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': False, + 'ports': {}, 'sensors': False, } @@ -75,27 +115,30 @@ async def test_async_setup_entry_default(hass): # ensure hass.http is available await async_setup_component(hass, 'igd') - # mock async_upnp_client.igd.IgdDevice - mock_igd_device = MagicMock() - mock_igd_device.udn = udn - mock_igd_device.async_add_port_mapping.return_value = mock_coro() - mock_igd_device.async_delete_port_mapping.return_value = mock_coro() - with patch.object(igd, '_async_create_igd_device') as mock_create_device: + # mock homeassistant.components.igd.device.Device + mock_device = MagicMock() + mock_device.udn = udn + mock_device.async_add_port_mappings.return_value = mock_coro() + mock_device.async_delete_port_mappings.return_value = mock_coro() + with patch.object(Device, 'async_create_device') as mock_create_device: mock_create_device.return_value = mock_coro( - return_value=mock_igd_device) - with patch('homeassistant.components.igd.get_local_ip', + return_value=mock_device) + with patch('homeassistant.components.igd.device.get_local_ip', return_value='192.168.1.10'): assert await igd.async_setup_entry(hass, entry) is True # ensure device is stored/used - assert hass.data[igd.DOMAIN]['devices'][udn] == mock_igd_device + assert hass.data[igd.DOMAIN]['devices'][udn] == mock_device - hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) - await hass.async_block_till_done() + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) + await hass.async_block_till_done() + # ensure cleaned up assert udn not in hass.data[igd.DOMAIN]['devices'] - assert len(mock_igd_device.async_add_port_mapping.mock_calls) == 0 - assert len(mock_igd_device.async_delete_port_mapping.mock_calls) == 0 + + # ensure no port-mapping-methods called + assert len(mock_device.async_add_port_mappings.mock_calls) == 0 + assert len(mock_device.async_delete_port_mappings.mock_calls) == 0 async def test_async_setup_entry_port_forward(hass): @@ -109,25 +152,30 @@ async def test_async_setup_entry_port_forward(hass): }) # ensure hass.http is available - await async_setup_component(hass, 'igd') + await async_setup_component(hass, 'igd', { + 'igd': {'port_forward': True, 'ports': {8123: 8123}}, + 'discovery': {}}) - mock_igd_device = MagicMock() - mock_igd_device.udn = udn - mock_igd_device.async_add_port_mapping.return_value = mock_coro() - mock_igd_device.async_delete_port_mapping.return_value = mock_coro() - with patch.object(igd, '_async_create_igd_device') as mock_create_device: - mock_create_device.return_value = mock_coro( - return_value=mock_igd_device) - with patch('homeassistant.components.igd.get_local_ip', + mock_device = MockDevice(udn) + with patch.object(Device, 'async_create_device') as mock_create_device: + mock_create_device.return_value = mock_coro(return_value=mock_device) + with patch('homeassistant.components.igd.device.get_local_ip', return_value='192.168.1.10'): assert await igd.async_setup_entry(hass, entry) is True # ensure device is stored/used - assert hass.data[igd.DOMAIN]['devices'][udn] == mock_igd_device + assert hass.data[igd.DOMAIN]['devices'][udn] == mock_device - hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) - await hass.async_block_till_done() + # ensure add-port-mapping-methods called + assert mock_device.added_port_mappings == [ + [8123, ip_address('192.168.1.10'), 8123] + ] + hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) + await hass.async_block_till_done() + + # ensure cleaned up assert udn not in hass.data[igd.DOMAIN]['devices'] - assert len(mock_igd_device.async_add_port_mapping.mock_calls) > 0 - assert len(mock_igd_device.async_delete_port_mapping.mock_calls) > 0 + + # ensure delete-port-mapping-methods called + assert mock_device.removed_port_mappings == [8123] From ca7911ec470a51254ec077d64779504ace735791 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Fri, 14 Sep 2018 21:30:08 +0200 Subject: [PATCH 14/24] Set ports (hass -> hass) when no ports have been specified --- homeassistant/components/igd/__init__.py | 27 +++++++++++++++------ homeassistant/components/igd/config_flow.py | 2 +- tests/components/igd/test_init.py | 21 ++++++++++------ 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/igd/__init__.py index 118cd82be12..1940430bdc1 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/igd/__init__.py @@ -43,11 +43,24 @@ CONFIG_SCHEMA = vol.Schema({ vol.Optional(CONF_ENABLE_SENSORS, default=True): cv.boolean, vol.Optional(CONF_LOCAL_IP): vol.All(ip_address, cv.string), vol.Optional(CONF_PORTS): - vol.Schema({vol.Any(CONF_HASS, cv.positive_int): cv.positive_int}) + vol.Schema({ + vol.Any(CONF_HASS, cv.positive_int): + vol.Any(CONF_HASS, cv.positive_int) + }) }), }, extra=vol.ALLOW_EXTRA) +def _substitute_hass_ports(ports, hass_port): + # substitute 'hass' for hass_port, both sides + if CONF_HASS in ports: + ports[hass_port] = ports[CONF_HASS] + del ports[CONF_HASS] + for port in ports: + if ports[port] == CONF_HASS: + ports[port] = hass_port + + # config async def async_setup(hass: HomeAssistantType, config: ConfigType): """Register a port mapping for Home Assistant via UPnP.""" @@ -62,20 +75,17 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): _LOGGER.warning('IGD needs discovery, please enable it') return False + # overridden local ip igd_config = config[DOMAIN] if CONF_LOCAL_IP in igd_config: hass.data[DOMAIN]['local_ip'] = igd_config[CONF_LOCAL_IP] # determine ports - ports = {} + ports = {CONF_HASS: CONF_HASS} # default, port_forward disabled by default if CONF_PORTS in igd_config: + # copy from config ports = igd_config[CONF_PORTS] - if CONF_HASS in ports: - internal_port = hass.http.server_port - ports[internal_port] = ports[CONF_HASS] - del ports[CONF_HASS] - hass.data[DOMAIN]['auto_config'] = { 'active': True, 'port_forward': igd_config[CONF_ENABLE_PORT_MAPPING], @@ -108,6 +118,9 @@ async def async_setup_entry(hass: HomeAssistantType, local_ip = hass.data[DOMAIN].get('local_ip') ports = hass.data[DOMAIN]['auto_config']['ports'] _LOGGER.debug('Enabling port mappings: %s', ports) + + hass_port = hass.http.server_port + _substitute_hass_ports(ports, hass_port) await device.async_add_port_mappings(ports, local_ip=local_ip) # sensors diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/igd/config_flow.py index 7af6c683ed9..1ad4b2a41de 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/igd/config_flow.py @@ -21,7 +21,7 @@ def ensure_domain_data(hass): 'active': False, 'port_forward': False, 'sensors': False, - 'ports': {}, + 'ports': {'hass': 'hass'}, }) diff --git a/tests/components/igd/test_init.py b/tests/components/igd/test_init.py index 336935710d8..048313affb7 100644 --- a/tests/components/igd/test_init.py +++ b/tests/components/igd/test_init.py @@ -54,7 +54,7 @@ async def test_async_setup_no_auto_config(hass): assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': False, 'port_forward': False, - 'ports': {}, + 'ports': {'hass': 'hass'}, 'sensors': False, } @@ -67,7 +67,7 @@ async def test_async_setup_auto_config(hass): assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': False, - 'ports': {}, + 'ports': {'hass': 'hass'}, 'sensors': True, } @@ -76,13 +76,16 @@ async def test_async_setup_auto_config_port_forward(hass): """Test async_setup.""" # setup component, enable auto_config await async_setup_component(hass, 'igd', { - 'igd': {'port_forward': True, 'ports': {8123: 8123}}, + 'igd': { + 'port_forward': True, + 'ports': {'hass': 'hass'}, + }, 'discovery': {}}) assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': True, - 'ports': {8123: 8123}, + 'ports': {'hass': 'hass'}, 'sensors': True, } @@ -97,7 +100,7 @@ async def test_async_setup_auto_config_no_sensors(hass): assert hass.data[igd.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': False, - 'ports': {}, + 'ports': {'hass': 'hass'}, 'sensors': False, } @@ -153,8 +156,12 @@ async def test_async_setup_entry_port_forward(hass): # ensure hass.http is available await async_setup_component(hass, 'igd', { - 'igd': {'port_forward': True, 'ports': {8123: 8123}}, - 'discovery': {}}) + 'igd': { + 'port_forward': True, + 'ports': {'hass': 'hass'}, + }, + 'discovery': {}, + }) mock_device = MockDevice(udn) with patch.object(Device, 'async_create_device') as mock_create_device: From 6588dceadded82ac5ad21e5182a1f6b5f861fbab Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Mon, 17 Sep 2018 21:34:39 +0200 Subject: [PATCH 15/24] Fix uppercase Fix translation --- homeassistant/components/igd/.translations/en.json | 2 +- homeassistant/components/igd/.translations/nl.json | 4 ++-- homeassistant/components/igd/strings.json | 7 +++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/igd/.translations/en.json b/homeassistant/components/igd/.translations/en.json index 3466b6cfa93..d6117f0052f 100644 --- a/homeassistant/components/igd/.translations/en.json +++ b/homeassistant/components/igd/.translations/en.json @@ -19,7 +19,7 @@ "abort": { "no_devices_discovered": "No IGDs discovered", "already_configured": "IGD is already configured", - "no_sensors_or_port_forward": "Enable at least sensors or Port forward", + "no_sensors_or_port_forward": "Enable at least sensors or port forward", "no_igds": "No IGDs discovered" } } diff --git a/homeassistant/components/igd/.translations/nl.json b/homeassistant/components/igd/.translations/nl.json index 5a42f6c3bea..9fdd831f74c 100644 --- a/homeassistant/components/igd/.translations/nl.json +++ b/homeassistant/components/igd/.translations/nl.json @@ -10,7 +10,7 @@ "data":{ "igd": "IGD", "sensors": "Verkeer sensors toevoegen", - "port_forward": "Maak port-forward voor Home Assistant" + "port_forward": "Maak port forward voor Home Assistant" } } }, @@ -19,7 +19,7 @@ "abort": { "no_devices_discovered": "Geen IGDs gevonden", "already_configured": "IGD is reeds geconfigureerd", - "no_sensors_or_port_forward": "Kies ten minste sensors of Port forward", + "no_sensors_or_port_forward": "Kies ten minste sensors of port forward", "no_igds": "Geen IGDs gevonden" } } diff --git a/homeassistant/components/igd/strings.json b/homeassistant/components/igd/strings.json index 3466b6cfa93..5ab50b3a7d0 100644 --- a/homeassistant/components/igd/strings.json +++ b/homeassistant/components/igd/strings.json @@ -17,10 +17,9 @@ "error": { }, "abort": { - "no_devices_discovered": "No IGDs discovered", - "already_configured": "IGD is already configured", - "no_sensors_or_port_forward": "Enable at least sensors or Port forward", - "no_igds": "No IGDs discovered" + "no_devices_discovered": "No UPnP/IGDs discovered", + "already_configured": "UPnP/IGD is already configured", + "no_sensors_or_port_forward": "Enable at least sensors or port forward" } } } From c19665fed41ecc2a233a59bc61b351f863f2fad1 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Mon, 17 Sep 2018 22:08:09 +0200 Subject: [PATCH 16/24] Rename igd to upnp --- homeassistant/components/discovery.py | 2 +- .../components/sensor/{igd.py => upnp.py} | 34 ++++++------ .../{igd => upnp}/.translations/en.json | 15 +++--- .../{igd => upnp}/.translations/nl.json | 15 +++--- .../components/{igd => upnp}/__init__.py | 23 ++++---- .../components/{igd => upnp}/config_flow.py | 28 +++++----- .../components/{igd => upnp}/const.py | 4 +- .../components/{igd => upnp}/device.py | 7 ++- .../components/{igd => upnp}/strings.json | 8 +-- homeassistant/config_entries.py | 2 +- tests/components/{igd => upnp}/__init__.py | 0 .../{igd => upnp}/test_config_flow.py | 42 +++++++-------- tests/components/{igd => upnp}/test_init.py | 54 +++++++++---------- 13 files changed, 115 insertions(+), 119 deletions(-) rename homeassistant/components/sensor/{igd.py => upnp.py} (88%) rename homeassistant/components/{igd => upnp}/.translations/en.json (55%) rename homeassistant/components/{igd => upnp}/.translations/nl.json (55%) rename homeassistant/components/{igd => upnp}/__init__.py (90%) rename homeassistant/components/{igd => upnp}/config_flow.py (86%) rename homeassistant/components/{igd => upnp}/const.py (77%) rename homeassistant/components/{igd => upnp}/device.py (97%) rename homeassistant/components/{igd => upnp}/strings.json (77%) rename tests/components/{igd => upnp}/__init__.py (100%) rename tests/components/{igd => upnp}/test_config_flow.py (87%) rename tests/components/{igd => upnp}/test_init.py (75%) diff --git a/homeassistant/components/discovery.py b/homeassistant/components/discovery.py index 8009f01222b..092e8b7b578 100644 --- a/homeassistant/components/discovery.py +++ b/homeassistant/components/discovery.py @@ -49,7 +49,7 @@ CONFIG_ENTRY_HANDLERS = { 'google_cast': 'cast', SERVICE_HUE: 'hue', 'sonos': 'sonos', - 'igd': 'igd', + 'upnp': 'igd', } SERVICE_HANDLERS = { diff --git a/homeassistant/components/sensor/igd.py b/homeassistant/components/sensor/upnp.py similarity index 88% rename from homeassistant/components/sensor/igd.py rename to homeassistant/components/sensor/upnp.py index ff93af89f34..3c3745145a4 100644 --- a/homeassistant/components/sensor/igd.py +++ b/homeassistant/components/sensor/upnp.py @@ -1,20 +1,20 @@ """ -Support for IGD Sensors. +Support for UPnP/IGD Sensors. For more details about this platform, please refer to the documentation at -https://home-assistant.io/components/sensor.igd/ +https://home-assistant.io/components/sensor.upnp/ """ # pylint: disable=invalid-name from datetime import datetime import logging -from homeassistant.components.igd import DOMAIN +from homeassistant.components.upnp import DOMAIN from homeassistant.helpers.entity import Entity _LOGGER = logging.getLogger(__name__) -DEPENDENCIES = ['igd'] +DEPENDENCIES = ['upnp'] BYTES_RECEIVED = 'bytes_received' BYTES_SENT = 'bytes_sent' @@ -47,7 +47,7 @@ KBYTE = 1024 async def async_setup_platform(hass, config, async_add_devices, discovery_info=None): - """Set up the IGD sensors.""" + """Set up the UPnP/IGD sensors.""" if discovery_info is None: return @@ -56,25 +56,25 @@ async def async_setup_platform(hass, config, async_add_devices, # raw sensors + per-second sensors sensors = [ - RawIGDSensor(device, name, sensor_type) + RawUPnPIGDSensor(device, name, sensor_type) for name, sensor_type in SENSOR_TYPES.items() ] sensors += [ - KBytePerSecondIGDSensor(device, IN), - KBytePerSecondIGDSensor(device, OUT), - PacketsPerSecondIGDSensor(device, IN), - PacketsPerSecondIGDSensor(device, OUT), + KBytePerSecondUPnPIGDSensor(device, IN), + KBytePerSecondUPnPIGDSensor(device, OUT), + PacketsPerSecondUPnPIGDSensor(device, IN), + PacketsPerSecondUPnPIGDSensor(device, OUT), ] hass.data[DOMAIN]['sensors'][udn] = sensors async_add_devices(sensors, True) return True -class RawIGDSensor(Entity): - """Representation of a UPnP IGD sensor.""" +class RawUPnPIGDSensor(Entity): + """Representation of a UPnP/IGD sensor.""" def __init__(self, device, sensor_type_name, sensor_type): - """Initialize the IGD sensor.""" + """Initialize the UPnP/IGD sensor.""" self._device = device self._type_name = sensor_type_name self._type = sensor_type @@ -118,7 +118,7 @@ class RawIGDSensor(Entity): self._state = await self._device.async_get_total_packets_sent() -class PerSecondIGDSensor(Entity): +class PerSecondUPnPIGDSensor(Entity): """Abstract representation of a X Sent/Received per second sensor.""" def __init__(self, device, direction): @@ -169,7 +169,7 @@ class PerSecondIGDSensor(Entity): return new_value < self._last_value async def async_update(self): - """Get the latest information from the IGD.""" + """Get the latest information from the UPnP/IGD.""" new_value = await self._async_fetch_value() if self._last_value is None: @@ -189,7 +189,7 @@ class PerSecondIGDSensor(Entity): self._last_update_time = now -class KBytePerSecondIGDSensor(PerSecondIGDSensor): +class KBytePerSecondUPnPIGDSensor(PerSecondUPnPIGDSensor): """Representation of a KBytes Sent/Received per second sensor.""" @property @@ -213,7 +213,7 @@ class KBytePerSecondIGDSensor(PerSecondIGDSensor): return format(float(self._state / KBYTE), '.1f') -class PacketsPerSecondIGDSensor(PerSecondIGDSensor): +class PacketsPerSecondUPnPIGDSensor(PerSecondUPnPIGDSensor): """Representation of a Packets Sent/Received per second sensor.""" @property diff --git a/homeassistant/components/igd/.translations/en.json b/homeassistant/components/upnp/.translations/en.json similarity index 55% rename from homeassistant/components/igd/.translations/en.json rename to homeassistant/components/upnp/.translations/en.json index d6117f0052f..829631254ad 100644 --- a/homeassistant/components/igd/.translations/en.json +++ b/homeassistant/components/upnp/.translations/en.json @@ -1,14 +1,14 @@ { "config": { - "title": "IGD", + "title": "UPnP/IGD", "step": { "init": { - "title": "IGD" + "title": "UPnP/IGD" }, "user": { - "title": "Configuration options for the IGD", + "title": "Configuration options for the UPnP/IGD", "data":{ - "igd": "IGD", + "igd": "UPnP/IGD", "sensors": "Add traffic sensors", "port_forward": "Enable port forward for Home Assistant" } @@ -17,10 +17,9 @@ "error": { }, "abort": { - "no_devices_discovered": "No IGDs discovered", - "already_configured": "IGD is already configured", - "no_sensors_or_port_forward": "Enable at least sensors or port forward", - "no_igds": "No IGDs discovered" + "no_devices_discovered": "No UPnP/IGDs discovered", + "already_configured": "UPnP/IGD is already configured", + "no_sensors_or_port_forward": "Enable at least sensors or port forward" } } } diff --git a/homeassistant/components/igd/.translations/nl.json b/homeassistant/components/upnp/.translations/nl.json similarity index 55% rename from homeassistant/components/igd/.translations/nl.json rename to homeassistant/components/upnp/.translations/nl.json index 9fdd831f74c..02d8fcc0913 100644 --- a/homeassistant/components/igd/.translations/nl.json +++ b/homeassistant/components/upnp/.translations/nl.json @@ -1,14 +1,14 @@ { "config": { - "title": "IGD", + "title": "UPnP/IGD", "step": { "init": { - "title": "IGD" + "title": "UPnP/IGD" }, "user": { - "title": "Extra configuratie options voor IGD", + "title": "Extra configuratie options voor UPnP/IGD", "data":{ - "igd": "IGD", + "igd": "UPnP/IGD", "sensors": "Verkeer sensors toevoegen", "port_forward": "Maak port forward voor Home Assistant" } @@ -17,10 +17,9 @@ "error": { }, "abort": { - "no_devices_discovered": "Geen IGDs gevonden", - "already_configured": "IGD is reeds geconfigureerd", - "no_sensors_or_port_forward": "Kies ten minste sensors of port forward", - "no_igds": "Geen IGDs gevonden" + "no_devices_discovered": "Geen UPnP/IGDs gevonden", + "already_configured": "UPnP/IGD is reeds geconfigureerd", + "no_sensors_or_port_forward": "Kies ten minste sensors of port forward" } } } diff --git a/homeassistant/components/igd/__init__.py b/homeassistant/components/upnp/__init__.py similarity index 90% rename from homeassistant/components/igd/__init__.py rename to homeassistant/components/upnp/__init__.py index 1940430bdc1..a39ae210131 100644 --- a/homeassistant/components/igd/__init__.py +++ b/homeassistant/components/upnp/__init__.py @@ -2,9 +2,8 @@ Will open a port in your router for Home Assistant and provide statistics. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/igd/ +https://home-assistant.io/components/upnp/ """ - import asyncio from ipaddress import ip_address @@ -34,7 +33,7 @@ from .device import Device REQUIREMENTS = ['async-upnp-client==0.12.4'] DEPENDENCIES = ['http'] -NOTIFICATION_ID = 'igd_notification' +NOTIFICATION_ID = 'upnp_notification' NOTIFICATION_TITLE = 'UPnP/IGD Setup' CONFIG_SCHEMA = vol.Schema({ @@ -72,25 +71,25 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): return True if DISCOVERY_DOMAIN not in config: - _LOGGER.warning('IGD needs discovery, please enable it') + _LOGGER.warning('UPNP needs discovery, please enable it') return False # overridden local ip - igd_config = config[DOMAIN] - if CONF_LOCAL_IP in igd_config: - hass.data[DOMAIN]['local_ip'] = igd_config[CONF_LOCAL_IP] + upnp_config = config[DOMAIN] + if CONF_LOCAL_IP in upnp_config: + hass.data[DOMAIN]['local_ip'] = upnp_config[CONF_LOCAL_IP] # determine ports ports = {CONF_HASS: CONF_HASS} # default, port_forward disabled by default - if CONF_PORTS in igd_config: + if CONF_PORTS in upnp_config: # copy from config - ports = igd_config[CONF_PORTS] + ports = upnp_config[CONF_PORTS] hass.data[DOMAIN]['auto_config'] = { 'active': True, - 'port_forward': igd_config[CONF_ENABLE_PORT_MAPPING], + 'port_forward': upnp_config[CONF_ENABLE_PORT_MAPPING], 'ports': ports, - 'sensors': igd_config[CONF_ENABLE_SENSORS], + 'sensors': upnp_config[CONF_ENABLE_SENSORS], } return True @@ -104,7 +103,7 @@ async def async_setup_entry(hass: HomeAssistantType, ensure_domain_data(hass) data = config_entry.data - # build IGD device + # build UPnP/IGD device ssdp_description = data[CONF_SSDP_DESCRIPTION] try: device = await Device.async_create_device(hass, ssdp_description) diff --git a/homeassistant/components/igd/config_flow.py b/homeassistant/components/upnp/config_flow.py similarity index 86% rename from homeassistant/components/igd/config_flow.py rename to homeassistant/components/upnp/config_flow.py index 1ad4b2a41de..5c75d0e0517 100644 --- a/homeassistant/components/igd/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -1,4 +1,4 @@ -"""Config flow for IGD.""" +"""Config flow for UPNP.""" import voluptuous as vol from homeassistant import config_entries @@ -26,13 +26,13 @@ def ensure_domain_data(hass): @config_entries.HANDLERS.register(DOMAIN) -class IgdFlowHandler(data_entry_flow.FlowHandler): +class UpnpFlowHandler(data_entry_flow.FlowHandler): """Handle a Hue config flow.""" VERSION = 1 @property - def _configured_igds(self): + def _configured_upnp_igds(self): """Get all configured IGDs.""" return { entry.data[CONF_UDN]: { @@ -42,7 +42,7 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): } @property - def _discovered_igds(self): + def _discovered_upnp_igds(self): """Get all discovered entries.""" return self.hass.data[DOMAIN]['discovered'] @@ -57,7 +57,7 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): async def async_step_discovery(self, discovery_info): """ - Handle a discovered IGD. + Handle a discovered UPnP/IGD. This flow is triggered by the discovery component. It will check if the host is already configured and delegate to the import step if not. @@ -71,7 +71,7 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): # ensure not already discovered/configured udn = discovery_info['udn'] - if udn in self._configured_igds: + if udn in self._configured_upnp_igds: return self.async_abort(reason='already_configured') # auto config? @@ -98,19 +98,19 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): # ensure not already configured configured_names = [ entry['friendly_name'] - for udn, entry in self._discovered_igds.items() - if udn in self._configured_igds + for udn, entry in self._discovered_upnp_igds.items() + if udn in self._configured_upnp_igds ] if user_input['name'] in configured_names: return self.async_abort(reason='already_configured') return await self._async_save_entry(user_input) - # let user choose from all discovered, non-configured, IGDs + # let user choose from all discovered, non-configured, UPnP/IGDs names = [ entry['friendly_name'] - for udn, entry in self._discovered_igds.items() - if udn not in self._configured_igds + for udn, entry in self._discovered_upnp_igds.items() + if udn not in self._configured_upnp_igds ] if not names: return self.async_abort(reason='no_devices_discovered') @@ -125,15 +125,15 @@ class IgdFlowHandler(data_entry_flow.FlowHandler): ) async def async_step_import(self, import_info): - """Import a new IGD as a config entry.""" + """Import a new UPnP/IGD as a config entry.""" return await self._async_save_entry(import_info) async def _async_save_entry(self, import_info): - """Store IGD as new entry.""" + """Store UPNP/IGD as new entry.""" # ensure we know the host name = import_info['name'] discovery_infos = [info - for info in self._discovered_igds.values() + for info in self._discovered_upnp_igds.values() if info['friendly_name'] == name] if not discovery_infos: return self.async_abort(reason='host_not_found') diff --git a/homeassistant/components/igd/const.py b/homeassistant/components/upnp/const.py similarity index 77% rename from homeassistant/components/igd/const.py rename to homeassistant/components/upnp/const.py index 8ba774447da..0728747af3d 100644 --- a/homeassistant/components/igd/const.py +++ b/homeassistant/components/upnp/const.py @@ -9,5 +9,5 @@ CONF_LOCAL_IP = 'local_ip' CONF_PORTS = 'ports' CONF_SSDP_DESCRIPTION = 'ssdp_description' CONF_UDN = 'udn' -DOMAIN = 'igd' -LOGGER = logging.getLogger('homeassistant.components.igd') +DOMAIN = 'upnp' +LOGGER = logging.getLogger('homeassistant.components.upnp') diff --git a/homeassistant/components/igd/device.py b/homeassistant/components/upnp/device.py similarity index 97% rename from homeassistant/components/igd/device.py rename to homeassistant/components/upnp/device.py index af16623e4b2..6dce3889eaf 100644 --- a/homeassistant/components/igd/device.py +++ b/homeassistant/components/upnp/device.py @@ -1,5 +1,4 @@ -"""Hass representation of an IGD.""" - +"""Hass representation of an UPnP/IGD.""" import asyncio from ipaddress import IPv4Address @@ -13,7 +12,7 @@ from .const import LOGGER as _LOGGER class Device: - """Hass representation of an IGD.""" + """Hass representation of an UPnP/IGD.""" def __init__(self, igd_device): """Initializer.""" @@ -24,7 +23,7 @@ class Device: async def async_create_device(cls, hass: HomeAssistantType, ssdp_description: str): - """Create IGD device.""" + """Create UPnP/IGD device.""" # build async_upnp_client requester from async_upnp_client.aiohttp import AiohttpSessionRequester session = async_get_clientsession(hass) diff --git a/homeassistant/components/igd/strings.json b/homeassistant/components/upnp/strings.json similarity index 77% rename from homeassistant/components/igd/strings.json rename to homeassistant/components/upnp/strings.json index 5ab50b3a7d0..829631254ad 100644 --- a/homeassistant/components/igd/strings.json +++ b/homeassistant/components/upnp/strings.json @@ -1,14 +1,14 @@ { "config": { - "title": "IGD", + "title": "UPnP/IGD", "step": { "init": { - "title": "IGD" + "title": "UPnP/IGD" }, "user": { - "title": "Configuration options for the IGD", + "title": "Configuration options for the UPnP/IGD", "data":{ - "igd": "IGD", + "igd": "UPnP/IGD", "sensors": "Add traffic sensors", "port_forward": "Enable port forward for Home Assistant" } diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index 93c904d3b91..19f7c071726 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -143,7 +143,7 @@ FLOWS = [ 'nest', 'sonos', 'zone', - 'igd', + 'upnp', ] diff --git a/tests/components/igd/__init__.py b/tests/components/upnp/__init__.py similarity index 100% rename from tests/components/igd/__init__.py rename to tests/components/upnp/__init__.py diff --git a/tests/components/igd/test_config_flow.py b/tests/components/upnp/test_config_flow.py similarity index 87% rename from tests/components/igd/test_config_flow.py rename to tests/components/upnp/test_config_flow.py index e02d5630e96..f6ec05a42ab 100644 --- a/tests/components/igd/test_config_flow.py +++ b/tests/components/upnp/test_config_flow.py @@ -1,16 +1,16 @@ -"""Tests for IGD config flow.""" +"""Tests for UPnP/IGD config flow.""" -from homeassistant.components import igd -from homeassistant.components.igd import config_flow as igd_config_flow +from homeassistant.components import upnp +from homeassistant.components.upnp import config_flow as upnp_config_flow from tests.common import MockConfigEntry async def test_flow_none_discovered(hass): """Test no device discovered flow.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'discovered': {} } @@ -21,12 +21,12 @@ async def test_flow_none_discovered(hass): async def test_flow_already_configured(hass): """Test device already configured flow.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # discovered device udn = 'uuid:device_1' - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'discovered': { udn: { 'friendly_name': '192.168.1.1 (Test device)', @@ -37,7 +37,7 @@ async def test_flow_already_configured(hass): } # configured entry - MockConfigEntry(domain=igd.DOMAIN, data={ + MockConfigEntry(domain=upnp.DOMAIN, data={ 'udn': udn, 'host': '192.168.1.1', }).add_to_hass(hass) @@ -53,12 +53,12 @@ async def test_flow_already_configured(hass): async def test_flow_no_sensors_no_port_forward(hass): """Test single device, no sensors, no port_forward.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # discovered device udn = 'uuid:device_1' - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'discovered': { udn: { 'friendly_name': '192.168.1.1 (Test device)', @@ -69,7 +69,7 @@ async def test_flow_no_sensors_no_port_forward(hass): } # configured entry - MockConfigEntry(domain=igd.DOMAIN, data={ + MockConfigEntry(domain=upnp.DOMAIN, data={ 'udn': udn, 'host': '192.168.1.1', }).add_to_hass(hass) @@ -85,12 +85,12 @@ async def test_flow_no_sensors_no_port_forward(hass): async def test_flow_discovered_form(hass): """Test single device discovered, show form flow.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # discovered device udn = 'uuid:device_1' - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'discovered': { udn: { 'friendly_name': '192.168.1.1 (Test device)', @@ -107,13 +107,13 @@ async def test_flow_discovered_form(hass): async def test_flow_two_discovered_form(hass): """Test single device discovered, show form flow.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # discovered device udn_1 = 'uuid:device_1' udn_2 = 'uuid:device_2' - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'discovered': { udn_1: { 'friendly_name': '192.168.1.1 (Test device)', @@ -145,11 +145,11 @@ async def test_flow_two_discovered_form(hass): async def test_config_entry_created(hass): """Test config entry is created.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # discovered device - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'discovered': { 'uuid:device_1': { 'friendly_name': '192.168.1.1 (Test device)', @@ -178,11 +178,11 @@ async def test_config_entry_created(hass): async def test_flow_discovery_auto_config_sensors(hass): """Test creation of device with auto_config.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # auto_config active - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'auto_config': { 'active': True, 'port_forward': False, @@ -210,11 +210,11 @@ async def test_flow_discovery_auto_config_sensors(hass): async def test_flow_discovery_auto_config_sensors_port_forward(hass): """Test creation of device with auto_config, with port forward.""" - flow = igd_config_flow.IgdFlowHandler() + flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass # auto_config active, with port_forward - hass.data[igd.DOMAIN] = { + hass.data[upnp.DOMAIN] = { 'auto_config': { 'active': True, 'port_forward': True, diff --git a/tests/components/igd/test_init.py b/tests/components/upnp/test_init.py similarity index 75% rename from tests/components/igd/test_init.py rename to tests/components/upnp/test_init.py index 048313affb7..581abc3190c 100644 --- a/tests/components/igd/test_init.py +++ b/tests/components/upnp/test_init.py @@ -1,11 +1,11 @@ -"""Test IGD setup process.""" +"""Test UPnP/IGD setup process.""" from ipaddress import ip_address from unittest.mock import patch, MagicMock from homeassistant.setup import async_setup_component -from homeassistant.components import igd -from homeassistant.components.igd.device import Device +from homeassistant.components import upnp +from homeassistant.components.upnp.device import Device from homeassistant.const import EVENT_HOMEASSISTANT_STOP from tests.common import MockConfigEntry @@ -49,9 +49,9 @@ class MockDevice(Device): async def test_async_setup_no_auto_config(hass): """Test async_setup.""" # setup component, enable auto_config - await async_setup_component(hass, 'igd') + await async_setup_component(hass, 'upnp') - assert hass.data[igd.DOMAIN]['auto_config'] == { + assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': False, 'port_forward': False, 'ports': {'hass': 'hass'}, @@ -62,9 +62,9 @@ async def test_async_setup_no_auto_config(hass): async def test_async_setup_auto_config(hass): """Test async_setup.""" # setup component, enable auto_config - await async_setup_component(hass, 'igd', {'igd': {}, 'discovery': {}}) + await async_setup_component(hass, 'upnp', {'upnp': {}, 'discovery': {}}) - assert hass.data[igd.DOMAIN]['auto_config'] == { + assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': False, 'ports': {'hass': 'hass'}, @@ -75,14 +75,14 @@ async def test_async_setup_auto_config(hass): async def test_async_setup_auto_config_port_forward(hass): """Test async_setup.""" # setup component, enable auto_config - await async_setup_component(hass, 'igd', { - 'igd': { + await async_setup_component(hass, 'upnp', { + 'upnp': { 'port_forward': True, 'ports': {'hass': 'hass'}, }, 'discovery': {}}) - assert hass.data[igd.DOMAIN]['auto_config'] == { + assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': True, 'ports': {'hass': 'hass'}, @@ -93,11 +93,11 @@ async def test_async_setup_auto_config_port_forward(hass): async def test_async_setup_auto_config_no_sensors(hass): """Test async_setup.""" # setup component, enable auto_config - await async_setup_component(hass, 'igd', { - 'igd': {'sensors': False}, + await async_setup_component(hass, 'upnp', { + 'upnp': {'sensors': False}, 'discovery': {}}) - assert hass.data[igd.DOMAIN]['auto_config'] == { + assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': True, 'port_forward': False, 'ports': {'hass': 'hass'}, @@ -108,7 +108,7 @@ async def test_async_setup_auto_config_no_sensors(hass): async def test_async_setup_entry_default(hass): """Test async_setup_entry.""" udn = 'uuid:device_1' - entry = MockConfigEntry(domain=igd.DOMAIN, data={ + entry = MockConfigEntry(domain=upnp.DOMAIN, data={ 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': udn, 'sensors': True, @@ -116,9 +116,9 @@ async def test_async_setup_entry_default(hass): }) # ensure hass.http is available - await async_setup_component(hass, 'igd') + await async_setup_component(hass, 'upnp') - # mock homeassistant.components.igd.device.Device + # mock homeassistant.components.upnp.device.Device mock_device = MagicMock() mock_device.udn = udn mock_device.async_add_port_mappings.return_value = mock_coro() @@ -126,18 +126,18 @@ async def test_async_setup_entry_default(hass): with patch.object(Device, 'async_create_device') as mock_create_device: mock_create_device.return_value = mock_coro( return_value=mock_device) - with patch('homeassistant.components.igd.device.get_local_ip', + with patch('homeassistant.components.upnp.device.get_local_ip', return_value='192.168.1.10'): - assert await igd.async_setup_entry(hass, entry) is True + assert await upnp.async_setup_entry(hass, entry) is True # ensure device is stored/used - assert hass.data[igd.DOMAIN]['devices'][udn] == mock_device + assert hass.data[upnp.DOMAIN]['devices'][udn] == mock_device hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done() # ensure cleaned up - assert udn not in hass.data[igd.DOMAIN]['devices'] + assert udn not in hass.data[upnp.DOMAIN]['devices'] # ensure no port-mapping-methods called assert len(mock_device.async_add_port_mappings.mock_calls) == 0 @@ -147,7 +147,7 @@ async def test_async_setup_entry_default(hass): async def test_async_setup_entry_port_forward(hass): """Test async_setup_entry.""" udn = 'uuid:device_1' - entry = MockConfigEntry(domain=igd.DOMAIN, data={ + entry = MockConfigEntry(domain=upnp.DOMAIN, data={ 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': udn, 'sensors': False, @@ -155,8 +155,8 @@ async def test_async_setup_entry_port_forward(hass): }) # ensure hass.http is available - await async_setup_component(hass, 'igd', { - 'igd': { + await async_setup_component(hass, 'upnp', { + 'upnp': { 'port_forward': True, 'ports': {'hass': 'hass'}, }, @@ -166,12 +166,12 @@ async def test_async_setup_entry_port_forward(hass): mock_device = MockDevice(udn) with patch.object(Device, 'async_create_device') as mock_create_device: mock_create_device.return_value = mock_coro(return_value=mock_device) - with patch('homeassistant.components.igd.device.get_local_ip', + with patch('homeassistant.components.upnp.device.get_local_ip', return_value='192.168.1.10'): - assert await igd.async_setup_entry(hass, entry) is True + assert await upnp.async_setup_entry(hass, entry) is True # ensure device is stored/used - assert hass.data[igd.DOMAIN]['devices'][udn] == mock_device + assert hass.data[upnp.DOMAIN]['devices'][udn] == mock_device # ensure add-port-mapping-methods called assert mock_device.added_port_mappings == [ @@ -182,7 +182,7 @@ async def test_async_setup_entry_port_forward(hass): await hass.async_block_till_done() # ensure cleaned up - assert udn not in hass.data[igd.DOMAIN]['devices'] + assert udn not in hass.data[upnp.DOMAIN]['devices'] # ensure delete-port-mapping-methods called assert mock_device.removed_port_mappings == [8123] From a7a16e4317b9b61d3e01b8ec54ad499d73a3a600 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Mon, 17 Sep 2018 22:15:44 +0200 Subject: [PATCH 17/24] Fix tracebacks --- homeassistant/components/upnp/config_flow.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index 5c75d0e0517..95deed73e63 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -89,6 +89,8 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): async def async_step_user(self, user_input=None): """Manual set up.""" + ensure_domain_data(self.hass) + # if user input given, handle it user_input = user_input or {} if 'name' in user_input: @@ -126,10 +128,14 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): async def async_step_import(self, import_info): """Import a new UPnP/IGD as a config entry.""" + ensure_domain_data(self.hass) + return await self._async_save_entry(import_info) async def _async_save_entry(self, import_info): """Store UPNP/IGD as new entry.""" + ensure_domain_data(self.hass) + # ensure we know the host name = import_info['name'] discovery_infos = [info From 5cb0c92e78f8e7c1306931e69b4707841f3507d0 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Mon, 17 Sep 2018 22:25:32 +0200 Subject: [PATCH 18/24] Fix discovery/config entry handlers --- homeassistant/components/discovery.py | 2 +- homeassistant/components/upnp/__init__.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/homeassistant/components/discovery.py b/homeassistant/components/discovery.py index 092e8b7b578..75929e76462 100644 --- a/homeassistant/components/discovery.py +++ b/homeassistant/components/discovery.py @@ -49,7 +49,7 @@ CONFIG_ENTRY_HANDLERS = { 'google_cast': 'cast', SERVICE_HUE: 'hue', 'sonos': 'sonos', - 'upnp': 'igd', + 'igd': 'upnp', } SERVICE_HANDLERS = { diff --git a/homeassistant/components/upnp/__init__.py b/homeassistant/components/upnp/__init__.py index a39ae210131..25650e6e637 100644 --- a/homeassistant/components/upnp/__init__.py +++ b/homeassistant/components/upnp/__init__.py @@ -63,7 +63,6 @@ def _substitute_hass_ports(ports, hass_port): # config async def async_setup(hass: HomeAssistantType, config: ConfigType): """Register a port mapping for Home Assistant via UPnP.""" - _LOGGER.debug('async_setup: %s', config.get(DOMAIN)) ensure_domain_data(hass) # ensure sane config From b770acd9a73e75725c375350e443385891c2cd9c Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Tue, 18 Sep 2018 20:21:52 +0200 Subject: [PATCH 19/24] Update requirements_all.txt --- requirements_all.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_all.txt b/requirements_all.txt index 01ff0fa17b2..fc9c9dd8c47 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -141,7 +141,7 @@ apns2==0.3.0 # homeassistant.components.asterisk_mbox asterisk_mbox==0.5.0 -# homeassistant.components.igd +# homeassistant.components.upnp # homeassistant.components.media_player.dlna_dmr async-upnp-client==0.12.4 From b72499a949d158fd537c6766229ffc907d7f6e84 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Tue, 18 Sep 2018 20:22:33 +0200 Subject: [PATCH 20/24] Fixes after rename --- .coveragerc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.coveragerc b/.coveragerc index 078b03f21d2..c3c6f419d17 100644 --- a/.coveragerc +++ b/.coveragerc @@ -674,7 +674,7 @@ omit = homeassistant/components/sensor/haveibeenpwned.py homeassistant/components/sensor/hp_ilo.py homeassistant/components/sensor/htu21d.py - homeassistant/components/sensor/igd.py + homeassistant/components/sensor/upnp.py homeassistant/components/sensor/imap_email_content.py homeassistant/components/sensor/imap.py homeassistant/components/sensor/influxdb.py From 6e01ea5929920b6441752c2f1746d6dae20b32b8 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Thu, 20 Sep 2018 18:15:04 +0200 Subject: [PATCH 21/24] Preserve compatibility with original upnp --- homeassistant/components/upnp/const.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/upnp/const.py b/homeassistant/components/upnp/const.py index 0728747af3d..ad57bc7d7f4 100644 --- a/homeassistant/components/upnp/const.py +++ b/homeassistant/components/upnp/const.py @@ -2,7 +2,7 @@ import logging -CONF_ENABLE_PORT_MAPPING = 'port_forward' +CONF_ENABLE_PORT_MAPPING = 'port_mapping' CONF_ENABLE_SENSORS = 'sensors' CONF_HASS = 'hass' CONF_LOCAL_IP = 'local_ip' From d732f8eca2201e0ad8ade2abb282f3507647de1e Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Mon, 1 Oct 2018 18:25:54 +0200 Subject: [PATCH 22/24] Changes after review by @MartinHjelmare --- homeassistant/components/sensor/upnp.py | 90 ++++++++++++------- .../components/upnp/.translations/en.json | 6 +- .../components/upnp/.translations/nl.json | 6 +- homeassistant/components/upnp/__init__.py | 31 +++---- homeassistant/components/upnp/config_flow.py | 34 +++---- homeassistant/components/upnp/device.py | 3 +- homeassistant/components/upnp/strings.json | 6 +- tests/components/upnp/test_config_flow.py | 58 ++++++------ tests/components/upnp/test_init.py | 28 +++--- 9 files changed, 147 insertions(+), 115 deletions(-) diff --git a/homeassistant/components/sensor/upnp.py b/homeassistant/components/sensor/upnp.py index 3c3745145a4..e511b2947e5 100644 --- a/homeassistant/components/sensor/upnp.py +++ b/homeassistant/components/sensor/upnp.py @@ -8,8 +8,10 @@ https://home-assistant.io/components/sensor.upnp/ from datetime import datetime import logging -from homeassistant.components.upnp import DOMAIN +from homeassistant.core import callback +from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity import Entity +from homeassistant.components.upnp.const import DOMAIN as DATA_UPNP _LOGGER = logging.getLogger(__name__) @@ -45,37 +47,65 @@ OUT = 'sent' KBYTE = 1024 -async def async_setup_platform(hass, config, async_add_devices, +async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): - """Set up the UPnP/IGD sensors.""" - if discovery_info is None: - return - - udn = discovery_info['udn'] - device = hass.data[DOMAIN]['devices'][udn] - - # raw sensors + per-second sensors - sensors = [ - RawUPnPIGDSensor(device, name, sensor_type) - for name, sensor_type in SENSOR_TYPES.items() - ] - sensors += [ - KBytePerSecondUPnPIGDSensor(device, IN), - KBytePerSecondUPnPIGDSensor(device, OUT), - PacketsPerSecondUPnPIGDSensor(device, IN), - PacketsPerSecondUPnPIGDSensor(device, OUT), - ] - hass.data[DOMAIN]['sensors'][udn] = sensors - async_add_devices(sensors, True) - return True + """Old way of setting up UPnP/IGD sensors.""" + _LOGGER.debug('async_setup_platform: config: %s, discovery: %s', + config, discovery_info) -class RawUPnPIGDSensor(Entity): +async def async_setup_entry(hass, config_entry, async_add_entities): + """Set up the UPnP/IGD sensor.""" + @callback + def async_add_sensor(device): + """Add sensors from UPnP/IGD device.""" + # raw sensors + per-second sensors + sensors = [ + RawUPnPIGDSensor(device, name, sensor_type) + for name, sensor_type in SENSOR_TYPES.items() + ] + sensors += [ + KBytePerSecondUPnPIGDSensor(device, IN), + KBytePerSecondUPnPIGDSensor(device, OUT), + PacketsPerSecondUPnPIGDSensor(device, IN), + PacketsPerSecondUPnPIGDSensor(device, OUT), + ] + async_add_entities(sensors, True) + + data = config_entry.data + udn = data['udn'] + device = hass.data[DATA_UPNP]['devices'][udn] + async_add_sensor(device) + + +class UpnpSensor(Entity): + """Base class for UPnP/IGD sensors.""" + + def __init__(self, device): + """Initialize the base sensor.""" + self._device = device + + async def async_added_to_hass(self): + """Subscribe to sensors events.""" + async_dispatcher_connect(self.hass, + 'upnp_remove_sensor', + self._upnp_remove_sensor) + + def _upnp_remove_sensor(self, device): + """Remove sensor.""" + if self._device != device: + # not for us + return + + self.hass.async_create_task(self.async_remove()) + + +class RawUPnPIGDSensor(UpnpSensor): """Representation of a UPnP/IGD sensor.""" def __init__(self, device, sensor_type_name, sensor_type): """Initialize the UPnP/IGD sensor.""" - self._device = device + super().__init__(device) self._type_name = sensor_type_name self._type = sensor_type self._name = '{} {}'.format(device.name, sensor_type['name']) @@ -94,7 +124,7 @@ class RawUPnPIGDSensor(Entity): @property def state(self) -> str: """Return the state of the device.""" - return self._state + return format(self._state, 'd') @property def icon(self) -> str: @@ -118,12 +148,12 @@ class RawUPnPIGDSensor(Entity): self._state = await self._device.async_get_total_packets_sent() -class PerSecondUPnPIGDSensor(Entity): +class PerSecondUPnPIGDSensor(UpnpSensor): """Abstract representation of a X Sent/Received per second sensor.""" def __init__(self, device, direction): """Initializer.""" - self._device = device + super().__init__(device) self._direction = direction self._state = None @@ -205,7 +235,7 @@ class KBytePerSecondUPnPIGDSensor(PerSecondUPnPIGDSensor): return await self._device.async_get_total_bytes_sent() @property - def state(self): + def state(self) -> str: """Return the state of the device.""" if self._state is None: return None @@ -229,7 +259,7 @@ class PacketsPerSecondUPnPIGDSensor(PerSecondUPnPIGDSensor): return await self._device.async_get_total_packets_sent() @property - def state(self): + def state(self) -> str: """Return the state of the device.""" if self._state is None: return None diff --git a/homeassistant/components/upnp/.translations/en.json b/homeassistant/components/upnp/.translations/en.json index 829631254ad..682150b7ddd 100644 --- a/homeassistant/components/upnp/.translations/en.json +++ b/homeassistant/components/upnp/.translations/en.json @@ -9,8 +9,8 @@ "title": "Configuration options for the UPnP/IGD", "data":{ "igd": "UPnP/IGD", - "sensors": "Add traffic sensors", - "port_forward": "Enable port forward for Home Assistant" + "enable_sensors": "Add traffic sensors", + "enable_port_mapping": "Enable port mapping for Home Assistant" } } }, @@ -19,7 +19,7 @@ "abort": { "no_devices_discovered": "No UPnP/IGDs discovered", "already_configured": "UPnP/IGD is already configured", - "no_sensors_or_port_forward": "Enable at least sensors or port forward" + "no_sensors_or_port_mapping": "Enable at least sensors or port mapping" } } } diff --git a/homeassistant/components/upnp/.translations/nl.json b/homeassistant/components/upnp/.translations/nl.json index 02d8fcc0913..55a94a8ea6f 100644 --- a/homeassistant/components/upnp/.translations/nl.json +++ b/homeassistant/components/upnp/.translations/nl.json @@ -9,8 +9,8 @@ "title": "Extra configuratie options voor UPnP/IGD", "data":{ "igd": "UPnP/IGD", - "sensors": "Verkeer sensors toevoegen", - "port_forward": "Maak port forward voor Home Assistant" + "enable_sensors": "Verkeer sensors toevoegen", + "enable_port_mapping": "Maak port mapping voor Home Assistant" } } }, @@ -19,7 +19,7 @@ "abort": { "no_devices_discovered": "Geen UPnP/IGDs gevonden", "already_configured": "UPnP/IGD is reeds geconfigureerd", - "no_sensors_or_port_forward": "Kies ten minste sensors of port forward" + "no_sensors_or_port_mapping": "Kies ten minste sensors of port mapping" } } } diff --git a/homeassistant/components/upnp/__init__.py b/homeassistant/components/upnp/__init__.py index 25650e6e637..4ccb07af44b 100644 --- a/homeassistant/components/upnp/__init__.py +++ b/homeassistant/components/upnp/__init__.py @@ -4,6 +4,7 @@ Will open a port in your router for Home Assistant and provide statistics. For more details about this component, please refer to the documentation at https://home-assistant.io/components/upnp/ """ +# pylint: disable=invalid-name import asyncio from ipaddress import ip_address @@ -12,9 +13,8 @@ import voluptuous as vol from homeassistant.config_entries import ConfigEntry from homeassistant.const import EVENT_HOMEASSISTANT_STOP -from homeassistant.exceptions import PlatformNotReady from homeassistant.helpers import config_validation as cv -from homeassistant.helpers import discovery +from homeassistant.helpers import dispatcher from homeassistant.helpers.typing import ConfigType from homeassistant.helpers.typing import HomeAssistantType from homeassistant.components.discovery import DOMAIN as DISCOVERY_DOMAIN @@ -79,16 +79,16 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): hass.data[DOMAIN]['local_ip'] = upnp_config[CONF_LOCAL_IP] # determine ports - ports = {CONF_HASS: CONF_HASS} # default, port_forward disabled by default + ports = {CONF_HASS: CONF_HASS} # default, port_mapping disabled by default if CONF_PORTS in upnp_config: # copy from config ports = upnp_config[CONF_PORTS] hass.data[DOMAIN]['auto_config'] = { 'active': True, - 'port_forward': upnp_config[CONF_ENABLE_PORT_MAPPING], + 'enable_sensors': upnp_config[CONF_ENABLE_SENSORS], + 'enable_port_mapping': upnp_config[CONF_ENABLE_PORT_MAPPING], 'ports': ports, - 'sensors': upnp_config[CONF_ENABLE_SENSORS], } return True @@ -98,7 +98,6 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Set up a bridge from a config entry.""" - _LOGGER.debug('async_setup_entry: %s', config_entry.data) ensure_domain_data(hass) data = config_entry.data @@ -107,7 +106,8 @@ async def async_setup_entry(hass: HomeAssistantType, try: device = await Device.async_create_device(hass, ssdp_description) except (asyncio.TimeoutError, aiohttp.ClientError): - raise PlatformNotReady() + _LOGGER.error('Unable to create upnp-device') + return hass.data[DOMAIN]['devices'][device.udn] = device @@ -124,12 +124,10 @@ async def async_setup_entry(hass: HomeAssistantType, # sensors if data.get(CONF_ENABLE_SENSORS): _LOGGER.debug('Enabling sensors') - discovery_info = { - 'udn': device.udn, - } - hass_config = config_entry.data - hass.async_create_task(discovery.async_load_platform( - hass, 'sensor', DOMAIN, discovery_info, hass_config)) + + # register sensor setup handlers + hass.async_create_task(hass.config_entries.async_forward_entry_setup( + config_entry, 'sensor')) async def unload_entry(event): """Unload entry on quit.""" @@ -142,7 +140,6 @@ async def async_setup_entry(hass: HomeAssistantType, async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Unload a config entry.""" - _LOGGER.debug('async_unload_entry: %s', config_entry.data) data = config_entry.data udn = data[CONF_UDN] @@ -156,9 +153,9 @@ async def async_unload_entry(hass: HomeAssistantType, await device.async_delete_port_mappings() # sensors - for sensor in hass.data[DOMAIN]['sensors'].get(udn, []): - _LOGGER.debug('Deleting sensor: %s', sensor) - await sensor.async_remove() + if data.get(CONF_ENABLE_SENSORS): + _LOGGER.debug('Deleting sensors') + dispatcher.async_dispatcher_send(hass, 'upnp_remove_sensor', device) # clear stored device del hass.data[DOMAIN]['devices'][udn] diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index 95deed73e63..65e2283115c 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -1,4 +1,6 @@ """Config flow for UPNP.""" +from collections import OrderedDict + import voluptuous as vol from homeassistant import config_entries @@ -15,12 +17,11 @@ def ensure_domain_data(hass): """Ensure hass.data is filled properly.""" hass.data[DOMAIN] = hass.data.get(DOMAIN, {}) hass.data[DOMAIN]['devices'] = hass.data[DOMAIN].get('devices', {}) - hass.data[DOMAIN]['sensors'] = hass.data[DOMAIN].get('sensors', {}) hass.data[DOMAIN]['discovered'] = hass.data[DOMAIN].get('discovered', {}) hass.data[DOMAIN]['auto_config'] = hass.data[DOMAIN].get('auto_config', { 'active': False, - 'port_forward': False, - 'sensors': False, + 'enable_sensors': False, + 'enable_port_mapping': False, 'ports': {'hass': 'hass'}, }) @@ -30,6 +31,7 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): """Handle a Hue config flow.""" VERSION = 1 + CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL @property def _configured_upnp_igds(self): @@ -79,8 +81,8 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): if auto_config['active']: import_info = { 'name': discovery_info['friendly_name'], - 'sensors': auto_config['sensors'], - 'port_forward': auto_config['port_forward'], + 'enable_sensors': auto_config['enable_sensors'], + 'enable_port_mapping': auto_config['enable_port_mapping'], } return await self._async_save_entry(import_info) @@ -94,8 +96,9 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): # if user input given, handle it user_input = user_input or {} if 'name' in user_input: - if not user_input['sensors'] and not user_input['port_forward']: - return self.async_abort(reason='no_sensors_or_port_forward') + if not user_input['enable_sensors'] and \ + not user_input['enable_port_mapping']: + return self.async_abort(reason='no_sensors_or_port_mapping') # ensure not already configured configured_names = [ @@ -119,12 +122,13 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): return self.async_show_form( step_id='user', - data_schema=vol.Schema({ - vol.Required('name'): vol.In(names), - vol.Optional('sensors', default=False): bool, - vol.Optional('port_forward', default=False): bool, - }) - ) + data_schema=vol.Schema( + OrderedDict([ + (vol.Required('name'), vol.In(names)), + (vol.Optional('enable_sensors', default=False), bool), + (vol.Optional('enable_port_mapping', default=False), bool), + ]) + )) async def async_step_import(self, import_info): """Import a new UPnP/IGD as a config entry.""" @@ -150,7 +154,7 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): data={ CONF_SSDP_DESCRIPTION: discovery_info['ssdp_description'], CONF_UDN: discovery_info['udn'], - CONF_ENABLE_SENSORS: import_info['sensors'], - CONF_ENABLE_PORT_MAPPING: import_info['port_forward'], + CONF_ENABLE_SENSORS: import_info['enable_sensors'], + CONF_ENABLE_PORT_MAPPING: import_info['enable_port_mapping'], }, ) diff --git a/homeassistant/components/upnp/device.py b/homeassistant/components/upnp/device.py index 6dce3889eaf..4a444aa3087 100644 --- a/homeassistant/components/upnp/device.py +++ b/homeassistant/components/upnp/device.py @@ -39,7 +39,7 @@ class Device: from async_upnp_client.igd import IgdDevice igd_device = IgdDevice(upnp_device, None) - return Device(igd_device) + return cls(igd_device) @property def udn(self): @@ -102,6 +102,7 @@ class Device: async def _async_delete_port_mapping(self, external_port): """Remove a port mapping.""" from async_upnp_client import UpnpError + _LOGGER.info('Deleting port mapping %s (TCP)', external_port) try: await self._igd_device.async_delete_port_mapping( remote_host=None, diff --git a/homeassistant/components/upnp/strings.json b/homeassistant/components/upnp/strings.json index 829631254ad..682150b7ddd 100644 --- a/homeassistant/components/upnp/strings.json +++ b/homeassistant/components/upnp/strings.json @@ -9,8 +9,8 @@ "title": "Configuration options for the UPnP/IGD", "data":{ "igd": "UPnP/IGD", - "sensors": "Add traffic sensors", - "port_forward": "Enable port forward for Home Assistant" + "enable_sensors": "Add traffic sensors", + "enable_port_mapping": "Enable port mapping for Home Assistant" } } }, @@ -19,7 +19,7 @@ "abort": { "no_devices_discovered": "No UPnP/IGDs discovered", "already_configured": "UPnP/IGD is already configured", - "no_sensors_or_port_forward": "Enable at least sensors or port forward" + "no_sensors_or_port_mapping": "Enable at least sensors or port mapping" } } } diff --git a/tests/components/upnp/test_config_flow.py b/tests/components/upnp/test_config_flow.py index f6ec05a42ab..3ff1316975f 100644 --- a/tests/components/upnp/test_config_flow.py +++ b/tests/components/upnp/test_config_flow.py @@ -44,15 +44,15 @@ async def test_flow_already_configured(hass): result = await flow.async_step_user({ 'name': '192.168.1.1 (Test device)', - 'sensors': True, - 'port_forward': False, + 'enable_sensors': True, + 'enable_port_mapping': False, }) assert result['type'] == 'abort' assert result['reason'] == 'already_configured' -async def test_flow_no_sensors_no_port_forward(hass): - """Test single device, no sensors, no port_forward.""" +async def test_flow_no_sensors_no_port_mapping(hass): + """Test single device, no sensors, no port_mapping.""" flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass @@ -76,11 +76,11 @@ async def test_flow_no_sensors_no_port_forward(hass): result = await flow.async_step_user({ 'name': '192.168.1.1 (Test device)', - 'sensors': False, - 'port_forward': False, + 'enable_sensors': False, + 'enable_port_mapping': False, }) assert result['type'] == 'abort' - assert result['reason'] == 'no_sensors_or_port_forward' + assert result['reason'] == 'no_sensors_or_port_mapping' async def test_flow_discovered_form(hass): @@ -106,7 +106,7 @@ async def test_flow_discovered_form(hass): async def test_flow_two_discovered_form(hass): - """Test single device discovered, show form flow.""" + """Test two devices discovered, show form flow with two devices.""" flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass @@ -133,13 +133,13 @@ async def test_flow_two_discovered_form(hass): assert result['step_id'] == 'user' assert result['data_schema']({ 'name': '192.168.1.1 (Test device)', - 'sensors': True, - 'port_forward': False, + 'enable_sensors': True, + 'enable_port_mapping': False, }) assert result['data_schema']({ - 'name': '192.168.1.1 (Test device)', - 'sensors': True, - 'port_forward': False, + 'name': '192.168.2.1 (Test device)', + 'enable_sensors': True, + 'enable_port_mapping': False, }) @@ -163,15 +163,15 @@ async def test_config_entry_created(hass): result = await flow.async_step_user({ 'name': '192.168.1.1 (Test device)', - 'sensors': True, - 'port_forward': False, + 'enable_sensors': True, + 'enable_port_mapping': False, }) assert result['type'] == 'create_entry' assert result['data'] == { - 'port_forward': False, - 'sensors': True, 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': 'uuid:device_1', + 'port_mapping': False, + 'sensors': True, } assert result['title'] == 'Test device 1' @@ -185,8 +185,8 @@ async def test_flow_discovery_auto_config_sensors(hass): hass.data[upnp.DOMAIN] = { 'auto_config': { 'active': True, - 'port_forward': False, - 'sensors': True, + 'enable_port_mapping': False, + 'enable_sensors': True, }, } @@ -200,25 +200,25 @@ async def test_flow_discovery_auto_config_sensors(hass): assert result['type'] == 'create_entry' assert result['data'] == { - 'port_forward': False, - 'sensors': True, 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': 'uuid:device_1', + 'sensors': True, + 'port_mapping': False, } assert result['title'] == 'Test device 1' -async def test_flow_discovery_auto_config_sensors_port_forward(hass): - """Test creation of device with auto_config, with port forward.""" +async def test_flow_discovery_auto_config_sensors_port_mapping(hass): + """Test creation of device with auto_config, with port mapping.""" flow = upnp_config_flow.UpnpFlowHandler() flow.hass = hass - # auto_config active, with port_forward + # auto_config active, with port_mapping hass.data[upnp.DOMAIN] = { 'auto_config': { 'active': True, - 'port_forward': True, - 'sensors': True, + 'enable_port_mapping': True, + 'enable_sensors': True, }, } @@ -232,9 +232,9 @@ async def test_flow_discovery_auto_config_sensors_port_forward(hass): assert result['type'] == 'create_entry' assert result['data'] == { - 'port_forward': True, - 'sensors': True, - 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': 'uuid:device_1', + 'ssdp_description': 'http://192.168.1.1/desc.xml', + 'sensors': True, + 'port_mapping': True, } assert result['title'] == 'Test device 1' diff --git a/tests/components/upnp/test_init.py b/tests/components/upnp/test_init.py index 581abc3190c..ce4656032a6 100644 --- a/tests/components/upnp/test_init.py +++ b/tests/components/upnp/test_init.py @@ -53,9 +53,9 @@ async def test_async_setup_no_auto_config(hass): assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': False, - 'port_forward': False, + 'enable_sensors': False, + 'enable_port_mapping': False, 'ports': {'hass': 'hass'}, - 'sensors': False, } @@ -66,27 +66,27 @@ async def test_async_setup_auto_config(hass): assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': True, - 'port_forward': False, + 'enable_sensors': True, + 'enable_port_mapping': False, 'ports': {'hass': 'hass'}, - 'sensors': True, } -async def test_async_setup_auto_config_port_forward(hass): +async def test_async_setup_auto_config_port_mapping(hass): """Test async_setup.""" # setup component, enable auto_config await async_setup_component(hass, 'upnp', { 'upnp': { - 'port_forward': True, + 'port_mapping': True, 'ports': {'hass': 'hass'}, }, 'discovery': {}}) assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': True, - 'port_forward': True, + 'enable_sensors': True, + 'enable_port_mapping': True, 'ports': {'hass': 'hass'}, - 'sensors': True, } @@ -99,9 +99,9 @@ async def test_async_setup_auto_config_no_sensors(hass): assert hass.data[upnp.DOMAIN]['auto_config'] == { 'active': True, - 'port_forward': False, + 'enable_sensors': False, + 'enable_port_mapping': False, 'ports': {'hass': 'hass'}, - 'sensors': False, } @@ -112,7 +112,7 @@ async def test_async_setup_entry_default(hass): 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': udn, 'sensors': True, - 'port_forward': False, + 'port_mapping': False, }) # ensure hass.http is available @@ -144,20 +144,20 @@ async def test_async_setup_entry_default(hass): assert len(mock_device.async_delete_port_mappings.mock_calls) == 0 -async def test_async_setup_entry_port_forward(hass): +async def test_async_setup_entry_port_mapping(hass): """Test async_setup_entry.""" udn = 'uuid:device_1' entry = MockConfigEntry(domain=upnp.DOMAIN, data={ 'ssdp_description': 'http://192.168.1.1/desc.xml', 'udn': udn, 'sensors': False, - 'port_forward': True, + 'port_mapping': True, }) # ensure hass.http is available await async_setup_component(hass, 'upnp', { 'upnp': { - 'port_forward': True, + 'port_mapping': True, 'ports': {'hass': 'hass'}, }, 'discovery': {}, From 3cb20c7b4d074c39ad04fb01fb7f07f0eb77faea Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Wed, 3 Oct 2018 11:07:25 +0200 Subject: [PATCH 23/24] Changes after review by @MartinHjelmare --- homeassistant/components/sensor/upnp.py | 5 +++-- homeassistant/components/upnp/__init__.py | 16 +++++++++++----- homeassistant/components/upnp/config_flow.py | 4 ++-- homeassistant/components/upnp/const.py | 1 + 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/sensor/upnp.py b/homeassistant/components/sensor/upnp.py index e511b2947e5..c05e2ce0ade 100644 --- a/homeassistant/components/sensor/upnp.py +++ b/homeassistant/components/sensor/upnp.py @@ -4,7 +4,6 @@ Support for UPnP/IGD Sensors. For more details about this platform, please refer to the documentation at https://home-assistant.io/components/sensor.upnp/ """ -# pylint: disable=invalid-name from datetime import datetime import logging @@ -12,6 +11,7 @@ from homeassistant.core import callback from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity import Entity from homeassistant.components.upnp.const import DOMAIN as DATA_UPNP +from homeassistant.components.upnp.const import SIGNAL_REMOVE_SENSOR _LOGGER = logging.getLogger(__name__) @@ -88,9 +88,10 @@ class UpnpSensor(Entity): async def async_added_to_hass(self): """Subscribe to sensors events.""" async_dispatcher_connect(self.hass, - 'upnp_remove_sensor', + SIGNAL_REMOVE_SENSOR, self._upnp_remove_sensor) + @callback def _upnp_remove_sensor(self, device): """Remove sensor.""" if self._device != device: diff --git a/homeassistant/components/upnp/__init__.py b/homeassistant/components/upnp/__init__.py index 4ccb07af44b..265f68eaac0 100644 --- a/homeassistant/components/upnp/__init__.py +++ b/homeassistant/components/upnp/__init__.py @@ -4,7 +4,6 @@ Will open a port in your router for Home Assistant and provide statistics. For more details about this component, please refer to the documentation at https://home-assistant.io/components/upnp/ """ -# pylint: disable=invalid-name import asyncio from ipaddress import ip_address @@ -23,6 +22,7 @@ from .const import ( CONF_ENABLE_PORT_MAPPING, CONF_ENABLE_SENSORS, CONF_HASS, CONF_LOCAL_IP, CONF_PORTS, CONF_UDN, CONF_SSDP_DESCRIPTION, + SIGNAL_REMOVE_SENSOR, ) from .const import DOMAIN from .const import LOGGER as _LOGGER @@ -51,14 +51,20 @@ CONFIG_SCHEMA = vol.Schema({ def _substitute_hass_ports(ports, hass_port): - # substitute 'hass' for hass_port, both sides + """Substitute 'hass' for the hass_port.""" + ports = ports.copy() + + # substitute 'hass' for hass_port, both keys and values if CONF_HASS in ports: ports[hass_port] = ports[CONF_HASS] del ports[CONF_HASS] + for port in ports: if ports[port] == CONF_HASS: ports[port] = hass_port + return ports + # config async def async_setup(hass: HomeAssistantType, config: ConfigType): @@ -107,7 +113,7 @@ async def async_setup_entry(hass: HomeAssistantType, device = await Device.async_create_device(hass, ssdp_description) except (asyncio.TimeoutError, aiohttp.ClientError): _LOGGER.error('Unable to create upnp-device') - return + return False hass.data[DOMAIN]['devices'][device.udn] = device @@ -118,7 +124,7 @@ async def async_setup_entry(hass: HomeAssistantType, _LOGGER.debug('Enabling port mappings: %s', ports) hass_port = hass.http.server_port - _substitute_hass_ports(ports, hass_port) + ports = _substitute_hass_ports(ports, hass_port) await device.async_add_port_mappings(ports, local_ip=local_ip) # sensors @@ -155,7 +161,7 @@ async def async_unload_entry(hass: HomeAssistantType, # sensors if data.get(CONF_ENABLE_SENSORS): _LOGGER.debug('Deleting sensors') - dispatcher.async_dispatcher_send(hass, 'upnp_remove_sensor', device) + dispatcher.async_dispatcher_send(hass, SIGNAL_REMOVE_SENSOR, device) # clear stored device del hass.data[DOMAIN]['devices'][udn] diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index 65e2283115c..178faafb5f7 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -125,8 +125,8 @@ class UpnpFlowHandler(data_entry_flow.FlowHandler): data_schema=vol.Schema( OrderedDict([ (vol.Required('name'), vol.In(names)), - (vol.Optional('enable_sensors', default=False), bool), - (vol.Optional('enable_port_mapping', default=False), bool), + (vol.Optional('enable_sensors'), bool), + (vol.Optional('enable_port_mapping'), bool), ]) )) diff --git a/homeassistant/components/upnp/const.py b/homeassistant/components/upnp/const.py index ad57bc7d7f4..7a906ae02be 100644 --- a/homeassistant/components/upnp/const.py +++ b/homeassistant/components/upnp/const.py @@ -11,3 +11,4 @@ CONF_SSDP_DESCRIPTION = 'ssdp_description' CONF_UDN = 'udn' DOMAIN = 'upnp' LOGGER = logging.getLogger('homeassistant.components.upnp') +SIGNAL_REMOVE_SENSOR = 'upnp_remove_sensor' From 5d693277f02ccbd08e599cc004223a868f165289 Mon Sep 17 00:00:00 2001 From: Steven Looman Date: Wed, 3 Oct 2018 11:27:38 +0200 Subject: [PATCH 24/24] Fix stale docstrings --- homeassistant/components/upnp/__init__.py | 2 +- homeassistant/components/upnp/config_flow.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/upnp/__init__.py b/homeassistant/components/upnp/__init__.py index 265f68eaac0..f70fbcc4d20 100644 --- a/homeassistant/components/upnp/__init__.py +++ b/homeassistant/components/upnp/__init__.py @@ -103,7 +103,7 @@ async def async_setup(hass: HomeAssistantType, config: ConfigType): # config flow async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): - """Set up a bridge from a config entry.""" + """Set up UPnP/IGD-device from a config entry.""" ensure_domain_data(hass) data = config_entry.data diff --git a/homeassistant/components/upnp/config_flow.py b/homeassistant/components/upnp/config_flow.py index 178faafb5f7..885f2f64211 100644 --- a/homeassistant/components/upnp/config_flow.py +++ b/homeassistant/components/upnp/config_flow.py @@ -28,7 +28,7 @@ def ensure_domain_data(hass): @config_entries.HANDLERS.register(DOMAIN) class UpnpFlowHandler(data_entry_flow.FlowHandler): - """Handle a Hue config flow.""" + """Handle a UPnP/IGD config flow.""" VERSION = 1 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_POLL