From 03486443c7ed1902f1f7f5cb015a29265ebb39c9 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 22 May 2023 11:45:59 +0100 Subject: [PATCH] Fix more SPICE case-insensitivity bugs. Fixes https://gitlab.com/kicad/code/kicad/-/issues/14793 --- eeschema/sim/sim_model.cpp | 9 ++- eeschema/sim/sim_model.h | 13 ++++- eeschema/sim/sim_model_ngspice.cpp | 71 +++++++++++------------ eeschema/sim/sim_model_serializer.cpp | 11 ++-- eeschema/sim/sim_model_spice_fallback.cpp | 5 +- 5 files changed, 57 insertions(+), 52 deletions(-) diff --git a/eeschema/sim/sim_model.cpp b/eeschema/sim/sim_model.cpp index e23cdc3df4..2bd39424d5 100644 --- a/eeschema/sim/sim_model.cpp +++ b/eeschema/sim/sim_model.cpp @@ -24,7 +24,6 @@ */ #include #include -#include #include #include @@ -800,13 +799,19 @@ const SIM_MODEL::PARAM& SIM_MODEL::GetParam( unsigned aParamIndex ) const } +bool SIM_MODEL::PARAM::INFO::Matches( const std::string& aParamName ) const +{ + return boost::iequals( name, aParamName ); +} + + int SIM_MODEL::doFindParam( const std::string& aParamName ) const { std::vector> params = GetParams(); for( int ii = 0; ii < (int) params.size(); ++ii ) { - if( boost::iequals( params[ii].get().info.name, aParamName ) ) + if( params[ii].get().Matches( aParamName ) ) return ii; } diff --git a/eeschema/sim/sim_model.h b/eeschema/sim/sim_model.h index 46cccd829e..49aa997dd3 100644 --- a/eeschema/sim/sim_model.h +++ b/eeschema/sim/sim_model.h @@ -362,6 +362,8 @@ public: enumValues( std::move( aEnumValues ) ) {} + bool Matches( const std::string& aName ) const; + std::string name; unsigned id; DIR dir; @@ -377,12 +379,17 @@ public: std::vector enumValues; }; - std::string value; - const INFO& info; - PARAM( const INFO& aInfo ) : info( aInfo ) {} + + bool Matches( const std::string& aName ) const + { + return info.Matches( aName ); + } + + std::string value; + const INFO& info; }; diff --git a/eeschema/sim/sim_model_ngspice.cpp b/eeschema/sim/sim_model_ngspice.cpp index 739e8842ae..a820b583f4 100644 --- a/eeschema/sim/sim_model_ngspice.cpp +++ b/eeschema/sim/sim_model_ngspice.cpp @@ -24,7 +24,7 @@ #include -#include +#include #include @@ -85,15 +85,13 @@ int SIM_MODEL_NGSPICE::doFindParam( const std::string& aParamName ) const { // Special case to allow escaped model parameters (suffixed with "_") - std::string lowerParamName = boost::to_lower_copy( aParamName ); - std::vector> params = GetParams(); for( int ii = 0; ii < (int) params.size(); ++ii ) { const PARAM& param = params[ii]; - if( param.info.name == lowerParamName || param.info.name == lowerParamName + "_" ) + if( param.Matches( aParamName ) || param.Matches( aParamName + "_" ) ) return ii; } @@ -105,27 +103,24 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, const std::string& aValue, SIM_VALUE_GRAMMAR::NOTATION aNotation ) { - std::string paramName = boost::to_lower_copy( aParamName ); - // "level" and "version" are not really parameters - they're part of the type - so silently // ignore them. - if( paramName == "level" || paramName == "version" ) + if( boost::iequals( aParamName, "level" ) || boost::iequals( aParamName, "version" ) ) return; // First we try to use the name as is. Note that you can't set instance parameters from this // function, it's for ".model" cards, not for instantiations. - std::string lowerParamName = boost::to_lower_copy( paramName ); - std::vector> params = GetParams(); for( int ii = 0; ii < (int) params.size(); ++ii ) { const PARAM& param = params[ii]; - if( !param.info.isSpiceInstanceParam - && param.info.category != PARAM::CATEGORY::SUPERFLUOUS - && ( param.info.name == lowerParamName || param.info.name == lowerParamName + "_" ) ) + if( param.info.isSpiceInstanceParam || param.info.category == PARAM::CATEGORY::SUPERFLUOUS ) + continue; + + if( param.Matches( aParamName ) || param.Matches( aParamName + "_" ) ) { SetParamValue( ii, aValue, aNotation ); return; @@ -139,16 +134,18 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, for( const PARAM::INFO& ngspiceParamInfo : ModelInfo( getModelType() ).modelParams ) { - if( ngspiceParamInfo.name == lowerParamName ) + if( ngspiceParamInfo.Matches( aParamName ) ) { // Find an actual parameter with the same id. Even if the ngspiceParam was // superfluous, its alias target might not be. for( int ii = 0; ii < (int) params.size(); ++ii ) { - const PARAM& param = params[ii]; + const PARAM::INFO& paramInfo = params[ii].get().info; - if( param.info.id == ngspiceParamInfo.id - && param.info.category != PARAM::CATEGORY::SUPERFLUOUS ) + if( paramInfo.category == PARAM::CATEGORY::SUPERFLUOUS ) + continue; + + if( paramInfo.id == ngspiceParamInfo.id ) { SetParamValue( ii, aValue, aNotation ); return; @@ -159,7 +156,7 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, } } - if( !canSilentlyIgnoreParam( paramName ) ) + if( !canSilentlyIgnoreParam( aParamName ) ) THROW_IO_ERROR( wxString::Format( "Unknown simulation model parameter '%s'", aParamName ) ); } @@ -167,19 +164,19 @@ void SIM_MODEL_NGSPICE::SetParamFromSpiceCode( const std::string& aParamName, bool SIM_MODEL_NGSPICE::canSilentlyIgnoreParam( const std::string& aParamName ) { // Ignore the purely informative LTspice-specific parameters "mfg" and "type". - if( aParamName == "mfg" || aParamName == "type" ) + if( boost::iequals( aParamName, "mfg" ) || boost::iequals( aParamName, "type" ) ) return true; if( GetDeviceType() == DEVICE_T::D ) { - if( aParamName == "perim" - || aParamName == "isw" - || aParamName == "ns" - || aParamName == "rsw" - || aParamName == "cjsw" - || aParamName == "vjsw" - || aParamName == "mjsw" - || aParamName == "fcs" ) + if( boost::iequals( aParamName, "perim" ) + || boost::iequals( aParamName, "isw" ) + || boost::iequals( aParamName, "ns" ) + || boost::iequals( aParamName, "rsw" ) + || boost::iequals( aParamName, "cjsw" ) + || boost::iequals( aParamName, "vjsw" ) + || boost::iequals( aParamName, "mjsw" ) + || boost::iequals( aParamName, "fcs" ) ) { return true; } @@ -188,20 +185,20 @@ bool SIM_MODEL_NGSPICE::canSilentlyIgnoreParam( const std::string& aParamName ) if( GetDeviceType() == DEVICE_T::NPN || GetDeviceType() == DEVICE_T::PNP ) { // Ignore the purely informative LTspice-specific parameters "icrating" and "vceo". - if( aParamName == "icrating" || aParamName == "vceo" ) + if( boost::iequals( aParamName, "icrating" ) || boost::iequals( aParamName, "vceo" ) ) return true; } if( GetType() == TYPE::NPN_GUMMELPOON || GetType() == TYPE::PNP_GUMMELPOON ) { // Ignore unused parameters. - if( aParamName == "bvcbo" - || aParamName == "nbvcbo" - || aParamName == "tbvcbo1" - || aParamName == "tbvcbo2" - || aParamName == "bvbe" - || aParamName == "ibvbe" - || aParamName == "nbvbe" ) + if( boost::iequals( aParamName, "bvcbo" ) + || boost::iequals( aParamName, "nbvcbo" ) + || boost::iequals( aParamName, "tbvcbo1" ) + || boost::iequals( aParamName, "tbvcbo2" ) + || boost::iequals( aParamName, "bvbe" ) + || boost::iequals( aParamName, "ibvbe" ) + || boost::iequals( aParamName, "nbvbe" ) ) { return true; } @@ -210,9 +207,9 @@ bool SIM_MODEL_NGSPICE::canSilentlyIgnoreParam( const std::string& aParamName ) if( GetType() == TYPE::NMOS_VDMOS || GetType() == TYPE::PMOS_VDMOS ) { // Ignore the purely informative LTspice-specific parameters "Vds", "Ron" and "Qg". - if( aParamName == "vds" - || aParamName == "ron" - || aParamName == "qg" ) + if( boost::iequals( aParamName, "vds" ) + || boost::iequals( aParamName, "ron" ) + || boost::iequals( aParamName, "qg" ) ) { return true; } diff --git a/eeschema/sim/sim_model_serializer.cpp b/eeschema/sim/sim_model_serializer.cpp index 05ed0c844e..c51b2b53bc 100644 --- a/eeschema/sim/sim_model_serializer.cpp +++ b/eeschema/sim/sim_model_serializer.cpp @@ -211,7 +211,7 @@ bool SIM_MODEL_SERIALIZER::ParseParams( const std::string& aParams ) } std::string paramName; - bool isPrimaryValueSet = false; + bool isPrimaryValueSet = false; for( const auto& node : root->children ) { @@ -225,11 +225,10 @@ bool SIM_MODEL_SERIALIZER::ParseParams( const std::string& aParams ) || node->is_type() ) { wxASSERT( paramName != "" ); - // TODO: Shouldn't be named "...fromSpiceCode" here... m_model.SetParamValue( paramName, node->string(), SIM_VALUE_GRAMMAR::NOTATION::SI ); - if( paramName == m_model.GetParam( 0 ).info.name ) + if( m_model.GetParam( 0 ).Matches( paramName ) ) isPrimaryValueSet = true; } else if( node->is_type() ) @@ -305,13 +304,13 @@ std::string SIM_MODEL_SERIALIZER::generateParamValuePair( const SIM_MODEL::PARAM std::string name = aParam.info.name; // Because of collisions with instance parameters, we append some model parameters with "_". - if( boost::ends_with( aParam.info.name, "_" ) ) - name = aParam.info.name.substr( 0, aParam.info.name.length() - 1 ); + if( boost::ends_with( name, "_" ) ) + name = name.substr( 0, aParam.info.name.length() - 1 ); std::string value = aParam.value; if( aParam.info.category == SIM_MODEL::PARAM::CATEGORY::FLAGS ) - return value == "1" ? aParam.info.name : ""; + return value == "1" ? name : ""; if( value == "" || value.find( ' ' ) != std::string::npos ) value = fmt::format( "\"{}\"", value ); diff --git a/eeschema/sim/sim_model_spice_fallback.cpp b/eeschema/sim/sim_model_spice_fallback.cpp index 691b92791d..814c5f60ee 100644 --- a/eeschema/sim/sim_model_spice_fallback.cpp +++ b/eeschema/sim/sim_model_spice_fallback.cpp @@ -23,7 +23,6 @@ #include #include -#include SIM_MODEL_SPICE_FALLBACK::SIM_MODEL_SPICE_FALLBACK( TYPE aType, const std::string& aRawSpiceCode ) : @@ -67,15 +66,13 @@ int SIM_MODEL_SPICE_FALLBACK::doFindParam( const std::string& aParamName ) const { // Special case to allow escaped model parameters (suffixed with "_") - std::string lowerParamName = boost::to_lower_copy( aParamName ); - std::vector> params = GetParams(); for( int ii = 0; ii < (int) params.size(); ++ii ) { const PARAM& param = params[ii]; - if( param.info.name == lowerParamName || param.info.name == lowerParamName + "_" ) + if( param.Matches( aParamName ) || param.Matches( aParamName + "_" ) ) return ii; }