Repository: theolind/pymysensors

Issue:
Setting node state on message send causes issues in case of message drop
Hi!

I am experiencing some issues with Home Assistant integration which uses this library.

It seems that the library stores the state of the node internally. 
And this state is set not only on message receive from the node, but on the command send to the node.

It also looks like the state is checked before sending new commands.

For me it causes issues with the `S_COVER` device class in case the `V_UP` or `V_DOWN` command never reaches the node.
There is no state report from the node itself in this case.

So Home Assistant thinks that the cover is e.g. `DOWN` and allows you to ask it to go up again, library thinks that it is `OPENING` and ignores subsequent `UP` commands, which renders the node unresponsive.

Detailed description of the case: home-assistant/core#51265.


Proposals:
- can the message delivery status be checked before setting the internal state?
- set state on message receive from the node only (or make it switchable - Home Assistant integration has the flag `optimistic` for this).
- is it really necessary to block commands based on the local state value?


Discussion:
gorbunovav: @MartinHjelmare take a look, please
MartinHjelmare: I'll have a look later in the week. Thanks!

Patch:
diff --git a/mysensors/__init__.py b/mysensors/__init__.py
index 2b517b0..4a672a4 100644
--- a/mysensors/__init__.py
+++ b/mysensors/__init__.py
@@ -3,7 +3,6 @@ import logging
 
 # pylint: disable=no-name-in-module, import-error
 from distutils.version import LooseVersion as parse_ver
-from functools import partial
 from pathlib import Path
 
 import voluptuous as vol
@@ -64,9 +63,9 @@ class Gateway:
         msg.gateway = self
         message_type = self.const.MessageType(msg.type)
         handler = message_type.get_handler(self.handlers)
-        msg = handler(msg)
-        msg = self._route_message(msg)
-        return msg.encode() if msg else None
+        reply = handler(msg)
+        reply = self._route_message(reply)
+        return reply.encode() if reply else None
 
     def alert(self, msg):
         """Tell anyone who wants to know that a sensor was updated."""
@@ -97,6 +96,41 @@ class Gateway:
             self.sensors[sensorid] = Sensor(sensorid)
         return sensorid if sensorid in self.sensors else None
 
+    def create_message_to_set_sensor_value(
+        self, sensor, child_id, value_type, value, **kwargs
+    ):
+        """Create a message to set specified sensor child value."""
+        msg_type = kwargs.get("msg_type", self.const.MessageType.set)
+        ack = kwargs.get("ack", 0)
+
+        try:
+            value_type = int(value_type)
+        except (ValueError, TypeError) as exc:
+            raise ValueError(f"Invalid value_type provided: {value_type}") from exc
+
+        value = str(value)
+
+        msg = Message(
+            node_id=sensor.sensor_id,
+            child_id=child_id,
+            type=msg_type,
+            ack=ack,
+            sub_type=value_type,
+            payload=value,
+        )
+
+        msg_string = msg.encode()
+
+        if msg_string is None:
+            raise ValueError(
+                f"Unable to encode message: node {sensor.sensor_id}, child {child_id}, "
+                "type {msg_type}, ack {ack}, sub_type {value_type}, payload {value}"
+            )
+
+        msg.validate(self.protocol_version)
+
+        return msg
+
     def is_sensor(self, sensorid, child_id=None):
         """Return True if a sensor and its child exist."""
         ret = sensorid in self.sensors
@@ -124,13 +158,16 @@ class Gateway:
             or msg.type == self.const.MessageType.presentation
         ):
             return None
+
         if (
             msg.node_id not in self.sensors
             or msg.type == self.const.MessageType.stream
-            or not self.sensors[msg.node_id].new_state
+            or not self.sensors[msg.node_id].is_smart_sleep_node
         ):
             return msg
+
         self.sensors[msg.node_id].queue.append(msg.encode())
+
         return None
 
     def set_child_value(self, sensor_id, child_id, value_type, value, **kwargs):
@@ -146,20 +183,18 @@ class Gateway:
         """
         if not self.is_sensor(sensor_id, child_id):
             return
-        if self.sensors[sensor_id].new_state:
-            self.sensors[sensor_id].set_child_value(
-                child_id, value_type, value, children=self.sensors[sensor_id].new_state
-            )
-        else:
-            self.tasks.add_job(
-                partial(
-                    self.sensors[sensor_id].set_child_value,
-                    child_id,
-                    value_type,
-                    value,
-                    **kwargs,
-                )
-            )
+
+        sensor = self.sensors[sensor_id]
+
+        if sensor.is_smart_sleep_node:
+            sensor.set_child_desired_state(child_id, value_type, value)
+            return
+
+        msg_to_send = self.create_message_to_set_sensor_value(
+            sensor, child_id, value_type, value, **kwargs
+        )
+
+        self.tasks.add_job(msg_to_send.encode)
 
     def send(self, message):
         """Write a message to the arduino gateway."""
diff --git a/mysensors/handler.py b/mysensors/handler.py
index ad93407..c53c2b5 100644
--- a/mysensors/handler.py
+++ b/mysensors/handler.py
@@ -4,7 +4,6 @@ import logging
 import time
 
 from .const import SYSTEM_CHILD_ID
-from .sensor import ChildSensor
 from .util import Registry
 
 _LOGGER = logging.getLogger(__name__)
@@ -14,22 +13,33 @@ HANDLERS = Registry()
 
 def handle_smartsleep(msg):
     """Process a message before going back to smartsleep."""
-    while msg.gateway.sensors[msg.node_id].queue:
-        msg.gateway.tasks.add_job(str, msg.gateway.sensors[msg.node_id].queue.popleft())
-    for child in msg.gateway.sensors[msg.node_id].children.values():
-        new_child = msg.gateway.sensors[msg.node_id].new_state.get(
-            child.id, ChildSensor(child.id, child.type, child.description)
-        )
-        msg.gateway.sensors[msg.node_id].new_state[child.id] = new_child
-        for value_type, value in child.values.items():
-            new_value = new_child.values.get(value_type)
-            if new_value is not None and new_value != value:
-                msg.gateway.tasks.add_job(
-                    msg.gateway.sensors[msg.node_id].set_child_value,
-                    child.id,
-                    value_type,
-                    new_value,
-                )
+    sensor = msg.gateway.sensors[msg.node_id]
+
+    sensor.init_smart_sleep_mode()
+
+    while sensor.queue:
+        job = sensor.queue.popleft()
+        msg.gateway.tasks.add_job(str, job)
+
+    for child in sensor.children.values():
+        new_child_state = sensor.new_state.get(child.id)
+
+        if not new_child_state:
+            continue
+
+        for value_type, _ in child.values.items():
+            new_value = new_child_state.values.get(value_type)
+            if new_value is None:
+                continue
+
+            msg_to_send = msg.gateway.create_message_to_set_sensor_value(
+                sensor, child.id, value_type, new_value
+            )
+
+            if msg_to_send is None:
+                continue
+
+            msg.gateway.tasks.add_job(msg_to_send.encode)
 
 
 @HANDLERS.register("presentation")
@@ -66,19 +76,19 @@ def handle_set(msg):
     """Process a set message."""
     if not msg.gateway.is_sensor(msg.node_id, msg.child_id):
         return None
-    msg.gateway.sensors[msg.node_id].set_child_value(
-        msg.child_id, msg.sub_type, msg.payload
+
+    sensor = msg.gateway.sensors[msg.node_id]
+
+    sensor.update_child_value(
+        msg.child_id,
+        msg.sub_type,
+        msg.payload,
     )
-    if msg.gateway.sensors[msg.node_id].new_state:
-        msg.gateway.sensors[msg.node_id].set_child_value(
-            msg.child_id,
-            msg.sub_type,
-            msg.payload,
-            children=msg.gateway.sensors[msg.node_id].new_state,
-        )
+
     msg.gateway.alert(msg)
+
     # Check if reboot is true
-    if msg.gateway.sensors[msg.node_id].reboot:
+    if sensor.reboot:
         return msg.copy(
             child_id=SYSTEM_CHILD_ID,
             type=msg.gateway.const.MessageType.internal,
@@ -98,12 +108,15 @@ def handle_req(msg):
     """
     if not msg.gateway.is_sensor(msg.node_id, msg.child_id):
         return None
-    value = (
-        msg.gateway.sensors[msg.node_id].children[msg.child_id].values.get(msg.sub_type)
-    )
-    if value is not None:
-        return msg.copy(type=msg.gateway.const.MessageType.set, payload=value)
-    return None
+
+    sensor = msg.gateway.sensors[msg.node_id]
+
+    value = sensor.get_desired_value(msg.child_id, msg.sub_type)
+
+    if value is None:
+        return None
+
+    return msg.copy(type=msg.gateway.const.MessageType.set, payload=value)
 
 
 @HANDLERS.register("internal")
diff --git a/mysensors/sensor.py b/mysensors/sensor.py
index 8dbdc17..400267d 100644
--- a/mysensors/sensor.py
+++ b/mysensors/sensor.py
@@ -3,7 +3,6 @@ import logging
 from collections import deque
 
 import voluptuous as vol
-from voluptuous.humanize import humanize_error
 
 from .const import get_const
 from .message import Message
@@ -80,6 +79,11 @@ class Sensor:
         """Set valid heartbeat value."""
         self._heartbeat = is_heartbeat(value)
 
+    @property
+    def is_smart_sleep_node(self):
+        """Return True if the node uses smart sleep mode."""
+        return bool(self.new_state)
+
     @property
     def protocol_version(self):
         """Return protocol version."""
@@ -103,46 +107,92 @@ class Sensor:
         self.children[child_id] = ChildSensor(child_id, child_type, description)
         return child_id
 
-    def set_child_value(self, child_id, value_type, value, **kwargs):
-        """Set a child sensor's value."""
-        children = kwargs.get("children", self.children)
-        if not isinstance(children, dict) or child_id not in children:
+    def get_desired_value(self, child_id, value_type):
+        """Return sensor state value taking into account node type."""
+        if child_id not in self.children:
             return None
-        msg_type = kwargs.get("msg_type", 1)
-        ack = kwargs.get("ack", 0)
-        msg = Message().modify(
+
+        value = None
+
+        if self.is_smart_sleep_node:
+            child = self.new_state[child_id]
+            value = child.values.get(value_type) if child else None
+
+        if value:
+            return value
+
+        child = self.children[child_id]
+
+        return child.values.get(value_type)
+
+    def init_smart_sleep_mode(self):
+        """Init desired state dict for all known children."""
+        for child in self.children.values():
+            if child.id in self.new_state:
+                continue
+
+            self.new_state[child.id] = ChildSensor(
+                child.id, child.type, child.description
+            )
+
+    def set_child_desired_state(self, child_id, value_type, value):
+        """Set a desired child sensor's value for smart sleep nodes."""
+        if child_id not in self.new_state:
+            raise ValueError(
+                f"Child with id {child_id} not found for sensor {self.sensor_id}"
+            )
+
+        self.validate_child_state(child_id, value_type, value)
+
+        child = self.new_state[child_id]
+        child.values[value_type] = value
+
+    def update_child_value(self, child_id, value_type, value):
+        """Update a child sensor's local state."""
+        if child_id not in self.children:
+            return
+
+        child = self.children[child_id]
+        child.values[value_type] = value
+
+        if child_id not in self.new_state:
+            return
+
+        # New state received from the node -
+        # we can clear the desired state value to indicate that no changes are required
+        new_state_child = self.new_state[child_id]
+
+        new_state_child.values[value_type] = None
+
+    def validate_child_state(self, child_id, value_type, value):
+        """Check if we will be able to generate a set message from these values."""
+        const = get_const(self.protocol_version)
+        msg_type = const.MessageType.set
+
+        try:
+            value_type = int(value_type)
+        except (ValueError, TypeError) as exc:
+            raise ValueError(f"Invalid value_type provided: {value_type}") from exc
+
+        value = str(value)
+
+        msg = Message(
             node_id=self.sensor_id,
             child_id=child_id,
             type=msg_type,
-            ack=ack,
             sub_type=value_type,
             payload=value,
         )
+
         msg_string = msg.encode()
+
         if msg_string is None:
-            _LOGGER.error(
-                "Not a valid message: node %s, child %s, type %s, ack %s, "
-                "sub_type %s, payload %s",
-                self.sensor_id,
-                child_id,
-                msg_type,
-                ack,
-                value_type,
-                value,
+            raise ValueError(
+                f"Unable to encode message: node {self.sensor_id}, child {child_id}, "
+                "type {msg_type}, sub_type {value_type}, payload {value}"
             )
-            return None
-        msg = Message(msg_string)
-        try:
-            msg.validate(self.protocol_version)
-        except AttributeError as exc:
-            _LOGGER.error("Invalid %s: %s", msg, exc)
-            return None
-        except vol.Invalid as exc:
-            _LOGGER.error("Invalid %s: %s", msg, humanize_error(msg.__dict__, exc))
-            return None
-        child = children[msg.child_id]
-        child.values[msg.sub_type] = msg.payload
-        return msg_string
+
+        msg.validate(self.protocol_version)
 
 
 class ChildSensor:
diff --git a/requirements_lint.txt b/requirements_lint.txt
index bd7059d..ff0f187 100644
--- a/requirements_lint.txt
+++ b/requirements_lint.txt
@@ -1,4 +1,4 @@
-black==21.5b1
+black==21.6b0
 flake8==3.9.2
 pydocstyle==6.1.1
-pylint==2.8.2
+pylint==2.8.3
diff --git a/requirements_pypi.txt b/requirements_pypi.txt
index c484618..bbccbe0 100644
--- a/requirements_pypi.txt
+++ b/requirements_pypi.txt
@@ -1,3 +1,3 @@
-setuptools==57.0.0
+setuptools==57.1.0
 twine==3.4.1
 wheel==0.36.2


Test patch:
diff --git a/requirements_test.txt b/requirements_test.txt
index d96cd44..cccec11 100644
--- a/requirements_test.txt
+++ b/requirements_test.txt
@@ -1,4 +1,4 @@
 pytest==6.2.4
-pytest-cov==2.12.0
+pytest-cov==2.12.1
 pytest-sugar==0.9.4
 pytest-timeout==1.4.2
diff --git a/tests/test_mysensors.py b/tests/test_mysensors.py
index 2146142..69581f2 100644
--- a/tests/test_mysensors.py
+++ b/tests/test_mysensors.py
@@ -333,7 +333,7 @@ def test_req(gateway, add_sensor):
     """Test req message in case where value exists."""
     sensor = add_sensor(1)
     sensor.add_child_sensor(1, gateway.const.Presentation.S_POWER)
-    sensor.set_child_value(1, gateway.const.SetReq.V_WATT, 42)
+    sensor.update_child_value(1, gateway.const.SetReq.V_WATT, 42)
     ret = gateway.logic("1;1;2;0;17;\n")
     assert ret == "1;1;1;0;17;42\n"
 
@@ -342,7 +342,7 @@ def test_req_zerovalue(gateway, add_sensor):
     """Test req message in case where value exists but is zero."""
     sensor = add_sensor(1)
     sensor.add_child_sensor(1, gateway.const.Presentation.S_POWER)
-    sensor.set_child_value(1, gateway.const.SetReq.V_WATT, 0)
+    sensor.update_child_value(1, gateway.const.SetReq.V_WATT, 0)
     ret = gateway.logic("1;1;2;0;17;\n")
     assert ret == "1;1;1;0;17;0\n"
 
@@ -460,33 +460,25 @@ def test_non_presented_child(protocol_version, return_value):
     assert 1 not in sensor.children
     ret = gateway.tasks.run_job()
     assert ret == return_value
-
-
-def test_set_child_no_children(gateway, add_sensor):
-    """Test Gateway method set_child_value without child in children."""
-    sensor = add_sensor(1)
-    sensor.add_child_sensor(0, gateway.const.Presentation.S_LIGHT)
-    gateway.set_child_value(1, 0, gateway.const.SetReq.V_LIGHT, 1, children={})
+    gateway.set_child_value(1, 0, gateway.const.SetReq.V_LIGHT, 1)
     ret = gateway.tasks.run_job()
-    assert ret is None
+    assert ret == return_value
 
 
 def test_set_child_value_bad_type(gateway, add_sensor):
     """Test Gateway method set_child_value with bad type."""
     sensor = add_sensor(1)
     sensor.add_child_sensor(0, gateway.const.Presentation.S_LIGHT)
-    gateway.set_child_value(1, 0, gateway.const.SetReq.V_LIGHT, 1, msg_type="one")
-    ret = gateway.tasks.run_job()
-    assert ret is None
+    with pytest.raises(ValueError):
+        gateway.set_child_value(1, 0, gateway.const.SetReq.V_LIGHT, 1, msg_type="one")
 
 
 def test_set_child_value_bad_ack(gateway, add_sensor):
     """Test Gateway method set_child_value with bad ack."""
     sensor = add_sensor(1)
     sensor.add_child_sensor(0, gateway.const.Presentation.S_LIGHT)
-    gateway.set_child_value(1, 0, gateway.const.SetReq.V_LIGHT, 1, ack="one")
-    ret = gateway.tasks.run_job()
-    assert ret is None
+    with pytest.raises(ValueError):
+        gateway.set_child_value(1, 0, gateway.const.SetReq.V_LIGHT, 1, ack="one")
 
 
 def test_set_child_value_value_type(gateway, add_sensor):
@@ -565,6 +557,31 @@ def test_set_rgbw(protocol_version):
     assert sensor.children[0].values[gateway.const.SetReq.V_RGBW] == "ffffffff"
 
 
+@pytest.mark.parametrize(
+    "protocol_version, wake_msg",
+    [
+        ("2.0", "1;255;3;0;22;123456\n"),
+        ("2.1", "1;255;3;0;22;123456\n"),
+        ("2.2", "1;255;3;0;32;500\n"),
+    ],
+)
+def test_node_mark_as_smartsleep(protocol_version, wake_msg):
+    """Test that node is correctly marked as smartsleep after the heartbeat message."""
+    gateway = get_gateway(protocol_version=protocol_version)
+    sensor = get_sensor(1, gateway)
+    sensor.add_child_sensor(0, gateway.const.Presentation.S_LIGHT_LEVEL)
+    gateway.logic("1;0;1;0;23;43\n")
+    assert not sensor.is_smart_sleep_node
+
+    # heartbeat
+    gateway.logic(wake_msg)
+
+    assert sensor.is_smart_sleep_node
+
+    # check that the new_state dict was initialized
+    assert 0 in sensor.new_state
+
+
 @pytest.mark.parametrize(
     "protocol_version, wake_msg",
     [
@@ -594,9 +611,9 @@ def test_smartsleep(protocol_version, wake_msg):
     # heartbeat comes in
     gateway.logic(wake_msg)
     ret = gateway.tasks.run_job()
-    # instance responds with new values
+    # instance sends new values to the node
     assert ret == "1;0;1;0;23;57\n"
-    # request from node
+    # node requests value from the instance
     gateway.logic("1;0;2;0;23;\n")
     ret = gateway.tasks.run_job()
     # no heartbeat
@@ -604,13 +621,31 @@ def test_smartsleep(protocol_version, wake_msg):
     # heartbeat
     gateway.logic(wake_msg)
     ret = gateway.tasks.run_job()
-    # instance responds to request with current value
+    # instance responds to request with the desired(new) value
+    # despite the lack of new values confirmation from the node
+    # reply to the incoming message is handled first
+    assert ret == "1;0;1;0;23;57\n"
+    ret = gateway.tasks.run_job()
+    # then instance sends new values to the node again
+    # because there was no confirmation from the node
     assert ret == "1;0;1;0;23;57\n"
+    # node confirms new values
+    gateway.logic("1;0;1;0;23;57\n")
+    ret = gateway.tasks.run_job()
+    # no actions before heartbeat
+    assert ret is None
     # heartbeat
     gateway.logic(wake_msg)
     ret = gateway.tasks.run_job()
-    # nothing has changed
+    # no changes required since states match
     assert ret is None
+    # node requests value from the instance
+    gateway.logic("1;0;2;0;23;\n")
+    # heartbeat
+    gateway.logic(wake_msg)
+    ret = gateway.tasks.run_job()
+    # instance responds to request with current values
+    assert ret == "1;0;1;0;23;57\n"
 
 
 @pytest.mark.parametrize(
@@ -643,12 +678,26 @@ def test_set_with_new_state(protocol_version, wake_msg):
     sensor = get_sensor(1, gateway)
     sensor.add_child_sensor(0, gateway.const.Presentation.S_LIGHT_LEVEL)
     gateway.logic("1;0;1;0;23;43\n")
+
+    # Mark sensor as smart sleep
     gateway.logic(wake_msg)
+
+    value_type = gateway.const.SetReq.V_LIGHT_LEVEL
+
+    # Set desired value
+    gateway.set_child_value(sensor.sensor_id, 0, value_type, "65")
+
+    # Check that we have old value in the state
+    # and desired one in the new_state
+    assert sensor.children[0].values[value_type] == "43"
+    assert sensor.new_state[0].values[value_type] == "65"
+
+    # Receive state update from the node
     gateway.logic("1;0;1;0;23;57\n")
-    assert (
-        sensor.children[0].values[gateway.const.SetReq.V_LIGHT_LEVEL]
-        == sensor.new_state[0].values[gateway.const.SetReq.V_LIGHT_LEVEL]
-    )
+
+    # Check that the desired state value is cleared
+    assert sensor.children[0].values[value_type] == "57"
+    assert sensor.new_state[0].values[value_type] is None
 
 
 @pytest.mark.parametrize("protocol_version", ["2.0", "2.1", "2.2"])
diff --git a/tests/test_sensor.py b/tests/test_sensor.py
new file mode 100644
index 0000000..b353a54
--- /dev/null
+++ b/tests/test_sensor.py
@@ -0,0 +1,178 @@
+"""Test task module."""
+import pytest
+import voluptuous
+
+from mysensors.sensor import ChildSensor, Sensor
+from mysensors.const import get_const
+
+
+def test_is_smart_sleep_node():
+    """Test schedule persistence on threading gateway."""
+    const = get_const("1.4")
+    sensor_id = 1
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(0, const.Presentation.S_LIGHT_LEVEL)
+
+    assert not sensor.is_smart_sleep_node
+
+    sensor.new_state[sensor_id] = {}
+
+    assert sensor.is_smart_sleep_node
+
+
+def test_init_smart_sleep_mode():
+    """Test that the new state array is populated correctly."""
+    const = get_const("1.4")
+    sensor_id = 1
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(0, const.Presentation.S_LIGHT_LEVEL)
+    sensor.add_child_sensor(1, const.Presentation.S_LIGHT_LEVEL)
+
+    assert not sensor.new_state
+
+    sensor.init_smart_sleep_mode()
+
+    assert 0 in sensor.new_state
+    assert isinstance(sensor.new_state[0], ChildSensor)
+    assert 1 in sensor.new_state
+    assert isinstance(sensor.new_state[1], ChildSensor)
+
+
+def test_get_desired_value():
+    """Test that sensor returns correct desired value in different states."""
+    const = get_const("1.4")
+    sensor_id = 1
+    child_id = 0
+    value_type = const.SetReq.V_LIGHT_LEVEL
+    wrong_child_id = 100
+    wrong_value_type = 9999
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(child_id, const.Presentation.S_LIGHT_LEVEL)
+
+    assert sensor.get_desired_value(child_id, value_type) is None
+
+    sensor.update_child_value(child_id, value_type, "50")
+    assert sensor.get_desired_value(child_id, value_type) == "50"
+
+    sensor.init_smart_sleep_mode()
+
+    sensor.set_child_desired_state(child_id, value_type, "90")
+    assert sensor.get_desired_value(child_id, value_type) == "90"
+
+    sensor.update_child_value(child_id, value_type, "70")
+    assert sensor.get_desired_value(child_id, value_type) == "70"
+
+    assert sensor.get_desired_value(wrong_child_id, value_type) is None
+    assert sensor.get_desired_value(child_id, wrong_value_type) is None
+
+
+def test_set_child_desired_state():
+    """Test that we area able to set the desired state."""
+    const = get_const("1.4")
+    sensor_id = 1
+    child_id = 0
+    value_type = const.SetReq.V_LIGHT_LEVEL
+    wrong_child_id = 100
+    wrong_value_type = 99999
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(child_id, const.Presentation.S_LIGHT_LEVEL)
+    sensor.init_smart_sleep_mode()
+
+    # able to set
+    sensor.set_child_desired_state(child_id, value_type, "50")
+    assert child_id in sensor.new_state
+    assert value_type in sensor.new_state[child_id].values
+    assert sensor.new_state[child_id].values[value_type] == "50"
+
+    # able to update
+    sensor.set_child_desired_state(child_id, value_type, "90")
+    assert sensor.new_state[child_id].values[value_type] == "90"
+
+    # does not set unknown child
+    with pytest.raises(ValueError):
+        sensor.set_child_desired_state(wrong_child_id, value_type, "50")
+
+    # does not set wrong value type
+    with pytest.raises(voluptuous.error.MultipleInvalid):
+        sensor.set_child_desired_state(child_id, wrong_value_type, "50")
+
+    # does not set wrong value type
+    with pytest.raises(voluptuous.error.MultipleInvalid):
+        sensor.set_child_desired_state(child_id, value_type, "bad value")
+
+
+def test_update_child_value():
+    """Test that we can update child state."""
+    const = get_const("1.4")
+    sensor_id = 1
+    child_id = 0
+    wrong_child_id = 100
+    value_type = const.SetReq.V_LIGHT_LEVEL
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(child_id, const.Presentation.S_LIGHT_LEVEL)
+
+    assert value_type not in sensor.children[child_id].values
+
+    # able to set
+    sensor.update_child_value(child_id, value_type, "50")
+    assert sensor.children[child_id].values[value_type] == "50"
+
+    # able to update
+    sensor.update_child_value(child_id, value_type, "90")
+    assert sensor.children[child_id].values[value_type] == "90"
+
+    # does not set unknown child
+    sensor.update_child_value(wrong_child_id, value_type, "50")
+    assert wrong_child_id not in sensor.children
+
+
+def test_update_child_value_resets_new_state():
+    """Test that update of child state resets the new state."""
+    const = get_const("1.4")
+    sensor_id = 1
+    child_id = 0
+    value_type = const.SetReq.V_LIGHT_LEVEL
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(child_id, const.Presentation.S_LIGHT_LEVEL)
+    sensor.init_smart_sleep_mode()
+    sensor.set_child_desired_state(child_id, value_type, "50")
+
+    assert sensor.new_state[child_id].values[value_type] == "50"
+
+    sensor.update_child_value(child_id, value_type, "60")
+
+    assert sensor.new_state[child_id].values[value_type] is None
+
+
+def test_validate_child_state():
+    """Test that update of child state resets the new state."""
+    const = get_const("1.4")
+    sensor_id = 1
+    child_id = 0
+    value_type = const.SetReq.V_LIGHT_LEVEL
+
+    sensor = Sensor(sensor_id)
+    sensor.add_child_sensor(child_id, const.Presentation.S_LIGHT_LEVEL)
+
+    with pytest.raises(voluptuous.error.MultipleInvalid):
+        sensor.validate_child_state(child_id, value_type, "bad value")
+
+    with pytest.raises(voluptuous.error.MultipleInvalid):
+        sensor.validate_child_state(300, value_type, "50")
+
+    with pytest.raises(voluptuous.error.MultipleInvalid):
+        sensor.validate_child_state(child_id, 9999, "50")
+
+    with pytest.raises(ValueError):
+        sensor.validate_child_state(child_id, "bad value type", "50")
+
+    with pytest.raises(ValueError):
+        sensor.validate_child_state(child_id, None, "50")
+
+    sensor.validate_child_state(child_id, value_type, "50")