From afea007b77cf76611bcd7f0b690442d61d018755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 7 May 2026 10:34:10 +0200 Subject: [PATCH 1/4] eds: Centralize object type logic in build_variable. Pass object type to build_variable() instead of the boolean is_domain parameter. Add type hints to aid in validating all call sites have been adjusted. --- canopen/objectdictionary/eds.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 19c4b02f..e1298c29 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -129,7 +129,7 @@ def import_eds(source, node_id): storage_location = None if object_type in (objectcodes.VAR, objectcodes.DOMAIN): - var = build_variable(eds, section, node_id, index, is_domain=object_type == objectcodes.DOMAIN) + var = build_variable(eds, section, node_id, object_type, index) od.add_object(var) elif object_type == objectcodes.ARRAY and eds.has_option(section, "CompactSubObj"): arr = ODArray(name, index) @@ -137,7 +137,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, node_id, object_type, index, 1)) arr.storage_location = storage_location od.add_object(arr) elif object_type == objectcodes.ARRAY: @@ -162,7 +162,7 @@ def import_eds(source, node_id): object_type = int(eds.get(section, "ObjectType"), 0) except NoOptionError: object_type = objectcodes.VAR - var = build_variable(eds, section, node_id, index, subindex, is_domain=object_type == objectcodes.DOMAIN) + var = build_variable(eds, section, node_id, object_type, index, subindex) entry.add_member(var) # Match [index]Name @@ -256,8 +256,16 @@ def _revert_variable(var_type, value): return f"0x{value:02X}" -def build_variable(eds, section, node_id, index, subindex=0, is_domain=False): - """Creates a object dictionary entry. +def build_variable( + eds: RawConfigParser, + section: str, + node_id: int, + object_type: int, + index: int, + subindex: int = 0 +) -> ODVariable: + """Create a object dictionary entry. + :param eds: String stream of the eds file :param section: :param node_id: Node ID @@ -273,7 +281,7 @@ def build_variable(eds, section, node_id, index, subindex=0, is_domain=False): var.storage_location = None var.data_type = int(eds.get(section, "DataType"), 0) var.access_type = eds.get(section, "AccessType").lower() - var.is_domain = is_domain + var.is_domain = object_type == objectcodes.DOMAIN if var.data_type > 0x1B: # The object dictionary editor from CANFestival creates an optional object if min max values are used # This optional object is then placed in the eds under the section [A0] (start point, iterates for more) From cf92c25c2be3b51949abb20622e957aedc602456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 7 May 2026 11:41:42 +0200 Subject: [PATCH 2/4] Remove unnecessary duplicate lookup from ConfigParser. --- 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 e1298c29..629ed276 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -319,13 +319,13 @@ def build_variable( var.default_raw = eds.get(section, "DefaultValue") if '$NODEID' in var.default_raw: var.relative = True - var.default = _convert_variable(node_id, var.data_type, eds.get(section, "DefaultValue")) + var.default = _convert_variable(node_id, var.data_type, var.default_raw) except ValueError: pass if eds.has_option(section, "ParameterValue"): try: var.value_raw = eds.get(section, "ParameterValue") - var.value = _convert_variable(node_id, var.data_type, eds.get(section, "ParameterValue")) + var.value = _convert_variable(node_id, var.data_type, var.value_raw) except ValueError: pass # 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 From 7858b3abb76f8536ac0540eb320e020f534b1a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 7 May 2026 11:43:33 +0200 Subject: [PATCH 3/4] Fix mypy errors in build_variable(). The storage_location attribute needs an explicit annotation (assumed only None type) on the OD variable classes, to accept strings from eds. Silence mypy in import_eds() for a custom attribute addition. --- canopen/objectdictionary/__init__.py | 6 +++--- canopen/objectdictionary/eds.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/canopen/objectdictionary/__init__.py b/canopen/objectdictionary/__init__.py index f5208f56..40ab8fbf 100644 --- a/canopen/objectdictionary/__init__.py +++ b/canopen/objectdictionary/__init__.py @@ -206,7 +206,7 @@ def __init__(self, name: str, index: int): #: Name of record self.name = name #: Storage location of index - self.storage_location = None + self.storage_location: Optional[str] = None self.subindices: dict[int, ODVariable] = {} self.names: dict[str, ODVariable] = {} @@ -267,7 +267,7 @@ def __init__(self, name: str, index: int): #: Name of array self.name = name #: Storage location of index - self.storage_location = None + self.storage_location: Optional[str] = None self.subindices: dict[int, ODVariable] = {} self.names: dict[str, ODVariable] = {} @@ -377,7 +377,7 @@ def __init__(self, name: str, index: int, subindex: int = 0): #: Dictionary of bitfield definitions self.bit_definitions: dict[str, list[int]] = {} #: Storage location of index - self.storage_location = None + self.storage_location: Optional[str] = None #: Can this variable be mapped to a PDO self.pdo_mappable = False diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 629ed276..48b7dc6a 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -41,7 +41,7 @@ def import_eds(source, node_id): od = ObjectDictionary() if eds.has_section("FileInfo"): - od.__edsFileInfo = { + od.__edsFileInfo = { # type: ignore[attr-defined] # custom addition opt: eds.get("FileInfo", opt) for opt in eds.options("FileInfo") } From b34d97a993a0a4198ff4bf1078d5eecedb4e2ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 7 May 2026 11:44:34 +0200 Subject: [PATCH 4/4] Respect line-length in build_variable + small style fix. --- canopen/objectdictionary/eds.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index 48b7dc6a..d47a3019 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -50,7 +50,7 @@ def import_eds(source, node_id): linecount = int(eds.get("Comments", "Lines"), 0) od.comments = '\n'.join([ eds.get("Comments", f"Line{line}") - for line in range(1, linecount+1) + for line in range(1, linecount + 1) ]) if not eds.has_section("DeviceInfo"): @@ -214,7 +214,9 @@ def _calc_bit_length(data_type): elif data_type == datatypes.INTEGER64: return 64 else: - raise ValueError(f"Invalid data_type '{data_type}', expecting a signed integer data_type.") + raise ValueError( + f"Invalid data_type '{data_type}', expecting a signed integer data_type." + ) def _signed_int_from_hex(hex_str, bit_length): @@ -283,14 +285,17 @@ def build_variable( var.access_type = eds.get(section, "AccessType").lower() var.is_domain = object_type == objectcodes.DOMAIN if var.data_type > 0x1B: - # The object dictionary editor from CANFestival creates an optional object if min max values are used - # This optional object is then placed in the eds under the section [A0] (start point, iterates for more) - # The eds.get function gives us 0x00A0 now convert to String without hex representation and upper case - # The sub2 part is then the section where the type parameter stands + # The object dictionary editor from CANFestival creates an optional object if min max + # values are used. This optional object is then placed in the eds under the section + # [A0] (start point, iterates for more). The eds.get function gives us 0x00A0 now + # convert to String without hex representation and upper case. The sub2 part is then + # the section where the type parameter stands. try: var.data_type = int(eds.get(f"{var.data_type:X}sub1", "DefaultValue"), 0) except NoSectionError: - logger.warning("%s has an unknown or unsupported data type (0x%X)", name, var.data_type) + logger.warning( + "%s has an unknown or unsupported data type (0x%X)", name, var.data_type + ) # Assume DOMAIN to force application to interpret the byte data var.data_type = datatypes.DOMAIN @@ -328,7 +333,8 @@ def build_variable( var.value = _convert_variable(node_id, var.data_type, var.value_raw) except ValueError: pass - # 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 + # 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"): try: var.factor = float(eds.get(section, "Factor"))