From 617647c5fd1705848dd2bf425de9d7ace3a42103 Mon Sep 17 00:00:00 2001 From: Bob Clough Date: Thu, 21 Jun 2018 19:59:03 +0100 Subject: [PATCH] Fix MQTT Light with RGB and Brightness (#15053) * Fix MQTT Light with RGB and Brightness When an MQTT light is given an RGB and Brightness topic, the RGB is scaled by the brightness *as well* as the brightness being set This causes 255,0,0 at 50% brightness to be sent as 127,0,0 at 50% brightness, which ends up as 63,0,0 after the RGB bulb has applied its brightness scaling. Fixes the same issue in mqtt, mqtt-json and mqtt-template. Related Issue: #13725 * Add comment to mqtt_json as well --- homeassistant/components/light/mqtt.py | 11 +++++++++-- homeassistant/components/light/mqtt_json.py | 11 ++++++++--- homeassistant/components/light/mqtt_template.py | 11 +++++++++-- tests/components/light/test_mqtt.py | 14 +++++++------- tests/components/light/test_mqtt_json.py | 4 ++-- 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/light/mqtt.py b/homeassistant/components/light/mqtt.py index 97a4cc8c137..c0e363f85d6 100644 --- a/homeassistant/components/light/mqtt.py +++ b/homeassistant/components/light/mqtt.py @@ -442,8 +442,15 @@ class MqttLight(MqttAvailability, Light): self._topic[CONF_RGB_COMMAND_TOPIC] is not None: hs_color = kwargs[ATTR_HS_COLOR] - brightness = kwargs.get( - ATTR_BRIGHTNESS, self._brightness if self._brightness else 255) + + # If there's a brightness topic set, we don't want to scale the RGB + # values given using the brightness. + if self._topic[CONF_BRIGHTNESS_COMMAND_TOPIC] is not None: + brightness = 255 + else: + brightness = kwargs.get( + ATTR_BRIGHTNESS, self._brightness if self._brightness else + 255) rgb = color_util.color_hsv_to_RGB( hs_color[0], hs_color[1], brightness / 255 * 100) tpl = self._templates[CONF_RGB_COMMAND_TEMPLATE] diff --git a/homeassistant/components/light/mqtt_json.py b/homeassistant/components/light/mqtt_json.py index 14f5ee7a9b9..705e106fdff 100644 --- a/homeassistant/components/light/mqtt_json.py +++ b/homeassistant/components/light/mqtt_json.py @@ -345,9 +345,14 @@ class MqttJson(MqttAvailability, Light): hs_color = kwargs[ATTR_HS_COLOR] message['color'] = {} if self._rgb: - brightness = kwargs.get( - ATTR_BRIGHTNESS, - self._brightness if self._brightness else 255) + # If there's a brightness topic set, we don't want to scale the + # RGB values given using the brightness. + if self._brightness is not None: + brightness = 255 + else: + brightness = kwargs.get( + ATTR_BRIGHTNESS, + self._brightness if self._brightness else 255) rgb = color_util.color_hsv_to_RGB( hs_color[0], hs_color[1], brightness / 255 * 100) message['color']['r'] = rgb[0] diff --git a/homeassistant/components/light/mqtt_template.py b/homeassistant/components/light/mqtt_template.py index e32c13fc5b6..f6b3fbe8b70 100644 --- a/homeassistant/components/light/mqtt_template.py +++ b/homeassistant/components/light/mqtt_template.py @@ -317,8 +317,15 @@ class MqttTemplate(MqttAvailability, Light): if ATTR_HS_COLOR in kwargs: hs_color = kwargs[ATTR_HS_COLOR] - brightness = kwargs.get( - ATTR_BRIGHTNESS, self._brightness if self._brightness else 255) + + # If there's a brightness topic set, we don't want to scale the RGB + # values given using the brightness. + if self._templates[CONF_BRIGHTNESS_TEMPLATE] is not None: + brightness = 255 + else: + brightness = kwargs.get( + ATTR_BRIGHTNESS, self._brightness if self._brightness else + 255) rgb = color_util.color_hsv_to_RGB( hs_color[0], hs_color[1], brightness / 255 * 100) values['red'] = rgb[0] diff --git a/tests/components/light/test_mqtt.py b/tests/components/light/test_mqtt.py index 8b51adb2187..49bcd8a73ec 100644 --- a/tests/components/light/test_mqtt.py +++ b/tests/components/light/test_mqtt.py @@ -523,24 +523,24 @@ class TestLightMQTT(unittest.TestCase): self.mock_publish.reset_mock() light.turn_on(self.hass, 'light.test', brightness=50, xy_color=[0.123, 0.123]) - light.turn_on(self.hass, 'light.test', rgb_color=[75, 75, 75], + light.turn_on(self.hass, 'light.test', rgb_color=[255, 128, 0], white_value=80) self.hass.block_till_done() self.mock_publish.async_publish.assert_has_calls([ mock.call('test_light_rgb/set', 'on', 2, False), - mock.call('test_light_rgb/rgb/set', '50,50,50', 2, False), + mock.call('test_light_rgb/rgb/set', '255,128,0', 2, False), mock.call('test_light_rgb/brightness/set', 50, 2, False), mock.call('test_light_rgb/white_value/set', 80, 2, False), - mock.call('test_light_rgb/xy/set', '0.323,0.329', 2, False), + mock.call('test_light_rgb/xy/set', '0.14,0.131', 2, False), ], any_order=True) state = self.hass.states.get('light.test') self.assertEqual(STATE_ON, state.state) - self.assertEqual((255, 255, 255), state.attributes['rgb_color']) + self.assertEqual((255, 128, 0), state.attributes['rgb_color']) self.assertEqual(50, state.attributes['brightness']) self.assertEqual(80, state.attributes['white_value']) - self.assertEqual((0.323, 0.329), state.attributes['xy_color']) + self.assertEqual((0.611, 0.375), state.attributes['xy_color']) def test_sending_mqtt_rgb_command_with_template(self): """Test the sending of RGB command with template.""" @@ -808,11 +808,11 @@ class TestLightMQTT(unittest.TestCase): # Turn on w/ just a color to insure brightness gets # added and sent. - light.turn_on(self.hass, 'light.test', rgb_color=[75, 75, 75]) + light.turn_on(self.hass, 'light.test', rgb_color=[255, 128, 0]) self.hass.block_till_done() self.mock_publish.async_publish.assert_has_calls([ - mock.call('test_light/rgb', '50,50,50', 0, False), + mock.call('test_light/rgb', '255,128,0', 0, False), mock.call('test_light/bright', 50, 0, False) ], any_order=True) diff --git a/tests/components/light/test_mqtt_json.py b/tests/components/light/test_mqtt_json.py index 275fb42ede9..af560bff9c3 100644 --- a/tests/components/light/test_mqtt_json.py +++ b/tests/components/light/test_mqtt_json.py @@ -381,8 +381,8 @@ class TestLightMQTTJSON(unittest.TestCase): self.assertEqual(50, message_json["brightness"]) self.assertEqual({ 'r': 0, - 'g': 50, - 'b': 4, + 'g': 255, + 'b': 21, }, message_json["color"]) self.assertEqual("ON", message_json["state"])