Fix #289 and other misc enhancements

dhoomakethu · dhoomakethu · commit 3b490fee80aa · 2018-04-20T14:32:40.000+05:30
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -10,7 +10,9 @@ Version 1.5.0
 * Move framers from transaction.py to respective modules
 * Fix modbus payload builder and decoder
 * Async servers can now have an option to defer `reactor.run()` when using `Start<Tcp/Serial/Udo>Server(...,defer_reactor_run=True)`
-* Fix UDP client issue while handling MEI messages (ReadDeviceInformationRequest) 
+* Fix UDP client issue while handling MEI messages (ReadDeviceInformationRequest)
+* Add expected response lengths for WriteMultipleCoilRequest and WriteMultipleRegisterRequest
+* Fix struct errors while decoding stray response
 * Fix Misc examples
 
 Version 1.4.0
diff --git a/examples/common/synchronous_client.py b/examples/common/synchronous_client.py
@@ -17,7 +17,7 @@
 # import the various server implementations
 # --------------------------------------------------------------------------- # 
 from pymodbus.client.sync import ModbusTcpClient as ModbusClient
-#from pymodbus.client.sync import ModbusUdpClient as ModbusClient
+# from pymodbus.client.sync import ModbusUdpClient as ModbusClient
 # from pymodbus.client.sync import ModbusSerialClient as ModbusClient
 
 # --------------------------------------------------------------------------- # 
@@ -62,6 +62,7 @@ def run_sync_client():
     #    client = ModbusClient('localhost', retries=3, retry_on_empty=True)
     # ------------------------------------------------------------------------# 
     client = ModbusClient('localhost', port=5020)
+    # from pymodbus.transaction import ModbusRtuFramer
     # client = ModbusClient('localhost', port=5020, framer=ModbusRtuFramer)
     # client = ModbusClient(method='binary', port='/dev/ptyp0', timeout=1)
     # client = ModbusClient(method='ascii', port='/dev/ptyp0', timeout=1)
@@ -76,6 +77,7 @@ def run_sync_client():
     # individual request. This can be done by specifying the `unit` parameter
     # which defaults to `0x00`
     # ----------------------------------------------------------------------- #
+    rr = client.read_holding_registers(0, 130, unit=UNIT)
     log.debug("Reading Coils")
     rr = client.read_coils(1, 1, unit=UNIT)
     log.debug(rr)
diff --git a/examples/common/synchronous_client_ext.py b/examples/common/synchronous_client_ext.py
@@ -13,7 +13,7 @@
 # from pymodbus.client.sync import ModbusTcpClient as ModbusClient
 # from pymodbus.client.sync import ModbusUdpClient as ModbusClient
 from pymodbus.client.sync import ModbusSerialClient as ModbusClient
-from pymodbus.transaction import ModbusRtuFramer
+
 
 # --------------------------------------------------------------------------- # 
 # import the extended messages to perform
@@ -51,6 +51,8 @@ def execute_extended_requests():
     # client = ModbusClient(method='ascii', port="/dev/ptyp0")
     # client = ModbusClient(method='binary', port="/dev/ptyp0")
     # client = ModbusClient('127.0.0.1', port=5020)
+    # from pymodbus.transaction import ModbusRtuFramer
+    # client = ModbusClient('127.0.0.1', port=5020, framer=ModbusRtuFramer)
     client.connect()
 
     # ----------------------------------------------------------------------- # 
diff --git a/pymodbus/bit_write_message.py b/pymodbus/bit_write_message.py
@@ -214,6 +214,13 @@ def __str__(self):
         params = (self.address, len(self.values))
         return "WriteNCoilRequest (%d) => %d " % params
 
+    def get_response_pdu_size(self):
+        """
+        Func_code (1 byte) + Output Address (2 byte) + Quantity of Outputs  (2 Bytes)
+        :return:
+        """
+        return 1 + 2 + 2
+
 
 class WriteMultipleCoilsResponse(ModbusResponse):
     '''
diff --git a/pymodbus/client/sync.py b/pymodbus/client/sync.py
@@ -230,7 +230,35 @@ def _recv(self, size):
         """
         if not self.socket:
             raise ConnectionException(self.__str__())
-        return self.socket.recv(size)
+            # socket.recv(size) waits until it gets some data from the host but
+            # not necessarily the entire response that can be fragmented in
+            # many packets.
+            # To avoid the splitted responses to be recognized as invalid
+            # messages and to be discarded, loops socket.recv until full data
+            # is received or timeout is expired.
+            # If timeout expires returns the read data, also if its length is
+            # less than the expected size.
+        self.socket.setblocking(0)
+        begin = time.time()
+
+        data = b''
+        if size is not None:
+            while len(data) < size:
+                try:
+                    data += self.socket.recv(size - len(data))
+                except socket.error:
+                    pass
+                if not self.timeout or (time.time() - begin > self.timeout):
+                    break
+        else:
+            while True:
+                try:
+                    data += self.socket.recv(1)
+                except socket.error:
+                    pass
+                if not self.timeout or (time.time() - begin > self.timeout):
+                    break
+        return data
 
     def is_socket_open(self):
         return True if self.socket is not None else False
@@ -423,6 +451,16 @@ def close(self):
             self.socket.close()
         self.socket = None
 
+    def _in_waiting(self):
+        in_waiting = ("in_waiting" if hasattr(
+            self.socket, "in_waiting") else "inWaiting")
+
+        if in_waiting == "in_waiting":
+            waitingbytes = getattr(self.socket, in_waiting)
+        else:
+            waitingbytes = getattr(self.socket, in_waiting)()
+        return waitingbytes
+
     def _send(self, request):
         """ Sends data on the underlying socket
 
@@ -438,13 +476,7 @@ def _send(self, request):
             raise ConnectionException(self.__str__())
         if request:
             try:
-                in_waiting = ("in_waiting" if hasattr(
-                    self.socket, "in_waiting") else "inWaiting")
-
-                if in_waiting == "in_waiting":
-                    waitingbytes = getattr(self.socket, in_waiting)
-                else:
-                    waitingbytes = getattr(self.socket, in_waiting)()
+                waitingbytes = self._in_waiting()
                 if waitingbytes:
                     result = self.socket.read(waitingbytes)
                     if _logger.isEnabledFor(logging.WARNING):
@@ -465,6 +497,8 @@ def _recv(self, size):
         """
         if not self.socket:
             raise ConnectionException(self.__str__())
+        if size is None:
+            size = self._in_waiting()
         result = self.socket.read(size)
         return result
 
diff --git a/pymodbus/exceptions.py b/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
     """
diff --git a/pymodbus/factory.py b/pymodbus/factory.py
@@ -223,6 +223,9 @@ def decode(self, message):
             return self._helper(message)
         except ModbusException as er:
             _logger.error("Unable to decode response %s" % er)
+
+        except Exception as ex:
+            _logger.error(ex)
         return None
 
     def _helper(self, data):
@@ -234,8 +237,13 @@ def _helper(self, data):
         :param data: The response packet to decode
         :returns: The decoded request or an exception response object
         '''
-        function_code = byte2int(data[0])
-        _logger.debug("Factory Response[%d]" % function_code)
+        fc_string = function_code = byte2int(data[0])
+        if function_code in self.__lookup:
+            fc_string = "%s: %s" % (
+                str(self.__lookup[function_code]).split('.')[-1].rstrip("'>"),
+                function_code
+            )
+        _logger.debug("Factory Response[%s]" % fc_string)
         response = self.__lookup.get(function_code, lambda: None)()
         if function_code > 0x80:
             code = function_code & 0x7f  # strip error portion
diff --git a/pymodbus/framer/rtu_framer.py b/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
@@ -213,27 +213,16 @@ def processIncomingPacket(self, data, callback, unit, **kwargs):
             unit = [unit]
         self.addToFrame(data)
         single = kwargs.get("single", False)
-        while True:
-            if self.isFrameReady():
-                if self.checkFrame():
-                    if self._validate_unit_id(unit, single):
-                        self._process(callback)
-                    else:
-                        _logger.debug("Not a valid unit id - {}, "
-                                      "ignoring!!".format(self._header['uid']))
-                        self.resetFrame()
-
+        if self.isFrameReady():
+            if self.checkFrame():
+                if self._validate_unit_id(unit, single):
+                    self._process(callback)
                 else:
-                    # Could be an error response
-                    if len(self._buffer):
-                        # Possible error ???
-                        self._process(callback, error=True)
-            else:
-                if len(self._buffer):
-                    # Possible error ???
-                    if self._header.get('len', 0) < 2:
-                        self._process(callback, error=True)
-                break
+                    _logger.debug("Not a valid unit id - {}, "
+                                  "ignoring!!".format(self._header['uid']))
+                    self.resetFrame()
+        else:
+            _logger.debug("Frame - [{}] not ready".format(data))
 
     def buildPacket(self, message):
         """
@@ -258,7 +247,7 @@ def sendPacket(self, message):
         #     ModbusTransactionState.to_string(self.client.state))
         # )
         while self.client.state != ModbusTransactionState.IDLE:
-            if self.client.state == ModbusTransactionState.TRANSCATION_COMPLETE:
+            if self.client.state == ModbusTransactionState.TRANSACTION_COMPLETE:
                 ts = round(time.time(), 6)
                 _logger.debug("Changing state to IDLE - Last Frame End - {}, "
                               "Current Time stamp - {}".format(
@@ -296,11 +285,6 @@ def recvPacket(self, size):
         :return:
         """
         result = self.client.recv(size)
-        # if self.client.state != ModbusTransactionState.PROCESSING_REPLY:
-        #     _logger.debug("Changing transaction state from "
-        #                   "'WAITING FOR REPLY' to 'PROCESSING REPLY'")
-        #     self.client.state = ModbusTransactionState.PROCESSING_REPLY
-
         self.client.last_frame_end = round(time.time(), 6)
         return result
 
@@ -313,7 +297,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()
diff --git a/pymodbus/framer/socket_framer.py b/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()
diff --git a/pymodbus/register_write_message.py b/pymodbus/register_write_message.py
@@ -192,6 +192,13 @@ def execute(self, context):
         context.setValues(self.function_code, self.address, self.values)
         return WriteMultipleRegistersResponse(self.address, self.count)
 
+    def get_response_pdu_size(self):
+        """
+        Func_code (1 byte) + Starting Address (2 byte) + Quantity of Reggisters  (2 Bytes)
+        :return:
+        """
+        return 1 + 2 + 2
+
     def __str__(self):
         ''' Returns a string representation of the instance
 
diff --git a/pymodbus/transaction.py b/pymodbus/transaction.py
@@ -4,9 +4,10 @@
 import struct
 import socket
 from threading import RLock
+from functools import partial
 
 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
@@ -143,6 +144,8 @@ def execute(self, request):
                 c_str = str(self.client)
                 if "modbusudpclient" in c_str.lower().strip():
                     full = True
+                    if not expected_response_length:
+                        expected_response_length = 1024
                 response, last_exception = self._transact(request,
                                                           expected_response_length,
                                                           full=full
@@ -167,25 +170,29 @@ def execute(self, request):
                             retries -= 1
                             continue
                         break
+                addTransaction = partial(self.addTransaction,
+                                         tid=request.transaction_id)
                 self.client.framer.processIncomingPacket(response,
-                                                         self.addTransaction,
+                                                         addTransaction,
                                                          request.unit_id)
                 response = self.getTransaction(request.transaction_id)
                 if not response:
                     if len(self.transactions):
                         response = self.getTransaction(tid=0)
                     else:
-                        last_exception = last_exception or ("No Response received "
-                                                            "from the remote unit")
+                        last_exception = last_exception or (
+                            "No Response received from the remote unit")
                         response = ModbusIOException(last_exception)
                 if hasattr(self.client, "state"):
                     _logger.debug("Changing transaction state from "
-                                  "'PROCESSING REPLY' to 'TRANSCATION_COMPLETE'")
-                    self.client.state = ModbusTransactionState.TRANSCATION_COMPLETE
+                                  "'PROCESSING REPLY' to "
+                                  "'TRANSACTION_COMPLETE'")
+                    self.client.state = (
+                        ModbusTransactionState.TRANSACTION_COMPLETE)
                 return response
             except Exception as ex:
                 _logger.exception(ex)
-                self.client.state = ModbusTransactionState.TRANSCATION_COMPLETE
+                self.client.state = ModbusTransactionState.TRANSACTION_COMPLETE
                 raise
 
     def _transact(self, packet, response_length, full=False):
@@ -208,11 +215,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 +230,7 @@ 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
+        total = None
         if not full:
             exception_length = self._calculate_exception_length()
             if isinstance(self.client.framer, ModbusSocketFramer):
@@ -256,8 +263,9 @@ def _recv(self, expected_response_length, full):
                         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
+                    if expected_response_length is not None:
+                        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
@@ -269,7 +277,7 @@ def _recv(self, expected_response_length, full):
         result = self.client.framer.recvPacket(expected_response_length)
         result = read_min + result
         actual = len(result)
-        if actual != total:
+        if total is not None and actual != total:
             _logger.debug("Incomplete message received, "
                           "Expected {} bytes Recieved "
                           "{} bytes !!!!".format(total, actual))
diff --git a/pymodbus/utilities.py b/pymodbus/utilities.py
diff --git a/test/test_client_sync.py b/test/test_client_sync.py
diff --git a/test/test_transaction.py b/test/test_transaction.py
