Fixes for extra fields handling

Add purpose class ExtraFieldData that parsers should return.
It can store field data by index instead of by ref to work around
the issue of duplicate references.

Issue #385

Author: qu1ck

InteractiveHtmlBom/core/ibom.py

@@ -138,9 +138,12 @@ def natural_sort(lst):
                     group_key.append(f.footprint)
                     group_key.append(f.attr)
             else:
-                fields.append(f.extra_fields.get(field, ''))
+                field_key = field
+                if config.normalize_field_case:
+                    field_key = field.lower()
+                fields.append(f.extra_fields.get(field_key, ''))
                 if field in group_by:
-                    group_key.append(f.extra_fields.get(field, ''))
+                    group_key.append(f.extra_fields.get(field_key, ''))
 
         index_to_fields[i] = fields
         refs = part_groups.setdefault(tuple(group_key), [])

InteractiveHtmlBom/dialog/dialog_base.py

@@ -546,7 +546,7 @@ def __init__( self, parent, id = wx.ID_ANY, pos = wx.DefaultPosition, size = wx.
         self.fieldsGrid.Bind( wx.grid.EVT_GRID_CELL_LEFT_CLICK, self.OnGridCellClicked )
         self.m_btnUp.Bind( wx.EVT_BUTTON, self.OnFieldsUp )
         self.m_btnDown.Bind( wx.EVT_BUTTON, self.OnFieldsDown )
-        self.normalizeCaseCheckbox.Bind( wx.EVT_CHECKBOX, self.OnNetlistFileChanged )
+        self.normalizeCaseCheckbox.Bind( wx.EVT_CHECKBOX, self.OnExtraDataFileChanged )
         self.boardVariantFieldBox.Bind( wx.EVT_COMBOBOX, self.OnBoardVariantFieldChange )
 
     def __del__( self ):
@@ -569,8 +569,6 @@ def OnFieldsUp( self, event ):
     def OnFieldsDown( self, event ):
         event.Skip()
 
-    def OnNetlistFileChanged( self, event ):
-        event.Skip()
 
     def OnBoardVariantFieldChange( self, event ):
         event.Skip()

InteractiveHtmlBom/dialog/settings_dialog.py

@@ -222,6 +222,9 @@ class FieldsPanel(dialog_base.FieldsPanelBase):
 
     def __init__(self, parent, extra_data_func, extra_data_wildcard):
         dialog_base.FieldsPanelBase.__init__(self, parent)
+        self.show_fields = []
+        self.group_fields = []
+
         self.extra_data_func = extra_data_func
         self.extra_field_data = None
 
@@ -333,8 +336,9 @@ def OnExtraDataFileChanged(self, event):
             self.extraDataFilePicker.Path = ''
 
         if self.extra_field_data is not None:
-            field_list = list(self.extra_field_data[0])
+            field_list = list(self.extra_field_data.fields)
             self._setFieldsList(["Value", "Footprint"] + field_list)
+            self.SetCheckedFields()
             field_list.append(self.NONE_STRING)
             self.boardVariantFieldBox.SetItems(field_list)
             self.boardVariantFieldBox.SetStringSelection(self.NONE_STRING)
@@ -351,7 +355,7 @@ def OnBoardVariantFieldChange(self, event):
             self.boardVariantBlacklist.Clear()
             return
         variant_set = set()
-        for _, field_dict in self.extra_field_data[1].items():
+        for _, field_dict in self.extra_field_data.fields_by_ref.items():
             if selection in field_dict:
                 variant_set.add(field_dict[selection])
         self.boardVariantWhitelist.SetItems(list(variant_set))
@@ -377,14 +381,20 @@ def GetGroupFields(self):
                 result.append(self.fieldsGrid.GetCellValue(row, 2))
         return result
 
-    def SetCheckedFields(self, show, group):
-        group = [s for s in group if s in show]
+    def SetCheckedFields(self, show=None, group=None):
+        self.show_fields = show or self.show_fields
+        self.group_fields = group or self.group_fields
+        self.group_fields = [
+            s for s in self.group_fields if s in self.show_fields
+        ]
         current = []
         for row in range(self.fieldsGrid.NumberRows):
             current.append(self.fieldsGrid.GetCellValue(row, 2))
-        new = [s for s in current if s not in show]
-        self._setFieldsList(show + new)
+        new = [s for s in current if s not in self.show_fields]
+        self._setFieldsList(self.show_fields + new)
         for row in range(self.fieldsGrid.NumberRows):
             field = self.fieldsGrid.GetCellValue(row, 2)
-            self.fieldsGrid.SetCellValue(row, 0, "1" if field in show else "")
-            self.fieldsGrid.SetCellValue(row, 1, "1" if field in group else "")
+            self.fieldsGrid.SetCellValue(
+                row, 0, "1" if field in self.show_fields else "")
+            self.fieldsGrid.SetCellValue(
+                row, 1, "1" if field in self.group_fields else "")

InteractiveHtmlBom/ecad/common.py

@@ -3,6 +3,13 @@
 from .svgpath import parse_path
 
 
+class ExtraFieldData(object):
+    def __init__(self, fields, fields_by_ref, fields_by_index=None):
+        self.fields = fields
+        self.fields_by_ref = fields_by_ref
+        self.fields_by_index = fields_by_index
+
+
 class EcadParser(object):
 
     def __init__(self, file_name, config, logger):
@@ -28,33 +35,28 @@ def parse(self):
 
     @staticmethod
     def normalize_field_names(data):
-        field_map = {f.lower(): f for f in reversed(data[0])}
-
+        # type: (ExtraFieldData) -> ExtraFieldData
         def remap(ref_fields):
-            return {field_map[f.lower()]: v for (f, v) in
-                    sorted(ref_fields.items(), reverse=True)}
+            return {f.lower(): v for (f, v) in
+                    sorted(ref_fields.items(), reverse=True) if v}
+
+        by_ref = {r: remap(d) for (r, d) in data.fields_by_ref.items()}
+        if data.fields_by_index:
+            by_index = {i: remap(d) for (i, d) in data.fields_by_index.items()}
+            print([a.get("blah", "") for a in by_index.values()])
+        else:
+            by_index = None
 
-        field_data = {r: remap(d) for (r, d) in data[1].items()}
-        return field_map.values(), field_data
+        field_map = {f.lower(): f for f in sorted(data.fields, reverse=True)}
+        return ExtraFieldData(field_map.values(), by_ref, by_index)
 
     def get_extra_field_data(self, file_name):
         """
         Abstract method that may be overridden in implementations that support
         extra field data.
-        :return: tuple of the format
-            (
-                [field_name1, field_name2,... ],
-                {
-                    ref1: {
-                        field_name1: field_value1,
-                        field_name2: field_value2,
-                        ...
-                    ],
-                    ref2: ...
-                }
-            )
+        :return: ExtraFieldData
         """
-        return [], {}
+        return ExtraFieldData([], {})
 
     def parse_extra_data(self, file_name, normalize_case):
         """
@@ -67,7 +69,8 @@ def parse_extra_data(self, file_name, normalize_case):
         data = self.get_extra_field_data(file_name)
         if normalize_case:
             data = self.normalize_field_names(data)
-        return sorted(data[0]), data[1]
+        return ExtraFieldData(
+            sorted(data.fields), data.fields_by_ref, data.fields_by_index)
 
     def latest_extra_data(self, extra_dirs=None):
         """

InteractiveHtmlBom/ecad/genericjson.py

@@ -3,7 +3,7 @@
 import os.path
 from jsonschema import validate, ValidationError
 
-from .common import EcadParser, Component, BoundingBox
+from .common import EcadParser, Component, BoundingBox, ExtraFieldData
 from ..core.fontparser import FontParser
 from ..errors import ParsingException
 
@@ -32,7 +32,11 @@ def get_extra_field_data(self, file_name):
                 field_set.add(k)
                 ref_fields[k] = v
 
-        return list(field_set), comp_dict
+        by_index = {
+            i: components[i].extra_fields for i in range(len(components))
+        }
+
+        return ExtraFieldData(list(field_set), comp_dict, by_index)
 
     def get_generic_json_pcb(self):
         with io.open(self.file_name, 'r', encoding='utf-8') as f:

InteractiveHtmlBom/ecad/kicad.py

@@ -3,7 +3,7 @@
 
 import pcbnew
 
-from .common import EcadParser, Component
+from .common import EcadParser, Component, ExtraFieldData
 from .kicad_extra import find_latest_schematic_data, parse_schematic_data
 from .svgpath import create_path
 from ..core import ibom
@@ -29,22 +29,29 @@ def get_extra_field_data(self, file_name):
             return self.parse_extra_data_from_pcb()
         if os.path.splitext(file_name)[1] == '.kicad_pcb':
             return None
-        return parse_schematic_data(file_name)
+
+        data = parse_schematic_data(file_name)
+
+        return ExtraFieldData(data[0], data[1])
 
     def parse_extra_data_from_pcb(self):
         field_set = set()
-        comp_dict = {}
+        by_ref = {}
 
         for f in self.footprints:  # type: pcbnew.FOOTPRINT
             props = f.GetProperties()
             ref = f.GetReference()
-            ref_fields = comp_dict.setdefault(ref, {})
+            ref_fields = by_ref.setdefault(ref, {})
 
             for k, v in props.items():
                 field_set.add(k)
                 ref_fields[k] = v
 
-        return list(field_set), comp_dict
+        by_index = {
+            i: f.GetProperties() for (i, f) in enumerate(self.footprints)
+        }
+
+        return ExtraFieldData(list(field_set), by_ref, by_index)
 
     def latest_extra_data(self, extra_dirs=None):
         base_name = os.path.splitext(os.path.basename(self.file_name))[0]
@@ -685,8 +692,6 @@ def parse(self):
             raise ParsingException(
                 'Failed parsing %s' % self.config.extra_data_file)
 
-        extra_field_data = extra_field_data[1] if extra_field_data else None
-
         title_block = self.board.GetTitleBlock()
         title = title_block.GetTitle()
         revision = title_block.GetRevision()
@@ -752,26 +757,33 @@ def parse(self):
         if self.config.include_nets and hasattr(self.board, "GetNetInfo"):
             pcbdata["nets"] = self.parse_netlist(self.board.GetNetInfo())
 
-        warning_shown = False
         if extra_field_data and need_extra_fields:
-            e = []
-            for f in self.footprints:
-                e.append(extra_field_data.get(f.GetReference(), {}))
-                if f.GetReference() not in extra_field_data:
-                    # Some components are on pcb but not in schematic data.
-                    # Show a warning about possibly outdated netlist/xml file.
-                    self.logger.warn(
-                        'Component %s is missing from schematic data.'
-                        % f.GetReference())
-                    warning_shown = True
+            extra_fields = extra_field_data.fields_by_index
+            if extra_fields:
+                extra_fields = extra_fields.values()
+
+            if extra_fields is None:
+                extra_fields = []
+                field_map = extra_field_data.fields_by_ref
+                warning_shown = False
+
+                for f in self.footprints:
+                    extra_fields.append(field_map.get(f.GetReference(), {}))
+                    if f.GetReference() not in field_map:
+                        # Some components are on pcb but not in schematic data.
+                        # Show a warning about outdated extra data file.
+                        self.logger.warn(
+                            'Component %s is missing from schematic data.'
+                            % f.GetReference())
+                        warning_shown = True
+
+                if warning_shown:
+                    self.logger.warn('Netlist/xml file is likely out of date.')
         else:
-            e = [{}] * len(self.footprints)
-
-        if warning_shown:
-            self.logger.warn('Netlist/xml file is likely out of date.')
+            extra_fields = [{}] * len(self.footprints)
 
-        components = [self.footprint_to_component(f, ee)
-                      for (f, ee) in zip(self.footprints, e)]
+        components = [self.footprint_to_component(f, e)
+                      for (f, e) in zip(self.footprints, extra_fields)]
 
         return pcbdata, components
 

settings_dialog.fbp

@@ -3284,7 +3284,7 @@
                                 <property name="window_extra_style"></property>
                                 <property name="window_name"></property>
                                 <property name="window_style"></property>
-                                <event name="OnCheckBox">OnNetlistFileChanged</event>
+                                <event name="OnCheckBox">OnExtraDataFileChanged</event>
                             </object>
                         </object>
                     </object>