From 8a0d3600d29c7fd07d02cc3791ada4513199571b Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Wed, 6 May 2020 15:00:40 -0500 Subject: [PATCH 01/13] include more clarity on data types --- README.md | 10 ++++++++-- clickplc/__init__.py | 2 +- clickplc/driver.py | 12 +++++++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index afec2ee..f37ab7c 100644 --- a/README.md +++ b/README.md @@ -57,5 +57,11 @@ The entire API is `get` and `set`, and takes a range of inputs: >>> await plc.set('y101', True) # Sets Y101 to true ``` -Currently, only X, Y, C, DS, and DF are supported. I personally haven't needed to -use the other categories, but they are straightforward to add if needed. +Currently, only X, Y, C, DS, and DF are supported: +| x | bool | Input point | +| y | bool | Output point | +| c | bool | (C)ontrol relay | +| df | float | (D)ata register, (f)loating point | +| ds | int16 | (D)ata register, (s)igned int | +I personally haven't needed to use the other categories, but they are +straightforward to add if needed. diff --git a/clickplc/__init__.py b/clickplc/__init__.py index 38a8b44..3186904 100644 --- a/clickplc/__init__.py +++ b/clickplc/__init__.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/python3 """ A Python driver for Koyo ClickPLC ethernet units. diff --git a/clickplc/driver.py b/clickplc/driver.py index 5386b7c..de35992 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -17,7 +17,13 @@ class ClickPLC(AsyncioModbusClient): abstracting corner cases and providing a simple asynchronous interface. """ - supported = ['x', 'y', 'c', 'df', 'ds'] + data_types = { + 'x': 'bool', # Input point + 'y': 'bool', # Output point + 'c': 'bool', # (C)ontrol relay + 'df': 'float', # (D)ata register (f)loating point + 'ds': 'int16', # (D)ata register (s)igned int + } async def get(self, address): """Get variables from the ClickPLC. @@ -47,7 +53,7 @@ async def get(self, address): if end_index is not None and end_index < start_index: raise ValueError("End address must be greater than start address.") - if category not in self.supported: + if category not in self.data_types: raise ValueError("{} currently unsupported.".format(category)) if end is not None and end[:i].lower() != category: raise ValueError("Inter-category ranges are unsupported.") @@ -73,7 +79,7 @@ async def set(self, address, data): i = address.index(next(s for s in address if s.isdigit())) category, index = address[:i].lower(), int(address[i:]) - if category not in self.supported: + if category not in self.data_types: raise ValueError("{} currently unsupported.".format(category)) return await getattr(self, '_set_' + category)(index, data) From 737b9b4ab0e66b7871bbe5ab59ba3d7b80362453 Mon Sep 17 00:00:00 2001 From: jamesjeffryes Date: Mon, 18 May 2020 09:01:28 -0500 Subject: [PATCH 02/13] add type hints --- clickplc/driver.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index 5386b7c..a7ebaf7 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -4,6 +4,8 @@ Distributed under the GNU General Public License v2 Copyright (C) 2019 NuMat Technologies """ +from typing import Union, List + from pymodbus.constants import Endian from pymodbus.payload import BinaryPayloadDecoder, BinaryPayloadBuilder @@ -19,7 +21,7 @@ class ClickPLC(AsyncioModbusClient): supported = ['x', 'y', 'c', 'df', 'ds'] - async def get(self, address): + async def get(self, address: str) -> dict: """Get variables from the ClickPLC. Args: @@ -77,7 +79,7 @@ async def set(self, address, data): raise ValueError("{} currently unsupported.".format(category)) return await getattr(self, '_set_' + category)(index, data) - async def _get_x(self, start, end): + async def _get_x(self, start: int, end: int) -> dict: """Read X addresses. Called by `get`. X entries start at 0 (1 in the Click software's 1-indexed @@ -128,7 +130,7 @@ async def _get_x(self, start, end): current += 1 return output - async def _get_y(self, start, end): + async def _get_y(self, start: int, end: int) -> dict: """Read Y addresses. Called by `get`. Y entries start at 8192 (8193 in the Click software's 1-indexed @@ -179,7 +181,7 @@ async def _get_y(self, start, end): current += 1 return output - async def _get_c(self, start, end): + async def _get_c(self, start: int, end: int) -> Union[dict, bool]: """Read C addresses. Called by `get`. C entries start at 16384 (16385 in the Click software's 1-indexed @@ -206,7 +208,7 @@ async def _get_c(self, start, end): return coils.bits[0] return {'c{:d}'.format(start + i): bit for i, bit in enumerate(coils.bits)} - async def _get_df(self, start, end): + async def _get_df(self, start: int, end: int) -> Union[dict, float]: """Read DF registers. Called by `get`. DF entries start at Modbus address 28672 (28673 in the Click software's @@ -228,7 +230,7 @@ async def _get_df(self, start, end): return decoder.decode_32bit_float() return {f'df{n}': decoder.decode_32bit_float() for n in range(start, end + 1)} - async def _get_ds(self, start, end): + async def _get_ds(self, start: int, end: int) -> Union[dict, int]: """Read DS registers. Called by `get`. DS entries start at Modbus address 0 (1 in the Click software's @@ -249,7 +251,7 @@ async def _get_ds(self, start, end): return decoder.decode_16bit_int() return {f'ds{n}': decoder.decode_16bit_int() for n in range(start, end + 1)} - async def _set_x(self, start, data): + async def _set_x(self, start: int, data: Union[List[bool], bool]): """Set X addresses. Called by `set`. For more information on the quirks of X coils, read the `_get_x` @@ -277,7 +279,7 @@ async def _set_x(self, start, data): else: await self.write_coil(coil, data) - async def _set_y(self, start, data): + async def _set_y(self, start: int, data: Union[List[bool], bool]): """Set Y addresses. Called by `set`. For more information on the quirks of Y coils, read the `_get_y` @@ -305,7 +307,7 @@ async def _set_y(self, start, data): else: await self.write_coil(coil, data) - async def _set_c(self, start, data): + async def _set_c(self, start: int, data: Union[List[bool], bool]): """Set C addresses. Called by `set`. For more information on the quirks of C coils, read the `_get_c` @@ -322,7 +324,7 @@ async def _set_c(self, start, data): else: await self.write_coil(coil, data) - async def _set_df(self, start, data): + async def _set_df(self, start: int, data: Union[List[float], float]): """Set DF registers. Called by `set`. The ClickPLC is little endian, but on registers ("words") instead @@ -351,7 +353,7 @@ def _pack(value): else: await self.write_register(address, _pack(data), skip_encode=True) - async def _set_ds(self, start, data): + async def _set_ds(self, start: int, data: Union[List[int], int]): """Set DS registers. Called by `set`. See _get_ds for more information. From a8653398600884060b9837d12630538506188a5d Mon Sep 17 00:00:00 2001 From: jamesjeffryes Date: Mon, 18 May 2020 14:42:35 -0500 Subject: [PATCH 03/13] Add tests and Azure CI pipeline --- azure-pipelines.yml | 45 +++++++++++++++ clickplc/driver.py | 2 +- clickplc/mock.py | 55 +++++++++++++++++++ clickplc/test_driver.py | 118 ++++++++++++++++++++++++++++++++++++++++ setup.py | 6 ++ 5 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 azure-pipelines.yml create mode 100644 clickplc/mock.py create mode 100644 clickplc/test_driver.py diff --git a/azure-pipelines.yml b/azure-pipelines.yml new file mode 100644 index 0000000..5180e9b --- /dev/null +++ b/azure-pipelines.yml @@ -0,0 +1,45 @@ +# Python Django +# Test a Django project on multiple versions of Python. +# Add steps that analyze code, save build artifacts, deploy, and more: +# https://docs.microsoft.com/azure/devops/pipelines/languages/python + +trigger: +- master + +pool: + vmImage: 'ubuntu-latest' +strategy: + matrix: + Python37: + PYTHON_VERSION: '3.7' + maxParallel: 3 + +steps: +- task: UsePythonVersion@0 + inputs: + versionSpec: '$(PYTHON_VERSION)' + architecture: 'x64' + +- script: | + python -m pip install --upgrade pip + pip install '.[test]' + displayName: 'Install dependencies' + +- script: | + cd clickplc + pytest --junitxml=../reports/test-coverage.xml --cov=. --cov-report=xml + env: + USER_TOKEN: $(USER_TOKEN) + displayName: 'Run tests' + +- task: PublishTestResults@2 + inputs: + testResultsFiles: 'reports/test-coverage.xml' + testRunTitle: '$(Agent.OS) - $(Build.BuildNumber)[$(Agent.JobName)] - Python $(python.version)' + condition: succeededOrFailed() + +- task: PublishCodeCoverageResults@1 + inputs: + codeCoverageTool: Cobertura + summaryFileLocation: '$(System.DefaultWorkingDirectory)/**/coverage.xml' + reportDirectory: '$(System.DefaultWorkingDirectory)/**/htmlcov' \ No newline at end of file diff --git a/clickplc/driver.py b/clickplc/driver.py index a7ebaf7..2f4f1a9 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -359,7 +359,7 @@ async def _set_ds(self, start: int, data: Union[List[int], int]): See _get_ds for more information. """ if start < 1 or start > 4500: - raise ValueError('DS must be in [1, 500]') + raise ValueError('DS must be in [1, 4500]') address = (start - 1) def _pack(value): diff --git a/clickplc/mock.py b/clickplc/mock.py new file mode 100644 index 0000000..ed7a538 --- /dev/null +++ b/clickplc/mock.py @@ -0,0 +1,55 @@ +from collections import defaultdict +from unittest.mock import MagicMock + +from pymodbus.bit_read_message import ReadCoilsResponse, ReadDiscreteInputsResponse +from pymodbus.bit_write_message import WriteSingleCoilResponse, WriteMultipleCoilsResponse +from pymodbus.register_read_message import ReadHoldingRegistersResponse +from pymodbus.register_write_message import WriteMultipleRegistersResponse + +from clickplc.driver import ClickPLC as realClickPLC + + +class AsyncMock(MagicMock): + """Magic mock that works with async methods""" + async def __call__(self, *args, **kwargs): + return super(AsyncMock, self).__call__(*args, **kwargs) + + +class ClickPLC(realClickPLC): + """A version of the driver with the remote communication replaced with local data storage + for testing""" + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.client = AsyncMock() + self._coils = defaultdict(bool) + self._discrete_inputs = defaultdict(bool) + self._registers = defaultdict(bytes) + + async def _request(self, method, *args, **kwargs): + if method == 'read_coils': + address, count = args + return ReadCoilsResponse([self._coils[address + i] for i in range(count)]) + if method == 'read_discrete_inputs': + address, count = args + return ReadDiscreteInputsResponse([self._discrete_inputs[address + i] + for i in range(count)]) + elif method == 'read_holding_registers': + address, count = args + return ReadHoldingRegistersResponse([int.from_bytes(self._registers[address + i], + byteorder='big') + for i in range(count)]) + elif method == 'write_coil': + address, data = args + self._coils[address] = data + return WriteSingleCoilResponse(address, data) + elif method == 'write_coils': + address, data = args + for i, d in enumerate(data): + self._coils[address + i] = d + return WriteMultipleCoilsResponse(address, data) + elif method == 'write_registers': + address, data = args + for i, d in enumerate(data): + self._registers[address + i] = d + return WriteMultipleRegistersResponse(address, data) + return NotImplementedError(f'Unrecognised method: {method}') diff --git a/clickplc/test_driver.py b/clickplc/test_driver.py new file mode 100644 index 0000000..c66de0b --- /dev/null +++ b/clickplc/test_driver.py @@ -0,0 +1,118 @@ +import asyncio + +from unittest import TestCase + +from clickplc.mock import ClickPLC + + +class TestClickPLC(TestCase): + """Test the functionality of the ClickPLC dirve + + Tests use the mocked version of the driver which replaces remote communications + with a local data store""" + + @classmethod + def setUp(cls): + cls.loop = asyncio.get_event_loop() + cls.clickplc = ClickPLC('fake IP') + + def set(self, *args): + return self.loop.run_until_complete(self.clickplc.set(*args)) + + def get(self, *args): + return self.loop.run_until_complete(self.clickplc.get(*args)) + + def bool_roundtrip(self, prefix): + self.set(f'{prefix}2', True) + self.set(f'{prefix}3', [False, True]) + expected = {f'{prefix}001': False, f'{prefix}002': True, f'{prefix}003': False, + f'{prefix}004': True, f'{prefix}005': False} + assert expected == self.get(f'{prefix}1-{prefix}5') + + def test_x_roundtrip(self): + self.bool_roundtrip('x') + + def test_y_roundtrip(self): + self.bool_roundtrip('y') + + def test_c_roundtrip(self): + self.set('c2', True) + self.set('c3', [False, True]) + expected = {'c1': False, 'c2': True, 'c3': False, 'c4': True, 'c5': False} + assert expected == self.get('c1-c5') + + def test_df_roundtrip(self): + self.set('df2', 2.0) + self.set('df3', [3.0, 4.0]) + expected = {'df1': 0.0, 'df2': 2.0, 'df3': 3.0, 'df4': 4.0, 'df5': 0.0} + assert expected == self.get('df1-df5') + + def test_ds_roundtrip(self): + self.set('df2', 2) + self.set('df3', [3, 4]) + expected = {'df1': 0, 'df2': 2, 'df3': 3, 'df4': 4, 'df5': 0} + assert expected == self.get('df1-df5') + + def test_get_error_handling(self): + with self.assertRaisesRegex(ValueError, 'End address must be greater than start address.'): + self.get('c3-c1') + with self.assertRaisesRegex(ValueError, 'foo currently unsupported'): + self.get('foo1') + with self.assertRaisesRegex(ValueError, 'Inter-category ranges are unsupported.'): + self.get('c1-x3') + + def test_set_error_handling(self): + with self.assertRaisesRegex(ValueError, 'foo currently unsupported'): + self.set('foo1', 1) + + def xy_error_handling(self, prefix): + with self.assertRaisesRegex(ValueError, 'address must be \*01-\*16.'): + self.get(f'{prefix}17') + with self.assertRaisesRegex(ValueError, 'address must be in \[001, 816\].'): + self.get(f'{prefix}1001') + with self.assertRaisesRegex(ValueError, 'address must be \*01-\*16.'): + self.get(f'{prefix}1-{prefix}17') + with self.assertRaisesRegex(ValueError, 'address must be in \[001, 816\].'): + self.get(f'{prefix}1-{prefix}1001') + with self.assertRaisesRegex(ValueError, 'address must be \*01-\*16.'): + self.set(f'{prefix}17', True) + with self.assertRaisesRegex(ValueError, 'address must be in \[001, 816\].'): + self.set(f'{prefix}1001', True) + with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): + self.set(f'{prefix}816', [True, True]) + + def test_x_error_handling(self): + self.xy_error_handling("x") + + def test_y_error_handling(self): + self.xy_error_handling("y") + + def test_c_error_handling(self): + with self.assertRaisesRegex(ValueError, 'C start address must be 1-2000.'): + self.get('c2001') + with self.assertRaisesRegex(ValueError, 'C end address must be >start and <2000.'): + self.get('c1-c2001') + with self.assertRaisesRegex(ValueError, 'C start address must be 1-2000.'): + self.set('c2001', True) + with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): + self.set('c2000', [True, True]) + + def test_df_error_handling(self): + with self.assertRaisesRegex(ValueError, 'DF must be in \[1, 500\]'): + self.get('df501') + with self.assertRaisesRegex(ValueError, 'DF end must be in \[1, 500\]'): + self.get('df1-df501') + with self.assertRaisesRegex(ValueError, 'DF must be in \[1, 500\]'): + self.set('df501', 1.0) + with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): + self.set('df500', [1.0, 2.0]) + + def test_ds_error_handling(self): + with self.assertRaisesRegex(ValueError, 'DS must be in \[1, 4500\]'): + self.get('ds4501') + with self.assertRaisesRegex(ValueError, 'DS end must be in \[1, 4500\]'): + self.get('ds1-ds4501') + with self.assertRaisesRegex(ValueError, 'DS must be in \[1, 4500\]'): + self.set('ds4501', 1) + with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): + self.set('ds4500', [1, 2]) diff --git a/setup.py b/setup.py index 5427fdb..851a49f 100644 --- a/setup.py +++ b/setup.py @@ -24,6 +24,12 @@ install_requires=[ 'pymodbus==2.2.0rc1' ], + extras_require={ + 'test': [ + 'pytest', + 'pytest-cov', + ], + }, license='GPLv2', classifiers=[ 'License :: OSI Approved :: GNU General Public License v2 (GPLv2)', From 493f08e4b0178f6cb2b3837e110524eaaf600249 Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Mon, 18 May 2020 18:51:32 -0500 Subject: [PATCH 04/13] type check on set() call --- clickplc/driver.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index de35992..7e50cd0 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -2,8 +2,10 @@ A Python driver for Koyo ClickPLC ethernet units. Distributed under the GNU General Public License v2 -Copyright (C) 2019 NuMat Technologies +Copyright (C) 2020 NuMat Technologies """ +import pydoc +from string import digits from pymodbus.constants import Endian from pymodbus.payload import BinaryPayloadDecoder, BinaryPayloadBuilder @@ -81,6 +83,11 @@ async def set(self, address, data): category, index = address[:i].lower(), int(address[i:]) if category not in self.data_types: raise ValueError("{} currently unsupported.".format(category)) + data_type = self.data_types[category].rstrip(digits) + if isinstance(data, int) and data_type == 'float': + data = float(data) + if not isinstance(data, pydoc.locate(data_type)): + raise ValueError(f"Expected {address} to be a {data_type}.") return await getattr(self, '_set_' + category)(index, data) async def _get_x(self, start, end): From daea9cb5b61cc41357f89bb875ade1e1a0a75605 Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Mon, 18 May 2020 19:07:03 -0500 Subject: [PATCH 05/13] change to f-strings --- clickplc/driver.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index 7e50cd0..fee35bd 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -82,7 +82,7 @@ async def set(self, address, data): i = address.index(next(s for s in address if s.isdigit())) category, index = address[:i].lower(), int(address[i:]) if category not in self.data_types: - raise ValueError("{} currently unsupported.".format(category)) + raise ValueError(f"{category} currently unsupported.") data_type = self.data_types[category].rstrip(digits) if isinstance(data, int) and data_type == 'float': data = float(data) @@ -135,7 +135,7 @@ async def _get_x(self, start, end): if current > end: break elif current % 100 <= 16: - output['x{:03d}'.format(current)] = bit + output[f'x{current:03}'] = bit elif current % 100 == 32: current += 100 - 32 current += 1 @@ -186,7 +186,7 @@ async def _get_y(self, start, end): if current > end: break elif current % 100 <= 16: - output['y{:03d}'.format(current)] = bit + output[f'y{current:03}' = bit elif current % 100 == 32: current += 100 - 32 current += 1 @@ -217,7 +217,7 @@ async def _get_c(self, start, end): coils = await self.read_coils(start_coil, count) if count == 1: return coils.bits[0] - return {'c{:d}'.format(start + i): bit for i, bit in enumerate(coils.bits)} + return {f'c{(start + i}': bit for i, bit in enumerate(coils.bits)} async def _get_df(self, start, end): """Read DF registers. Called by `get`. From 32cc9a9fa6ef6c7b173e1e8d878e2eb2a04ffc73 Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Mon, 18 May 2020 19:16:36 -0500 Subject: [PATCH 06/13] bump version numbers --- README.md | 2 +- setup.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index f37ab7c..8ad3157 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ clickplc ======== -Python ≥3.5 driver and command-line tool for [Koyo Ethernet ClickPLCs](https://www.automationdirect.com/adc/Overview/Catalog/Programmable_Controllers/CLICK_Series_PLCs_(Stackable_Micro_Brick)). +Python ≥3.6 driver and command-line tool for [Koyo Ethernet ClickPLCs](https://www.automationdirect.com/adc/Overview/Catalog/Programmable_Controllers/CLICK_Series_PLCs_(Stackable_Micro_Brick)).

diff --git a/setup.py b/setup.py index 5427fdb..686b681 100644 --- a/setup.py +++ b/setup.py @@ -2,15 +2,15 @@ from platform import python_version from setuptools import setup -if python_version() < '3.5': - raise ImportError("This module requires Python >=3.5") +if python_version() < '3.6': + raise ImportError("This module requires Python >=3.6") with open('README.md', 'r') as in_file: long_description = in_file.read() setup( name='clickplc', - version='0.2.6', + version='0.2.7', description="Python driver for Koyo Ethernet ClickPLCs.", long_description=long_description, long_description_content_type='text/markdown', @@ -22,7 +22,7 @@ 'console_scripts': [('clickplc = clickplc:command_line')] }, install_requires=[ - 'pymodbus==2.2.0rc1' + 'pymodbus==2.2.0' ], license='GPLv2', classifiers=[ @@ -31,9 +31,9 @@ 'Natural Language :: English', 'Programming Language :: Python', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'Programming Language :: Python :: 3.7', + 'Programming Language :: Python :: 3.8', 'Topic :: Scientific/Engineering :: Human Machine Interfaces' ] ) From 8919ec210ec1bfe1a3ff58c8c6aa871a10b93df3 Mon Sep 17 00:00:00 2001 From: Alex <52292902+alexrudd2@users.noreply.github.com> Date: Mon, 18 May 2020 19:26:50 -0500 Subject: [PATCH 07/13] missing bracket --- clickplc/driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index bfee6ed..36b4f37 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -188,7 +188,7 @@ async def _get_y(self, start: int, end: int) -> dict: if current > end: break elif current % 100 <= 16: - output[f'y{current:03}' = bit + output[f'y{current:03}'] = bit elif current % 100 == 32: current += 100 - 32 current += 1 From f2e127f7643a491fcde6236ba02996d16c5d3609 Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Mon, 18 May 2020 19:36:11 -0500 Subject: [PATCH 08/13] missing parenthesis --- clickplc/driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index 36b4f37..00dfdd3 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -219,7 +219,7 @@ async def _get_c(self, start: int, end: int) -> Union[dict, bool]: coils = await self.read_coils(start_coil, count) if count == 1: return coils.bits[0] - return {f'c{(start + i}': bit for i, bit in enumerate(coils.bits)} + return {f'c{(start + i)}': bit for i, bit in enumerate(coils.bits)} async def _get_df(self, start: int, end: int) -> Union[dict, float]: """Read DF registers. Called by `get`. From 966891ed36be8c39329c7351797a8c8ebcaf740b Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Mon, 18 May 2020 21:17:56 -0500 Subject: [PATCH 09/13] allow for data to be a list (fix failed test) --- clickplc/driver.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index 00dfdd3..7e27040 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -86,10 +86,11 @@ async def set(self, address, data): if category not in self.data_types: raise ValueError(f"{category} currently unsupported.") data_type = self.data_types[category].rstrip(digits) - if isinstance(data, int) and data_type == 'float': - data = float(data) - if not isinstance(data, pydoc.locate(data_type)): - raise ValueError(f"Expected {address} to be a {data_type}.") + for datum in data: + if isinstance(datum, int) and data_type == 'float': + datum = float(datum) + if not isinstance(datum, pydoc.locate(data_type)): + raise ValueError(f"Expected {address} as a {data_type}.") return await getattr(self, '_set_' + category)(index, data) async def _get_x(self, start: int, end: int) -> dict: From 9c36465b251f3db67dad5a96c9ac6ecd516ae5d5 Mon Sep 17 00:00:00 2001 From: Alex Ruddick Date: Mon, 18 May 2020 21:40:39 -0500 Subject: [PATCH 10/13] Fix pytest warnings for unescaped backslashes --- clickplc/test_driver.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/clickplc/test_driver.py b/clickplc/test_driver.py index c66de0b..b6b2d7c 100644 --- a/clickplc/test_driver.py +++ b/clickplc/test_driver.py @@ -6,10 +6,11 @@ class TestClickPLC(TestCase): - """Test the functionality of the ClickPLC dirve + """Test the functionality of the ClickPLC driver. Tests use the mocked version of the driver which replaces remote communications - with a local data store""" + with a local data store + """ @classmethod def setUp(cls): @@ -66,17 +67,17 @@ def test_set_error_handling(self): self.set('foo1', 1) def xy_error_handling(self, prefix): - with self.assertRaisesRegex(ValueError, 'address must be \*01-\*16.'): + with self.assertRaisesRegex(ValueError, 'address must be \\*01-\\*16.'): self.get(f'{prefix}17') - with self.assertRaisesRegex(ValueError, 'address must be in \[001, 816\].'): + with self.assertRaisesRegex(ValueError, 'address must be in \\[001, 816\\].'): self.get(f'{prefix}1001') - with self.assertRaisesRegex(ValueError, 'address must be \*01-\*16.'): + with self.assertRaisesRegex(ValueError, 'address must be \\*01-\\*16.'): self.get(f'{prefix}1-{prefix}17') - with self.assertRaisesRegex(ValueError, 'address must be in \[001, 816\].'): + with self.assertRaisesRegex(ValueError, 'address must be in \\[001, 816\\].'): self.get(f'{prefix}1-{prefix}1001') - with self.assertRaisesRegex(ValueError, 'address must be \*01-\*16.'): + with self.assertRaisesRegex(ValueError, 'address must be \\*01-\\*16.'): self.set(f'{prefix}17', True) - with self.assertRaisesRegex(ValueError, 'address must be in \[001, 816\].'): + with self.assertRaisesRegex(ValueError, 'address must be in \\[001, 816\\].'): self.set(f'{prefix}1001', True) with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): self.set(f'{prefix}816', [True, True]) @@ -98,21 +99,21 @@ def test_c_error_handling(self): self.set('c2000', [True, True]) def test_df_error_handling(self): - with self.assertRaisesRegex(ValueError, 'DF must be in \[1, 500\]'): + with self.assertRaisesRegex(ValueError, 'DF must be in \\[1, 500\\]'): self.get('df501') - with self.assertRaisesRegex(ValueError, 'DF end must be in \[1, 500\]'): + with self.assertRaisesRegex(ValueError, 'DF end must be in \\[1, 500\\]'): self.get('df1-df501') - with self.assertRaisesRegex(ValueError, 'DF must be in \[1, 500\]'): + with self.assertRaisesRegex(ValueError, 'DF must be in \\[1, 500\\]'): self.set('df501', 1.0) with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): self.set('df500', [1.0, 2.0]) def test_ds_error_handling(self): - with self.assertRaisesRegex(ValueError, 'DS must be in \[1, 4500\]'): + with self.assertRaisesRegex(ValueError, 'DS must be in \\[1, 4500\\]'): self.get('ds4501') - with self.assertRaisesRegex(ValueError, 'DS end must be in \[1, 4500\]'): + with self.assertRaisesRegex(ValueError, 'DS end must be in \\[1, 4500\\]'): self.get('ds1-ds4501') - with self.assertRaisesRegex(ValueError, 'DS must be in \[1, 4500\]'): + with self.assertRaisesRegex(ValueError, 'DS must be in \\[1, 4500\\]'): self.set('ds4501', 1) with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): self.set('ds4500', [1, 2]) From 2a1f76e79ede87cdaa3cb9cc4e4dfa26f5ddbf06 Mon Sep 17 00:00:00 2001 From: James Jeffryes Date: Tue, 19 May 2020 07:53:37 -0500 Subject: [PATCH 11/13] use pymodbus 2.2.0rc1 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 5979b78..6cebf34 100644 --- a/setup.py +++ b/setup.py @@ -22,7 +22,7 @@ 'console_scripts': [('clickplc = clickplc:command_line')] }, install_requires=[ - 'pymodbus==2.2.0' + 'pymodbus==2.2.0rc1' ], extras_require={ 'test': [ From e371e1593e1ff7f9f2cb62f35ed2fe5b16b31eec Mon Sep 17 00:00:00 2001 From: jamesjeffryes Date: Tue, 19 May 2020 09:39:52 -0500 Subject: [PATCH 12/13] strict type checking --- clickplc/driver.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clickplc/driver.py b/clickplc/driver.py index 7e27040..bc60af8 100644 --- a/clickplc/driver.py +++ b/clickplc/driver.py @@ -87,10 +87,10 @@ async def set(self, address, data): raise ValueError(f"{category} currently unsupported.") data_type = self.data_types[category].rstrip(digits) for datum in data: - if isinstance(datum, int) and data_type == 'float': + if type(datum) == int and data_type == 'float': datum = float(datum) - if not isinstance(datum, pydoc.locate(data_type)): - raise ValueError(f"Expected {address} as a {data_type}.") + if type(datum) != pydoc.locate(data_type): + raise ValueError(f"Expected {address} as a {data_type}.") return await getattr(self, '_set_' + category)(index, data) async def _get_x(self, start: int, end: int) -> dict: From 487dd365b0d3794ba2f06aa2d9fdf7890f5a2edd Mon Sep 17 00:00:00 2001 From: jamesjeffryes Date: Tue, 19 May 2020 09:40:38 -0500 Subject: [PATCH 13/13] update tests and bump minor version --- clickplc/test_driver.py | 250 ++++++++++++++++++++++------------------ setup.py | 3 +- 2 files changed, 139 insertions(+), 114 deletions(-) diff --git a/clickplc/test_driver.py b/clickplc/test_driver.py index b6b2d7c..4a6e553 100644 --- a/clickplc/test_driver.py +++ b/clickplc/test_driver.py @@ -1,119 +1,143 @@ import asyncio -from unittest import TestCase +import pytest from clickplc.mock import ClickPLC -class TestClickPLC(TestCase): - """Test the functionality of the ClickPLC driver. - - Tests use the mocked version of the driver which replaces remote communications - with a local data store - """ - - @classmethod - def setUp(cls): - cls.loop = asyncio.get_event_loop() - cls.clickplc = ClickPLC('fake IP') - - def set(self, *args): - return self.loop.run_until_complete(self.clickplc.set(*args)) - - def get(self, *args): - return self.loop.run_until_complete(self.clickplc.get(*args)) - - def bool_roundtrip(self, prefix): - self.set(f'{prefix}2', True) - self.set(f'{prefix}3', [False, True]) - expected = {f'{prefix}001': False, f'{prefix}002': True, f'{prefix}003': False, - f'{prefix}004': True, f'{prefix}005': False} - assert expected == self.get(f'{prefix}1-{prefix}5') - - def test_x_roundtrip(self): - self.bool_roundtrip('x') - - def test_y_roundtrip(self): - self.bool_roundtrip('y') - - def test_c_roundtrip(self): - self.set('c2', True) - self.set('c3', [False, True]) - expected = {'c1': False, 'c2': True, 'c3': False, 'c4': True, 'c5': False} - assert expected == self.get('c1-c5') - - def test_df_roundtrip(self): - self.set('df2', 2.0) - self.set('df3', [3.0, 4.0]) - expected = {'df1': 0.0, 'df2': 2.0, 'df3': 3.0, 'df4': 4.0, 'df5': 0.0} - assert expected == self.get('df1-df5') - - def test_ds_roundtrip(self): - self.set('df2', 2) - self.set('df3', [3, 4]) - expected = {'df1': 0, 'df2': 2, 'df3': 3, 'df4': 4, 'df5': 0} - assert expected == self.get('df1-df5') - - def test_get_error_handling(self): - with self.assertRaisesRegex(ValueError, 'End address must be greater than start address.'): - self.get('c3-c1') - with self.assertRaisesRegex(ValueError, 'foo currently unsupported'): - self.get('foo1') - with self.assertRaisesRegex(ValueError, 'Inter-category ranges are unsupported.'): - self.get('c1-x3') - - def test_set_error_handling(self): - with self.assertRaisesRegex(ValueError, 'foo currently unsupported'): - self.set('foo1', 1) - - def xy_error_handling(self, prefix): - with self.assertRaisesRegex(ValueError, 'address must be \\*01-\\*16.'): - self.get(f'{prefix}17') - with self.assertRaisesRegex(ValueError, 'address must be in \\[001, 816\\].'): - self.get(f'{prefix}1001') - with self.assertRaisesRegex(ValueError, 'address must be \\*01-\\*16.'): - self.get(f'{prefix}1-{prefix}17') - with self.assertRaisesRegex(ValueError, 'address must be in \\[001, 816\\].'): - self.get(f'{prefix}1-{prefix}1001') - with self.assertRaisesRegex(ValueError, 'address must be \\*01-\\*16.'): - self.set(f'{prefix}17', True) - with self.assertRaisesRegex(ValueError, 'address must be in \\[001, 816\\].'): - self.set(f'{prefix}1001', True) - with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): - self.set(f'{prefix}816', [True, True]) - - def test_x_error_handling(self): - self.xy_error_handling("x") - - def test_y_error_handling(self): - self.xy_error_handling("y") - - def test_c_error_handling(self): - with self.assertRaisesRegex(ValueError, 'C start address must be 1-2000.'): - self.get('c2001') - with self.assertRaisesRegex(ValueError, 'C end address must be >start and <2000.'): - self.get('c1-c2001') - with self.assertRaisesRegex(ValueError, 'C start address must be 1-2000.'): - self.set('c2001', True) - with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): - self.set('c2000', [True, True]) - - def test_df_error_handling(self): - with self.assertRaisesRegex(ValueError, 'DF must be in \\[1, 500\\]'): - self.get('df501') - with self.assertRaisesRegex(ValueError, 'DF end must be in \\[1, 500\\]'): - self.get('df1-df501') - with self.assertRaisesRegex(ValueError, 'DF must be in \\[1, 500\\]'): - self.set('df501', 1.0) - with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): - self.set('df500', [1.0, 2.0]) - - def test_ds_error_handling(self): - with self.assertRaisesRegex(ValueError, 'DS must be in \\[1, 4500\\]'): - self.get('ds4501') - with self.assertRaisesRegex(ValueError, 'DS end must be in \\[1, 4500\\]'): - self.get('ds1-ds4501') - with self.assertRaisesRegex(ValueError, 'DS must be in \\[1, 4500\\]'): - self.set('ds4501', 1) - with self.assertRaisesRegex(ValueError, 'Data list longer than available addresses.'): - self.set('ds4500', [1, 2]) +@pytest.fixture +def plc_driver(): + return ClickPLC('fake ip') + + +@pytest.mark.asyncio +@pytest.mark.parametrize('prefix', ['x', 'y']) +async def test_bool_roundtrip(plc_driver, prefix): + await plc_driver.set(f'{prefix}2', True) + await plc_driver.set(f'{prefix}3', [False, True]) + expected = {f'{prefix}001': False, f'{prefix}002': True, f'{prefix}003': False, + f'{prefix}004': True, f'{prefix}005': False} + assert expected == await plc_driver.get(f'{prefix}1-{prefix}5') + + +@pytest.mark.asyncio +async def test_c_roundtrip(plc_driver): + await plc_driver.set('c2', True) + await plc_driver.set('c3', [False, True]) + expected = {'c1': False, 'c2': True, 'c3': False, 'c4': True, 'c5': False} + assert expected == await plc_driver.get('c1-c5') + + +@pytest.mark.asyncio +async def test_df_roundtrip(plc_driver): + await plc_driver.set('df2', 2.0) + await plc_driver.set('df3', [3.0, 4.0]) + expected = {'df1': 0.0, 'df2': 2.0, 'df3': 3.0, 'df4': 4.0, 'df5': 0.0} + assert expected == await plc_driver.get('df1-df5') + + +@pytest.mark.asyncio +async def test_ds_roundtrip(plc_driver): + await plc_driver.set('ds2', 2) + await plc_driver.set('ds3', [3, 4]) + expected = {'ds1': 0, 'ds2': 2, 'ds3': 3, 'ds4': 4, 'ds5': 0} + assert expected == await plc_driver.get('ds1-ds5') + + +@pytest.mark.asyncio +async def test_get_error_handling(plc_driver): + with pytest.raises(ValueError, match='End address must be greater than start address.'): + await plc_driver.get('c3-c1') + with pytest.raises(ValueError, match='foo currently unsupported'): + await plc_driver.get('foo1') + with pytest.raises(ValueError, match='Inter-category ranges are unsupported.'): + await plc_driver.get('c1-x3') + + +@pytest.mark.asyncio +async def test_set_error_handling(plc_driver): + with pytest.raises(ValueError, match='foo currently unsupported'): + await plc_driver.set('foo1', 1) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('prefix', ['x', 'y']) +async def test_xy_error_handling(plc_driver, prefix): + with pytest.raises(ValueError, match='address must be \*01-\*16.'): + await plc_driver.get(f'{prefix}17') + with pytest.raises(ValueError, match='address must be in \[001, 816\].'): + await plc_driver.get(f'{prefix}1001') + with pytest.raises(ValueError, match='address must be \*01-\*16.'): + await plc_driver.get(f'{prefix}1-{prefix}17') + with pytest.raises(ValueError, match='address must be in \[001, 816\].'): + await plc_driver.get(f'{prefix}1-{prefix}1001') + with pytest.raises(ValueError, match='address must be \*01-\*16.'): + await plc_driver.set(f'{prefix}17', True) + with pytest.raises(ValueError, match='address must be in \[001, 816\].'): + await plc_driver.set(f'{prefix}1001', True) + with pytest.raises(ValueError, match='Data list longer than available addresses.'): + await plc_driver.set(f'{prefix}816', [True, True]) + + +@pytest.mark.asyncio +async def test_c_error_handling(plc_driver): + with pytest.raises(ValueError, match='C start address must be 1-2000.'): + await plc_driver.get('c2001') + with pytest.raises(ValueError, match='C end address must be >start and <2000.'): + await plc_driver.get('c1-c2001') + with pytest.raises(ValueError, match='C start address must be 1-2000.'): + await plc_driver.set('c2001', True) + with pytest.raises(ValueError, match='Data list longer than available addresses.'): + await plc_driver.set('c2000', [True, True]) + + +@pytest.mark.asyncio +async def test_df_error_handling(plc_driver): + with pytest.raises(ValueError, match='DF must be in \[1, 500\]'): + await plc_driver.get('df501') + with pytest.raises(ValueError, match='DF end must be in \[1, 500\]'): + await plc_driver.get('df1-df501') + with pytest.raises(ValueError, match='DF must be in \[1, 500\]'): + await plc_driver.set('df501', 1.0) + with pytest.raises(ValueError, match='Data list longer than available addresses.'): + await plc_driver.set('df500', [1.0, 2.0]) + + +@pytest.mark.asyncio +async def test_ds_error_handling(plc_driver): + with pytest.raises(ValueError, match='DS must be in \[1, 4500\]'): + await plc_driver.get('ds4501') + with pytest.raises(ValueError, match='DS end must be in \[1, 4500\]'): + await plc_driver.get('ds1-ds4501') + with pytest.raises(ValueError, match='DS must be in \[1, 4500\]'): + await plc_driver.set('ds4501', 1) + with pytest.raises(ValueError, match='Data list longer than available addresses.'): + await plc_driver.set('ds4500', [1, 2]) + + +@pytest.mark.asyncio +@pytest.mark.parametrize('prefix', ['x', 'y', 'c']) +async def test_bool_typechecking(plc_driver, prefix): + with pytest.raises(ValueError, match='Expected .+ as a bool'): + await plc_driver.set(f'{prefix}1', 1) + with pytest.raises(ValueError, match='Expected .+ as a bool'): + await plc_driver.set(f'{prefix}1', [1.0, 1]) + + +@pytest.mark.asyncio +async def test_df_typechecking(plc_driver): + await plc_driver.set('df1', 1) + with pytest.raises(ValueError, match='Expected .+ as a float'): + await plc_driver.set('df1', True) + with pytest.raises(ValueError, match='Expected .+ as a float'): + await plc_driver.set('df1', [True, True]) + + +@pytest.mark.asyncio +async def test_ds_typechecking(plc_driver): + with pytest.raises(ValueError, match='Expected .+ as a int'): + await plc_driver.set('ds1', 1.0) + with pytest.raises(ValueError, match='Expected .+ as a int'): + await plc_driver.set('ds1', True) + with pytest.raises(ValueError, match='Expected .+ as a int'): + await plc_driver.set('ds1', [True, True]) diff --git a/setup.py b/setup.py index 6cebf34..0bf16dc 100644 --- a/setup.py +++ b/setup.py @@ -10,7 +10,7 @@ setup( name='clickplc', - version='0.2.7', + version='0.3.0', description="Python driver for Koyo Ethernet ClickPLCs.", long_description=long_description, long_description_content_type='text/markdown', @@ -28,6 +28,7 @@ 'test': [ 'pytest', 'pytest-cov', + 'pytest-asyncio', ], }, license='GPLv2',