Bug fixes for simulator.

1) Finish removing pin-count based APIs.  We need to know the pin names
as well, not just the count.

2) Fix a bug in the regexp for SPICE functions to allow both lowercase
and uppercase function names

3) Move CreatePins() overrides to the new API so that we get pins on
raw SPICE models.

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

eeschema/dialogs/dialog_sim_model.cpp

@@ -1159,8 +1159,15 @@ void DIALOG_SIM_MODEL<T_symbol, T_field>::onDeviceTypeChoice( wxCommandEvent& aE
 template <typename T_symbol, typename T_field>
 void DIALOG_SIM_MODEL<T_symbol, T_field>::onTypeChoice( wxCommandEvent& aEvent )
 {
-    SIM_MODEL::DEVICE_T deviceType = curModel().GetDeviceType();
+    SIM_MODEL::DEVICE_T   deviceType = curModel().GetDeviceType();
-    wxString            typeDescription = m_typeChoice->GetString( m_typeChoice->GetSelection() );
+    wxString              typeDescription = m_typeChoice->GetString( m_typeChoice->GetSelection() );
+    std::vector<LIB_PIN*> sourcePins = m_symbol.GetAllLibPins();
+
+    std::sort( sourcePins.begin(), sourcePins.end(),
+               []( const LIB_PIN* lhs, const LIB_PIN* rhs )
+               {
+                   return StrNumCmp( lhs->GetNumber(), rhs->GetNumber(), true ) < 0;
+               } );
 
     for( SIM_MODEL::TYPE type : SIM_MODEL::TYPE_ITERATOR() )
     {
@@ -1177,7 +1184,8 @@ void DIALOG_SIM_MODEL<T_symbol, T_field>::onTypeChoice( wxCommandEvent& aEvent )
 
                 auto& kibisModel = static_cast<SIM_MODEL_KIBIS&>( m_libraryModelsMgr.GetModels().at( idx ).get() );
 
-                m_libraryModelsMgr.SetModel( idx, std::make_unique<SIM_MODEL_KIBIS>( type, kibisModel, m_fields ) );
+                m_libraryModelsMgr.SetModel( idx, std::make_unique<SIM_MODEL_KIBIS>( type, kibisModel,
+                                                                                     m_fields, sourcePins ) );
             }
 
             m_curModelType = type;

eeschema/sim/sim_model.cpp

@@ -412,20 +412,6 @@ TYPE SIM_MODEL::InferTypeFromLegacyFields( const std::vector<T>& aFields )
 }
 
 
-template <>
-void SIM_MODEL::ReadDataFields( unsigned aSymbolPinCount, const std::vector<SCH_FIELD>* aFields )
-{
-    doReadDataFields( aSymbolPinCount, aFields );
-}
-
-
-template <>
-void SIM_MODEL::ReadDataFields( unsigned aSymbolPinCount, const std::vector<LIB_FIELD>* aFields )
-{
-    doReadDataFields( aSymbolPinCount, aFields );
-}
-
-
 template <>
 void SIM_MODEL::ReadDataFields( const std::vector<SCH_FIELD>* aFields,
                                 const std::vector<LIB_PIN*>& aPins )
@@ -983,7 +969,7 @@ SIM_MODEL::SIM_MODEL( TYPE aType, std::unique_ptr<SPICE_GENERATOR> aSpiceGenerat
 }
 
 
-void SIM_MODEL::CreatePins( unsigned aSymbolPinCount )
+void SIM_MODEL::CreatePins( const std::vector<LIB_PIN*>& aSymbolPins )
 {
     // Default pin sequence: model pins are the same as symbol pins.
     // Excess model pins are set as Not Connected.
@@ -992,68 +978,23 @@ void SIM_MODEL::CreatePins( unsigned aSymbolPinCount )
     // SIM_MODEL pins must be ordered by symbol pin numbers -- this is assumed by the code that
     // accesses them.
 
-    for( unsigned modelPinIndex = 0; modelPinIndex < getPinNames().size(); ++modelPinIndex )
+    std::vector<std::string> pinNames = getPinNames();
-    {
-        if( modelPinIndex < aSymbolPinCount )
-            AddPin( { getPinNames().at( modelPinIndex ), fmt::format( "{}", modelPinIndex + 1 ) } );
-        else
-            AddPin( { getPinNames().at( modelPinIndex ), "" } );
-    }
-}
 
-
+    for( unsigned modelPinIndex = 0; modelPinIndex < pinNames.size(); ++modelPinIndex )
-void SIM_MODEL::CreatePins( const std::vector<LIB_PIN*> aSymbolPins )
-{
-    // Default pin sequence: model pins are the same as symbol pins.
-    // Excess model pins are set as Not Connected.
-    // Note that intentionally nothing is added if `getPinNames()` returns an empty vector.
-
-    // SIM_MODEL pins must be ordered by symbol pin numbers -- this is assumed by the code that
-    // accesses them.
-
-    for( unsigned modelPinIndex = 0; modelPinIndex < getPinNames().size(); ++modelPinIndex )
     {
         if( modelPinIndex < aSymbolPins.size() )
         {
-            AddPin( { getPinNames().at( modelPinIndex ),
+            AddPin( { pinNames.at( modelPinIndex ),
                       aSymbolPins[ modelPinIndex ]->GetNumber().ToStdString() } );
         }
         else
         {
-            AddPin( { getPinNames().at( modelPinIndex ), "" } );
+            AddPin( { pinNames.at( modelPinIndex ), "" } );
         }
     }
 }
 
 
-// TODO: remove this API.  If we have symbol fields, then we have symbol pins and we should be
-// creating a model with the real symbol pin names, not indexes...
-
-template <typename T>
-void SIM_MODEL::doReadDataFields( unsigned aSymbolPinCount, const std::vector<T>* aFields )
-{
-    bool diffMode = GetFieldValue( aFields, SIM_LIBRARY_KIBIS::DIFF_FIELD ) == "1";
-    SwitchSingleEndedDiff( diffMode );
-
-    try
-    {
-        m_serde->ParseEnable( GetFieldValue( aFields, ENABLE_FIELD ) );
-
-        CreatePins( aSymbolPinCount );
-        m_serde->ParsePins( GetFieldValue( aFields, PINS_FIELD ) );
-
-        std::string paramsField = GetFieldValue( aFields, PARAMS_FIELD );
-
-        if( !m_serde->ParseParams( paramsField ) )
-            m_serde->ParseValue( GetFieldValue( aFields, VALUE_FIELD ) );
-    }
-    catch( IO_ERROR& err )
-    {
-        DisplayErrorMessage( nullptr, err.What() );
-    }
-}
-
-
 template <typename T>
 void SIM_MODEL::doReadDataFields( const std::vector<T>* aFields,
                                   const std::vector<LIB_PIN*>& aPins )
@@ -1387,6 +1328,10 @@ void SIM_MODEL::MigrateSimModel( T_symbol& aSymbol, const PROJECT* aProject )
         return;
     }
 
+    spiceDeviceType = spiceDeviceType.Trim( true ).Trim( false );
+    spiceModel = spiceModel.Trim( true ).Trim( false );
+    spiceType = spiceType.Trim( true ).Trim( false );
+
     bool libraryModel = false;
     bool internalModel = false;
 
@@ -1417,10 +1362,10 @@ void SIM_MODEL::MigrateSimModel( T_symbol& aSymbol, const PROJECT* aProject )
     }
     else
     {
-        // Convert functional SPICE model syntax to name=value pairs.  For instance, "dc(1)"
+        // See if we have a function-style SPICE model (ie: "sin(0 1 60)") that can be handled
-        // needs to go to "dc=1", while "sin(0, 1, 60)" needs to go to "dc=0 ampl=1 f=60".
+        // by a built-in SIM_MODEL.
 
-        wxRegEx  regex( wxT( "^[a-z]+\\(.*\\)$" ) );
+        wxRegEx  regex( wxT( "^[a-zA-Z]+\\(.*\\)$" ) );
 
         if( regex.Matches( spiceModel ) )
         {

eeschema/sim/sim_model.h

@@ -433,9 +433,6 @@ public:
     SIM_MODEL( SIM_MODEL&& aOther ) = default;
     SIM_MODEL& operator=(SIM_MODEL&& aOther ) = delete;
 
-    template <typename T>
-    void ReadDataFields( unsigned aSymbolPinCount, const std::vector<T>* aFields );
-
     template <typename T>
     void ReadDataFields( const std::vector<T>* aFields, const std::vector<LIB_PIN*>& aPins );
 
@@ -537,13 +534,9 @@ protected:
     SIM_MODEL( TYPE aType, std::unique_ptr<SPICE_GENERATOR> aSpiceGenerator,
                std::unique_ptr<SIM_SERDE> aSerde );
 
-    virtual void CreatePins( unsigned aSymbolPinCount );
+    virtual void CreatePins( const std::vector<LIB_PIN*>& aSymbolPins );
-    virtual void CreatePins( const std::vector<LIB_PIN*> aSymbolPins );
 
 private:
-    template <typename T>
-    void doReadDataFields( unsigned aSymbolPinCount, const std::vector<T>* aFields );
-
     template <typename T>
     void doReadDataFields( const std::vector<T>* aFields, const std::vector<LIB_PIN*>& aPins );
 

eeschema/sim/sim_model_kibis.cpp

@@ -278,23 +278,28 @@ SIM_MODEL_KIBIS::SIM_MODEL_KIBIS( TYPE aType, const SIM_MODEL_KIBIS& aSource ) :
     m_enableDiff = aSource.CanDifferential();
 }
 
-SIM_MODEL_KIBIS::SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource,
-                                  const std::vector<LIB_FIELD>& aFields ) :
-        SIM_MODEL_KIBIS( aType, aSource )
-{
-    ReadDataFields( 2, &aFields );
-}
 
 SIM_MODEL_KIBIS::SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource,
-                                  const std::vector<SCH_FIELD>& aFields ) :
+                                  const std::vector<LIB_FIELD>& aFields,
+                                  const std::vector<LIB_PIN*>& aPins  ) :
         SIM_MODEL_KIBIS( aType, aSource )
 {
+    ReadDataFields( &aFields, aPins );
 }
 
 
-void SIM_MODEL_KIBIS::CreatePins( unsigned aSymbolPinCount )
+SIM_MODEL_KIBIS::SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource,
+                                  const std::vector<SCH_FIELD>& aFields,
+                                  const std::vector<LIB_PIN*>& aPins  ) :
+        SIM_MODEL_KIBIS( aType, aSource )
 {
-    SIM_MODEL::CreatePins( aSymbolPinCount );
+    ReadDataFields( &aFields, aPins );
+}
+
+
+void SIM_MODEL_KIBIS::CreatePins( const std::vector<LIB_PIN*>& aSymbolPins )
+{
+    SIM_MODEL::CreatePins( aSymbolPins );
 
     // Reset the pins to Not Connected. Linear order is not as common, and reordering the pins is
     // more effort in the GUI than assigning them from scratch.

eeschema/sim/sim_model_kibis.h

@@ -59,8 +59,10 @@ public:
     // creates a a model with aType, but tries to match parameters from aSource.
     SIM_MODEL_KIBIS( TYPE aType, const SIM_MODEL_KIBIS& aSource );
 
-    SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector<LIB_FIELD>& aFields );
+    SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector<LIB_FIELD>& aFields,
-    SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector<SCH_FIELD>& aFields );
+                     const std::vector<LIB_PIN*>& aPins );
+    SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector<SCH_FIELD>& aFields,
+                         const std::vector<LIB_PIN*>& aPins );
 
     std::vector<std::pair<std::string, std::string>> GetIbisPins() const
     {
@@ -92,7 +94,7 @@ public:
     bool m_enableDiff;
 
 protected:
-    void CreatePins( unsigned aSymbolPinCount ) override;
+    void CreatePins( const std::vector<LIB_PIN*>& aSymbolPins ) override;
 
 private:
     bool requiresSpiceModelLine() const override { return true; }

eeschema/sim/sim_model_raw_spice.cpp

@@ -123,9 +123,9 @@ SIM_MODEL_RAW_SPICE::SIM_MODEL_RAW_SPICE() :
 }
 
 
-void SIM_MODEL_RAW_SPICE::CreatePins( unsigned aSymbolPinCount )
+void SIM_MODEL_RAW_SPICE::CreatePins( const std::vector<LIB_PIN*>& aSymbolPins )
 {
-    for( unsigned symbolPinIndex = 0; symbolPinIndex < aSymbolPinCount; ++symbolPinIndex )
+    for( unsigned symbolPinIndex = 0; symbolPinIndex < aSymbolPins.size(); ++symbolPinIndex )
         AddPin( { fmt::format( "{}", symbolPinIndex + 1 ), "" } );
 }
 

eeschema/sim/sim_model_raw_spice.h

@@ -75,7 +75,7 @@ public:
     }
 
 protected:
-    void CreatePins( unsigned aSymbolPinCount ) override;
+    void CreatePins( const std::vector<LIB_PIN*>& aSymbolPins ) override;
 
 private:
     static std::vector<PARAM::INFO> makeParamInfos();

eeschema/sim/sim_model_subckt.cpp

@@ -157,9 +157,9 @@ void SIM_MODEL_SUBCKT::SetBaseModel( const SIM_MODEL& aBaseModel )
 }
 
 
-void SIM_MODEL_SUBCKT::CreatePins( unsigned aSymbolPinCount )
+void SIM_MODEL_SUBCKT::CreatePins( const std::vector<LIB_PIN*>& aSymbolPins )
 {
-    SIM_MODEL::CreatePins( aSymbolPinCount );
+    SIM_MODEL::CreatePins( aSymbolPins );
 
     // Reset the pins to Not Connected. Linear order is not as common, and reordering the pins is
     // more effort in the GUI than assigning them from scratch.

eeschema/sim/sim_model_subckt.h

@@ -58,7 +58,7 @@ public:
     void SetBaseModel( const SIM_MODEL& aBaseModel ) override;
 
 protected:
-    void CreatePins( unsigned aSymbolPinCount ) override;
+    void CreatePins( const std::vector<LIB_PIN*>& aSymbolPins ) override;
 
 private:
     bool requiresSpiceModelLine() const override { return true; }