Repository: jmccrohan/pysolarmanv5

Issue:
`if response_frame == b""` statement in `_send_receive_frame`
This
https://github.com/jmccrohan/pysolarmanv5/blob/a5dea0639a9bb0b7697d01dacaa519ad5f37caa6/pysolarmanv5/pysolarmanv5_async.py#L267
will never happen because we never add empty frame into the queue
https://github.com/jmccrohan/pysolarmanv5/blob/a5dea0639a9bb0b7697d01dacaa519ad5f37caa6/pysolarmanv5/pysolarmanv5_async.py#L221
right?

Discussion:
@githubDante: Currently yes. Sending the empty packet is commented out with fb5169eb but it should be part of the if/else block below it in case that the auto-reconnect is not enabled. Thank you for pointing it out. @davidrapan: Oh yeah I forgot to mention
https://github.com/jmccrohan/pysolarmanv5/blob/a5dea0639a9bb0b7697d01dacaa519ad5f37caa6/pysolarmanv5/pysolarmanv5_async.py#L237
too! 😉

So the intended purpose was to end the wait when frame resending was not yet implemented, got it! 😉

Solution patch:
diff --git a/pysolarmanv5/pysolarmanv5_async.py b/pysolarmanv5/pysolarmanv5_async.py
index 7ca4138..b18dd6b 100644
--- a/pysolarmanv5/pysolarmanv5_async.py
+++ b/pysolarmanv5/pysolarmanv5_async.py
@@ -208,6 +208,7 @@ class PySolarmanV5Async(PySolarmanV5):
         :return: None
 
         """
+        data = b""
         while True:
             try:
                 data = await self.reader.read(1024)
@@ -234,7 +235,7 @@ class PySolarmanV5Async(PySolarmanV5):
                 self.log.debug("Data received but nobody waits for it... Discarded")
         self.reader = None
         self.writer = None
-        # self._send_data(b"")
+
         if self._needs_reconnect:
             self.log.debug(
                 "[%s] Auto reconnect enabled. Will try to restart the socket reader",
@@ -242,6 +243,9 @@ class PySolarmanV5Async(PySolarmanV5):
             )
             loop = asyncio.get_running_loop()
             loop.create_task(self.reconnect())
+        else:
+            if self.data_wanted_ev.is_set():
+               self._send_data(data)
 
     async def _send_receive_frame(self, frame: bytes) -> bytes:
         """
@@ -272,7 +276,7 @@ class PySolarmanV5Async(PySolarmanV5):
             raise NoSocketAvailableError("Connection already closed") from exc
         except NoSocketAvailableError:
             raise
-        except TimeoutError:
+        except asyncio.exceptions.TimeoutError:
             raise
         except OSError as exc:
             if exc.errno == errno.EHOSTUNREACH:


Test patch:
diff --git a/tests/setup_test.py b/tests/setup_test.py
index b6c082b..ab85aa2 100644
--- a/tests/setup_test.py
+++ b/tests/setup_test.py
@@ -15,7 +15,7 @@ from umodbus.functions import (
     create_function_from_request_pdu,
 )
 
-from pysolarmanv5.pysolarmanv5 import CONTROL_CODE, PySolarmanV5
+from pysolarmanv5.pysolarmanv5 import CONTROL_CODE, PySolarmanV5, V5FrameError
 
 
 _WIN_PLATFORM = True if platform.system() == "Windows" else False
@@ -207,10 +207,13 @@ async def stream_handler(reader: asyncio.StreamReader, writer: asyncio.StreamWri
                 log.debug(f'[AioHandler] Sending frame: {bytes(enc).hex(" ")}')
                 writer.write(bytes(enc))
                 await writer.drain()
+            except V5FrameError as e:
+                """ Close immediately - allows testing with wrong serial numbers, sequence numbers etc. """
+                log.debug(f"[AioHandler] V5FrameError({' '.join(e.args)}). Closing immediately... ")
+                break
             except Exception as e:
                 log.exception(e)
                 writer.write(data)
-
             if cl_packets == 3:
                 # Write counter packet and wait some time to be consumed
                 await random_delay()
diff --git a/tests/test_aio_solarman.py b/tests/test_aio_solarman.py
index 5e097e5..b83eb56 100644
--- a/tests/test_aio_solarman.py
+++ b/tests/test_aio_solarman.py
@@ -14,7 +14,7 @@ def test_async():
     async def wrapper():
         log.debug("Async starting")
         solarman = PySolarmanV5Async(
-            "127.0.0.1", 2612749371, auto_reconnect=True, verbose=True, socket_timeout=5
+            "127.0.0.1", 2612749371, auto_reconnect=True, verbose=True, socket_timeout=10
         )
         await solarman.connect()
         log.debug("Async connected!!!")
@@ -52,3 +52,43 @@ def test_async():
         loop.run_until_complete(wrapper())
     except RuntimeError:
         asyncio.run(wrapper())
+
+
+def test_async_no_socket_available():
+    async def wrapper():
+        log.debug("Async NoSocketAvailable Test - starting")
+        solarman = PySolarmanV5Async(
+            "127.0.0.1", 2612749271, auto_reconnect=False, verbose=True, socket_timeout=5
+        )
+        await solarman.connect()
+        log.debug("Async connected!!!")
+        with pytest.raises(NoSocketAvailableError) as einfo:
+            res = await solarman.read_holding_registers(20, 4)
+        assert einfo.type is NoSocketAvailableError
+
+
+    try:
+        loop = asyncio.get_running_loop()
+        loop.run_until_complete(wrapper())
+    except RuntimeError:
+        asyncio.run(wrapper())
+
+
+def test_async_timeout_error():
+    async def wrapper():
+        log.debug("Async TimeoutError Test - starting")
+        solarman = PySolarmanV5Async(
+            "127.0.0.1", 2612749271, auto_reconnect=True, verbose=True, socket_timeout=1.5
+        )
+        await solarman.connect()
+        log.debug("Async connected!!!")
+        with pytest.raises(asyncio.exceptions.TimeoutError) as einfo:
+            res = await solarman.read_holding_registers(20, 4)
+        assert einfo.type is asyncio.exceptions.TimeoutError
+
+
+    try:
+        loop = asyncio.get_running_loop()
+        loop.run_until_complete(wrapper())
+    except RuntimeError:
+        asyncio.run(wrapper())
\ No newline at end of file