electron-rare/kicad-source-mirror
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix empty project name in symbol and sheet instances

Browse Source 
Virtual root sheet set symbol and sheet instance projectnames
to be saved as empty strings. SCH_SHEET_PATH::Path()
skips the virtual root, so instance paths never start
with niluuid. Existing root UUID comparison was dead
code.

Use GetTopLevelSheets() to find current project instances
when the root is virtual and do direct root UUID comparison
for non-virtual roots. At save time, populate the project
name from GetProjectName() for current project instances
instead of using the stored (empty) m_ProjectName.

This fixes project rename not updating instance data,
and broken cross-project sheet sharing.
This commit is contained in:
Damjan Prerad
2026-03-03 19:53:24 +01:00
parent 40205eb181
commit 4836d172c7
3 changed files with 204 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp
+38 -7
View File
@@ -811,12 +811,27 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
if( !aSymbol->GetInstances().empty() )
{
std::map<KIID, std::vector<SCH_SYMBOL_INSTANCE>> projectInstances;
std::set<KIID> currentProjectKeys;
m_out->Print( "(instances" );
wxString projectName;
KIID rootSheetUuid = aSchematic.Root().m_Uuid;
// Collect top-level sheet UUIDs to identify current project instances.
// When root is virtual (niluuid), Path() skips it, so instance paths
// start with the real top-level sheet UUID, not niluuid.
if( rootSheetUuid == niluuid )
{
for( const SCH_SHEET* sheet : aSchematic.GetTopLevelSheets() )
currentProjectKeys.insert( sheet->m_Uuid );
}
else
{
currentProjectKeys.insert( rootSheetUuid );
}
for( const SCH_SYMBOL_INSTANCE& inst : aSymbol->GetInstances() )
{
// Zero length KIID_PATH objects are not valid and will cause a crash below.
@@ -830,7 +845,6 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
// paths may include the virtual root, but SCH_SHEET_PATH::Path() skips it. We need
// to normalize the path by removing the virtual root before comparison.
KIID_PATH pathToCheck = inst.m_Path;
bool hasVirtualRoot = false;
// If root is virtual (niluuid) and path starts with virtual root, strip it
if( rootSheetUuid == niluuid && !pathToCheck.empty() && pathToCheck[0] == niluuid )
@@ -838,7 +852,6 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
if( pathToCheck.size() > 1 )
{
pathToCheck.erase( pathToCheck.begin() );
hasVirtualRoot = true;
}
else
{
@@ -850,7 +863,8 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
// Check if this instance is orphaned (no matching sheet path)
// For virtual root, we check if the first real sheet matches one of the top-level sheets
// For non-virtual root, we check if it matches the root sheet UUID
bool belongsToThisProject = hasVirtualRoot || pathToCheck[0] == rootSheetUuid;
bool belongsToThisProject = currentProjectKeys.count( pathToCheck[0] );
bool isOrphaned = belongsToThisProject && !aSheetList.GetSheetPathByKIIDPath( pathToCheck );
// Keep all instance data when copying to the clipboard. They may be needed on paste.
@@ -878,7 +892,10 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
return aLhs.m_Path < aRhs.m_Path;
} );
projectName = instances[0].m_ProjectName;
if( currentProjectKeys.count( uuid ) )
projectName = m_schematic->Project().GetProjectName();
else
projectName = instances[0].m_ProjectName;
m_out->Print( "(project %s", m_out->Quotew( projectName ).c_str() );
@@ -1096,6 +1113,18 @@ void SCH_IO_KICAD_SEXPR::saveSheet( SCH_SHEET* aSheet, const SCH_SHEET_LIST& aSh
KIID rootSheetUuid = m_schematic->Root().m_Uuid;
bool inProjectClause = false;
std::set<KIID> currentProjectKeys;
if( rootSheetUuid == niluuid )
{
for( const SCH_SHEET* sheet : m_schematic->GetTopLevelSheets() )
currentProjectKeys.insert( sheet->m_Uuid );
}
else
{
currentProjectKeys.insert( rootSheetUuid );
}
for( size_t i = 0; i < sheetInstances.size(); i++ )
{
// If the instance data is part of this design but no longer has an associated sheet
@@ -1103,8 +1132,10 @@ void SCH_IO_KICAD_SEXPR::saveSheet( SCH_SHEET* aSheet, const SCH_SHEET_LIST& aSh
// current project from accumulating in the schematic files.
//
// Keep all instance data when copying to the clipboard. It may be needed on paste.
if( ( sheetInstances[i].m_Path[0] == rootSheetUuid )
&& !aSheetList.GetSheetPathByKIIDPath( sheetInstances[i].m_Path, false ) )
bool belongsToThisProject =
!sheetInstances[i].m_Path.empty() && currentProjectKeys.count( sheetInstances[i].m_Path[0] );
if( belongsToThisProject && !aSheetList.GetSheetPathByKIIDPath( sheetInstances[i].m_Path, false ) )
{
if( inProjectClause && ( ( i + 1 == sheetInstances.size() )
|| lastProjectUuid != sheetInstances[i+1].m_Path[0] ) )
@@ -1120,7 +1151,7 @@ void SCH_IO_KICAD_SEXPR::saveSheet( SCH_SHEET* aSheet, const SCH_SHEET_LIST& aSh
{
wxString projectName;
if( sheetInstances[i].m_Path[0] == rootSheetUuid )
if( belongsToThisProject )
projectName = m_schematic->Project().GetProjectName();
else
projectName = sheetInstances[i].m_ProjectName;
qa/tests/eeschema/CMakeLists.txt
+1
View File
@@ -124,6 +124,7 @@ set( QA_EESCHEMA_SRCS
test_flat_hierarchy.cpp
test_multi_top_level_sheets.cpp
test_save_load_schematic.cpp
test_project_name_instances.cpp
test_legacy_load.cpp
test_symbol_library_manager.cpp
test_symbol_import_manager.cpp
qa/tests/eeschema/test_project_name_instances.cpp
+165
View File
@@ -0,0 +1,165 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright The KiCad Developers, see AUTHORS.txt for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, you may find one here:
* http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
* or you may search the http://www.gnu.org website for the version 2 license,
* or you may write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
*/
/**
* @file test_project_name_instances.cpp
* Test that symbol and sheet instances are saved with the correct project name,
* not an empty string. Regression test for the virtual root project name bug.
*/
#include <boost/test/unit_test.hpp>
#include <eeschema_test_utils.h>
#include <sch_io/kicad_sexpr/sch_io_kicad_sexpr.h>
#include <sch_screen.h>
#include <sch_sheet.h>
#include <sch_symbol.h>
#include <schematic.h>
#include <kiid.h>
#include <locale_io.h>
#include <settings/settings_manager.h>
#include <wx/filename.h>
#include <wx/stdpaths.h>
#include <wx/textfile.h>
struct PROJECT_NAME_FIXTURE
{
PROJECT_NAME_FIXTURE() :
m_settingsManager()
{
wxString tempDir = wxStandardPaths::Get().GetTempDir();
wxString projectPath = tempDir + wxFileName::GetPathSeparator() + wxT( "test_project.kicad_pro" );
m_tempFiles.push_back( projectPath );
m_settingsManager.LoadProject( projectPath.ToStdString() );
m_schematic = std::make_unique<SCHEMATIC>( nullptr );
m_project = &m_settingsManager.Prj();
m_schematic->SetProject( m_project );
}
~PROJECT_NAME_FIXTURE()
{
for( const wxString& file : m_tempFiles )
{
if( wxFileExists( file ) )
wxRemoveFile( file );
}
m_schematic.reset();
}
wxString GetTempSchFile( const wxString& aPrefix )
{
wxString tempDir = wxStandardPaths::Get().GetTempDir();
wxString fileName = wxFileName::CreateTempFileName( tempDir + wxFileName::GetPathSeparator() + aPrefix );
fileName += wxT( ".kicad_sch" );
m_tempFiles.push_back( fileName );
return fileName;
}
SETTINGS_MANAGER m_settingsManager;
std::unique_ptr<SCHEMATIC> m_schematic;
PROJECT* m_project;
std::vector<wxString> m_tempFiles;
};
BOOST_FIXTURE_TEST_SUITE( ProjectNameInstances, PROJECT_NAME_FIXTURE )
/**
* Test that symbol instances are saved with the current project name,
* not an empty string, when using the virtual root pattern.
*/
BOOST_AUTO_TEST_CASE( SymbolInstanceProjectName )
{
LOCALE_IO dummy;
m_schematic->CreateDefaultScreens();
std::vector<SCH_SHEET*> topSheets = m_schematic->GetTopLevelSheets();
BOOST_REQUIRE( !topSheets.empty() );
SCH_SCREEN* screen = topSheets[0]->GetScreen();
BOOST_REQUIRE( screen != nullptr );
// Create a symbol
SCH_SYMBOL* symbol = new SCH_SYMBOL();
symbol->SetLibId( LIB_ID( wxT( "Device" ), wxT( "R" ) ) );
symbol->SetPosition( VECTOR2I( 0, 0 ) );
symbol->GetField( FIELD_T::REFERENCE )->SetText( wxT( "R1" ) );
symbol->SetPrefix( wxT( "R" ) );
screen->Append( symbol );
// Build hierarchy and set the reference (this creates an instance
// with empty m_ProjectName, simulating the bug condition)
m_schematic->RefreshHierarchy();
SCH_SHEET_LIST hierarchy = m_schematic->Hierarchy();
BOOST_REQUIRE( !hierarchy.empty() );
SCH_SHEET_PATH& path = hierarchy[0];
symbol->SetRef( &path, wxT( "R1" ) );
// Verify the instance exists but has empty project name (the bug)
SCH_SYMBOL_INSTANCE inst;
BOOST_REQUIRE( symbol->GetInstance( inst, path.Path() ) );
BOOST_CHECK( inst.m_ProjectName.IsEmpty() );
// Save the schematic
wxString fileName = GetTempSchFile( wxT( "test_projname" ) );
SCH_IO_KICAD_SEXPR io;
io.SaveSchematicFile( fileName, topSheets[0], m_schematic.get() );
BOOST_REQUIRE( wxFileExists( fileName ) );
BOOST_TEST_MESSAGE( "Saved file: " + fileName.ToStdString() );
BOOST_TEST_MESSAGE( "Project name: " + m_project->GetProjectName().ToStdString() );
// Read the saved file as text and check for project name
wxTextFile textFile;
BOOST_REQUIRE( textFile.Open( fileName ) );
bool foundProjectClause = false;
bool foundEmptyProject = false;
for( size_t i = 0; i < textFile.GetLineCount(); i++ )
{
wxString line = textFile.GetLine( i );
if( line.Contains( wxT( "(project \"test_project\"" ) ) )
foundProjectClause = true;
if( line.Contains( wxT( "(project \"\"" ) ) )
foundEmptyProject = true;
}
textFile.Close();
BOOST_CHECK_MESSAGE( foundProjectClause, "Saved file must contain (project \"test_project\")" );
BOOST_CHECK_MESSAGE( !foundEmptyProject, "Saved file must NOT contain (project \"\")" );
}
BOOST_AUTO_TEST_SUITE_END()