From 1f1115f631cb3fae99f1c68a3812e2ac79814e92 Mon Sep 17 00:00:00 2001
From: Matthew Treinish <mtreinish@kortar.org>
Date: Fri, 8 Dec 2017 09:18:52 -0800
Subject: [PATCH] Serialize mochad requests (#11029)

All mochad devices are sharing a single socket interface. When multiple
threads are issuing requests to the mochad daemon at the same time the
write read cycle might get crossed between the threads. This is normally
not an issue for 1-way X10 devices because as long as the request issued
successfully and data is read over the socket then we know as much as
mochad will tell us (since there is no ACK from the request for most
X10 devices). However, where it does matter is on the device __init__()
because we're relying on the mochad daemon's internal state to take an
educated guess at the device's state to intialize things with. When
there are multiple devices being initialized at the same time the wires
can get crossed between and the wrong device state may be read.

To address this potential issue this commit adds locking using a
semaphore around all pairs of send_cmd() and read_data() (which is what
pymochad.device.Device.get_status() does internally) calls to the mochad
controller to ensure we're only ever dealing with a single request at a
time.

Fixes mtreinish/pymochad#4
---
 homeassistant/components/light/mochad.py  | 13 ++++++++-----
 homeassistant/components/mochad.py        |  3 +++
 homeassistant/components/switch/mochad.py | 13 ++++++++-----
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/homeassistant/components/light/mochad.py b/homeassistant/components/light/mochad.py
index fffaa293188..3d67edaf7cb 100644
--- a/homeassistant/components/light/mochad.py
+++ b/homeassistant/components/light/mochad.py
@@ -62,7 +62,8 @@ class MochadLight(Light):
 
     def _get_device_status(self):
         """Get the status of the light from mochad."""
-        status = self.device.get_status().rstrip()
+        with mochad.REQ_LOCK:
+            status = self.device.get_status().rstrip()
         return status == 'on'
 
     @property
@@ -88,12 +89,14 @@ class MochadLight(Light):
     def turn_on(self, **kwargs):
         """Send the command to turn the light on."""
         self._brightness = kwargs.get(ATTR_BRIGHTNESS, 255)
-        self.device.send_cmd("xdim {}".format(self._brightness))
-        self._controller.read_data()
+        with mochad.REQ_LOCK:
+            self.device.send_cmd("xdim {}".format(self._brightness))
+            self._controller.read_data()
         self._state = True
 
     def turn_off(self, **kwargs):
         """Send the command to turn the light on."""
-        self.device.send_cmd('off')
-        self._controller.read_data()
+        with mochad.REQ_LOCK:
+            self.device.send_cmd('off')
+            self._controller.read_data()
         self._state = False
diff --git a/homeassistant/components/mochad.py b/homeassistant/components/mochad.py
index 165c43f488f..3cc4eda7675 100644
--- a/homeassistant/components/mochad.py
+++ b/homeassistant/components/mochad.py
@@ -5,6 +5,7 @@ For more details about this component, please refer to the documentation at
 https://home-assistant.io/components/mochad/
 """
 import logging
+import threading
 
 import voluptuous as vol
 
@@ -23,6 +24,8 @@ CONF_COMM_TYPE = 'comm_type'
 
 DOMAIN = 'mochad'
 
+REQ_LOCK = threading.Lock()
+
 CONFIG_SCHEMA = vol.Schema({
     DOMAIN: vol.Schema({
         vol.Optional(CONF_HOST, default='localhost'): cv.string,
diff --git a/homeassistant/components/switch/mochad.py b/homeassistant/components/switch/mochad.py
index a67b27a6a91..da8f96dc1f0 100644
--- a/homeassistant/components/switch/mochad.py
+++ b/homeassistant/components/switch/mochad.py
@@ -60,18 +60,21 @@ class MochadSwitch(SwitchDevice):
     def turn_on(self, **kwargs):
         """Turn the switch on."""
         self._state = True
-        self.device.send_cmd('on')
-        self._controller.read_data()
+        with mochad.REQ_LOCK:
+            self.device.send_cmd('on')
+            self._controller.read_data()
 
     def turn_off(self, **kwargs):
         """Turn the switch off."""
         self._state = False
-        self.device.send_cmd('off')
-        self._controller.read_data()
+        with mochad.REQ_LOCK:
+            self.device.send_cmd('off')
+            self._controller.read_data()
 
     def _get_device_status(self):
         """Get the status of the switch from mochad."""
-        status = self.device.get_status().rstrip()
+        with mochad.REQ_LOCK:
+            status = self.device.get_status().rstrip()
         return status == 'on'
 
     @property