Patch 1 (#292)

* Fix #289 and other misc enhancements

* Replace nosetest with pytest

* Update Changelog

* serial sync client wait till timeout/some data is available in read buffer + update changelog

* serial sync client read updates when timeout is None and Zero

* fix sync client unit test and example

Author: dhoomakethu

CHANGELOG.rst

@@ -10,7 +10,11 @@ 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
+* Modbus read retries works only when empty/no message is received
+* Change test runner from nosetest to pytest
 * Fix Misc examples
 
 Version 1.4.0

Makefile

@@ -39,7 +39,7 @@ check: install
 
 test: install
 	@pip install --quiet --requirement=requirements-tests.txt
-	@nosetests --with-coverage --cover-html
+	@py.test
 	@coverage report --fail-under=90
 
 tox: install

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)

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()
 
     # ----------------------------------------------------------------------- # 

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):
     '''

pymodbus/client/sync.py

@@ -2,7 +2,7 @@
 import serial
 import time
 import sys
-
+from functools import partial
 from pymodbus.constants import Defaults
 from pymodbus.utilities import hexlify_packets, ModbusTransactionState
 from pymodbus.factory import ClientDecoder
@@ -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):
@@ -457,6 +489,19 @@ def _send(self, request):
             return size
         return 0
 
+    def _wait_for_data(self):
+        if self.timeout is not None and self.timeout != 0:
+            condition = partial(lambda start, timeout: (time.time() - start) <= timeout, timeout=self.timeout)
+        else:
+            condition = partial(lambda dummy1, dummy2: True, dummy2=None)
+        start = time.time()
+        while condition(start):
+            size = self._in_waiting()
+            if size:
+                break
+            time.sleep(0.01)
+        return size
+
     def _recv(self, size):
         """ Reads data from the underlying descriptor
 
@@ -465,6 +510,8 @@ def _recv(self, size):
         """
         if not self.socket:
             raise ConnectionException(self.__str__())
+        if size is None:
+            size = self._wait_for_data()
         result = self.socket.read(size)
         return result
 

pymodbus/constants.py

@@ -103,7 +103,7 @@ class Defaults(Singleton):
     Stopbits            = 1
     ZeroMode            = False
     IgnoreMissingSlaves = False
-
+    ReadSize            = 1024
 
 class ModbusStatus(Singleton):
     '''

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/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

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()