-
Notifications
You must be signed in to change notification settings - Fork 354
Memorized DOMAIN ObjectType for a proper export #655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PEP 8: missing spaces around the
Suggested change
|
||||||
| od.add_object(var) | ||||||
| elif object_type == objectcodes.ARRAY and eds.has_option(section, "CompactSubObj"): | ||||||
| arr = ODArray(name, index) | ||||||
|
|
@@ -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) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same PEP 8 spacing issue as above.
Suggested change
|
||||||
| entry.add_member(var) | ||||||
|
|
||||||
| # Match [index]Name | ||||||
|
|
@@ -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) | ||||||
|
|
@@ -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) | ||||||
|
|
@@ -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: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: for boolean checks the
Suggested change
|
||||||
| self.assertEqual(var.default, 0) | ||||||
| self.assertFalse(var.relative) | ||||||
|
|
||||||
|
|
@@ -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] | ||||||
|
|
@@ -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 | ||||||
|
|
@@ -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): | ||||||
|
|
@@ -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): | ||||||
|
|
||||||
There was a problem hiding this comment.
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_domainonly captures one special ObjectType. Storing the fullobject_type: intattribute (defaulting toobjectcodes.VAR) would be more flexible (e.g.NULL,DEFTYPE) and avoids losing that information for manually constructedODVariableinstances. Just something to think about — a boolean is perfectly fine for the stated use-case.