From 7db8bbf38599e97560dc224d8259a9b9d2c4265d Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Sat, 16 Dec 2017 15:52:40 -0800 Subject: [PATCH] Fix X10 commands for mochad light turn on (#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 #8943 * make code more readable * fix style * fix lint * fix tests --- homeassistant/components/light/mochad.py | 45 ++++++++++++++-- tests/components/light/test_mochad.py | 68 +++++++++++++++++++++++- 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/light/mochad.py b/homeassistant/components/light/mochad.py index 3d67edaf7cb..efc62b05434 100644 --- a/homeassistant/components/light/mochad.py +++ b/homeassistant/components/light/mochad.py @@ -12,13 +12,15 @@ import voluptuous as vol 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, @@ -26,6 +28,8 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ 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 @@ class MochadLight(Light): 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,12 +91,38 @@ class MochadLight(Light): """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): @@ -99,4 +130,8 @@ class MochadLight(Light): 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 diff --git a/tests/components/light/test_mochad.py b/tests/components/light/test_mochad.py index e69ebdb4aef..5c82ab06085 100644 --- a/tests/components/light/test_mochad.py +++ b/tests/components/light/test_mochad.py @@ -60,7 +60,8 @@ class TestMochadLight(unittest.TestCase): """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 @@ class TestMochadLight(unittest.TestCase): """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 @@ class TestMochadLight(unittest.TestCase): """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')