From 321de57d7c8ad912399a3db2b46cfa0c31cd8fed Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sat, 17 Dec 2022 20:25:29 +0000 Subject: [PATCH] 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 | 14 ++++- eeschema/sim/sim_model.cpp | 79 ++++----------------------- eeschema/sim/sim_model.h | 9 +-- eeschema/sim/sim_model_kibis.cpp | 23 +++++--- eeschema/sim/sim_model_kibis.h | 8 ++- eeschema/sim/sim_model_raw_spice.cpp | 4 +- eeschema/sim/sim_model_raw_spice.h | 2 +- eeschema/sim/sim_model_subckt.cpp | 4 +- eeschema/sim/sim_model_subckt.h | 2 +- 9 files changed, 49 insertions(+), 96 deletions(-) diff --git a/eeschema/dialogs/dialog_sim_model.cpp b/eeschema/dialogs/dialog_sim_model.cpp index 85acf7de33..28d2cb4ae0 100644 --- a/eeschema/dialogs/dialog_sim_model.cpp +++ b/eeschema/dialogs/dialog_sim_model.cpp @@ -1159,8 +1159,15 @@ void DIALOG_SIM_MODEL::onDeviceTypeChoice( wxCommandEvent& aE template void DIALOG_SIM_MODEL::onTypeChoice( wxCommandEvent& aEvent ) { - SIM_MODEL::DEVICE_T deviceType = curModel().GetDeviceType(); - wxString typeDescription = m_typeChoice->GetString( m_typeChoice->GetSelection() ); + SIM_MODEL::DEVICE_T deviceType = curModel().GetDeviceType(); + wxString typeDescription = m_typeChoice->GetString( m_typeChoice->GetSelection() ); + std::vector 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::onTypeChoice( wxCommandEvent& aEvent ) auto& kibisModel = static_cast( m_libraryModelsMgr.GetModels().at( idx ).get() ); - m_libraryModelsMgr.SetModel( idx, std::make_unique( type, kibisModel, m_fields ) ); + m_libraryModelsMgr.SetModel( idx, std::make_unique( type, kibisModel, + m_fields, sourcePins ) ); } m_curModelType = type; diff --git a/eeschema/sim/sim_model.cpp b/eeschema/sim/sim_model.cpp index 5410072166..23b7d63809 100644 --- a/eeschema/sim/sim_model.cpp +++ b/eeschema/sim/sim_model.cpp @@ -412,20 +412,6 @@ TYPE SIM_MODEL::InferTypeFromLegacyFields( const std::vector& aFields ) } -template <> -void SIM_MODEL::ReadDataFields( unsigned aSymbolPinCount, const std::vector* aFields ) -{ - doReadDataFields( aSymbolPinCount, aFields ); -} - - -template <> -void SIM_MODEL::ReadDataFields( unsigned aSymbolPinCount, const std::vector* aFields ) -{ - doReadDataFields( aSymbolPinCount, aFields ); -} - - template <> void SIM_MODEL::ReadDataFields( const std::vector* aFields, const std::vector& aPins ) @@ -983,7 +969,7 @@ SIM_MODEL::SIM_MODEL( TYPE aType, std::unique_ptr aSpiceGenerat } -void SIM_MODEL::CreatePins( unsigned aSymbolPinCount ) +void SIM_MODEL::CreatePins( const std::vector& 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 ) - { - if( modelPinIndex < aSymbolPinCount ) - AddPin( { getPinNames().at( modelPinIndex ), fmt::format( "{}", modelPinIndex + 1 ) } ); - else - AddPin( { getPinNames().at( modelPinIndex ), "" } ); - } -} + std::vector pinNames = getPinNames(); - -void SIM_MODEL::CreatePins( const std::vector 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 ) + for( unsigned modelPinIndex = 0; modelPinIndex < pinNames.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 -void SIM_MODEL::doReadDataFields( unsigned aSymbolPinCount, const std::vector* 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 void SIM_MODEL::doReadDataFields( const std::vector* aFields, const std::vector& 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)" - // needs to go to "dc=1", while "sin(0, 1, 60)" needs to go to "dc=0 ampl=1 f=60". + // See if we have a function-style SPICE model (ie: "sin(0 1 60)") that can be handled + // by a built-in SIM_MODEL. - wxRegEx regex( wxT( "^[a-z]+\\(.*\\)$" ) ); + wxRegEx regex( wxT( "^[a-zA-Z]+\\(.*\\)$" ) ); if( regex.Matches( spiceModel ) ) { diff --git a/eeschema/sim/sim_model.h b/eeschema/sim/sim_model.h index 93f8bc21d6..2b4eab39e4 100644 --- a/eeschema/sim/sim_model.h +++ b/eeschema/sim/sim_model.h @@ -433,9 +433,6 @@ public: SIM_MODEL( SIM_MODEL&& aOther ) = default; SIM_MODEL& operator=(SIM_MODEL&& aOther ) = delete; - template - void ReadDataFields( unsigned aSymbolPinCount, const std::vector* aFields ); - template void ReadDataFields( const std::vector* aFields, const std::vector& aPins ); @@ -537,13 +534,9 @@ protected: SIM_MODEL( TYPE aType, std::unique_ptr aSpiceGenerator, std::unique_ptr aSerde ); - virtual void CreatePins( unsigned aSymbolPinCount ); - virtual void CreatePins( const std::vector aSymbolPins ); + virtual void CreatePins( const std::vector& aSymbolPins ); private: - template - void doReadDataFields( unsigned aSymbolPinCount, const std::vector* aFields ); - template void doReadDataFields( const std::vector* aFields, const std::vector& aPins ); diff --git a/eeschema/sim/sim_model_kibis.cpp b/eeschema/sim/sim_model_kibis.cpp index 877f4ecedc..c3cbbc5989 100644 --- a/eeschema/sim/sim_model_kibis.cpp +++ b/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& aFields ) : - SIM_MODEL_KIBIS( aType, aSource ) -{ - ReadDataFields( 2, &aFields ); -} SIM_MODEL_KIBIS::SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, - const std::vector& aFields ) : + const std::vector& aFields, + const std::vector& 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& aFields, + const std::vector& aPins ) : + SIM_MODEL_KIBIS( aType, aSource ) { - SIM_MODEL::CreatePins( aSymbolPinCount ); + ReadDataFields( &aFields, aPins ); +} + + +void SIM_MODEL_KIBIS::CreatePins( const std::vector& 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. diff --git a/eeschema/sim/sim_model_kibis.h b/eeschema/sim/sim_model_kibis.h index 210b79fd8b..3dcc268e37 100644 --- a/eeschema/sim/sim_model_kibis.h +++ b/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& aFields ); - SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector& aFields ); + SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector& aFields, + const std::vector& aPins ); + SIM_MODEL_KIBIS( TYPE aType, SIM_MODEL_KIBIS& aSource, const std::vector& aFields, + const std::vector& aPins ); std::vector> GetIbisPins() const { @@ -92,7 +94,7 @@ public: bool m_enableDiff; protected: - void CreatePins( unsigned aSymbolPinCount ) override; + void CreatePins( const std::vector& aSymbolPins ) override; private: bool requiresSpiceModelLine() const override { return true; } diff --git a/eeschema/sim/sim_model_raw_spice.cpp b/eeschema/sim/sim_model_raw_spice.cpp index 2289f047c9..22c246ed32 100644 --- a/eeschema/sim/sim_model_raw_spice.cpp +++ b/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& aSymbolPins ) { - for( unsigned symbolPinIndex = 0; symbolPinIndex < aSymbolPinCount; ++symbolPinIndex ) + for( unsigned symbolPinIndex = 0; symbolPinIndex < aSymbolPins.size(); ++symbolPinIndex ) AddPin( { fmt::format( "{}", symbolPinIndex + 1 ), "" } ); } diff --git a/eeschema/sim/sim_model_raw_spice.h b/eeschema/sim/sim_model_raw_spice.h index f235280f89..606aed774d 100644 --- a/eeschema/sim/sim_model_raw_spice.h +++ b/eeschema/sim/sim_model_raw_spice.h @@ -75,7 +75,7 @@ public: } protected: - void CreatePins( unsigned aSymbolPinCount ) override; + void CreatePins( const std::vector& aSymbolPins ) override; private: static std::vector makeParamInfos(); diff --git a/eeschema/sim/sim_model_subckt.cpp b/eeschema/sim/sim_model_subckt.cpp index 5fc4571499..f84f4b3af1 100644 --- a/eeschema/sim/sim_model_subckt.cpp +++ b/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& 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. diff --git a/eeschema/sim/sim_model_subckt.h b/eeschema/sim/sim_model_subckt.h index 9a2c2767ab..cb2a240a81 100644 --- a/eeschema/sim/sim_model_subckt.h +++ b/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& aSymbolPins ) override; private: bool requiresSpiceModelLine() const override { return true; }