Merge pull request #678 from balloob/mqtt-light

Fixes for MQTT light with RGB colors

homeassistant/components/light/mqtt.py

@@ -8,7 +8,6 @@ https://home-assistant.io/components/light.mqtt/
 """
 import logging
 
-import homeassistant.util.color as color_util
 import homeassistant.components.mqtt as mqtt
 from homeassistant.components.light import (Light,
                                             ATTR_BRIGHTNESS, ATTR_RGB_COLOR)
@@ -37,45 +36,40 @@ def setup_platform(hass, config, add_devices_callback, discovery_info=None):
     add_devices_callback([MqttLight(
         hass,
         config.get('name', DEFAULT_NAME),
-        {"state_topic": config.get('state_topic'),
+        {
-         "command_topic": config.get('command_topic'),
+            "state_topic": config.get('state_topic'),
-         "brightness_state_topic": config.get('brightness_state_topic'),
+            "command_topic": config.get('command_topic'),
-         "brightness_command_topic":
+            "brightness_state_topic": config.get('brightness_state_topic'),
-             config.get('brightness_command_topic'),
+            "brightness_command_topic": config.get('brightness_command_topic'),
-         "rgb_state_topic": config.get('rgb_state_topic'),
+            "rgb_state_topic": config.get('rgb_state_topic'),
-         "rgb_command_topic": config.get('rgb_command_topic')},
+            "rgb_command_topic": config.get('rgb_command_topic')
-        config.get('rgb', None),
+        },
         config.get('qos', DEFAULT_QOS),
-        {"on": config.get('payload_on', DEFAULT_PAYLOAD_ON),
+        {
-         "off": config.get('payload_off', DEFAULT_PAYLOAD_OFF)},
+            "on": config.get('payload_on', DEFAULT_PAYLOAD_ON),
-        config.get('brightness'),
+            "off": config.get('payload_off', DEFAULT_PAYLOAD_OFF)
+        },
         config.get('optimistic', DEFAULT_OPTIMISTIC))])
 
-# pylint: disable=too-many-instance-attributes
-
 
 class MqttLight(Light):
     """ Provides a MQTT light. """
 
-    # pylint: disable=too-many-arguments
+    # pylint: disable=too-many-arguments,too-many-instance-attributes
-    def __init__(self, hass, name,
+    def __init__(self, hass, name, topic, qos, payload, optimistic):
-                 topic,
-                 rgb, qos,
-                 payload,
-                 brightness, optimistic):
 
         self._hass = hass
         self._name = name
         self._topic = topic
-        self._rgb = rgb
         self._qos = qos
         self._payload = payload
-        self._brightness = brightness
+        self._optimistic = optimistic or topic["state_topic"] is None
-        self._optimistic = optimistic
+        self._optimistic_rgb = optimistic or topic["rgb_state_topic"] is None
+        self._optimistic_brightness = (optimistic or
+                                       topic["brightness_state_topic"] is None)
         self._state = False
-        self._xy = None
 
-        def message_received(topic, payload, qos):
+        def state_received(topic, payload, qos):
             """ A new MQTT message has been received. """
             if payload == self._payload["on"]:
                 self._state = True
@@ -84,27 +78,15 @@ class MqttLight(Light):
 
             self.update_ha_state()
 
-        if self._topic["state_topic"] is None:
+        if self._topic["state_topic"] is not None:
-            # force optimistic mode
-            self._optimistic = True
-        else:
-            # Subscribe the state_topic
             mqtt.subscribe(self._hass, self._topic["state_topic"],
-                           message_received, self._qos)
+                           state_received, self._qos)
 
         def brightness_received(topic, payload, qos):
             """ A new MQTT message for the brightness has been received. """
             self._brightness = int(payload)
             self.update_ha_state()
 
-        def rgb_received(topic, payload, qos):
-            """ A new MQTT message has been received. """
-            self._rgb = [int(val) for val in payload.split(',')]
-            self._xy = color_util.color_RGB_to_xy(int(self._rgb[0]),
-                                                  int(self._rgb[1]),
-                                                  int(self._rgb[2]))
-            self.update_ha_state()
-
         if self._topic["brightness_state_topic"] is not None:
             mqtt.subscribe(self._hass, self._topic["brightness_state_topic"],
                            brightness_received, self._qos)
@@ -112,12 +94,17 @@ class MqttLight(Light):
         else:
             self._brightness = None
 
+        def rgb_received(topic, payload, qos):
+            """ A new MQTT message has been received. """
+            self._rgb = [int(val) for val in payload.split(',')]
+            self.update_ha_state()
+
         if self._topic["rgb_state_topic"] is not None:
             mqtt.subscribe(self._hass, self._topic["rgb_state_topic"],
                            rgb_received, self._qos)
-            self._xy = [0, 0]
+            self._rgb = [255, 255, 255]
         else:
-            self._xy = None
+            self._rgb = None
 
     @property
     def brightness(self):
@@ -129,11 +116,6 @@ class MqttLight(Light):
         """ RGB color value. """
         return self._rgb
 
-    @property
-    def color_xy(self):
-        """ RGB color value. """
-        return self._xy
-
     @property
     def should_poll(self):
         """ No polling needed for a MQTT light. """
@@ -151,19 +133,25 @@ class MqttLight(Light):
 
     def turn_on(self, **kwargs):
         """ Turn the device on. """
+        should_update = False
 
         if ATTR_RGB_COLOR in kwargs and \
            self._topic["rgb_command_topic"] is not None:
-            self._rgb = kwargs[ATTR_RGB_COLOR]
             rgb = DEFAULT_RGB_PATTERN % tuple(self._rgb)
             mqtt.publish(self._hass, self._topic["rgb_command_topic"],
                          rgb, self._qos)
 
+            if self._optimistic_rgb:
+                self._rgb = kwargs[ATTR_RGB_COLOR]
+                should_update = True
+
         if ATTR_BRIGHTNESS in kwargs and \
            self._topic["brightness_command_topic"] is not None:
-            self._brightness = kwargs[ATTR_BRIGHTNESS]
             mqtt.publish(self._hass, self._topic["brightness_command_topic"],
                          self._brightness, self._qos)
+            if self._optimistic_brightness:
+                self._brightness = kwargs[ATTR_BRIGHTNESS]
+                should_update = True
 
         mqtt.publish(self._hass, self._topic["command_topic"],
                      self._payload["on"], self._qos)
@@ -171,6 +159,9 @@ class MqttLight(Light):
         if self._optimistic:
             # optimistically assume that switch has changed state
             self._state = True
+            should_update = True
+
+        if should_update:
             self.update_ha_state()
 
     def turn_off(self, **kwargs):

tests/components/light/test_mqtt.py

@@ -44,7 +44,6 @@ light:
   payload_off: "off"
 """
 import unittest
-import homeassistant.util.color as color_util
 
 from homeassistant.const import STATE_ON, STATE_OFF
 import homeassistant.core as ha
@@ -63,6 +62,29 @@ class TestLightMQTT(unittest.TestCase):
         """ Stop down stuff we started. """
         self.hass.stop()
 
+    def test_no_color_or_brightness_if_no_topics(self):
+        self.assertTrue(light.setup(self.hass, {
+            'light': {
+                'platform': 'mqtt',
+                'name': 'test',
+                'state_topic': 'test_light_rgb/status',
+                'command_topic': 'test_light_rgb/set',
+            }
+        }))
+
+        state = self.hass.states.get('light.test')
+        self.assertEqual(STATE_OFF, state.state)
+        self.assertIsNone(state.attributes.get('rgb_color'))
+        self.assertIsNone(state.attributes.get('brightness'))
+
+        fire_mqtt_message(self.hass, 'test_light_rgb/status', 'on')
+        self.hass.pool.block_till_done()
+
+        state = self.hass.states.get('light.test')
+        self.assertEqual(STATE_ON, state.state)
+        self.assertIsNone(state.attributes.get('rgb_color'))
+        self.assertIsNone(state.attributes.get('brightness'))
+
     def test_controlling_state_via_topic(self):
         self.assertTrue(light.setup(self.hass, {
             'light': {
@@ -82,12 +104,16 @@ class TestLightMQTT(unittest.TestCase):
 
         state = self.hass.states.get('light.test')
         self.assertEqual(STATE_OFF, state.state)
+        self.assertIsNone(state.attributes.get('rgb_color'))
+        self.assertIsNone(state.attributes.get('brightness'))
 
         fire_mqtt_message(self.hass, 'test_light_rgb/status', 'on')
         self.hass.pool.block_till_done()
 
         state = self.hass.states.get('light.test')
         self.assertEqual(STATE_ON, state.state)
+        self.assertEqual([255, 255, 255], state.attributes.get('rgb_color'))
+        self.assertEqual(255, state.attributes.get('brightness'))
 
         fire_mqtt_message(self.hass, 'test_light_rgb/status', 'off')
         self.hass.pool.block_till_done()