Refactor rfxtrx code

homeassistant/components/rfxtrx.py

@@ -12,12 +12,14 @@ import homeassistant.helpers.config_validation as cv
 from homeassistant.util import slugify
 from homeassistant.const import EVENT_HOMEASSISTANT_STOP
 from homeassistant.helpers.entity import Entity
-from homeassistant.const import ATTR_ENTITY_ID
+from homeassistant.const import (ATTR_ENTITY_ID, TEMP_CELSIUS)
 
 REQUIREMENTS = ['pyRFXtrx==0.6.5']
 
 DOMAIN = "rfxtrx"
 
+DEFAULT_SIGNAL_REPETITIONS = 1
+
 ATTR_AUTOMATIC_ADD = 'automatic_add'
 ATTR_DEVICE = 'device'
 ATTR_DEBUG = 'debug'
@@ -28,55 +30,77 @@ ATTR_DATA_TYPE = 'data_type'
 ATTR_DUMMY = 'dummy'
 CONF_SIGNAL_REPETITIONS = 'signal_repetitions'
 CONF_DEVICES = 'devices'
-DEFAULT_SIGNAL_REPETITIONS = 1
-
 EVENT_BUTTON_PRESSED = 'button_pressed'
 
+DATA_TYPES = OrderedDict([
+    ('Temperature', TEMP_CELSIUS),
+    ('Humidity', '%'),
+    ('Barometer', ''),
+    ('Wind direction', ''),
+    ('Rain rate', ''),
+    ('Energy usage', 'W'),
+    ('Total usage', 'W')])
+
 RECEIVED_EVT_SUBSCRIBERS = []
 RFX_DEVICES = {}
 _LOGGER = logging.getLogger(__name__)
 RFXOBJECT = None
 
 
-def validate_packetid(value):
+def _valid_device(value, device_type):
-    """Validate that value is a valid packet id for rfxtrx."""
+    """Validate a dictionary of devices definitions."""
-    if get_rfx_object(value):
+    config = OrderedDict()
-        return value
+    for key, device in value.items():
+
+        # Still accept old configuration
+        if 'packetid' in device.keys():
+            msg = 'You are using an outdated configuration of the rfxtrx ' +\
+                  'device, {}.'.format(key) +\
+                  ' Your new config should be:\n    {}: \n        name: {}'\
+                  .format(device.get('packetid'),
+                          device.get(ATTR_NAME, 'deivce_name'))
+            _LOGGER.warning(msg)
+            key = device.get('packetid')
+            device.pop('packetid')
+
+        if get_rfx_object(key) is None:
+            raise vol.Invalid('Rfxtrx device {} is invalid: '
+                              'Invalid device id for {}'.format(key, value))
+
+        if device_type == 'sensor':
+            config[key] = DEVICE_SCHEMA_SENSOR(device)
+        elif device_type == 'light_switch':
+            config[key] = DEVICE_SCHEMA(device)
         else:
-        raise vol.Invalid('invalid packet id for {}'.format(value))
+            raise vol.Invalid('Rfxtrx device is invalid')
+
+        if not config[key][ATTR_NAME]:
+            config[key][ATTR_NAME] = key
+    return config
+
+
+def valid_sensor(value):
+    """Validate sensor configuration."""
+    return _valid_device(value, "sensor")
+
+
+def _valid_light_switch(value):
+    return _valid_device(value, "light_switch")
 
 DEVICE_SCHEMA = vol.Schema({
     vol.Required(ATTR_NAME): cv.string,
     vol.Optional(ATTR_FIREEVENT, default=False): cv.boolean,
 })
 
-
+DEVICE_SCHEMA_SENSOR = vol.Schema({
-def _valid_device(value):
+    vol.Optional(ATTR_NAME, default=None): cv.string,
-    """Validate a dictionary of devices definitions."""
+    vol.Optional(ATTR_DATA_TYPE, default=[]):
-    config = OrderedDict()
+        vol.All(cv.ensure_list, [vol.In(DATA_TYPES.keys())]),
-    for key, device in value.items():
+})
-        # Still accept old configuration
-        if 'packetid' in device.keys():
-            msg = 'You are using an outdated configuration of the rfxtrx ' +\
-                  'devuce, {}. Your new config should be:\n{}: \n\t name:{}\n'\
-                  .format(key, device.get('packetid'),
-                          device.get(ATTR_NAME, 'deivce_name'))
-            _LOGGER.warning(msg)
-            key = device.get('packetid')
-            device.pop('packetid')
-        try:
-            key = validate_packetid(key)
-            config[key] = DEVICE_SCHEMA(device)
-            if not config[key][ATTR_NAME]:
-                config[key][ATTR_NAME] = key
-        except vol.MultipleInvalid as ex:
-            raise vol.Invalid('Rfxtrx deive {} is invalid: {}'
-                              .format(key, ex))
-    return config
 
 DEFAULT_SCHEMA = vol.Schema({
     vol.Required("platform"): DOMAIN,
-    vol.Required(CONF_DEVICES): vol.All(dict, _valid_device),
+    vol.Optional(CONF_DEVICES, default={}): vol.All(dict, _valid_light_switch),
     vol.Optional(ATTR_AUTOMATIC_ADD, default=False):  cv.boolean,
     vol.Optional(CONF_SIGNAL_REPETITIONS, default=DEFAULT_SIGNAL_REPETITIONS):
         vol.Coerce(int),
@@ -99,11 +123,7 @@ def setup(hass, config):
         # Log RFXCOM event
         if not event.device.id_string:
             return
-        entity_id = slugify(event.device.id_string.lower())
+        _LOGGER.info("Receive RFXCOM event from %s", event.device)
-        packet_id = "".join("{0:02x}".format(x) for x in event.data)
-        entity_name = "%s : %s" % (entity_id, packet_id)
-        _LOGGER.info("Receive RFXCOM event from %s => %s",
-                     event.device, entity_name)
 
         # Callback to HA registered components.
         for subscriber in RECEIVED_EVT_SUBSCRIBERS:
@@ -138,20 +158,17 @@ def get_rfx_object(packetid):
     import RFXtrx as rfxtrxmod
 
     binarypacket = bytearray.fromhex(packetid)
-
     pkt = rfxtrxmod.lowlevel.parse(binarypacket)
-    if pkt is not None:
+    if pkt is None:
+        return None
     if isinstance(pkt, rfxtrxmod.lowlevel.SensorPacket):
         obj = rfxtrxmod.SensorEvent(pkt)
     elif isinstance(pkt, rfxtrxmod.lowlevel.Status):
         obj = rfxtrxmod.StatusEvent(pkt)
     else:
         obj = rfxtrxmod.ControlEvent(pkt)
-
     return obj
 
-    return None
-
 
 def get_devices_from_config(config, device):
     """Read rfxtrx configuration."""
@@ -182,8 +199,7 @@ def get_new_device(event, config, device):
     if device_id in RFX_DEVICES:
         return
 
-    automatic_add = config[ATTR_AUTOMATIC_ADD]
+    if not config[ATTR_AUTOMATIC_ADD]:
-    if not automatic_add:
         return
 
     _LOGGER.info(
@@ -193,10 +209,9 @@ def get_new_device(event, config, device):
         event.device.subtype
     )
     pkt_id = "".join("{0:02x}".format(x) for x in event.data)
-    entity_name = "%s : %s" % (device_id, pkt_id)
     datas = {ATTR_STATE: False, ATTR_FIREEVENT: False}
     signal_repetitions = config[CONF_SIGNAL_REPETITIONS]
-    new_device = device(entity_name, event, datas,
+    new_device = device(pkt_id, event, datas,
                         signal_repetitions)
     RFX_DEVICES[device_id] = new_device
     return new_device
@@ -244,7 +259,7 @@ def apply_received_command(event):
 class RfxtrxDevice(Entity):
     """Represents a Rfxtrx device.
 
-    Contains the common logic for all Rfxtrx devices.
+    Contains the common logic for Rfxtrx lights and switches.
 
     """
 

homeassistant/components/sensor/rfxtrx.py

@@ -5,87 +5,52 @@ For more details about this platform, please refer to the documentation at
 https://home-assistant.io/components/sensor.rfxtrx/
 """
 import logging
-from collections import OrderedDict
 import voluptuous as vol
 
 import homeassistant.components.rfxtrx as rfxtrx
-from homeassistant.const import TEMP_CELSIUS
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity import Entity
 from homeassistant.util import slugify
 from homeassistant.components.rfxtrx import (
     ATTR_AUTOMATIC_ADD, ATTR_NAME,
-    CONF_DEVICES, ATTR_DATA_TYPE)
+    CONF_DEVICES, ATTR_DATA_TYPE, DATA_TYPES)
 
 DEPENDENCIES = ['rfxtrx']
 
-DATA_TYPES = OrderedDict([
-    ('Temperature', TEMP_CELSIUS),
-    ('Humidity', '%'),
-    ('Barometer', ''),
-    ('Wind direction', ''),
-    ('Rain rate', ''),
-    ('Energy usage', 'W'),
-    ('Total usage', 'W')])
 _LOGGER = logging.getLogger(__name__)
 
-DEVICE_SCHEMA = vol.Schema({
-    vol.Optional(ATTR_NAME, default=None): cv.string,
-    vol.Optional(ATTR_DATA_TYPE, default=[]):
-        vol.All(cv.ensure_list, [vol.In(DATA_TYPES.keys())]),
-})
-
-
-def _valid_sensor(value):
-    """Validate a dictionary of devices definitions."""
-    config = OrderedDict()
-    for key, device in value.items():
-        # Still accept old configuration
-        if 'packetid' in device.keys():
-            msg = 'You are using an outdated configuration of the rfxtrx ' +\
-                  'sensor, {}. Your new config should be:\n{}: \n\t name:{}\n'\
-                  .format(key, device.get('packetid'),
-                          device.get(ATTR_NAME, 'sensor_name'))
-            _LOGGER.warning(msg)
-            key = device.get('packetid')
-            device.pop('packetid')
-        try:
-            key = rfxtrx.validate_packetid(key)
-            config[key] = DEVICE_SCHEMA(device)
-            if not config[key][ATTR_NAME]:
-                config[key][ATTR_NAME] = key
-        except vol.MultipleInvalid as ex:
-            raise vol.Invalid('Rfxtrx sensor {} is invalid: {}'
-                              .format(key, ex))
-    return config
-
-
 PLATFORM_SCHEMA = vol.Schema({
     vol.Required("platform"): rfxtrx.DOMAIN,
-    vol.Required(CONF_DEVICES): vol.All(dict, _valid_sensor),
+    vol.Optional(CONF_DEVICES, default={}): vol.All(dict, rfxtrx.valid_sensor),
     vol.Optional(ATTR_AUTOMATIC_ADD, default=False):  cv.boolean,
 }, extra=vol.ALLOW_EXTRA)
 
 
 def setup_platform(hass, config, add_devices_callback, discovery_info=None):
     """Setup the RFXtrx platform."""
+    # pylint: disable=too-many-locals
     from RFXtrx import SensorEvent
-
     sensors = []
     for packet_id, entity_info in config['devices'].items():
         event = rfxtrx.get_rfx_object(packet_id)
         device_id = "sensor_" + slugify(event.device.id_string.lower())
-
         if device_id in rfxtrx.RFX_DEVICES:
             continue
         _LOGGER.info("Add %s rfxtrx.sensor", entity_info[ATTR_NAME])
+
         sub_sensors = {}
-        for _data_type in cv.ensure_list(entity_info[ATTR_DATA_TYPE]):
+        data_types = entity_info[ATTR_DATA_TYPE]
+        if len(data_types) == 0:
+            for data_type in DATA_TYPES:
+                if data_type in event.values:
+                    data_types = [data_type]
+                    break
+        for _data_type in data_types:
             new_sensor = RfxtrxSensor(event, entity_info[ATTR_NAME],
                                       _data_type)
             sensors.append(new_sensor)
             sub_sensors[_data_type] = new_sensor
-        rfxtrx.RFX_DEVICES[slugify(device_id)] = sub_sensors
+        rfxtrx.RFX_DEVICES[device_id] = sub_sensors
 
     add_devices_callback(sensors)
 
@@ -103,15 +68,17 @@ def setup_platform(hass, config, add_devices_callback, discovery_info=None):
             return
 
         # Add entity if not exist and the automatic_add is True
-        if config[ATTR_AUTOMATIC_ADD]:
+        if not config[ATTR_AUTOMATIC_ADD]:
-            pkt_id = "".join("{0:02x}".format(x) for x in event.data)
+            return
-            entity_name = "%s : %s" % (device_id, pkt_id)
-            _LOGGER.info(
-                "Automatic add rfxtrx.sensor: (%s : %s)",
-                device_id,
-                pkt_id)
 
-            new_sensor = RfxtrxSensor(event, entity_name)
+        pkt_id = "".join("{0:02x}".format(x) for x in event.data)
+        _LOGGER.info("Automatic add rfxtrx.sensor: %s",
+                     device_id)
+
+        for data_type in DATA_TYPES:
+            if data_type in event.values:
+                new_sensor = RfxtrxSensor(event, pkt_id, data_type)
+                break
         sub_sensors = {}
         sub_sensors[new_sensor.data_type] = new_sensor
         rfxtrx.RFX_DEVICES[device_id] = sub_sensors
@@ -124,7 +91,7 @@ def setup_platform(hass, config, add_devices_callback, discovery_info=None):
 class RfxtrxSensor(Entity):
     """Representation of a RFXtrx sensor."""
 
-    def __init__(self, event, name, data_type=None):
+    def __init__(self, event, name, data_type):
         """Initialize the sensor."""
         self.event = event
         self._unit_of_measurement = None
@@ -133,12 +100,6 @@ class RfxtrxSensor(Entity):
         if data_type:
             self.data_type = data_type
             self._unit_of_measurement = DATA_TYPES[data_type]
-            return
-        for data_type in DATA_TYPES:
-            if data_type in self.event.values:
-                self._unit_of_measurement = DATA_TYPES[data_type]
-                self.data_type = data_type
-                break
 
     def __str__(self):
         """Return the name of the sensor."""

tests/components/light/test_rfxtrx.py

@@ -194,7 +194,7 @@ class TestLightRfxtrx(unittest.TestCase):
         rfxtrx_core.RECEIVED_EVT_SUBSCRIBERS[0](event)
         entity = rfxtrx_core.RFX_DEVICES['0e611622']
         self.assertEqual(1, len(rfxtrx_core.RFX_DEVICES))
-        self.assertEqual('<Entity 0e611622 : 0b11009e00e6116202020070: on>',
+        self.assertEqual('<Entity 0b11009e00e6116202020070: on>',
                          entity.__str__())
 
         event = rfxtrx_core.get_rfx_object('0b11009e00e6116201010070')
@@ -210,7 +210,7 @@ class TestLightRfxtrx(unittest.TestCase):
         rfxtrx_core.RECEIVED_EVT_SUBSCRIBERS[0](event)
         entity = rfxtrx_core.RFX_DEVICES['118cdea2']
         self.assertEqual(2, len(rfxtrx_core.RFX_DEVICES))
-        self.assertEqual('<Entity 118cdea2 : 0b1100120118cdea02020070: on>',
+        self.assertEqual('<Entity 0b1100120118cdea02020070: on>',
                          entity.__str__())
 
         # trying to add a sensor

tests/components/sensor/test_rfxtrx.py

@@ -71,6 +71,25 @@ class TestSensorRfxtrx(unittest.TestCase):
                           'Humidity status numeric': 2},
                          entity.device_state_attributes)
 
+    def test_one_sensor_no_datatype(self):
+        """Test with 1 sensor."""
+        self.assertTrue(_setup_component(self.hass, 'sensor', {
+            'sensor': {'platform': 'rfxtrx',
+                       'devices':
+                           {'0a52080705020095220269': {
+                               'name': 'Test'}}}}))
+
+        self.assertEqual(1, len(rfxtrx_core.RFX_DEVICES))
+        entity = rfxtrx_core.RFX_DEVICES['sensor_0502']['Temperature']
+        self.assertEqual('Test', entity.name)
+        self.assertEqual(TEMP_CELSIUS, entity.unit_of_measurement)
+        self.assertEqual(14.9, entity.state)
+        self.assertEqual({'Humidity status': 'normal', 'Temperature': 14.9,
+                          'Rssi numeric': 6, 'Humidity': 34,
+                          'Battery numeric': 9,
+                          'Humidity status numeric': 2},
+                         entity.device_state_attributes)
+
     def test_several_sensors(self):
         """Test with 3 sensors."""
         self.assertTrue(_setup_component(self.hass, 'sensor', {
@@ -145,7 +164,7 @@ class TestSensorRfxtrx(unittest.TestCase):
                           'Battery numeric': 9,
                           'Humidity status numeric': 2},
                          entity.device_state_attributes)
-        self.assertEqual('sensor_0701 : 0a520801070100b81b0279',
+        self.assertEqual('0a520801070100b81b0279',
                          entity.__str__())
 
         rfxtrx_core.RECEIVED_EVT_SUBSCRIBERS[0](event)
@@ -162,7 +181,7 @@ class TestSensorRfxtrx(unittest.TestCase):
                           'Battery numeric': 9,
                           'Humidity status numeric': 2},
                          entity.device_state_attributes)
-        self.assertEqual('sensor_0502 : 0a52080405020095240279',
+        self.assertEqual('0a52080405020095240279',
                          entity.__str__())
 
         event = rfxtrx_core.get_rfx_object('0a52085e070100b31b0279')
@@ -176,7 +195,7 @@ class TestSensorRfxtrx(unittest.TestCase):
                           'Battery numeric': 9,
                           'Humidity status numeric': 2},
                          entity.device_state_attributes)
-        self.assertEqual('sensor_0701 : 0a520801070100b81b0279',
+        self.assertEqual('0a520801070100b81b0279',
                          entity.__str__())
 
         # trying to add a switch

tests/components/switch/test_rfxtrx.py

@@ -190,7 +190,7 @@ class TestSwitchRfxtrx(unittest.TestCase):
         rfxtrx_core.RECEIVED_EVT_SUBSCRIBERS[0](event)
         entity = rfxtrx_core.RFX_DEVICES['118cdea2']
         self.assertEqual(1, len(rfxtrx_core.RFX_DEVICES))
-        self.assertEqual('<Entity 118cdea2 : 0b1100100118cdea01010f70: on>',
+        self.assertEqual('<Entity 0b1100100118cdea01010f70: on>',
                          entity.__str__())
 
         rfxtrx_core.RECEIVED_EVT_SUBSCRIBERS[0](event)
@@ -203,7 +203,7 @@ class TestSwitchRfxtrx(unittest.TestCase):
         rfxtrx_core.RECEIVED_EVT_SUBSCRIBERS[0](event)
         entity = rfxtrx_core.RFX_DEVICES['118cdeb2']
         self.assertEqual(2, len(rfxtrx_core.RFX_DEVICES))
-        self.assertEqual('<Entity 118cdeb2 : 0b1100120118cdea02000070: on>',
+        self.assertEqual('<Entity 0b1100120118cdea02000070: on>',
                          entity.__str__())
 
         # Trying to add a sensor

tests/components/test_rfxtrx.py

@@ -37,10 +37,10 @@ class TestRFXTRX(unittest.TestCase):
                        'automatic_add': True,
                        'devices': {}}}))
 
-        while len(rfxtrx.RFX_DEVICES) < 2:
+        while len(rfxtrx.RFX_DEVICES) < 1:
             time.sleep(0.1)
 
-        self.assertEqual(len(rfxtrx.RFXOBJECT.sensors()), 2)
+        self.assertEqual(len(rfxtrx.RFXOBJECT.sensors()), 1)
 
     def test_valid_config(self):
         """Test configuration."""