Fix bugs during GUI testing

* Fix failure on reading the "need" field from node
* Increase the viewport size of the index parts in GUI
* Add error message if entering invalid data in GUI table editor
* Fix bug where UI changes in the table didn't update the data
* Added in-line documentation in ODStructTypes
* Fix incorrect ValueError when having arrays
* FIxed Node.DumpFile not guessing the filetype correctly
* Fix handling of DOMAIN, which address #10
Testing
* Add new "equiv_files" fixture to test equivalent files
* Add test for testing `odg compare`
* Added OD-file for DOMAIN and updated OD-files for consistency

src/objdictgen/__main__.py

@@ -311,7 +311,8 @@ def main(debugopts: DebugOpts, args: Sequence[str]|None = None):
             print(f"{objdictgen.ODG_PROGRAM}: '{opts.od1}' and '{opts.od2}' are equal")
 
         print_diffs(diffs, show=opts.show)
-        parser.exit(errcode)
+        if errcode:
+            parser.exit(errcode)
 
 
     # -- EDIT command --

src/objdictgen/eds_utils.py

@@ -655,7 +655,7 @@ def generate_eds_content(node: "Node", filepath: TPath):
         if 0x2000 <= entry <= 0x5FFF:
             manufacturers.append(entry)
         # Second case, entry is required, then it's a mandatory entry
-        elif entry_infos["need"]:
+        elif entry_infos.get("need"):
             mandatories.append(entry)
         # In any other case, it's an optional entry
         else:

src/objdictgen/maps.py

@@ -67,23 +67,33 @@ class ODStructTypes:
     #
     # Properties of entry structure in the Object Dictionary
     #
-    Subindex = 1             # Entry has at least one subindex
-    MultipleSubindexes = 2   # Entry has more than one subindex
-    IdenticalSubindexes = 4  # Subindexes of entry have the same description
-    IdenticalIndexes = 8     # Entry has the same description on multiple indexes
+    Subindex = 1
+    """Entry has at least one subindex"""
+    MultipleSubindexes = 2
+    """Entry has more than one subindex"""
+    IdenticalSubindexes = 4
+    """Subindexes of entry have the same description"""
+    IdenticalIndexes = 8
+    """Entry has the same description on multiple objects"""
 
     #
     # Structures of entry in the Object Dictionary, sum of the properties described above
     # for all sorts of entries use in CAN Open specification
     #
     NOSUB = 0  # Entry without subindex (only for type declaration)
     VAR = Subindex  # 1
+    """Variable object structure"""
     RECORD = Subindex | MultipleSubindexes  # 3
+    """Record object structure, i.e. subindexes with different descriptions"""
     ARRAY = Subindex | MultipleSubindexes | IdenticalSubindexes  # 7
+    """Array object structure, i.e. subindexes with the same type"""
     # Entries identical on multiple indexes
     NVAR = Subindex | IdenticalIndexes  # 9
+    """Variable object structure that spans several objects"""
     NRECORD = Subindex | MultipleSubindexes | IdenticalIndexes  # 11, Example : PDO Parameters
+    """Record object structure that spans several objects"""
     NARRAY = Subindex | MultipleSubindexes | IdenticalSubindexes | IdenticalIndexes  # 15, Example : PDO Mapping
+    """Array object structure that spans several objects"""
 
     #
     # Mapping against name and structure number
@@ -425,7 +435,7 @@ def FindMandatoryIndexes(self) -> list[int]:
         return [
             index
             for index in self
-            if index >= 0x1000 and self[index]["need"]
+            if index >= 0x1000 and self[index].get("need")
         ]
 
     def FindEntryName(self, index: int, compute=True) -> str:

src/objdictgen/node.py

@@ -20,6 +20,7 @@
 
 import copy
 import logging
+from pathlib import Path
 from typing import Any, Generator, Iterable, Iterator
 
 import colorama
@@ -195,8 +196,15 @@ def LoadJson(contents: str) -> "Node":
         """ Import a new Node from a JSON string """
         return jsonod.generate_node(contents)
 
-    def DumpFile(self, filepath: TPath, filetype: str = "json", **kwargs):
+    def DumpFile(self, filepath: TPath, filetype: str|None = "json", **kwargs):
         """ Save node into file """
+
+        # Attempt to determine the filetype from the filepath
+        if not filetype:
+            filetype = Path(filepath).suffix[1:]
+        if not filetype:
+            filetype = "json"
+
         if filetype == 'od':
             log.debug("Writing XML OD '%s'", filepath)
             with open(filepath, "w", encoding="utf-8") as f:
@@ -860,8 +868,11 @@ def RemoveMappingEntry(self, index: int, subindex: int|None = None):
         if subindex is None:
             self.UserMapping.pop(index)
             return
-        if subindex == len(self.UserMapping[index]["values"]) - 1:
-            self.UserMapping[index]["values"].pop(subindex)
+        obj = self.UserMapping[index]
+        if subindex == len(obj["values"]) - 1:
+            obj["values"].pop(subindex)
+            return
+        if obj['struct'] & OD.IdenticalSubindexes:
             return
         raise ValueError(f"Invalid subindex {subindex} for index 0x{index:04x}")
 

src/objdictgen/nodemanager.py

@@ -377,10 +377,7 @@ def RemoveSubentriesFromCurrent(self, index: int, number: int):
         length = node.GetEntry(index, 0)
         # FIXME: This code assumes that subindex 0 is the length of the entry
         assert isinstance(length, int)
-        if "nbmin" in infos:
-            nbmin = infos["nbmin"]
-        else:
-            nbmin = 1
+        nbmin = infos.get("nbmin", 1)
         # Entry is an array, or is an array/record of manufacturer specific
         # FIXME: What is the intended order of the conditions? or-and on same level
         if (infos["struct"] & OD.IdenticalSubindexes or 0x2000 <= index <= 0x5FFF
@@ -574,7 +571,7 @@ def RemoveCurrentVariable(self, index: int, subindex: int|None = None):
             node.RemoveMapVariable(index, subindex or 0)
             if not found:
                 infos = node.GetEntryInfos(index)
-                if not infos["need"]:
+                if not infos.get("need"):
                     node.RemoveEntry(index, subindex)
             if index in mappings[-1]:
                 node.RemoveMappingEntry(index, subindex)
@@ -700,8 +697,7 @@ def SetCurrentEntry(self, index: int, subindex: int, value: str, name: str, edit
                     # Might fail with binascii.Error if hex is malformed
                     if len(value) % 2 != 0:
                         value = "0" + value
-                    # FIXME: decode() produce bytes, which is not supported as of now
-                    bvalue = codecs.decode(value, 'hex_codec')
+                    bvalue = codecs.decode(value, 'hex_codec').decode()
                     node.SetEntry(index, subindex, bvalue)
                 elif editor == "dcf":
                     node.SetEntry(index, subindex, value)
@@ -737,12 +733,14 @@ def SetCurrentEntry(self, index: int, subindex: int, value: str, name: str, edit
                         raise ValueError("Number must be positive")
                     node.SetParamsEntry(index, subindex, params={"buffer_size": nvalue})
             else:
+                nvalue: str|int = value
                 if editor == "type":
                     nvalue = node.GetTypeIndex(value)
+                    # All type object shall have size
                     size = node.GetEntryInfos(nvalue)["size"]
                     node.UpdateMapVariable(index, subindex, size)
                 elif editor in ["access", "raccess"]:
-                    value = {
+                    nvalue = {  # type: ignore[assignment]
                         access: abbrev
                         for abbrev, access in maps.ACCESS_TYPE.items()
                     }[value]
@@ -753,7 +751,7 @@ def SetCurrentEntry(self, index: int, subindex: int, value: str, name: str, edit
                         node.AddMappingEntry(index, entry={"name": entry_infos["name"], "struct": OD.ARRAY})
                         node.AddMappingSubEntry(index, 0, values=subindex0_infos)
                         node.AddMappingSubEntry(index, 1, values=subindex1_infos)
-                node.SetMappingSubEntry(index, subindex, values={name: value})  # type: ignore[misc]
+                node.SetMappingSubEntry(index, subindex, values={name: nvalue})  # type: ignore[misc]
             if not disable_buffer:
                 self.BufferCurrentNode()
 
@@ -1049,7 +1047,7 @@ def GetNodeEntryValues(self, node: Node, index: int) -> tuple[list[dict], list[d
                                 editor["value"] = "dcf"
                             else:
                                 editor["value"] = "domain"
-                            dic["value"] = codecs.encode(dic["value"].encode(), 'hex_codec')
+                            dic["value"] = codecs.encode(dic["value"].encode(), 'hex_codec').decode()
                         elif dic["type"] == "BOOLEAN":
                             editor["value"] = "bool"
                             dic["value"] = maps.BOOL_TYPE[dic["value"]]
@@ -1065,7 +1063,7 @@ def GetNodeEntryValues(self, node: Node, index: int) -> tuple[list[dict], list[d
                                     raise  # FIXME: Originial code swallows exception
                                 try:
                                     dic["value"] = fmt.format(dic["value"])
-                                except TypeError as exc:
+                                except ValueError as exc:
                                     log.debug("ValueError: '%s': %s", dic["value"], exc)
                                     # FIXME: dict["value"] can contain $NODEID for PDOs i.e. $NODEID+0x200
                                 editor["value"] = "string"

src/objdictgen/ui/nodeeditortemplate.py

@@ -89,7 +89,7 @@ def SetStatusBarText(self, selection, node: Node):
                 entryinfos = node.GetEntryInfos(index)
                 name = entryinfos["name"]
                 category = "Optional"
-                if entryinfos["need"]:
+                if entryinfos.get("need"):
                     category = "Mandatory"
                 struct = "VAR"
                 number = ""

src/objdictgen/ui/subindextable.py

@@ -432,7 +432,7 @@ def _init_ctrls(self, parent):
 
         self.PartList = wx.ListBox(choices=[], id=ID_EDITINGPANELPARTLIST,
             name='PartList', parent=self, pos=wx.Point(0, 0),
-            size=wx.Size(-1, -1), style=0)
+            size=wx.Size(-1, 180), style=0)
         self.PartList.Bind(wx.EVT_LISTBOX, self.OnPartListBoxClick,
             id=ID_EDITINGPANELPARTLIST)
 
@@ -729,7 +729,10 @@ def ShowDCFEntryDialog(self, row, col):
                     dialog.SetValues(codecs.decode(self.Table.GetValue(row, col), "hex_codec"))
                     if dialog.ShowModal() == wx.ID_OK and self.Editable:
                         value = dialog.GetValues()
-                        self.Manager.SetCurrentEntry(index, row, value, "value", "dcf")
+                        try:
+                            self.Manager.SetCurrentEntry(index, row, value, "value", "dcf")
+                        except Exception as e:  # pylint: disable=broad-except
+                            display_error_dialog(self, f"Failed to set value: {e}", "Failed to set value")
                         self.ParentWindow.RefreshBufferState()
                         wx.CallAfter(self.RefreshTable)
 
@@ -741,7 +744,10 @@ def OnSubindexGridCellChange(self, event):
             name = self.Table.GetColLabelValue(col, False)
             value = self.Table.GetValue(subindex, col, False)
             editor = self.Table.GetEditor(subindex, col)
-            self.Manager.SetCurrentEntry(index, subindex, value, name, editor)
+            try:
+                self.Manager.SetCurrentEntry(index, subindex, value, name, editor)
+            except Exception as e:  # pylint: disable=broad-except
+                display_error_dialog(self, f"Failed to set value: {e}", "Failed to set value")
             self.ParentWindow.RefreshBufferState()
             wx.CallAfter(self.RefreshTable)
         event.Skip()

tests/conftest.py

@@ -39,6 +39,25 @@
     ODDIR / "legacy-strings.od",  # The legacy tool silently crash on this input
 ]
 
+# Equivalent files that should compare as equal
+COMPARE_EQUIVS = [
+    ('alltypes',             'legacy-alltypes'),
+    ('master',               'legacy-master'),
+    ('slave',                'legacy-slave'),
+    #( "profile-test",        "legacy-profile-test"),
+    ( "profile-ds302",       "legacy-profile-ds302"),
+    ( "profile-ds401",       "legacy-profile-ds401"),
+    ( "profile-ds302-ds401", "legacy-profile-ds302-ds401"),
+    #( "profile-ds302-test",  "legacy-profile-ds302-test"),
+    ( "slave-ds302",         "legacy-slave-ds302"),
+    ( "slave-emcy",          "legacy-slave-emcy"),
+    ( "slave-heartbeat",     "legacy-slave-heartbeat"),
+    ( "slave-nodeguarding",  "legacy-slave-nodeguarding"),
+    ( "slave-sync",          "legacy-slave-sync"),
+    ( "strings",             "legacy-strings"),
+    ( "domain",              "legacy-domain"),
+]
+
 
 class ODPath(type(Path())):
     """ Overload on Path to add OD specific methods """
@@ -224,6 +243,11 @@ def odids(odlist):
         metafunc.parametrize("py2", [Py2(py2_path, objdictgen_dir)],
                                 indirect=False, scope="session")
 
+    # Add "equiv_files" fixture
+    if "equiv_files" in metafunc.fixturenames:
+        metafunc.parametrize("equiv_files", COMPARE_EQUIVS, ids=(e[0] for e in COMPARE_EQUIVS),
+                                indirect=False, scope="session")
+
 
 def pytest_collection_modifyitems(items):
     """Modifies test items in place to ensure test modules run in a given order."""

tests/od/domain.json

@@ -0,0 +1,30 @@
+{
+  "$id": "od data",
+  "$version": "1",
+  "$description": "Canfestival object dictionary data",
+  "$tool": "odg 3.4",
+  "$date": "2024-04-10T22:48:13.333406",
+  "name": "domain",
+  "description": "",
+  "type": "master",
+  "id": 0,
+  "profile": "None",
+  "dictionary": [
+    {
+      "index": "0x2000",  // 8192
+      "name": "Domain",
+      "struct": "var",
+      "group": "user",
+      "mandatory": false,
+      "sub": [
+        {
+          "name": "Domain",
+          "type": "DOMAIN",  // 15
+          "access": "rw",
+          "pdo": true,
+          "value": "\u0002@ABC\u0001"
+        }
+      ]
+    }
+  ]
+}

tests/od/domain.od

@@ -0,0 +1,65 @@
+<?xml version="1.0"?>
+<!DOCTYPE PyObject SYSTEM "PyObjects.dtd">
+<PyObject module="node" class="Node" id="2217354715472">
+<attr name="Profile" type="dict" id="2217369299472">
+</attr>
+<attr name="Description" type="string" value="" />
+<attr name="Dictionary" type="dict" id="2217369505408">
+  <entry>
+    <key type="numeric" value="8192" />
+    <val type="string" value="\x02@ABC\x01" />
+  </entry>
+</attr>
+<attr name="SpecificMenu" type="list" id="2217369263296">
+</attr>
+<attr name="ParamsDictionary" type="dict" id="2217369504960">
+</attr>
+<attr name="UserMapping" type="dict" id="2217355415632">
+  <entry>
+    <key type="numeric" value="8192" />
+    <val type="dict" id="2217369504832">
+      <entry>
+        <key type="string" value="need" />
+        <val type="False" value="" />
+      </entry>
+      <entry>
+        <key type="string" value="values" />
+        <val type="list" id="2217369261952">
+          <item type="dict" id="2217352937216">
+            <entry>
+              <key type="string" value="access" />
+              <val type="string" value="rw" />
+            </entry>
+            <entry>
+              <key type="string" value="pdo" />
+              <val type="True" value="" />
+            </entry>
+            <entry>
+              <key type="string" value="type" />
+              <val type="numeric" value="15" />
+            </entry>
+            <entry>
+              <key type="string" value="name" />
+              <val type="string" value="Domain" />
+            </entry>
+          </item>
+        </val>
+      </entry>
+      <entry>
+        <key type="string" value="name" />
+        <val type="string" value="Domain" />
+      </entry>
+      <entry>
+        <key type="string" value="struct" />
+        <val type="numeric" value="1" />
+      </entry>
+    </val>
+  </entry>
+</attr>
+<attr name="DS302" type="dict" id="2217369297168">
+</attr>
+<attr name="ProfileName" type="string" value="None" />
+<attr name="Type" type="string" value="master" />
+<attr name="ID" type="numeric" value="0" />
+<attr name="Name" type="string" value="domain" />
+</PyObject>