Fix more lifetime issues in simulator tuning.

Fixes https://gitlab.com/kicad/code/kicad/issues/13257
This commit is contained in:
Jeff Young 2022-12-23 22:02:59 +00:00
parent 981254543a
commit 6f44468385
8 changed files with 172 additions and 174 deletions

View File

@ -1,5 +1,6 @@
{
"board": {
"3dviewports": [],
"design_settings": {
"defaults": {
"board_outline_line_width": 0.1,
@ -219,6 +220,10 @@
"hier_label_mismatch": "error",
"label_dangling": "error",
"lib_symbol_issues": "warning",
"missing_bidi_pin": "warning",
"missing_input_pin": "warning",
"missing_power_pin": "error",
"missing_unit": "warning",
"multiple_net_names": "warning",
"net_not_bus_member": "warning",
"no_connect_connected": "warning",
@ -228,6 +233,7 @@
"pin_to_pin": "warning",
"power_pin_not_driven": "error",
"similar_labels": "warning",
"simulation_model_issue": "error",
"unannotated": "error",
"unit_value_mismatch": "error",
"unresolved_variable": "error",
@ -245,7 +251,7 @@
"net_settings": {
"classes": [
{
"bus_width": 12.0,
"bus_width": 12,
"clearance": 0.2,
"diff_pair_gap": 0.25,
"diff_pair_via_gap": 0.25,
@ -259,13 +265,15 @@
"track_width": 0.25,
"via_diameter": 0.8,
"via_drill": 0.4,
"wire_width": 6.0
"wire_width": 6
}
],
"meta": {
"version": 2
"version": 3
},
"net_colors": null
"net_colors": null,
"netclass_assignments": null,
"netclass_patterns": []
},
"pcbnew": {
"last_paths": {
@ -318,6 +326,8 @@
"plot_directory": "",
"spice_adjust_passive_values": false,
"spice_external_command": "spice \"%I\"",
"spice_save_all_currents": false,
"spice_save_all_voltages": false,
"subpart_first_id": 65,
"subpart_id_separator": 0
},

View File

@ -826,34 +826,14 @@ SCH_ITEM* SCH_SHEET_LIST::GetItem( const KIID& aID, SCH_SHEET_PATH* aPathOut ) c
{
for( const SCH_SHEET_PATH& sheet : *this )
{
SCH_SCREEN* screen = sheet.LastScreen();
SCH_ITEM* item = sheet.GetItem( aID );
for( SCH_ITEM* aItem : screen->Items() )
if( item )
{
if( aItem->m_Uuid == aID )
{
if( aPathOut )
*aPathOut = sheet;
if( aPathOut )
*aPathOut = sheet;
return aItem;
}
SCH_ITEM* childMatch = nullptr;
aItem->RunOnChildren(
[&]( SCH_ITEM* aChild )
{
if( aChild->m_Uuid == aID )
childMatch = aChild;
} );
if( childMatch )
{
if( aPathOut )
*aPathOut = sheet;
return childMatch;
}
return item;
}
}
@ -862,6 +842,30 @@ SCH_ITEM* SCH_SHEET_LIST::GetItem( const KIID& aID, SCH_SHEET_PATH* aPathOut ) c
}
SCH_ITEM* SCH_SHEET_PATH::GetItem( const KIID& aID ) const
{
for( SCH_ITEM* aItem : LastScreen()->Items() )
{
if( aItem->m_Uuid == aID )
return aItem;
SCH_ITEM* childMatch = nullptr;
aItem->RunOnChildren(
[&]( SCH_ITEM* aChild )
{
if( aChild->m_Uuid == aID )
childMatch = aChild;
} );
if( childMatch )
return childMatch;
}
return nullptr;
}
void SCH_SHEET_LIST::FillItemMap( std::map<KIID, EDA_ITEM*>& aMap )
{
for( const SCH_SHEET_PATH& sheet : *this )

View File

@ -267,6 +267,11 @@ public:
///< @copydoc SCH_SHEET_PATH::LastScreen()
SCH_SCREEN* LastScreen() const;
/**
* Fetch a SCH_ITEM by ID.
*/
SCH_ITEM* GetItem( const KIID& aID ) const;
/**
* Return the path of time stamps which do not changes even when editing sheet parameters.
*

View File

@ -93,17 +93,9 @@ public:
switch( aNewState )
{
case SIM_IDLE:
event = new wxCommandEvent( EVT_SIM_FINISHED );
break;
case SIM_RUNNING:
event = new wxCommandEvent( EVT_SIM_STARTED );
break;
default:
wxFAIL;
return;
case SIM_IDLE: event = new wxCommandEvent( EVT_SIM_FINISHED ); break;
case SIM_RUNNING: event = new wxCommandEvent( EVT_SIM_STARTED ); break;
default: wxFAIL; return;
}
wxQueueEvent( m_parent, event );
@ -505,8 +497,8 @@ void SIM_PLOT_FRAME::StartSimulation( const wxString& aSimCommand )
{
wxBusyCursor toggle;
updateTuners();
applyTuners();
// Prevents memory leak on succeding simulations by deleting old vectors
m_simulator->Clean();
m_simulator->Run();
@ -561,35 +553,31 @@ void SIM_PLOT_FRAME::AddCurrentPlot( const wxString& aDeviceName )
}
void SIM_PLOT_FRAME::AddTuner( SCH_SYMBOL* aSymbol )
void SIM_PLOT_FRAME::AddTuner( const SCH_SHEET_PATH& aSheetPath, SCH_SYMBOL* aSymbol )
{
SIM_PANEL_BASE* plotPanel = getCurrentPlotWindow();
if( !plotPanel )
return;
wxString ref = aSymbol->GetField( REFERENCE_FIELD )->GetShownText();
wxString ref = aSymbol->GetRef( &aSheetPath );
// Do not add multiple instances for the same component.
auto tunerIt = std::find_if( m_tuners.begin(), m_tuners.end(),
[ref]( const TUNER_SLIDER* tuner )
{
return tuner->GetComponentName() == ref;
} );
for( TUNER_SLIDER* tuner : m_tuners )
{
if( tuner->GetSymbolRef() == ref )
return;
}
if( tunerIt != m_tuners.end() )
return; // We already have it.
const SPICE_ITEM* item = GetExporter()->FindItem( std::string( ref.ToUTF8() ) );
const SIM_MODEL::PARAM* tunerParam = item->model->GetTunerParam();
const SPICE_ITEM* item = GetExporter()->FindItem( std::string( ref.ToUTF8() ) );
// Do nothing if the symbol is not tunable.
if( !tunerParam )
if( !item || !item->model->GetTunerParam() )
return;
try
{
TUNER_SLIDER* tuner = new TUNER_SLIDER( this, m_tunePanel, aSymbol );
TUNER_SLIDER* tuner = new TUNER_SLIDER( this, m_tunePanel, aSheetPath, aSymbol );
m_tuneSizer->Add( tuner );
m_tuners.push_back( tuner );
m_tunePanel->Layout();
@ -601,28 +589,40 @@ void SIM_PLOT_FRAME::AddTuner( SCH_SYMBOL* aSymbol )
}
void SIM_PLOT_FRAME::UpdateTunerValue( const KIID& aSymbol, const wxString& aValue )
void SIM_PLOT_FRAME::UpdateTunerValue( const SCH_SHEET_PATH& aSheetPath, const KIID& aSymbol,
const wxString& aRef, const wxString& aValue )
{
SCH_SHEET_PATH sheet;
SCH_ITEM* item = m_schematicFrame->Schematic().GetSheets().GetItem( aSymbol, &sheet );
SCH_SYMBOL* symbol = dynamic_cast<SCH_SYMBOL*>( item );
SCH_ITEM* item = aSheetPath.GetItem( aSymbol );
SCH_SYMBOL* symbol = dynamic_cast<SCH_SYMBOL*>( item );
if( symbol )
if( !symbol )
{
SIM_LIB_MGR mgr( &Prj() );
SIM_MODEL& model = mgr.CreateModel( &sheet, *symbol ).model;
const SIM_MODEL::PARAM* tunerParam = model.GetTunerParam();
if( !tunerParam )
return;
model.SetParamValue( tunerParam->info.name, std::string( aValue.ToUTF8() ) );
model.WriteFields( symbol->GetFields() );
m_schematicFrame->UpdateItem( symbol, false, true );
m_schematicFrame->OnModify();
// TODO: 8.0: wxT() --> _()
DisplayErrorMessage( this, wxString::Format( wxT( "Could not apply tuned value.\n"
"%s not found." ),
aRef ) );
return;
}
SIM_LIB_MGR mgr( &Prj() );
SIM_MODEL& model = mgr.CreateModel( &aSheetPath, *symbol ).model;
const SIM_MODEL::PARAM* tunerParam = model.GetTunerParam();
if( !tunerParam )
{
// TODO: 8.0: wxT() --> _()
DisplayErrorMessage( this, wxString::Format( wxT( "Could not apply tuned value.\n"
"%s is not tunable." ),
aRef ) );
return;
}
model.SetParamValue( tunerParam->info.name, std::string( aValue.ToUTF8() ) );
model.WriteFields( symbol->GetFields() );
m_schematicFrame->UpdateItem( symbol, false, true );
m_schematicFrame->OnModify();
}
@ -640,9 +640,8 @@ SIM_PLOT_PANEL* SIM_PLOT_FRAME::GetCurrentPlot() const
{
SIM_PANEL_BASE* curPage = getCurrentPlotWindow();
return ( ( !curPage || curPage->GetType() == ST_UNKNOWN ) ?
nullptr :
dynamic_cast<SIM_PLOT_PANEL*>( curPage ) );
return !curPage || curPage->GetType() == ST_UNKNOWN ? nullptr
: dynamic_cast<SIM_PLOT_PANEL*>( curPage );
}
@ -652,14 +651,6 @@ const NGSPICE_CIRCUIT_MODEL* SIM_PLOT_FRAME::GetExporter() const
}
std::shared_ptr<SPICE_SIMULATOR_SETTINGS>& SIM_PLOT_FRAME::GetSimulatorSettings()
{
wxASSERT( m_simulator->Settings() );
return m_simulator->Settings();
}
void SIM_PLOT_FRAME::addPlot( const wxString& aName, SIM_PLOT_TYPE aType )
{
SIM_TYPE simType = m_circuitModel->GetSimType();
@ -708,9 +699,7 @@ void SIM_PLOT_FRAME::addPlot( const wxString& aName, SIM_PLOT_TYPE aType )
}
if( updated )
{
updateSignalList();
}
}
@ -909,38 +898,42 @@ void SIM_PLOT_FRAME::updateSignalList()
}
void SIM_PLOT_FRAME::updateTuners()
{
const std::list<SPICE_ITEM>& spiceItems = m_circuitModel->GetItems();
for( auto it = m_tuners.begin(); it != m_tuners.end(); /* iteration inside the loop */ )
{
const wxString& ref = (*it)->GetComponentName();
if( std::find_if( spiceItems.begin(), spiceItems.end(),
[&]( const SPICE_ITEM& item )
{
return item.refName == ref;
}
) == spiceItems.end() )
{
// The component does not exist anymore, remove the associated tuner
TUNER_SLIDER* tuner = *it;
it = m_tuners.erase( it );
RemoveTuner( tuner, false );
}
else
{
++it;
}
}
}
void SIM_PLOT_FRAME::applyTuners()
{
wxString errors;
WX_STRING_REPORTER reporter( &errors );
for( const TUNER_SLIDER* tuner : m_tuners )
m_simulator->Command( tuner->GetTunerCommand() );
{
SCH_SHEET_PATH sheetPath;
wxString ref = tuner->GetSymbolRef();
KIID symbolId = tuner->GetSymbol( &sheetPath );
SCH_ITEM* schItem = sheetPath.GetItem( symbolId );
SCH_SYMBOL* symbol = dynamic_cast<SCH_SYMBOL*>( schItem );
if( !symbol )
{
reporter.Report( wxString::Format( _( "%s not found." ), ref ) );
continue;
}
const SPICE_ITEM* item = GetExporter()->FindItem( tuner->GetSymbolRef().ToStdString() );
if( !item || !item->model->GetTunerParam() )
{
// TODO: 8.0: wxT() --> _()
reporter.Report( wxString::Format( wxT( "%s is not tunable." ), ref ) );
continue;
}
SIM_VALUE_FLOAT floatVal( tuner->GetValue().ToDouble() );
m_simulator->Command( item->model->SpiceGenerator().TunerCommand( *item, floatVal ) );
}
// TODO: 8.0: wxT() --> _()
if( reporter.HasMessage() )
DisplayErrorMessage( this, wxT( "Errors applying tuned values.\n\n" ) + errors );
}
@ -1903,7 +1896,9 @@ void SIM_PLOT_FRAME::onSimUpdate( wxCommandEvent& aEvent )
m_simulator->Run();
}
else
{
DisplayErrorMessage( this, _( "Another simulation is already running." ) );
}
}
updateInProgress = false;
}
@ -1944,17 +1939,9 @@ void SIM_PLOT_FRAME::SIGNAL_CONTEXT_MENU::onMenuEvent( wxMenuEvent& aEvent )
switch( aEvent.GetId() )
{
case HIDE_SIGNAL:
m_plotFrame->removePlot( m_signal );
break;
case SHOW_CURSOR:
plot->EnableCursor( m_signal, true );
break;
case HIDE_CURSOR:
plot->EnableCursor( m_signal, false );
break;
case HIDE_SIGNAL: m_plotFrame->removePlot( m_signal ); break;
case SHOW_CURSOR: plot->EnableCursor( m_signal, true ); break;
case HIDE_CURSOR: plot->EnableCursor( m_signal, false ); break;
}
}

View File

@ -28,11 +28,6 @@
#ifndef __SIM_PLOT_FRAME__
#define __SIM_PLOT_FRAME__
/**
* @file sim_plot_frame.h
*
* Subclass of SIM_PLOT_FRAME_BASE, which is generated by wxFormBuilder.
*/
#include "sim_plot_frame_base.h"
#include "sim_types.h"
@ -97,7 +92,7 @@ public:
/**
* Add a tuner for a symbol.
*/
void AddTuner( SCH_SYMBOL* aSymbol );
void AddTuner( const SCH_SHEET_PATH& aSheetPath, SCH_SYMBOL* aSymbol );
/**
* Remove an existing tuner.
@ -116,7 +111,8 @@ public:
* @param aId id of the symbol field
* @param aValue new value of the symbol field
*/
void UpdateTunerValue( const KIID& aSymbol, const wxString& aValue );
void UpdateTunerValue( const SCH_SHEET_PATH& aSheetPath, const KIID& aSymbol,
const wxString& aRef, const wxString& aValue );
/**
* Return the currently opened plot panel (or NULL if there is none).
@ -143,8 +139,6 @@ public:
// Simulator doesn't host a tool framework
wxWindow* GetToolCanvas() const override { return nullptr; }
std::shared_ptr<SPICE_SIMULATOR_SETTINGS>& GetSimulatorSettings();
private:
/**
* Load the currently active workbook stored in the project settings. If there is none,
@ -195,13 +189,6 @@ private:
*/
void updateSignalList();
/**
* Filter out tuners for components that do not exist anymore.
*
* Decisions are based on the current #NETLIST_EXPORTER_BASE data.
*/
void updateTuners();
/**
* Apply component values specified using tuner sliders to the current netlist.
*/

View File

@ -1019,15 +1019,16 @@ int SCH_EDITOR_CONTROL::SimTune( const TOOL_EVENT& aEvent )
return false;
}
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
KIWAY_PLAYER* simFrame = m_frame->Kiway().Player( FRAME_SIMULATOR, false );
SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item );
SCH_SHEET_PATH sheetPath = symbol->Schematic()->CurrentSheet();
KIWAY_PLAYER* simFrame = m_frame->Kiway().Player( FRAME_SIMULATOR, false );
if( simFrame )
{
if( wxWindow* blocking_win = simFrame->Kiway().GetBlockingDialog() )
blocking_win->Close( true );
static_cast<SIM_PLOT_FRAME*>( simFrame )->AddTuner( symbol );
static_cast<SIM_PLOT_FRAME*>( simFrame )->AddTuner( sheetPath, symbol );
}
// We do not really want to keep a symbol selected in schematic,

View File

@ -36,19 +36,20 @@
// Must be after other includes to avoid conflict with a window header on msys2
#include "tuner_slider.h"
TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent, SCH_SYMBOL* aSymbol ) :
TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent,
const SCH_SHEET_PATH& aSheetPath, SCH_SYMBOL* aSymbol ) :
TUNER_SLIDER_BASE( aParent ),
m_symbol( aSymbol->m_Uuid ),
m_symbol( aSymbol->m_Uuid ), m_sheetPath( aSheetPath ),
m_min( 0.0 ),
m_max( 0.0 ),
m_value( 0.0 ),
m_changed( false ),
m_frame ( aFrame )
{
wxString ref = aSymbol->GetField( REFERENCE_FIELD )->GetShownText();
m_item = aFrame->GetExporter()->FindItem( std::string( ref.ToUTF8() ) );
wxString ref = aSymbol->GetRef( &m_sheetPath );
const SPICE_ITEM* item = aFrame->GetExporter()->FindItem( std::string( ref.ToUTF8() ) );
if( !m_item )
if( !item )
{
throw KI_PARAM_ERROR( wxString::Format( _( "Could not find Spice item with reference '%s'" ),
ref ) );
@ -57,21 +58,21 @@ TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent, SCH_SYMBO
m_name->SetLabel( ref );
m_closeBtn->SetBitmap( KiBitmap( BITMAPS::small_trash ) );
const SIM_MODEL::PARAM* tunerParam = m_item->model->GetTunerParam();
const SIM_MODEL::PARAM* tunerParam = item->model->GetTunerParam();
if( !tunerParam )
{
throw KI_PARAM_ERROR( wxString::Format( _( "Symbol '%s' has simulation model of type '%s %s', "
"which cannot be tuned" ),
ref,
m_item->model->GetDeviceInfo().fieldValue,
m_item->model->GetTypeInfo().fieldValue ) );
item->model->GetDeviceInfo().fieldValue,
item->model->GetTypeInfo().fieldValue ) );
}
// Special case for potentiometers because we don't have value ranges implemented yet.
if( m_item->model->GetType() == SIM_MODEL::TYPE::R_POT )
if( item->model->GetType() == SIM_MODEL::TYPE::R_POT )
{
std::string valueStr = m_item->model->GetTunerParam()->value->ToSpiceString();
std::string valueStr = item->model->GetTunerParam()->value->ToSpiceString();
if( valueStr != "" )
m_value = SPICE_VALUE( valueStr );
@ -83,7 +84,7 @@ TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent, SCH_SYMBO
}
else
{
m_value = SPICE_VALUE( m_item->model->GetTunerParam()->value->ToSpiceString() );
m_value = SPICE_VALUE( item->model->GetTunerParam()->value->ToSpiceString() );
m_min = SPICE_VALUE( 0.5 ) * m_value;
m_max = SPICE_VALUE( 2.0 ) * m_value;
}
@ -101,13 +102,6 @@ TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent, SCH_SYMBO
}
std::string TUNER_SLIDER::GetTunerCommand() const
{
return m_item->model->SpiceGenerator().TunerCommand( *m_item,
SIM_VALUE_FLOAT( m_value.ToDouble() ) );
}
bool TUNER_SLIDER::SetValue( const SPICE_VALUE& aVal )
{
// Get the value into the current range boundaries
@ -235,7 +229,7 @@ void TUNER_SLIDER::onClose( wxCommandEvent& event )
void TUNER_SLIDER::onSave( wxCommandEvent& event )
{
m_frame->UpdateTunerValue( m_symbol, m_value.ToOrigString() );
m_frame->UpdateTunerValue( m_sheetPath, m_symbol, GetSymbolRef(), m_value.ToOrigString() );
}

View File

@ -43,9 +43,10 @@ class SCH_SYMBOL;
class TUNER_SLIDER : public TUNER_SLIDER_BASE
{
public:
TUNER_SLIDER( SIM_PLOT_FRAME *aFrame, wxWindow* aParent, SCH_SYMBOL* aSymbol );
TUNER_SLIDER( SIM_PLOT_FRAME *aFrame, wxWindow* aParent, const SCH_SHEET_PATH& aSheetPath,
SCH_SYMBOL* aSymbol );
wxString GetComponentName() const
wxString GetSymbolRef() const
{
return m_name->GetLabel();
}
@ -60,7 +61,16 @@ public:
return m_max;
}
std::string GetTunerCommand() const;
const SPICE_VALUE& GetValue() const
{
return m_value;
}
KIID GetSymbol( SCH_SHEET_PATH* aSheetPath ) const
{
*aSheetPath = m_sheetPath;
return m_symbol;
}
bool SetValue( const SPICE_VALUE& aVal );
bool SetMin( const SPICE_VALUE& aVal );
@ -92,15 +102,15 @@ private:
///< Timer that restarts the simulation after the slider value has changed
wxTimer m_simTimer;
KIID m_symbol;
const SPICE_ITEM* m_item;
KIID m_symbol;
SCH_SHEET_PATH m_sheetPath;
SPICE_VALUE m_min;
SPICE_VALUE m_max;
SPICE_VALUE m_value;
bool m_changed;
SPICE_VALUE m_min;
SPICE_VALUE m_max;
SPICE_VALUE m_value;
bool m_changed;
SIM_PLOT_FRAME* m_frame;
SIM_PLOT_FRAME* m_frame;
};
#endif /* TUNER_SLIDER_H */