From 7f8ab9f94f8bca755d555b33aad5526a0c13f142 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 2 Sep 2024 14:33:00 +0200 Subject: [PATCH 1/9] Skip saving and log read-only PDO parameters. For PDO communication or mapping parameters which are read-only on a node, the PdoMap.save() method currently aborts with an SDO exception. However, individual parameters may simply not support configuration changes. Rely on the Object Dictionary to tell which ones are expected to fail and skip them. For communication record entries, log what is not being saved. --- canopen/pdo/base.py | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index 0ba65199..5d8be956 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -15,7 +15,7 @@ if TYPE_CHECKING: from canopen import LocalNode, RemoteNode from canopen.pdo import RPDO, TPDO - from canopen.sdo import SdoRecord + from canopen.sdo import SdoRecord, SdoVariable PDO_NOT_VALID = 1 << 31 @@ -404,18 +404,22 @@ def save(self) -> None: return logger.info("Setting COB-ID 0x%X and temporarily disabling PDO", self.cob_id) self.com_record[1].raw = self.cob_id | PDO_NOT_VALID | (RTR_NOT_ALLOWED if not self.rtr_allowed else 0x0) - if self.trans_type is not None: - logger.info("Setting transmission type to %d", self.trans_type) - self.com_record[2].raw = self.trans_type - if self.inhibit_time is not None: - logger.info("Setting inhibit time to %d us", (self.inhibit_time * 100)) - self.com_record[3].raw = self.inhibit_time - if self.event_timer is not None: - logger.info("Setting event timer to %d ms", self.event_timer) - self.com_record[5].raw = self.event_timer - if self.sync_start_value is not None: - logger.info("Setting SYNC start value to %d", self.sync_start_value) - self.com_record[6].raw = self.sync_start_value + + def _set_com_record( + subindex: int, value: Optional[int], log_fmt: str, log_factor: int = 1 + ): + if value is None: + return + if self.com_record[subindex].writable: + logger.info(f"Setting {log_fmt}", value * log_factor) + self.com_record[subindex].raw = value + else: + logger.info(f"Cannot set {log_fmt}, not writable", value * log_factor) + + _set_com_record(2, self.trans_type, "transmission type to %d") + _set_com_record(3, self.inhibit_time, "inhibit time to %d us", 100) + _set_com_record(5, self.event_timer, "event timer to %d ms") + _set_com_record(6, self.sync_start_value, "SYNC start value to %d") try: self.map_array[0].raw = 0 @@ -425,20 +429,16 @@ def save(self) -> None: # mappings for an invalid object 0x0000:00 to overwrite any # excess entries with all-zeros. self._fill_map(self.map_array[0].raw) - subindex = 1 - for var in self.map: + for var, entry in zip(self.map, self.map_array): + if not entry.writable: + continue logger.info("Writing %s (0x%04X:%02X, %d bits) to PDO map", var.name, var.index, var.subindex, var.length) if getattr(self.pdo_node.node, "curtis_hack", False): # Curtis HACK: mixed up field order - self.map_array[subindex].raw = (var.index | - var.subindex << 16 | - var.length << 24) + entry.raw = var.index | var.subindex << 16 | var.length << 24 else: - self.map_array[subindex].raw = (var.index << 16 | - var.subindex << 8 | - var.length) - subindex += 1 + entry.raw = var.index << 16 | var.subindex << 8 | var.length try: self.map_array[0].raw = len(self.map) except SdoAbortedError as e: From 160c81b8e6435c353e1ea4fa5e68b6cc190b9c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 19:50:39 +0200 Subject: [PATCH 2/9] Cosmetics [black]. --- canopen/pdo/base.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index 5d8be956..235e356c 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -403,7 +403,11 @@ def save(self) -> None: logger.info("Skip saving %s: COB-ID was never set", self.com_record.od.name) return logger.info("Setting COB-ID 0x%X and temporarily disabling PDO", self.cob_id) - self.com_record[1].raw = self.cob_id | PDO_NOT_VALID | (RTR_NOT_ALLOWED if not self.rtr_allowed else 0x0) + self.com_record[1].raw = ( + self.cob_id + | PDO_NOT_VALID + | (RTR_NOT_ALLOWED if not self.rtr_allowed else 0) + ) def _set_com_record( subindex: int, value: Optional[int], log_fmt: str, log_factor: int = 1 @@ -432,8 +436,13 @@ def _set_com_record( for var, entry in zip(self.map, self.map_array): if not entry.writable: continue - logger.info("Writing %s (0x%04X:%02X, %d bits) to PDO map", - var.name, var.index, var.subindex, var.length) + logger.info( + "Writing %s (0x%04X:%02X, %d bits) to PDO map", + var.name, + var.index, + var.subindex, + var.length, + ) if getattr(self.pdo_node.node, "curtis_hack", False): # Curtis HACK: mixed up field order entry.raw = var.index | var.subindex << 16 | var.length << 24 From 848b03f2b69fd07b2c912d51a81b33a5ea3c47de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 20:00:48 +0200 Subject: [PATCH 3/9] Remove leftover type import. --- 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 235e356c..72edf7ab 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -15,7 +15,7 @@ if TYPE_CHECKING: from canopen import LocalNode, RemoteNode from canopen.pdo import RPDO, TPDO - from canopen.sdo import SdoRecord, SdoVariable + from canopen.sdo import SdoRecord PDO_NOT_VALID = 1 << 31 From 44d302eea29fa375c01c3d4f769be411cde1a93b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 20:32:07 +0200 Subject: [PATCH 4/9] Fix iteration, correct writable attribute. --- canopen/pdo/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/canopen/pdo/base.py b/canopen/pdo/base.py index 72edf7ab..7bef6e62 100644 --- a/canopen/pdo/base.py +++ b/canopen/pdo/base.py @@ -433,8 +433,8 @@ def _set_com_record( # mappings for an invalid object 0x0000:00 to overwrite any # excess entries with all-zeros. self._fill_map(self.map_array[0].raw) - for var, entry in zip(self.map, self.map_array): - if not entry.writable: + for var, entry in zip(self.map, self.map_array.values()): + if not entry.od.writable: continue logger.info( "Writing %s (0x%04X:%02X, %d bits) to PDO map", From 880c4de667f8d5c7aa63a66e5bff87e6fc9117e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 20:34:14 +0200 Subject: [PATCH 5/9] Switch to a LocalNode in TestPDO to not require a network. --- 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 1badc89d..be6d132b 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -7,7 +7,7 @@ class TestPDO(unittest.TestCase): def setUp(self): - node = canopen.Node(1, SAMPLE_EDS) + node = canopen.LocalNode(1, SAMPLE_EDS) pdo = node.pdo.tx[1] pdo.add_variable('INTEGER16 value') # 0x2001 pdo.add_variable('UNSIGNED8 value', length=4) # 0x2002 From 022ebe05d9fcf12873742a5ff8f4b3ad72c8982f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 20:35:08 +0200 Subject: [PATCH 6/9] Test PdoMap.save() skipping readonly entries. --- test/test_pdo.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test_pdo.py b/test/test_pdo.py index be6d132b..f2f626b9 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -64,6 +64,12 @@ def test_pdo_save(self): self.node.tpdo.save() self.node.rpdo.save() + def test_pdo_save_skip_readonly(self): + """Expect no exception when a record entry is not writable.""" + self.node.tpdo[1].cob_id = self.node.tpdo[1].predefined_cob_id + self.node.tpdo[1].com_record[2].od.access_type = "r" + self.node.tpdo[1].save() + def test_pdo_export(self): try: import canmatrix From 12096e493718337232f93c4b5dbc1b3a22666b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 20:54:17 +0200 Subject: [PATCH 7/9] Specify parameters to have the test not skip them. --- test/test_pdo.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_pdo.py b/test/test_pdo.py index f2f626b9..74aaff08 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -66,7 +66,10 @@ def test_pdo_save(self): def test_pdo_save_skip_readonly(self): """Expect no exception when a record entry is not writable.""" + # Saving only happens with a defined COB ID and for specified parameters self.node.tpdo[1].cob_id = self.node.tpdo[1].predefined_cob_id + self.node.tpdo[1].trans_type = 1 + self.node.tpdo[1].inhibit_time = 10 self.node.tpdo[1].com_record[2].od.access_type = "r" self.node.tpdo[1].save() From e74899cb6bc086858ead10744353794412e43fef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 20:59:55 +0200 Subject: [PATCH 8/9] Fix and duplicate test, one writable and the other not. --- test/test_pdo.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/test_pdo.py b/test/test_pdo.py index 74aaff08..e362da75 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -69,10 +69,13 @@ def test_pdo_save_skip_readonly(self): # Saving only happens with a defined COB ID and for specified parameters self.node.tpdo[1].cob_id = self.node.tpdo[1].predefined_cob_id self.node.tpdo[1].trans_type = 1 - self.node.tpdo[1].inhibit_time = 10 - self.node.tpdo[1].com_record[2].od.access_type = "r" self.node.tpdo[1].save() + self.node.tpdo[2].cob_id = self.node.tpdo[2].predefined_cob_id + self.node.tpdo[2].trans_type = 1 + self.node.tpdo[2].com_record[2].od.access_type = "r" + self.node.tpdo[2].save() + def test_pdo_export(self): try: import canmatrix From c9cbc566eb1d4f225c3d174de54c87e492ed018b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 21:02:43 +0200 Subject: [PATCH 9/9] Test read-only mapping array entry as well. --- test/test_pdo.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_pdo.py b/test/test_pdo.py index e362da75..b8bb0599 100644 --- a/test/test_pdo.py +++ b/test/test_pdo.py @@ -69,6 +69,7 @@ def test_pdo_save_skip_readonly(self): # Saving only happens with a defined COB ID and for specified parameters self.node.tpdo[1].cob_id = self.node.tpdo[1].predefined_cob_id self.node.tpdo[1].trans_type = 1 + self.node.tpdo[1].map_array[1].od.access_type = "r" self.node.tpdo[1].save() self.node.tpdo[2].cob_id = self.node.tpdo[2].predefined_cob_id