From 9951378992889f3f7196a9c21429ed374bfedacf Mon Sep 17 00:00:00 2001 From: Auke Willem Oosterhoff <1565144+OrangeTux@users.noreply.github.com> Date: Wed, 4 Dec 2019 15:42:35 +0100 Subject: [PATCH 01/12] Update dev builds The dev depedencies have been updated to a recent version. Support for Python 3.3 has been removed. Python 3.3 is depricated as of 2017-09-29. Travis doesn't support Python 3.3 builds anymore. On the other side, support for Python 3.7 and Python 3.8 have been added. --- .travis.yml | 3 ++- dev_requirements.txt | 32 ++++++-------------------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index dfdfc00..2f01318 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,10 @@ language: python python: + - 3.8 + - 3.7 - 3.6 - 3.5 - 3.4 - - 3.3 - 2.7 install: diff --git a/dev_requirements.txt b/dev_requirements.txt index 20084bd..acb5d77 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,28 +1,8 @@ -r requirements.txt -alabaster==0.7.10 -attrs==17.4.0 -Babel==2.5.1 -certifi==2017.11.5 -chardet==3.0.4 -coverage==4.4.2 -docutils==0.14 -idna==2.6 -imagesize==0.7.1 -Jinja2==2.10 -MarkupSafe==1.0 -mock==2.0.0 -pbr==3.1.1 -pluggy==0.6.0 -py==1.5.2 -Pygments==2.2.0 -pytest==3.3.2 -pytest-cov==2.5.1 -pytz==2017.3 -requests==2.18.4 -six==1.11.0 -snowballstemmer==1.2.1 -Sphinx==1.6.5 -sphinx-rtd-theme==0.2.4 -sphinxcontrib-websupport==1.0.1 -urllib3==1.22 +pytest==5.3.1;python_version>="3.5" +pytest==4.6.6;python_version<"3.5" +pytest-cov==2.8.1 +Sphinx==2.2.2;python_version>="3.5" +Sphinx==1.8.5;python_version<"3.5" +sphinx-rtd-theme==0.4.3 From bfd074d8ce12a0a9bc1043c9951d142e6e7cd300 Mon Sep 17 00:00:00 2001 From: Auke Willem Oosterhoff <1565144+OrangeTux@users.noreply.github.com> Date: Wed, 4 Dec 2019 15:52:11 +0100 Subject: [PATCH 02/12] Fix use of depricated `inspect.getargspec()` Fixes: #76 --- umodbus/functions.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/umodbus/functions.py b/umodbus/functions.py index fff53ac..b6251fe 100644 --- a/umodbus/functions.py +++ b/umodbus/functions.py @@ -57,8 +57,15 @@ """ from __future__ import division import struct -import inspect import math + +try: + from inspect import getfullargspec +except ImportError: + # inspect.getfullargspec was introduced in Python 3.4. + # Earlier versions have inspect.getargspec. + from inspect import getargspec as getfullargspec + try: from functools import reduce except ImportError: @@ -126,7 +133,7 @@ def create_function_from_response_pdu(resp_pdu, req_pdu=None): function = function_code_to_function_map[function_code] if req_pdu is not None and \ - 'req_pdu' in inspect.getargspec(function.create_from_response_pdu).args: # NOQA + 'req_pdu' in getfullargspec(function.create_from_response_pdu).args: # NOQA return function.create_from_response_pdu(resp_pdu, req_pdu) From 749d69f62fcda09a6d0bc1776f301ee819cb6076 Mon Sep 17 00:00:00 2001 From: Auke Willem Oosterhoff Date: Wed, 4 Dec 2019 16:20:47 +0100 Subject: [PATCH 03/12] Bump to 1.0.3 --- docs/source/changelog.rst | 9 +++++++++ docs/source/conf.py | 6 +++--- setup.py | 7 ++++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index da26c2f..22b036f 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -1,6 +1,15 @@ Changelog ========= +1.0.3 (2019-12-04) +++++++++++++++++++ + +* `#76`_ Remove use of deprecated `inspect.getargspec()` for Python>=3.5. +* Drop support for Python 3.3 +* Add support for Python 3.7 and Python 3.8 + +.. _#76: https://github.com/AdvancedClimateSystems/uModbus/issues/76 + 1.0.2 (2018-05-22) ++++++++++++++++++ diff --git a/docs/source/conf.py b/docs/source/conf.py index d61b2cb..cabdca5 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -51,8 +51,8 @@ # General information about the project. project = 'uModbus' -copyright = '2018, Auke Willem Oosterhoff ' -author = 'Auke Willem Oosterhoff ' +copyright = '2019, Auke Willem Oosterhoff ' +author = 'Auke Willem Oosterhoff ' # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the @@ -61,7 +61,7 @@ # The short X.Y version. version = '1.0' # The full version, including alpha/beta/rc tags. -release = '1.0.0' +release = '1.0.3' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index e3c619b..5e83d3a 100755 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ #!/usr/bin/env python """ uModbus is a pure Python implementation of the Modbus protocol with support -for Python 2.7, 3.3, 3.4, 3.5 and 3.6. +for Python 2.7, 3.4, 3.5, 3.6, 3.7 and 3.8. """ import os @@ -12,7 +12,7 @@ long_description = open(os.path.join(cwd, 'README.rst'), 'r').read() setup(name='uModbus', - version='1.0.2', + version='1.0.3', author='Auke Willem Oosterhoff', author_email='a.oosterhoff@climotion.com', description='Implementation of the Modbus protocol in pure Python.', @@ -35,9 +35,10 @@ 'License :: OSI Approved :: Mozilla Public License 2.0 (MPL 2.0)', 'Operating System :: OS Independent', 'Programming Language :: Python :: 2.7', - 'Programming Language :: Python :: 3.3', 'Programming Language :: Python :: 3.4', 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', + 'Programming Language :: Python :: 3.7', + 'Programming Language :: Python :: 3.8', 'Topic :: Software Development :: Embedded Systems', ]) From 5fdc3b8ffd7c4bff564749170e6a9b0124c692b7 Mon Sep 17 00:00:00 2001 From: Auke Willem Oosterhoff Date: Sun, 7 Jun 2020 10:31:45 +0200 Subject: [PATCH 04/12] Fix 2 error codes The error codes where off by 1. Fixes: #90 --- tests/unit/test_functions.py | 4 ++-- umodbus/exceptions.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_functions.py b/tests/unit/test_functions.py index 3185601..abbbb47 100644 --- a/tests/unit/test_functions.py +++ b/tests/unit/test_functions.py @@ -121,8 +121,8 @@ def test_create_function_from_request_pdu(pdu, cls): (5, AcknowledgeError), (6, ServerDeviceBusyError), (8, MemoryParityError), - (11, GatewayPathUnavailableError), - (12, GatewayTargetDeviceFailedToRespondError), + (10, GatewayPathUnavailableError), + (11, GatewayTargetDeviceFailedToRespondError), ]) def test_create_from_response_pdu_raising_exception(error_code, exception_class): diff --git a/umodbus/exceptions.py b/umodbus/exceptions.py index b57c504..85a9a56 100644 --- a/umodbus/exceptions.py +++ b/umodbus/exceptions.py @@ -74,7 +74,7 @@ def __repr__(self): class GatewayPathUnavailableError(ModbusError): """ The gateway is probably misconfigured or overloaded. """ - error_code = 11 + error_code = 10 def __repr__(self): return self.__doc__ @@ -82,7 +82,7 @@ def __repr__(self): class GatewayTargetDeviceFailedToRespondError(ModbusError): """ Didn't get a response from target device. """ - error_code = 12 + error_code = 11 def __repr__(self): return self.__doc__ From 0faaf904f67327ebdff89b936d5c06a801ad7da7 Mon Sep 17 00:00:00 2001 From: Ryan Govostes Date: Sat, 25 Jul 2020 06:14:41 -0400 Subject: [PATCH 05/12] Explicitly check for missing endpoints (#100) --- umodbus/functions.py | 131 ++++++++++++++++--------------------------- 1 file changed, 49 insertions(+), 82 deletions(-) diff --git a/umodbus/functions.py b/umodbus/functions.py index b6251fe..56dca1b 100644 --- a/umodbus/functions.py +++ b/umodbus/functions.py @@ -363,22 +363,17 @@ def execute(self, slave_id, route_map): :param eindpoint: Instance of modbus.route.Map. :return: Result of call to endpoint. """ - try: - values = [] - - for address in range(self.starting_address, - self.starting_address + self.quantity): - endpoint = route_map.match(slave_id, self.function_code, - address) - values.append(endpoint(slave_id=slave_id, address=address, - function_code=self.function_code)) + values = [] - return values + for address in range(self.starting_address, + self.starting_address + self.quantity): + endpoint = route_map.match(slave_id, self.function_code, address) + if endpoint is None: + raise IllegalDataAddressError() + values.append(endpoint(slave_id=slave_id, address=address, + function_code=self.function_code)) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: - raise IllegalDataAddressError() + return values class ReadDiscreteInputs(ModbusFunction): @@ -576,22 +571,17 @@ def execute(self, slave_id, route_map): :param eindpoint: Instance of modbus.route.Map. :return: Result of call to endpoint. """ - try: - values = [] + values = [] - for address in range(self.starting_address, - self.starting_address + self.quantity): - endpoint = route_map.match(slave_id, self.function_code, - address) - values.append(endpoint(slave_id=slave_id, address=address, - function_code=self.function_code)) - - return values + for address in range(self.starting_address, + self.starting_address + self.quantity): + endpoint = route_map.match(slave_id, self.function_code, address) + if endpoint is None: + raise IllegalDataAddressError() + values.append(endpoint(slave_id=slave_id, address=address, + function_code=self.function_code)) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: - raise IllegalDataAddressError() + return values class ReadHoldingRegisters(ModbusFunction): @@ -756,22 +746,17 @@ def execute(self, slave_id, route_map): :param eindpoint: Instance of modbus.route.Map. :return: Result of call to endpoint. """ - try: - values = [] - - for address in range(self.starting_address, - self.starting_address + self.quantity): - endpoint = route_map.match(slave_id, self.function_code, - address) - values.append(endpoint(slave_id=slave_id, address=address, - function_code=self.function_code)) + values = [] - return values + for address in range(self.starting_address, + self.starting_address + self.quantity): + endpoint = route_map.match(slave_id, self.function_code, address) + if endpoint is None: + raise IllegalDataAddressError() + values.append(endpoint(slave_id=slave_id, address=address, + function_code=self.function_code)) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: - raise IllegalDataAddressError() + return values class ReadInputRegisters(ModbusFunction): @@ -934,22 +919,17 @@ def execute(self, slave_id, route_map): :param eindpoint: Instance of modbus.route.Map. :return: Result of call to endpoint. """ - try: - values = [] + values = [] - for address in range(self.starting_address, - self.starting_address + self.quantity): - endpoint = route_map.match(slave_id, self.function_code, - address) - values.append(endpoint(slave_id=slave_id, address=address, - function_code=self.function_code)) - - return values + for address in range(self.starting_address, + self.starting_address + self.quantity): + endpoint = route_map.match(slave_id, self.function_code, address) + if endpoint is None: + raise IllegalDataAddressError() + values.append(endpoint(slave_id=slave_id, address=address, + function_code=self.function_code)) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: - raise IllegalDataAddressError() + return values class WriteSingleCoil(ModbusFunction): @@ -1101,13 +1081,10 @@ def execute(self, slave_id, route_map): :param eindpoint: Instance of modbus.route.Map. """ endpoint = route_map.match(slave_id, self.function_code, self.address) - try: - endpoint(slave_id=slave_id, address=self.address, value=self.value, - function_code=self.function_code) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: + if endpoint is None: raise IllegalDataAddressError() + endpoint(slave_id=slave_id, address=self.address, value=self.value, + function_code=self.function_code) class WriteSingleRegister(ModbusFunction): @@ -1245,13 +1222,10 @@ def execute(self, slave_id, route_map): :param eindpoint: Instance of modbus.route.Map. """ endpoint = route_map.match(slave_id, self.function_code, self.address) - try: - endpoint(slave_id=slave_id, address=self.address, value=self.value, - function_code=self.function_code) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: + if endpoint is None: raise IllegalDataAddressError() + endpoint(slave_id=slave_id, address=self.address, value=self.value, + function_code=self.function_code) class WriteMultipleCoils(ModbusFunction): @@ -1461,14 +1435,10 @@ def execute(self, slave_id, route_map): for index, value in enumerate(self.values): address = self.starting_address + index endpoint = route_map.match(slave_id, self.function_code, address) - - try: - endpoint(slave_id=slave_id, address=address, value=value, - function_code=self.function_code) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: + if endpoint is None: raise IllegalDataAddressError() + endpoint(slave_id=slave_id, address=address, value=value, + function_code=self.function_code) class WriteMultipleRegisters(ModbusFunction): @@ -1613,14 +1583,11 @@ def execute(self, slave_id, route_map): for index, value in enumerate(self.values): address = self.starting_address + index endpoint = route_map.match(slave_id, self.function_code, address) - - try: - endpoint(slave_id=slave_id, address=address, value=value, - function_code=self.function_code) - # route_map.match() returns None if no match is found. Calling None - # results in TypeError. - except TypeError: + if endpoint is None: raise IllegalDataAddressError() + endpoint(slave_id=slave_id, address=address, value=value, + function_code=self.function_code) + function_code_to_function_map = { READ_COILS: ReadCoils, From f3b438dfaa951d8ff34db01abb5d33ca37275374 Mon Sep 17 00:00:00 2001 From: Ryan Govostes Date: Mon, 27 Jul 2020 03:06:54 -0400 Subject: [PATCH 06/12] Demote hex dump of ADU to debug log (#104) --- umodbus/server/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/umodbus/server/__init__.py b/umodbus/server/__init__.py index 842e455..cf496e8 100644 --- a/umodbus/server/__init__.py +++ b/umodbus/server/__init__.py @@ -103,6 +103,6 @@ def respond(self, response_adu): :param response_adu: A bytearray containing the response of an ADU. """ - log.info('--> {0} - {1}.'.format(self.client_address[0], - hexlify(response_adu))) + log.debug('--> {0} - {1}.'.format(self.client_address[0], + hexlify(response_adu))) self.request.sendall(response_adu) From f125365099212aea1feafb838a38aa4461ca1c47 Mon Sep 17 00:00:00 2001 From: Ryan Govostes Date: Mon, 27 Jul 2020 03:09:35 -0400 Subject: [PATCH 07/12] Change factory methods to class methods (#103) Instead of declaring the create_from_request_pdu and create_from_response_pdu factory methods as @staticmethod, use @classmethod. Then the class is passed to the method as the cls parameter, which can be used to instantiate an instance, rather than hardcoding the class name. This allows the function classes to be subclassed without having to reimplement the factory methods just to change the class name. Also fixes up some comments that were incorrect. --- umodbus/functions.py | 118 ++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/umodbus/functions.py b/umodbus/functions.py index 56dca1b..c4047a3 100644 --- a/umodbus/functions.py +++ b/umodbus/functions.py @@ -278,8 +278,8 @@ def request_pdu(self): return struct.pack('>BHH', self.function_code, self.starting_address, self.quantity) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. :param pdu: A request PDU. @@ -287,7 +287,7 @@ def create_from_request_pdu(pdu): """ _, starting_address, quantity = struct.unpack('>BHH', pdu) - instance = ReadCoils() + instance = cls() instance.starting_address = starting_address instance.quantity = quantity @@ -324,8 +324,8 @@ def create_response_pdu(self, data): fmt = '>BB' + self.format_character * len(bytes_) return struct.pack(fmt, self.function_code, len(bytes_), *bytes_) - @staticmethod - def create_from_response_pdu(resp_pdu, req_pdu): + @classmethod + def create_from_response_pdu(cls, resp_pdu, req_pdu): """ Create instance from response PDU. Response PDU is required together with the quantity of coils read. @@ -334,7 +334,7 @@ def create_from_response_pdu(resp_pdu, req_pdu): :param quantity: Number of coils read. :return: Instance of :class:`ReadCoils`. """ - read_coils = ReadCoils() + read_coils = cls() read_coils.quantity = struct.unpack('>H', req_pdu[-2:])[0] byte_count = struct.unpack('>B', resp_pdu[1:2])[0] @@ -360,7 +360,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. :return: Result of call to endpoint. """ values = [] @@ -486,8 +486,8 @@ def request_pdu(self): return struct.pack('>BHH', self.function_code, self.starting_address, self.quantity) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. :param pdu: A request PDU. @@ -495,7 +495,7 @@ def create_from_request_pdu(pdu): """ _, starting_address, quantity = struct.unpack('>BHH', pdu) - instance = ReadDiscreteInputs() + instance = cls() instance.starting_address = starting_address instance.quantity = quantity @@ -532,8 +532,8 @@ def create_response_pdu(self, data): fmt = '>BB' + self.format_character * len(bytes_) return struct.pack(fmt, self.function_code, len(bytes_), *bytes_) - @staticmethod - def create_from_response_pdu(resp_pdu, req_pdu): + @classmethod + def create_from_response_pdu(cls, resp_pdu, req_pdu): """ Create instance from response PDU. Response PDU is required together with the quantity of inputs read. @@ -542,7 +542,7 @@ def create_from_response_pdu(resp_pdu, req_pdu): :param quantity: Number of inputs read. :return: Instance of :class:`ReadDiscreteInputs`. """ - read_discrete_inputs = ReadDiscreteInputs() + read_discrete_inputs = cls() read_discrete_inputs.quantity = struct.unpack('>H', req_pdu[-2:])[0] byte_count = struct.unpack('>B', resp_pdu[1:2])[0] @@ -568,7 +568,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. :return: Result of call to endpoint. """ values = [] @@ -685,16 +685,16 @@ def request_pdu(self): return struct.pack('>BHH', self.function_code, self.starting_address, self.quantity) - @staticmethod - def create_from_request_pdu(pdu): - + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. + :param pdu: A request PDU. :return: Instance of this class. """ _, starting_address, quantity = struct.unpack('>BHH', pdu) - instance = ReadHoldingRegisters() + instance = cls() instance.starting_address = starting_address instance.quantity = quantity @@ -719,8 +719,8 @@ def create_response_pdu(self, data): return struct.pack(fmt, self.function_code, len(data) * 2, *data) - @staticmethod - def create_from_response_pdu(resp_pdu, req_pdu): + @classmethod + def create_from_response_pdu(cls, resp_pdu, req_pdu): """ Create instance from response PDU. Response PDU is required together with the number of registers read. @@ -729,7 +729,7 @@ def create_from_response_pdu(resp_pdu, req_pdu): :param quantity: Number of coils read. :return: Instance of :class:`ReadCoils`. """ - read_holding_registers = ReadHoldingRegisters() + read_holding_registers = cls() read_holding_registers.quantity = struct.unpack('>H', req_pdu[-2:])[0] read_holding_registers.byte_count = \ struct.unpack('>B', resp_pdu[1:2])[0] @@ -743,7 +743,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. :return: Result of call to endpoint. """ values = [] @@ -860,8 +860,8 @@ def request_pdu(self): return struct.pack('>BHH', self.function_code, self.starting_address, self.quantity) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. :param pdu: A request PDU. @@ -869,7 +869,7 @@ def create_from_request_pdu(pdu): """ _, starting_address, quantity = struct.unpack('>BHH', pdu) - instance = ReadInputRegisters() + instance = cls() instance.starting_address = starting_address instance.quantity = quantity @@ -894,8 +894,8 @@ def create_response_pdu(self, data): return struct.pack(fmt, self.function_code, len(data) * 2, *data) - @staticmethod - def create_from_response_pdu(resp_pdu, req_pdu): + @classmethod + def create_from_response_pdu(cls, resp_pdu, req_pdu): """ Create instance from response PDU. Response PDU is required together with the number of registers read. @@ -904,7 +904,7 @@ def create_from_response_pdu(resp_pdu, req_pdu): :param quantity: Number of coils read. :return: Instance of :class:`ReadCoils`. """ - read_input_registers = ReadInputRegisters() + read_input_registers = cls() read_input_registers.quantity = struct.unpack('>H', req_pdu[-2:])[0] fmt = '>' + (conf.TYPE_CHAR * read_input_registers.quantity) @@ -916,7 +916,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. :return: Result of call to endpoint. """ values = [] @@ -1024,17 +1024,18 @@ def request_pdu(self): return struct.pack('>BHH', self.function_code, self.address, self._value) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. :param pdu: A response PDU. + :return: Instance of this class. """ _, address, value = struct.unpack('>BHH', pdu) value = 1 if value == 0xFF00 else value - instance = WriteSingleCoil() + instance = cls() instance.address = address instance.value = value @@ -1057,14 +1058,14 @@ def create_response_pdu(self): fmt = '>BHH' return struct.pack(fmt, self.function_code, self.address, self._value) - @staticmethod - def create_from_response_pdu(resp_pdu): + @classmethod + def create_from_response_pdu(cls, resp_pdu): """ Create instance from response PDU. :param resp_pdu: Byte array with request PDU. :return: Instance of :class:`WriteSingleCoil`. """ - write_single_coil = WriteSingleCoil() + write_single_coil = cls() address, value = struct.unpack('>HH', resp_pdu[1:5]) value = 1 if value == 0xFF00 else value @@ -1078,7 +1079,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. """ endpoint = route_map.match(slave_id, self.function_code, self.address) if endpoint is None: @@ -1172,16 +1173,17 @@ def request_pdu(self): return struct.pack('>BH' + conf.TYPE_CHAR, self.function_code, self.address, self.value) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. - :param pdu: A response PDU. + :param pdu: A request PDU. + :return: Instance of this class. """ _, address, value = \ struct.unpack('>BH' + conf.MULTI_BIT_VALUE_FORMAT_CHARACTER, pdu) - instance = WriteSingleRegister() + instance = cls() instance.address = address instance.value = value @@ -1199,14 +1201,14 @@ def create_response_pdu(self): fmt = '>BH' + conf.TYPE_CHAR return struct.pack(fmt, self.function_code, self.address, self.value) - @staticmethod - def create_from_response_pdu(resp_pdu): + @classmethod + def create_from_response_pdu(cls, resp_pdu): """ Create instance from response PDU. :param resp_pdu: Byte array with request PDU. :return: Instance of :class:`WriteSingleRegister`. """ - write_single_register = WriteSingleRegister() + write_single_register = cls() address, value = struct.unpack('>H' + conf.TYPE_CHAR, resp_pdu[1:5]) @@ -1219,7 +1221,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. """ endpoint = route_map.match(slave_id, self.function_code, self.address) if endpoint is None: @@ -1322,8 +1324,8 @@ def request_pdu(self): len(self.values), (len(self.values) // 8) + 1, *bytes_) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. This method requires some clarification regarding the unpacking of @@ -1390,7 +1392,7 @@ def create_from_request_pdu(pdu): # and reverse the list. res = res + [int(i) for i in fmt.format(value)][::-1] - instance = WriteMultipleCoils() + instance = cls() instance.starting_address = starting_address instance.quantity = quantity @@ -1415,9 +1417,9 @@ def create_response_pdu(self): return struct.pack('>BHH', self.function_code, self.starting_address, len(self.values)) - @staticmethod - def create_from_response_pdu(resp_pdu): - write_multiple_coils = WriteMultipleCoils() + @classmethod + def create_from_response_pdu(cls, resp_pdu): + write_multiple_coils = cls() starting_address, data = struct.unpack('>HH', resp_pdu[1:5]) @@ -1430,7 +1432,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. """ for index, value in enumerate(self.values): address = self.starting_address + index @@ -1524,8 +1526,8 @@ def request_pdu(self): len(self.values), len(self.values) * 2, *self.values) - @staticmethod - def create_from_request_pdu(pdu): + @classmethod + def create_from_request_pdu(cls, pdu): """ Create instance from request PDU. :param pdu: A request PDU. @@ -1540,7 +1542,7 @@ def create_from_request_pdu(pdu): values = list(struct.unpack(fmt, pdu[6:])) - instance = WriteMultipleRegisters() + instance = cls() instance.starting_address = starting_address instance.values = values @@ -1563,9 +1565,9 @@ def create_response_pdu(self): return struct.pack('>BHH', self.function_code, self.starting_address, len(self.values)) - @staticmethod - def create_from_response_pdu(resp_pdu): - write_multiple_registers = WriteMultipleRegisters() + @classmethod + def create_from_response_pdu(cls, resp_pdu): + write_multiple_registers = cls() starting_address, data = struct.unpack('>HH', resp_pdu[1:5]) @@ -1578,7 +1580,7 @@ def execute(self, slave_id, route_map): """ Execute the Modbus function registered for a route. :param slave_id: Slave id. - :param eindpoint: Instance of modbus.route.Map. + :param route_map: Instance of modbus.route.Map. """ for index, value in enumerate(self.values): address = self.starting_address + index From 82f0872b938f1cc0c659d87bbcdf611d1debbaf2 Mon Sep 17 00:00:00 2001 From: Ryan Govostes Date: Mon, 27 Jul 2020 03:10:15 -0400 Subject: [PATCH 08/12] Remove redundant exception traceback (#102) `log.exception` already includes a traceback, so don't add another. --- umodbus/server/__init__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/umodbus/server/__init__.py b/umodbus/server/__init__.py index cf496e8..03b598b 100644 --- a/umodbus/server/__init__.py +++ b/umodbus/server/__init__.py @@ -48,9 +48,7 @@ def handle(self): response_adu = self.process(mbap_header + request_pdu) self.respond(response_adu) except: - import traceback - log.exception('Error while handling request: {0}.' - .format(traceback.print_exc())) + log.exception('Error while handling request') raise def process(self, request_adu): @@ -91,8 +89,8 @@ def execute_route(self, meta_data, request_pdu): except ModbusError as e: function_code = get_function_code_from_request_pdu(request_pdu) return pack_exception_pdu(function_code, e.error_code) - except Exception as e: - log.exception('Could not handle request: {0}.'.format(e)) + except Exception: + log.exception('Could not handle request') function_code = get_function_code_from_request_pdu(request_pdu) return pack_exception_pdu(function_code, From 63c7679fa3feb4d85e8b4c3ef892c2f264dbf4b2 Mon Sep 17 00:00:00 2001 From: Ryan Govostes Date: Mon, 27 Jul 2020 03:11:04 -0400 Subject: [PATCH 09/12] Allow omitted rule constraints to match any value (#101) The definition of umodbus.server.route gives default values of None to the arguments, suggesting they can be omitted. However, if you actually provide None, it will fail later when evaluating DataRule.match. This change makes it possible to omit any rule constraint to match on any value. --- dev_requirements.txt | 1 + tests/unit/test_route.py | 36 ++++++++++++++++++++++++++++++++++++ umodbus/route.py | 11 +++++------ umodbus/server/__init__.py | 8 +++++--- 4 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 tests/unit/test_route.py diff --git a/dev_requirements.txt b/dev_requirements.txt index acb5d77..b00073a 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,5 +1,6 @@ -r requirements.txt +mock==3.0.5;python_version<"3.3" pytest==5.3.1;python_version>="3.5" pytest==4.6.6;python_version<"3.5" pytest-cov==2.8.1 diff --git a/tests/unit/test_route.py b/tests/unit/test_route.py new file mode 100644 index 0000000..8c052e8 --- /dev/null +++ b/tests/unit/test_route.py @@ -0,0 +1,36 @@ +import pytest + +from umodbus.route import DataRule + + +endpoint = lambda slave_id, function_code, address: 0 + + +def test_basic_route(): + rule = DataRule(endpoint, slave_ids=[1], function_codes=[1], addresses=[1]) + assert rule.match(slave_id=1, function_code=1, address=1) + assert not rule.match(slave_id=0, function_code=1, address=1) + assert not rule.match(slave_id=1, function_code=0, address=1) + assert not rule.match(slave_id=1, function_code=1, address=0) + + +def test_other_iterables(): + # Other iterable types should work, not just lists + rule = DataRule(endpoint, + slave_ids=set([1]), function_codes=[1], addresses=[1]) + assert rule.match(slave_id=1, function_code=1, address=1) + + +def test_wildcard_slave_id(): + rule = DataRule(endpoint, slave_ids=None, function_codes=[1], addresses=[1]) + assert rule.match(slave_id=1, function_code=1, address=1) + + +def test_wildcard_function_code(): + rule = DataRule(endpoint, slave_ids=[1], function_codes=None, addresses=[1]) + assert rule.match(slave_id=1, function_code=1, address=1) + + +def test_wildcard_address(): + rule = DataRule(endpoint, slave_ids=[1], function_codes=[1], addresses=None) + assert rule.match(slave_id=1, function_code=1, address=1) diff --git a/umodbus/route.py b/umodbus/route.py index b95e6f4..3765e87 100644 --- a/umodbus/route.py +++ b/umodbus/route.py @@ -20,9 +20,8 @@ def __init__(self, endpoint, slave_ids, function_codes, addresses): self.addresses = addresses def match(self, slave_id, function_code, address): - if slave_id in self.slave_ids and\ - function_code in self.function_codes and \ - address in self.addresses: - return True - - return False + # A constraint of None matches any value + matches = lambda values, v: values is None or v in values + return matches(self.slave_ids, slave_id) and \ + matches(self.function_codes, function_code) and \ + matches(self.addresses, address) diff --git a/umodbus/server/__init__.py b/umodbus/server/__init__.py index 03b598b..f9d4093 100644 --- a/umodbus/server/__init__.py +++ b/umodbus/server/__init__.py @@ -19,9 +19,11 @@ def route(self, slave_ids=None, function_codes=None, addresses=None): def read_single_bit_values(slave_id, address): return random.choise([0, 1]) - :param slave_ids: A list or set with slave id's. - :param function_codes: A list or set with function codes. - :param addresses: A list or set with addresses. + Any argument can be omitted to match any value. + + :param slave_ids: A list (or iterable) of slave ids. + :param function_codes: A list (or iterable) of function codes. + :param addresses: A list (or iterable) of addresses. """ def inner(f): self.route_map.add_rule(f, slave_ids, function_codes, addresses) From b928ff469a0f05b0d3e363dcad8adf1c6db452b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 27 Aug 2020 22:05:08 +0200 Subject: [PATCH 10/12] Fix byte count when for WriteMultipleCoils (#105) For full bytes, the Byte Count field was calculated too high: 8 // 8 + 1 = 2 while only one byte is needed to represent 8 coils. Offset the values length before the truncating division to correctly round to whole bytes. This is easier than what the standard formulates: N = Quantity of Outputs / 8, if the remainder is different of 0 ==> N = N+1 Note that the actual data bytes were already handled with the correct length. So in case of 8 coils, uModbus would send a Byte Count of 2, but only one data byte before the checksum. --- umodbus/functions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/umodbus/functions.py b/umodbus/functions.py index c4047a3..4db3cec 100644 --- a/umodbus/functions.py +++ b/umodbus/functions.py @@ -1254,8 +1254,8 @@ class WriteMultipleCoils(ModbusFunction): ================ =============== Function code 1 Starting Address 2 - Byte count 1 Quantity 2 + Byte count 1 Value n ================ =============== @@ -1321,7 +1321,7 @@ def request_pdu(self): fmt = '>BHHB' + 'B' * len(bytes_) return struct.pack(fmt, self.function_code, self.starting_address, - len(self.values), (len(self.values) // 8) + 1, + len(self.values), (len(self.values) + 7) // 8, *bytes_) @classmethod From 4fd98bab9fe320260b9bc0b0eb5357f043aa6ec3 Mon Sep 17 00:00:00 2001 From: Auke Willem Oosterhoff Date: Thu, 27 Aug 2020 22:19:05 +0200 Subject: [PATCH 11/12] Bump to 1.0.4 --- docs/source/changelog.rst | 24 ++++++++++++++++++++++++ docs/source/conf.py | 2 +- setup.py | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 22b036f..29e97bf 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -1,6 +1,30 @@ Changelog ========= +1.0.4 (2020-08-27) +++++++++++++++++++ + +**Bugs** + +* `#90`_ Fix error code of 2 Modbus errors. Thanks `@rgov`! +* `#100`_ Improve check for missing routes. Thanks `@rgov`! +* `#101`_ Fix crash if 1 of arguments of `umodbus.server.route` is `None` .Thanks `@rgov`! +* `#105`_ Fix byte count when for WriteMultipleCoils. Thank `@acolomb`! + +**Improvements** + +* `#102`_ Remove redundant exception traceback. Thanks `@rgov`! +* `#103`_ Fix error code of 2 Modbus errors. Thanks `@rgov`! +* `#104`_ Denote hex dump of ADU in debug log. Thanks `@rgov`! + +.. _#90: https://github.com/AdvancedClimateSystems/uModbus/issues/90 +.. _#100: https://github.com/AdvancedClimateSystems/uModbus/issues/100 +.. _#101: https://github.com/AdvancedClimateSystems/uModbus/issues/101 +.. _#102: https://github.com/AdvancedClimateSystems/uModbus/issues/102 +.. _#103: https://github.com/AdvancedClimateSystems/uModbus/issues/103 +.. _#104: https://github.com/AdvancedClimateSystems/uModbus/issues/103 +.. _#105: https://github.com/AdvancedClimateSystems/uModbus/issues/105 + 1.0.3 (2019-12-04) ++++++++++++++++++ diff --git a/docs/source/conf.py b/docs/source/conf.py index cabdca5..097733c 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -61,7 +61,7 @@ # The short X.Y version. version = '1.0' # The full version, including alpha/beta/rc tags. -release = '1.0.3' +release = '1.0.4' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index 5e83d3a..1c36cb5 100755 --- a/setup.py +++ b/setup.py @@ -12,7 +12,7 @@ long_description = open(os.path.join(cwd, 'README.rst'), 'r').read() setup(name='uModbus', - version='1.0.3', + version='1.0.4', author='Auke Willem Oosterhoff', author_email='a.oosterhoff@climotion.com', description='Implementation of the Modbus protocol in pure Python.', From f1128a73e43f565bacedd1ae99d077d7c9c831f3 Mon Sep 17 00:00:00 2001 From: Jose Tiago Macara Coutinho Date: Wed, 11 Nov 2020 20:44:31 +0100 Subject: [PATCH 12/12] Use non privileged tcp port in examples (#109) The server example had port 502 hard coded. This port is a privileged port. Trying to use it without permissions could result in a failure of the example. Port 502 is still the default, but a nice error message is printed to warn the user about the problem. Also a flag has been introduced to change the port. To keep the example client working with the example server code a similar change has been made to the example client. --- README.rst | 47 ++++++++++++++++++++------- scripts/examples/simple_tcp_client.py | 23 ++++++++----- scripts/examples/simple_tcp_server.py | 21 ++++++++++-- 3 files changed, 69 insertions(+), 22 deletions(-) diff --git a/README.rst b/README.rst index 01ae9e5..7ffd213 100644 --- a/README.rst +++ b/README.rst @@ -36,6 +36,7 @@ Creating a Modbus TCP server is easy: import logging from socketserver import TCPServer from collections import defaultdict + from argparse import ArgumentParser from umodbus import conf from umodbus.server.tcp import RequestHandler, get_server @@ -44,23 +45,37 @@ Creating a Modbus TCP server is easy: # Add stream handler to logger 'uModbus'. log_to_stream(level=logging.DEBUG) - # A very simple data store which maps addresss against their values. + # A very simple data store which maps addresses against their values. data_store = defaultdict(int) # Enable values to be signed (default is False). conf.SIGNED_VALUES = True - TCPServer.allow_reuse_address = True - app = get_server(TCPServer, ('localhost', 502), RequestHandler) + # Parse command line arguments + parser = ArgumentParser() + parser.add_argument("-b", "--bind", default="localhost:502") + args = parser.parse_args() + if ":" not in args.bind: + args.bind += ":502" + host, port = args.bind.rsplit(":", 1) + port = int(port) - @app.route(slave_ids=[1], function_codes=[3, 4], addresses=list(range(0, 10))) + TCPServer.allow_reuse_address = True + try: + app = get_server(TCPServer, (host, port), RequestHandler) + except PermissionError: + print("You don't have permission to bind on {}".format(args.bind)) + print("Hint: try with a different port (ex: --bind localhost:50200)") + exit(1) + + @app.route(slave_ids=[1], function_codes=[1, 2], addresses=list(range(0, 10))) def read_data_store(slave_id, function_code, address): """" Return value of address. """ return data_store[address] - @app.route(slave_ids=[1], function_codes=[6, 16], addresses=list(range(0, 10))) + @app.route(slave_ids=[1], function_codes=[5, 15], addresses=list(range(0, 10))) def write_data_store(slave_id, function_code, address, value): """" Set value for address. """ data_store[address] = value @@ -82,7 +97,8 @@ Doing a Modbus request requires even less code: #!/usr/bin/env python # scripts/examples/simple_tcp_client.py - import socket + from argparse import ArgumentParser + from socket import create_connection from umodbus import conf from umodbus.client import tcp @@ -90,18 +106,25 @@ Doing a Modbus request requires even less code: # Enable values to be signed (default is False). conf.SIGNED_VALUES = True - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - sock.connect(('localhost', 502)) + # Parse command line arguments + parser = ArgumentParser() + parser.add_argument("-a", "--address", default="localhost:502") + + args = parser.parse_args() + if ":" not in args.address: + args.address += ":502" + host, port = args.address.rsplit(":", 1) + port = int(port) # Returns a message or Application Data Unit (ADU) specific for doing # Modbus TCP/IP. message = tcp.write_multiple_coils(slave_id=1, starting_address=1, values=[1, 0, 1, 1]) - # Response depends on Modbus function code. This particular returns the - # amount of coils written, in this case it is. - response = tcp.send_message(message, sock) + with create_connection((host, port)) as sock: + # Response depends on Modbus function code. This particular returns the + # amount of coils written, in this case it is. + response = tcp.send_message(message, sock) - sock.close() Features -------- diff --git a/scripts/examples/simple_tcp_client.py b/scripts/examples/simple_tcp_client.py index 9b30dbe..a516354 100755 --- a/scripts/examples/simple_tcp_client.py +++ b/scripts/examples/simple_tcp_client.py @@ -1,6 +1,7 @@ #!/usr/bin/env python # scripts/examples/simple_tcp_client.py -import socket +from argparse import ArgumentParser +from socket import create_connection from umodbus import conf from umodbus.client import tcp @@ -8,15 +9,21 @@ # Enable values to be signed (default is False). conf.SIGNED_VALUES = True -sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) -sock.connect(('localhost', 502)) +# Parse command line arguments +parser = ArgumentParser() +parser.add_argument("-a", "--address", default="localhost:502") + +args = parser.parse_args() +if ":" not in args.address: + args.address += ":502" +host, port = args.address.rsplit(":", 1) +port = int(port) # Returns a message or Application Data Unit (ADU) specific for doing # Modbus TCP/IP. message = tcp.write_multiple_coils(slave_id=1, starting_address=1, values=[1, 0, 1, 1]) -# Response depends on Modbus function code. This particular returns the -# amount of coils written, in this case it is. -response = tcp.send_message(message, sock) - -sock.close() +with create_connection((host, port)) as sock: + # Response depends on Modbus function code. This particular returns the + # amount of coils written, in this case it is. + response = tcp.send_message(message, sock) diff --git a/scripts/examples/simple_tcp_server.py b/scripts/examples/simple_tcp_server.py index c186cd4..147f3e1 100755 --- a/scripts/examples/simple_tcp_server.py +++ b/scripts/examples/simple_tcp_server.py @@ -1,8 +1,9 @@ #!/usr/bin/env python -# scripts/examples/simple_data_store.py +# scripts/examples/simple_tcp_server.py import logging from socketserver import TCPServer from collections import defaultdict +from argparse import ArgumentParser from umodbus import conf from umodbus.server.tcp import RequestHandler, get_server @@ -17,8 +18,23 @@ # Enable values to be signed (default is False). conf.SIGNED_VALUES = True +# Parse command line arguments +parser = ArgumentParser() +parser.add_argument("-b", "--bind", default="localhost:502") + +args = parser.parse_args() +if ":" not in args.bind: + args.bind += ":502" +host, port = args.bind.rsplit(":", 1) +port = int(port) + TCPServer.allow_reuse_address = True -app = get_server(TCPServer, ('localhost', 502), RequestHandler) +try: + app = get_server(TCPServer, (host, port), RequestHandler) +except PermissionError: + print("You don't have permission to bind on {}".format(args.bind)) + print("Hint: try with a different port (ex: --bind localhost:50200)") + exit(1) @app.route(slave_ids=[1], function_codes=[1, 2], addresses=list(range(0, 10))) @@ -32,6 +48,7 @@ def write_data_store(slave_id, function_code, address, value): """" Set value for address. """ data_store[address] = value + if __name__ == '__main__': try: app.serve_forever()