Fix Eeschema sheet recursion bugs.

* Add code to test for sheet path recursion to SCH_SHEET_PATH and SCH_SHEET_LIST.
* Add recursion tests to edit sheet code in Eeschema.
* Add recursion tests to block paste code in Eeschema.
This commit is contained in:
Wayne Stambaugh 2015-06-07 16:31:55 -04:00
parent 3bfb3da091
commit 54bad8b5e8
7 changed files with 326 additions and 61 deletions

View File

@ -46,6 +46,7 @@
#include <sch_text.h>
#include <sch_component.h>
#include <sch_sheet.h>
#include <sch_sheet_path.h>
#include <boost/foreach.hpp>
@ -292,7 +293,7 @@ bool SCH_EDIT_FRAME::HandleBlockEnd( wxDC* aDC )
m_canvas->Refresh();
break;
case BLOCK_SAVE: // Save acopy of items in paste buffer
case BLOCK_SAVE: // Save a copy of items in paste buffer
GetScreen()->UpdatePickList();
DrawAndSizingBlockOutlines( m_canvas, aDC, wxDefaultPosition, false );
@ -448,34 +449,83 @@ void SCH_EDIT_FRAME::copyBlockItems( PICKED_ITEMS_LIST& aItemsList )
void SCH_EDIT_FRAME::PasteListOfItems( wxDC* DC )
{
SCH_ITEM* Struct;
unsigned i;
SCH_ITEM* item;
SCH_SHEET_LIST hierarchy; // This is the entire schematic hierarcy.
if( m_blockItems.GetCount() == 0 )
{
DisplayError( this, wxT( "No struct to paste" ) );
DisplayError( this, _( "No item to paste." ) );
return;
}
wxFileName destFn = m_CurrentSheet->Last()->GetFileName();
if( destFn.IsRelative() )
destFn.MakeAbsolute( Prj().GetProjectPath() );
// Make sure any sheets in the block to be pasted will not cause recursion in
// the destination sheet.
for( i = 0; i < m_blockItems.GetCount(); i++ )
{
item = (SCH_ITEM*) m_blockItems.GetItem( i );
if( item->Type() == SCH_SHEET_T )
{
SCH_SHEET* sheet = (SCH_SHEET*)item;
wxFileName srcFn = sheet->GetFileName();
if( srcFn.IsRelative() )
srcFn.MakeAbsolute( Prj().GetProjectPath() );
SCH_SHEET_LIST sheetHierarchy( sheet );
if( hierarchy.TestForRecursion( sheetHierarchy,
destFn.GetFullPath( wxPATH_UNIX ) ) )
{
wxString msg;
msg.Printf( _( "The sheet changes cannot be made because the destination "
"sheet already has the sheet <%s> or one of it's subsheets "
"as a parent somewhere in the schematic hierarchy." ),
GetChars( sheet->GetFileName() ) );
DisplayError( this, msg );
return;
}
// Duplicate sheet names and sheet time stamps are not valid. Use a time stamp
// based sheet name and update the time stamp for each sheet in the block.
unsigned long timeStamp = (unsigned long)GetNewTimeStamp();
sheet->SetName( wxString::Format( wxT( "sheet%8.8lX" ), timeStamp ) );
sheet->SetTimeStamp( (time_t)timeStamp );
}
}
PICKED_ITEMS_LIST picklist;
for( unsigned ii = 0; ii < m_blockItems.GetCount(); ii++ )
for( i = 0; i < m_blockItems.GetCount(); i++ )
{
Struct = DuplicateStruct( (SCH_ITEM*) m_blockItems.GetItem( ii ) );
item = DuplicateStruct( (SCH_ITEM*) m_blockItems.GetItem( i ) );
// Creates data, and push it as new data in undo item list buffer
ITEM_PICKER picker( Struct, UR_NEW );
ITEM_PICKER picker( item, UR_NEW );
picklist.PushItem( picker );
// Clear annotation and init new time stamp for the new components:
if( Struct->Type() == SCH_COMPONENT_T )
// Clear annotation and init new time stamp for the new components and sheets:
if( item->Type() == SCH_COMPONENT_T )
{
( (SCH_COMPONENT*) Struct )->SetTimeStamp( GetNewTimeStamp() );
( (SCH_COMPONENT*) Struct )->ClearAnnotation( NULL );
( (SCH_COMPONENT*) item )->SetTimeStamp( GetNewTimeStamp() );
( (SCH_COMPONENT*) item )->ClearAnnotation( NULL );
}
else if( item->Type() == SCH_SHEET_T )
{
( (SCH_SHEET*) item )->SetTimeStamp( GetNewTimeStamp() );
}
SetSchItemParent( Struct, GetScreen() );
Struct->Draw( m_canvas, DC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
GetScreen()->Append( Struct );
SetSchItemParent( item, GetScreen() );
item->Draw( m_canvas, DC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
GetScreen()->Append( item );
}
SaveCopyInUndoList( picklist, UR_NEW );

View File

@ -45,6 +45,7 @@
#include <dialogs/dialog_schematic_find.h>
#include <boost/foreach.hpp>
#include <wx/filename.h>
SCH_SHEET_PATH::SCH_SHEET_PATH()
@ -61,7 +62,7 @@ bool SCH_SHEET_PATH::BuildSheetPathInfoFromSheetPathValue( const wxString& aPath
if( aFound )
return true;
if( GetSheetsCount() == 0 )
if( GetCount() == 0 )
Push( g_RootSheet );
if( aPath == Path() )
@ -69,7 +70,7 @@ bool SCH_SHEET_PATH::BuildSheetPathInfoFromSheetPathValue( const wxString& aPath
SCH_ITEM* schitem = LastDrawList();
while( schitem && GetSheetsCount() < NB_MAX_SHEET )
while( schitem && GetCount() < NB_MAX_SHEET )
{
if( schitem->Type() == SCH_SHEET_T )
{
@ -468,16 +469,101 @@ bool SCH_SHEET_PATH::operator==( const SCH_SHEET_PATH& d1 ) const
}
bool SCH_SHEET_PATH::TestForRecursion( const wxString& aSrcFileName,
const wxString& aDestFileName ) const
{
wxFileName rootFn = g_RootSheet->GetFileName();
wxFileName srcFn = aSrcFileName;
wxFileName destFn = aDestFileName;
if( srcFn.IsRelative() )
srcFn.MakeAbsolute( rootFn.GetPath() );
if( destFn.IsRelative() )
destFn.MakeAbsolute( rootFn.GetPath() );
// The source and destination sheet file names cannot be the same.
if( srcFn == destFn )
return true;
/// @todo Store sheet file names with full path, either relative to project path
/// or absolute path. The current design always assumes subsheet files are
/// located in the project folder which may or may not be desirable.
unsigned i = 0;
while( i < m_numSheets )
{
wxFileName cmpFn = m_sheets[i]->GetFileName();
if( cmpFn.IsRelative() )
cmpFn.MakeAbsolute( rootFn.GetPath() );
// Test if the file name of the destination sheet is in anywhere in this sheet path.
if( cmpFn == destFn )
break;
i++;
}
// The destination sheet file name was not found in the sheet path or the destination
// sheet file name is the root sheet so no recursion is possible.
if( i >= m_numSheets || i == 0 )
return false;
// Walk back up to the root sheet to see if the source file name is already a parent in
// the sheet path. If so, recursion will occur.
do
{
i -= 1;
wxFileName cmpFn = m_sheets[i]->GetFileName();
if( cmpFn.IsRelative() )
cmpFn.MakeAbsolute( rootFn.GetPath() );
if( cmpFn == srcFn )
return true;
} while( i != 0 );
// The source sheet file name is not a parent of the destination sheet file name.
return false;
}
int SCH_SHEET_PATH::FindSheet( const wxString& aFileName ) const
{
for( unsigned i = 0; i < m_numSheets; i++ )
{
if( m_sheets[i]->GetFileName().CmpNoCase( aFileName ) == 0 )
return (int)i;
}
return SHEET_NOT_FOUND;
}
SCH_SHEET* SCH_SHEET_PATH::FindSheetByName( const wxString& aSheetName )
{
for( unsigned i = 0; i < m_numSheets; i++ )
{
if( m_sheets[i]->GetName().CmpNoCase( aSheetName ) == 0 )
return m_sheets[i];
}
return NULL;
}
/********************************************************************/
/* Class SCH_SHEET_LIST to handle the list of Sheets in a hierarchy */
/********************************************************************/
SCH_SHEET_LIST::SCH_SHEET_LIST( SCH_SHEET* aSheet )
{
m_index = 0;
m_count = 0;
m_List = NULL;
m_list = NULL;
m_isRootSheet = false;
if( aSheet == NULL )
@ -492,7 +578,7 @@ SCH_SHEET_PATH* SCH_SHEET_LIST::GetFirst()
m_index = 0;
if( GetCount() > 0 )
return &( m_List[0] );
return &( m_list[0] );
return NULL;
}
@ -529,10 +615,10 @@ SCH_SHEET_PATH* SCH_SHEET_LIST::GetPrevious()
}
SCH_SHEET_PATH* SCH_SHEET_LIST::GetSheet( int aIndex )
SCH_SHEET_PATH* SCH_SHEET_LIST::GetSheet( int aIndex ) const
{
if( aIndex < GetCount() )
return &( m_List[aIndex] );
return &( m_list[aIndex] );
return NULL;
}
@ -564,18 +650,18 @@ void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet )
if( aSheet == g_RootSheet )
m_isRootSheet = true;
if( m_List == NULL )
if( m_list == NULL )
{
int count = aSheet->CountSheets();
m_count = count;
m_index = 0;
m_List = new SCH_SHEET_PATH[ count ];
m_list = new SCH_SHEET_PATH[ count ];
m_currList.Clear();
}
m_currList.Push( aSheet );
m_List[m_index] = m_currList;
m_list[m_index] = m_currList;
m_index++;
if( aSheet->GetScreen() )
@ -771,23 +857,70 @@ bool SCH_SHEET_LIST::SetComponentFootprint( const wxString& aReference,
}
bool SCH_SHEET_LIST::IsComplexHierarchy()
bool SCH_SHEET_LIST::IsComplexHierarchy() const
{
wxString fileName;
for( int i = 0; i < GetCount(); i++ )
for( int i = 0; i < m_count; i++ )
{
fileName = GetSheet( i )->Last()->GetFileName();
fileName = m_list[i].Last()->GetFileName();
for( int j = 0; j < GetCount(); j++ )
for( int j = 0; j < m_count; j++ )
{
if( i == j )
continue;
if( fileName == GetSheet( j )->Last()->GetFileName() )
if( fileName == m_list[j].Last()->GetFileName() )
return true;
}
}
return false;
}
bool SCH_SHEET_LIST::TestForRecursion( const SCH_SHEET_LIST& aSrcSheetHierarchy,
const wxString& aDestFileName ) const
{
wxFileName rootFn = g_RootSheet->GetFileName();
wxFileName destFn = aDestFileName;
if( destFn.IsRelative() )
destFn.MakeAbsolute( rootFn.GetPath() );
// Test each SCH_SHEET_PATH in this SCH_SHEET_LIST for potential recursion.
for( int i = 0; i < m_count; i++ )
{
// Test each SCH_SHEET_PATH in the source sheet.
for( int j = 0; j < aSrcSheetHierarchy.GetCount(); j++ )
{
SCH_SHEET_PATH* sheetPath = aSrcSheetHierarchy.GetSheet( j );
for( unsigned k = 0; k < sheetPath->GetCount(); k++ )
{
if( m_list[i].TestForRecursion( sheetPath->GetSheet( k )->GetFileName(),
aDestFileName ) )
return true;
}
}
}
// The source sheet file can safely be added to the destination sheet file.
return false;
}
SCH_SHEET* SCH_SHEET_LIST::FindSheetByName( const wxString& aSheetName )
{
SCH_SHEET* sheet = NULL;
for( int i = 0; i < m_count; i++ )
{
sheet = m_list[i].FindSheetByName( aSheetName );
if( sheet )
return sheet;
}
return NULL;
}

View File

@ -85,6 +85,9 @@ class SCH_ITEM;
class SCH_REFERENCE_LIST;
class PART_LIBS;
#define SHEET_NOT_FOUND -1
/**
* Type SCH_MULTI_UNIT_REFERENCE_MAP
* is used to create a map of reference designators for multi-unit parts.
@ -117,11 +120,19 @@ public:
m_numSheets = 0;
}
unsigned GetSheetsCount()
unsigned GetCount()
{
return m_numSheets;
}
SCH_SHEET* GetSheet( unsigned index )
{
if( index < m_numSheets )
return m_sheets[index];
return NULL;
}
/**
* Function Cmp
* Compare if this is the same sheet path as aSheetPathToTest
@ -283,6 +294,31 @@ public:
*/
SCH_ITEM* FindPreviousItem( KICAD_T aType, SCH_ITEM* aLastItem = NULL, bool aWrap = false ) const;
/**
* Function TestForRecursion
*
* test the SCH_SHEET_PATH file names to check adding the sheet stored in the file
* \a aSrcFileName to the sheet stored in file \a aDestFileName will cause a sheet
* path recursion.
*
* @param aSrcFileName is the source file name of the sheet add to \a aDestFileName.
* @param aDestFileName is the file name of the destination sheet for \a aSrcFileName.
* @return true if \a aFileName will cause recursion in the sheet path. Otherwise false.
*/
bool TestForRecursion( const wxString& aSrcFileName, const wxString& aDestFileName ) const;
int FindSheet( const wxString& aFileName ) const;
/**
* Function FindSheetByName
*
* searches the #SCH_SHEET_PATH for a sheet named \a aSheetName.
*
* @param aSheetName is the name of the sheet to find.
* @return a pointer to the sheet named \a aSheetName if found or NULL if not found.
*/
SCH_SHEET* FindSheetByName( const wxString& aSheetName );
SCH_SHEET_PATH& operator=( const SCH_SHEET_PATH& d1 );
bool operator==( const SCH_SHEET_PATH& d1 ) const;
@ -303,14 +339,14 @@ public:
class SCH_SHEET_LIST
{
private:
SCH_SHEET_PATH* m_List;
SCH_SHEET_PATH* m_list;
int m_count; /* Number of sheets included in hierarchy,
* starting at the given sheet in constructor .
* the given sheet is counted
*/
int m_index; /* internal variable to handle GetNext(): cleared by
* GetFirst() and incremented by GetNext() after
* returning the next item in m_List. Also used for
* returning the next item in m_list. Also used for
* internal calculations in BuildSheetList()
*/
bool m_isRootSheet;
@ -328,10 +364,10 @@ public:
~SCH_SHEET_LIST()
{
if( m_List )
delete[] m_List;
if( m_list )
delete[] m_list;
m_List = NULL;
m_list = NULL;
}
/**
@ -349,13 +385,13 @@ public:
/**
* Function GetFirst
* @return the first item (sheet) in m_List and prepare calls to GetNext()
* @return the first item (sheet) in m_list and prepare calls to GetNext()
*/
SCH_SHEET_PATH* GetFirst();
/**
* Function GetNext
* @return the next item (sheet) in m_List or NULL if no more item in
* @return the next item (sheet) in m_list or NULL if no more item in
* sheet list
*/
SCH_SHEET_PATH* GetNext();
@ -381,10 +417,10 @@ public:
* Function GetSheet
*
* @param aIndex A index in sheet list to get the sheet.
* @return the sheet at \a aIndex position in m_List or NULL if \a aIndex is
* @return the sheet at \a aIndex position in m_list or NULL if \a aIndex is
* outside the bounds of the index list.
*/
SCH_SHEET_PATH* GetSheet( int aIndex );
SCH_SHEET_PATH* GetSheet( int aIndex ) const;
/**
* Function GetSheet
@ -491,7 +527,30 @@ public:
*
* @return true if the #SCH_SHEET_LIST is a complex hierarchy.
*/
bool IsComplexHierarchy();
bool IsComplexHierarchy() const;
/**
* Function TestForRecursion
*
* test every SCH_SHEET_PATH in the SCH_SHEET_LIST to verify if adding the sheets stored
* in \a aSrcSheetHierarchy to the sheet stored in \a aDestFileName will cause recursion.
*
* @param aSrcSheetHierarchy is the SCH_SHEET_LIST of the source sheet add to \a aDestFileName.
* @param aDestFileName is the file name of the destination sheet for \a aSrcFileName.
* @return true if \a aFileName will cause recursion in the sheet path. Otherwise false.
*/
bool TestForRecursion( const SCH_SHEET_LIST& aSrcSheetHierarchy,
const wxString& aDestFileName ) const;
/**
* Function FindSheetByName
*
* searches the entire #SCH_SHEET_LIST for a sheet named \a aSheetName.
*
* @param aSheetName is the name of the sheet to find.
* @return a pointer to the sheet named \a aSheetName if found or NULL if not found.
*/
SCH_SHEET* FindSheetByName( const wxString& aSheetName );
private:

View File

@ -951,7 +951,7 @@ void SCH_EDIT_FRAME::OnEditItem( wxCommandEvent& aEvent )
}
case SCH_SHEET_T:
EditSheet( (SCH_SHEET*) item, &dc );
EditSheet( (SCH_SHEET*) item, m_CurrentSheet, &dc );
break;
case SCH_SHEET_PIN_T:

View File

@ -1202,7 +1202,7 @@ void SCH_EDIT_FRAME::addCurrentItemToList( wxDC* aDC )
// the m_mouseCaptureCallback function.
m_canvas->SetMouseCapture( NULL, NULL );
if( !EditSheet( (SCH_SHEET*)item, aDC ) )
if( !EditSheet( (SCH_SHEET*)item, m_CurrentSheet, aDC ) )
{
screen->SetCurItem( NULL );
item->Draw( m_canvas, aDC, wxPoint( 0, 0 ), g_XorMode );

View File

@ -994,7 +994,7 @@ public:
* the current associated screen file name is changed and saved to disk.</li>
* </ul> </p>
*/
bool EditSheet( SCH_SHEET* aSheet, wxDC* aDC );
bool EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy, wxDC* aDC );
wxPoint GetLastSheetPinPosition() const { return m_lastSheetPinPosition; }

View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2015 Jean-Pierre Charras, jp.charras at wanadoo.fr
* Copyright (C) 2004-2011 KiCad Developers, see change_log.txt for contributors.
* Copyright (C) 2004-2015 KiCad Developers, see change_log.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
@ -31,19 +31,23 @@
#include <confirm.h>
#include <schframe.h>
#include <base_units.h>
#include <kiface_i.h>
#include <sch_sheet.h>
#include <sch_sheet_path.h>
#include <dialogs/dialog_sch_sheet_props.h>
#include <wildcards_and_files_ext.h>
#include <project.h>
bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy, wxDC* aDC )
{
if( aSheet == NULL )
if( aSheet == NULL || aHierarchy == NULL )
return false;
SCH_SHEET_LIST hierarchy; // This is the schematic sheet hierarchy.
// Get the new texts
DIALOG_SCH_SHEET_PROPS dlg( this );
@ -86,9 +90,9 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
}
// Duplicate sheet names are not valid.
const SCH_SHEET* sheet = GetScreen()->GetSheet( dlg.GetSheetName() );
const SCH_SHEET* sheet = hierarchy.FindSheetByName( dlg.GetSheetName() );
if( sheet && sheet != aSheet )
if( sheet && (sheet != aSheet) )
{
DisplayError( this, wxString::Format( _( "A sheet named \"%s\" already exists." ),
GetChars( dlg.GetSheetName() ) ) );
@ -119,24 +123,23 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
}
// Inside Eeschema, filenames are stored using unix notation
newFilename.Replace( wxT("\\"), wxT("/") );
newFilename.Replace( wxT( "\\" ), wxT( "/" ) );
if( aSheet->GetScreen() == NULL ) // New sheet.
if( aSheet->GetScreen() == NULL ) // New sheet.
{
if( useScreen || loadFromFile ) // Load from existing file.
{
if( useScreen != NULL )
{
msg.Printf( _( "A file named '%s' already exists in the current schematic hierarchy." ),
GetChars( newFilename ) );
msg.Printf( _( "A file named '%s' already exists in the current schematic "
"hierarchy." ), GetChars( newFilename ) );
}
else
{
msg.Printf( _( "A file named '%s' already exists." ),
GetChars( newFilename ) );
msg.Printf( _( "A file named '%s' already exists." ), GetChars( newFilename ) );
}
msg += _("\n\nDo you want to create a sheet with the contents of this file?" );
msg += _( "\n\nDo you want to create a sheet with the contents of this file?" );
if( !IsOK( this, msg ) )
{
@ -167,13 +170,14 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
isUndoable = false;
msg = _( "Changing the sheet file name cannot be undone. " );
if( useScreen || loadFromFile ) // Load from existing file.
if( useScreen || loadFromFile ) // Load from existing file.
{
wxString tmp;
if( useScreen != NULL )
{
tmp.Printf( _( "A file named <%s> already exists in the current schematic hierarchy." ),
GetChars( newFilename ) );
tmp.Printf( _( "A file named <%s> already exists in the current schematic "
"hierarchy." ), GetChars( newFilename ) );
}
else
{
@ -182,7 +186,7 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
}
msg += tmp;
msg += _("\n\nDo you want to replace the sheet with the contents of this file?" );
msg += _( "\n\nDo you want to replace the sheet with the contents of this file?" );
if( !IsOK( this, msg ) )
return false;
@ -242,6 +246,25 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
aSheet->SetName( wxString::Format( wxT( "Sheet%8.8lX" ),
(long unsigned) aSheet->GetTimeStamp() ) );
// Make sure the sheet changes do not cause any recursion.
SCH_SHEET_LIST sheetHierarchy( aSheet );
// Make sure files have fully qualified path and file name.
wxFileName destFn = aHierarchy->Last()->GetFileName();
if( destFn.IsRelative() )
destFn.MakeAbsolute( Prj().GetProjectPath() );
if( hierarchy.TestForRecursion( sheetHierarchy, destFn.GetFullPath( wxPATH_UNIX ) ) )
{
msg.Printf( _( "The sheet changes cannot be made because the destination sheet already "
"has the sheet <%s> or one of it's subsheets as a parent somewhere in "
"the schematic hierarchy." ),
GetChars( newFilename ) );
DisplayError( this, msg );
return false;
}
m_canvas->MoveCursorToCrossHair();
m_canvas->SetIgnoreMouseEvents( false );
aSheet->Draw( m_canvas, aDC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
@ -257,7 +280,7 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
* But the (very small code) relative to sheet move is still present here
*/
static void resizeSheetWithMouseCursor( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& aPosition,
bool aErase )
bool aErase )
{
wxPoint moveVector;
BASE_SCREEN* screen = aPanel->GetScreen();
@ -392,7 +415,7 @@ void SCH_EDIT_FRAME::ReSizeSheet( SCH_SHEET* aSheet, wxDC* aDC )
SetUndoItem( aSheet );
}
#define GRID_SIZE_REF 50
void SCH_EDIT_FRAME::RotateHierarchicalSheet( SCH_SHEET* aSheet, bool aRotCCW )
{
if( aSheet == NULL )