Change refactor structure

* Make a flatter one level inheritance, with MySensorsLight as parent
	with four children, one per light type.
* Break out helper methods. One per plain light, dimmer and RGB/RGBW
	children and per update, turn_on and turn_off, nine in total. Put
	these in the parent.
* Call the helper methods as needed from the child methods update,
	turn_on and turn_off.
* Change name of MySensorsLightLight to MySensorsLightPlain.
* Fix module docstrings according to pep257.
* Change name of color util method from rgb_hex_to_list to
	rgb_hex_to_rgb_list.
* Add unit tests for rgb_hex_to_rgb_list.

homeassistant/components/light/mysensors.py

@@ -1,11 +1,8 @@
 """
-homeassistant.components.light.mysensors.
-
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Support for MySensors lights.
 
 For more details about this platform, please refer to the documentation at
-https://home-assistant.io/components/light.mysensors.html
+https://home-assistant.io/components/light.mysensors/
 """
 import logging
 
@@ -13,7 +10,7 @@ from homeassistant.components.light import (
     Light, ATTR_BRIGHTNESS, ATTR_RGB_COLOR)
 
 from homeassistant.util.color import (
-    rgb_hex_to_list)
+    rgb_hex_to_rgb_list)
 
 from homeassistant.const import (
     ATTR_BATTERY_LEVEL,
@@ -23,6 +20,8 @@ import homeassistant.components.mysensors as mysensors
 
 _LOGGER = logging.getLogger(__name__)
 ATTR_RGB_WHITE = 'rgb_white'
+ATTR_VALUE = 'value'
+ATTR_VALUE_TYPE = 'value_type'
 
 
 def setup_platform(hass, config, add_devices, discovery_info=None):
@@ -42,7 +41,7 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
             pres.S_DIMMER: [set_req.V_DIMMER],
         }
         device_class_map = {
-            pres.S_LIGHT: MySensorsLightLight,
+            pres.S_LIGHT: MySensorsLightPlain,
             pres.S_DIMMER: MySensorsLightDimmer,
         }
         if float(gateway.version) >= 1.5:
@@ -75,6 +74,9 @@ class MySensorsLight(Light):
         self.battery_level = 0
         self._values = {}
         self._state = None
+        self._brightness = None
+        self._rgb = None
+        self._white = None
 
     @property
     def should_poll(self):
@@ -86,6 +88,21 @@ class MySensorsLight(Light):
         """The name of this entity."""
         return self._name
 
+    @property
+    def brightness(self):
+        """Brightness of this light between 0..255."""
+        return self._brightness
+
+    @property
+    def rgb_color(self):
+        """RGB color value [int, int, int]."""
+        return self._rgb
+
+    @property
+    def rgb_white(self):  # not implemented in the frontend yet
+        """White value in RGBW, value between 0..255."""
+        return self._white
+
     @property
     def device_state_attributes(self):
         """Return device specific state attributes."""
@@ -109,49 +126,8 @@ class MySensorsLight(Light):
         """True if device is on."""
         return self._state
 
-    def turn_on(self, **kwargs):
+    def _turn_on_light(self):
-        """Turn the device on."""
+        """Turn on light child device."""
-        if self.gateway.optimistic:
-            # optimistically assume that light has changed state
-            self.update_ha_state()
-
-    def turn_off(self, **kwargs):
-        """Turn the device off."""
-        value_type = kwargs.get('value_type')
-        value = kwargs.get('value')
-        if value_type is not None and value is not None:
-            self.gateway.set_child_value(
-                self.node_id, self.child_id, value_type, value)
-        else:
-            _LOGGER.warning(
-                '%s: value_type %s, value = %s, '
-                'None is not valid argument when setting child value'
-                '', self._name, value_type, value)
-        if self.gateway.optimistic:
-            # optimistically assume that light has changed state
-            self._state = False
-            self.update_ha_state()
-
-    def update(self):
-        """Update the controller with the latest value from a sensor."""
-        node = self.gateway.sensors[self.node_id]
-        child = node.children[self.child_id]
-        self.battery_level = node.battery_level
-        for value_type, value in child.values.items():
-            _LOGGER.debug(
-                '%s: value_type %s, value = %s', self._name, value_type, value)
-            self._values[value_type] = value
-
-
-class MySensorsLightLight(MySensorsLight):
-    """Light child class to MySensorsLight."""
-
-    def __init__(self, *args):
-        """Setup instance attributes."""
-        super().__init__(*args)
-
-    def turn_on(self, **kwargs):
-        """Turn the device on."""
         set_req = self.gateway.const.SetReq
 
         if not self._state:
@@ -161,44 +137,9 @@ class MySensorsLightLight(MySensorsLight):
         if self.gateway.optimistic:
             # optimistically assume that light has changed state
             self._state = True
-        super().turn_on(**kwargs)
 
-    def turn_off(self, **kwargs):
+    def _turn_on_dimmer(self, **kwargs):
-        """Turn the device off."""
+        """Turn on dimmer child device."""
-        set_req = self.gateway.const.SetReq
-        value_type = kwargs.get('value_type')
-        value = kwargs.get('value')
-        value_type = (
-            set_req.V_LIGHT
-            if set_req.V_LIGHT in self._values else value_type)
-        value = 0 if set_req.V_LIGHT in self._values else value
-        super().turn_off(value_type=value_type, value=value)
-
-    def update(self):
-        """Update the controller with the latest value from a sensor."""
-        super().update()
-        value_type = self.gateway.const.SetReq.V_LIGHT
-        if value_type in self._values:
-            self._values[value_type] = (
-                STATE_ON if int(self._values[value_type]) == 1 else STATE_OFF)
-            self._state = self._values[value_type] == STATE_ON
-
-
-class MySensorsLightDimmer(MySensorsLightLight):
-    """Dimmer child class to MySensorsLight."""
-
-    def __init__(self, *args):
-        """Setup instance attributes."""
-        self._brightness = None
-        super().__init__(*args)
-
-    @property
-    def brightness(self):
-        """Brightness of this light between 0..255."""
-        return self._brightness
-
-    def turn_on(self, **kwargs):
-        """Turn the device on."""
         set_req = self.gateway.const.SetReq
         brightness = self._brightness
 
@@ -212,22 +153,89 @@ class MySensorsLightDimmer(MySensorsLightLight):
         if self.gateway.optimistic:
             # optimistically assume that light has changed state
             self._brightness = brightness
-        super().turn_on(**kwargs)
 
-    def turn_off(self, **kwargs):
+    def _turn_on_rgb_and_w(self, hex_template, **kwargs):
-        """Turn the device off."""
+        """Turn on RGB or RGBW child device."""
+        rgb = self._rgb
+        white = self._white
+
+        if ATTR_RGB_WHITE in kwargs and \
+                kwargs[ATTR_RGB_WHITE] != self._white:
+            white = kwargs[ATTR_RGB_WHITE]
+
+        if ATTR_RGB_COLOR in kwargs and \
+                kwargs[ATTR_RGB_COLOR] != self._rgb:
+            rgb = kwargs[ATTR_RGB_COLOR]
+            if white is not None and hex_template == '%02x%02x%02x%02x':
+                rgb.append(white)
+            hex_color = hex_template % tuple(rgb)
+            self.gateway.set_child_value(
+                self.node_id, self.child_id, self.value_type, hex_color)
+
+        if self.gateway.optimistic:
+            # optimistically assume that light has changed state
+            self._rgb = rgb
+            self._white = white
+
+    def _turn_on(self):
+        """Turn the device on."""
+        if self.gateway.optimistic:
+            # optimistically assume that light has changed state
+            self.update_ha_state()
+
+    def _turn_off_light(self, value_type=None, value=None):
+        """Turn off light child device."""
+        set_req = self.gateway.const.SetReq
+        value_type = (
+            set_req.V_LIGHT
+            if set_req.V_LIGHT in self._values else value_type)
+        value = 0 if set_req.V_LIGHT in self._values else value
+        return {ATTR_VALUE_TYPE: value_type, ATTR_VALUE: value}
+
+    def _turn_off_dimmer(self, value_type=None, value=None):
+        """Turn off dimmer child device."""
         set_req = self.gateway.const.SetReq
-        value_type = kwargs.get('value_type')
-        value = kwargs.get('value')
         value_type = (
             set_req.V_DIMMER
             if set_req.V_DIMMER in self._values else value_type)
         value = 0 if set_req.V_DIMMER in self._values else value
-        super().turn_off(value_type=value_type, value=value)
+        return {ATTR_VALUE_TYPE: value_type, ATTR_VALUE: value}
 
-    def update(self):
+    def _turn_off_rgb_or_w(self, value_type=None, value=None):
-        """Update the controller with the latest value from a sensor."""
+        """Turn off RGB or RGBW child device."""
-        super().update()
+        if float(self.gateway.version) >= 1.5:
+            set_req = self.gateway.const.SetReq
+            if self.value_type == set_req.V_RGB:
+                value = '000000'
+            elif self.value_type == set_req.V_RGBW:
+                value = '00000000'
+        return {ATTR_VALUE_TYPE: self.value_type, ATTR_VALUE: value}
+
+    def _turn_off(self, value_type=None, value=None):
+        """Turn the device off."""
+        if value_type is None or value is None:
+            _LOGGER.warning(
+                '%s: value_type %s, value = %s, '
+                'None is not valid argument when setting child value'
+                '', self._name, value_type, value)
+            return
+        self.gateway.set_child_value(
+            self.node_id, self.child_id, value_type, value)
+        if self.gateway.optimistic:
+            # optimistically assume that light has changed state
+            self._state = False
+            self.update_ha_state()
+
+    def _update_light(self):
+        """Update the controller with values from light child."""
+        value_type = self.gateway.const.SetReq.V_LIGHT
+        if value_type in self._values:
+            self._values[value_type] = (
+                STATE_ON if int(self._values[value_type]) == 1 else STATE_OFF)
+            self._state = self._values[value_type] == STATE_ON
+
+    def _update_dimmer(self):
+        """Update the controller with values from dimmer child."""
         set_req = self.gateway.const.SetReq
         value_type = set_req.V_DIMMER
         if value_type in self._values:
@@ -237,121 +245,140 @@ class MySensorsLightDimmer(MySensorsLightLight):
             if set_req.V_LIGHT not in self._values:
                 self._state = self._brightness > 0
 
+    def _update_rgb_or_w(self):
+        """Update the controller with values from RGB or RGBW child."""
+        set_req = self.gateway.const.SetReq
+        value = self._values[self.value_type]
+        color_list = rgb_hex_to_rgb_list(value)
+        if set_req.V_LIGHT not in self._values and \
+                set_req.V_DIMMER not in self._values:
+            self._state = max(color_list) > 0
+        if len(color_list) > 3:
+            self._white = color_list.pop()
+        self._rgb = color_list
 
-class MySensorsLightRGB(MySensorsLightDimmer):
+    def _update(self):
+        """Update the controller with the latest value from a sensor."""
+        node = self.gateway.sensors[self.node_id]
+        child = node.children[self.child_id]
+        self.battery_level = node.battery_level
+        for value_type, value in child.values.items():
+            _LOGGER.debug(
+                '%s: value_type %s, value = %s', self._name, value_type, value)
+            self._values[value_type] = value
+
+
+class MySensorsLightPlain(MySensorsLight):
+    """Light child class to MySensorsLight."""
+
+    def __init__(self, *args):
+        """Setup instance attributes."""
+        super().__init__(*args)
+
+    def turn_on(self, **kwargs):
+        """Turn the device on."""
+        super()._turn_on_light()
+        super()._turn_on()
+
+    def turn_off(self, **kwargs):
+        """Turn the device off."""
+        ret = super()._turn_off_light()
+        super()._turn_off(value_type=ret[
+            ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
+
+    def update(self):
+        """Update the controller with the latest value from a sensor."""
+        super()._update()
+        super()._update_light()
+
+
+class MySensorsLightDimmer(MySensorsLight):
+    """Dimmer child class to MySensorsLight."""
+
+    def __init__(self, *args):
+        """Setup instance attributes."""
+        super().__init__(*args)
+
+    def turn_on(self, **kwargs):
+        """Turn the device on."""
+        super()._turn_on_light()
+        super()._turn_on_dimmer(**kwargs)
+        super()._turn_on()
+
+    def turn_off(self, **kwargs):
+        """Turn the device off."""
+        ret = super()._turn_off_dimmer()
+        ret = super()._turn_off_light(
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
+        super()._turn_off(
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
+
+    def update(self):
+        """Update the controller with the latest value from a sensor."""
+        super()._update()
+        super()._update_light()
+        super()._update_dimmer()
+
+
+class MySensorsLightRGB(MySensorsLight):
     """RGB child class to MySensorsLight."""
 
     def __init__(self, *args):
         """Setup instance attributes."""
-        self._rgb = None
         super().__init__(*args)
 
-    @property
-    def rgb_color(self):
-        """RGB color value [int, int, int]."""
-        return self._rgb
-
     def turn_on(self, **kwargs):
         """Turn the device on."""
-        rgb = self._rgb
+        super()._turn_on_light()
-        if ATTR_RGB_COLOR in kwargs and kwargs[ATTR_RGB_COLOR] != self._rgb:
+        super()._turn_on_dimmer(**kwargs)
-            rgb = kwargs[ATTR_RGB_COLOR]
+        super()._turn_on_rgb_and_w('%02x%02x%02x', **kwargs)
-            hex_color = '%02x%02x%02x' % tuple(rgb)
+        super()._turn_on()
-            self.gateway.set_child_value(
-                self.node_id, self.child_id, self.value_type, hex_color)
-
-        if self.gateway.optimistic:
-            # optimistically assume that light has changed state
-            self._rgb = rgb
-        super().turn_on(**kwargs)
 
     def turn_off(self, **kwargs):
         """Turn the device off."""
-        value_type = None
+        ret = super()._turn_off_rgb_or_w()
-        value = None
+        ret = super()._turn_off_dimmer(
-        if float(self.gateway.version) >= 1.5:
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
-            value_type = self.gateway.const.SetReq.V_RGB
+        ret = super()._turn_off_light(
-            value = '000000'
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
-        super().turn_off(value_type=value_type, value=value)
+        super()._turn_off(
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
 
     def update(self):
         """Update the controller with the latest value from a sensor."""
-        super().update()
+        super()._update()
-        set_req = self.gateway.const.SetReq
+        super()._update_light()
-        if float(self.gateway.version) >= 1.5 and \
+        super()._update_dimmer()
-                set_req.V_RGB in self._values:
+        super()._update_rgb_or_w()
-            value = self._values[set_req.V_RGB]
-            self._rgb = rgb_hex_to_list(value)
-            if set_req.V_LIGHT not in self._values and \
-                    set_req.V_DIMMER not in self._values:
-                self._state = max(self._rgb) > 0
 
 
-class MySensorsLightRGBW(MySensorsLightDimmer):
+class MySensorsLightRGBW(MySensorsLight):
     """RGBW child class to MySensorsLight."""
 
     def __init__(self, *args):
         """Setup instance attributes."""
-        self._rgb = None
-        self._white = None
         super().__init__(*args)
 
-    @property
-    def rgb_color(self):
-        """RGB color value [int, int, int]."""
-        return self._rgb
-
-    @property
-    def rgb_white(self):  # not implemented in the frontend yet
-        """White value in RGBW, value between 0..255."""
-        return self._white
-
     def turn_on(self, **kwargs):
         """Turn the device on."""
-        rgb = self._rgb
+        super()._turn_on_light()
-        white = self._white
+        super()._turn_on_dimmer(**kwargs)
-
+        super()._turn_on_rgb_and_w('%02x%02x%02x%02x', **kwargs)
-        if float(self.gateway.version) >= 1.5:
+        super()._turn_on()
-
-            if ATTR_RGB_WHITE in kwargs and \
-                    kwargs[ATTR_RGB_WHITE] != self._white:
-                white = kwargs[ATTR_RGB_WHITE]
-
-            if ATTR_RGB_COLOR in kwargs and \
-                    kwargs[ATTR_RGB_COLOR] != self._rgb:
-                rgb = kwargs[ATTR_RGB_COLOR]
-                if white is not None:
-                    rgb.append(white)
-                hex_color = '%02x%02x%02x%02x' % tuple(rgb)
-                self.gateway.set_child_value(
-                    self.node_id, self.child_id, self.value_type, hex_color)
-
-        if self.gateway.optimistic:
-            # optimistically assume that light has changed state
-            self._rgb = rgb
-            self._white = white
-        super().turn_on(**kwargs)
 
     def turn_off(self, **kwargs):
         """Turn the device off."""
-        value_type = None
+        ret = super()._turn_off_rgb_or_w()
-        value = None
+        ret = super()._turn_off_dimmer(
-        if float(self.gateway.version) >= 1.5:
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
-            value_type = self.gateway.const.SetReq.V_RGBW
+        ret = super()._turn_off_light(
-            value = '00000000'
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
-        super().turn_off(value_type=value_type, value=value)
+        super()._turn_off(
+            value_type=ret[ATTR_VALUE_TYPE], value=ret[ATTR_VALUE])
 
     def update(self):
         """Update the controller with the latest value from a sensor."""
-        super().update()
+        super()._update()
-        set_req = self.gateway.const.SetReq
+        super()._update_light()
-        if float(self.gateway.version) >= 1.5 and \
+        super()._update_dimmer()
-                set_req.V_RGBW in self._values:
+        super()._update_rgb_or_w()
-            value = self._values[set_req.V_RGBW]
-            color_list = rgb_hex_to_list(value)
-            if set_req.V_LIGHT not in self._values and \
-                    set_req.V_DIMMER not in self._values:
-                self._state = max(color_list) > 0
-            if len(color_list) > 3:
-                self._white = color_list.pop()
-            self._rgb = color_list

homeassistant/components/sensor/mysensors.py

@@ -1,7 +1,4 @@
 """
-homeassistant.components.sensor.mysensors.
-
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Support for MySensors sensors.
 
 For more details about this platform, please refer to the documentation at

homeassistant/components/switch/mysensors.py

@@ -1,11 +1,8 @@
 """
-homeassistant.components.switch.mysensors.
-
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Support for MySensors switches.
 
 For more details about this platform, please refer to the documentation at
-https://home-assistant.io/components/sensor.mysensors.html
+https://home-assistant.io/components/switch.mysensors/
 """
 import logging
 

homeassistant/util/color.py

@@ -1,9 +1,4 @@
-"""
+"""Color util methods."""
-homeassistant.util.color.
-
-~~~~~~~~~~~~~~~~~~~~~~~~
-Color util methods.
-"""
 
 
 # Taken from: http://www.cse.unr.edu/~quiroz/inc/colortransforms.py
@@ -89,7 +84,7 @@ def color_xy_brightness_to_RGB(vX, vY, brightness):
     return (r, g, b)
 
 
-def rgb_hex_to_list(hex_string):
+def rgb_hex_to_rgb_list(hex_string):
     """Return an RGB color value list from a hex color string."""
     return [int(hex_string[i:i + len(hex_string) // 3], 16)
             for i in range(0,

tests/util/test_color.py

@@ -1,17 +1,14 @@
-"""
+"""Tests Home Assistant color util methods."""
-tests.util.test_color
-~~~~~~~~~~~~~~~~~~~~~
-
-Tests Home Assistant color util methods.
-"""
 import unittest
 import homeassistant.util.color as color_util
 
 
 class TestColorUtil(unittest.TestCase):
+    """Test color util methods."""
+
     # pylint: disable=invalid-name
     def test_color_RGB_to_xy(self):
-        """ Test color_RGB_to_xy. """
+        """Test color_RGB_to_xy."""
         self.assertEqual((0, 0), color_util.color_RGB_to_xy(0, 0, 0))
         self.assertEqual((0.3127159072215825, 0.3290014805066623),
                          color_util.color_RGB_to_xy(255, 255, 255))
@@ -25,7 +22,7 @@ class TestColorUtil(unittest.TestCase):
                          color_util.color_RGB_to_xy(255, 0, 0))
 
     def test_color_xy_brightness_to_RGB(self):
-        """ Test color_RGB_to_xy. """
+        """Test color_RGB_to_xy."""
         self.assertEqual((0, 0, 0),
                          color_util.color_xy_brightness_to_RGB(1, 1, 0))
 
@@ -40,3 +37,23 @@ class TestColorUtil(unittest.TestCase):
 
         self.assertEqual((0, 83, 255),
                          color_util.color_xy_brightness_to_RGB(0, 0, 255))
+
+    def test_rgb_hex_to_rgb_list(self):
+        """Test rgb_hex_to_rgb_list."""
+        self.assertEqual([255, 255, 255],
+                         color_util.rgb_hex_to_rgb_list('ffffff'))
+
+        self.assertEqual([0, 0, 0],
+                         color_util.rgb_hex_to_rgb_list('000000'))
+
+        self.assertEqual([255, 255, 255, 255],
+                         color_util.rgb_hex_to_rgb_list('ffffffff'))
+
+        self.assertEqual([0, 0, 0, 0],
+                         color_util.rgb_hex_to_rgb_list('00000000'))
+
+        self.assertEqual([51, 153, 255],
+                         color_util.rgb_hex_to_rgb_list('3399ff'))
+
+        self.assertEqual([51, 153, 255, 0],
+                         color_util.rgb_hex_to_rgb_list('3399ff00'))