From 84f927d0575c262f49d8a4e843831c3ab773aeb3 Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Thu, 15 Dec 2022 09:57:53 -0500 Subject: [PATCH] Schematic import bug fix. Check for already loaded schematics in the current sheet path as well as the current project root sheet path to prevent multiple loads of shared schematic. This bug was causing shared sheets to be loaded more than once which caused instance data to get separated by each copy rather than saved in one copy of the schematic which would result in all instance data being lost except the last saved copy of the schematic. This bug has been around forever and may be the cause of some unexplained schematic instance data corruption issues. This bug does not apply when opening the full project. Fixes https://gitlab.com/kicad/code/kicad/-/issues/11076 --- common/trace_helpers.cpp | 1 + .../sch_plugins/kicad/sch_sexpr_plugin.cpp | 23 +++++++++++++------ eeschema/sch_plugins/kicad/sch_sexpr_plugin.h | 1 + include/trace_helpers.h | 7 ++++++ 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/common/trace_helpers.cpp b/common/trace_helpers.cpp index 4c361f9968..8c6da2a6c0 100644 --- a/common/trace_helpers.cpp +++ b/common/trace_helpers.cpp @@ -39,6 +39,7 @@ const wxChar* const kicadTraceCoroutineStack = wxT( "KICAD_COROUTINE_STACK" ); const wxChar* const traceSchLibMem = wxT( "KICAD_SCH_LIB_MEM" ); const wxChar* const traceFindItem = wxT( "KICAD_FIND_ITEM" ); const wxChar* const traceSchLegacyPlugin = wxT( "KICAD_SCH_LEGACY_PLUGIN" ); +const wxChar* const traceSchPlugin = wxT( "KICAD_SCH_PLUGIN" ); const wxChar* const traceGedaPcbPlugin = wxT( "KICAD_GEDA_PLUGIN" ); const wxChar* const traceKicadPcbPlugin = wxT( "KICAD_PCB_PLUGIN" ); const wxChar* const tracePrinting = wxT( "KICAD_PRINT" ); diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp index 0ef5ed8e78..db6de68937 100644 --- a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp +++ b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp @@ -115,7 +115,7 @@ SCH_SHEET* SCH_SEXPR_PLUGIN::Load( const wxString& aFileName, SCHEMATIC* aSchema if( aAppendToMe ) { m_appending = true; - wxLogTrace( traceSchLegacyPlugin, "Append \"%s\" to sheet \"%s\".", + wxLogTrace( traceSchPlugin, "Append \"%s\" to sheet \"%s\".", aFileName, aAppendToMe->GetFileName() ); wxFileName normedFn = aAppendToMe->GetFileName(); @@ -129,7 +129,7 @@ SCH_SHEET* SCH_SEXPR_PLUGIN::Load( const wxString& aFileName, SCHEMATIC* aSchema if( m_path.IsEmpty() ) m_path = aSchematic->Prj().GetProjectPath(); - wxLogTrace( traceSchLegacyPlugin, "Normalized append path \"%s\".", m_path ); + wxLogTrace( traceSchPlugin, "Normalized append path \"%s\".", m_path ); } else { @@ -178,6 +178,8 @@ SCH_SHEET* SCH_SEXPR_PLUGIN::Load( const wxString& aFileName, SCHEMATIC* aSchema void SCH_SEXPR_PLUGIN::loadHierarchy( const SCH_SHEET_PATH& aParentSheetPath, SCH_SHEET* aSheet ) { + m_currentSheetPath.push_back( aSheet ); + SCH_SCREEN* screen = nullptr; if( !aSheet->GetScreen() ) @@ -193,10 +195,10 @@ void SCH_SEXPR_PLUGIN::loadHierarchy( const SCH_SHEET_PATH& aParentSheetPath, SC // Save the current path so that it gets restored when descending and ascending the // sheet hierarchy which allows for sheet schematic files to be nested in folders // relative to the last path a schematic was loaded from. - wxLogTrace( traceSchLegacyPlugin, "Saving path '%s'", m_currentPath.top() ); + wxLogTrace( traceSchPlugin, "Saving path '%s'", m_currentPath.top() ); m_currentPath.push( fileName.GetPath() ); - wxLogTrace( traceSchLegacyPlugin, "Current path '%s'", m_currentPath.top() ); - wxLogTrace( traceSchLegacyPlugin, "Loading '%s'", fileName.GetFullPath() ); + wxLogTrace( traceSchPlugin, "Current path '%s'", m_currentPath.top() ); + wxLogTrace( traceSchPlugin, "Loading '%s'", fileName.GetFullPath() ); SCH_SHEET_PATH ancestorSheetPath = aParentSheetPath; @@ -221,7 +223,12 @@ void SCH_SEXPR_PLUGIN::loadHierarchy( const SCH_SHEET_PATH& aParentSheetPath, SC } if( ancestorSheetPath.empty() ) - m_rootSheet->SearchHierarchy( fileName.GetFullPath(), &screen ); + { + // Existing schematics could be either in the root sheet path or the current sheet + // load path so we have to check both. + if( !m_rootSheet->SearchHierarchy( fileName.GetFullPath(), &screen ) ) + m_currentSheetPath.at( 0 )->SearchHierarchy( fileName.GetFullPath(), &screen ); + } if( screen ) { @@ -278,8 +285,10 @@ void SCH_SEXPR_PLUGIN::loadHierarchy( const SCH_SHEET_PATH& aParentSheetPath, SC } m_currentPath.pop(); - wxLogTrace( traceSchLegacyPlugin, "Restoring path \"%s\"", m_currentPath.top() ); + wxLogTrace( traceSchPlugin, "Restoring path \"%s\"", m_currentPath.top() ); } + + m_currentSheetPath.pop_back(); } diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h index 11cbc2935e..6c3ad293e4 100644 --- a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h +++ b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h @@ -174,6 +174,7 @@ protected: wxString m_path; ///< Root project path for loading child sheets. std::stack m_currentPath; ///< Stack to maintain nested sheet paths SCH_SHEET* m_rootSheet; ///< The root sheet of the schematic being loaded. + SCH_SHEET_PATH m_currentSheetPath; SCHEMATIC* m_schematic; OUTPUTFORMATTER* m_out; ///< The formatter for saving SCH_SCREEN objects. SCH_SEXPR_PLUGIN_CACHE* m_cache; diff --git a/include/trace_helpers.h b/include/trace_helpers.h index 9b87ce0c53..6c5f3a393b 100644 --- a/include/trace_helpers.h +++ b/include/trace_helpers.h @@ -106,6 +106,13 @@ extern const wxChar* const traceAutoSave; */ extern const wxChar* const traceSchLibMem; +/** + * Flag to enable legacy schematic plugin debug output. + * + * Use "KICAD_SCH_PLUGIN" to enable. + */ +extern const wxChar* const traceSchPlugin; + /** * Flag to enable legacy schematic plugin debug output. *