From 5cf474e1c822dc8b2465880a01ae2703a116e65a Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Tue, 5 May 2020 10:55:54 -0400 Subject: [PATCH] Eeschema: fix bugs in symbol edit properties dialog. Use flattened (root) library symbols to prevent broken library symbols in schematic files. Remove the edited symbol from screen before making changes to the symbol to prevent potential orphaned symbol libraries being saved in schematic file. Add some defensive programming to let developers know that an invalid library symbol link was used when calling SCH_COMPONENT::SetLibSymbol(). --- .../dialog_edit_component_in_schematic.cpp | 18 +++++++++++++++--- eeschema/sch_component.cpp | 2 ++ eeschema/sch_component.h | 11 +++++++---- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/eeschema/dialogs/dialog_edit_component_in_schematic.cpp b/eeschema/dialogs/dialog_edit_component_in_schematic.cpp index 481e16e19a..c4939ed21e 100644 --- a/eeschema/dialogs/dialog_edit_component_in_schematic.cpp +++ b/eeschema/dialogs/dialog_edit_component_in_schematic.cpp @@ -370,6 +370,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::Validate() return false; } } + m_libraryNameTextCtrl->SetValue( id.Format() ); // Check for missing field names. @@ -398,6 +399,14 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataFromWindow() if( !wxDialog::TransferDataFromWindow() ) // Calls our Validate() method. return false; + SCH_SCREEN* currentScreen = GetParent()->GetScreen(); + + wxCHECK( currentScreen, false ); + + // This needs to be done before the LIB_ID is changed to prevent stale library symbols in + // the schematic file. + currentScreen->Remove( m_cmp ); + wxString msg; // save old cmp in undo list if not already in edit, or moving ... @@ -428,7 +437,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataFromWindow() return false; } - m_cmp->SetLibSymbol( new LIB_PART( *libSymbol ) ); + m_cmp->SetLibSymbol( libSymbol->Flatten().release() ); m_cmp->SetLibId( id ); // For symbols with multiple shapes (De Morgan representation) Set the selected shape: @@ -520,8 +529,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataFromWindow() } } - m_cmp->UpdatePins(); - + currentScreen->Append( m_cmp ); GetParent()->TestDanglingEnds(); GetParent()->RefreshItem( m_cmp ); GetParent()->OnModify(); @@ -579,7 +587,9 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnDeleteField( wxCommandEvent& event ) int curRow = m_grid->GetGridCursorRow(); if( curRow < 0 ) + { return; + } else if( curRow < MANDATORY_FIELDS ) { DisplayError( this, wxString::Format( _( "The first %d fields are mandatory." ), @@ -621,7 +631,9 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnMoveUp( wxCommandEvent& event ) m_grid->MakeCellVisible( m_grid->GetGridCursorRow(), m_grid->GetGridCursorCol() ); } else + { wxBell(); + } } diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp index 6379b2f21a..4a5b1bbde0 100644 --- a/eeschema/sch_component.cpp +++ b/eeschema/sch_component.cpp @@ -247,6 +247,8 @@ wxString SCH_COMPONENT::GetSchSymbolLibraryName() const void SCH_COMPONENT::SetLibSymbol( LIB_PART* aLibSymbol ) { + wxCHECK2( ( aLibSymbol == nullptr ) || ( aLibSymbol->IsRoot() ), aLibSymbol = nullptr ); + m_part.reset( aLibSymbol ); UpdatePins(); } diff --git a/eeschema/sch_component.h b/eeschema/sch_component.h index 5862a6d9ca..f326cb2c94 100644 --- a/eeschema/sch_component.h +++ b/eeschema/sch_component.h @@ -224,14 +224,17 @@ public: * Set this schematic symbol library symbol reference to \a aLibSymbol * * The schematic symbol object owns \a aLibSymbol and the pin list will be updated - * accordingly. The #LIB_ID object must be valid ( have both a library nickname and - * symbol name ) in order to set the schematic symbol #LIB_ID. Otherwise an assertion - * will be raised in debug builds and the library symbol will be cleared. The new file - * format will no longer require a cache library so all library symbols must be valid. + * accordingly. The #LIB_PART object can be null to clear the library symbol link + * as well as the pin map. If the #LIB_PART object is not null, it must be a root + * symbol. Otherwise an assertion will be raised in debug builds and the library + * symbol will be cleared. The new file format will no longer require a cache + * library so all library symbols must be valid. * * @note This is the only way to publicly set the library symbol for a schematic * symbol except for the ctors that take a LIB_PART reference. All previous * public resolvers have been deprecated. + * + * @param aLibSymbol is the library symbol to associate with this schematic symbol. */ void SetLibSymbol( LIB_PART* aLibSymbol );