haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Apply symbol field id fixup logic to lib parts. 

Fixes https://gitlab.com/kicad/code/kicad/issues/7903
This commit is contained in:
Jeff Young 2021-03-26 16:48:55 +00:00
parent 23667fc4c4
commit 3d6c05b210
3 changed files with 91 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp
View File

@ -127,7 +127,9 @@ LIB_PART* SCH_SEXPR_PARSER::ParseSymbol( LIB_PART_MAP& aSymbolLibMap, int aFileV
wxString name;
wxString error;
LIB_ITEM* item;
LIB_FIELD* field;
std::unique_ptr<LIB_PART> symbol = std::make_unique<LIB_PART>( wxEmptyString );
std::set<int> fieldIDsRead;
m_requiredVersion = aFileVersion;
symbol->SetUnitCount( 1 );
@ -197,7 +199,23 @@ LIB_PART* SCH_SEXPR_PARSER::ParseSymbol( LIB_PART_MAP& aSymbolLibMap, int aFileV
break;
case T_property:
parseProperty( symbol );
field = parseProperty( symbol );
if( field )
{
// It would appear that at some point we allowed duplicate ids to slip through
// when writing files. The easiest (and most complete) solution is to disallow
// multiple instances of the same id (for all files since the source of the error
// *might* in fact be hand-edited files).
//
// While no longer used, -1 is still a valid id for user field. It gets converted
// to the next unused number on save.
if( fieldIDsRead.count( field->GetId() ) )
field->SetId( -1 );
else if( field )
fieldIDsRead.insert( field->GetId() );
}
break;
case T_extends:
@ -609,28 +627,12 @@ void SCH_SEXPR_PARSER::parseEDA_TEXT( EDA_TEXT* aText )
{
switch( token )
{
case T_left:
aText->SetHorizJustify( GR_TEXT_HJUSTIFY_LEFT );
break;
case T_right:
aText->SetHorizJustify( GR_TEXT_HJUSTIFY_RIGHT );
break;
case T_top:
aText->SetVertJustify( GR_TEXT_VJUSTIFY_TOP );
break;
case T_bottom:
aText->SetVertJustify( GR_TEXT_VJUSTIFY_BOTTOM );
break;
case T_mirror:
aText->SetMirrored( true );
break;
default:
Expecting( "left, right, top, bottom, or mirror" );
case T_left: aText->SetHorizJustify( GR_TEXT_HJUSTIFY_LEFT ); break;
case T_right: aText->SetHorizJustify( GR_TEXT_HJUSTIFY_RIGHT ); break;
case T_top: aText->SetVertJustify( GR_TEXT_VJUSTIFY_TOP ); break;
case T_bottom: aText->SetVertJustify( GR_TEXT_VJUSTIFY_BOTTOM ); break;
case T_mirror: aText->SetMirrored( true ); break;
default: Expecting( "left, right, top, bottom, or mirror" );
}
}
@ -712,20 +714,20 @@ void SCH_SEXPR_PARSER::parsePinNames( std::unique_ptr<LIB_PART>& aSymbol )
}
else if( token != T_RIGHT )
{
error.Printf(
_( "Invalid symbol names definition in\nfile: \"%s\"\nline: %d\noffset: %d" ),
CurSource().c_str(), CurLineNumber(), CurOffset() );
error.Printf( _( "Invalid symbol names definition in\nfile: '%s'\nline: %d\noffset: %d" ),
CurSource().c_str(),
CurLineNumber(),
CurOffset() );
THROW_IO_ERROR( error );
}
}
void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
LIB_FIELD* SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
{
wxCHECK_RET( CurTok() == T_property,
wxT( "Cannot parse " ) + GetTokenString( CurTok() ) +
wxT( " as a property token." ) );
wxCHECK( aSymbol, /* void */ );
wxCHECK_MSG( CurTok() == T_property, nullptr,
wxT( "Cannot parse " ) + GetTokenString( CurTok() ) + wxT( " as a property." ) );
wxCHECK( aSymbol, nullptr );
wxString error;
wxString name;
@ -736,8 +738,10 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
if( !IsSymbol( token ) )
{
error.Printf( _( "Invalid property name in\nfile: \"%s\"\nline: %d\noffset: %d" ),
CurSource().c_str(), CurLineNumber(), CurOffset() );
error.Printf( _( "Invalid property name in\nfile: '%s'\nline: %d\noffset: %d" ),
CurSource().c_str(),
CurLineNumber(),
CurOffset() );
THROW_IO_ERROR( error );
}
@ -745,8 +749,10 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
if( name.IsEmpty() )
{
error.Printf( _( "Empty property name in\nfile: \"%s\"\nline: %d\noffset: %d" ),
CurSource().c_str(), CurLineNumber(), CurOffset() );
error.Printf( _( "Empty property name in\nfile: '%s'\nline: %d\noffset: %d" ),
CurSource().c_str(),
CurLineNumber(),
CurOffset() );
THROW_IO_ERROR( error );
}
@ -755,8 +761,10 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
if( !IsSymbol( token ) )
{
error.Printf( _( "Invalid property value in\nfile: \"%s\"\nline: %d\noffset: %d" ),
CurSource().c_str(), CurLineNumber(), CurOffset() );
error.Printf( _( "Invalid property value in\nfile: '%s'\nline: %d\noffset: %d" ),
CurSource().c_str(),
CurLineNumber(),
CurOffset() );
THROW_IO_ERROR( error );
}
@ -801,16 +809,19 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
existingField = aSymbol->GetFieldById( field->GetId() );
*existingField = *field;
return existingField;
}
else if( name == "ki_keywords" )
{
// Not a LIB_FIELD object yet.
aSymbol->SetKeyWords( value );
return nullptr;
}
else if( name == "ki_description" )
{
// Not a LIB_FIELD object yet.
aSymbol->SetDescription( value );
return nullptr;
}
else if( name == "ki_fp_filters" )
{
@ -822,24 +833,28 @@ void SCH_SEXPR_PARSER::parseProperty( std::unique_ptr<LIB_PART>& aSymbol )
filters.Add( tokenizer.GetNextToken() );
aSymbol->SetFPFilters( filters );
return nullptr;
}
else if( name == "ki_locked" )
{
// This is a temporary LIB_FIELD object until interchangeable units are determined on
// the fly.
aSymbol->LockUnits( true );
return nullptr;
}
else
{
existingField = aSymbol->GetFieldById( field->GetId() );
existingField = aSymbol->FindField( field->GetCanonicalName() );
if( !existingField )
{
aSymbol->AddDrawItem( field.release(), false );
aSymbol->AddDrawItem( field.get(), false );
return field.release();
}
else
{
*existingField = *field;
return existingField;
}
}
}
@ -2280,8 +2295,8 @@ SCH_COMPONENT* SCH_SEXPR_PARSER::parseSchematicSymbol()
// multiple instances of the same id (for all files since the source of the error
// *might* in fact be hand-edited files).
//
// While no longer used, -1 is still a valid id for user fields and will
// get written out as the next unused number on save.
// While no longer used, -1 is still a valid id for user field. It gets converted
// to the next unused number on save.
if( fieldIDsRead.count( field->GetId() ) )
field->SetId( -1 );
else
@ -2533,8 +2548,8 @@ SCH_SHEET* SCH_SEXPR_PARSER::parseSheet()
// complete) solution is to disallow multiple instances of the same id (for all
// files since the source of the error *might* in fact be hand-edited files).
//
// While no longer used, -1 is still a valid id for user fields and will
// get written out as the next unused number on save.
// While no longer used, -1 is still a valid id for user field. It gets converted
// to the next unused number on save.
if( fieldIDsRead.count( field->GetId() ) )
field->SetId( -1 );
else

2
eeschema/sch_plugins/kicad/sch_sexpr_parser.h
View File

@ -173,7 +173,7 @@ class SCH_SEXPR_PARSER : public SCHEMATIC_LEXER
void parseEDA_TEXT( EDA_TEXT* aText );
void parsePinNames( std::unique_ptr<LIB_PART>& aSymbol );
void parseProperty( std::unique_ptr<LIB_PART>& aSymbol );
LIB_FIELD* parseProperty( std::unique_ptr<LIB_PART>& aSymbol );
LIB_ARC* parseArc();
LIB_BEZIER* parseBezier();

66
eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp
View File

@ -297,8 +297,7 @@ class SCH_SEXPR_PLUGIN_CACHE
bool m_isWritable;
bool m_isModified;
int m_versionMajor;
int m_versionMinor;
SCH_LIB_TYPE m_libType; // Is this cache a component or symbol library.
SCH_LIB_TYPE m_libType; // Is this cache a component or symbol library.
LIB_PART* removeSymbol( LIB_PART* aAlias );
@ -309,8 +308,8 @@ class SCH_SEXPR_PLUGIN_CACHE
int aNestLevel = 0 );
static void saveCircle( LIB_CIRCLE* aCircle, OUTPUTFORMATTER& aFormatter,
int aNestLevel = 0 );
static void saveField( const LIB_FIELD* aField, OUTPUTFORMATTER& aFormatter,
int aNestLevel = 0 );
static void saveField( LIB_FIELD* aField, OUTPUTFORMATTER& aFormatter,
int& aNextFreeFieldId, int aNestLevel );
static void savePin( LIB_PIN* aPin, OUTPUTFORMATTER& aFormatter, int aNestLevel = 0 );
static void savePolyLine( LIB_POLYLINE* aPolyLine, OUTPUTFORMATTER& aFormatter,
int aNestLevel = 0 );
@ -319,7 +318,7 @@ class SCH_SEXPR_PLUGIN_CACHE
static void saveText( LIB_TEXT* aText, OUTPUTFORMATTER& aFormatter, int aNestLevel = 0 );
static void saveDcmInfoAsFields( LIB_PART* aSymbol, OUTPUTFORMATTER& aFormatter,
int aNestLevel = 0, int aFirstId = MANDATORY_FIELDS );
int& aNextFreeFieldId, int aNestLevel );
friend SCH_SEXPR_PLUGIN;
@ -1317,7 +1316,6 @@ SCH_SEXPR_PLUGIN_CACHE::SCH_SEXPR_PLUGIN_CACHE( const wxString& aFullPathAndFile
m_isModified( false )
{
m_versionMajor = -1;
m_versionMinor = -1;
m_libType = SCH_LIB_TYPE::LT_EESCHEMA;
}
@ -1556,7 +1554,7 @@ void SCH_SEXPR_PLUGIN_CACHE::SaveSymbol( LIB_PART* aSymbol, OUTPUTFORMATTER& aFo
wxCHECK2( wxLocale::GetInfo( wxLOCALE_DECIMAL_POINT, wxLOCALE_CAT_NUMBER ) == ".",
LOCALE_IO toggle );
int lastFieldId;
int nextFreeFieldId = MANDATORY_FIELDS;
std::vector<LIB_FIELD*> fields;
std::string name = aFormatter.Quotew( aSymbol->GetLibId().Format().wx_str() );
std::string unitName = aSymbol->GetLibId().GetLibItemName();
@ -1612,22 +1610,19 @@ void SCH_SEXPR_PLUGIN_CACHE::SaveSymbol( LIB_PART* aSymbol, OUTPUTFORMATTER& aFo
aSymbol->GetFields( fields );
for( const LIB_FIELD* field : fields )
saveField( field, aFormatter, aNestLevel + 1 );
lastFieldId = fields.back()->GetId() + 1;
for( LIB_FIELD* field : fields )
saveField( field, aFormatter, nextFreeFieldId, aNestLevel + 1 );
// @todo At some point in the future the lock status (all units interchangeable) should
// be set deterministically. For now a custom lock propertery is used to preserve the
// locked flag state.
if( aSymbol->UnitsLocked() )
{
LIB_FIELD locked( lastFieldId, "ki_locked" );
saveField( &locked, aFormatter, aNestLevel + 1 );
lastFieldId += 1;
LIB_FIELD locked( -1, "ki_locked" );
saveField( &locked, aFormatter, nextFreeFieldId, aNestLevel + 1 );
}
saveDcmInfoAsFields( aSymbol, aFormatter, aNestLevel, lastFieldId );
saveDcmInfoAsFields( aSymbol, aFormatter, nextFreeFieldId, aNestLevel );
// Save the draw items grouped by units.
std::vector<PART_UNITS> units = aSymbol->GetUnitDrawItems();
@ -1667,12 +1662,10 @@ void SCH_SEXPR_PLUGIN_CACHE::SaveSymbol( LIB_PART* aSymbol, OUTPUTFORMATTER& aFo
aSymbol->GetFields( fields );
for( const LIB_FIELD* field : fields )
saveField( field, aFormatter, aNestLevel + 1 );
for( LIB_FIELD* field : fields )
saveField( field, aFormatter, nextFreeFieldId, aNestLevel + 1 );
lastFieldId = fields.back()->GetId() + 1;
saveDcmInfoAsFields( aSymbol, aFormatter, aNestLevel, lastFieldId );
saveDcmInfoAsFields( aSymbol, aFormatter, nextFreeFieldId, aNestLevel );
}
aFormatter.Print( aNestLevel, ")\n" );
@ -1680,28 +1673,24 @@ void SCH_SEXPR_PLUGIN_CACHE::SaveSymbol( LIB_PART* aSymbol, OUTPUTFORMATTER& aFo
void SCH_SEXPR_PLUGIN_CACHE::saveDcmInfoAsFields( LIB_PART* aSymbol, OUTPUTFORMATTER& aFormatter,
int aNestLevel, int aFirstId )
int& aNextFreeFieldId, int aNestLevel )
{
wxCHECK_RET( aSymbol, "Invalid LIB_PART pointer." );
int id = aFirstId;
if( !aSymbol->GetKeyWords().IsEmpty() )
{
LIB_FIELD keywords( id, wxString( "ki_keywords" ) );
LIB_FIELD keywords( -1, wxString( "ki_keywords" ) );
keywords.SetVisible( false );
keywords.SetText( aSymbol->GetKeyWords() );
saveField( &keywords, aFormatter, aNestLevel + 1 );
id += 1;
saveField( &keywords, aFormatter, aNextFreeFieldId, aNestLevel + 1 );
}
if( !aSymbol->GetDescription().IsEmpty() )
{
LIB_FIELD description( id, wxString( "ki_description" ) );
LIB_FIELD description( -1, wxString( "ki_description" ) );
description.SetVisible( false );
description.SetText( aSymbol->GetDescription() );
saveField( &description, aFormatter, aNestLevel + 1 );
id += 1;
saveField( &description, aFormatter, aNextFreeFieldId, aNestLevel + 1 );
}
wxArrayString fpFilters = aSymbol->GetFPFilters();
@ -1718,11 +1707,10 @@ void SCH_SEXPR_PLUGIN_CACHE::saveDcmInfoAsFields( LIB_PART* aSymbol, OUTPUTFORMA
tmp += " " + filter;
}
LIB_FIELD description( id, wxString( "ki_fp_filters" ) );
LIB_FIELD description( -1, wxString( "ki_fp_filters" ) );
description.SetVisible( false );
description.SetText( tmp );
saveField( &description, aFormatter, aNestLevel + 1 );
id += 1;
saveField( &description, aFormatter, aNextFreeFieldId, aNestLevel + 1 );
}
}
@ -1874,13 +1862,23 @@ void SCH_SEXPR_PLUGIN_CACHE::saveCircle( LIB_CIRCLE* aCircle,
}
void SCH_SEXPR_PLUGIN_CACHE::saveField( const LIB_FIELD* aField, OUTPUTFORMATTER& aFormatter,
int aNestLevel )
void SCH_SEXPR_PLUGIN_CACHE::saveField( LIB_FIELD* aField, OUTPUTFORMATTER& aFormatter,
int& aNextFreeFieldId, int aNestLevel )
{
wxCHECK_RET( aField && aField->Type() == LIB_FIELD_T, "Invalid LIB_FIELD object." );
wxString fieldName = aField->GetName();
if( aField->GetId() == -1 /* undefined ID */ )
{
aField->SetId( aNextFreeFieldId );
aNextFreeFieldId += 1;
}
else if( aField->GetId() >= aNextFreeFieldId )
{
aNextFreeFieldId = aField->GetId() + 1;
}
if( aField->GetId() >= 0 && aField->GetId() < MANDATORY_FIELDS )
fieldName = TEMPLATE_FIELDNAME::GetDefaultFieldName( aField->GetId(), false );