From 8b5f3938041ec7644d3810af4c8813d0d09e4f0c Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Tue, 17 Aug 2021 15:37:33 +0100 Subject: [PATCH] DRC bug fixes and debugging improvements. 1) Don't wait for UpdateUserInterface to build the (global) list of layer names. 2) Report clearance resolution between a silk layer and a mask layer. 3) When writing DRC reports, include info about the violating rule. 4) Report "no relevant layers" if we fail to find anything to write a clearance resolution report about. Fixes https://gitlab.com/kicad/code/kicad/issues/8963 --- common/rc_item.cpp | 11 +++++++---- include/rc_item.h | 5 +++++ pcbnew/dialogs/dialog_constraints_reporter.cpp | 6 ++++++ pcbnew/dialogs/dialog_constraints_reporter.h | 3 +++ pcbnew/drc/drc_item.cpp | 11 +++++++++-- pcbnew/drc/drc_item.h | 2 ++ pcbnew/pcb_edit_frame.cpp | 15 +++++++++++++++ pcbnew/tools/board_inspection_tool.cpp | 16 +++++++++++++++- 8 files changed, 62 insertions(+), 7 deletions(-) diff --git a/common/rc_item.cpp b/common/rc_item.cpp index e0e6b837d5..a19ad66713 100644 --- a/common/rc_item.cpp +++ b/common/rc_item.cpp @@ -109,13 +109,14 @@ wxString RC_ITEM::ShowReport( EDA_UNITS aUnits, SEVERITY aSeverity, // Note: some customers machine-process these. So: // 1) don't translate // 2) try not to re-order or change syntax - // 3) report numeric error code (which should be more stable) in addition to message + // 3) report settings key (which should be more stable) in addition to message if( mainItem && auxItem ) { - return wxString::Format( wxT( "[%s]: %s %s\n %s: %s\n %s: %s\n" ), + return wxString::Format( wxT( "[%s]: %s\n %s; %s\n %s: %s\n %s: %s\n" ), GetSettingsKey(), GetErrorMessage(), + GetViolatingRuleDesc(), severity, ShowCoord( aUnits, mainItem->GetPosition() ), mainItem->GetSelectMenuText( aUnits ), @@ -124,18 +125,20 @@ wxString RC_ITEM::ShowReport( EDA_UNITS aUnits, SEVERITY aSeverity, } else if( mainItem ) { - return wxString::Format( wxT( "[%s]: %s %s\n %s: %s\n" ), + return wxString::Format( wxT( "[%s]: %s\n %s; %s\n %s: %s\n" ), GetSettingsKey(), GetErrorMessage(), + GetViolatingRuleDesc(), severity, ShowCoord( aUnits, mainItem->GetPosition() ), mainItem->GetSelectMenuText( aUnits ) ); } else { - return wxString::Format( wxT( "[%s]: %s %s\n" ), + return wxString::Format( wxT( "[%s]: %s\n %s; %s\n" ), GetSettingsKey(), GetErrorMessage(), + GetViolatingRuleDesc(), severity ); } } diff --git a/include/rc_item.h b/include/rc_item.h index 0f4073e686..28486147e6 100644 --- a/include/rc_item.h +++ b/include/rc_item.h @@ -152,6 +152,11 @@ public: return m_settingsKey; } + virtual wxString GetViolatingRuleDesc() const + { + return wxEmptyString; + } + /** * Format a coordinate or position to text. */ diff --git a/pcbnew/dialogs/dialog_constraints_reporter.cpp b/pcbnew/dialogs/dialog_constraints_reporter.cpp index c182742d2b..e81c88c736 100644 --- a/pcbnew/dialogs/dialog_constraints_reporter.cpp +++ b/pcbnew/dialogs/dialog_constraints_reporter.cpp @@ -80,3 +80,9 @@ WX_HTML_REPORT_BOX* DIALOG_CONSTRAINTS_REPORTER::AddPage( const wxString& aTitle return reporter; } + +int DIALOG_CONSTRAINTS_REPORTER::GetPageCount() const +{ + return m_notebook->GetPageCount(); +} + diff --git a/pcbnew/dialogs/dialog_constraints_reporter.h b/pcbnew/dialogs/dialog_constraints_reporter.h index 93a374a7b0..1f2d5a961c 100644 --- a/pcbnew/dialogs/dialog_constraints_reporter.h +++ b/pcbnew/dialogs/dialog_constraints_reporter.h @@ -45,8 +45,11 @@ public: void OnErrorLinkClicked( wxHtmlLinkEvent& event ); void DeleteAllPages(); + WX_HTML_REPORT_BOX* AddPage( const wxString& pageTitle ); + int GetPageCount() const; + protected: PCB_EDIT_FRAME* m_frame; }; diff --git a/pcbnew/drc/drc_item.cpp b/pcbnew/drc/drc_item.cpp index 0313831e7e..1d8e5bc97a 100644 --- a/pcbnew/drc/drc_item.cpp +++ b/pcbnew/drc/drc_item.cpp @@ -26,6 +26,7 @@ #include "wx/html/m_templ.h" #include "wx/html/styleparams.h" #include +#include #include @@ -297,8 +298,6 @@ std::shared_ptr DRC_ITEM::Create( int aErrorCode ) wxFAIL_MSG( "Unknown DRC error code" ); return nullptr; } - - return nullptr; } @@ -315,3 +314,11 @@ std::shared_ptr DRC_ITEM::Create( const wxString& aErrorKey ) } +wxString DRC_ITEM::GetViolatingRuleDesc() const +{ + if( m_violatingRule ) + return wxString::Format( _( "Rule: %s" ), m_violatingRule->m_Name ); + + return wxEmptyString; +} + diff --git a/pcbnew/drc/drc_item.h b/pcbnew/drc/drc_item.h index cb3596a730..3c67ec4714 100644 --- a/pcbnew/drc/drc_item.h +++ b/pcbnew/drc/drc_item.h @@ -107,6 +107,8 @@ public: void SetViolatingRule ( DRC_RULE *aRule ) { m_violatingRule = aRule; } DRC_RULE* GetViolatingRule() const { return m_violatingRule; } + wxString GetViolatingRuleDesc() const override; + void SetViolatingTest( DRC_TEST_PROVIDER *aProvider ) { m_violatingTest = aProvider; } DRC_TEST_PROVIDER* GetViolatingTest() const { return m_violatingTest; } diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index abfd8bafc0..65fcee1292 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -1070,6 +1070,21 @@ void PCB_EDIT_FRAME::SetActiveLayer( PCB_LAYER_ID aLayer ) void PCB_EDIT_FRAME::onBoardLoaded() { + // JEY TODO: move this global to the board + ENUM_MAP& layerEnum = ENUM_MAP::Instance(); + + layerEnum.Choices().Clear(); + layerEnum.Undefined( UNDEFINED_LAYER ); + + for( LSEQ seq = LSET::AllLayersMask().Seq(); seq; ++seq ) + { + // Canonical name + layerEnum.Map( *seq, LSET::Name( *seq ) ); + + // User name + layerEnum.Map( *seq, GetBoard()->GetLayerName( *seq ) ); + } + DRC_TOOL* drcTool = m_toolManager->GetTool(); try diff --git a/pcbnew/tools/board_inspection_tool.cpp b/pcbnew/tools/board_inspection_tool.cpp index 08c23f943a..443e7e5bcd 100644 --- a/pcbnew/tools/board_inspection_tool.cpp +++ b/pcbnew/tools/board_inspection_tool.cpp @@ -487,7 +487,11 @@ int BOARD_INSPECTION_TOOL::InspectClearance( const TOOL_EVENT& aEvent ) for( PCB_LAYER_ID layer : { F_SilkS, B_SilkS } ) { - if( layerIntersection.test( layer ) ) + PCB_LAYER_ID correspondingMask = IsFrontLayer( layer ) ? F_Mask : B_Mask; + + if( ( a->IsOnLayer( layer ) && b->IsOnLayer( layer ) ) + || ( a->IsOnLayer( layer ) && b->IsOnLayer( correspondingMask ) ) + || ( b->IsOnLayer( layer ) && a->IsOnLayer( correspondingMask ) ) ) { r = m_inspectClearanceDialog->AddPage( m_frame->GetBoard()->GetLayerName( layer ) ); @@ -619,6 +623,16 @@ int BOARD_INSPECTION_TOOL::InspectClearance( const TOOL_EVENT& aEvent ) } } + if( m_inspectClearanceDialog->GetPageCount() == 0 ) + { + r = m_inspectClearanceDialog->AddPage( _( "Clearance" ) ); + r->Report( "" + _( "Items share no relevant layers:" ) + "" ); + r->Report( wxString::Format( "
  • %s
  • %s
", + EscapeHTML( getItemDescription( a ) ), + EscapeHTML( getItemDescription( b ) ) ) ); + r->Flush(); + } + m_inspectClearanceDialog->Raise(); m_inspectClearanceDialog->Show( true ); return 0;