From 261a39aa2502cff0d6d3aa87a18f6bba91848691 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 3 Jul 2024 19:11:19 +0200 Subject: [PATCH 1/6] Add back PdoBase.export() dependency (canmatrix) The canmatrix optional dependency was removed on Oct 10, 2021 with commit c46228f. It is now added back as an optional dependency, using the same name as previously: db_export. To install the dependency: $ python3 -m pip install 'canopen[db_export]' Resolves #488 --- .github/workflows/pythonpackage.yml | 3 +- canopen/pdo/base.py | 23 +++++++++ pyproject.toml | 5 ++ test/test_pdo.py | 79 +++++++++++++++++++---------- 4 files changed, 81 insertions(+), 29 deletions(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 8fbd0aca..03a8c826 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -17,6 +17,7 @@ jobs: fail-fast: false matrix: python-version: ['3.x'] + features: ['', '[db_export]'] steps: - uses: actions/checkout@v3 @@ -28,7 +29,7 @@ jobs: run: | python -m pip install --upgrade pip pip install pytest pytest-cov - pip install -e . + pip install -e '.${{ matrix.features }}' - name: Test with pytest run: | pytest -v --cov=canopen --cov-report=xml --cov-branch diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index d467cdb3..f5d57bf9 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -1,4 +1,5 @@ from __future__ import annotations +import functools import threading import math from typing import Callable, Dict, Iterator, List, Optional, Union, TYPE_CHECKING @@ -9,6 +10,10 @@ from canopen.sdo import SdoAbortedError from canopen import objectdictionary from canopen import variable +try: + import canmatrix +except ImportError: + canmatrix = None if TYPE_CHECKING: from canopen.network import Network @@ -16,6 +21,17 @@ from canopen.pdo import RPDO, TPDO from canopen.sdo import SdoRecord + +def _disable_if(condition): + """Conditionally disable given function/method.""" + def deco(func): + @functools.wraps(func) + def wrapper(*args, **kwds): + if not condition: + return func(*args, **kwds) + return wrapper + return deco + PDO_NOT_VALID = 1 << 31 RTR_NOT_ALLOWED = 1 << 30 @@ -75,9 +91,16 @@ def subscribe(self): for pdo_map in self.map.values(): pdo_map.subscribe() + @_disable_if(canmatrix is None) def export(self, filename): """Export current configuration to a database file. + .. note:: + This API depends on the :mod:`!canmatrix` optional dependency, + which can be installed by requesting the ``db_export`` feature:: + + python3 -m pip install 'canopen[db_export]' + :param str filename: Filename to save to (e.g. DBC, DBF, ARXML, KCD etc) diff --git a/pyproject.toml b/pyproject.toml index 67a435d8..a87ba589 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,11 @@ dependencies = [ ] dynamic = ["version"] +[project.optional-dependencies] +db_export = [ + "canmatrix ~= 1.0", +] + [project.urls] documentation = "https://canopen.readthedocs.io/en/stable/" repository = "https://github.com/christiansandberg/canopen" diff --git a/test/test_pdo.py b/test/test_pdo.py index 32963cf3..758dfd68 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -1,41 +1,51 @@ import os.path import unittest + import canopen +try: + import canmatrix +except ImportError: + canmatrix = None EDS_PATH = os.path.join(os.path.dirname(__file__), 'sample.eds') class TestPDO(unittest.TestCase): - - def test_bit_mapping(self): + def setUp(self): node = canopen.Node(1, EDS_PATH) - map = node.pdo.tx[1] - map.add_variable('INTEGER16 value') # 0x2001 - map.add_variable('UNSIGNED8 value', length=4) # 0x2002 - map.add_variable('INTEGER8 value', length=4) # 0x2003 - map.add_variable('INTEGER32 value') # 0x2004 - map.add_variable('BOOLEAN value', length=1) # 0x2005 - map.add_variable('BOOLEAN value 2', length=1) # 0x2006 + pdo = node.pdo.tx[1] + pdo.add_variable('INTEGER16 value') # 0x2001 + pdo.add_variable('UNSIGNED8 value', length=4) # 0x2002 + pdo.add_variable('INTEGER8 value', length=4) # 0x2003 + pdo.add_variable('INTEGER32 value') # 0x2004 + pdo.add_variable('BOOLEAN value', length=1) # 0x2005 + pdo.add_variable('BOOLEAN value 2', length=1) # 0x2006 # Write some values - map['INTEGER16 value'].raw = -3 - map['UNSIGNED8 value'].raw = 0xf - map['INTEGER8 value'].raw = -2 - map['INTEGER32 value'].raw = 0x01020304 - map['BOOLEAN value'].raw = False - map['BOOLEAN value 2'].raw = True + pdo['INTEGER16 value'].raw = -3 + pdo['UNSIGNED8 value'].raw = 0xf + pdo['INTEGER8 value'].raw = -2 + pdo['INTEGER32 value'].raw = 0x01020304 + pdo['BOOLEAN value'].raw = False + pdo['BOOLEAN value 2'].raw = True + + self.pdo = pdo + self.node = node - # Check expected data - self.assertEqual(map.data, b'\xfd\xff\xef\x04\x03\x02\x01\x02') + def test_pdo_map_bit_mapping(self): + self.assertEqual(self.pdo.data, b'\xfd\xff\xef\x04\x03\x02\x01\x02') - # Read values from data - self.assertEqual(map['INTEGER16 value'].raw, -3) - self.assertEqual(map['UNSIGNED8 value'].raw, 0xf) - self.assertEqual(map['INTEGER8 value'].raw, -2) - self.assertEqual(map['INTEGER32 value'].raw, 0x01020304) - self.assertEqual(map['BOOLEAN value'].raw, False) - self.assertEqual(map['BOOLEAN value 2'].raw, True) + def test_pdo_map_getitem(self): + pdo = self.pdo + self.assertEqual(pdo['INTEGER16 value'].raw, -3) + self.assertEqual(pdo['UNSIGNED8 value'].raw, 0xf) + self.assertEqual(pdo['INTEGER8 value'].raw, -2) + self.assertEqual(pdo['INTEGER32 value'].raw, 0x01020304) + self.assertEqual(pdo['BOOLEAN value'].raw, False) + self.assertEqual(pdo['BOOLEAN value 2'].raw, True) + def test_pdo_getitem(self): + node = self.node self.assertEqual(node.tpdo[1]['INTEGER16 value'].raw, -3) self.assertEqual(node.tpdo[1]['UNSIGNED8 value'].raw, 0xf) self.assertEqual(node.tpdo[1]['INTEGER8 value'].raw, -2) @@ -55,10 +65,23 @@ def test_bit_mapping(self): self.assertEqual(node.tpdo[0x2002].raw, 0xf) self.assertEqual(node.pdo[0x1600][0x2002].raw, 0xf) - def test_save_pdo(self): - node = canopen.Node(1, EDS_PATH) - node.tpdo.save() - node.rpdo.save() + def test_pdo_save(self): + self.node.tpdo.save() + self.node.rpdo.save() + + @unittest.skipIf(canmatrix is None, "Requires canmatrix dependency") + def test_pdo_export(self): + import tempfile + + for pdo in "tpdo", "rpdo": + with tempfile.NamedTemporaryFile(suffix=".csv") as tmp: + fn = tmp.name + with self.subTest(pdo=pdo, filename=fn): + getattr(self.node, pdo).export(fn) + with open(fn) as csv: + header = csv.readline() + self.assertIn("ID", header) + self.assertIn("Frame Name", header) if __name__ == "__main__": From 0fe7c1d91707eb2f09b48ef03981e56213bf1132 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 4 Jul 2024 01:31:41 +0200 Subject: [PATCH 2/6] Update test/test_pdo.py --- test/test_pdo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_pdo.py b/test/test_pdo.py index 758dfd68..e34e769b 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -76,7 +76,7 @@ def test_pdo_export(self): for pdo in "tpdo", "rpdo": with tempfile.NamedTemporaryFile(suffix=".csv") as tmp: fn = tmp.name - with self.subTest(pdo=pdo, filename=fn): + with self.subTest(filename=fn, pdo=pdo): getattr(self.node, pdo).export(fn) with open(fn) as csv: header = csv.readline() From 0b2eae97e1e259a802cb18498c525b85f268c486 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 5 Jul 2024 22:19:05 +0200 Subject: [PATCH 3/6] Address review: simply relay ImportError if the API is not available --- canopen/pdo/base.py | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index a992c50d..1bb5f676 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -1,5 +1,4 @@ from __future__ import annotations -import functools import threading import math from typing import Callable, Dict, Iterator, List, Optional, Union, TYPE_CHECKING @@ -10,10 +9,6 @@ from canopen.sdo import SdoAbortedError from canopen import objectdictionary from canopen import variable -try: - import canmatrix -except ImportError: - canmatrix = None if TYPE_CHECKING: from canopen.network import Network @@ -21,17 +16,6 @@ from canopen.pdo import RPDO, TPDO from canopen.sdo import SdoRecord - -def _disable_if(condition): - """Conditionally disable given function/method.""" - def deco(func): - @functools.wraps(func) - def wrapper(*args, **kwds): - if not condition: - return func(*args, **kwds) - return wrapper - return deco - PDO_NOT_VALID = 1 << 31 RTR_NOT_ALLOWED = 1 << 30 @@ -91,7 +75,6 @@ def subscribe(self): for pdo_map in self.map.values(): pdo_map.subscribe() - @_disable_if(canmatrix is None) def export(self, filename): """Export current configuration to a database file. @@ -103,6 +86,8 @@ def export(self, filename): :param str filename: Filename to save to (e.g. DBC, DBF, ARXML, KCD etc) + :raises ImportError: + When the ``db_feature`` is not installed. :return: The CanMatrix object created :rtype: canmatrix.canmatrix.CanMatrix From f8c913dcbc0d6b75d025c51c912170672b623df9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 5 Jul 2024 23:29:31 +0200 Subject: [PATCH 4/6] Typo --- canopen/pdo/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index 1bb5f676..0f2d458f 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -87,7 +87,7 @@ def export(self, filename): :param str filename: Filename to save to (e.g. DBC, DBF, ARXML, KCD etc) :raises ImportError: - When the ``db_feature`` is not installed. + When the ``db_export`` is not installed. :return: The CanMatrix object created :rtype: canmatrix.canmatrix.CanMatrix From 962993ee2d56d28f6fe860a6866f4a13a76c1a1f Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 5 Jul 2024 23:30:53 +0200 Subject: [PATCH 5/6] Amend wording --- canopen/pdo/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index 0f2d458f..a646660d 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -87,7 +87,7 @@ def export(self, filename): :param str filename: Filename to save to (e.g. DBC, DBF, ARXML, KCD etc) :raises ImportError: - When the ``db_export`` is not installed. + When the ``db_export`` feature is not installed. :return: The CanMatrix object created :rtype: canmatrix.canmatrix.CanMatrix From 013ff93cacb4261dd9b5c95dd7bb139056abab00 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sat, 6 Jul 2024 19:19:34 +0200 Subject: [PATCH 6/6] Address review: - skip test from within the test method - raise NotImplementedError from within the export() method --- canopen/pdo/base.py | 14 ++++++++------ test/test_pdo.py | 10 ++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index a646660d..f2a7d205 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -79,21 +79,23 @@ def export(self, filename): """Export current configuration to a database file. .. note:: - This API depends on the :mod:`!canmatrix` optional dependency, - which can be installed by requesting the ``db_export`` feature:: + This API requires the ``db_export`` feature to be installed:: python3 -m pip install 'canopen[db_export]' :param str filename: Filename to save to (e.g. DBC, DBF, ARXML, KCD etc) - :raises ImportError: - When the ``db_export`` feature is not installed. + :raises NotImplementedError: + When the ``canopen[db_export]`` feature is not installed. :return: The CanMatrix object created :rtype: canmatrix.canmatrix.CanMatrix """ - from canmatrix import canmatrix - from canmatrix import formats + try: + from canmatrix import canmatrix + from canmatrix import formats + except ImportError: + raise NotImplementedError("This feature requires the 'canopen[db_export]' feature") db = canmatrix.CanMatrix() for pdo_map in self.map.values(): diff --git a/test/test_pdo.py b/test/test_pdo.py index e34e769b..20de3633 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -1,11 +1,6 @@ import os.path import unittest - import canopen -try: - import canmatrix -except ImportError: - canmatrix = None EDS_PATH = os.path.join(os.path.dirname(__file__), 'sample.eds') @@ -69,9 +64,12 @@ def test_pdo_save(self): self.node.tpdo.save() self.node.rpdo.save() - @unittest.skipIf(canmatrix is None, "Requires canmatrix dependency") def test_pdo_export(self): import tempfile + try: + import canmatrix + except ImportError: + raise unittest.SkipTest("The PDO export API requires canmatrix") for pdo in "tpdo", "rpdo": with tempfile.NamedTemporaryFile(suffix=".csv") as tmp: