Handle the reception of incomplete responses.

If an incomplete response is received strange things happened
(e.g. receiving 0xFF raise an IndexError, receiving 0x011012
raise a struct.error).
Now if an incomplete response is received an error is logged.

Related to discussion in pymodbus-dev#188 , Fix pymodbus-dev#211

Author: Claudio Catterina

pymodbus/exceptions.py

@@ -78,7 +78,7 @@ def __init__(self, string=""):
         ModbusException.__init__(self, message)
 
 
-class InvalidMessageRecievedException(ModbusException):
+class InvalidMessageReceivedException(ModbusException):
     """
     Error resulting from invalid response received or decoded
     """

pymodbus/framer/rtu_framer.py

@@ -2,7 +2,7 @@
 import time
 
 from pymodbus.exceptions import ModbusIOException
-from pymodbus.exceptions import InvalidMessageRecievedException
+from pymodbus.exceptions import InvalidMessageReceivedException
 from pymodbus.utilities import checkCRC, computeCRC
 from pymodbus.utilities import hexlify_packets, ModbusTransactionState
 from pymodbus.compat import byte2int
@@ -313,7 +313,7 @@ def _process(self, callback, error=False):
         if result is None:
             raise ModbusIOException("Unable to decode request")
         elif error and result.function_code < 0x80:
-            raise InvalidMessageRecievedException(result)
+            raise InvalidMessageReceivedException(result)
         else:
             self.populateResult(result)
             self.advanceFrame()

pymodbus/framer/socket_framer.py

@@ -1,6 +1,6 @@
 import struct
 from pymodbus.exceptions import ModbusIOException
-from pymodbus.exceptions import InvalidMessageRecievedException
+from pymodbus.exceptions import InvalidMessageReceivedException
 from pymodbus.utilities import hexlify_packets
 from pymodbus.framer import ModbusFramer, SOCKET_FRAME_HEADER
 
@@ -174,7 +174,7 @@ def _process(self, callback, error=False):
         if result is None:
             raise ModbusIOException("Unable to decode request")
         elif error and result.function_code < 0x80:
-            raise InvalidMessageRecievedException(result)
+            raise InvalidMessageReceivedException(result)
         else:
             self.populateResult(result)
             self.advanceFrame()

pymodbus/transaction.py

@@ -6,7 +6,7 @@
 from threading import RLock
 
 from pymodbus.exceptions import ModbusIOException, NotImplementedException
-from pymodbus.exceptions import InvalidMessageRecievedException
+from pymodbus.exceptions import InvalidMessageReceivedException
 from pymodbus.constants import Defaults
 from pymodbus.framer.ascii_framer import ModbusAsciiFramer
 from pymodbus.framer.rtu_framer import ModbusRtuFramer
@@ -74,14 +74,9 @@ def _set_adu_size(self):
             self.base_adu_size = 7  # start(1)+ Address(2), LRC(2) + end(2)
         elif isinstance(self.client.framer, ModbusBinaryFramer):
             self.base_adu_size = 5  # start(1) + Address(1), CRC(2) + end(1)
-        else:
-            self.base_adu_size = -1
 
     def _calculate_response_length(self, expected_pdu_size):
-        if self.base_adu_size == -1:
-            return None
-        else:
-            return self.base_adu_size + expected_pdu_size
+        return self.base_adu_size + expected_pdu_size
 
     def _calculate_exception_length(self):
         ''' Returns the length of the Modbus Exception Response according to
@@ -94,8 +89,6 @@ def _calculate_exception_length(self):
         elif isinstance(self.client.framer, (ModbusRtuFramer, ModbusBinaryFramer)):
             return self.base_adu_size + 2  # Fcode(1), ExcecptionCode(1)
 
-        return None
-
     def _check_response(self, response):
         ''' Checks if the response is a Modbus Exception.
         '''
@@ -208,11 +201,11 @@ def _transact(self, packet, response_length, full=False):
                 _logger.debug("Changing transaction state from 'SENDING' "
                               "to 'WAITING FOR REPLY'")
                 self.client.state = ModbusTransactionState.WAITING_FOR_REPLY
-            result = self._recv(response_length or 1024, full)
+            result = self._recv(response_length, full)
             if _logger.isEnabledFor(logging.DEBUG):
                 _logger.debug("RECV: " + hexlify_packets(result))
         except (socket.error, ModbusIOException,
-                InvalidMessageRecievedException) as msg:
+                InvalidMessageReceivedException) as msg:
             self.client.close()
             _logger.debug("Transaction failed. (%s) " % msg)
             last_exception = msg
@@ -223,7 +216,6 @@ def _send(self, packet):
         return self.client.framer.sendPacket(packet)
 
     def _recv(self, expected_response_length, full):
-        expected_response_length = expected_response_length or 1024
         if not full:
             exception_length = self._calculate_exception_length()
             if isinstance(self.client.framer, ModbusSocketFramer):
@@ -238,31 +230,37 @@ def _recv(self, expected_response_length, full):
                 min_size = expected_response_length
 
             read_min = self.client.framer.recvPacket(min_size)
-            if read_min:
+            if not read_min:
+                return read_min
+
+            if len(read_min) < min_size:
+                raise InvalidMessageReceivedException(
+                    "Incomplete message received, expected at least %d bytes (%d received)"
+                    % (min_size, len(read_min)))
+
+            if isinstance(self.client.framer, ModbusSocketFramer):
+                func_code = byte2int(read_min[-1])
+            elif isinstance(self.client.framer, ModbusRtuFramer):
+                func_code = byte2int(read_min[-1])
+            elif isinstance(self.client.framer, ModbusAsciiFramer):
+                func_code = int(read_min[3:5], 16)
+            elif isinstance(self.client.framer, ModbusBinaryFramer):
+                func_code = byte2int(read_min[-1])
+            else:
+                func_code = -1
+
+            if func_code < 0x80:    # Not an error
                 if isinstance(self.client.framer, ModbusSocketFramer):
-                    func_code = byte2int(read_min[-1])
-                elif isinstance(self.client.framer, ModbusRtuFramer):
-                    func_code = byte2int(read_min[-1])
-                elif isinstance(self.client.framer, ModbusAsciiFramer):
-                    func_code = int(read_min[3:5], 16)
-                elif isinstance(self.client.framer, ModbusBinaryFramer):
-                    func_code = byte2int(read_min[-1])
-                else:
-                    func_code = -1
-
-                if func_code < 0x80:    # Not an error
-                    if isinstance(self.client.framer, ModbusSocketFramer):
-                        # Ommit UID, which is included in header size
-                        h_size = self.client.framer._hsize
-                        length = struct.unpack(">H", read_min[4:6])[0] - 1
-                        expected_response_length = h_size + length
-                    expected_response_length -= min_size
-                    total = expected_response_length + min_size
-                else:
-                    expected_response_length = exception_length - min_size
-                    total = expected_response_length + min_size
+                    # Ommit UID, which is included in header size
+                    h_size = self.client.framer._hsize
+                    length = struct.unpack(">H", read_min[4:6])[0] - 1
+                    expected_response_length = h_size + length
+                expected_response_length -= min_size
+                total = expected_response_length + min_size
             else:
-                total = expected_response_length
+                expected_response_length = exception_length - min_size
+                total = expected_response_length + min_size
+
         else:
             read_min = b''
             total = expected_response_length
@@ -273,6 +271,9 @@ def _recv(self, expected_response_length, full):
             _logger.debug("Incomplete message received, "
                           "Expected {} bytes Recieved "
                           "{} bytes !!!!".format(total, actual))
+            raise InvalidMessageReceivedException(
+                "Incomplete message received, %d bytes expected (%d received)"
+                % (total, actual))
         if self.client.state != ModbusTransactionState.PROCESSING_REPLY:
             _logger.debug("Changing transaction state from "
                           "'WAITING FOR REPLY' to 'PROCESSING REPLY'")

test/test_transaction.py

@@ -8,10 +8,13 @@
     ModbusRtuFramer, ModbusBinaryFramer
 )
 from pymodbus.factory import ServerDecoder
-from pymodbus.compat import byte2int
-from mock import MagicMock
+from pymodbus.compat import IS_PYTHON3, byte2int
+if IS_PYTHON3: # Python 3
+    from unittest.mock import MagicMock, PropertyMock
+else: # Python 2
+    from mock import MagicMock, PropertyMock
 from pymodbus.exceptions import (
-    NotImplementedException, ModbusIOException, InvalidMessageRecievedException
+    NotImplementedException, ModbusIOException, InvalidMessageReceivedException
 )
 
 class ModbusTransactionTest(unittest.TestCase):
@@ -41,6 +44,25 @@ def tearDown(self):
         del self._rtu
         del self._ascii
 
+    def testTransactionManagerRecv(self):
+        mock_client = MagicMock()
+        mock_client.recv.side_effect = iter([b'\x11\x04', b'\x02\x00\x0a\xf8\xf4'])
+        framer = ModbusRtuFramer(self.decoder, mock_client)
+        type(mock_client).framer = PropertyMock(return_value=framer)
+        manager = ModbusTransactionManager(mock_client)
+        self.assertEqual(manager._recv(7, False), b'\x11\x04\x02\x00\x0a\xf8\xf4')
+
+        mock_client.recv.side_effect = iter([b'\x11'])
+        with self.assertRaises(InvalidMessageReceivedException):
+            manager._recv(7, False)
+
+        mock_client.recv.side_effect = iter([b''])
+        self.assertEqual(manager._recv(7, False), b'')
+
+        mock_client.recv.side_effect = iter([b'\x11\x04', b'\x02'])
+        with self.assertRaises(InvalidMessageReceivedException):
+            manager._recv(7, False)
+
     #---------------------------------------------------------------------------#
     # Dictionary based transaction manager
     #---------------------------------------------------------------------------#