Fix multi-unit symbol bugs.

1) SPICE pins need to include all units
2) Already-seen-unit-x needs to be reset when getting netlist again.

Fixes https://gitlab.com/kicad/code/kicad/issues/13164

eeschema/dialogs/dialog_sim_model.cpp

@@ -63,9 +63,7 @@ DIALOG_SIM_MODEL<T_symbol, T_field>::DIALOG_SIM_MODEL( wxWindow* aParent, T_symb
 {
     m_browseButton->SetBitmap( KiBitmap( BITMAPS::small_folder ) );
 
-    // Note that to get ALL pins, not only of the current part, you need to use
+    m_sortedPartPins = m_symbol.GetAllLibPins();
-    // m_symbol.GetRawPins().
-    m_sortedPartPins = m_symbol.GetLibPins();
 
     std::sort( m_sortedPartPins.begin(), m_sortedPartPins.end(),
                []( const LIB_PIN* lhs, const LIB_PIN* rhs )
@@ -247,9 +245,9 @@ bool DIALOG_SIM_MODEL<T_symbol, T_field>::TransferDataToWindow()
         try
         {
             if( m_useInstanceModelRadioButton->GetValue() && type == m_curModelType )
-                m_builtinModelsMgr.CreateModel( m_fields, m_symbol.GetPinCount() );
+                m_builtinModelsMgr.CreateModel( m_fields, m_symbol.GetFullPinCount() );
             else
-                m_builtinModelsMgr.CreateModel( type, m_symbol.GetPinCount() );
+                m_builtinModelsMgr.CreateModel( type, m_symbol.GetFullPinCount() );
         }
         catch( const IO_ERROR& e )
         {
@@ -665,9 +663,9 @@ void DIALOG_SIM_MODEL<T_symbol, T_field>::loadLibrary( const wxString& aLibraryP
         for( auto& [baseModelName, baseModel] : library()->GetModels() )
         {
             if( baseModelName == modelName )
-                m_libraryModelsMgr.CreateModel( baseModel, m_symbol.GetPinCount(), m_fields );
+                m_libraryModelsMgr.CreateModel( baseModel, m_symbol.GetFullPinCount(), m_fields );
             else
-                m_libraryModelsMgr.CreateModel( baseModel, m_symbol.GetPinCount() );
+                m_libraryModelsMgr.CreateModel( baseModel, m_symbol.GetFullPinCount() );
         }
     }
     catch( const IO_ERROR& e )

eeschema/dialogs/dialog_symbol_properties.cpp

@@ -1241,7 +1241,7 @@ void DIALOG_SYMBOL_PROPERTIES::OnUnitChoice( wxCommandEvent& event )
         m_dataModel->SortRows( COL_NUMBER, true );
         m_dataModel->BuildAttrs();
 
-        m_symbol->SetUnit(old_unit );
+        m_symbol->SetUnit( old_unit );
 
         // Restore m_Flag modified by SetUnit()
         m_symbol->ClearFlags();

eeschema/lib_symbol.cpp

@@ -685,7 +685,7 @@ void LIB_SYMBOL::Print( const RENDER_SETTINGS* aSettings, const VECTOR2I& aOffse
         }
         else if( item.Type() == LIB_FIELD_T )
         {
-            item.Print( aSettings, aOffset, (void*) NULL, aOpts.transform, aDimmed );
+            item.Print( aSettings, aOffset, nullptr, aOpts.transform, aDimmed );
         }
         else if( item.Type() == LIB_SHAPE_T )
         {
@@ -878,11 +878,11 @@ void LIB_SYMBOL::GetPins( LIB_PINS& aList, int aUnit, int aConvert ) const
 }
 
 
-std::vector<LIB_PIN*> LIB_SYMBOL::GetLibPins() const
+std::vector<LIB_PIN*> LIB_SYMBOL::GetAllLibPins() const
 {
     std::vector<LIB_PIN*> pinList;
 
-    GetPins( pinList );
+    GetPins( pinList, 0, 0 );
     return pinList;
 }
 
@@ -894,12 +894,12 @@ LIB_PIN* LIB_SYMBOL::GetPin( const wxString& aNumber, int aUnit, int aConvert )
 
     GetPins( pinList, aUnit, aConvert );
 
-    for( size_t i = 0; i < pinList.size(); i++ )
+    for( LIB_PIN* pin : pinList )
     {
-        wxASSERT( pinList[i]->Type() == LIB_PIN_T );
+        wxASSERT( pin->Type() == LIB_PIN_T );
 
-        if( aNumber == pinList[i]->GetNumber() )
+        if( aNumber == pin->GetNumber() )
-            return pinList[i];
+            return pin;
     }
 
     return nullptr;

eeschema/lib_symbol.h

@@ -409,8 +409,8 @@ public:
      *
      * This is just a pin object specific version of GetNextDrawItem().
      *
-     * @param aItem - Pointer to the previous pin item, or NULL to get the
+     * @param aItem - Pointer to the previous pin item, or NULL to get the first pin in the draw
-     *                first pin in the draw object list.
+     *                object list.
      * @return - The next pin object in the list if found, otherwise NULL.
      */
     LIB_PIN* GetNextPin( LIB_PIN* aItem = nullptr )
@@ -421,36 +421,41 @@ public:
     /**
      * Return a list of pin object pointers from the draw item list.
      *
-     * Note pin objects are owned by the draw list of the symbol.
+     * Note pin objects are owned by the draw list of the symbol.  Deleting any of the objects
-     * Deleting any of the objects will leave list in a unstable state
+     * will leave list in a unstable state and will likely segfault when the list is destroyed.
-     * and will likely segfault when the list is destroyed.
      *
      * @param aList - Pin list to place pin object pointers into.
-     * @param aUnit - Unit number of pin to add to list.  Set to 0 to
+     * @param aUnit - Unit number of pins to collect.  Set to 0 to get pins from any symbol unit.
-     *                get pins from any symbol unit.
+     * @param aConvert - Convert number of pins to collect.  Set to 0 to get pins from any
-     * @param aConvert - Convert number of pin to add to list.  Set to 0 to
+     *                   DeMorgan variant of symbol.
-     *                   get pins from any convert of symbol.
      */
     void GetPins( LIB_PINS& aList, int aUnit = 0, int aConvert = 0 ) const;
 
-    std::vector<LIB_PIN*> GetLibPins() const;
+    /**
+     * Return a list of pin pointers for all units / converts.  Used primarily for SPICE where
+     * we want to treat all unit as a single part.
+     */
+    std::vector<LIB_PIN*> GetAllLibPins() const;
+
+    /**
+     * @return a count of pins for all units / converts.
+     */
+    size_t GetFullPinCount() { return GetAllLibPins().size(); }
 
     /**
      * Return pin object with the requested pin \a aNumber.
      *
      * @param aNumber - Number of the pin to find.
-     * @param aUnit - Unit of the symbol to find.  Set to 0 if a specific
+     * @param aUnit - Unit filter.  Set to 0 if a specific unit number is not required.
-     *                unit number is not required.
+     * @param aConvert - DeMorgan variant filter.  Set to 0 if no specific DeMorgan variant is
-     * @param aConvert - Alternate body style filter (DeMorgan).  Set to 0 if
+     *                   required.
-     *                   no alternate body style is required.
      * @return The pin object if found.  Otherwise NULL.
      */
     LIB_PIN* GetPin( const wxString& aNumber, int aUnit = 0, int aConvert = 0 ) const;
 
     /**
-     * Return true if this symbol's pins do not match another symbol's pins. This
+     * Return true if this symbol's pins do not match another symbol's pins. This is used to
-     * is used to detect whether the project cache is out of sync with the
+     * detect whether the project cache is out of sync with the system libs.
-     * system libs.
      *
      * @param aOtherSymbol - The other library symbol to test
      * @param aTestNums - Whether two pins at the same point must have the same number.

eeschema/netlist_exporters/netlist_exporter_spice.cpp

@@ -166,6 +166,7 @@ bool NETLIST_EXPORTER_SPICE::ReadSchematicAndLibraries( unsigned aNetlistOptions
 
     m_nets.clear();
     m_items.clear();
+    m_referencesAlreadyFound.Clear();
     m_libParts.clear();
 
     wxFileName cacheDir;

eeschema/sch_symbol.cpp

@@ -37,6 +37,8 @@
 #include <refdes_utils.h>
 #include <wx/log.h>
 #include <string_utils.h>
+
+#include <utility>
 #include "plotters/plotter.h"
 
 
@@ -997,11 +999,13 @@ void SCH_SYMBOL::GetLibPins( std::vector<LIB_PIN*>& aPinsList ) const
 }
 
 
-std::vector<LIB_PIN*> SCH_SYMBOL::GetLibPins() const
+std::vector<LIB_PIN*> SCH_SYMBOL::GetAllLibPins() const
 {
     std::vector<LIB_PIN*> pinList;
 
-    GetLibPins( pinList );
+    if( m_part )
+        m_part->GetPins( pinList, 0, 0 );
+
     return pinList;
 }
 
@@ -1040,7 +1044,7 @@ std::vector<SCH_PIN*> SCH_SYMBOL::GetPins( const SCH_SHEET_PATH* aSheet ) const
 
 void SCH_SYMBOL::SwapData( SCH_ITEM* aItem )
 {
-    wxCHECK_RET( (aItem != nullptr) && (aItem->Type() == SCH_SYMBOL_T),
+    wxCHECK_RET( aItem != nullptr && aItem->Type() == SCH_SYMBOL_T,
                  wxT( "Cannot swap data with invalid symbol." ) );
 
     SCH_SYMBOL* symbol = (SCH_SYMBOL*) aItem;
@@ -1048,7 +1052,7 @@ void SCH_SYMBOL::SwapData( SCH_ITEM* aItem )
     std::swap( m_lib_id, symbol->m_lib_id );
 
     LIB_SYMBOL* libSymbol = symbol->m_part.release();
-    symbol->m_part.reset( m_part.release() );
+    symbol->m_part = std::move( m_part );
     symbol->UpdatePins();
     m_part.reset( libSymbol );
     UpdatePins();

eeschema/sch_symbol.h

@@ -518,14 +518,19 @@ public:
     void GetLibPins( std::vector<LIB_PIN*>& aPinsList ) const;
 
     /**
-     * Return a vector with all the pins from the library object.
+     * @return a list of pin pointers for all units / converts.  Used primarily for SPICE where
-     *
+     * we want to treat all units together as a single SPICE element.
-     * @return List of the pins
      */
-    std::vector<LIB_PIN*> GetLibPins() const;
+    std::vector<LIB_PIN*> GetAllLibPins() const;
 
-    size_t GetPinCount() { return GetLibPins().size(); }
+    /**
+     * @return a count of pins for all units / converts.
+     */
+    size_t GetFullPinCount() { return GetAllLibPins().size(); }
 
+    /**
+     * @return the SCH_PIN associated with a particular LIB_PIN.
+     */
     SCH_PIN* GetPin( LIB_PIN* aLibPin ) const;
 
     /**

eeschema/sim/sim_model.cpp

@@ -1089,7 +1089,7 @@ bool SIM_MODEL::InferPassiveSimModel( T& aSymbol, bool aResolve,
                                      aValue );
             }
 
-            std::vector<LIB_PIN*> pins = aSymbol.GetLibPins();
+            std::vector<LIB_PIN*> pins = aSymbol.GetAllLibPins();
 
             if( pins.size() == 2 )
             {
@@ -1271,7 +1271,7 @@ void SIM_MODEL::MigrateSimModel( T_symbol& aSymbol, const PROJECT* aProject )
 
             wxStringSplit( legacyPins->GetText(), pinIndexes, ' ' );
 
-            std::vector<LIB_PIN*> sourcePins = aSymbol.GetLibPins();
+            std::vector<LIB_PIN*> sourcePins = aSymbol.GetAllLibPins();
 
             if( isPassive && pinIndexes.size() == 2 && sourcePins.size() == 2 )
             {
@@ -1321,7 +1321,7 @@ void SIM_MODEL::MigrateSimModel( T_symbol& aSymbol, const PROJECT* aProject )
         {
             std::vector<T_field> emptyFields;
             SIM_LIBRARY::MODEL model = libMgr.CreateModel( spiceLib, spiceModel.ToStdString(),
-                                                           emptyFields, aSymbol.GetPinCount() );
+                                                           emptyFields, aSymbol.GetFullPinCount() );
 
             spiceParams = wxString( model.model.GetBaseModel()->Serde().GenerateParams() );
         }
@@ -1392,7 +1392,7 @@ void SIM_MODEL::MigrateSimModel( T_symbol& aSymbol, const PROJECT* aProject )
     {
         // Generate a 1:1 pin map.  We don't necessarily know the SPICE model pinNames, so just
         // use indexes.
-        std::vector<LIB_PIN*> sourcePins = aSymbol.GetLibPins();
+        std::vector<LIB_PIN*> sourcePins = aSymbol.GetAllLibPins();
 
         for( unsigned ii = 0; ii < sourcePins.size(); ++ii )
         {

eeschema/tools/sch_editor_control.cpp

@@ -874,7 +874,6 @@ int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent )
                     {
                         LIB_PIN* pin = static_cast<SCH_PIN*>( item )->GetLibPin();
                         SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item->GetParent() );
-                        std::vector<LIB_PIN*> pins = symbol->GetLibPins();
 
                         SIM_LIB_MGR mgr( &m_frame->Prj() );
                         SIM_MODEL&  model = mgr.CreateModel( &sheet, *symbol ).model;