From 2b43ffd12da195f945c1a53d5dedd5a35d48a785 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Wed, 16 Sep 2020 21:38:23 +0100 Subject: [PATCH] Better error reporting; better nullptr safety. Also clears the marker lists before running a DRC, and sets the drcRun flags afterwards so that the notebook tab counts get updated. Fixes https://gitlab.com/kicad/code/kicad/issues/5659 --- pcbnew/dialogs/dialog_drc.cpp | 16 +-- pcbnew/dialogs/dialog_drc.h | 3 + pcbnew/dialogs/dialog_drc_base.cpp | 4 +- pcbnew/dialogs/dialog_drc_base.fbp | 4 +- pcbnew/dialogs/dialog_plot.cpp | 2 +- pcbnew/dialogs/panel_setup_rules.cpp | 17 ++- pcbnew/drc/drc_engine.cpp | 159 ++++++++++++++------------- pcbnew/drc/drc_engine.h | 46 +++++--- pcbnew/drc/drc_rule_parser.cpp | 91 ++++++++++----- pcbnew/drc/drc_rule_parser.h | 4 +- pcbnew/pcb_base_edit_frame.cpp | 11 +- pcbnew/tools/drc_tool.cpp | 33 +++++- pcbnew/tools/drc_tool.h | 13 +-- pcbnew/tools/pcb_inspection_tool.cpp | 13 ++- 14 files changed, 258 insertions(+), 158 deletions(-) diff --git a/pcbnew/dialogs/dialog_drc.cpp b/pcbnew/dialogs/dialog_drc.cpp index 86981a15b1..80c5332228 100644 --- a/pcbnew/dialogs/dialog_drc.cpp +++ b/pcbnew/dialogs/dialog_drc.cpp @@ -115,7 +115,7 @@ void DIALOG_DRC::OnActivateDlg( wxActivateEvent& aEvent ) Close(); DRC_TOOL* drcTool = m_brdEditor->GetToolManager()->GetTool(); - drcTool->DestroyDRCDialog( wxID_CANCEL ); + drcTool->DestroyDRCDialog(); return; } @@ -185,8 +185,6 @@ void DIALOG_DRC::SetCurrentProgress( double aProgress ) } - - // Don't globally define this; different facilities use different definitions of "ALL" static int RPT_SEVERITY_ALL = RPT_SEVERITY_WARNING | RPT_SEVERITY_ERROR | RPT_SEVERITY_EXCLUSION; @@ -213,9 +211,9 @@ void DIALOG_DRC::OnRunDRCClick( wxCommandEvent& aEvent ) m_brdEditor->RecordDRCExclusions(); deleteAllMarkers( true ); + m_unconnectedTreeModel->DeleteItems( false, true, true ); + m_footprintWarningsTreeModel->DeleteItems( false, true, true ); - wxBeginBusyCursor(); - wxWindowDisabler disabler( this ); Raise(); m_Notebook->ChangeSelection( 0 ); // Display the "Messages" tab @@ -224,12 +222,6 @@ void DIALOG_DRC::OnRunDRCClick( wxCommandEvent& aEvent ) drcTool->RunTests( this, testTracksAgainstZones, refillZones, reportAllTrackErrors, testFootprints ); - m_drcRun = true; - - if( testFootprints ) - m_footprintTestsRun = true; - - wxEndBusyCursor(); refreshBoardEditor(); @@ -569,7 +561,7 @@ void DIALOG_DRC::OnCancelClick( wxCommandEvent& aEvent ) // The dialog can be modal or not modal. // Leave the DRC caller destroy (or not) the dialog DRC_TOOL* drcTool = m_brdEditor->GetToolManager()->GetTool(); - drcTool->DestroyDRCDialog( wxID_CANCEL ); + drcTool->DestroyDRCDialog(); } diff --git a/pcbnew/dialogs/dialog_drc.h b/pcbnew/dialogs/dialog_drc.h index 703fe66438..24c085117d 100644 --- a/pcbnew/dialogs/dialog_drc.h +++ b/pcbnew/dialogs/dialog_drc.h @@ -50,6 +50,9 @@ public: DIALOG_DRC( PCB_EDIT_FRAME* aEditorFrame, wxWindow* aParent ); ~DIALOG_DRC(); + void SetDrcRun() { m_drcRun = true; } + void SetFootprintTestsRun() { m_footprintTestsRun = true; } + void SetMarkersProvider( RC_ITEMS_PROVIDER* aProvider ); void SetUnconnectedProvider( RC_ITEMS_PROVIDER* aProvider ); void SetFootprintsProvider( RC_ITEMS_PROVIDER* aProvider ); diff --git a/pcbnew/dialogs/dialog_drc_base.cpp b/pcbnew/dialogs/dialog_drc_base.cpp index a5b492ba58..89c3f63d9e 100644 --- a/pcbnew/dialogs/dialog_drc_base.cpp +++ b/pcbnew/dialogs/dialog_drc_base.cpp @@ -72,7 +72,7 @@ DIALOG_DRC_BASE::DIALOG_DRC_BASE( wxWindow* parent, wxWindowID id, const wxStrin m_panelMessages->SetSizer( bSizer10 ); m_panelMessages->Layout(); bSizer10->Fit( m_panelMessages ); - m_Notebook->AddPage( m_panelMessages, _("Messages"), false ); + m_Notebook->AddPage( m_panelMessages, _("Messages"), true ); m_panelViolations = new wxPanel( m_Notebook, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL ); wxBoxSizer* bSizerViolationsBox; bSizerViolationsBox = new wxBoxSizer( wxVERTICAL ); @@ -87,7 +87,7 @@ DIALOG_DRC_BASE::DIALOG_DRC_BASE( wxWindow* parent, wxWindowID id, const wxStrin m_panelViolations->SetSizer( bSizerViolationsBox ); m_panelViolations->Layout(); bSizerViolationsBox->Fit( m_panelViolations ); - m_Notebook->AddPage( m_panelViolations, _("Violations (%d)"), true ); + m_Notebook->AddPage( m_panelViolations, _("Violations (%d)"), false ); m_panelUnconnectedItems = new wxPanel( m_Notebook, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL ); wxBoxSizer* bSizerUnconnectedBox; bSizerUnconnectedBox = new wxBoxSizer( wxVERTICAL ); diff --git a/pcbnew/dialogs/dialog_drc_base.fbp b/pcbnew/dialogs/dialog_drc_base.fbp index a37b27f46a..fcb6687f59 100644 --- a/pcbnew/dialogs/dialog_drc_base.fbp +++ b/pcbnew/dialogs/dialog_drc_base.fbp @@ -409,7 +409,7 @@ Messages - 0 + 1 1 1 @@ -611,7 +611,7 @@ Violations (%d) - 1 + 0 1 1 diff --git a/pcbnew/dialogs/dialog_plot.cpp b/pcbnew/dialogs/dialog_plot.cpp index 2c1dd2669a..2c8fa3d73d 100644 --- a/pcbnew/dialogs/dialog_plot.cpp +++ b/pcbnew/dialogs/dialog_plot.cpp @@ -907,7 +907,7 @@ void DIALOG_PLOT::onRunDRC( wxCommandEvent& event ) // First close an existing dialog if open // (low probability, but can happen) - drcTool->DestroyDRCDialog( wxID_OK ); + drcTool->DestroyDRCDialog(); // Open a new drc dialod, with the right parent frame, and in Modal Mode drcTool->ShowDRCDialog( this ); diff --git a/pcbnew/dialogs/panel_setup_rules.cpp b/pcbnew/dialogs/panel_setup_rules.cpp index db2ea822a1..5addcef628 100644 --- a/pcbnew/dialogs/panel_setup_rules.cpp +++ b/pcbnew/dialogs/panel_setup_rules.cpp @@ -294,7 +294,7 @@ void PANEL_SETUP_RULES::OnCompile( wxCommandEvent& event ) { std::vector dummyRules; - DRC_RULES_PARSER parser( m_frame->GetBoard(), m_textEditor->GetText(), _( "DRC rules" ) ); + DRC_RULES_PARSER parser( m_textEditor->GetText(), _( "DRC rules" ) ); parser.Parse( dummyRules, m_errorsReport ); } @@ -363,7 +363,7 @@ bool PANEL_SETUP_RULES::TransferDataFromWindow() { std::vector dummyRules; - DRC_RULES_PARSER parser( m_frame->GetBoard(), m_textEditor->GetText(), _( "DRC rules" ) ); + DRC_RULES_PARSER parser( m_textEditor->GetText(), _( "DRC rules" ) ); parser.Parse( dummyRules, m_errorsReport ); } @@ -375,9 +375,18 @@ bool PANEL_SETUP_RULES::TransferDataFromWindow() wxString rulesFilepath = m_frame->Prj().AbsolutePath( "drc-rules" ); - if( m_textEditor->SaveFile( rulesFilepath ) ) + try { - m_frame->GetBoard()->GetDesignSettings().m_DRCEngine->LoadRules( rulesFilepath ); + if( m_textEditor->SaveFile( rulesFilepath ) ) + { + m_frame->GetBoard()->GetDesignSettings().m_DRCEngine->LoadRules( rulesFilepath ); + return true; + } + } + catch( PARSE_ERROR& pe ) + { + // Don't lock them in to the Setup dialog if they have bad rules. They've already + // saved them so we can allow an exit. return true; } diff --git a/pcbnew/drc/drc_engine.cpp b/pcbnew/drc/drc_engine.cpp index 0716a12072..ad97ceb29e 100644 --- a/pcbnew/drc/drc_engine.cpp +++ b/pcbnew/drc/drc_engine.cpp @@ -260,11 +260,11 @@ static wxString formatConstraint( const DRC_CONSTRAINT& constraint ) } -bool DRC_ENGINE::LoadRules( const wxFileName& aPath ) +/** + * @throws PARSE_ERROR + */ +void DRC_ENGINE::LoadRules( const wxFileName& aPath ) { - NULL_REPORTER nullReporter; - REPORTER* reporter = m_reporter ? m_reporter : &nullReporter; - if( aPath.FileExists() ) { m_ruleConditions.clear(); @@ -276,8 +276,8 @@ bool DRC_ENGINE::LoadRules( const wxFileName& aPath ) { try { - DRC_RULES_PARSER parser( m_board, fp, aPath.GetFullPath() ); - parser.Parse( m_rules, reporter ); + DRC_RULES_PARSER parser( fp, aPath.GetFullPath() ); + parser.Parse( m_rules, m_reporter ); } catch( PARSE_ERROR& pe ) { @@ -285,17 +285,10 @@ bool DRC_ENGINE::LoadRules( const wxFileName& aPath ) m_ruleConditions.clear(); m_rules.clear(); - // JEY TODO - //wxSafeYield( m_editFrame ); - //m_editFrame->ShowBoardSetupDialog( _( "Rules" ), pe.What(), ID_RULES_EDITOR, - // pe.lineNumber, pe.byteIndex ); - - return false; + throw pe; } } } - - return true; } @@ -375,6 +368,9 @@ bool DRC_ENGINE::CompileRules() } +/** + * @throws PARSE_ERROR + */ void DRC_ENGINE::InitEngine( const wxFileName& aRulePath ) { m_testProviders = DRC_TEST_PROVIDER_REGISTRY::Instance().GetTestProviders(); @@ -442,6 +438,8 @@ DRC_CONSTRAINT DRC_ENGINE::EvalRulesForItems( DRC_CONSTRAINT_TYPE_T aConstraintI const BOARD_CONNECTED_ITEM* connectedA = dynamic_cast( a ); const BOARD_CONNECTED_ITEM* connectedB = dynamic_cast( b ); + const DRC_CONSTRAINT* constraintRef = nullptr; + bool implicit = false; // Local overrides take precedence if( aConstraintId == DRC_CONSTRAINT_TYPE_CLEARANCE ) @@ -477,80 +475,83 @@ DRC_CONSTRAINT DRC_ENGINE::EvalRulesForItems( DRC_CONSTRAINT_TYPE_T aConstraintI } } - std::vector* ruleset = m_constraintMap[ aConstraintId ]; - - const DRC_CONSTRAINT* constraintRef = nullptr; - bool implicit = false; - - // Last matching rule wins, so process in reverse order - for( int ii = (int) ruleset->size() - 1; ii >= 0; --ii ) + if( m_constraintMap.count( aConstraintId ) ) { - wxString msg; - const CONSTRAINT_WITH_CONDITIONS* rcons = ruleset->at( ii ); - implicit = rcons->parentRule && rcons->parentRule->m_Implicit; + std::vector* ruleset = m_constraintMap[ aConstraintId ]; - REPORT( "" ) - - if( implicit ) + // Last matching rule wins, so process in reverse order + for( int ii = (int) ruleset->size() - 1; ii >= 0; --ii ) { - msg = wxString::Format( _( "Checking %s;" ), - rcons->parentRule->m_Name ); - } - else - { - msg = wxString::Format( _( "Checking rule %s;"), - rcons->parentRule->m_Name ); - } + wxString msg; + const CONSTRAINT_WITH_CONDITIONS* rcons = ruleset->at( ii ); + implicit = rcons->parentRule && rcons->parentRule->m_Implicit; - if( aConstraintId == DRC_CONSTRAINT_TYPE_CLEARANCE ) - { - int clearance = rcons->constraint.m_Value.Min(); + REPORT( "" ) - msg += " "; - msg += wxString::Format( _( "clearance: %s." ), - MessageTextFromValue( UNITS, clearance, true ) ); - } - - REPORT( msg ); - - if( aLayer != UNDEFINED_LAYER && !rcons->layerTest.test( aLayer ) ) - { - REPORT( wxString::Format( _( "Rule layer \"%s\" not matched." ), - rcons->parentRule->m_LayerSource ) ) - REPORT( "Rule not applied." ) - - continue; - } - - if( !rcons->condition || rcons->condition->GetExpression().IsEmpty() ) - { - REPORT( implicit ? _( "Unconditional constraint applied." ) - : _( "Unconditional rule applied." ) ) - - constraintRef = &rcons->constraint; - break; - } - else - { - // Don't report on implicit rule conditions; they're synthetic. - if( !implicit ) + if( implicit ) { - REPORT( wxString::Format( _( "Checking rule condition \"%s\"." ), - rcons->condition->GetExpression() ) ) + msg = wxString::Format( _( "Checking %s;" ), + rcons->constraint.GetName() ); + } + else + { + msg = wxString::Format( _( "Checking rule %s;"), + rcons->constraint.GetName() ); } - if( rcons->condition->EvaluateFor( a, b, aLayer, aReporter ) ) + if( aConstraintId == DRC_CONSTRAINT_TYPE_CLEARANCE ) { - REPORT( implicit ? _( "Constraint applicable." ) - : _( "Rule applied." ) ) + int clearance = rcons->constraint.m_Value.Min(); + + msg += " "; + msg += wxString::Format( _( "clearance: %s." ), + MessageTextFromValue( UNITS, clearance, true ) ); + } + + REPORT( msg ); + + if( aLayer != UNDEFINED_LAYER && !rcons->layerTest.test( aLayer ) ) + { + if( rcons->parentRule ) + { + REPORT( wxString::Format( _( "Rule layer \"%s\" not matched." ), + rcons->parentRule->m_LayerSource ) ) + REPORT( "Rule not applied." ) + } + + continue; + } + + if( !rcons->condition || rcons->condition->GetExpression().IsEmpty() ) + { + REPORT( implicit ? _( "Unconditional constraint applied." ) + : _( "Unconditional rule applied." ) ) constraintRef = &rcons->constraint; break; } else { - REPORT( implicit ? _( "Membership not satisfied; constraint not applicable." ) - : _( "Condition not satisfied; rule not applied." ) ) + // Don't report on implicit rule conditions; they're synthetic. + if( !implicit ) + { + REPORT( wxString::Format( _( "Checking rule condition \"%s\"." ), + rcons->condition->GetExpression() ) ) + } + + if( rcons->condition->EvaluateFor( a, b, aLayer, aReporter ) ) + { + REPORT( implicit ? _( "Constraint applicable." ) + : _( "Rule applied." ) ) + + constraintRef = &rcons->constraint; + break; + } + else + { + REPORT( implicit ? _( "Membership not satisfied; constraint not applicable." ) + : _( "Condition not satisfied; rule not applied." ) ) + } } } } @@ -695,8 +696,11 @@ std::vector DRC_ENGINE::QueryConstraintsById( DRC_CONSTRAINT_TYP { std::vector rv; - for ( CONSTRAINT_WITH_CONDITIONS* c : *m_constraintMap[constraintID] ) - rv.push_back( c->constraint ); + if( m_constraintMap.count( constraintID ) ) + { + for ( CONSTRAINT_WITH_CONDITIONS* c : *m_constraintMap[ constraintID ] ) + rv.push_back( c->constraint ); + } return rv; } @@ -705,7 +709,10 @@ std::vector DRC_ENGINE::QueryConstraintsById( DRC_CONSTRAINT_TYP bool DRC_ENGINE::HasRulesForConstraintType( DRC_CONSTRAINT_TYPE_T constraintID ) { //drc_dbg(10,"hascorrect id %d size %d\n", ruleID, m_ruleMap[ruleID]->sortedRules.size( ) ); - return m_constraintMap[constraintID]->size() != 0; + if( m_constraintMap.count( constraintID ) ) + return m_constraintMap[ constraintID ]->size() > 0; + + return false; } diff --git a/pcbnew/drc/drc_engine.h b/pcbnew/drc/drc_engine.h index e1f0709112..721c6259fd 100644 --- a/pcbnew/drc/drc_engine.h +++ b/pcbnew/drc/drc_engine.h @@ -69,12 +69,14 @@ std::function& aItem, wxPoint aPos )> DRC_ /** - * Design Rule Checker object that performs all the DRC tests. The output of - * the checking goes to the BOARD file in the form of two MARKER lists. Those - * two lists are displayable in the drc dialog box. And they can optionally - * be sent to a text file on disk. - * This class is given access to the windows and the BOARD - * that it needs via its constructor or public access functions. + * Design Rule Checker object that performs all the DRC tests. + * + * Optionally reports violations via a DRC_VIOLATION_HANDLER, user-level progress via a + * PROGRESS_REPORTER and rule parse errors via a REPORTER, all set through various setter + * calls. + * + * Note that EvalRulesForItems() has yet another optional REPORTER for reporting resolution + * info to the user. */ class DRC_ENGINE { @@ -88,8 +90,8 @@ public: void SetWorksheet( KIGFX::WS_PROXY_VIEW_ITEM* aWorksheet ) { m_worksheet = aWorksheet; } KIGFX::WS_PROXY_VIEW_ITEM* GetWorksheet() const { return m_worksheet; } - /* - * Receives DRC violation reports (a DRC_ITEM and a position). + /** + * Set an optional DRC violation handler (receives DRC_ITEMs and positions). */ void SetViolationHandler( DRC_VIOLATION_HANDLER aHandler ) { @@ -101,23 +103,41 @@ public: m_violationHandler = DRC_VIOLATION_HANDLER(); } - /* - * Receives progress information to show the user. + /** + * Set an optional reporter for user-level progress info. */ void SetProgressReporter( PROGRESS_REPORTER* aProgRep ) { m_progressReporter = aProgRep; } /* - * Receives debug information for a developer. + * Set an optional reporter for rule parse/compile/run-time errors and log-level progress + * information. + * + * Note: if no log reporter is installed rule parse/compile/run-time errors are returned + * via a thrown PARSE_ERROR exception. */ void SetLogReporter( REPORTER* aReporter ) { m_reporter = aReporter; } - /* + /** * Loads and parses a rule set from an sexpr text file. + * + * @throws PARSE_ERROR */ - bool LoadRules( const wxFileName& aPath ); + void LoadRules( const wxFileName& aPath ); + /** + * Initializes the DRC engine. + * + * @throws PARSE_ERROR if the rules file contains errors + */ void InitEngine( const wxFileName& aRulePath ); + /** + * Runs the DRC tests. + * @param aUnits + * @param aTestTracksAgainstZones + * @param aReportAllTrackErrors + * @param aTestFootprints + */ void RunTests( EDA_UNITS aUnits = EDA_UNITS::MILLIMETRES, bool aTestTracksAgainstZones = true, bool aReportAllTrackErrors = true, bool aTestFootprints = true ); diff --git a/pcbnew/drc/drc_rule_parser.cpp b/pcbnew/drc/drc_rule_parser.cpp index 9afdb8683d..966d18490b 100644 --- a/pcbnew/drc/drc_rule_parser.cpp +++ b/pcbnew/drc/drc_rule_parser.cpp @@ -33,8 +33,7 @@ using namespace DRCRULE_T; -DRC_RULES_PARSER::DRC_RULES_PARSER( BOARD* aBoard, const wxString& aSource, - const wxString& aSourceDescr ) : +DRC_RULES_PARSER::DRC_RULES_PARSER( const wxString& aSource, const wxString& aSourceDescr ) : DRC_RULES_LEXER( aSource.ToStdString(), aSourceDescr ), m_requiredVersion( 0 ), m_tooRecent( false ), @@ -43,7 +42,7 @@ DRC_RULES_PARSER::DRC_RULES_PARSER( BOARD* aBoard, const wxString& aSource, } -DRC_RULES_PARSER::DRC_RULES_PARSER( BOARD* aBoard, FILE* aFile, const wxString& aFilename ) : +DRC_RULES_PARSER::DRC_RULES_PARSER( FILE* aFile, const wxString& aFilename ) : DRC_RULES_LEXER( aFile, aFilename ), m_requiredVersion( 0 ), m_tooRecent( false ), @@ -56,13 +55,25 @@ void DRC_RULES_PARSER::reportError( const wxString& aMessage ) { wxString rest; wxString first = aMessage.BeforeFirst( '|', &rest ); - wxString msg = wxString::Format( _( "ERROR: %s%s" ), - CurLineNumber(), - CurOffset(), - first, - rest ); - m_reporter->Report( msg, RPT_SEVERITY_ERROR ); + if( m_reporter ) + { + wxString msg = wxString::Format( _( "ERROR: %s%s" ), + CurLineNumber(), + CurOffset(), + first, + rest ); + + m_reporter->Report( msg, RPT_SEVERITY_ERROR ); + } + else + { + wxString msg = wxString::Format( _( "ERROR: %s%s" ), + first, + rest ); + + THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(), CurOffset() ); + } } @@ -124,13 +135,15 @@ void DRC_RULES_PARSER::Parse( std::vector& aRules, REPORTER* aReporte } else { - msg.Printf( _( "Unrecognized item '%s'.| Expected version number" ), FromUTF8() ); + msg.Printf( _( "Unrecognized item '%s'.| Expected version number" ), + FromUTF8() ); reportError( msg ); } if( (int) token != DSN_RIGHT ) { - msg.Printf( _( "Unrecognized item '%s'." ), FromUTF8() ); + msg.Printf( _( "Unrecognized item '%s'." ), + FromUTF8() ); reportError( msg ); parseUnknown(); } @@ -147,13 +160,14 @@ void DRC_RULES_PARSER::Parse( std::vector& aRules, REPORTER* aReporte default: msg.Printf( _( "Unrecognized item '%s'.| Expected %s." ), - FromUTF8(), "'rule', 'version'" ); + FromUTF8(), + "'rule', 'version'" ); reportError( msg ); parseUnknown(); } } - if( !m_reporter->HasMessage() ) + if( m_reporter && !m_reporter->HasMessage() ) m_reporter->Report( _( "No errors found." ), RPT_SEVERITY_INFO ); m_reporter = nullptr; @@ -207,7 +221,8 @@ DRC_RULE* DRC_RULES_PARSER::parseDRC_RULE() if( (int) NextTok() != DSN_RIGHT ) { - reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) ); + reportError( wxString::Format( _( "Unrecognized item '%s'." ), + FromUTF8() ) ); parseUnknown(); } @@ -302,8 +317,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule ) default: msg.Printf( _( "Unrecognized item '%s'.| Expected %s." ), FromUTF8(), - "'track', 'via', 'micro_via', " - "'blind_via', 'pad', 'zone', 'text', 'graphic', 'hole'." + "'track', 'via', 'micro_via', 'blind_via', 'pad', 'zone', 'text', " + "'graphic', 'hole'." ); reportError( msg ); break; @@ -339,7 +354,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule ) if( (int) NextTok() != DSN_RIGHT ) { - reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) ); + reportError( wxString::Format( _( "Unrecognized item '%s'." ), + FromUTF8() ) ); parseUnknown(); } @@ -359,7 +375,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule ) if( (int) NextTok() != DSN_RIGHT ) { - reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) ); + reportError( wxString::Format( _( "Unrecognized item '%s'." ), + FromUTF8() ) ); parseUnknown(); } @@ -379,7 +396,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule ) if( (int) NextTok() != DSN_RIGHT ) { - reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) ); + reportError( wxString::Format( _( "Unrecognized item '%s'." ), + FromUTF8() ) ); parseUnknown(); } @@ -391,7 +409,8 @@ void DRC_RULES_PARSER::parseConstraint( DRC_RULE* aRule ) default: msg.Printf( _( "Unrecognized item '%s'.| Expected %s." ), - FromUTF8(), "'min', 'max', 'opt'" ); + FromUTF8(), + "'min', 'max', 'opt'" ); reportError( msg ); parseUnknown(); } @@ -410,13 +429,25 @@ void DRC_RULES_PARSER::parseValueWithUnits( const wxString& aExpr, int& aResult { wxString rest; wxString first = aMessage.BeforeFirst( '|', &rest ); - wxString msg = wxString::Format( _( "ERROR: %s%s" ), - CurLineNumber(), - CurOffset() + aOffset, - first, - rest ); - m_reporter->Report( msg, RPT_SEVERITY_ERROR ); + if( m_reporter ) + { + wxString msg = wxString::Format( _( "ERROR: %s%s" ), + CurLineNumber(), + CurOffset(), + first, + rest ); + + m_reporter->Report( msg, RPT_SEVERITY_ERROR ); + } + else + { + wxString msg = wxString::Format( _( "ERROR: %s%s" ), + first, + rest ); + + THROW_PARSE_ERROR( msg, CurSource(), CurLine(), CurLineNumber(), CurOffset() ); + } }; PCB_EXPR_EVALUATOR evaluator; @@ -459,12 +490,16 @@ LSET DRC_RULES_PARSER::parseLayer() } if( !retVal.any() ) - reportError( wxString::Format( _( "Unrecognized layer '%s'." ), layerName ) ); + { + reportError( wxString::Format( _( "Unrecognized layer '%s'." ), + layerName ) ); + } } if( (int) NextTok() != DSN_RIGHT ) { - reportError( wxString::Format( _( "Unrecognized item '%s'." ), FromUTF8() ) ); + reportError( wxString::Format( _( "Unrecognized item '%s'." ), + FromUTF8() ) ); parseUnknown(); } diff --git a/pcbnew/drc/drc_rule_parser.h b/pcbnew/drc/drc_rule_parser.h index e4bfa7e50b..b5b58d3709 100644 --- a/pcbnew/drc/drc_rule_parser.h +++ b/pcbnew/drc/drc_rule_parser.h @@ -40,8 +40,8 @@ class BOARD_ITEM; class DRC_RULES_PARSER : public DRC_RULES_LEXER { public: - DRC_RULES_PARSER( BOARD* aBoard, const wxString& aSource, const wxString& aSourceDescr ); - DRC_RULES_PARSER( BOARD* aBoard, FILE* aFile, const wxString& aFilename ); + DRC_RULES_PARSER( const wxString& aSource, const wxString& aSourceDescr ); + DRC_RULES_PARSER( FILE* aFile, const wxString& aFilename ); void Parse( std::vector& aRules, REPORTER* aReporter ); diff --git a/pcbnew/pcb_base_edit_frame.cpp b/pcbnew/pcb_base_edit_frame.cpp index 372e311be8..134df5994c 100644 --- a/pcbnew/pcb_base_edit_frame.cpp +++ b/pcbnew/pcb_base_edit_frame.cpp @@ -133,7 +133,16 @@ void PCB_BASE_EDIT_FRAME::SetBoard( BOARD* aBoard ) { BOARD_DESIGN_SETTINGS& bds = aBoard->GetDesignSettings(); bds.m_DRCEngine = std::make_shared( aBoard, &bds ); - bds.m_DRCEngine->InitEngine( Prj().AbsolutePath( "drc-rules" ) ); + + try + { + bds.m_DRCEngine->InitEngine( Prj().AbsolutePath( "drc-rules" ) ); + } + catch( PARSE_ERROR& pe ) + { + // TODO: We could redirect to Board Setup here and report the error. Or we could + // wait till they run DRC or do an Inspect Clearance. Not sure which is better.... + } if( m_toolManager ) m_toolManager->ResetTools( TOOL_BASE::MODEL_RELOAD ); diff --git a/pcbnew/tools/drc_tool.cpp b/pcbnew/tools/drc_tool.cpp index 9c2cc8b194..480b5e4b63 100644 --- a/pcbnew/tools/drc_tool.cpp +++ b/pcbnew/tools/drc_tool.cpp @@ -36,7 +36,7 @@ #include #include #include - +#include DRC_TOOL::DRC_TOOL() : PCB_TOOL_BASE( "pcbnew.DRCTool" ), @@ -59,7 +59,7 @@ void DRC_TOOL::Reset( RESET_REASON aReason ) if( m_pcb != m_editFrame->GetBoard() ) { if( m_drcDialog ) - DestroyDRCDialog( wxID_OK ); + DestroyDRCDialog(); m_pcb = m_editFrame->GetBoard(); m_drcEngine = m_pcb->GetDesignSettings().m_DRCEngine; @@ -118,7 +118,7 @@ bool DRC_TOOL::IsDRCDialogShown() } -void DRC_TOOL::DestroyDRCDialog( int aReason ) +void DRC_TOOL::DestroyDRCDialog() { if( m_drcDialog ) { @@ -134,6 +134,25 @@ void DRC_TOOL::RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksA ZONE_FILLER_TOOL* zoneFiller = m_toolMgr->GetTool(); BOARD_COMMIT commit( m_editFrame ); NETLIST netlist; + wxWindowDisabler disabler( /* disable everything except: */ m_drcDialog ); + + // This is not the time to have stale or buggy rules. Ensure they're up-to-date + // and that they at least parse. + try + { + m_drcEngine->InitEngine( m_editFrame->Prj().AbsolutePath( "drc-rules" ) ); + } + catch( PARSE_ERROR& pe ) + { + // Shouldn't be necessary, but we get into all kinds of wxWidgets window ordering + // issues (on at least OSX) if we don't. + DestroyDRCDialog(); + + m_editFrame->ShowBoardSetupDialog( _( "Rules" ), pe.What(), ID_RULES_EDITOR, + pe.lineNumber, pe.byteIndex ); + + return; + } if( aRefillZones ) { @@ -148,9 +167,6 @@ void DRC_TOOL::RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksA zoneFiller->CheckAllZones( m_drcDialog, aProgressReporter ); } - // Re-initialize the DRC_ENGINE to make doubly sure everything is up-to-date - // - m_drcEngine->InitEngine( m_editFrame->Prj().AbsolutePath( "drc-rules" ) ); m_drcEngine->SetWorksheet( m_editFrame->GetCanvas()->GetWorksheet() ); if( aTestFootprints && !Kiface().IsSingle() ) @@ -192,6 +208,11 @@ void DRC_TOOL::RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksA m_drcEngine->SetProgressReporter( nullptr ); m_drcEngine->ClearViolationHandler(); + m_drcDialog->SetDrcRun(); + + if( !netlist.IsEmpty() ) + m_drcDialog->SetFootprintTestsRun(); + commit.Push( _( "DRC" ), false ); // update the m_drcDialog listboxes diff --git a/pcbnew/tools/drc_tool.h b/pcbnew/tools/drc_tool.h index 48c053f277..354b5260a3 100644 --- a/pcbnew/tools/drc_tool.h +++ b/pcbnew/tools/drc_tool.h @@ -98,19 +98,12 @@ public: bool IsDRCDialogShown(); /** - * Deletes this ui dialog box and zeros out its pointer to remember - * the state of the dialog's existence. - * - * @param aReason Indication of which button was clicked to cause the destruction. - * if aReason == wxID_OK, design parameters values which can be entered from the dialog - * will bbe saved in design parameters list + * Closes and frees the DRC dialog. */ - void DestroyDRCDialog( int aReason ); + void DestroyDRCDialog(); /** - * Run all the tests specified with a previous call to - * SetSettings() - * @param aMessages = a wxTextControl where to display some activity messages. Can be NULL + * Run the DRC tests. */ void RunTests( PROGRESS_REPORTER* aProgressReporter, bool aTestTracksAgainstZones, bool aRefillZones, bool aReportAllTrackErrors, bool aTestFootprints ); diff --git a/pcbnew/tools/pcb_inspection_tool.cpp b/pcbnew/tools/pcb_inspection_tool.cpp index 4099da0d2e..f6d0892b98 100644 --- a/pcbnew/tools/pcb_inspection_tool.cpp +++ b/pcbnew/tools/pcb_inspection_tool.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include "pcb_inspection_tool.h" @@ -186,7 +187,17 @@ void PCB_INSPECTION_TOOL::reportCopperClearance( PCB_LAYER_ID aLayer, BOARD_CONN r->Report( "" ); DRC_ENGINE drcEngine( m_frame->GetBoard(), &m_frame->GetBoard()->GetDesignSettings() ); - drcEngine.InitEngine( m_frame->Prj().AbsolutePath( "drc-rules" ) ); + + try + { + drcEngine.InitEngine( m_frame->Prj().AbsolutePath( "drc-rules" ) ); + } + catch( PARSE_ERROR& pe ) + { + m_frame->ShowBoardSetupDialog( _( "Rules" ), pe.What(), ID_RULES_EDITOR, + pe.lineNumber, pe.byteIndex ); + return; + } auto constraint = drcEngine.EvalRulesForItems( DRC_CONSTRAINT_TYPE_CLEARANCE, aA, aB, aLayer, r );