Repository: mill1000/midea-msmart (python)

Issue:
Device should remain online if at least one response is received during refresh
When refreshing the device state a number of commands are sent
https://github.com/mill1000/midea-msmart/blob/294e74b412ea86f47999875a222c08aed1120709/msmart/device/AC/device.py#L459-L482

However, each must be responded to otherwise the device can be marked offline,

We should instead consider the device online if any of the commands get a response.

Pull request description:
Keep device online of any command received a response
Update command/response handling to keep device online if at least 1 response is received during a refresh.

As a side effect, only calls to refresh() will update the device online state.

Close #186 

Patch:
diff --git a/msmart/base_device.py b/msmart/base_device.py
index c2795c8..d838a88 100644
--- a/msmart/base_device.py
+++ b/msmart/base_device.py
@@ -25,7 +25,7 @@ class Device():
         self._supported = False
         self._online = False
 
-    async def _send_command(self, command: Frame) -> Optional[list[bytes]]:
+    async def _send_command(self, command: Frame) -> list[bytes]:
         """Send a command to the device and return any responses."""
 
         data = command.tobytes()
@@ -33,24 +33,23 @@ class Device():
                       self.ip, self.port, data.hex())
 
         start = time.time()
-        responses = None
+        responses = []
         try:
             responses = await self._lan.send(data)
         except ProtocolError as e:
             _LOGGER.error("Network error %s:%d: %s", self.ip, self.port, e)
-            return None
+            return []
         except TimeoutError as e:
             _LOGGER.warning("Network timeout %s:%d: %s", self.ip, self.port, e)
         finally:
             response_time = round(time.time() - start, 2)
 
-        if responses is None:
+        if len(responses) == 0:
             _LOGGER.warning("No response from %s:%d in %f seconds.",
                             self.ip, self.port, response_time)
-            return None
-
-        _LOGGER.debug("Response from %s:%d in %f seconds.",
-                      self.ip, self.port, response_time)
+        else:
+            _LOGGER.debug("Response from %s:%d in %f seconds.",
+                          self.ip, self.port, response_time)
 
         return responses
 
diff --git a/msmart/device/AC/device.py b/msmart/device/AC/device.py
index cd870b0..dcfb3fd 100644
--- a/msmart/device/AC/device.py
+++ b/msmart/device/AC/device.py
@@ -407,14 +407,6 @@ class AirConditioner(Device):
 
         responses = await super()._send_command(command)
 
-        # No response from device
-        if responses is None:
-            self._online = False
-            return []
-
-        # Device is online if we received any response
-        self._online = True
-
         valid_responses = []
         for data in responses:
             try:
@@ -424,11 +416,11 @@ class AirConditioner(Device):
                 _LOGGER.error(e)
                 continue
 
-            # Device is supported if we can process a response
-            self._supported = True
-
             valid_responses.append(response)
 
+        # Device is supported if we can process any response
+        self._supported = len(valid_responses) > 0
+
         return valid_responses
 
     async def _send_command_get_response_with_id(self, command, response_id: ResponseId) -> Optional[Response]:
@@ -526,10 +518,19 @@ class AirConditioner(Device):
         if len(self._supported_properties):
             commands.append(GetPropertiesCommand(self._supported_properties))
 
-        # Send commands and process any responses
-        for cmd in commands:
-            for response in await self._send_command_get_responses(cmd):
-                self._update_state(response)
+        # Send all commands and collect responses
+        responses = [
+            resp
+            for cmd in commands
+            for resp in await self._send_command_get_responses(cmd)
+        ]
+
+        # Device is online if any response received
+        self._online = len(responses) > 0
+
+        # Update state from responses
+        for response in responses:
+            self._update_state(response)
 
     async def _apply_properties(self, properties: dict[PropertyId, Union[int, bool]]) -> None:
         """Apply the provided properties to the device."""


Test patch:
diff --git a/msmart/device/AC/test_device.py b/msmart/device/AC/test_device.py
index d854f80..cf08fdc 100644
--- a/msmart/device/AC/test_device.py
+++ b/msmart/device/AC/test_device.py
@@ -1,9 +1,10 @@
 import logging
 import unittest
+from unittest.mock import patch
 
 from .command import (CapabilitiesResponse, EnergyUsageResponse,
-                      HumidityResponse, PropertiesResponse, Response,
-                      StateResponse)
+                      GetStateCommand, HumidityResponse, PropertiesResponse,
+                      Response, StateResponse)
 from .device import AirConditioner as AC
 from .device import PropertyId
 
@@ -554,5 +555,96 @@ class TestSetState(unittest.TestCase):
         self.assertNotIn(PropertyId.BREEZE_CONTROL, device._updated_properties)
 
 
+class TestSendCommandGetResponse(unittest.IsolatedAsyncioTestCase):
+    # pylint: disable=protected-access
+
+    async def test_refresh_no_response(self) -> None:
+        """Test that a refresh() with no response marks a device as offline."""
+
+        # Create a dummy device
+        device = AC(0, 0, 0)
+
+        # Patch _send_command to return no responses
+        with patch("msmart.base_device.Device._send_command", return_value=[]) as patched_method:
+
+            # Force device online
+            device._online = True
+            self.assertEqual(device.online, True)
+
+            # Refresh device
+            await device.refresh()
+
+            # Assert patch method was awaited
+            patched_method.assert_awaited()
+
+            # Assert device is now offline
+            self.assertEqual(device.online, False)
+
+    async def test_refresh_valid_response(self) -> None:
+        """Test that a refresh() with any response marks a device as online."""
+        TEST_RESPONSE = bytes.fromhex(
+            "aa23ac00000000000303c00145660000003c0010045c6b20000000000000000000020d79")
+
+        # Create a dummy device
+        device = AC(0, 0, 0)
+
+        # Patch _send_command to return a valid state response
+        with patch("msmart.base_device.Device._send_command", return_value=[TEST_RESPONSE]) as patched_method:
+
+            # Assert device is offline and unsupported
+            self.assertEqual(device.online, False)
+            self.assertEqual(device.supported, False)
+
+            # Refresh device
+            await device.refresh()
+
+            # Assert patch method was awaited
+            patched_method.assert_awaited()
+
+            # Assert device is now online and supported
+            self.assertEqual(device.online, True)
+            self.assertEqual(device.supported, True)
+
+    async def test_refresh_one_response(self) -> None:
+        """Test that a refresh() with only one response stays online."""
+        TEST_RESPONSE = bytes.fromhex(
+            "aa23ac00000000000303c00145660000003c0010045c6b20000000000000000000020d79")
+
+        # Create a dummy device
+        device = AC(0, 0, 0)
+
+        # Dummy method to only respond to state commands
+        packet_count = 0
+
+        async def _get_responses_sometimes(self, command) -> list[bytes]:
+            nonlocal packet_count
+
+            packet_count += 1
+            if isinstance(command, GetStateCommand):
+                return [TEST_RESPONSE]
+            else:
+                return []
+
+        # Patch _send_command to return test responses
+        with patch("msmart.base_device.Device._send_command", new=_get_responses_sometimes):
+
+            # Force device online
+            device._online = True
+            self.assertEqual(device.online, True)
+
+            # Force additional features so refresh() sends multuple requests are sent
+            device._request_energy_usage = True
+            device._supports_humidity = True
+
+            # Refresh device
+            await device.refresh()
+
+            # Assert expected number of packets was sent
+            self.assertEqual(packet_count, 3)
+
+            # Assert device is still online
+            self.assertEqual(device.online, True)
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/msmart/tests/test_device.py b/msmart/tests/test_device.py
new file mode 100644
index 0000000..1b39d3f
--- /dev/null
+++ b/msmart/tests/test_device.py
@@ -0,0 +1,64 @@
+import logging
+import unittest
+from unittest.mock import patch
+
+from msmart.base_device import Device
+from msmart.const import DeviceType, FrameType
+from msmart.frame import Frame
+from msmart.lan import ProtocolError
+
+
+class TestSendCommand(unittest.IsolatedAsyncioTestCase):
+    # pylint: disable=protected-access
+
+    async def test_timeout(self) -> None:
+        """Test that _send_command with a timeout returns any empty list."""
+
+        # Create a dummy device
+        device = Device(ip=0, port=0, device_id=0,
+                        device_type=DeviceType.AIR_CONDITIONER)
+
+        # Patch send to timeout
+        with patch("msmart.lan.LAN.send", side_effect=TimeoutError) as patched_method:
+            with self.assertLogs("msmart", logging.WARNING) as log:
+                # Send dummy command
+                cmd = Frame(device_type=DeviceType.AIR_CONDITIONER,
+                            frame_type=FrameType.CONTROL)
+                responses = await device._send_command(cmd)
+
+                # Check warning message is generated for timeout
+                self.assertRegex("\n".join(log.output), "Network timeout .*")
+
+            # Assert patched method was awaited
+            patched_method.assert_awaited()
+
+            # Assrt empty list was returned
+            self.assertEqual(responses, [])
+
+    async def test_protocol_error(self) -> None:
+        """Test that _send_command with a protocol error returns any empty list."""
+
+        # Create a dummy device
+        device = Device(ip=0, port=0, device_id=0,
+                        device_type=DeviceType.AIR_CONDITIONER)
+
+        # Patch send to throw protocol error
+        with patch("msmart.lan.LAN.send", side_effect=ProtocolError) as patched_method:
+            with self.assertLogs("msmart", logging.ERROR) as log:
+                # Send dummy command
+                cmd = Frame(device_type=DeviceType.AIR_CONDITIONER,
+                            frame_type=FrameType.CONTROL)
+                responses = await device._send_command(cmd)
+
+                # Check warning message is generated for timeout
+                self.assertRegex("\n".join(log.output), "Network error .*")
+
+            # Assert patched method was awaited
+            patched_method.assert_awaited()
+
+            # Assrt empty list was returned
+            self.assertEqual(responses, [])
+
+
+if __name__ == "__main__":
+    unittest.main()