From 1b2cf657bf1a751ebf0eb344255b18261340801c Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 2 Jul 2024 16:13:05 +0200 Subject: [PATCH 1/4] Alt 1 --- canopen/objectdictionary/eds.py | 12 +++++++----- test/sample.eds | 2 +- test/test_eds.py | 9 +++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index cd64f604..17919ba5 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -86,8 +86,10 @@ def import_eds(source, node_id): if eds.has_section("DeviceComissioning"): od.bitrate = int(eds.get("DeviceComissioning", "Baudrate")) * 1000 - od.node_id = int(eds.get("DeviceComissioning", "NodeID"), 0) - node_id = node_id or od.node_id + + default = None if node_id is None else {"NodeID": node_id} + id_ = eds.get("DeviceComissioning", "NodeID", vars=default) + od.node_id = int(id_, base=0) for section in eds.sections(): # Match dummy definitions @@ -119,7 +121,7 @@ def import_eds(source, node_id): storage_location = None if object_type in (VAR, DOMAIN): - var = build_variable(eds, section, node_id, index) + var = build_variable(eds, section, od.node_id, index) od.add_object(var) elif object_type == ARR and eds.has_option(section, "CompactSubObj"): arr = objectdictionary.ODArray(name, index) @@ -127,7 +129,7 @@ def import_eds(source, node_id): "Number of entries", index, 0) last_subindex.data_type = datatypes.UNSIGNED8 arr.add_member(last_subindex) - arr.add_member(build_variable(eds, section, node_id, index, 1)) + arr.add_member(build_variable(eds, section, od.node_id, index, 1)) arr.storage_location = storage_location od.add_object(arr) elif object_type == ARR: @@ -149,7 +151,7 @@ def import_eds(source, node_id): entry = od[index] if isinstance(entry, (objectdictionary.ODRecord, objectdictionary.ODArray)): - var = build_variable(eds, section, node_id, index, subindex) + var = build_variable(eds, section, od.node_id, index, subindex) entry.add_member(var) # Match [index]Name diff --git a/test/sample.eds b/test/sample.eds index 16d0c31a..3c1bbcf9 100644 --- a/test/sample.eds +++ b/test/sample.eds @@ -30,7 +30,7 @@ NrOfTXPDO=4 LSS_Supported=0 [DeviceComissioning] -NodeID=2 +NodeID=0x10 NodeName=Some name Baudrate=500 NetNumber=0 diff --git a/test/test_eds.py b/test/test_eds.py index 693fac2c..e92edeac 100644 --- a/test/test_eds.py +++ b/test/test_eds.py @@ -63,6 +63,15 @@ def test_load_file_object(self): od = canopen.import_od(fp) self.assertTrue(len(od) > 0) + def test_load_implicit_nodeid(self): + # sample.eds has a DeviceComissioning section with NodeID set to 0x10. + od = canopen.import_od(EDS_PATH) + self.assertEqual(od.node_id, 16) + + def test_load_explicit_nodeid(self): + od = canopen.import_od(EDS_PATH, node_id=3) + self.assertEqual(od.node_id, 3) + def test_variable(self): var = self.od['Producer heartbeat time'] self.assertIsInstance(var, canopen.objectdictionary.ODVariable) From e4cfa6ecc3b906d61c40268cf0325866b6451ba8 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 2 Jul 2024 16:14:07 +0200 Subject: [PATCH 2/4] Alt 2 --- canopen/objectdictionary/eds.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 17919ba5..00fe451d 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -87,9 +87,8 @@ def import_eds(source, node_id): if eds.has_section("DeviceComissioning"): od.bitrate = int(eds.get("DeviceComissioning", "Baudrate")) * 1000 - default = None if node_id is None else {"NodeID": node_id} - id_ = eds.get("DeviceComissioning", "NodeID", vars=default) - od.node_id = int(id_, base=0) + id_ = int(eds.get("DeviceComissioning", "NodeID"), base=0) + od.node_id = id_ if node_id is None else node_id for section in eds.sections(): # Match dummy definitions From 8a22e7ed329afbda46ac629c7c6974223477c0c2 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 3 Jul 2024 22:56:53 +0200 Subject: [PATCH 3/4] Address review: load node_id from EDS if it's not provided --- canopen/objectdictionary/eds.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 00fe451d..a84eed4f 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -87,8 +87,9 @@ def import_eds(source, node_id): if eds.has_section("DeviceComissioning"): od.bitrate = int(eds.get("DeviceComissioning", "Baudrate")) * 1000 - id_ = int(eds.get("DeviceComissioning", "NodeID"), base=0) - od.node_id = id_ if node_id is None else node_id + if node_id is None: + node_id = int(eds.get("DeviceComissioning", "NodeID"), base=0) + od.node_id = node_id for section in eds.sections(): # Match dummy definitions @@ -120,7 +121,7 @@ def import_eds(source, node_id): storage_location = None if object_type in (VAR, DOMAIN): - var = build_variable(eds, section, od.node_id, index) + var = build_variable(eds, section, node_id, index) od.add_object(var) elif object_type == ARR and eds.has_option(section, "CompactSubObj"): arr = objectdictionary.ODArray(name, index) @@ -128,7 +129,7 @@ def import_eds(source, node_id): "Number of entries", index, 0) last_subindex.data_type = datatypes.UNSIGNED8 arr.add_member(last_subindex) - arr.add_member(build_variable(eds, section, od.node_id, index, 1)) + arr.add_member(build_variable(eds, section, node_id, index, 1)) arr.storage_location = storage_location od.add_object(arr) elif object_type == ARR: @@ -150,7 +151,7 @@ def import_eds(source, node_id): entry = od[index] if isinstance(entry, (objectdictionary.ODRecord, objectdictionary.ODArray)): - var = build_variable(eds, section, od.node_id, index, subindex) + var = build_variable(eds, section, node_id, index, subindex) entry.add_member(var) # Match [index]Name From ac79a35c792fc60bb0fabad56e60f4d659747a1b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 3 Jul 2024 23:36:24 +0200 Subject: [PATCH 4/4] Add fallback for edge case --- canopen/objectdictionary/eds.py | 3 ++- test/test_eds.py | 27 ++++++++++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index a84eed4f..40342d32 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -88,7 +88,8 @@ def import_eds(source, node_id): od.bitrate = int(eds.get("DeviceComissioning", "Baudrate")) * 1000 if node_id is None: - node_id = int(eds.get("DeviceComissioning", "NodeID"), base=0) + if val := eds.get("DeviceComissioning", "NodeID", fallback=None): + node_id = int(val, base=0) od.node_id = node_id for section in eds.sections(): diff --git a/test/test_eds.py b/test/test_eds.py index f46cc0ec..da59e0e6 100644 --- a/test/test_eds.py +++ b/test/test_eds.py @@ -4,7 +4,9 @@ from canopen.objectdictionary.eds import _signed_int_from_hex from canopen.utils import pretty_index -EDS_PATH = os.path.join(os.path.dirname(__file__), 'sample.eds') + +SAMPLE_EDS = os.path.join(os.path.dirname(__file__), 'sample.eds') +DATATYPES_EDS = os.path.join(os.path.dirname(__file__), 'datatypes.eds') class TestEDS(unittest.TestCase): @@ -48,7 +50,7 @@ class TestEDS(unittest.TestCase): } def setUp(self): - self.od = canopen.import_od(EDS_PATH, 2) + self.od = canopen.import_od(SAMPLE_EDS, 2) def test_load_nonexisting_file(self): with self.assertRaises(IOError): @@ -59,17 +61,32 @@ def test_load_unsupported_format(self): canopen.import_od(__file__) def test_load_file_object(self): - with open(EDS_PATH) as fp: + with open(SAMPLE_EDS) as fp: od = canopen.import_od(fp) self.assertTrue(len(od) > 0) def test_load_implicit_nodeid(self): # sample.eds has a DeviceComissioning section with NodeID set to 0x10. - od = canopen.import_od(EDS_PATH) + od = canopen.import_od(SAMPLE_EDS) self.assertEqual(od.node_id, 16) + def test_load_implicit_nodeid_fallback(self): + import io + + # First, remove the NodeID option from DeviceComissioning. + with open(SAMPLE_EDS) as f: + lines = [L for L in f.readlines() if not L.startswith("NodeID=")] + with io.StringIO("".join(lines)) as buf: + buf.name = "mock.eds" + od = canopen.import_od(buf) + self.assertIsNone(od.node_id) + + # Next, try an EDS file without a DeviceComissioning section. + od = canopen.import_od(DATATYPES_EDS) + self.assertIsNone(od.node_id) + def test_load_explicit_nodeid(self): - od = canopen.import_od(EDS_PATH, node_id=3) + od = canopen.import_od(SAMPLE_EDS, node_id=3) self.assertEqual(od.node_id, 3) def test_variable(self):