Fix not being able to re-add IGD when removed

.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

homeassistant/components/igd/.translations/en.json

@@ -8,11 +8,9 @@
             "user": {
                 "title": "Configuration options for the IGD",
                 "data":{
-                    "sensors": "Add traffic in/out sensors",
+                    "igd": "IGD",
-                    "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!",
+                    "sensors": "Add traffic sensors",
-                    "ssdp_url": "SSDP URL",
+                    "port_forward": "Enable port forward for Home Assistant<br>Only enable this when your Home Assistant is password protected!"
-                    "udn": "UDN",
-                    "name": "Name"
                 }
             }
         },

homeassistant/components/igd/.translations/nl.json

@@ -8,11 +8,9 @@
             "user": {
                 "title": "Extra configuratie options voor IGD",
                 "data":{
-                    "sensors": "Verkeer in/out sensors",
+                    "igd": "IGD",
-                    "port_forward": "Maak port-forward voor Home Assistant\nZet dit alleen aan wanneer uw Home Assistant een wachtwoord heeft!",
+                    "sensors": "Verkeer sensors toevoegen",
-                    "ssdp_url": "SSDP URL",
+                    "port_forward": "Maak port-forward voor Home Assistant<br>Zet dit alleen aan wanneer uw Home Assistant een wachtwoord heeft!"
-                    "udn": "UDN",
-                    "name": "Naam"
                 }
             }
         },

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] = {
+    hass.data[DOMAIN] = hass.data.get(DOMAIN, {})
-        'auto_config': {
+    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

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 = [
+            # ensure nto already configured
-                entry['host']
+            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
+        # let user choose from all discovered, non-configured, IGDs
-        igd_hosts = [
+        names = [
-            entry['host']
+            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('name'): vol.In(names),
-                vol.Required('sensors'): bool,
+                vol.Optional('sensors', default=False): bool,
-                vol.Required('port_forward'): 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]

homeassistant/components/igd/strings.json

@@ -8,11 +8,9 @@
             "user": {
                 "title": "Configuration options for the IGD",
                 "data":{
-                    "sensors": "Add traffic in/out sensors",
+                    "igd": "IGD",
-                    "port_forward": "Enable port forward for Home Assistant\nOnly enable this when your Home Assistant is password protected!",
+                    "sensors": "Add traffic sensors",
-                    "ssdp_url": "SSDP URL",
+                    "port_forward": "Enable port forward for Home Assistant<br>Only enable this when your Home Assistant is password protected!"
-                    "udn": "UDN",
-                    "name": "Name"
                 }
             }
         },

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
-from homeassistant.components.igd import DOMAIN, UNITS
 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_RECEIVED: {
-    BYTES_SENT: ['bytes sent', True, 'mdi:server-network', float],
+        'name': 'bytes received',
-    PACKETS_RECEIVED: ['packets received', False, 'mdi:server-network', int],
+        'unit': 'bytes',
-    PACKETS_SENT: ['packets sent', False, 'mdi:server-network', int],
+    },
+    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]
+    igd_device = hass.data[DOMAIN]['devices'][udn]
-    unit = discovery_info['unit']
+
+    # raw sensors
     async_add_devices([
-        IGDSensor(device, t, unit if SENSOR_TYPES[t][1] else '#')
+        RawIGDSensor(igd_device, name, sensor_type)
-        for t in SENSOR_TYPES])
+        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._type_name = sensor_type_name
-        self.unit = unit
+        self._type = sensor_type
-        self.unit_factor = UNITS[unit] if unit in UNITS else 1
+        self._name = 'IGD {}'.format(sensor_type['name'])
-        self._name = 'IGD {}'.format(SENSOR_TYPES[sensor_type][0])
         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 self._state
-            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):
+    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
+        _LOGGER.debug('%s: async_update', self)
-        if self.type == BYTES_RECEIVED:
+        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:
+        now = datetime.now()
-            # try to get the state from history
+        if self._is_overflowed(new_value):
-            self._state = self._last_value_from_state or 0
+            _LOGGER.debug('%s: Overflow: old value: %s, new value: %s',
-
+                          self, self._last_value, new_value)
-        _LOGGER.debug('%s: state: %s, last_value: %s',
+            self._state = None  # temporarily report nothing
-                      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
+            delta_time = (now - self._last_update_time).seconds
-            diff = OVERFLOW_AT - self._last_value
+            delta_value = new_value - self._last_value
-            if new_value >= 0:
+            value = (delta_value / delta_time) / KBYTE
-                diff += new_value
+            self._state = format(float(value), '.1f')
-            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._last_update_time = now
-                      self.entity_id, diff, self._state, self._last_value)
+
+
+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

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,
     })