From 56ea55ae9c9fbd49da016d9ce2960c0293b9d707 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 31 Jan 2021 00:30:40 +0000 Subject: [PATCH] Make sure that NPTH pads still get handled in copper clearance checker. ... because that's where we do hole-to-copper clearance testing. Also augments the Clearance Resolution reporter to report on said hole clearances. And changes the interpretation of HOLE_CLEARANCE_CONSTRAINT to include local pad clearance overrides. Fixes https://gitlab.com/kicad/code/kicad/issues/7325 --- pcbnew/drc/drc_engine.cpp | 4 +- pcbnew/drc/drc_rtree.h | 13 +++++- pcbnew/drc/drc_test_provider.cpp | 4 +- pcbnew/tools/board_inspection_tool.cpp | 55 +++++++++++++++++++++----- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/pcbnew/drc/drc_engine.cpp b/pcbnew/drc/drc_engine.cpp index 42e216f838..51d7ccc6b0 100644 --- a/pcbnew/drc/drc_engine.cpp +++ b/pcbnew/drc/drc_engine.cpp @@ -729,7 +729,7 @@ DRC_CONSTRAINT DRC_ENGINE::EvalRulesForItems( DRC_CONSTRAINT_T aConstraintId, bool implicit = false; // Local overrides take precedence - if( aConstraintId == CLEARANCE_CONSTRAINT ) + if( aConstraintId == CLEARANCE_CONSTRAINT || aConstraintId == HOLE_CLEARANCE_CONSTRAINT ) { int overrideA = 0; int overrideB = 0; @@ -756,7 +756,7 @@ DRC_CONSTRAINT DRC_ENGINE::EvalRulesForItems( DRC_CONSTRAINT_T aConstraintId, if( overrideA || overrideB ) { - DRC_CONSTRAINT constraint( CLEARANCE_CONSTRAINT, m_msg ); + DRC_CONSTRAINT constraint( aConstraintId, m_msg ); constraint.m_Value.SetMin( std::max( overrideA, overrideB ) ); return constraint; } diff --git a/pcbnew/drc/drc_rtree.h b/pcbnew/drc/drc_rtree.h index faf7e21f65..c0e5bdf770 100644 --- a/pcbnew/drc/drc_rtree.h +++ b/pcbnew/drc/drc_rtree.h @@ -122,7 +122,18 @@ public: } else { - for( int layer : aItem->GetLayerSet().Seq() ) + LSET layers = aItem->GetLayerSet(); + + // Special-case pad holes which pierce all the copper layers + if( aItem->Type() == PCB_PAD_T ) + { + PAD* pad = static_cast( aItem ); + + if( pad->GetDrillSizeX() > 0 && pad->GetDrillSizeY() > 0 ) + layers |= LSET::AllCuMask(); + } + + for( int layer : layers.Seq() ) addLayer( (PCB_LAYER_ID) layer ); } } diff --git a/pcbnew/drc/drc_test_provider.cpp b/pcbnew/drc/drc_test_provider.cpp index 4d80eb7896..634985f2bf 100644 --- a/pcbnew/drc/drc_test_provider.cpp +++ b/pcbnew/drc/drc_test_provider.cpp @@ -263,7 +263,9 @@ int DRC_TEST_PROVIDER::forEachGeometryItem( const std::vector& aTypes, { for( PAD* pad : footprint->Pads() ) { - if( ( pad->GetLayerSet() & aLayers ).any() ) + // Careful: if a pad has a hole then it pierces all layers + if( ( pad->GetDrillSizeX() > 0 && pad->GetDrillSizeY() > 0 ) + || ( pad->GetLayerSet() & aLayers ).any() ) { if( !aFunc( pad ) ) return n; diff --git a/pcbnew/tools/board_inspection_tool.cpp b/pcbnew/tools/board_inspection_tool.cpp index a27c7e582a..5ac6c41e5e 100644 --- a/pcbnew/tools/board_inspection_tool.cpp +++ b/pcbnew/tools/board_inspection_tool.cpp @@ -282,6 +282,13 @@ int BOARD_INSPECTION_TOOL::InspectClearance( const TOOL_EVENT& aEvent ) BOARD_ITEM* a = static_cast( selection.GetItem( 0 ) ); BOARD_ITEM* b = static_cast( selection.GetItem( 1 ) ); + auto hasHole = + []( BOARD_ITEM* aItem ) + { + PAD* pad = dynamic_cast( aItem ); + return pad && pad->GetDrillSizeX() > 0 && pad->GetDrillSizeY() > 0; + }; + wxCHECK( a && b, 0 ); if( a->Type() == PCB_GROUP_T ) @@ -322,16 +329,16 @@ int BOARD_INSPECTION_TOOL::InspectClearance( const TOOL_EVENT& aEvent ) { PAD* pad = static_cast( a ); - if( pad->GetAttribute() == PAD_ATTRIB_SMD && pad->IsOnLayer( F_Cu ) ) + if( pad->IsOnLayer( F_Cu ) ) layer = F_Cu; else layer = B_Cu; } - else if( b->Type() == PCB_PAD_T ) + else if( b->Type() == PCB_PAD_T && static_cast( a )->GetAttribute() == PAD_ATTRIB_SMD ) { PAD* pad = static_cast( b ); - if( pad->GetAttribute() == PAD_ATTRIB_SMD && pad->IsOnLayer( F_Cu ) ) + if( pad->IsOnLayer( F_Cu ) ) layer = F_Cu; else layer = B_Cu; @@ -356,15 +363,45 @@ int BOARD_INSPECTION_TOOL::InspectClearance( const TOOL_EVENT& aEvent ) } else if( !( a->GetLayerSet() & LSET( 3, layer, Edge_Cuts, Margin ) ).any() ) { - r->Report( wxString::Format( _( "%s not present on layer %s. No clearance defined." ), - a->GetSelectMenuText( r->GetUnits() ), - m_frame->GetBoard()->GetLayerName( layer ) ) ); + if( hasHole( a ) ) + { + r->Report( "" + _( "Hole clearance resolution for:" ) + "" ); + + r->Report( wxString::Format( "
  • %s %s
  • %s
  • %s
", + _( "Layer" ), + EscapeHTML( m_frame->GetBoard()->GetLayerName( layer ) ), + EscapeHTML( getItemDescription( a ) ), + EscapeHTML( getItemDescription( b ) ) ) ); + + reportClearance( HOLE_CLEARANCE_CONSTRAINT, layer, a, b, r ); + } + else + { + r->Report( wxString::Format( _( "%s not present on layer %s. No clearance defined." ), + a->GetSelectMenuText( r->GetUnits() ), + m_frame->GetBoard()->GetLayerName( layer ) ) ); + } } else if( !( b->GetLayerSet() & LSET( 3, layer, Edge_Cuts, Margin ) ).any() ) { - r->Report( wxString::Format( _( "%s not present on layer %s. No clearance defined." ), - b->GetSelectMenuText( r->GetUnits() ), - m_frame->GetBoard()->GetLayerName( layer ) ) ); + if( hasHole( b ) ) + { + r->Report( "" + _( "Hole clearance resolution for:" ) + "" ); + + r->Report( wxString::Format( "
  • %s %s
  • %s
  • %s
", + _( "Layer" ), + EscapeHTML( m_frame->GetBoard()->GetLayerName( layer ) ), + EscapeHTML( getItemDescription( b ) ), + EscapeHTML( getItemDescription( a ) ) ) ); + + reportClearance( HOLE_CLEARANCE_CONSTRAINT, layer, b, a, r ); + } + else + { + r->Report( wxString::Format( _( "%s not present on layer %s. No clearance defined." ), + b->GetSelectMenuText( r->GetUnits() ), + m_frame->GetBoard()->GetLayerName( layer ) ) ); + } } else if( a->GetLayer() == Edge_Cuts || a->GetLayer() == Margin || b->GetLayer() == Edge_Cuts || b->GetLayer() == Margin )