Improve simulation error reporting.

1) More REPORTER, less exception processing
2) Remove UI calls from SPICE_MODEL
3) Don't replace netlist with errors; show both
4) Don't bail out of netlist generation after single error

Fixes https://gitlab.com/kicad/code/kicad/issues/14295
This commit is contained in:
Jeff Young 2023-03-16 15:57:27 +00:00
parent 8b144539e8
commit 6d296038f3
8 changed files with 146 additions and 111 deletions

View File

@ -140,6 +140,9 @@ bool DIALOG_SIM_MODEL<T_symbol, T_field>::TransferDataToWindow()
wxString pinMap; wxString pinMap;
bool storeInValue = false; bool storeInValue = false;
wxString msg;
WX_STRING_REPORTER reporter( &msg );
// Infer RLC and VI models if they aren't specified // Infer RLC and VI models if they aren't specified
if( SIM_MODEL::InferSimModel( m_symbol, &m_fields, false, SIM_VALUE_GRAMMAR::NOTATION::SI, if( SIM_MODEL::InferSimModel( m_symbol, &m_fields, false, SIM_VALUE_GRAMMAR::NOTATION::SI,
&deviceType, &modelType, &modelParams, &pinMap ) ) &deviceType, &modelType, &modelParams, &pinMap ) )
@ -184,7 +187,7 @@ bool DIALOG_SIM_MODEL<T_symbol, T_field>::TransferDataToWindow()
if( !loadLibrary( libraryFilename ) ) if( !loadLibrary( libraryFilename ) )
{ {
m_libraryPathText->ChangeValue( libraryFilename ); m_libraryPathText->ChangeValue( libraryFilename );
m_curModelType = SIM_MODEL::ReadTypeFromFields( m_fields ); m_curModelType = SIM_MODEL::ReadTypeFromFields( m_fields, &reporter );
m_libraryModelsMgr.CreateModel( nullptr, sourcePins, m_fields ); m_libraryModelsMgr.CreateModel( nullptr, sourcePins, m_fields );
m_modelNameChoice->Append( _( "<unknown>" ) ); m_modelNameChoice->Append( _( "<unknown>" ) );
@ -260,25 +263,32 @@ bool DIALOG_SIM_MODEL<T_symbol, T_field>::TransferDataToWindow()
{ {
// The model is sourced from the instance. // The model is sourced from the instance.
m_useInstanceModelRadioButton->SetValue( true ); m_useInstanceModelRadioButton->SetValue( true );
m_curModelType = SIM_MODEL::ReadTypeFromFields( m_fields );
msg.clear();
m_curModelType = SIM_MODEL::ReadTypeFromFields( m_fields, &reporter );
if( reporter.HasMessage() )
DisplayErrorMessage( this, msg );
} }
m_builtinModelsMgr.SetReporter( &reporter );
for( SIM_MODEL::TYPE type : SIM_MODEL::TYPE_ITERATOR() ) for( SIM_MODEL::TYPE type : SIM_MODEL::TYPE_ITERATOR() )
{ {
wxString msg;
WX_STRING_REPORTER reporter( &msg );
m_builtinModelsMgr.SetReporter( &reporter );
if( m_useInstanceModelRadioButton->GetValue() && type == m_curModelType ) if( m_useInstanceModelRadioButton->GetValue() && type == m_curModelType )
m_builtinModelsMgr.CreateModel( m_fields, sourcePins, false );
else
m_builtinModelsMgr.CreateModel( type, sourcePins );
if( reporter.HasMessage() )
{ {
DisplayErrorMessage( this, _( "Failed to read simulation model from fields." ) msg.clear();
+ wxT( "\n\n" ) + msg ); m_builtinModelsMgr.CreateModel( m_fields, sourcePins, false );
if( reporter.HasMessage() )
{
DisplayErrorMessage( this, _( "Failed to read simulation model from fields." )
+ wxT( "\n\n" ) + msg );
}
}
else
{
m_builtinModelsMgr.CreateModel( type, sourcePins );
} }
SIM_MODEL::DEVICE_T deviceTypeT = SIM_MODEL::TypeInfo( type ).deviceType; SIM_MODEL::DEVICE_T deviceTypeT = SIM_MODEL::TypeInfo( type ).deviceType;

View File

@ -83,20 +83,13 @@ namespace NETLIST_EXPORTER_SPICE_PARSER
std::string NAME_GENERATOR::Generate( const std::string& aProposedName ) std::string NAME_GENERATOR::Generate( const std::string& aProposedName )
{ {
if( !m_names.count( aProposedName ) ) std::string name = aProposedName;
return aProposedName; int ii = 1;
for( uint64_t i = 1; i < UINT64_MAX; ++i ) while( m_names.count( name ) )
{ name = fmt::format( "{}#{}", aProposedName, ii++ );
std::string name = fmt::format( "{}#{}", aProposedName, i );
if( !m_names.count( name ) ) return name;
return name;
}
// Should never happen.
THROW_IO_ERROR( wxString::Format( _( "Failed to generate a name for '%s': exceeded UINT64_MAX" ),
aProposedName ) );
} }
@ -126,8 +119,7 @@ bool NETLIST_EXPORTER_SPICE::DoWriteNetlist( OUTPUTFORMATTER& aFormatter, unsign
// Cleanup list to avoid duplicate if the netlist exporter is run more than once. // Cleanup list to avoid duplicate if the netlist exporter is run more than once.
m_rawIncludes.clear(); m_rawIncludes.clear();
if( !ReadSchematicAndLibraries( aNetlistOptions, aReporter ) ) bool result = ReadSchematicAndLibraries( aNetlistOptions, aReporter );
return false;
WriteHead( aFormatter, aNetlistOptions ); WriteHead( aFormatter, aNetlistOptions );
@ -142,7 +134,7 @@ bool NETLIST_EXPORTER_SPICE::DoWriteNetlist( OUTPUTFORMATTER& aFormatter, unsign
WriteTail( aFormatter, aNetlistOptions ); WriteTail( aFormatter, aNetlistOptions );
return true; return result;
} }
@ -165,6 +157,8 @@ bool NETLIST_EXPORTER_SPICE::ReadSchematicAndLibraries( unsigned aNetlistOptions
std::set<std::string> refNames; // Set of reference names to check for duplication. std::set<std::string> refNames; // Set of reference names to check for duplication.
int ncCounter = 1; int ncCounter = 1;
m_libMgr.SetReporter( &aReporter );
ReadDirectives( aNetlistOptions ); ReadDirectives( aNetlistOptions );
m_nets.clear(); m_nets.clear();
@ -240,27 +234,19 @@ bool NETLIST_EXPORTER_SPICE::ReadSchematicAndLibraries( unsigned aNetlistOptions
wxString modelParams; wxString modelParams;
wxString pinMap; wxString pinMap;
try readRefName( sheet, *symbol, spiceItem, refNames );
{ readModel( sheet, *symbol, spiceItem );
readRefName( sheet, *symbol, spiceItem, refNames ); readPinNumbers( *symbol, spiceItem, pins );
readModel( sheet, *symbol, spiceItem ); readPinNetNames( *symbol, spiceItem, pins, ncCounter );
readPinNumbers( *symbol, spiceItem, pins );
readPinNetNames( *symbol, spiceItem, pins, ncCounter );
// TODO: transmission line handling? // TODO: transmission line handling?
m_items.push_back( std::move( spiceItem ) ); m_items.push_back( std::move( spiceItem ) );
}
catch( const IO_ERROR& e )
{
msg.Printf( _( "Error reading simulation model from symbol '%s':\n%s" ),
symbol->GetRef( &sheet ),
e.Problem() );
aReporter.Report( msg, RPT_SEVERITY_ERROR );
}
} }
} }
m_libMgr.SetReporter( nullptr );
return !aReporter.HasMessage(); return !aReporter.HasMessage();
} }

View File

@ -49,7 +49,7 @@ void SIM_LIBRARY_KIBIS::ReadFile( const std::string& aFilePath, REPORTER* aRepor
for( KIBIS_COMPONENT& kcomp : m_kibis.m_components ) for( KIBIS_COMPONENT& kcomp : m_kibis.m_components )
{ {
m_models.push_back( SIM_MODEL::Create( SIM_MODEL::TYPE::KIBIS_DEVICE, pins, nullptr ) ); m_models.push_back( SIM_MODEL::Create( SIM_MODEL::TYPE::KIBIS_DEVICE, pins, aReporter ) );
m_modelNames.emplace_back( kcomp.m_name ); m_modelNames.emplace_back( kcomp.m_name );
SIM_MODEL_KIBIS* libcomp = dynamic_cast<SIM_MODEL_KIBIS*>( m_models.back().get() ); SIM_MODEL_KIBIS* libcomp = dynamic_cast<SIM_MODEL_KIBIS*>( m_models.back().get() );

View File

@ -361,11 +361,13 @@ SIM_MODEL::SPICE_INFO SIM_MODEL::SpiceInfo( TYPE aType )
} }
template TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<SCH_FIELD>& aFields ); template TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<SCH_FIELD>& aFields,
template TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<LIB_FIELD>& aFields ); REPORTER* aReporter );
template TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<LIB_FIELD>& aFields,
REPORTER* aReporter );
template <typename T> template <typename T>
TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<T>& aFields ) TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<T>& aFields, REPORTER* aReporter )
{ {
std::string deviceTypeFieldValue = GetFieldValue( &aFields, SIM_DEVICE_TYPE_FIELD ); std::string deviceTypeFieldValue = GetFieldValue( &aFields, SIM_DEVICE_TYPE_FIELD );
std::string typeFieldValue = GetFieldValue( &aFields, SIM_TYPE_FIELD ); std::string typeFieldValue = GetFieldValue( &aFields, SIM_TYPE_FIELD );
@ -392,6 +394,22 @@ TYPE SIM_MODEL::ReadTypeFromFields( const std::vector<T>& aFields )
if( typeFromLegacyFields != TYPE::NONE ) if( typeFromLegacyFields != TYPE::NONE )
return typeFromLegacyFields; return typeFromLegacyFields;
if( aReporter )
{
if( aFields.size() > REFERENCE_FIELD )
{
aReporter->Report( wxString::Format( _( "No simulation model definition found for "
"symbol '%s'." ),
aFields[REFERENCE_FIELD].GetText() ),
RPT_SEVERITY_ERROR );
}
else
{
aReporter->Report( _( "No simulation model definition found." ),
RPT_SEVERITY_ERROR );
}
}
return TYPE::NONE; return TYPE::NONE;
} }
@ -455,10 +473,7 @@ std::unique_ptr<SIM_MODEL> SIM_MODEL::Create( TYPE aType, const std::vector<LIB_
} }
catch( IO_ERROR& err ) catch( IO_ERROR& err )
{ {
if( aReporter ) wxFAIL_MSG( "Shouldn't throw reading empty fields!" );
aReporter->Report( err.What(), RPT_SEVERITY_ERROR );
else
DisplayErrorMessage( nullptr, err.What() );
} }
return model; return model;
@ -481,19 +496,16 @@ std::unique_ptr<SIM_MODEL> SIM_MODEL::Create( const SIM_MODEL* aBaseModel,
else else
model = Create( type ); model = Create( type );
if( aBaseModel )
model->SetBaseModel( *aBaseModel );
try try
{ {
if( aBaseModel )
model->SetBaseModel( *aBaseModel );
model->ReadDataFields( static_cast<const std::vector<SCH_FIELD>*>( nullptr ), aPins ); model->ReadDataFields( static_cast<const std::vector<SCH_FIELD>*>( nullptr ), aPins );
} }
catch( IO_ERROR& err ) catch( IO_ERROR& err )
{ {
if( aReporter ) wxFAIL_MSG( "Shouldn't throw reading empty fields!" );
aReporter->Report( err.What(), RPT_SEVERITY_ERROR );
else
DisplayErrorMessage( nullptr, err.What() );
} }
return model; return model;
@ -506,7 +518,7 @@ std::unique_ptr<SIM_MODEL> SIM_MODEL::Create( const SIM_MODEL* aBaseModel,
const std::vector<T>& aFields, const std::vector<T>& aFields,
REPORTER* aReporter ) REPORTER* aReporter )
{ {
TYPE type = ReadTypeFromFields( aFields ); TYPE type = ReadTypeFromFields( aFields, aReporter );
// If the model has a specified type, it takes priority over the type of its base class. // If the model has a specified type, it takes priority over the type of its base class.
if( type == TYPE::NONE && aBaseModel ) if( type == TYPE::NONE && aBaseModel )
@ -523,19 +535,23 @@ std::unique_ptr<SIM_MODEL> SIM_MODEL::Create( const SIM_MODEL* aBaseModel,
else else
model = Create( type ); model = Create( type );
if( aBaseModel )
model->SetBaseModel( *aBaseModel );
try try
{ {
if( aBaseModel )
model->SetBaseModel( *aBaseModel );
model->ReadDataFields( &aFields, aPins ); model->ReadDataFields( &aFields, aPins );
} }
catch( IO_ERROR& err ) catch( IO_ERROR& err )
{ {
if( aReporter ) if( aReporter )
aReporter->Report( err.What(), RPT_SEVERITY_ERROR ); {
else aReporter->Report( wxString::Format( _( "Error reading simulation model from "
DisplayErrorMessage( nullptr, err.What() ); "symbol '%s':\n%s" ),
aFields[REFERENCE_FIELD].GetText(),
err.Problem() ),
RPT_SEVERITY_ERROR );
}
} }
return model; return model;
@ -557,7 +573,7 @@ std::unique_ptr<SIM_MODEL> SIM_MODEL::Create( const std::vector<T>& aFields,
const std::vector<LIB_PIN*>& aPins, const std::vector<LIB_PIN*>& aPins,
bool aResolved, REPORTER* aReporter ) bool aResolved, REPORTER* aReporter )
{ {
TYPE type = ReadTypeFromFields( aFields ); TYPE type = ReadTypeFromFields( aFields, aReporter );
std::unique_ptr<SIM_MODEL> model = SIM_MODEL::Create( type ); std::unique_ptr<SIM_MODEL> model = SIM_MODEL::Create( type );
try try
@ -589,11 +605,15 @@ std::unique_ptr<SIM_MODEL> SIM_MODEL::Create( const std::vector<T>& aFields,
} }
catch( const IO_ERROR& err ) catch( const IO_ERROR& err )
{ {
// We own the pin syntax, so if we can't parse it then there's an error, full stop. // We own the pin syntax, so if we can't parse it then there's an error.
if( aReporter ) if( aReporter )
aReporter->Report( err.Problem(), RPT_SEVERITY_ERROR ); {
else aReporter->Report( wxString::Format( _( "Error reading simulation model from "
THROW_IO_ERROR( err.Problem() ); "symbol '%s':\n%s" ),
aFields[REFERENCE_FIELD].GetText(),
err.Problem() ),
RPT_SEVERITY_ERROR );
}
} }
} }
@ -726,21 +746,8 @@ void SIM_MODEL::AddParam( const PARAM::INFO& aInfo )
void SIM_MODEL::SetBaseModel( const SIM_MODEL& aBaseModel ) void SIM_MODEL::SetBaseModel( const SIM_MODEL& aBaseModel )
{ {
auto describe = wxASSERT_MSG( GetType() == aBaseModel.GetType(),
[]( const SIM_MODEL* aModel ) wxS( "Simulation model type must be the same as its base class!" ) );
{
return fmt::format( "{} ({})",
aModel->GetDeviceInfo().fieldValue,
aModel->GetTypeInfo().description );
};
if( GetType() != aBaseModel.GetType() )
{
THROW_IO_ERROR( wxString::Format( _( "Simulation model type must be the same as of its "
"base class: '%s', but is '%s'" ),
describe( &aBaseModel ),
describe( this ) ) );
}
m_baseModel = &aBaseModel; m_baseModel = &aBaseModel;
} }

View File

@ -384,7 +384,7 @@ public:
template <typename T> template <typename T>
static TYPE ReadTypeFromFields( const std::vector<T>& aFields ); static TYPE ReadTypeFromFields( const std::vector<T>& aFields, REPORTER* aReporter );
template <typename T> template <typename T>
static TYPE InferTypeFromLegacyFields( const std::vector<T>& aFields ); static TYPE InferTypeFromLegacyFields( const std::vector<T>& aFields );

View File

@ -55,7 +55,6 @@ std::vector<std::string> SPICE_GENERATOR_NGSPICE::CurrentNames( const SPICE_ITEM
return SPICE_GENERATOR::CurrentNames( aItem ); return SPICE_GENERATOR::CurrentNames( aItem );
default: default:
wxFAIL_MSG( "Unhandled model device type in SIM_MODEL_NGSPICE" );
return {}; return {};
} }
} }

View File

@ -95,6 +95,10 @@ std::string SPICE_GENERATOR::ModelLine( const SPICE_ITEM& aItem ) const
value ) ); value ) );
} }
// Don't send SPICE empty models.
if( result.length() == indentLength + 1 /* line ending */ )
result.clear();
return result; return result;
} }

View File

@ -29,6 +29,7 @@
#include <kiway.h> #include <kiway.h>
#include <confirm.h> #include <confirm.h>
#include <wildcards_and_files_ext.h> #include <wildcards_and_files_ext.h>
#include <widgets/wx_html_report_box.h>
#include <project/project_file.h> #include <project/project_file.h>
#include <sch_edit_frame.h> #include <sch_edit_frame.h>
#include <sim/simulator_frame.h> #include <sim/simulator_frame.h>
@ -350,45 +351,74 @@ public:
wxPostEvent( this, wxCommandEvent( wxEVT_COMMAND_BUTTON_CLICKED, wxID_CANCEL ) ); wxPostEvent( this, wxCommandEvent( wxEVT_COMMAND_BUTTON_CLICKED, wxID_CANCEL ) );
} }
NETLIST_VIEW_DIALOG( wxWindow* parent, const wxString& source) : NETLIST_VIEW_DIALOG( wxWindow* parent ) :
DIALOG_SHIM( parent, wxID_ANY, _( "SPICE Netlist" ), wxDefaultPosition, DIALOG_SHIM( parent, wxID_ANY, _( "SPICE Netlist" ), wxDefaultPosition,
wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER ) wxSize( 800, 800 ), wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER ),
m_textCtrl( nullptr ),
m_reporter( nullptr )
{ {
wxStyledTextCtrl* textCtrl = new wxStyledTextCtrl( this, wxID_ANY ); m_splitter = new wxSplitterWindow( this, wxID_ANY, wxDefaultPosition, wxDefaultSize,
textCtrl->SetMinSize( wxSize( 600, 400 ) ); wxSP_LIVE_UPDATE | wxSP_NOBORDER | wxSP_3DSASH );
textCtrl->SetMarginWidth( MARGIN_LINE_NUMBERS, 50 ); //Avoid the splitter window being assigned as the Parent to additional windows
textCtrl->StyleSetForeground( wxSTC_STYLE_LINENUMBER, wxColour( 75, 75, 75 ) ); m_splitter->SetExtraStyle( wxWS_EX_TRANSIENT );
textCtrl->StyleSetBackground( wxSTC_STYLE_LINENUMBER, wxColour( 220, 220, 220 ) );
textCtrl->SetMarginType( MARGIN_LINE_NUMBERS, wxSTC_MARGIN_NUMBER ); m_textCtrl = new wxStyledTextCtrl( m_splitter, wxID_ANY );
m_textCtrl->SetMarginWidth( MARGIN_LINE_NUMBERS, 50 );
m_textCtrl->StyleSetForeground( wxSTC_STYLE_LINENUMBER, wxColour( 75, 75, 75 ) );
m_textCtrl->StyleSetBackground( wxSTC_STYLE_LINENUMBER, wxColour( 220, 220, 220 ) );
m_textCtrl->SetMarginType( MARGIN_LINE_NUMBERS, wxSTC_MARGIN_NUMBER );
wxFont fixedFont = KIUI::GetMonospacedUIFont(); wxFont fixedFont = KIUI::GetMonospacedUIFont();
for( int i = 0; i < wxSTC_STYLE_MAX; ++i ) for( int i = 0; i < wxSTC_STYLE_MAX; ++i )
textCtrl->StyleSetFont( i, fixedFont ); m_textCtrl->StyleSetFont( i, fixedFont );
textCtrl->StyleClearAll(); // Addresses a bug in wx3.0 where styles are not correctly set m_textCtrl->StyleClearAll(); // Addresses a bug in wx3.0 where styles are not correctly set
textCtrl->SetWrapMode( wxSTC_WRAP_WORD ); m_textCtrl->SetWrapMode( wxSTC_WRAP_WORD );
m_textCtrl->SetLexer( wxSTC_LEX_SPICE );
m_textCtrl->SetMinSize( wxSize( 40, 40 ) );
m_textCtrl->SetSize( wxSize( 40, 40 ) );
textCtrl->SetText( source ); m_reporter = new WX_HTML_REPORT_BOX( m_splitter, wxID_ANY );
m_reporter->SetMinSize( wxSize( 40, 40 ) );
m_reporter->SetSize( wxSize( 40, 40 ) );
textCtrl->SetLexer( wxSTC_LEX_SPICE ); m_splitter->SetMinimumPaneSize( 40 );
m_splitter->SetSashPosition( 760 );
textCtrl->SetEditable( false ); m_splitter->SetSashGravity( 0.9 );
m_splitter->SplitHorizontally( m_textCtrl, m_reporter );
wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL ); wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL );
sizer->Add( textCtrl, 1, wxEXPAND | wxALL, 5 ); sizer->Add( m_splitter, 1, wxEXPAND | wxALL, 5 );
SetSizer( sizer ); SetSizer( sizer );
Layout();
Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( NETLIST_VIEW_DIALOG::onClose ), Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( NETLIST_VIEW_DIALOG::onClose ),
nullptr, this ); nullptr, this );
m_scintillaTricks = std::make_unique<SCINTILLA_TRICKS>( textCtrl, wxT( "{}" ), false ); m_scintillaTricks = std::make_unique<SCINTILLA_TRICKS>( m_textCtrl, wxT( "{}" ), false );
finishDialogSettings(); finishDialogSettings();
} }
void SetNetlist( const wxString& aSource )
{
m_textCtrl->SetText( aSource );
m_textCtrl->SetEditable( false );
m_reporter->Flush();
}
REPORTER* GetReporter() { return m_reporter; }
private:
wxSplitterWindow* m_splitter;
wxStyledTextCtrl* m_textCtrl;
WX_HTML_REPORT_BOX* m_reporter;
std::unique_ptr<SCINTILLA_TRICKS> m_scintillaTricks; std::unique_ptr<SCINTILLA_TRICKS> m_scintillaTricks;
}; };
@ -411,14 +441,13 @@ int SIMULATOR_CONTROL::ShowNetlist( const TOOL_EVENT& aEvent )
if( m_schematicFrame == nullptr || m_simulator == nullptr ) if( m_schematicFrame == nullptr || m_simulator == nullptr )
return -1; return -1;
wxString errors; STRING_FORMATTER formatter;
WX_STRING_REPORTER reporter( &errors ); NETLIST_VIEW_DIALOG dlg( m_simulatorFrame );
STRING_FORMATTER formatter;
m_circuitModel->SetSimOptions( m_simulatorFrame->GetCurrentOptions() ); m_circuitModel->SetSimOptions( m_simulatorFrame->GetCurrentOptions() );
m_circuitModel->GetNetlist( &formatter, reporter ); m_circuitModel->GetNetlist( &formatter, *dlg.GetReporter() );
NETLIST_VIEW_DIALOG dlg( m_simulatorFrame, errors.IsEmpty() ? wxString( formatter.GetString() ) : errors ); dlg.SetNetlist( wxString( formatter.GetString() ) );
dlg.ShowModal(); dlg.ShowModal();
return 0; return 0;