Group: fix thread safety

homeassistant/components/group.py

@@ -1,11 +1,11 @@
 """
-homeassistant.components.group
+Provides functionality to group entities.
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Provides functionality to group devices that can be turned on or off.
 
 For more details about this component, please refer to the documentation at
 https://home-assistant.io/components/group/
 """
+import threading
+
 import homeassistant.core as ha
 from homeassistant.const import (
     ATTR_ENTITY_ID, CONF_ICON, CONF_NAME, STATE_CLOSED, STATE_HOME,
@@ -32,7 +32,7 @@ _GROUP_TYPES = [(STATE_ON, STATE_OFF), (STATE_HOME, STATE_NOT_HOME),
 
 
 def _get_group_on_off(state):
-    """ Determine the group on/off states based on a state. """
+    """Determine the group on/off states based on a state."""
     for states in _GROUP_TYPES:
         if state in states:
             return states
@@ -41,7 +41,7 @@ def _get_group_on_off(state):
 
 
 def is_on(hass, entity_id):
-    """ Returns if the group state is in its ON-state. """
+    """Test if the group state is in its ON-state."""
     state = hass.states.get(entity_id)
 
     if state:
@@ -54,8 +54,7 @@ def is_on(hass, entity_id):
 
 
 def expand_entity_ids(hass, entity_ids):
-    """ Returns the given list of entity ids and expands group ids into
+    """Return entity_ids with group entity ids replaced by their members."""
-        the entity ids it represents if found. """
     found_ids = []
 
     for entity_id in entity_ids:
@@ -86,7 +85,7 @@ def expand_entity_ids(hass, entity_ids):
 
 
 def get_entity_ids(hass, entity_id, domain_filter=None):
-    """ Get the entity ids that make up this group. """
+    """Get members of this group."""
     entity_id = entity_id.lower()
 
     try:
@@ -107,7 +106,7 @@ def get_entity_ids(hass, entity_id, domain_filter=None):
 
 
 def setup(hass, config):
-    """ Sets up all groups found definded in the configuration. """
+    """Set up all groups found definded in the configuration."""
     for object_id, conf in config.get(DOMAIN, {}).items():
         if not isinstance(conf, dict):
             conf = {CONF_ENTITIES: conf}
@@ -127,12 +126,13 @@ def setup(hass, config):
 
 
 class Group(Entity):
-    """ Tracks a group of entity ids. """
+    """Track a group of entity ids."""
 
     # pylint: disable=too-many-instance-attributes, too-many-arguments
 
     def __init__(self, hass, name, entity_ids=None, user_defined=True,
                  icon=None, view=False, object_id=None):
+        """Initialize a group."""
         self.hass = hass
         self._name = name
         self._state = STATE_UNKNOWN
@@ -146,6 +146,7 @@ class Group(Entity):
         self.group_on = None
         self.group_off = None
         self._assumed_state = False
+        self._lock = threading.Lock()
 
         if entity_ids is not None:
             self.update_tracked_entity_ids(entity_ids)
@@ -154,26 +155,35 @@ class Group(Entity):
 
     @property
     def should_poll(self):
+        """No need to poll because groups will update themselves."""
         return False
 
     @property
     def name(self):
+        """Name of the group."""
         return self._name
 
     @property
     def state(self):
+        """State of the group."""
         return self._state
 
     @property
     def icon(self):
+        """Icon of the group."""
         return self._icon
 
     @property
     def hidden(self):
+        """If group should be hidden or not.
+
+        true if group is a view or not user defined.
+        """
         return not self._user_defined or self._view
 
     @property
     def state_attributes(self):
+        """State attributes for the group."""
         data = {
             ATTR_ENTITY_ID: self.tracking,
             ATTR_ORDER: self._order,
@@ -186,11 +196,11 @@ class Group(Entity):
 
     @property
     def assumed_state(self):
-        """Return True if unable to access real state of entity."""
+        """Test if any member has an assumed state."""
         return self._assumed_state
 
     def update_tracked_entity_ids(self, entity_ids):
-        """ Update the tracked entity IDs. """
+        """Update the member entity IDs."""
         self.stop()
         self.tracking = tuple(ent_id.lower() for ent_id in entity_ids)
         self.group_on, self.group_off = None, None
@@ -200,24 +210,24 @@ class Group(Entity):
         self.start()
 
     def start(self):
-        """ Starts the tracking. """
+        """Start tracking members."""
         track_state_change(
             self.hass, self.tracking, self._state_changed_listener)
 
     def stop(self):
-        """ Unregisters the group from Home Assistant. """
+        """Unregisters the group from Home Assistant."""
         self.hass.states.remove(self.entity_id)
 
         self.hass.bus.remove_listener(
             ha.EVENT_STATE_CHANGED, self._state_changed_listener)
 
     def update(self):
-        """ Query all the tracked states and determine current group state. """
+        """Query all members and determine current group state."""
         self._state = STATE_UNKNOWN
         self._update_group_state()
 
     def _state_changed_listener(self, entity_id, old_state, new_state):
-        """ Listener to receive state changes of tracked entities. """
+        """Respond to a member state changing."""
         self._update_group_state(new_state)
         self.update_ha_state()
 
@@ -242,49 +252,53 @@ class Group(Entity):
         """
         # pylint: disable=too-many-branches
         # To store current states of group entities. Might not be needed.
-        states = None
+        with self._lock:
-        gr_state, gr_on, gr_off = self._state, self.group_on, self.group_off
+            states = None
+            gr_state = self._state
+            gr_on = self.group_on
+            gr_off = self.group_off
 
-        # We have not determined type of group yet
+            # We have not determined type of group yet
-        if gr_on is None:
+            if gr_on is None:
-            if tr_state is None:
+                if tr_state is None:
-                states = self._tracking_states
+                    states = self._tracking_states
 
-                for state in states:
+                    for state in states:
-                    gr_on, gr_off = \
+                        gr_on, gr_off = \
-                        _get_group_on_off(state.state)
+                            _get_group_on_off(state.state)
-                    if gr_on is not None:
+                        if gr_on is not None:
-                        break
+                            break
-            else:
+                else:
-                gr_on, gr_off = _get_group_on_off(tr_state.state)
+                    gr_on, gr_off = _get_group_on_off(tr_state.state)
 
-            if gr_on is not None:
+                if gr_on is not None:
-                self.group_on, self.group_off = gr_on, gr_off
+                    self.group_on, self.group_off = gr_on, gr_off
 
-        # We cannot determine state of the group
+            # We cannot determine state of the group
-        if gr_on is None:
+            if gr_on is None:
-            return
+                return
 
-        if tr_state is None or (gr_state == gr_on and
+            if tr_state is None or (gr_state == gr_on and
-                                tr_state.state == gr_off):
+                                    tr_state.state == gr_off):
-            if states is None:
+                if states is None:
-                states = self._tracking_states
+                    states = self._tracking_states
 
-            if any(state.state == gr_on for state in states):
+                if any(state.state == gr_on for state in states):
-                self._state = gr_on
+                    self._state = gr_on
-            else:
+                else:
-                self._state = gr_off
+                    self._state = gr_off
 
-        elif tr_state.state in (gr_on, gr_off):
+            elif tr_state.state in (gr_on, gr_off):
-            self._state = tr_state.state
+                self._state = tr_state.state
 
-        if tr_state is None or self._assumed_state and \
+            if tr_state is None or self._assumed_state and \
-           not tr_state.attributes.get(ATTR_ASSUMED_STATE):
+               not tr_state.attributes.get(ATTR_ASSUMED_STATE):
-            if states is None:
+                if states is None:
-                states = self._tracking_states
+                    states = self._tracking_states
 
-            self._assumed_state = any(state.attributes.get(ATTR_ASSUMED_STATE)
+                self._assumed_state = any(
-                                      for state in states)
+                    state.attributes.get(ATTR_ASSUMED_STATE) for state
+                    in states)
 
-        elif tr_state.attributes.get(ATTR_ASSUMED_STATE):
+            elif tr_state.attributes.get(ATTR_ASSUMED_STATE):
-            self._assumed_state = True
+                self._assumed_state = True