Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions canopen/objectdictionary/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ def __init__(self, name: str, index: int, subindex: int = 0):
self.data_type: Optional[int] = None
#: Access type, should be "rw", "ro", "wo", or "const"
self.access_type: str = "rw"
#: The variable represents a DOMAIN ObjectType
self.is_domain: bool = False
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design consideration: a boolean is_domain only captures one special ObjectType. Storing the full object_type: int attribute (defaulting to objectcodes.VAR) would be more flexible (e.g. NULL, DEFTYPE) and avoids losing that information for manually constructed ODVariable instances. Just something to think about — a boolean is perfectly fine for the stated use-case.

#: Description of variable
self.description: str = ""
#: Dictionary of value descriptions
Expand Down
17 changes: 12 additions & 5 deletions canopen/objectdictionary/eds.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
var = build_variable(eds, section, node_id, index, is_domain=object_type==objectcodes.DOMAIN)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP 8: missing spaces around the == comparison inside the keyword argument.

Suggested change
var = build_variable(eds, section, node_id, index, is_domain=object_type==objectcodes.DOMAIN)
var = build_variable(eds, section, node_id, index, is_domain=object_type == objectcodes.DOMAIN)

od.add_object(var)
elif object_type == objectcodes.ARRAY and eds.has_option(section, "CompactSubObj"):
arr = ODArray(name, index)
Expand Down Expand Up @@ -158,7 +158,11 @@ def import_eds(source, node_id):
subindex = int(match.group(2), 16)
entry = od[index]
if isinstance(entry, (ODRecord, ODArray)):
var = build_variable(eds, section, node_id, index, subindex)
try:
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)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same PEP 8 spacing issue as above.

Suggested change
var = build_variable(eds, section, node_id, index, subindex, is_domain=object_type==objectcodes.DOMAIN)
var = build_variable(eds, section, node_id, index, subindex, is_domain=object_type == objectcodes.DOMAIN)

entry.add_member(var)

# Match [index]Name
Expand Down Expand Up @@ -252,13 +256,14 @@ def _revert_variable(var_type, value):
return f"0x{value:02X}"


def build_variable(eds, section, node_id, index, subindex=0):
def build_variable(eds, section, node_id, index, subindex=0, is_domain=False):
"""Creates a object dictionary entry.
:param eds: String stream of the eds file
:param section:
:param node_id: Node ID
:param index: Index of the CANOpen object
:param subindex: Subindex of the CANOpen object (if presente, else 0)
:param subindex: Subindex of the CANOpen object (if present, else 0)
:param is_domain: variable represents a DOMAIN ObjectType (if present, else False)
"""
name = eds.get(section, "ParameterName")
var = ODVariable(name, index, subindex)
Expand All @@ -268,6 +273,7 @@ def build_variable(eds, section, node_id, index, subindex=0):
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
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)
Expand Down Expand Up @@ -370,7 +376,8 @@ def export_variable(var, eds):
section = f"{var.index:04X}sub{var.subindex:X}"

export_common(var, eds, section)
eds.set(section, "ObjectType", f"0x{objectcodes.VAR:X}")
object_type = objectcodes.DOMAIN if var.is_domain else objectcodes.VAR
eds.set(section, "ObjectType", f"0x{object_type:X}")
if var.data_type:
eds.set(section, "DataType", f"0x{var.data_type:04X}")
if var.access_type:
Expand Down
27 changes: 27 additions & 0 deletions test/sample.eds
Original file line number Diff line number Diff line change
Expand Up @@ -1017,3 +1017,30 @@ PDOMapping=0x0
Factor=ERROR
Description=
Unit=

[3063]
ParameterName=DOMAIN object
ObjectType=0x2
DataType=0x0007
AccessType=rw
PDOMapping=0

[3064]
ParameterName=Record with DOMAIN sub-object
SubNumber=0x2
ObjectType=0x9

[3064sub0]
ParameterName=Highest subindex
ObjectType=0x7
DataType=0x0005
AccessType=ro
DefaultValue=0x01
PDOMapping=0

[3064sub1]
ParameterName=DOMAIN sub-object
ObjectType=0x2
DataType=0x0007
AccessType=rw
PDOMapping=0
37 changes: 37 additions & 0 deletions test/test_eds.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def test_variable(self):
self.assertEqual(var.name, 'Producer heartbeat time')
self.assertEqual(var.data_type, canopen.objectdictionary.UNSIGNED16)
self.assertEqual(var.access_type, 'rw')
self.assertEqual(var.is_domain, False)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: for boolean checks the unittest idiom is assertFalse/assertTrue rather than assertEqual(..., False/True). Same pattern applies to the other four analogous assertions added in this PR.

Suggested change
self.assertEqual(var.is_domain, False)
self.assertFalse(var.is_domain)

self.assertEqual(var.default, 0)
self.assertFalse(var.relative)

Expand All @@ -132,6 +133,7 @@ def test_record(self):
self.assertEqual(var.subindex, 1)
self.assertEqual(var.data_type, canopen.objectdictionary.UNSIGNED32)
self.assertEqual(var.access_type, 'ro')
self.assertEqual(var.is_domain, False)

def test_record_with_limits(self):
int8 = self.od[0x3020]
Expand Down Expand Up @@ -166,6 +168,7 @@ def test_array_compact_subobj(self):
self.assertEqual(var.subindex, 5)
self.assertEqual(var.data_type, canopen.objectdictionary.UNSIGNED32)
self.assertEqual(var.access_type, 'ro')
self.assertEqual(var.is_domain, False)

def test_explicit_name_subobj(self):
name = self.od[0x3004].name
Expand Down Expand Up @@ -197,6 +200,7 @@ def test_dummy_variable(self):
self.assertEqual(var.name, 'Dummy0003')
self.assertEqual(var.data_type, canopen.objectdictionary.INTEGER16)
self.assertEqual(var.access_type, 'const')
self.assertEqual(var.is_domain, False)
self.assertEqual(len(var), 16)

def test_dummy_variable_undefined(self):
Expand All @@ -213,6 +217,39 @@ def test_reading_factor(self):
self.assertEqual(var2.factor, 1)
self.assertEqual(var2.unit, '')

def test_read_domain_object(self):
var = self.od[0x3063]
self.assertIsInstance(var, canopen.objectdictionary.ODVariable)
self.assertEqual(var.index, 0x3063)
self.assertEqual(var.subindex, 0)
self.assertEqual(var.name, 'DOMAIN object')
self.assertEqual(var.data_type, canopen.objectdictionary.UNSIGNED32)
self.assertEqual(var.access_type, 'rw')
self.assertEqual(var.is_domain, True)

def test_read_domain_subobject(self):
record = self.od[0x3064]
var = record[1]
self.assertIsInstance(var, canopen.objectdictionary.ODVariable)
self.assertEqual(var.index, 0x3064)
self.assertEqual(var.subindex, 1)
self.assertEqual(var.name, 'DOMAIN sub-object')
self.assertEqual(var.data_type, canopen.objectdictionary.UNSIGNED32)
self.assertEqual(var.access_type, 'rw')
self.assertEqual(var.is_domain, True)

def test_roundtrip_domain_objects(self):
# ObjectType==DOMAIN survive an EDS export/import round-trip
import io
with io.StringIO() as dest:
canopen.export_od(self.od, dest, 'eds')
dest.name = 'mock.eds'
dest.seek(0)
od2 = canopen.import_od(dest)
self.assertEqual(od2['Producer heartbeat time'].is_domain, False)
self.assertEqual(od2['Identity object']['Vendor-ID'].is_domain, False)
self.assertEqual(od2[0x3063].is_domain, True)
self.assertEqual(od2[0x3064][1].is_domain, True)


def test_comments(self):
Expand Down