PCB Fields: fix up python, API

Remove SetFields from both symbol and footprint, it doesn't handle
mandatory fields and is unlikely to ever be safe.

eeschema/sch_symbol.h

@@ -463,16 +463,6 @@ public:
      */
     SCH_FIELD* FindField( const wxString& aFieldName, bool aIncludeDefaultFields = true );
 
-    /**
-     * Set multiple schematic fields.
-     *
-     * @param aFields are the fields to set in this symbol.
-     */
-    void SetFields( std::vector<SCH_FIELD>& aFields )
-    {
-        m_fields = aFields;     // vector copying, length is changed possibly
-    }
-
     const wxString GetValueFieldText( bool aResolve, const SCH_SHEET_PATH* aPath,
                                       bool aAllowExtraText ) const;
     void SetValueFieldText( const wxString& aValue );

pcbnew/dialogs/dialog_footprint_properties.cpp

@@ -472,12 +472,10 @@ bool DIALOG_FOOTPRINT_PROPERTIES::TransferDataFromWindow()
             field->DeleteStructure();
     }
 
-    // if there are still grid table entries, create new texts for them
+    // if there are still grid table entries, create new fields for them
     while( i < m_fields->size() )
     {
-        PCB_FIELD* newField = m_fields->at( i++ );
+        view->Add( m_footprint->AddField( *m_fields->at( i++ ) ) );
-        m_footprint->Add( newField, ADD_MODE::APPEND );
-        view->Add( newField );
     }
 
     // Initialize masks clearances

pcbnew/dialogs/dialog_footprint_properties_fp_editor.cpp

@@ -520,12 +520,10 @@ bool DIALOG_FOOTPRINT_PROPERTIES_FP_EDITOR::TransferDataFromWindow()
     }
 
 
-    // if there are still grid table entries, create new texts for them
+    // if there are still grid table entries, create new fields for them
     while( i < m_fields->size() )
     {
-        PCB_FIELD* field = m_fields->at( i++ );
+        view->Add( m_footprint->AddField( *m_fields->at( i++ ) ) );
-        m_footprint->AddField( field );
-        view->Add( field );
     }
 
     LSET privateLayers;

pcbnew/footprint.cpp

@@ -80,7 +80,8 @@ FOOTPRINT::FOOTPRINT( BOARD* parent ) :
     // These are the mandatory fields for the editor to work
     for( int i = 0; i < MANDATORY_FIELDS; i++ )
     {
-        PCB_FIELD* field = AddField( new PCB_FIELD( this, i ) );
+        PCB_FIELD* field = new PCB_FIELD( this, i );
+        m_fields.push_back( field );
 
         // Style according to the board settings if we have them
         if( parent )
@@ -333,11 +334,11 @@ void FOOTPRINT::GetFields( std::vector<PCB_FIELD*>& aVector, bool aVisibleOnly )
 }
 
 
-PCB_FIELD* FOOTPRINT::AddField( PCB_FIELD* aField )
+PCB_FIELD* FOOTPRINT::AddField( const PCB_FIELD& aField )
 {
     int newNdx = m_fields.size();
 
-    m_fields.push_back( aField );
+    m_fields.push_back( new PCB_FIELD( aField ) );
     return m_fields[newNdx];
 }
 
@@ -696,10 +697,8 @@ void FOOTPRINT::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode, bool aSkipConnectiv
     switch( aBoardItem->Type() )
     {
     case PCB_FIELD_T:
-        if( aMode == ADD_MODE::APPEND )
+        // Always append fields
-            m_fields.push_back( static_cast<PCB_FIELD*>( aBoardItem ) );
+        m_fields.push_back( static_cast<PCB_FIELD*>( aBoardItem ) );
-        else
-            m_fields.push_front( static_cast<PCB_FIELD*>( aBoardItem ) );
 
         break;
 

pcbnew/footprint.h

@@ -634,7 +634,7 @@ public:
      *
      * @return the newly inserted field.
      */
-    PCB_FIELD* AddField( PCB_FIELD* aField );
+    PCB_FIELD* AddField( const PCB_FIELD& aField );
 
     /**
      * Remove a user field from the footprint.
@@ -643,15 +643,6 @@ public:
      */
     void RemoveField( const wxString& aFieldName );
 
-    void RemoveField( PCB_FIELD* aField ) { RemoveField( aField->GetName() ); }
-
-    /**
-     * Set multiple schematic fields.
-     *
-     * @param aFields are the fields to set in this symbol.
-     */
-    void SetFields( PCB_FIELDS aFields ) { m_fields = aFields; }
-
     /**
      * Return the number of fields in this symbol.
      */

pcbnew/netlist_reader/board_netlist_updater.cpp

@@ -366,15 +366,13 @@ bool BOARD_NETLIST_UPDATER::updateFootprintParameters( FOOTPRINT* aPcbFootprint,
                     aPcbFootprint->GetFieldByName( pair.first )->SetText( pair.second );
                 else
                 {
-                    PCB_FIELD* newField = new PCB_FIELD(
+                    PCB_FIELD* newField = aPcbFootprint->AddField( PCB_FIELD(
-                            aPcbFootprint, aPcbFootprint->GetFieldCount(), pair.first );
+                            aPcbFootprint, aPcbFootprint->GetFieldCount(), pair.first ) );
 
                     newField->SetText( pair.second );
                     newField->SetVisible( false );
                     newField->SetLayer( aPcbFootprint->GetLayer() == F_Cu ? F_Fab : B_Fab );
                     newField->StyleFromSettings( m_frame->GetDesignSettings() );
-
-                    aPcbFootprint->AddField( newField );
                 }
             }
         }

pcbnew/pcb_edit_frame.cpp

@@ -2280,6 +2280,21 @@ void PCB_EDIT_FRAME::ExchangeFootprint( FOOTPRINT* aExisting, FOOTPRINT* aNew,
         }
     }
 
+    for( PCB_FIELD* field : aExisting->GetFields() )
+    {
+        PCB_FIELD* newField = aNew->GetFieldByName( field->GetName() );
+
+        if( !newField )
+        {
+            newField = new PCB_FIELD( *field );
+            aNew->Add( newField );
+            processTextItem( *field, *newField, true, true, true, aUpdated );
+        }
+        else
+            processTextItem( *field, *newField, false, resetTextLayers, resetTextEffects, aUpdated );
+
+    }
+
     if( resetFabricationAttrs )
     {
         // We've replaced the existing footprint with the library one, so the fabrication attrs
@@ -2310,7 +2325,6 @@ void PCB_EDIT_FRAME::ExchangeFootprint( FOOTPRINT* aExisting, FOOTPRINT* aNew,
 
     // Updating other parameters
     const_cast<KIID&>( aNew->m_Uuid ) = aExisting->m_Uuid;
-    aNew->SetFields( aExisting->GetFields() );
     aNew->SetPath( aExisting->GetPath() );
 
     aCommit.Remove( aExisting );

pcbnew/plugins/kicad/pcb_parser.cpp

@@ -3856,13 +3856,12 @@ FOOTPRINT* PCB_PARSER::parseFOOTPRINT_unchecked( wxArrayString* aInitialComments
             }
             else
             {
-                field = new PCB_FIELD( footprint.get(), footprint->GetFieldCount(), pName );
+                field = footprint->AddField(
+                        PCB_FIELD( footprint.get(), footprint->GetFieldCount(), pName ) );
 
                 field->SetText( pValue );
                 field->SetLayer( footprint->GetLayer() == F_Cu ? F_Fab : B_Fab );
                 field->StyleFromSettings( m_board->GetDesignSettings() );
-
-                footprint->AddField( field );
             }
 
             field->SetVisible( true );

pcbnew/python/swig/board.i

@@ -55,6 +55,7 @@ HANDLE_EXCEPTIONS(BOARD::TracksInNetBetweenPoints)
 %include zone_settings.i
 %include pcb_shape.i
 %include pcb_text.i
+%include pcb_field.i
 %include pcb_dimension.i
 %include pcb_marker.i
 %include pcb_target.i

pcbnew/python/swig/footprint.i

@@ -31,7 +31,6 @@
 
 %template(MAP_STRING_STRING) std::map<wxString, wxString>;
 %rename(GetFieldsNative) FOOTPRINT::GetFields;
-%rename(SetFieldsNative) FOOTPRINT::SetFields;
 %feature("flatnested");
 %include footprint.h
 %feature("flatnested", "");
@@ -63,21 +62,30 @@
     def GetFields(self):
       """ Returns footprint fields map. """
       fields = self.GetFieldsNative()
-      return {str(k): str(v) for k, v in fields.items()}
+      return {str(field.GetName()): str(field.GetText()) for field in fields}
 
     def GetField(self, key):
       """ Returns Field with a given key if it exists, throws KeyError otherwise. """
-      if self.HasField(key):
+      if self.HasFieldByName(key):
-        return self.GetFieldByName(key)
+        return self.GetFieldByName(key).GetText()
       else:
         raise KeyError("Field not found: " + key)
 
+    def SetField(self, key, value):
+      if self.HasFieldByName(key):
+        self.GetFieldByName(key).SetText(value)
+      else:
+        field = PCB_FIELD(self, self.GetFieldCount(), key)
+        field.SetText(value)
+        self.AddField(field)
+
+    def HasField(self, key):
+      return self.HasFieldByName(key)
+
     def SetFields(self, fields):
       """ Sets footprint fields map. """
-      wxfields = MAP_STRING_STRING()
       for k, v in fields.items():
-        wxfields[k] = v
+        self.SetField(k, v)
-      self.SetFieldsNative(wxfields)
 
     %}
 }

pcbnew/python/swig/pcb_field.i

@@ -0,0 +1,7 @@
+
+
+%include pcb_field.h
+%{
+#include <pcb_field.h>
+%}
+

qa/tests/pcbnewswig/test_002_board_class.py

@@ -124,11 +124,10 @@ class TestBoardClass:
         pcb = LoadBoard("../data/pcbnew/custom_fields.kicad_pcb")
         footprint = pcb.FindFootprintByReference('J1')
         expected_fields = {
-            'Sheet file': 'custom_fields.kicad_sch',
-            'Sheet name': '',
             'myfield': 'myvalue'
         }
-        assert footprint.GetFields() == expected_fields
+        assert footprint.GetSheetfile() == 'custom_fields.kicad_sch'
+        assert footprint.GetSheetname() == ''
         assert footprint.GetField('myfield') == 'myvalue'
         assert footprint.HasField('myfield') == True
         assert footprint.HasField('abcd') == False