From dea9dd2361bf82eb266030434eee1473ea0591fa Mon Sep 17 00:00:00 2001 From: Dick Hollenbeck Date: Thu, 13 Jun 2013 11:09:35 -0500 Subject: [PATCH] better error reporting and parsing of bad legacy footprint libs --- eeschema/dialogs/dialog_annotate.cpp | 2 +- include/reporter.h | 4 +- kicad/dialogs/dialog_template_selector.h | 16 ++-- kicad/project_template.h | 2 +- pcbnew/legacy_plugin.cpp | 117 ++++++++++++++--------- pcbnew/legacy_plugin.h | 2 +- 6 files changed, 89 insertions(+), 54 deletions(-) diff --git a/eeschema/dialogs/dialog_annotate.cpp b/eeschema/dialogs/dialog_annotate.cpp index 6c0c2eea71..c37d8a1e38 100644 --- a/eeschema/dialogs/dialog_annotate.cpp +++ b/eeschema/dialogs/dialog_annotate.cpp @@ -1,5 +1,5 @@ /** - * @file annotate_dialog.cpp + * @file dialog_annotate.cpp * @brief Annotation dialog functions. */ diff --git a/include/reporter.h b/include/reporter.h index efda4c6368..5c88de6449 100644 --- a/include/reporter.h +++ b/include/reporter.h @@ -44,8 +44,8 @@ class wxTextCtrl; * The purpose of the REPORTER object is to offer a way for a procedural function * to report multiple errors without having to: * */ class REPORTER diff --git a/kicad/dialogs/dialog_template_selector.h b/kicad/dialogs/dialog_template_selector.h index 41006aab25..f4ea0f3229 100644 --- a/kicad/dialogs/dialog_template_selector.h +++ b/kicad/dialogs/dialog_template_selector.h @@ -25,17 +25,18 @@ #ifndef PROJECT_TEMPLATE_SELECTOR_H #define PROJECT_TEMPLATE_SELECTOR_H -#include "dialogs/dialog_template_selector_base.h" +#include #include "project_template.h" + class TEMPLATE_WIDGET : public TEMPLATE_WIDGET_BASE { protected: - wxDialog* dialog; - wxWindow* parent; - wxPanel* panel; + wxDialog* dialog; + wxWindow* parent; + wxPanel* panel; + bool selected; - bool selected; PROJECT_TEMPLATE* templ; void OnKillFocus( wxFocusEvent& event ); @@ -56,6 +57,7 @@ public: bool IsSelected(); }; + class TEMPLATE_SELECTION_PANEL : public TEMPLATE_SELECTION_PANEL_BASE { protected: @@ -65,6 +67,7 @@ protected: public: /** * @param aParent The window creating the dialog + * @param aPath the path */ TEMPLATE_SELECTION_PANEL( wxWindow* aParent, const wxString& aPath ); ~TEMPLATE_SELECTION_PANEL(); @@ -72,6 +75,7 @@ public: const wxString& GetPath() { return m_templatesPath; } }; + class DIALOG_TEMPLATE_SELECTOR : public DIALOG_TEMPLATE_SELECTOR_BASE { protected: @@ -92,7 +96,7 @@ public: TEMPLATE_WIDGET* GetWidget(); void SetWidget( TEMPLATE_WIDGET* aWidget ); void onNotebookResize( wxSizeEvent& event ); - void OnPageChange( wxNotebookEvent& event ); + void OnPageChange( wxNotebookEvent& event ); }; #endif diff --git a/kicad/project_template.h b/kicad/project_template.h index 0014703ba9..98234755a9 100644 --- a/kicad/project_template.h +++ b/kicad/project_template.h @@ -59,7 +59,7 @@ ~~~~~~~~~~~~~~ /info.html - Contains html formatted information about the template which is used by the - user to determine if the template is what they are after. The tag + user to determine if the template is what they are after. The <title> tag determines the actual name of the template that is exposed to the user for template selection. Using html to format this document means that images can be in-lined without having to invent a new scheme. Only HTML supported by diff --git a/pcbnew/legacy_plugin.cpp b/pcbnew/legacy_plugin.cpp index dc7c46bb9a..ababcf2649 100644 --- a/pcbnew/legacy_plugin.cpp +++ b/pcbnew/legacy_plugin.cpp @@ -276,8 +276,9 @@ void LEGACY_PLUGIN::loadAllSections( bool doAppend ) if( TESTLINE( "$MODULE" ) ) { - MODULE* m = LoadMODULE(); - m_board->Add( m, ADD_APPEND ); + auto_ptr<MODULE> module( new MODULE( m_board ) ); + LoadMODULE( module.get() ); + m_board->Add( module.release(), ADD_APPEND ); } else if( TESTLINE( "$DRAWSEGMENT" ) ) @@ -918,9 +919,8 @@ void LEGACY_PLUGIN::loadSETUP() } -MODULE* LEGACY_PLUGIN::LoadMODULE() +void LEGACY_PLUGIN::LoadMODULE( MODULE* aModule ) { - auto_ptr<MODULE> module( new MODULE( m_board ) ); char* line; while( ( line = READLINE( m_reader ) ) != NULL ) @@ -929,14 +929,14 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() // most frequently encountered ones at the top - if( TESTSUBSTR( "D" ) ) // read a drawing item, e.g. "DS" + if( TESTSUBSTR( "D" ) && strchr( "SCAP", line[1] ) ) // read a drawing item, e.g. "DS" { - loadMODULE_EDGE( module.get() ); + loadMODULE_EDGE( aModule ); } else if( TESTLINE( "$PAD" ) ) { - loadPAD( module.get() ); + loadPAD( aModule ); } // Read a footprint text description (ref, value, or drawing) @@ -951,17 +951,17 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() switch( tnum ) { case TEXTE_MODULE::TEXT_is_REFERENCE: - textm = &module->Reference(); + textm = &aModule->Reference(); break; case TEXTE_MODULE::TEXT_is_VALUE: - textm = &module->Value(); + textm = &aModule->Value(); break; default: // text is a drawing - textm = new TEXTE_MODULE( module.get() ); - module->GraphicalItems().PushBack( textm ); + textm = new TEXTE_MODULE( aModule ); + aModule->GraphicalItems().PushBack( textm ); } loadMODULE_TEXT( textm ); @@ -986,29 +986,29 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() // data is now a two character long string // Note: some old files do not have this field if( data && data[0] == 'F' ) - module->SetLocked( true ); + aModule->SetLocked( true ); if( data && data[1] == 'P' ) - module->SetIsPlaced( true ); + aModule->SetIsPlaced( true ); - module->SetPosition( wxPoint( pos_x, pos_y ) ); - module->SetLayer( layer ); - module->SetOrientation( orient ); - module->SetTimeStamp( timestamp ); - module->SetLastEditTime( edittime ); + aModule->SetPosition( wxPoint( pos_x, pos_y ) ); + aModule->SetLayer( layer ); + aModule->SetOrientation( orient ); + aModule->SetTimeStamp( timestamp ); + aModule->SetLastEditTime( edittime ); } else if( TESTLINE( "Li" ) ) // Library name of footprint { // There can be whitespace in the footprint name on some old libraries. // Grab everything after "Li" up to end of line: - module->SetLibRef( FROM_UTF8( StrPurge( line + SZ( "Li" ) ) ) ); + // aModule->SetLibRef( FROM_UTF8( StrPurge( line + SZ( "Li" ) ) ) ); } else if( TESTLINE( "Sc" ) ) // timestamp { time_t timestamp = hexParse( line + SZ( "Sc" ) ); - module->SetTimeStamp( timestamp ); + aModule->SetTimeStamp( timestamp ); } else if( TESTLINE( "Op" ) ) // (Op)tions for auto placement @@ -1020,7 +1020,7 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() if( cntRot180 > 10 ) cntRot180 = 10; - module->SetPlacementCost180( cntRot180 ); + aModule->SetPlacementCost180( cntRot180 ); int cntRot90 = itmp1 & 0x0F; if( cntRot90 > 10 ) @@ -1030,7 +1030,7 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() if( itmp1 > 10 ) itmp1 = 0; - module->SetPlacementCost90( (itmp1 << 4) | cntRot90 ); + aModule->SetPlacementCost90( (itmp1 << 4) | cntRot90 ); } else if( TESTLINE( "At" ) ) // (At)tributes of module @@ -1045,30 +1045,31 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() if( strstr( data, "VIRTUAL" ) ) attrs |= MOD_VIRTUAL; - module->SetAttributes( attrs ); + aModule->SetAttributes( attrs ); } else if( TESTLINE( "AR" ) ) // Alternate Reference { // e.g. "AR /47BA2624/45525076" data = strtok( line + SZ( "AR" ), delims ); - module->SetPath( FROM_UTF8( data ) ); + if( data ) + aModule->SetPath( FROM_UTF8( data ) ); } else if( TESTLINE( "$SHAPE3D" ) ) { - load3D( module.get() ); + load3D( aModule ); } else if( TESTLINE( "Cd" ) ) { // e.g. "Cd Double rangee de contacts 2 x 4 pins\r\n" - module->SetDescription( FROM_UTF8( StrPurge( line + SZ( "Cd" ) ) ) ); + aModule->SetDescription( FROM_UTF8( StrPurge( line + SZ( "Cd" ) ) ) ); } else if( TESTLINE( "Kw" ) ) // Key words { - module->SetKeywords( FROM_UTF8( StrPurge( line + SZ( "Kw" ) ) ) ); + aModule->SetKeywords( FROM_UTF8( StrPurge( line + SZ( "Kw" ) ) ) ); } else if( TESTLINE( ".SolderPasteRatio" ) ) @@ -1082,54 +1083,58 @@ MODULE* LEGACY_PLUGIN::LoadMODULE() tmp = -0.50; if( tmp > 0.0 ) tmp = 0.0; - module->SetLocalSolderPasteMarginRatio( tmp ); + aModule->SetLocalSolderPasteMarginRatio( tmp ); } else if( TESTLINE( ".SolderPaste" ) ) { BIU tmp = biuParse( line + SZ( ".SolderPaste" ) ); - module->SetLocalSolderPasteMargin( tmp ); + aModule->SetLocalSolderPasteMargin( tmp ); } else if( TESTLINE( ".SolderMask" ) ) { BIU tmp = biuParse( line + SZ( ".SolderMask" ) ); - module->SetLocalSolderMaskMargin( tmp ); + aModule->SetLocalSolderMaskMargin( tmp ); } else if( TESTLINE( ".LocalClearance" ) ) { BIU tmp = biuParse( line + SZ( ".LocalClearance" ) ); - module->SetLocalClearance( tmp ); + aModule->SetLocalClearance( tmp ); } else if( TESTLINE( ".ZoneConnection" ) ) { int tmp = intParse( line + SZ( ".ZoneConnection" ) ); - module->SetZoneConnection( (ZoneConnection)tmp ); + aModule->SetZoneConnection( (ZoneConnection)tmp ); } else if( TESTLINE( ".ThermalWidth" ) ) { BIU tmp = biuParse( line + SZ( ".ThermalWidth" ) ); - module->SetThermalWidth( tmp ); + aModule->SetThermalWidth( tmp ); } else if( TESTLINE( ".ThermalGap" ) ) { BIU tmp = biuParse( line + SZ( ".ThermalGap" ) ); - module->SetThermalGap( tmp ); + aModule->SetThermalGap( tmp ); } else if( TESTLINE( "$EndMODULE" ) ) { - module->CalculateBoundingBox(); + aModule->CalculateBoundingBox(); - return module.release(); // preferred exit + return; // preferred exit } } - THROW_IO_ERROR( "Missing '$EndMODULE'" ); + wxString msg = wxString::Format( + wxT( "Missing '$EndMODULE' for MODULE '%s'" ), + GetChars( aModule->GetLibRef() ) ); + + THROW_IO_ERROR( msg ); } @@ -1175,8 +1180,12 @@ void LEGACY_PLUGIN::loadPAD( MODULE* aModule ) case 'O': padshape = PAD_OVAL; break; case 'T': padshape = PAD_TRAPEZOID; break; default: - m_error.Printf( _( "Unknown padshape '%s' on line:%d" ), - FROM_UTF8( line ).GetData(), m_reader->LineNumber() ); + m_error.Printf( _( "Unknown padshape '%c=0x%02x' on line:%d of module:'%s'" ), + (unsigned char) padshape, + (unsigned char) padshape, + m_reader->LineNumber(), + GetChars( aModule->GetLibRef() ) + ); THROW_IO_ERROR( m_error ); } @@ -1372,8 +1381,12 @@ void LEGACY_PLUGIN::loadMODULE_EDGE( MODULE* aModule ) case 'A': shape = S_ARC; break; case 'P': shape = S_POLYGON; break; default: - m_error.Printf( wxT( "Unknown EDGE_MODULE type '%s' line %d" ), - FROM_UTF8( line ).GetData(), m_reader->LineNumber() ); + m_error.Printf( wxT( "Unknown EDGE_MODULE type:'%c=0x%02x' on line:%d of module:'%s'" ), + (unsigned char) line[1], + (unsigned char) line[1], + m_reader->LineNumber(), + GetChars( aModule->GetLibRef() ) + ); THROW_IO_ERROR( m_error ); } @@ -4036,9 +4049,27 @@ void FPL_CACHE::LoadModules( LINE_READER* aReader ) // test first for the $MODULE, even before reading because of INDEX bug. if( TESTLINE( "$MODULE" ) ) { - MODULE* m = m_owner->LoadMODULE(); + auto_ptr<MODULE> module( new MODULE( m_owner->m_board ) ); - std::string footprintName = TO_UTF8( m->GetLibRef() ); + std::string footprintName = StrPurge( line + SZ( "$MODULE" ) ); + + // set the footprint name first thing, so exceptions can use name. + module->SetLibRef( FROM_UTF8( footprintName.c_str() ) ); + +#if 0 && defined( DEBUG ) + printf( "%s\n", footprintName.c_str() ); + if( footprintName == "QFN40" ) + { + int breakhere = 1; + (void) breakhere; + } +#endif + + m_owner->LoadMODULE( module.get() ); + + MODULE* m = module.release(); // exceptions after this are not expected. + + wxASSERT( footprintName == TO_UTF8( m->GetLibRef() ) ); /* diff --git a/pcbnew/legacy_plugin.h b/pcbnew/legacy_plugin.h index 82d0681e6c..cefba1ff14 100644 --- a/pcbnew/legacy_plugin.h +++ b/pcbnew/legacy_plugin.h @@ -106,7 +106,7 @@ public: void SetReader( LINE_READER* aReader ) { m_reader = aReader; } void SetFilePtr( FILE* aFile ) { m_fp = aFile; } - MODULE* LoadMODULE(); + void LoadMODULE( MODULE* aModule ); void SaveMODULE( const MODULE* aModule ) const; void SaveModule3D( const MODULE* aModule ) const; void SaveBOARD( const BOARD* aBoard ) const;