diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..a08f75b10 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,259 @@ +name: CI + +on: + push: + branches: + - dev + - master + tags: + - v* + pull_request: + branches: + - "*" + schedule: + # Daily at 05:14 + - cron: '14 5 * * *' + +jobs: + test: + # Should match JOB_NAME below + name: ${{ matrix.task.name }} - ${{ matrix.os.name }} ${{ matrix.python.name }} ${{ matrix.arch.name }} + runs-on: ${{ matrix.os.runs-on }} + container: ${{ matrix.os.container[matrix.python.docker] }} + strategy: + fail-fast: false + matrix: + task: + - name: Test + tox: test + coverage: true + os: + - name: Linux + runs-on: ubuntu-latest + python_platform: linux + matrix: linux + container: + 2.7: docker://python:2.7-buster + 3.6: docker://python:3.6-buster + 3.7: docker://python:3.7-buster + 3.8: docker://python:3.8-buster + 3.9: docker://python:3.9-buster + pypy2: docker://pypy:2-jessie + pypy3: docker://pypy:3-stretch +# - name: Windows +# runs-on: windows-latest +# python_platform: win32 +# matrix: windows +# - name: macOS +# runs-on: macos-latest +# python_platform: darwin +# matrix: macos + python: + - name: CPython 2.7 + tox: py27 + action: 2.7 + docker: 2.7 + implementation: cpython + - name: PyPy 2.7 + tox: pypy27 + action: pypy-2.7 + docker: pypy2.7 + implementation: pypy + - name: CPython 3.6 + tox: py36 + action: 3.6 + docker: 3.6 + implementation: cpython + - name: CPython 3.7 + tox: py37 + action: 3.7 + docker: 3.7 + implementation: cpython + - name: CPython 3.8 + tox: py38 + action: 3.8 + docker: 3.8 + implementation: cpython + - name: CPython 3.9 + tox: py39 + action: 3.9 + docker: 3.9 + implementation: cpython + - name: PyPy 3.6 + tox: pypy36 + action: pypy-3.6 + docker: pypy3.6 + implementation: pypy + - name: PyPy 3.7 + tox: pypy37 + action: pypy-3.7 + docker: pypy3.7 + implementation: pypy + arch: + - name: x86 + action: x86 + matrix: x86 + - name: x64 + action: x64 + matrix: x64 + exclude: + - os: + matrix: linux + arch: + matrix: x86 + - os: + matrix: macos + arch: + matrix: x86 + env: + # Should match name above + JOB_NAME: ${{ matrix.task.name }} - ${{ matrix.os.name }} ${{ matrix.python.name }} ${{ matrix.arch.name }} + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Set up ${{ matrix.python.name }} (if CPython) + if: ${{ job.container == '' && matrix.python.implementation == 'cpython'}} + uses: actions/setup-python@v2 + with: + python-version: '${{ matrix.python.action }}.0-alpha - ${{ matrix.python.action }}.X' + architecture: '${{ matrix.arch.action }}' + - name: Set up ${{ matrix.python.name }} (if PyPy) + if: ${{ job.container == '' && matrix.python.implementation == 'pypy'}} + uses: actions/setup-python@v2 + with: + python-version: '${{ matrix.python.action }}' + architecture: '${{ matrix.arch.action }}' + - name: Install + run: | + pip install --upgrade pip setuptools wheel + pip install --upgrade tox + - uses: twisted/python-info-action@v1.0.1 + - name: Test + run: | + tox -vv -e ${{ matrix.python.tox }} + - name: Coverage Processing + if: matrix.task.coverage + run: | + mkdir coverage_reports + cp .coverage "coverage_reports/.coverage.${{ env.JOB_NAME }}" + cp coverage.xml "coverage_reports/coverage.${{ env.JOB_NAME }}.xml" + - name: Upload Coverage + if: matrix.task.coverage + uses: actions/upload-artifact@v2 + with: + name: coverage + path: coverage_reports/* + check: + # Should match JOB_NAME below + name: ${{ matrix.task.name }} - ${{ matrix.os.name }} ${{ matrix.python.name }} ${{ matrix.arch.name }} + runs-on: ${{ matrix.os.runs-on }} + container: ${{ matrix.os.container[matrix.python.docker] }} + strategy: + fail-fast: false + matrix: + task: + - name: flake8 + tox: flake8 + continue_on_error: true + - name: Docs + tox: docs + os: + - name: Linux + runs-on: ubuntu-latest + python_platform: linux + matrix: linux + container: + 3.8: docker://python:3.8-buster + python: + - name: CPython 3.8 + tox: py38 + action: 3.8 + docker: 3.8 + implementation: cpython + arch: + - name: x64 + action: x64 + matrix: x64 + env: + # Should match name above + JOB_NAME: ${{ matrix.task.name }} - ${{ matrix.os.name }} ${{ matrix.python.name }} ${{ matrix.arch.name }} + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Install + run: | + pip install --upgrade pip setuptools wheel + pip install --upgrade tox + - uses: twisted/python-info-action@v1.0.1 + - name: Test + continue-on-error: ${{ matrix.task.continue_on_error == true }} + run: | + tox -vv -e ${{ matrix.task.tox }} + coverage: + # Should match JOB_NAME below + name: ${{ matrix.task.name }} - ${{ matrix.os.name }} ${{ matrix.python.name }} ${{ matrix.arch.name }} + runs-on: ${{ matrix.os.runs-on }} + needs: + - test + container: ${{ matrix.os.container[matrix.python.docker] }} + strategy: + fail-fast: false + matrix: + task: + - name: Coverage + tox: combined-coverage + download_coverage: true + os: + - name: Linux + runs-on: ubuntu-latest + python_platform: linux + matrix: linux + container: + 3.8: docker://python:3.8-buster + python: + - name: CPython 3.8 + tox: py38 + action: 3.8 + docker: 3.8 + implementation: cpython + arch: + - name: x64 + action: x64 + matrix: x64 + env: + # Should match name above + JOB_NAME: ${{ matrix.task.name }} - ${{ matrix.os.name }} ${{ matrix.python.name }} ${{ matrix.arch.name }} + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + - name: Install + run: | + pip install --upgrade pip setuptools wheel + pip install --upgrade tox + pip install --upgrade six + - uses: twisted/python-info-action@v1.0.1 + - name: Download Coverage + if: matrix.task.download_coverage + uses: actions/download-artifact@v2 + with: + name: coverage + path: coverage_reports + - name: Test + continue-on-error: ${{ matrix.task.continue_on_error == true }} + run: | + tox -vv -e ${{ matrix.task.tox }} + all: + name: All + runs-on: ubuntu-latest + needs: + - check + - coverage + - test + steps: + - name: This + shell: python + run: | + import this diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index c83cf395d..000000000 --- a/.travis.yml +++ /dev/null @@ -1,36 +0,0 @@ -sudo: false -language: python -matrix: - include: - - os: linux - python: "2.7" - - os: linux - python: "3.6" - - os: linux - python: "3.7" - - os: linux - python: "3.8" - - os: osx - osx_image: xcode8.3 - language: generic -before_install: - - if [ $TRAVIS_OS_NAME = osx ]; then brew update; fi - - if [ $TRAVIS_OS_NAME = osx ]; then brew install openssl; fi -# - if [$TRAVIS_OS_NAME = osx ]; then python -c "import fcntl; fcntl.fcntl(1, fcntl.F_SETFL, 0)"; fi - -install: -# - scripts/travis.sh pip install pip-accel - - if [ $TRAVIS_OS_NAME = osx ]; then scripts/travis.sh pip install -U pip "\"setuptools<45"\"; else pip install -U pip setuptools --upgrade ; fi - - scripts/travis.sh pip install coveralls - - scripts/travis.sh pip install --requirement=requirements-checks.txt - - scripts/travis.sh pip install --requirement=requirements-tests.txt - - scripts/travis.sh LC_ALL=C pip install --upgrade . -# - scripts/travis.sh pip freeze --all -script: -# - scripts/travis.sh make check - - scripts/travis.sh make test -after_success: - - scripts/travis.sh coveralls -branches: - except: - - /^[0-9]/ diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0856cedbd..c70f4799b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,3 +1,14 @@ +version 2.5.0 +---------------------------------------------------------- +* Support response types `stray` and `empty` in repl server. +* Minor updates in asyncio server. +* Update reactive server to send stray response of given length. +* Transaction manager updates on retries for empty and invalid packets. +* Test fixes for asyncio client and transaction manager. +* Fix sync client and processing of incomplete frames with rtu framers +* Support synchronous diagnostic client (TCP) +* Server updates (REPL and async) + version 2.5.0rc3 ---------------------------------------------------------- * Minor fix in documentations diff --git a/doc/conf.py b/doc/conf.py index 294b41b79..4f0dc4122 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -47,7 +47,7 @@ # ones. #extensions = ['sphinx.ext.autodoc', 'm2r', 'recommonmark'] -extensions = ['sphinx.ext.autodoc', 'm2r'] +extensions = ['sphinx.ext.autodoc', 'm2r2'] # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] diff --git a/pymodbus/client/sync.py b/pymodbus/client/sync.py index e4e083c7d..aa7fd23cc 100644 --- a/pymodbus/client/sync.py +++ b/pymodbus/client/sync.py @@ -155,8 +155,8 @@ def _dump(self, data, direction): try: fd.write(hexlify_packets(data)) except Exception as e: - self._logger.debug(hexlify_packets(data)) - self._logger.exception(e) + _logger.debug(hexlify_packets(data)) + _logger.exception(e) def register(self, function): """ @@ -258,7 +258,12 @@ def _recv(self, size): """ Reads data from the underlying descriptor :param size: The number of bytes to read - :return: The bytes read + :return: The bytes read if the peer sent a response, or a zero-length + response if no data packets were received from the client at + all. + :raises: ConnectionException if the socket is not initialized, or the + peer either has closed the connection before this method is + invoked or closes it before sending any data before timeout. """ if not self.socket: raise ConnectionException(self.__str__()) @@ -275,9 +280,9 @@ def _recv(self, size): timeout = self.timeout - # If size isn't specified read 1 byte at a time. + # If size isn't specified read up to 4096 bytes at a time. if size is None: - recv_size = 1 + recv_size = 4096 else: recv_size = size @@ -289,6 +294,9 @@ def _recv(self, size): ready = select.select([self.socket], [], [], end - time_) if ready[0]: recv_data = self.socket.recv(recv_size) + if recv_data == b'': + return self._handle_abrupt_socket_close( + size, data, time.time() - time_) data.append(recv_data) data_length += len(recv_data) time_ = time.time() @@ -305,6 +313,35 @@ def _recv(self, size): return b"".join(data) + def _handle_abrupt_socket_close(self, size, data, duration): + """ Handle unexpected socket close by remote end + + Intended to be invoked after determining that the remote end + has unexpectedly closed the connection, to clean up and handle + the situation appropriately. + + :param size: The number of bytes that was attempted to read + :param data: The actual data returned + :param duration: Duration from the read was first attempted + until it was determined that the remote closed the + socket + :return: The more than zero bytes read from the remote end + :raises: ConnectionException If the remote end didn't send any + data at all before closing the connection. + """ + self.close() + readsize = ("read of %s bytes" % size if size + else "unbounded read") + msg = ("%s: Connection unexpectedly closed " + "%.6f seconds into %s" % (self, duration, readsize)) + if data: + result = b"".join(data) + msg += " after returning %s bytes" % len(result) + _logger.warning(msg) + return result + msg += " without response from unit before it closed connection" + raise ConnectionException(msg) + def is_socket_open(self): return True if self.socket is not None else False diff --git a/pymodbus/client/sync_diag.py b/pymodbus/client/sync_diag.py new file mode 100644 index 000000000..e2e41291f --- /dev/null +++ b/pymodbus/client/sync_diag.py @@ -0,0 +1,167 @@ +import socket +import logging +import time + +from pymodbus.constants import Defaults +from pymodbus.client.sync import ModbusTcpClient +from pymodbus.transaction import ModbusSocketFramer +from pymodbus.exceptions import ConnectionException + +_logger = logging.getLogger(__name__) + +LOG_MSGS = { + 'conn_msg': 'Connecting to modbus device %s', + 'connfail_msg': 'Connection to (%s, %s) failed: %s', + 'discon_msg': 'Disconnecting from modbus device %s', + 'timelimit_read_msg': + 'Modbus device read took %.4f seconds, ' + 'returned %s bytes in timelimit read', + 'timeout_msg': + 'Modbus device timeout after %.4f seconds, ' + 'returned %s bytes %s', + 'delay_msg': + 'Modbus device read took %.4f seconds, ' + 'returned %s bytes of %s expected', + 'read_msg': + 'Modbus device read took %.4f seconds, ' + 'returned %s bytes of %s expected', + 'unexpected_dc_msg': '%s %s'} + + +class ModbusTcpDiagClient(ModbusTcpClient): + """ + Variant of pymodbus.client.sync.ModbusTcpClient with additional + logging to diagnose network issues. + + The following events are logged: + + +---------+-----------------------------------------------------------------+ + | Level | Events | + +=========+=================================================================+ + | ERROR | Failure to connect to modbus unit; unexpected disconnect by | + | | modbus unit | + +---------+-----------------------------------------------------------------+ + | WARNING | Timeout on normal read; read took longer than warn_delay_limit | + +---------+-----------------------------------------------------------------+ + | INFO | Connection attempt to modbus unit; disconnection from modbus | + | | unit; each time limited read | + +---------+-----------------------------------------------------------------+ + | DEBUG | Normal read with timing information | + +---------+-----------------------------------------------------------------+ + + Reads are differentiated between "normal", which reads a specified number of + bytes, and "time limited", which reads all data for a duration equal to the + timeout period configured for this instance. + """ + + # pylint: disable=no-member + + def __init__(self, host='127.0.0.1', port=Defaults.Port, + framer=ModbusSocketFramer, **kwargs): + """ Initialize a client instance + + The keys of LOG_MSGS can be used in kwargs to customize the messages. + + :param host: The host to connect to (default 127.0.0.1) + :param port: The modbus port to connect to (default 502) + :param source_address: The source address tuple to bind to (default ('', 0)) + :param timeout: The timeout to use for this socket (default Defaults.Timeout) + :param warn_delay_limit: Log reads that take longer than this as warning. + Default True sets it to half of "timeout". None never logs these as + warning, 0 logs everything as warning. + :param framer: The modbus framer to use (default ModbusSocketFramer) + + .. note:: The host argument will accept ipv4 and ipv6 hosts + """ + self.warn_delay_limit = kwargs.get('warn_delay_limit', True) + super(ModbusTcpDiagClient, self).__init__(host, port, framer, **kwargs) + if self.warn_delay_limit is True: + self.warn_delay_limit = self.timeout / 2 + + # Set logging messages, defaulting to LOG_MSGS + for (k, v) in LOG_MSGS.items(): + self.__dict__[k] = kwargs.get(k, v) + + def connect(self): + """ Connect to the modbus tcp server + + :returns: True if connection succeeded, False otherwise + """ + if self.socket: + return True + try: + _logger.info(self.conn_msg, self) + self.socket = socket.create_connection( + (self.host, self.port), + timeout=self.timeout, + source_address=self.source_address) + except socket.error as msg: + _logger.error(self.connfail_msg, self.host, self.port, msg) + self.close() + return self.socket is not None + + def close(self): + """ Closes the underlying socket connection + """ + if self.socket: + _logger.info(self.discon_msg, self) + self.socket.close() + self.socket = None + + def _recv(self, size): + try: + start = time.time() + + result = super(ModbusTcpDiagClient, self)._recv(size) + + delay = time.time() - start + if self.warn_delay_limit is not None and delay >= self.warn_delay_limit: + self._log_delayed_response(len(result), size, delay) + elif not size: + _logger.debug(self.timelimit_read_msg, delay, len(result)) + else: + _logger.debug(self.read_msg, delay, len(result), size) + + return result + except ConnectionException as ex: + # Only log actual network errors, "if not self.socket" then it's a internal code issue + if 'Connection unexpectedly closed' in ex.string: + _logger.error(self.unexpected_dc_msg, self, ex) + raise ex + + def _log_delayed_response(self, result_len, size, delay): + if not size and result_len > 0: + _logger.info(self.timelimit_read_msg, delay, result_len) + elif (result_len == 0 or (size and result_len < size)) and delay >= self.timeout: + read_type = ("of %i expected" % size) if size else "in timelimit read" + _logger.warning(self.timeout_msg, delay, result_len, read_type) + else: + _logger.warning(self.delay_msg, delay, result_len, size) + + def __str__(self): + """ Builds a string representation of the connection + + :returns: The string representation + """ + return "ModbusTcpDiagClient(%s:%s)" % (self.host, self.port) + + +def get_client(): + """ Returns an appropriate client based on logging level + + This will be ModbusTcpDiagClient by default, or the parent class + if the log level is such that the diagnostic client will not log + anything. + + :returns: ModbusTcpClient or a child class thereof + """ + return ModbusTcpDiagClient if _logger.isEnabledFor(logging.ERROR) else ModbusTcpClient + + +# --------------------------------------------------------------------------- # +# Exported symbols +# --------------------------------------------------------------------------- # + +__all__ = [ + "ModbusTcpDiagClient", "get_client" +] diff --git a/pymodbus/framer/rtu_framer.py b/pymodbus/framer/rtu_framer.py index b60efb1ea..6c246bae1 100644 --- a/pymodbus/framer/rtu_framer.py +++ b/pymodbus/framer/rtu_framer.py @@ -60,7 +60,7 @@ def __init__(self, decoder, client=None): :param decoder: The decoder factory implementation to use """ self._buffer = b'' - self._header = {'uid': 0x00, 'len': 0, 'crc': '0000'} + self._header = {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'} self._hsize = 0x01 self._end = b'\x0d\x0a' self._min_frame_size = 4 @@ -89,14 +89,9 @@ def checkFrame(self): self.populateHeader() frame_size = self._header['len'] data = self._buffer[:frame_size - 2] - crc = self._buffer[frame_size - 2:frame_size] + crc = self._header['crc'] crc_val = (byte2int(crc[0]) << 8) + byte2int(crc[1]) - if checkCRC(data, crc_val): - return True - else: - _logger.debug("CRC invalid, discarding header!!") - self.resetFrame() - return False + return checkCRC(data, crc_val) except (IndexError, KeyError, struct.error): return False @@ -107,13 +102,10 @@ def advanceFrame(self): it or determined that it contains an error. It also has to reset the current frame header handle """ - try: - self._buffer = self._buffer[self._header['len']:] - except KeyError: - # Error response, no header len found - self.resetFrame() + + self._buffer = self._buffer[self._header['len']:] _logger.debug("Frame advanced, resetting header!!") - self._header = {} + self._header = {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'} def resetFrame(self): """ @@ -127,7 +119,7 @@ def resetFrame(self): _logger.debug("Resetting frame - Current Frame in " "buffer - {}".format(hexlify_packets(self._buffer))) self._buffer = b'' - self._header = {} + self._header = {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'} def isFrameReady(self): """ @@ -137,31 +129,38 @@ def isFrameReady(self): :returns: True if ready, False otherwise """ - if len(self._buffer) > self._hsize: - if not self._header: - self.populateHeader() + if len(self._buffer) <= self._hsize: + return False - return self._header and len(self._buffer) >= self._header['len'] - else: + try: + # Frame is ready only if populateHeader() successfully populates crc field which finishes RTU frame + # Otherwise, if buffer is not yet long enough, populateHeader() raises IndexError + self.populateHeader() + except IndexError: return False + return True + def populateHeader(self, data=None): """ Try to set the headers `uid`, `len` and `crc`. This method examines `self._buffer` and writes meta - information into `self._header`. It calculates only the - values for headers that are not already in the dictionary. + information into `self._header`. Beware that this method will raise an IndexError if `self._buffer` is not yet long enough. """ - data = data if data else self._buffer + data = data if data is not None else self._buffer self._header['uid'] = byte2int(data[0]) func_code = byte2int(data[1]) pdu_class = self.decoder.lookupPduClass(func_code) size = pdu_class.calculateRtuFrameSize(data) self._header['len'] = size + + if len(data) < size: + # crc yet not available + raise IndexError self._header['crc'] = data[size - 2:size] def addToFrame(self, message): diff --git a/pymodbus/repl/client/helper.py b/pymodbus/repl/client/helper.py index 38a29e9df..eb7ede644 100644 --- a/pymodbus/repl/client/helper.py +++ b/pymodbus/repl/client/helper.py @@ -160,7 +160,7 @@ def get_meta(self, cmd): def __str__(self): if self.doc: - return "Command {0:>50}{:<20}".format(self.name, self.doc) + return "Command {:>50}{:<20}".format(self.name, self.doc) return "Command {}".format(self.name) diff --git a/pymodbus/repl/server/cli.py b/pymodbus/repl/server/cli.py index 6e1c0db17..26a8c5e52 100644 --- a/pymodbus/repl/server/cli.py +++ b/pymodbus/repl/server/cli.py @@ -31,8 +31,9 @@ BOTTOM_TOOLBAR = HTML('(MODBUS SERVER) Type "help" ' 'for list of available commands') -COMMAND_ARGS = ["response_type", "error_code", "delay_by", "clear_after"] -RESPONSE_TYPES = ["normal", "error", "delayed"] +COMMAND_ARGS = ["response_type", "error_code", "delay_by", + "clear_after", "data_len"] +RESPONSE_TYPES = ["normal", "error", "delayed", "empty", "stray"] COMMANDS = { "manipulator": { "response_type": None, @@ -155,7 +156,8 @@ async def interactive_shell(server): "type request - {}".format(value)) warning("Choose from {}".format(RESPONSE_TYPES)) valid = False - elif arg in ["error_code", "delay_by"]: + elif arg in ["error_code", "delay_by", + "clear_after", "data_len"]: try: value = int(value) except ValueError: @@ -166,7 +168,8 @@ async def interactive_shell(server): if valid: val_dict[arg] = value if val_dict: - server.manipulator_config = val_dict + server.update_manipulator_config(val_dict) + # server.manipulator_config = val_dict # result = await run_command(tester, *command) except (EOFError, KeyboardInterrupt): diff --git a/pymodbus/server/async_io.py b/pymodbus/server/async_io.py index c4a7e2836..c6e5df1c8 100755 --- a/pymodbus/server/async_io.py +++ b/pymodbus/server/async_io.py @@ -223,20 +223,28 @@ def execute(self, request, *addr): if not broadcast: response.transaction_id = request.transaction_id response.unit_id = request.unit_id + skip_encoding = False if self.server.response_manipulator: - response = self.server.response_manipulator(response) - self.send(response, *addr) + response, skip_encoding = self.server.response_manipulator(response) + self.send(response, *addr, skip_encoding=skip_encoding) - def send(self, message, *addr): - if message.should_respond: - # self.server.control.Counter.BusMessage += 1 - pdu = self.framer.buildPacket(message) + def send(self, message, *addr, **kwargs): + def __send(msg, *addr): if _logger.isEnabledFor(logging.DEBUG): - _logger.debug('send: [%s]- %s' % (message, b2a_hex(pdu))) + _logger.debug('send: [%s]- %s' % (message, b2a_hex(msg))) if addr == (None,): - self._send_(pdu) + self._send_(msg) else: - self._send_(pdu, *addr) + self._send_(msg, *addr) + skip_encoding = kwargs.get("skip_encoding", False) + if skip_encoding: + __send(message, *addr) + elif message.should_respond: + # self.server.control.Counter.BusMessage += 1 + pdu = self.framer.buildPacket(message) + __send(pdu, *addr) + else: + _logger.debug("Skipping sending response!!") # ----------------------------------------------------------------------- # # Derived class implementations diff --git a/pymodbus/server/reactive/main.py b/pymodbus/server/reactive/main.py index 50ed50447..7e1a30a63 100644 --- a/pymodbus/server/reactive/main.py +++ b/pymodbus/server/reactive/main.py @@ -2,10 +2,12 @@ Copyright (c) 2020 by RiptideIO All rights reserved. """ +import os import asyncio import time import random import logging +from pymodbus.version import version as pymodbus_version from pymodbus.compat import IS_PYTHON3, PYTHON_VERSION from pymodbus.pdu import ExceptionResponse, ModbusExceptions from pymodbus.datastore.store import (ModbusSparseDataBlock, @@ -15,7 +17,7 @@ if not IS_PYTHON3 or PYTHON_VERSION < (3, 6): print(f"You are running {PYTHON_VERSION}." - "Reactive server requires python3.6 or above".PYTHON_VERSION) + "Reactive server requires python3.6 or above") exit() @@ -102,6 +104,7 @@ def __init__(self, host, port, modbus_server, loop=None): self._modbus_server = modbus_server self._loop = loop self._add_routes() + self._counter = 0 self._modbus_server.response_manipulator = self.manipulate_response self._manipulator_config = dict(**DEFAULT_MANIPULATOR) self._web_app.on_startup.append(self.start_modbus_server) @@ -132,9 +135,18 @@ async def start_modbus_server(self, app): """ try: if isinstance(self._modbus_server, ModbusSerialServer): - app["modbus_serial_server"] = asyncio.create_task( - self._modbus_server.start()) - app["modbus_server"] = asyncio.create_task(self._modbus_server.serve_forever()) + if hasattr(asyncio, "create_task"): + app["modbus_serial_server"] = asyncio.create_task( + self._modbus_server.start()) + app["modbus_server"] = asyncio.create_task( + self._modbus_server.serve_forever()) + else: + app["modbus_serial_server"] = asyncio.ensure_future( + self._modbus_server.start() + ) + app["modbus_server"] = asyncio.ensure_future( + self._modbus_server.serve_forever()) + logger.info("Modbus server started") except Exception as e: logger.error("Error starting modbus server") @@ -157,7 +169,7 @@ async def _response_manipulator(self, request): """ POST request Handler for response manipulation end point Payload is a dict with following fields - :response_type : One among (normal, delayed, error, empty) + :response_type : One among (normal, delayed, error, empty, stray) :error_code: Modbus error code for error response :delay_by: Delay sending response by seconds @@ -168,15 +180,31 @@ async def _response_manipulator(self, request): self._manipulator_config.update(data) return web.json_response(data=data) + def update_manipulator_config(self, config): + """ + Updates manipulator config. Resets previous counters + :param config: Manipulator config (dict) + :return: + """ + self._counter = 0 + self._manipulator_config = config + def manipulate_response(self, response): """ Manipulates the actual response according to the required error state. :param response: Modbus response object :return: Modbus response """ + skip_encoding = False if not self._manipulator_config: return response else: + clear_after = self._manipulator_config.get("clear_after") + if clear_after and self._counter > clear_after: + logger.info("Resetting manipulator" + " after {} responses".format(clear_after)) + self.update_manipulator_config(dict(DEFAULT_MANIPULATOR)) + return response response_type = self._manipulator_config.get("response_type") if response_type == "error": error_code = self._manipulator_config.get("error_code") @@ -186,15 +214,28 @@ def manipulate_response(self, response): err_response.transaction_id = response.transaction_id err_response.unit_id = response.unit_id response = err_response + self._counter += 1 elif response_type == "delayed": delay_by = self._manipulator_config.get("delay_by") logger.warning( "Delaying response by {}s for " "all incoming requests".format(delay_by)) time.sleep(delay_by) + self._counter += 1 elif response_type == "empty": logger.warning("Sending empty response") - return response + self._counter += 1 + response.should_respond = False + elif response_type == "stray": + data_len = self._manipulator_config.get("data_len", 10) + if data_len <= 0: + logger.warning(f"Invalid data_len {data_len}. " + f"Using default lenght 10") + data_len = 10 + response = os.urandom(data_len) + self._counter += 1 + skip_encoding = True + return response, skip_encoding def run(self): """ @@ -225,7 +266,7 @@ def create_identity(cls, vendor="Pymodbus", product_code="PM", vendor_url='http://github.com/riptideio/pymodbus/', product_name="Pymodbus Server", model_name="Reactive Server", - version="2.5.0"): + version=pymodbus_version.short()): """ Create modbus identity :param vendor: @@ -324,3 +365,4 @@ def factory(cls, server, framer=None, context=None, unit=1, single=False, **kwargs) return ReactiveServer(host, web_port, server, loop) +# __END__ diff --git a/pymodbus/transaction.py b/pymodbus/transaction.py index 9f09477ba..e04b7ea66 100644 --- a/pymodbus/transaction.py +++ b/pymodbus/transaction.py @@ -153,7 +153,6 @@ def execute(self, request): self._transact(request, None, broadcast=True) response = b'Broadcast write sent - no response expected' else: - invalid_response = False expected_response_length = None if not isinstance(self.client.framer, ModbusSocketFramer): if hasattr(request, "get_response_pdu_size"): @@ -171,54 +170,38 @@ def execute(self, request): full = True if not expected_response_length: expected_response_length = Defaults.ReadSize - # retries += 1 + response, last_exception = self._transact( + request, + expected_response_length, + full=full, + broadcast=broadcast + ) while retries > 0: - response, last_exception = self._transact( - request, - expected_response_length, - full=full, - broadcast=broadcast - ) valid_response = self._validate_response( request, response, expected_response_length ) - if not response and request.unit_id \ - not in self._no_response_devices: - self._no_response_devices.append(request.unit_id) - elif request.unit_id in self._no_response_devices and response: - self._no_response_devices.remove(request.unit_id) - if not response and self.retry_on_empty: - _logger.debug("Retry on empty - {}".format(retries)) - retries -= 1 - _logger.debug("Changing transaction state from " - "'WAITING_FOR_REPLY' to 'RETRYING'") - self.client.state = ModbusTransactionState.RETRYING - continue - elif not response: - break - mbap = self.client.framer.decode_data(response) - if mbap.get('unit') == request.unit_id: - break - if ('length' in mbap and expected_response_length and - mbap.get('length') == expected_response_length and - mbap.get('fcode') == request.function_code): + if valid_response: + if request.unit_id in self._no_response_devices and response: + self._no_response_devices.remove(request.unit_id) + _logger.debug("Got response!!!") break else: - invalid_response = True - if invalid_response and not self.retry_on_invalid: - break - _logger.debug("Retry on invalid - {}".format(retries)) - if hasattr(self.client, "state"): - _logger.debug("RESETTING Transaction " - "state to 'RETRY' for retry") - self.client.state = ModbusTransactionState.RETRYING - if self.backoff: - delay = 2 ** (self.retries - retries) * self.backoff - time.sleep(delay) - _logger.debug("Sleeping {}".format(delay)) - full = False - broadcast = False - retries -= 1 + if not response: + if request.unit_id not in self._no_response_devices: + self._no_response_devices.append(request.unit_id) + if self.retry_on_empty: + response, last_exception = self._retry_transaction(retries, "empty", request, expected_response_length, full=full) + retries -= 1 + else: + # No response received and retries not enabled + break + else: + if self.retry_on_invalid: + response, last_exception = self._retry_transaction(retries, "invalid", request, expected_response_length, full=full) + retries -= 1 + else: + break + # full = False addTransaction = partial(self.addTransaction, tid=request.transaction_id) self.client.framer.processIncomingPacket(response, @@ -234,6 +217,7 @@ def execute(self, request): "/Unable to decode response") response = ModbusIOException(last_exception, request.function_code) + self.client.close() if hasattr(self.client, "state"): _logger.debug("Changing transaction state from " "'PROCESSING REPLY' to " @@ -244,12 +228,22 @@ def execute(self, request): return response except ModbusIOException as ex: # Handle decode errors in processIncomingPacket method + self.client.close() _logger.exception(ex) self.client.close() self.client.state = ModbusTransactionState.TRANSACTION_COMPLETE return ex - def _retry(self, packet, response_length, full=False): + def _retry_transaction(self, retries, reason, + packet, response_length, full=False): + _logger.debug("Retry on {} response - {}".format(reason, retries)) + _logger.debug("Changing transaction state from " + "'WAITING_FOR_REPLY' to 'RETRYING'") + self.client.state = ModbusTransactionState.RETRYING + if self.backoff: + delay = 2 ** (self.retries - retries) * self.backoff + time.sleep(delay) + _logger.debug("Sleeping {}".format(delay)) self.client.connect() in_waiting = self.client._in_waiting() if in_waiting: @@ -327,9 +321,10 @@ def _recv(self, expected_response_length, full): read_min = self.client.framer.recvPacket(min_size) if len(read_min) != min_size: + msg_start = "Incomplete message" if read_min else "No response" raise InvalidMessageReceivedException( - "Incomplete message received, expected at least %d bytes " - "(%d received)" % (min_size, len(read_min)) + "%s received, expected at least %d bytes " + "(%d received)" % (msg_start, min_size, len(read_min)) ) if read_min: if isinstance(self.client.framer, ModbusSocketFramer): @@ -364,9 +359,14 @@ def _recv(self, expected_response_length, full): result = read_min + result actual = len(result) if total is not None and actual != total: - _logger.debug("Incomplete message received, " + msg_start = "Incomplete message" if actual else "No response" + _logger.debug("{} received, " "Expected {} bytes Recieved " - "{} bytes !!!!".format(total, actual)) + "{} bytes !!!!".format(msg_start, total, actual)) + elif actual == 0: + # If actual == 0 and total is not None then the above + # should be triggered, so total must be None here + _logger.debug("No response received to unbounded read !!!!") if self.client.state != ModbusTransactionState.PROCESSING_REPLY: _logger.debug("Changing transaction state from " "'WAITING FOR REPLY' to 'PROCESSING REPLY'") diff --git a/pymodbus/utilities.py b/pymodbus/utilities.py index 1aebb6de4..9e31d0974 100644 --- a/pymodbus/utilities.py +++ b/pymodbus/utilities.py @@ -245,7 +245,10 @@ def hexlify_packets(packet): """ if not packet: return '' - return " ".join([hex(byte2int(x)) for x in packet]) + if IS_PYTHON3: + return " ".join([hex(byte2int(x)) for x in packet]) + else: + return u" ".join([hex(byte2int(x)) for x in packet]) # --------------------------------------------------------------------------- # # Exported symbols # --------------------------------------------------------------------------- # diff --git a/pymodbus/version.py b/pymodbus/version.py index 74b2497c7..59e0c5d40 100644 --- a/pymodbus/version.py +++ b/pymodbus/version.py @@ -41,7 +41,7 @@ def __str__(self): return '[%s, version %s]' % (self.package, self.short()) -version = Version('pymodbus', 2, 5, 0, "rc3") +version = Version('pymodbus', 2, 5, 0) version.__name__ = 'pymodbus' # fix epydoc error diff --git a/requirements-coverage.txt b/requirements-coverage.txt new file mode 100644 index 000000000..9808587c9 --- /dev/null +++ b/requirements-coverage.txt @@ -0,0 +1 @@ +coverage >= 4.2 diff --git a/requirements-docs.txt b/requirements-docs.txt index f1165c9c6..fc6cdda47 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -9,9 +9,9 @@ recommonmark>=0.4.0 Sphinx>=1.6.5 sphinx-rtd-theme>=0.2.4 SQLAlchemy>=1.1.15 # Required to parse some files -tornado>=4.5.3 # Required to parse some files +tornado==4.5.3 # Required to parse some files Twisted>=17.1.0 # Required to parse some files prompt_toolkit>=2.0.4 click>=7.0 -m2r>=0.2.0 +m2r2>=0.2.0 diff --git a/requirements-tests.txt b/requirements-tests.txt index eba656a38..5eca1922c 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -1,6 +1,6 @@ bcrypt>=3.1.6 capturer >= 2.2 -coverage >= 4.2 +-r requirements-coverage.txt cryptography>= 2.3 mock >= 1.0.1 pyserial-asyncio>=0.4.0;python_version>="3.4" @@ -14,6 +14,6 @@ sqlalchemy>=1.1.15 #wsgiref>=0.1.2 verboselogs >= 1.5 tornado==4.5.3 -Twisted>=20.3.0 +Twisted[serial]>=20.3.0 zope.interface>=4.4.0 asynctest>=0.10.0 diff --git a/scripts/travis.sh b/scripts/travis.sh deleted file mode 100755 index 8f4338270..000000000 --- a/scripts/travis.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash -e -set -x -if [ "$TRAVIS_OS_NAME" = osx ]; then - VIRTUAL_ENV="$HOME/.virtualenvs/python2.7" - if [ ! -x "$VIRTUAL_ENV/bin/python" ]; then - virtualenv "$VIRTUAL_ENV" - fi - source "$VIRTUAL_ENV/bin/activate" -fi - -eval "$@" diff --git a/setup.py b/setup.py index 50da034be..d20e1f572 100644 --- a/setup.py +++ b/setup.py @@ -53,6 +53,13 @@ """, classifiers=[ 'Development Status :: 4 - Beta', + "Programming Language :: Python :: 2", + "Programming Language :: Python :: 2.7", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.6", + "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", 'Environment :: Console', 'Environment :: X11 Applications :: GTK', 'Framework :: Twisted', @@ -77,6 +84,7 @@ platforms=['Linux', 'Mac OS X', 'Win'], include_package_data=True, zip_safe=True, + python_requires='>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*', install_requires=install_requires, extras_require={ 'quality': [ @@ -89,7 +97,7 @@ 'sphinx_rtd_theme', 'humanfriendly'], 'twisted': [ - 'twisted >= 20.3.0', + 'twisted[serial] >= 20.3.0', 'pyasn1 >= 0.1.4', ], 'tornado': [ diff --git a/test/conftest.py b/test/conftest.py index 748fd4b7a..932e8124c 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,3 +1,8 @@ -from pymodbus.compat import IS_PYTHON3, PYTHON_VERSION -if not IS_PYTHON3 or IS_PYTHON3 and PYTHON_VERSION.minor < 7: - collect_ignore = ["test_server_asyncio.py"] +from pymodbus.compat import PYTHON_VERSION +if PYTHON_VERSION < (3,): + # These files use syntax introduced between Python 2 and our lowest + # supported Python 3 version. We just won't run these tests in Python 2. + collect_ignore = [ + "test_client_async_asyncio.py", + "test_server_asyncio.py", + ] diff --git a/test/test_client_async_asyncio.py b/test/test_client_async_asyncio.py index 42455f53c..a666ebbca 100644 --- a/test/test_client_async_asyncio.py +++ b/test/test_client_async_asyncio.py @@ -50,10 +50,10 @@ def test_protocol_connection_state_propagation_to_factory(self): request = mock.MagicMock() protocol.transaction.addTransaction(request, 1) protocol.connection_lost(mock.sentinel.REASON) - if PYTHON_VERSION.major == 3 and PYTHON_VERSION.minor == 6: - call_args = protocol.raise_future.call_args[0] - else: + if PYTHON_VERSION.major == 3 and PYTHON_VERSION.minor >= 8: call_args = protocol.raise_future.call_args.args + else: + call_args = protocol.raise_future.call_args[0] protocol.raise_future.assert_called_once() assert call_args[0] == request assert isinstance(call_args[1], ConnectionException) diff --git a/test/test_client_sync.py b/test/test_client_sync.py old mode 100644 new mode 100755 index 5853713fe..1b679ed85 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -1,5 +1,7 @@ #!/usr/bin/env python import unittest +from itertools import count +from io import StringIO from pymodbus.compat import IS_PYTHON3 if IS_PYTHON3: # Python 3 @@ -17,6 +19,8 @@ from pymodbus.exceptions import ParameterException from pymodbus.transaction import ModbusAsciiFramer, ModbusRtuFramer from pymodbus.transaction import ModbusBinaryFramer +from pymodbus.transaction import ModbusSocketFramer +from pymodbus.utilities import hexlify_packets # ---------------------------------------------------------------------------# @@ -62,8 +66,8 @@ def testBaseModbusClient(self): client = BaseModbusClient(None) client.transaction = None self.assertRaises(NotImplementedException, lambda: client.connect()) - self.assertRaises(NotImplementedException, lambda: client._send(None)) - self.assertRaises(NotImplementedException, lambda: client._recv(None)) + self.assertRaises(NotImplementedException, lambda: client.send(None)) + 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()) @@ -71,6 +75,20 @@ def testBaseModbusClient(self): client.close() client.__exit__(0, 0, 0) + # Test information methods + client.last_frame_end = 2 + client.silent_interval = 2 + self.assertEqual(4, client.idle_time()) + client.last_frame_end = None + self.assertEqual(0, client.idle_time()) + + # Test debug/trace/_dump methods + self.assertEqual(False, client.debug_enabled()) + writable = StringIO() + client.trace(writable) + client._dump(b'\x00\x01\x02', None) + self.assertEqual(hexlify_packets(b'\x00\x01\x02'), writable.getvalue()) + # a successful execute client.connect = lambda: True client.transaction = Mock(**{'execute.return_value': True}) @@ -133,6 +151,11 @@ def settimeout(self, *a, **kwa): client = ModbusUdpClient() self.assertFalse(client.connect()) + def testUdpClientIsSocketOpen(self): + ''' Test the udp client is_socket_open method''' + client = ModbusUdpClient() + self.assertTrue(client.is_socket_open()) + def testUdpClientSend(self): ''' Test the udp client send method''' client = ModbusUdpClient() @@ -203,6 +226,11 @@ def testTcpClientConnect(self): client = ModbusTcpClient() self.assertFalse(client.connect()) + def testTcpClientIsSocketOpen(self): + ''' Test the tcp client is_socket_open method''' + client = ModbusTcpClient() + self.assertFalse(client.is_socket_open()) + def testTcpClientSend(self): ''' Test the tcp client send method''' client = ModbusTcpClient() @@ -212,11 +240,13 @@ def testTcpClientSend(self): self.assertEqual(0, client._send(None)) self.assertEqual(4, client._send('1234')) + @patch('pymodbus.client.sync.time') @patch('pymodbus.client.sync.select') - def testTcpClientRecv(self, mock_select): + def testTcpClientRecv(self, mock_select, mock_time): ''' Test the tcp client receive method''' mock_select.select.return_value = [True] + mock_time.time.side_effect = count() client = ModbusTcpClient() self.assertRaises(ConnectionException, lambda: client._recv(1024)) @@ -227,7 +257,7 @@ def testTcpClientRecv(self, mock_select): mock_socket = MagicMock() mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02']) client.socket = mock_socket - client.timeout = 1 + client.timeout = 3 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)) @@ -237,7 +267,16 @@ def testTcpClientRecv(self, mock_select): mock_select.select.return_value = [True] self.assertIn(b'\x00', client._recv(None)) - def testSerialClientRpr(self): + mock_socket = MagicMock() + mock_socket.recv.return_value = b'' + client.socket = mock_socket + self.assertRaises(ConnectionException, lambda: client._recv(1024)) + + mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02', b'']) + client.socket = mock_socket + self.assertEqual(b'\x00\x01\x02', client._recv(1024)) + + def testTcpClientRpr(self): client = ModbusTcpClient() rep = "<{} at {} socket={}, ipaddr={}, port={}, timeout={}>".format( client.__class__.__name__, hex(id(client)), client.socket, @@ -309,19 +348,25 @@ def testTlsClientSend(self): self.assertEqual(0, client._send(None)) self.assertEqual(4, client._send('1234')) - def testTlsClientRecv(self): + @patch('pymodbus.client.sync.time') + def testTlsClientRecv(self, mock_time): ''' Test the tls client receive method''' client = ModbusTlsClient() self.assertRaises(ConnectionException, lambda: client._recv(1024)) + mock_time.time.side_effect = count() + client.socket = mockSocket() self.assertEqual(b'', client._recv(0)) self.assertEqual(b'\x00' * 4, client._recv(4)) + client.timeout = 2 + self.assertIn(b'\x00', client._recv(None)) + mock_socket = MagicMock() mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02']) client.socket = mock_socket - client.timeout = 1 + client.timeout = 3 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)) @@ -356,6 +401,8 @@ def testSyncSerialClientInstantiation(self): ModbusRtuFramer)) self.assertTrue(isinstance(ModbusSerialClient(method='binary').framer, ModbusBinaryFramer)) + self.assertTrue(isinstance(ModbusSerialClient(method='socket').framer, + ModbusSocketFramer)) self.assertRaises(ParameterException, lambda: ModbusSerialClient(method='something')) @@ -386,6 +433,12 @@ def testBasicSyncSerialClient(self, mock_serial): self.assertTrue(client.connect()) client.close() + # rtu connect/disconnect + rtu_client = ModbusSerialClient(method='rtu', strict=True) + self.assertTrue(rtu_client.connect()) + self.assertEqual(rtu_client.socket.interCharTimeout, rtu_client.inter_char_timeout) + rtu_client.close() + # already closed socket client.socket = False client.close() @@ -404,6 +457,14 @@ def testSerialClientConnect(self): client = ModbusSerialClient() self.assertFalse(client.connect()) + @patch("serial.Serial") + def testSerialClientIsSocketOpen(self, mock_serial): + ''' Test the serial client is_socket_open method''' + client = ModbusSerialClient() + self.assertFalse(client.is_socket_open()) + client.socket = mock_serial + self.assertTrue(client.is_socket_open()) + @patch("serial.Serial") def testSerialClientSend(self, mock_serial): ''' Test the serial client send method''' @@ -446,6 +507,8 @@ def testSerialClientRecv(self): self.assertEqual(b'', client._recv(None)) client.socket.timeout = 0 self.assertEqual(b'', client._recv(0)) + client.timeout = None + self.assertEqual(b'', client._recv(None)) def testSerialClientRepr(self): client = ModbusSerialClient() diff --git a/test/test_client_sync_diag.py b/test/test_client_sync_diag.py new file mode 100755 index 000000000..c32d283e7 --- /dev/null +++ b/test/test_client_sync_diag.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python +import unittest +from itertools import count +from pymodbus.compat import IS_PYTHON3 + +if IS_PYTHON3: # Python 3 + from unittest.mock import patch, Mock, MagicMock +else: # Python 2 + from mock import patch, Mock, MagicMock +import socket + +from pymodbus.client.sync_diag import ModbusTcpDiagClient, get_client +from pymodbus.exceptions import ConnectionException, NotImplementedException +from pymodbus.exceptions import ParameterException +from test.test_client_sync import mockSocket + + +# ---------------------------------------------------------------------------# +# Fixture +# ---------------------------------------------------------------------------# +class SynchronousDiagnosticClientTest(unittest.TestCase): + ''' + This is the unittest for the pymodbus.client.sync_diag module. It is + a copy of parts of the test for the TCP class in the pymodbus.client.sync + module, as it should operate identically and only log some additional + lines. + ''' + + # -----------------------------------------------------------------------# + # Test TCP Diagnostic Client + # -----------------------------------------------------------------------# + + def testSyncTcpDiagClientInstantiation(self): + client = get_client() + self.assertNotEqual(client, None) + + def testBasicSyncTcpDiagClient(self): + ''' Test the basic methods for the tcp sync diag client''' + + # connect/disconnect + client = ModbusTcpDiagClient() + client.socket = mockSocket() + self.assertTrue(client.connect()) + client.close() + + def testTcpDiagClientConnect(self): + ''' Test the tcp sync diag client connection method''' + with patch.object(socket, 'create_connection') as mock_method: + mock_method.return_value = object() + client = ModbusTcpDiagClient() + self.assertTrue(client.connect()) + + with patch.object(socket, 'create_connection') as mock_method: + mock_method.side_effect = socket.error() + client = ModbusTcpDiagClient() + self.assertFalse(client.connect()) + + @patch('pymodbus.client.sync.time') + @patch('pymodbus.client.sync_diag.time') + @patch('pymodbus.client.sync.select') + def testTcpDiagClientRecv(self, mock_select, mock_diag_time, mock_time): + ''' Test the tcp sync diag client receive method''' + + mock_select.select.return_value = [True] + mock_time.time.side_effect = count() + mock_diag_time.time.side_effect = count() + client = ModbusTcpDiagClient() + self.assertRaises(ConnectionException, lambda: client._recv(1024)) + + client.socket = mockSocket() + # Test logging of non-delayed responses + self.assertIn(b'\x00', client._recv(None)) + self.assertEqual(b'\x00', client._recv(1)) + + # Fool diagnostic logger into thinking we're running late, + # test logging of delayed responses + mock_diag_time.time.side_effect = count(step=3) + 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 = 3 + 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_select.select.return_value = [False] + self.assertEqual(b'', client._recv(2)) + client.socket = mockSocket() + mock_select.select.return_value = [True] + self.assertIn(b'\x00', client._recv(None)) + + mock_socket = MagicMock() + mock_socket.recv.return_value = b'' + client.socket = mock_socket + self.assertRaises(ConnectionException, lambda: client._recv(1024)) + + mock_socket.recv.side_effect = iter([b'\x00', b'\x01', b'\x02', b'']) + client.socket = mock_socket + self.assertEqual(b'\x00\x01\x02', client._recv(1024)) + + def testTcpDiagClientRpr(self): + client = ModbusTcpDiagClient() + rep = "<{} at {} socket={}, ipaddr={}, port={}, timeout={}>".format( + client.__class__.__name__, hex(id(client)), client.socket, + client.host, client.port, client.timeout + ) + self.assertEqual(repr(client), rep) + + +# ---------------------------------------------------------------------------# +# Main +# ---------------------------------------------------------------------------# +if __name__ == "__main__": + unittest.main() diff --git a/test/test_framers.py b/test/test_framers.py index 520409fc6..d70dac3b9 100644 --- a/test/test_framers.py +++ b/test/test_framers.py @@ -8,9 +8,9 @@ from pymodbus.exceptions import ModbusIOException from pymodbus.compat import IS_PYTHON3 if IS_PYTHON3: - from unittest.mock import Mock + from unittest.mock import Mock, patch else: # Python 2 - from mock import Mock + from mock import Mock, patch @pytest.fixture @@ -44,7 +44,7 @@ def test_framer_initialization(framer): assert framer._start == b':' assert framer._end == b"\r\n" elif isinstance(framer, ModbusRtuFramer): - assert framer._header == {'uid': 0x00, 'len': 0, 'crc': '0000'} + assert framer._header == {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'} assert framer._hsize == 0x01 assert framer._end == b'\x0d\x0a' assert framer._min_frame_size == 4 @@ -64,47 +64,78 @@ def test_decode_data(rtu_framer, data): assert decoded == expected -@pytest.mark.parametrize("data", [(b'', False), - (b'\x02\x01\x01\x00Q\xcc', True)]) +@pytest.mark.parametrize("data", [ + (b'', False), + (b'\x02\x01\x01\x00Q\xcc', True), + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD', True), # valid frame + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAC', False), # invalid frame CRC +]) def test_check_frame(rtu_framer, data): data, expected = data rtu_framer._buffer = data assert expected == rtu_framer.checkFrame() -@pytest.mark.parametrize("data", [b'', b'abcd']) -def test_advance_framer(rtu_framer, data): - rtu_framer._buffer = data +@pytest.mark.parametrize("data", [ + (b'', {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'}, b''), + (b'abcd', {'uid': 0x00, 'len': 2, 'crc': b'\x00\x00'}, b'cd'), + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD\x12\x03', # real case, frame size is 11 + {'uid': 0x00, 'len': 11, 'crc': b'\x00\x00'}, b'\x12\x03'), +]) +def test_rtu_advance_framer(rtu_framer, data): + before_buf, before_header, after_buf = data + + rtu_framer._buffer = before_buf + rtu_framer._header = before_header rtu_framer.advanceFrame() - assert rtu_framer._header == {} - assert rtu_framer._buffer == data + assert rtu_framer._header == {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'} + assert rtu_framer._buffer == after_buf @pytest.mark.parametrize("data", [b'', b'abcd']) -def test_reset_framer(rtu_framer, data): +def test_rtu_reset_framer(rtu_framer, data): rtu_framer._buffer = data rtu_framer.resetFrame() - assert rtu_framer._header == {} + assert rtu_framer._header == {'uid': 0x00, 'len': 0, 'crc': b'\x00\x00'} assert rtu_framer._buffer == b'' @pytest.mark.parametrize("data", [ (b'', False), + (b'\x11', False), + (b'\x11\x03', False), (b'\x11\x03\x06', False), (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49', False), (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD', True), - (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD\xAB\xCD', True) + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD\xAB\xCD', True), ]) def test_is_frame_ready(rtu_framer, data): data, expected = data rtu_framer._buffer = data - rtu_framer.advanceFrame() + # rtu_framer.advanceFrame() assert rtu_framer.isFrameReady() == expected -def test_populate_header(rtu_framer): - rtu_framer.populateHeader(b'abcd') - assert rtu_framer._header == {'crc': b'd', 'uid': 97, 'len': 5} +@pytest.mark.parametrize("data", [ + b'', + b'\x11', + b'\x11\x03', + b'\x11\x03\x06', + b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x43', +]) +def test_rtu_populate_header_fail(rtu_framer, data): + with pytest.raises(IndexError): + rtu_framer.populateHeader(data) + + +@pytest.mark.parametrize("data", [ + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD', {'crc': b'\x49\xAD', 'uid': 17, 'len': 11}), + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD\x11\x03', {'crc': b'\x49\xAD', 'uid': 17, 'len': 11}) +]) +def test_rtu_populate_header(rtu_framer, data): + buffer, expected = data + rtu_framer.populateHeader(buffer) + assert rtu_framer._header == expected def test_add_to_frame(rtu_framer): @@ -126,12 +157,26 @@ def test_populate_result(rtu_framer): assert result.unit_id == 255 -@pytest.mark.parametrize('framer', [ascii_framer, rtu_framer, binary_framer]) -def test_process_incoming_packet(framer): - def cb(res): - return res - # data = b'' - # framer.processIncomingPacket(data, cb, unit=1, single=False) +@pytest.mark.parametrize("data", [ + (b'\x11', 17, False, False), # not complete frame + (b'\x11\x03', 17, False, False), # not complete frame + (b'\x11\x03\x06', 17, False, False), # not complete frame + (b'\x11\x03\x06\xAE\x41\x56\x52\x43', 17, False, False), # not complete frame + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40', 17, False, False), # not complete frame + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49', 17, False, False), # not complete frame + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAC', 17, True, False), # bad crc + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD', 17, False, True), # good frame + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD', 16, True, False), # incorrect unit id + (b'\x11\x03\x06\xAE\x41\x56\x52\x43\x40\x49\xAD\x11\x03', 17, False, True), # good frame + part of next frame +]) +def test_rtu_process_incoming_packet(rtu_framer, data): + buffer, units, reset_called, process_called = data + + with patch.object(rtu_framer, '_process') as mock_process, \ + patch.object(rtu_framer, 'resetFrame') as mock_reset: + rtu_framer.processIncomingPacket(buffer, Mock(), units) + assert mock_process.call_count == (1 if process_called else 0) + assert mock_reset.call_count == (1 if reset_called else 0) def test_build_packet(rtu_framer): diff --git a/test/test_transaction.py b/test/test_transaction.py old mode 100644 new mode 100755 index a3c469da1..1b34ca638 --- a/test/test_transaction.py +++ b/test/test_transaction.py @@ -1,6 +1,14 @@ #!/usr/bin/env python import pytest import unittest +from itertools import count +from pymodbus.compat import IS_PYTHON3 + +if IS_PYTHON3: # Python 3 + from unittest.mock import patch, Mock, MagicMock +else: # Python 2 + from mock import patch, Mock, MagicMock + from binascii import a2b_hex from pymodbus.pdu import * from pymodbus.transaction import * @@ -82,7 +90,10 @@ def testCalculateExceptionLength(self): self.assertEqual(self._tm._calculate_exception_length(), exception_length) - def testExecute(self): + @patch('pymodbus.transaction.time') + def testExecute(self, mock_time): + mock_time.time.side_effect = count() + client = MagicMock() client.framer = self._ascii client.framer._buffer = b'deadbeef' @@ -92,10 +103,16 @@ def testExecute(self): client.framer.buildPacket.return_value = b'deadbeef' client.framer.sendPacket = MagicMock() client.framer.sendPacket.return_value = len(b'deadbeef') - + client.framer.decode_data = MagicMock() + client.framer.decode_data.return_value = { + "unit": 1, + "fcode": 222, + "length": 27 + } request = MagicMock() request.get_response_pdu_size.return_value = 10 request.unit_id = 1 + request.function_code = 222 tm = ModbusTransactionManager(client) tm._recv = MagicMock(return_value=b'abcdef') self.assertEqual(tm.retries, 3) @@ -103,6 +120,7 @@ def testExecute(self): # tm._transact = MagicMock() # some response # tm._transact.return_value = (b'abcdef', None) + tm.getTransaction = MagicMock() tm.getTransaction.return_value = 'response' response = tm.execute(request) @@ -123,6 +141,15 @@ def testExecute(self): response = tm.execute(request) self.assertIsInstance(response, ModbusIOException) + # wrong handle_local_echo + tm._recv = MagicMock(side_effect=iter([b'abcdef', b'deadbe', b'123456'])) + client.handle_local_echo = True + tm.retry_on_empty = False + tm.retry_on_invalid = False + self.assertEqual(tm.execute(request).message, + '[Input/Output] Wrong local echo') + client.handle_local_echo = False + # retry on invalid response tm.retry_on_invalid = True tm._recv = MagicMock(side_effect=iter([b'', b'abcdef', b'deadbe', b'123456'])) @@ -136,6 +163,14 @@ def testExecute(self): client.framer.processIncomingPacket.side_effect = MagicMock(side_effect=ModbusIOException()) self.assertIsInstance(tm.execute(request), ModbusIOException) + # Broadcast + client.broadcast_enable = True + request.unit_id = 0 + response = tm.execute(request) + self.assertEqual(response, b'Broadcast write sent - ' + b'no response expected') + + # ----------------------------------------------------------------------- # # Dictionary based transaction manager # ----------------------------------------------------------------------- # @@ -455,7 +490,7 @@ def testRTUFramerTransactionReady(self): msg_parts = [b"\x00\x01\x00", b"\x00\x00\x01\xfc\x1b"] self._rtu.addToFrame(msg_parts[0]) - self.assertTrue(self._rtu.isFrameReady()) + self.assertFalse(self._rtu.isFrameReady()) self.assertFalse(self._rtu.checkFrame()) self._rtu.addToFrame(msg_parts[1]) diff --git a/tox.ini b/tox.ini index 909d6a74d..d84c48458 100644 --- a/tox.ini +++ b/tox.ini @@ -4,12 +4,38 @@ # directory. [tox] -envlist = py27, py35, py36, py37, pypy +envlist = py{27,py27,36,37,38,39,py36,py37} [testenv] deps = -r requirements-tests.txt -commands = py.test {posargs} -setenv = with_gmp=no +commands = + pytest {posargs:--cov=pymodbus/ --cov-report=term-missing --cov-report=xml} +setenv = + with_gmp=no + +[testenv:flake8] +deps = -r requirements-checks.txt +commands = + flake8 + +[testenv:docs] +allowlist_externals = + make +deps = -r requirements-docs.txt +commands = + make -C doc/ clean + make -C doc/ html + +[testenv:combined-coverage] +allowlist_externals = + ls +deps = + -r requirements-coverage.txt + -r requirements.txt +commands = + ls -la coverage_reports + coverage combine coverage_reports + coverage report --fail-under=85 --ignore-errors [flake8] exclude = .tox