From 3b490fee80aa7d06be6e88876a721eaaddd3810d Mon Sep 17 00:00:00 2001 From: dhoomakethu Date: Thu, 19 Apr 2018 20:47:27 +0530 Subject: [PATCH 1/6] Fix #289 and other misc enhancements --- CHANGELOG.rst | 4 +- examples/common/synchronous_client.py | 4 +- examples/common/synchronous_client_ext.py | 4 +- pymodbus/bit_write_message.py | 7 ++ pymodbus/client/sync.py | 50 +++++++-- pymodbus/exceptions.py | 2 +- pymodbus/factory.py | 12 ++- pymodbus/framer/rtu_framer.py | 40 +++---- pymodbus/framer/socket_framer.py | 4 +- pymodbus/register_write_message.py | 7 ++ pymodbus/transaction.py | 34 +++--- pymodbus/utilities.py | 4 +- test/test_client_sync.py | 121 ++++++++++++++-------- test/test_transaction.py | 2 +- 14 files changed, 190 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e8c9bc6d4..5722aaa45 100644 --- 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 `StartServer(...,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 index 41578e1fc..2afac5cbc 100755 --- 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 index 61d42ff0a..a7a8a83d6 100755 --- 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 index 641c1cf31..2dfb61892 100644 --- 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 index fb45ec6f7..a867c78ac 100644 --- 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 index a2ad48241..b225a4dd6 100644 --- 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 index 7b99fe1d6..37f3eb491 100644 --- 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 index b39649ea4..21c932842 100644 --- 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 index 37e3bfe9d..201018960 100644 --- 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 index 06e229319..3a128aba7 100644 --- 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 index 44beb8a79..6f03e7193 100644 --- 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 index 62967f4e0..dff3f10b2 100644 --- a/pymodbus/utilities.py +++ b/pymodbus/utilities.py @@ -19,7 +19,7 @@ class ModbusTransactionState(object): WAITING_TURNAROUND_DELAY = 3 PROCESSING_REPLY = 4 PROCESSING_ERROR = 5 - TRANSCATION_COMPLETE = 6 + TRANSACTION_COMPLETE = 6 @classmethod def to_string(cls, state): @@ -30,7 +30,7 @@ def to_string(cls, state): ModbusTransactionState.WAITING_TURNAROUND_DELAY: "WAITING_TURNAROUND_DELAY", ModbusTransactionState.PROCESSING_REPLY: "PROCESSING_REPLY", ModbusTransactionState.PROCESSING_ERROR: "PROCESSING_ERROR", - ModbusTransactionState.TRANSCATION_COMPLETE: "TRANSCATION_COMPLETE" + ModbusTransactionState.TRANSACTION_COMPLETE: "TRANSCATION_COMPLETE" } return states.get(state, None) diff --git a/test/test_client_sync.py b/test/test_client_sync.py index d503d0dda..3d3a2a01f 100644 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -1,10 +1,11 @@ #!/usr/bin/env python import unittest from pymodbus.compat import IS_PYTHON3 -if IS_PYTHON3: # Python 3 - from unittest.mock import patch, Mock -else: # Python 2 - from mock import patch, Mock + +if IS_PYTHON3: # Python 3 + from unittest.mock import patch, Mock, MagicMock +else: # Python 2 + from mock import patch, Mock, MagicMock import socket import serial @@ -15,30 +16,41 @@ from pymodbus.transaction import ModbusAsciiFramer, ModbusRtuFramer from pymodbus.transaction import ModbusBinaryFramer -#---------------------------------------------------------------------------# + +# ---------------------------------------------------------------------------# # Mock Classes -#---------------------------------------------------------------------------# +# ---------------------------------------------------------------------------# class mockSocket(object): def close(self): return True - def recv(self, size): return '\x00'*size - def read(self, size): return '\x00'*size + + def recv(self, size): return b'\x00' * size + + def read(self, size): return b'\x00' * size + def send(self, msg): return len(msg) + def write(self, msg): return len(msg) - def recvfrom(self, size): return ['\x00'*size] + + def recvfrom(self, size): return [b'\x00' * size] + def sendto(self, msg, *args): return len(msg) + + def setblocking(self, flag): return None + def in_waiting(self): return None -#---------------------------------------------------------------------------# + +# ---------------------------------------------------------------------------# # Fixture -#---------------------------------------------------------------------------# +# ---------------------------------------------------------------------------# class SynchronousClientTest(unittest.TestCase): ''' This is the unittest for the pymodbus.client.sync module ''' - #-----------------------------------------------------------------------# + # -----------------------------------------------------------------------# # Test Base Client - #-----------------------------------------------------------------------# + # -----------------------------------------------------------------------# def testBaseModbusClient(self): ''' Test the base class for all the clients ''' @@ -50,9 +62,10 @@ def testBaseModbusClient(self): self.assertRaises(NotImplementedException, lambda: client._recv(None)) self.assertRaises(NotImplementedException, lambda: client.__enter__()) self.assertRaises(NotImplementedException, lambda: client.execute()) + self.assertRaises(NotImplementedException, lambda: client.is_socket_open()) self.assertEqual("Null Transport", str(client)) client.close() - client.__exit__(0,0,0) + client.__exit__(0, 0, 0) # a successful execute client.connect = lambda: True @@ -65,9 +78,9 @@ def testBaseModbusClient(self): self.assertRaises(ConnectionException, lambda: client.__enter__()) self.assertRaises(ConnectionException, lambda: client.execute()) - #-----------------------------------------------------------------------# + # -----------------------------------------------------------------------# # Test UDP Client - #-----------------------------------------------------------------------# + # -----------------------------------------------------------------------# def testSyncUdpClientInstantiation(self): client = ModbusUdpClient() @@ -80,8 +93,8 @@ def testBasicSyncUdpClient(self): client = ModbusUdpClient() client.socket = mockSocket() self.assertEqual(0, client._send(None)) - self.assertEqual(1, client._send('\x00')) - self.assertEqual('\x00', client._recv(1)) + self.assertEqual(1, client._send(b'\x00')) + self.assertEqual(b'\x00', client._recv(1)) # connect/disconnect self.assertTrue(client.connect()) @@ -96,7 +109,8 @@ def testBasicSyncUdpClient(self): def testUdpClientAddressFamily(self): ''' Test the Udp client get address family method''' client = ModbusUdpClient() - self.assertEqual(socket.AF_INET, client._get_address_family('127.0.0.1')) + self.assertEqual(socket.AF_INET, + client._get_address_family('127.0.0.1')) self.assertEqual(socket.AF_INET6, client._get_address_family('::1')) def testUdpClientConnect(self): @@ -105,6 +119,7 @@ def testUdpClientConnect(self): class DummySocket(object): def settimeout(self, *a, **kwa): pass + mock_method.return_value = DummySocket() client = ModbusUdpClient() self.assertTrue(client.connect()) @@ -129,13 +144,14 @@ def testUdpClientRecv(self): self.assertRaises(ConnectionException, lambda: client._recv(1024)) client.socket = mockSocket() - self.assertEqual('', client._recv(0)) - self.assertEqual('\x00'*4, client._recv(4)) + self.assertEqual(b'', client._recv(0)) + self.assertEqual(b'\x00' * 4, client._recv(4)) + - #-----------------------------------------------------------------------# + # -----------------------------------------------------------------------# # Test TCP Client - #-----------------------------------------------------------------------# - + # -----------------------------------------------------------------------# + def testSyncTcpClientInstantiation(self): client = ModbusTcpClient() self.assertNotEqual(client, None) @@ -147,8 +163,8 @@ def testBasicSyncTcpClient(self): client = ModbusTcpClient() client.socket = mockSocket() self.assertEqual(0, client._send(None)) - self.assertEqual(1, client._send('\x00')) - self.assertEqual('\x00', client._recv(1)) + self.assertEqual(1, client._send(b'\x00')) + self.assertEqual(b'\x00', client._recv(1)) # connect/disconnect self.assertTrue(client.connect()) @@ -187,20 +203,35 @@ def testTcpClientRecv(self): self.assertRaises(ConnectionException, lambda: client._recv(1024)) client.socket = mockSocket() - self.assertEqual('', client._recv(0)) - self.assertEqual('\x00'*4, client._recv(4)) - - #-----------------------------------------------------------------------# + self.assertEqual(b'', client._recv(0)) + self.assertEqual(b'\x00' * 4, client._recv(4)) + + mock_socket = MagicMock() + mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02']) + client.socket = mock_socket + client.timeout = 1 + self.assertEqual(b'\x00\x01\x02', client._recv(3)) + mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02']) + self.assertEqual(b'\x00\x01', client._recv(2)) + + mock_socket.recv.side_effect = socket.error('No data') + self.assertEqual(b'', client._recv(2)) + + # -----------------------------------------------------------------------# # Test Serial Client - #-----------------------------------------------------------------------# + # -----------------------------------------------------------------------# def testSyncSerialClientInstantiation(self): client = ModbusSerialClient() self.assertNotEqual(client, None) - self.assertTrue(isinstance(ModbusSerialClient(method='ascii').framer, ModbusAsciiFramer)) - self.assertTrue(isinstance(ModbusSerialClient(method='rtu').framer, ModbusRtuFramer)) - self.assertTrue(isinstance(ModbusSerialClient(method='binary').framer, ModbusBinaryFramer)) - self.assertRaises(ParameterException, lambda: ModbusSerialClient(method='something')) + self.assertTrue(isinstance(ModbusSerialClient(method='ascii').framer, + ModbusAsciiFramer)) + self.assertTrue(isinstance(ModbusSerialClient(method='rtu').framer, + ModbusRtuFramer)) + self.assertTrue(isinstance(ModbusSerialClient(method='binary').framer, + ModbusBinaryFramer)) + self.assertRaises(ParameterException, + lambda: ModbusSerialClient(method='something')) def testSyncSerialRTUClientTimeouts(self): client = ModbusSerialClient(method="rtu", baudrate=9600) @@ -208,7 +239,6 @@ def testSyncSerialRTUClientTimeouts(self): client = ModbusSerialClient(method="rtu", baudrate=38400) assert client.silent_interval == round((1.75 / 1000), 6) - @patch("serial.Serial") def testBasicSyncSerialClient(self, mock_serial): ''' Test the basic methods for the serial sync client''' @@ -217,14 +247,14 @@ def testBasicSyncSerialClient(self, mock_serial): mock_serial.in_waiting = 0 mock_serial.write = lambda x: len(x) - mock_serial.read = lambda size: '\x00' * size + mock_serial.read = lambda size: b'\x00' * size client = ModbusSerialClient() client.socket = mock_serial client.state = 0 self.assertEqual(0, client._send(None)) client.state = 0 - self.assertEqual(1, client._send('\x00')) - self.assertEqual('\x00', client._recv(1)) + self.assertEqual(1, client._send(b'\x00')) + self.assertEqual(b'\x00', client._recv(1)) # connect/disconnect self.assertTrue(client.connect()) @@ -266,7 +296,7 @@ def testSerialClientSend(self, mock_serial): def testSerialClientCleanupBufferBeforeSend(self, mock_serial): ''' Test the serial client send method''' mock_serial.in_waiting = 4 - mock_serial.read = lambda x: b'1'*x + mock_serial.read = lambda x: b'1' * x mock_serial.write = lambda x: len(x) client = ModbusSerialClient() self.assertRaises(ConnectionException, lambda: client._send(None)) @@ -283,11 +313,12 @@ def testSerialClientRecv(self): self.assertRaises(ConnectionException, lambda: client._recv(1024)) client.socket = mockSocket() - self.assertEqual('', client._recv(0)) - self.assertEqual('\x00'*4, client._recv(4)) + self.assertEqual(b'', client._recv(0)) + self.assertEqual(b'\x00' * 4, client._recv(4)) + -#---------------------------------------------------------------------------# +# ---------------------------------------------------------------------------# # Main -#---------------------------------------------------------------------------# +# ---------------------------------------------------------------------------# if __name__ == "__main__": - unittest.main() + unittest.main() \ No newline at end of file diff --git a/test/test_transaction.py b/test/test_transaction.py index 164dc9626..7ab576dbf 100644 --- a/test/test_transaction.py +++ b/test/test_transaction.py @@ -11,7 +11,7 @@ from pymodbus.compat import byte2int from mock import MagicMock from pymodbus.exceptions import ( - NotImplementedException, ModbusIOException, InvalidMessageRecievedException + NotImplementedException, ModbusIOException, InvalidMessageReceivedException ) class ModbusTransactionTest(unittest.TestCase): From dcce9629a221e7e716599b991678ddbad093b0ce Mon Sep 17 00:00:00 2001 From: dhoomakethu Date: Fri, 20 Apr 2018 14:59:31 +0530 Subject: [PATCH 2/6] Replace nosetest with pytest --- Makefile | 2 +- requirements-tests.txt | 2 ++ setup.cfg | 6 +++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7022b2807..43d6e43ae 100644 --- a/Makefile +++ b/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 diff --git a/requirements-tests.txt b/requirements-tests.txt index 5c4639d1b..044b5a879 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -9,6 +9,8 @@ zope.interface>=4.4.0 pyasn1>=0.2.3 pycrypto>=2.6.1 pyserial>=3.4 +pytest-cov>=2.5.1 +pytest>=3.5.0 redis>=2.10.5 sqlalchemy>=1.1.15 #wsgiref>=0.1.2 diff --git a/setup.cfg b/setup.cfg index 219c63c5d..9e8519e74 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,4 +26,8 @@ all_files = 1 upload-dir = build/sphinx/html [bdist_wheel] -universal=1 \ No newline at end of file +universal=1 + +[tool:pytest] +addopts = --cov=pymodbus/ +testpaths = test \ No newline at end of file From 7616207a3e651a2b3fc8f49536300b56e2956f28 Mon Sep 17 00:00:00 2001 From: dhoomakethu Date: Mon, 23 Apr 2018 14:33:56 +0530 Subject: [PATCH 3/6] Update Changelog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5722aaa45..8d4f12cb7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,7 @@ Version 1.5.0 * Fix UDP client issue while handling MEI messages (ReadDeviceInformationRequest) * Add expected response lengths for WriteMultipleCoilRequest and WriteMultipleRegisterRequest * Fix struct errors while decoding stray response +* Change test runner from nosetest to pytest * Fix Misc examples Version 1.4.0 From 22a20b23fa8f0e59d8907914e7089a2a5c5f92a0 Mon Sep 17 00:00:00 2001 From: dhoomakethu Date: Wed, 25 Apr 2018 21:48:05 +0530 Subject: [PATCH 4/6] serial sync client wait till timeout/some data is available in read buffer + update changelog --- CHANGELOG.rst | 1 + pymodbus/client/sync.py | 7 ++++++- pymodbus/transaction.py | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8d4f12cb7..e150c61f1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,7 @@ Version 1.5.0 * 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 diff --git a/pymodbus/client/sync.py b/pymodbus/client/sync.py index a867c78ac..dbf87570c 100644 --- a/pymodbus/client/sync.py +++ b/pymodbus/client/sync.py @@ -498,7 +498,12 @@ def _recv(self, size): if not self.socket: raise ConnectionException(self.__str__()) if size is None: - size = self._in_waiting() + start = time.time() + while (time.time() - start) <= self.timeout: + size = self._in_waiting() + if size: + break + time.sleep(0.01) result = self.socket.read(size) return result diff --git a/pymodbus/transaction.py b/pymodbus/transaction.py index 6f03e7193..6f73c158d 100644 --- a/pymodbus/transaction.py +++ b/pymodbus/transaction.py @@ -181,7 +181,8 @@ def execute(self, request): response = self.getTransaction(tid=0) else: last_exception = last_exception or ( - "No Response received from the remote unit") + "No Response received from the remote unit" + "/Unable to decode response") response = ModbusIOException(last_exception) if hasattr(self.client, "state"): _logger.debug("Changing transaction state from " From 9af05572d3a23e506a1c8a91535158c26f20b9b5 Mon Sep 17 00:00:00 2001 From: dhoomakethu Date: Thu, 26 Apr 2018 17:52:37 +0530 Subject: [PATCH 5/6] serial sync client read updates when timeout is None and Zero --- pymodbus/client/sync.py | 22 +++++++++++++++------- pymodbus/constants.py | 2 +- pymodbus/other_message.py | 2 +- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pymodbus/client/sync.py b/pymodbus/client/sync.py index dbf87570c..f1642aee7 100644 --- a/pymodbus/client/sync.py +++ b/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 @@ -489,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 @@ -498,12 +511,7 @@ def _recv(self, size): if not self.socket: raise ConnectionException(self.__str__()) if size is None: - start = time.time() - while (time.time() - start) <= self.timeout: - size = self._in_waiting() - if size: - break - time.sleep(0.01) + size = self._wait_for_data() result = self.socket.read(size) return result diff --git a/pymodbus/constants.py b/pymodbus/constants.py index 45b722e19..5763c77d7 100644 --- a/pymodbus/constants.py +++ b/pymodbus/constants.py @@ -103,7 +103,7 @@ class Defaults(Singleton): Stopbits = 1 ZeroMode = False IgnoreMissingSlaves = False - + ReadSize = 1024 class ModbusStatus(Singleton): ''' diff --git a/pymodbus/other_message.py b/pymodbus/other_message.py index b30ea7598..bf7c991c6 100644 --- a/pymodbus/other_message.py +++ b/pymodbus/other_message.py @@ -278,7 +278,7 @@ class GetCommEventLogResponse(ModbusResponse): field defines the total length of the data in these four field ''' function_code = 0x0c - _rtu_byte_count_pos = 3 + _rtu_byte_count_pos = 2 def __init__(self, **kwargs): ''' Initializes a new instance From 1ac60fdb61ced6a1a3b2cd8f5841f7f38e909894 Mon Sep 17 00:00:00 2001 From: dhoomakethu Date: Thu, 26 Apr 2018 18:13:22 +0530 Subject: [PATCH 6/6] fix sync client unit test and example --- examples/common/synchronous_client.py | 1 - test/test_client_sync.py | 13 ++++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/examples/common/synchronous_client.py b/examples/common/synchronous_client.py index 2afac5cbc..f7186dd0f 100755 --- a/examples/common/synchronous_client.py +++ b/examples/common/synchronous_client.py @@ -77,7 +77,6 @@ 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/test/test_client_sync.py b/test/test_client_sync.py index 3d3a2a01f..0c33781b7 100644 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -21,6 +21,7 @@ # Mock Classes # ---------------------------------------------------------------------------# class mockSocket(object): + timeout = 2 def close(self): return True def recv(self, size): return b'\x00' * size @@ -40,6 +41,7 @@ def setblocking(self, flag): return None def in_waiting(self): return None + # ---------------------------------------------------------------------------# # Fixture # ---------------------------------------------------------------------------# @@ -213,9 +215,13 @@ def testTcpClientRecv(self): self.assertEqual(b'\x00\x01\x02', client._recv(3)) mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02']) self.assertEqual(b'\x00\x01', client._recv(2)) - mock_socket.recv.side_effect = socket.error('No data') self.assertEqual(b'', client._recv(2)) + client.socket = mockSocket() + client.socket.timeout = 0.1 + self.assertIn(b'\x00', client._recv(None)) + + # -----------------------------------------------------------------------# # Test Serial Client @@ -315,6 +321,11 @@ def testSerialClientRecv(self): client.socket = mockSocket() self.assertEqual(b'', client._recv(0)) self.assertEqual(b'\x00' * 4, client._recv(4)) + client.socket = MagicMock() + client.socket.read.return_value = b'' + self.assertEqual(b'', client._recv(None)) + client.socket.timeout = 0 + self.assertEqual(b'', client._recv(0)) # ---------------------------------------------------------------------------#