Fix X10 commands for mochad light turn on (home-assistant#11146)

* Fix X10 commands for mochad light turn on

This commit attempts to address issues that a lot of people are having
with the x10 light component. Originally this was written to use the
xdim (extended dim) X10 command. However, not every X10 dimmer device
supports the xdim command. Additionally, it turns out the number of
dim/brighness levels the X10 device supports is device specific and
there is no way to detect this (given the mostly 1 way nature of X10)

To address these issues, this commit removes the usage of xdim and
instead relies on using the 'on' command and the 'dim' command. This
should work on all x10 light devices. In an attempt to address the
different dim/brightness levels supported by different devices this
commit also adds a new optional config value, 'brightness_levels', to
specify if it's either 32, 64, or 256. By default 32 levels are used
as this is the normal case and what is documented by mochad.

Fixes home-assistant#8943

* make code more readable

* fix style

* fix lint

* fix tests

homeassistant/components/light/mochad.py

@@ -12,20 +12,24 @@
 from homeassistant.components.light import (
     ATTR_BRIGHTNESS, SUPPORT_BRIGHTNESS, Light, PLATFORM_SCHEMA)
 from homeassistant.components import mochad
-from homeassistant.const import (CONF_NAME, CONF_PLATFORM, CONF_DEVICES,
-                                 CONF_ADDRESS)
+from homeassistant.const import (
+    CONF_NAME, CONF_PLATFORM, CONF_DEVICES, CONF_ADDRESS)
 from homeassistant.helpers import config_validation as cv
 
 DEPENDENCIES = ['mochad']
 _LOGGER = logging.getLogger(__name__)
 
+CONF_BRIGHTNESS_LEVELS = 'brightness_levels'
+
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Required(CONF_PLATFORM): mochad.DOMAIN,
     CONF_DEVICES: [{
         vol.Optional(CONF_NAME): cv.string,
         vol.Required(CONF_ADDRESS): cv.x10_address,
         vol.Optional(mochad.CONF_COMM_TYPE): cv.string,
+        vol.Optional(CONF_BRIGHTNESS_LEVELS, default=32):
+            vol.All(vol.Coerce(int), vol.In([32, 64, 256])),
     }]
 })
 
@@ -54,6 +58,7 @@ def __init__(self, hass, ctrl, dev):
                                     comm_type=self._comm_type)
         self._brightness = 0
         self._state = self._get_device_status()
+        self._brightness_levels = dev.get(CONF_BRIGHTNESS_LEVELS) - 1
 
     @property
     def brightness(self):
@@ -86,17 +91,47 @@ def assumed_state(self):
         """X10 devices are normally 1-way so we have to assume the state."""
         return True
 
+    def _calculate_brightness_value(self, value):
+        return int(value * (float(self._brightness_levels) / 255.0))
+
+    def _adjust_brightness(self, brightness):
+        if self._brightness > brightness:
+            bdelta = self._brightness - brightness
+            mochad_brightness = self._calculate_brightness_value(bdelta)
+            self.device.send_cmd("dim {}".format(mochad_brightness))
+            self._controller.read_data()
+        elif self._brightness < brightness:
+            bdelta = brightness - self._brightness
+            mochad_brightness = self._calculate_brightness_value(bdelta)
+            self.device.send_cmd("bright {}".format(mochad_brightness))
+            self._controller.read_data()
+
     def turn_on(self, **kwargs):
         """Send the command to turn the light on."""
-        self._brightness = kwargs.get(ATTR_BRIGHTNESS, 255)
+        brightness = kwargs.get(ATTR_BRIGHTNESS, 255)
         with mochad.REQ_LOCK:
-            self.device.send_cmd("xdim {}".format(self._brightness))
-            self._controller.read_data()
+            if self._brightness_levels > 32:
+                out_brightness = self._calculate_brightness_value(brightness)
+                self.device.send_cmd('xdim {}'.format(out_brightness))
+                self._controller.read_data()
+            else:
+                self.device.send_cmd("on")
+                self._controller.read_data()
+                # There is no persistence for X10 modules so a fresh on command
+                # will be full brightness
+                if self._brightness == 0:
+                    self._brightness = 255
+                self._adjust_brightness(brightness)
+        self._brightness = brightness
         self._state = True
 
     def turn_off(self, **kwargs):
         """Send the command to turn the light on."""
         with mochad.REQ_LOCK:
             self.device.send_cmd('off')
             self._controller.read_data()
+            # There is no persistence for X10 modules so we need to prepare
+            # to track a fresh on command will full brightness
+            if self._brightness_levels == 31:
+                self._brightness = 0
         self._state = False

tests/components/light/test_mochad.py

@@ -60,7 +60,8 @@ def setUp(self):
         """Setup things to be run when tests are started."""
         self.hass = get_test_home_assistant()
         controller_mock = mock.MagicMock()
-        dev_dict = {'address': 'a1', 'name': 'fake_light'}
+        dev_dict = {'address': 'a1', 'name': 'fake_light',
+                    'brightness_levels': 32}
         self.light = mochad.MochadLight(self.hass, controller_mock,
                                         dev_dict)
 
@@ -72,6 +73,39 @@ def test_name(self):
         """Test the name."""
         self.assertEqual('fake_light', self.light.name)
 
+    def test_turn_on_with_no_brightness(self):
+        """Test turn_on."""
+        self.light.turn_on()
+        self.light.device.send_cmd.assert_called_once_with('on')
+
+    def test_turn_on_with_brightness(self):
+        """Test turn_on."""
+        self.light.turn_on(brightness=45)
+        self.light.device.send_cmd.assert_has_calls(
+            [mock.call('on'), mock.call('dim 25')])
+
+    def test_turn_off(self):
+        """Test turn_off."""
+        self.light.turn_off()
+        self.light.device.send_cmd.assert_called_once_with('off')
+
+
+class TestMochadLight256Levels(unittest.TestCase):
+    """Test for mochad light platform."""
+
+    def setUp(self):
+        """Setup things to be run when tests are started."""
+        self.hass = get_test_home_assistant()
+        controller_mock = mock.MagicMock()
+        dev_dict = {'address': 'a1', 'name': 'fake_light',
+                    'brightness_levels': 256}
+        self.light = mochad.MochadLight(self.hass, controller_mock,
+                                        dev_dict)
+
+    def teardown_method(self, method):
+        """Stop everything that was started."""
+        self.hass.stop()
+
     def test_turn_on_with_no_brightness(self):
         """Test turn_on."""
         self.light.turn_on()
@@ -86,3 +120,35 @@ def test_turn_off(self):
         """Test turn_off."""
         self.light.turn_off()
         self.light.device.send_cmd.assert_called_once_with('off')
+
+
+class TestMochadLight64Levels(unittest.TestCase):
+    """Test for mochad light platform."""
+
+    def setUp(self):
+        """Setup things to be run when tests are started."""
+        self.hass = get_test_home_assistant()
+        controller_mock = mock.MagicMock()
+        dev_dict = {'address': 'a1', 'name': 'fake_light',
+                    'brightness_levels': 64}
+        self.light = mochad.MochadLight(self.hass, controller_mock,
+                                        dev_dict)
+
+    def teardown_method(self, method):
+        """Stop everything that was started."""
+        self.hass.stop()
+
+    def test_turn_on_with_no_brightness(self):
+        """Test turn_on."""
+        self.light.turn_on()
+        self.light.device.send_cmd.assert_called_once_with('xdim 63')
+
+    def test_turn_on_with_brightness(self):
+        """Test turn_on."""
+        self.light.turn_on(brightness=45)
+        self.light.device.send_cmd.assert_called_once_with('xdim 11')
+
+    def test_turn_off(self):
+        """Test turn_off."""
+        self.light.turn_off()
+        self.light.device.send_cmd.assert_called_once_with('off')