From 241154be0cdd49f5741e6c71fa709d32e32c0593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 17:58:58 +0200 Subject: [PATCH 01/11] No else after return. --- canopen/objectdictionary/eds.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 608024f3..32c8e3ff 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -246,17 +246,15 @@ def _signed_int_from_hex(hex_str, bit_length): def _decode_from_eds(node_id: int, var_type: int, value: Any) -> Any: if var_type in (datatypes.OCTET_STRING, datatypes.DOMAIN): return bytes.fromhex(value) - elif var_type in (datatypes.VISIBLE_STRING, datatypes.UNICODE_STRING): + if var_type in (datatypes.VISIBLE_STRING, datatypes.UNICODE_STRING): return value - elif var_type in datatypes.FLOAT_TYPES: + if var_type in datatypes.FLOAT_TYPES: return float(value) - else: - # COB-ID can contain '$NODEID+' so replace this with node_id before converting - value = value.replace(" ", "").upper() - if '$NODEID' in value and node_id is not None: - return int(re.sub(r'\+?\$NODEID\+?', '', value), 0) + node_id - else: - return int(value, 0) + # COB-ID can contain '$NODEID+' so replace this with node_id before converting + value = value.replace(" ", "").upper() + if '$NODEID' in value and node_id is not None: + return int(re.sub(r'\+?\$NODEID\+?', '', value), 0) + node_id + return int(value, 0) def _encode_to_eds(var_type: int, value: Any) -> Any: @@ -264,12 +262,11 @@ def _encode_to_eds(var_type: int, value: Any) -> Any: return None if var_type in (datatypes.OCTET_STRING, datatypes.DOMAIN): return bytes.hex(value) - elif var_type in (datatypes.VISIBLE_STRING, datatypes.UNICODE_STRING): + if var_type in (datatypes.VISIBLE_STRING, datatypes.UNICODE_STRING): return value - elif var_type in datatypes.FLOAT_TYPES: + if var_type in datatypes.FLOAT_TYPES: return value - else: - return f"0x{value:02X}" + return f"0x{value:02X}" _STANDARD_OPTIONS = { From 45630163dd8fa22c8797d8b0570b3e37b10b7406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 17:59:30 +0200 Subject: [PATCH 02/11] Move regex definitions outside of loop, precompiled. Rename "match" variable, which is a language keyword. --- canopen/objectdictionary/eds.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 32c8e3ff..9aece505 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -100,10 +100,12 @@ def import_eds(source, node_id): node_id = int(val, base=0) od.node_id = node_id + DUMMY_SECTION_REGEX = re.compile(r"^[Dd]ummy[Uu]sage$") + INDEX_SECTION_REGEX = re.compile(r"^[0-9A-Fa-f]{4}$") + SUB_SECTION_REGEX = re.compile(r"^([0-9A-Fa-f]{4})[S|s]ub([0-9A-Fa-f]+)$") + NAME_SECTION_REGEX = re.compile(r"^([0-9A-Fa-f]{4})Name") for section in eds.sections(): - # Match dummy definitions - match = re.match(r"^[Dd]ummy[Uu]sage$", section) - if match is not None: + if DUMMY_SECTION_REGEX.match(section) is not None: for i in range(1, 8): key = f"Dummy{i:04d}" if eds.getint(section, key) == 1: @@ -112,9 +114,7 @@ def import_eds(source, node_id): var.access_type = "const" od.add_object(var) - # Match indexes - match = re.match(r"^[0-9A-Fa-f]{4}$", section) - if match is not None: + if INDEX_SECTION_REGEX.match(section) is not None: index = int(section, 16) name = eds.get(section, "ParameterName") try: @@ -154,11 +154,9 @@ def import_eds(source, node_id): continue - # Match subindexes - match = re.match(r"^([0-9A-Fa-f]{4})[S|s]ub([0-9A-Fa-f]+)$", section) - if match is not None: - index = int(match.group(1), 16) - subindex = int(match.group(2), 16) + if (m := SUB_SECTION_REGEX.match(section)) is not None: + index = int(m.group(1), 16) + subindex = int(m.group(2), 16) entry = od[index] if isinstance(entry, (ODRecord, ODArray)): try: @@ -169,9 +167,8 @@ def import_eds(source, node_id): entry.add_member(var) # Match [index]Name - match = re.match(r"^([0-9A-Fa-f]{4})Name", section) - if match is not None: - index = int(match.group(1), 16) + if (m := NAME_SECTION_REGEX.match(section)) is not None: + index = int(m.group(1), 16) num_of_entries = int(eds.get(section, "NrOfEntries")) entry = od[index] # For CompactSubObj index 1 is were we find the variable From 09c81cef34489bddbb6877969aba1762742c5fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 18:03:44 +0200 Subject: [PATCH 03/11] Avoid getattr() on attributes defined by constructor. --- canopen/objectdictionary/eds.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 9aece505..abbff622 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -439,30 +439,28 @@ def export_variable(var, eds): if getattr(var, 'default_raw', None) is not None: eds.set(section, "DefaultValue", var.default_raw) - elif getattr(var, 'default', None) is not None: - eds.set(section, "DefaultValue", _encode_to_eds( - var.data_type, var.default)) + elif var.default is not None: + eds.set(section, "DefaultValue", _encode_to_eds(var.data_type, var.default)) if device_commisioning: if getattr(var, 'value_raw', None) is not None: eds.set(section, "ParameterValue", var.value_raw) - elif getattr(var, 'value', None) is not None: - eds.set(section, "ParameterValue", - _encode_to_eds(var.data_type, var.value)) + elif var.value is not None: + eds.set(section, "ParameterValue", _encode_to_eds(var.data_type, var.value)) eds.set(section, "DataType", f"0x{var.data_type:04X}") eds.set(section, "PDOMapping", hex(var.pdo_mappable)) - if getattr(var, 'min', None) is not None: + if var.min is not None: eds.set(section, "LowLimit", var.min) - if getattr(var, 'max', None) is not None: + if var.max is not None: eds.set(section, "HighLimit", var.max) - if getattr(var, 'description', '') != '': + if var.description != '': eds.set(section, "Description", var.description) - if getattr(var, 'factor', 1) != 1: + if var.factor != 1: eds.set(section, "Factor", var.factor) - if getattr(var, 'unit', '') != '': + if var.unit != '': eds.set(section, "Unit", var.unit) for option, value in var.custom_options.items(): From 37c988fa9cd4f72d1e3328f55ee38c303d87647f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 17:58:33 +0200 Subject: [PATCH 04/11] Refactor string conversion with exception handling. Follow a common pattern of retrieving a raw_string from the parsed EDS outside of the block for catching ValueError, which is not raised from the RawConfigParser call. Fixes some type checker errors. Some more need to be squelched, for custom attribute additions. --- canopen/objectdictionary/eds.py | 40 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index abbff622..34b8bc7e 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -331,58 +331,58 @@ def build_variable( var.pdo_mappable = bool(int(eds.get(section, "PDOMapping", fallback="0"), 0)) if eds.has_option(section, "LowLimit"): + raw_string = eds.get(section, "LowLimit") try: - min_string = eds.get(section, "LowLimit") if var.data_type in datatypes.SIGNED_TYPES: - var.min = _signed_int_from_hex(min_string, _calc_bit_length(var.data_type)) + var.min = _signed_int_from_hex(raw_string, _calc_bit_length(var.data_type)) else: - var.min = int(min_string, 0) + var.min = int(raw_string, 0) except ValueError: logger.warning( - "Invalid LowLimit %r for %s (0x%X), ignoring", - min_string, var.name, var.index, + "Invalid LowLimit %r for %s (0x%X), ignoring", raw_string, var.name, var.index ) if eds.has_option(section, "HighLimit"): + raw_string = eds.get(section, "HighLimit") try: - max_string = eds.get(section, "HighLimit") if var.data_type in datatypes.SIGNED_TYPES: - var.max = _signed_int_from_hex(max_string, _calc_bit_length(var.data_type)) + var.max = _signed_int_from_hex(raw_string, _calc_bit_length(var.data_type)) else: - var.max = int(max_string, 0) + var.max = int(raw_string, 0) except ValueError: logger.warning( - "Invalid HighLimit %r for %s (0x%X), ignoring", - max_string, var.name, var.index, + "Invalid HighLimit %r for %s (0x%X), ignoring", raw_string, var.name, var.index ) if eds.has_option(section, "DefaultValue"): + raw_string = eds.get(section, "DefaultValue") + var.default_raw = raw_string # type: ignore[attr-defined] # custom round-trip addition try: - var.default_raw = eds.get(section, "DefaultValue") - if '$NODEID' in var.default_raw: + if '$NODEID' in raw_string: var.relative = True - var.default = _decode_from_eds(node_id, var.data_type, var.default_raw) + var.default = _decode_from_eds(node_id, var.data_type, raw_string) except ValueError: logger.warning( "Invalid DefaultValue %r for %s (0x%X), ignoring", - var.default_raw, var.name, var.index, + raw_string, var.name, var.index, ) if eds.has_option(section, "ParameterValue"): + raw_string = eds.get(section, "ParameterValue") + var.value_raw = raw_string # type: ignore[attr-defined] # custom round-trip addition try: - var.value_raw = eds.get(section, "ParameterValue") - var.value = _decode_from_eds(node_id, var.data_type, var.value_raw) + var.value = _decode_from_eds(node_id, var.data_type, raw_string) except ValueError: logger.warning( "Invalid ParameterValue %r for %s (0x%X), ignoring", - var.value_raw, var.name, var.index, + raw_string, var.name, var.index, ) # Factor, Description and Unit are not standard according to the CANopen specifications, but # they are implemented in the python canopen package, so we can at least try to use them if eds.has_option(section, "Factor"): + raw_string = eds.get(section, "Factor") try: - var.factor = float(eds.get(section, "Factor")) + var.factor = float(raw_string) except ValueError: logger.warning( - "Invalid Factor %r for %s (0x%X), ignoring", - eds.get(section, "Factor"), var.name, var.index, + "Invalid Factor %r for %s (0x%X), ignoring", raw_string, var.name, var.index ) if eds.has_option(section, "Description"): var.description = eds.get(section, "Description") From 9b016e791e4bdf461ddb169e2cf10b657f6add08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 18:28:41 +0200 Subject: [PATCH 05/11] Collapse has_option() and get() calls. --- canopen/objectdictionary/eds.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 34b8bc7e..40122ccd 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -330,8 +330,7 @@ def build_variable( var.pdo_mappable = bool(int(eds.get(section, "PDOMapping", fallback="0"), 0)) - if eds.has_option(section, "LowLimit"): - raw_string = eds.get(section, "LowLimit") + if (raw_string := eds.get(section, "LowLimit", fallback=None)) is not None: try: if var.data_type in datatypes.SIGNED_TYPES: var.min = _signed_int_from_hex(raw_string, _calc_bit_length(var.data_type)) @@ -341,8 +340,7 @@ def build_variable( logger.warning( "Invalid LowLimit %r for %s (0x%X), ignoring", raw_string, var.name, var.index ) - if eds.has_option(section, "HighLimit"): - raw_string = eds.get(section, "HighLimit") + if (raw_string := eds.get(section, "HighLimit", fallback=None)) is not None: try: if var.data_type in datatypes.SIGNED_TYPES: var.max = _signed_int_from_hex(raw_string, _calc_bit_length(var.data_type)) @@ -352,8 +350,7 @@ def build_variable( logger.warning( "Invalid HighLimit %r for %s (0x%X), ignoring", raw_string, var.name, var.index ) - if eds.has_option(section, "DefaultValue"): - raw_string = eds.get(section, "DefaultValue") + if (raw_string := eds.get(section, "DefaultValue", fallback=None)) is not None: var.default_raw = raw_string # type: ignore[attr-defined] # custom round-trip addition try: if '$NODEID' in raw_string: @@ -364,8 +361,7 @@ def build_variable( "Invalid DefaultValue %r for %s (0x%X), ignoring", raw_string, var.name, var.index, ) - if eds.has_option(section, "ParameterValue"): - raw_string = eds.get(section, "ParameterValue") + if (raw_string := eds.get(section, "ParameterValue", fallback=None)) is not None: var.value_raw = raw_string # type: ignore[attr-defined] # custom round-trip addition try: var.value = _decode_from_eds(node_id, var.data_type, raw_string) @@ -376,18 +372,17 @@ def build_variable( ) # Factor, Description and Unit are not standard according to the CANopen specifications, but # they are implemented in the python canopen package, so we can at least try to use them - if eds.has_option(section, "Factor"): - raw_string = eds.get(section, "Factor") + if (raw_string := eds.get(section, "Factor", fallback=None)) is not None: try: var.factor = float(raw_string) except ValueError: logger.warning( "Invalid Factor %r for %s (0x%X), ignoring", raw_string, var.name, var.index ) - if eds.has_option(section, "Description"): - var.description = eds.get(section, "Description") - if eds.has_option(section, "Unit"): - var.unit = eds.get(section, "Unit") + if (raw_string := eds.get(section, "Description", fallback=None)) is not None: + var.description = raw_string + if (raw_string := eds.get(section, "Unit", fallback=None)) is not None: + var.unit = raw_string var.custom_options = _get_custom_options(eds, section) return var From 37ac2c3cf7c47d96b34fe3c00c024bfe904c0adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 18:34:50 +0200 Subject: [PATCH 06/11] Ignore type checker error for custom attribute. --- canopen/objectdictionary/eds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 40122ccd..4396c7d7 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -485,7 +485,7 @@ def export_record(var, eds): try: # only if eds was loaded by us - origFileInfo = od.__edsFileInfo + origFileInfo = od.__edsFileInfo # type: ignore[attr-defined] # custom addition except AttributeError: origFileInfo = { # just set some defaults From 792aa320f17e5903f67b5dac77eaaaeeb235578e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 28 May 2026 18:35:19 +0200 Subject: [PATCH 07/11] Fix shadowed "list" built-in. --- canopen/objectdictionary/eds.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 4396c7d7..2a40cad8 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -571,12 +571,12 @@ def optional_indices(x): supported_optional_indices = list(filter(optional_indices, od)) supported_manufacturer_indices = list(filter(manufacturer_indices, od)) - def add_list(section, list): + def add_list(section, lst): eds.add_section(section) - eds.set(section, "SupportedObjects", len(list)) - for i in range(0, len(list)): - eds.set(section, (i + 1), f"0x{list[i]:04X}") - for index in list: + eds.set(section, "SupportedObjects", len(lst)) + for i in range(0, len(lst)): + eds.set(section, (i + 1), f"0x{lst[i]:04X}") + for index in lst: export_object(od[index], eds) add_list("MandatoryObjects", supported_mantatory_indices) From 9694edd524b7d470dd41cb0744163d40cbd8861f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Sat, 13 Jun 2026 22:32:24 +0200 Subject: [PATCH 08/11] Simplify and reformat DeviceInfo section parsing. --- canopen/objectdictionary/eds.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 2a40cad8..279a8669 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -80,16 +80,13 @@ def import_eds(source, node_id): (bool, "LSS_Supported", "LSS_supported"), ]: try: - if t in (int, bool): - setattr(od.device_information, odprop, - t(int(eds.get("DeviceInfo", eprop), 0)) - ) - elif t is str: - setattr(od.device_information, odprop, - eds.get("DeviceInfo", eprop) - ) + raw_string = eds.get("DeviceInfo", eprop) except NoOptionError: - pass + continue + if t in (int, bool): + setattr(od.device_information, odprop, t(int(raw_string, 0))) + elif t is str: + setattr(od.device_information, odprop, raw_string) if eds.has_section("DeviceComissioning"): if val := eds.getint("DeviceComissioning", "Baudrate", fallback=None): From 7327199d011b03351d2d7b70d52ac3579a5cb08a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Sat, 13 Jun 2026 22:48:14 +0200 Subject: [PATCH 09/11] Fix type checker errors around optionxform. Follow the mypy advice about incompatible type. Silence the method assignment error, as that's a legitimate an documented use case on this class. --- canopen/objectdictionary/eds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 279a8669..df1842af 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -25,7 +25,7 @@ def import_eds(source, node_id): eds = RawConfigParser(inline_comment_prefixes=(';',)) - eds.optionxform = str + eds.optionxform = lambda optionstr: str(optionstr) # type: ignore[method-assign] opened_here = False try: if hasattr(source, "read"): From 2efa12cd5937695cab53814c1ca8bf80a0e6780e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Sat, 13 Jun 2026 23:42:31 +0200 Subject: [PATCH 10/11] Add type guard for ODArray in CompactSubObj handling. --- canopen/objectdictionary/eds.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index df1842af..a7f39e68 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -168,8 +168,15 @@ def import_eds(source, node_id): index = int(m.group(1), 16) num_of_entries = int(eds.get(section, "NrOfEntries")) entry = od[index] + if not isinstance(entry, ODArray): + logger.warning( + "Compact subobject section %r refers to non-array object 0x%04X", + section, + index, + ) + continue # For CompactSubObj index 1 is were we find the variable - src_var = od[index][1] + src_var = entry[1] for subindex in range(1, num_of_entries + 1): var = copy_variable(eds, section, subindex, src_var) if var is not None: From 196580238ede5a0e9b0bf19285bbe704aca32705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Sat, 13 Jun 2026 23:44:18 +0200 Subject: [PATCH 11/11] Document "section" parameter in build_variable() docstring. --- canopen/objectdictionary/eds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index a7f39e68..3db13805 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -297,12 +297,12 @@ def build_variable( node_id: int, object_type: int, index: int, - subindex: int = 0 + subindex: int = 0, ) -> ODVariable: """Create a object dictionary entry. :param eds: String stream of the eds file - :param section: + :param section: EDS file section corresponding to the object :param node_id: Node ID :param index: Index of the CANOpen object :param subindex: Subindex of the CANOpen object (if present, else 0)