From 5aa561abe168bd0d9da0245fb163ad88462828e8 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 18 Mar 2022 19:35:40 +0000 Subject: [PATCH] Add overlapping pad test and share some tests between board & fp editor DRC. Fixes https://gitlab.com/kicad/code/kicad/issues/10086 --- pcbnew/board_design_settings.cpp | 4 + pcbnew/dialogs/dialog_footprint_checker.cpp | 57 ++++++----- pcbnew/drc/drc_item.cpp | 10 +- pcbnew/drc/drc_item.h | 2 + pcbnew/drc/drc_test_provider_hole_size.cpp | 76 ++++----------- pcbnew/footprint.cpp | 101 ++++++++++++++++---- pcbnew/footprint.h | 21 ++-- pcbnew/pcb_base_frame.cpp | 2 +- 8 files changed, 154 insertions(+), 119 deletions(-) diff --git a/pcbnew/board_design_settings.cpp b/pcbnew/board_design_settings.cpp index 44647378f2..55aede86ba 100644 --- a/pcbnew/board_design_settings.cpp +++ b/pcbnew/board_design_settings.cpp @@ -183,6 +183,10 @@ BOARD_DESIGN_SETTINGS::BOARD_DESIGN_SETTINGS( JSON_SETTINGS* aParent, const std: m_DRCSeverities[ DRCE_TEXT_HEIGHT ] = RPT_SEVERITY_WARNING; m_DRCSeverities[ DRCE_TEXT_THICKNESS ] = RPT_SEVERITY_WARNING; + m_DRCSeverities[ DRCE_FOOTPRINT_TYPE_MISMATCH ] = RPT_SEVERITY_WARNING; + m_DRCSeverities[ DRCE_LIB_FOOTPRINT_ISSUES ] = RPT_SEVERITY_WARNING; + m_DRCSeverities[ DRCE_LIB_FOOTPRINT_MISMATCH ] = RPT_SEVERITY_WARNING; + m_MaxError = ARC_HIGH_DEF; m_ZoneKeepExternalFillets = false; m_UseHeightForLengthCalcs = true; diff --git a/pcbnew/dialogs/dialog_footprint_checker.cpp b/pcbnew/dialogs/dialog_footprint_checker.cpp index 31e19b8dcf..1b10f7511f 100644 --- a/pcbnew/dialogs/dialog_footprint_checker.cpp +++ b/pcbnew/dialogs/dialog_footprint_checker.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -101,12 +102,15 @@ void DIALOG_FOOTPRINT_CHECKER::runChecks() return; } - OUTLINE_ERROR_HANDLER errorHandler = - [&]( const wxString& aMsg, BOARD_ITEM* aItemA, BOARD_ITEM* aItemB, const VECTOR2I& aPt ) + auto errorHandler = + [&]( const BOARD_ITEM* aItemA, const BOARD_ITEM* aItemB, int aErrorCode, + const wxString& aMsg, const VECTOR2I& aPt ) { - std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_MALFORMED_COURTYARD ); + std::shared_ptr drcItem = DRC_ITEM::Create( aErrorCode ); + + if( !aMsg.IsEmpty() ) + drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + aMsg ); - drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + aMsg ); drcItem->SetItems( aItemA, aItemB ); PCB_MARKER* marker = new PCB_MARKER( drcItem, aPt ); @@ -114,37 +118,32 @@ void DIALOG_FOOTPRINT_CHECKER::runChecks() m_frame->GetCanvas()->GetView()->Add( marker ); }; - footprint->BuildPolyCourtyards( &errorHandler ); - - - const std::function typeWarning = - [&]( const wxString& aMsg ) + OUTLINE_ERROR_HANDLER outlineErrorHandler = + [&]( const wxString& aMsg, BOARD_ITEM* aItemA, BOARD_ITEM* aItemB, const VECTOR2I& aPt ) { - std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_FOOTPRINT_TYPE_MISMATCH ); - - drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + aMsg ); - drcItem->SetItems( footprint ); - - PCB_MARKER* marker = new PCB_MARKER( drcItem, wxPoint( 0, 0 ) ); - board->Add( marker ); - m_frame->GetCanvas()->GetView()->Add( marker ); + errorHandler( aItemA, aItemB, DRCE_MALFORMED_COURTYARD, aMsg, aPt ); }; - const std::function tstHoleInTHPad = - [&]( const wxString& aMsg, const VECTOR2I& aPosition ) + footprint->BuildPolyCourtyards( &outlineErrorHandler ); + + footprint->CheckFootprintAttributes( + [&]( const wxString& msg ) { - std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_PAD_TH_WITH_NO_HOLE ); + errorHandler( footprint, nullptr, DRCE_FOOTPRINT_TYPE_MISMATCH, msg, { 0, 0 } ); + } ); - drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + aMsg ); - drcItem->SetItems( footprint ); + footprint->CheckPads( + [&]( const PAD* aPad, int aErrorCode, const wxString& aMsg ) + { + errorHandler( aPad, nullptr, aErrorCode, aMsg, aPad->GetPosition() ); + } ); - PCB_MARKER* marker = new PCB_MARKER( drcItem, aPosition ); - board->Add( marker ); - m_frame->GetCanvas()->GetView()->Add( marker ); - }; + footprint->CheckOverlappingPads( + [&]( const PAD* aPadA, const PAD* aPadB, const VECTOR2I& aPosition ) + { + errorHandler( aPadA, aPadB, DRCE_OVERLAPPING_PADS, wxEmptyString, aPosition ); + } ); - footprint->CheckFootprintAttributes( &typeWarning ); - footprint->CheckFootprintTHPadNoHoles( &tstHoleInTHPad ); m_checksRun = true; SetMarkersProvider( new DRC_ITEMS_PROVIDER( board, MARKER_BASE::MARKER_DRC ) ); @@ -178,7 +177,7 @@ void DIALOG_FOOTPRINT_CHECKER::OnSelectItem( wxDataViewEvent& aEvent ) if( node && item ) { - PCB_LAYER_ID principalLayer = item->GetLayer(); + PCB_LAYER_ID principalLayer = item->GetLayerSet().Seq()[0]; LSET violationLayers; std::shared_ptr rc_item = node->m_RcItem; diff --git a/pcbnew/drc/drc_item.cpp b/pcbnew/drc/drc_item.cpp index d3455b97ab..57c10098d4 100644 --- a/pcbnew/drc/drc_item.cpp +++ b/pcbnew/drc/drc_item.cpp @@ -248,6 +248,10 @@ DRC_ITEM DRC_ITEM::footprintTHPadhasNoHole( DRCE_PAD_TH_WITH_NO_HOLE, _( "Through hole pad has no hole" ), wxT( "through_hole_pad_without_hole" ) ); +DRC_ITEM DRC_ITEM::footprintOverlappingPads( DRCE_OVERLAPPING_PADS, + _( "Pads with different numbers overlap" ), + wxT( "overlapping_pads" ) ); + std::vector> DRC_ITEM::allItemTypes( { DRC_ITEM::heading_electrical, @@ -307,7 +311,8 @@ std::vector> DRC_ITEM::allItemTypes( { DRC_ITEM::footprintTypeMismatch, DRC_ITEM::libFootprintIssues, DRC_ITEM::libFootprintMismatch, - DRC_ITEM::footprintTHPadhasNoHole + DRC_ITEM::footprintTHPadhasNoHole, + DRC_ITEM::footprintOverlappingPads } ); @@ -364,7 +369,8 @@ std::shared_ptr DRC_ITEM::Create( int aErrorCode ) case DRCE_DIFF_PAIR_GAP_OUT_OF_RANGE: return std::make_shared( diffPairGapOutOfRange ); case DRCE_DIFF_PAIR_UNCOUPLED_LENGTH_TOO_LONG: return std::make_shared( diffPairUncoupledLengthTooLong ); case DRCE_FOOTPRINT_TYPE_MISMATCH: return std::make_shared( footprintTypeMismatch ); - case DRCE_PAD_TH_WITH_NO_HOLE: return std::make_shared( footprintTHPadhasNoHole ); + case DRCE_PAD_TH_WITH_NO_HOLE: return std::make_shared( footprintTHPadhasNoHole ); + case DRCE_OVERLAPPING_PADS: return std::make_shared( footprintOverlappingPads ); default: wxFAIL_MSG( wxT( "Unknown DRC error code" ) ); return nullptr; diff --git a/pcbnew/drc/drc_item.h b/pcbnew/drc/drc_item.h index ca6ab2e037..e1a1065185 100644 --- a/pcbnew/drc/drc_item.h +++ b/pcbnew/drc/drc_item.h @@ -75,6 +75,7 @@ enum PCB_DRC_CODE { DRCE_LIB_FOOTPRINT_ISSUES, // footprint not found in active libraries DRCE_LIB_FOOTPRINT_MISMATCH, // footprint does not match the current library DRCE_PAD_TH_WITH_NO_HOLE, // footprint has Plated Through-Hole with no hole + DRCE_OVERLAPPING_PADS, // footprint with overlapping pads DRCE_UNRESOLVED_VARIABLE, DRCE_ASSERTION_FAILURE, // user-defined (custom rule) assertion @@ -199,6 +200,7 @@ private: static DRC_ITEM diffPairUncoupledLengthTooLong; static DRC_ITEM footprintTypeMismatch; static DRC_ITEM footprintTHPadhasNoHole; + static DRC_ITEM footprintOverlappingPads; private: DRC_RULE* m_violatingRule = nullptr; diff --git a/pcbnew/drc/drc_test_provider_hole_size.cpp b/pcbnew/drc/drc_test_provider_hole_size.cpp index faad8518d7..a53f5f8a70 100644 --- a/pcbnew/drc/drc_test_provider_hole_size.cpp +++ b/pcbnew/drc/drc_test_provider_hole_size.cpp @@ -30,7 +30,6 @@ #include #include #include -#include "convert_basic_shapes_to_polygon.h" /* Drilled hole size test. scans vias/through-hole pads and checks for min drill sizes @@ -66,7 +65,6 @@ public: private: void checkViaHole( PCB_VIA* via, bool aExceedMicro, bool aExceedStd ); void checkPadHole( PAD* aPad ); - void checkPadStack( PAD* aPad ); }; @@ -79,23 +77,32 @@ bool DRC_TEST_PROVIDER_HOLE_SIZE::Run() for( FOOTPRINT* footprint : m_drcEngine->GetBoard()->Footprints() ) { - if( m_drcEngine->IsErrorLimitExceeded( DRCE_DRILL_OUT_OF_RANGE ) - && m_drcEngine->IsErrorLimitExceeded( DRCE_PADSTACK ) ) - { - break; - } - for( PAD* pad : footprint->Pads() ) { if( !m_drcEngine->IsErrorLimitExceeded( DRCE_DRILL_OUT_OF_RANGE ) ) checkPadHole( pad ); - - if( !m_drcEngine->IsErrorLimitExceeded( DRCE_PADSTACK ) ) - checkPadStack( pad ); } } } + for( FOOTPRINT* footprint : m_drcEngine->GetBoard()->Footprints() ) + { + footprint->CheckPads( + [&]( const PAD* pad, int errorCode, const wxString& msg ) + { + if( m_drcEngine->IsErrorLimitExceeded( errorCode ) ) + return; + + std::shared_ptr drcItem = DRC_ITEM::Create( errorCode ); + + if( !msg.IsEmpty() ) + drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + msg ); + + drcItem->SetItems( pad ); + reportViolation( drcItem, pad->GetPosition(), UNDEFINED_LAYER ); + } ); + } + if( !m_drcEngine->IsErrorLimitExceeded( DRCE_MICROVIA_DRILL_OUT_OF_RANGE ) || !m_drcEngine->IsErrorLimitExceeded( DRCE_DRILL_OUT_OF_RANGE ) ) { @@ -130,53 +137,6 @@ bool DRC_TEST_PROVIDER_HOLE_SIZE::Run() return !m_drcEngine->IsCancelled(); } -void DRC_TEST_PROVIDER_HOLE_SIZE::checkPadStack( PAD* aPad ) -{ - if( aPad->GetAttribute() == PAD_ATTRIB::PTH ) - { - if( !aPad->IsOnCopperLayer() ) - { - std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_PADSTACK ); - m_msg.Printf( _( " (PTH pad has no copper layers)" ) ); - - drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + m_msg ); - drcItem->SetItems( aPad ); - - reportViolation( drcItem, aPad->GetPosition(), UNDEFINED_LAYER ); - } - else - { - LSET lset = aPad->GetLayerSet() & LSET::AllCuMask(); - PCB_LAYER_ID layer = lset.Seq().at( 0 ); - int maxError = m_drcEngine->GetBoard()->GetDesignSettings().m_MaxError; - SHAPE_POLY_SET padOutline; - - aPad->TransformShapeWithClearanceToPolygon( padOutline, layer, 0, maxError, - ERROR_LOC::ERROR_INSIDE ); - - const SHAPE_SEGMENT* drillShape = aPad->GetEffectiveHoleShape(); - const SEG drillSeg = drillShape->GetSeg(); - SHAPE_POLY_SET drillOutline; - - TransformOvalToPolygon( drillOutline, drillSeg.A, drillSeg.B, - drillShape->GetWidth(), maxError, ERROR_LOC::ERROR_INSIDE ); - - padOutline.BooleanSubtract( drillOutline, SHAPE_POLY_SET::POLYGON_MODE::PM_FAST ); - - if( padOutline.IsEmpty() ) - { - std::shared_ptr drcItem = DRC_ITEM::Create( DRCE_PADSTACK ); - m_msg.Printf( _( " (PTH pad's hole leaves no copper)" ) ); - - drcItem->SetErrorMessage( drcItem->GetErrorText() + wxS( " " ) + m_msg ); - drcItem->SetItems( aPad ); - - reportViolation( drcItem, aPad->GetPosition(), UNDEFINED_LAYER ); - } - } - } -} - void DRC_TEST_PROVIDER_HOLE_SIZE::checkPadHole( PAD* aPad ) { int holeMinor = std::min( aPad->GetDrillSize().x, aPad->GetDrillSize().y ); diff --git a/pcbnew/footprint.cpp b/pcbnew/footprint.cpp index ae6ab9f08b..f55eab7810 100644 --- a/pcbnew/footprint.cpp +++ b/pcbnew/footprint.cpp @@ -45,9 +45,12 @@ #include #include #include +#include +#include #include #include #include "fp_textbox.h" +#include "convert_basic_shapes_to_polygon.h" FOOTPRINT::FOOTPRINT( BOARD* parent ) : BOARD_ITEM_CONTAINER((BOARD_ITEM*) parent, PCB_FOOTPRINT_T ), @@ -2179,11 +2182,10 @@ void FOOTPRINT::BuildPolyCourtyards( OUTLINE_ERROR_HANDLER* aErrorHandler ) } -void FOOTPRINT::CheckFootprintAttributes( const std::function* aErrorHandler ) +void FOOTPRINT::CheckFootprintAttributes( const std::function& aErrorHandler ) { - - int likelyAttr = GetLikelyAttribute(); - int setAttr = ( GetAttributes() & ( FP_SMD | FP_THROUGH_HOLE ) ); + int likelyAttr = GetLikelyAttribute(); + int setAttr = ( GetAttributes() & ( FP_SMD | FP_THROUGH_HOLE ) ); // This is only valid if the footprint doesn't have FP_SMD and FP_THROUGH_HOLE set // Which is, unfortunately, possible in theory but not in the UI (I think) @@ -2206,29 +2208,94 @@ void FOOTPRINT::CheckFootprintAttributes( const std::function* aErrorHandler ) + +void FOOTPRINT::CheckPads( const std::function& aErrorHandler ) { if( aErrorHandler == nullptr ) return; - for( const PAD* pad: Pads() ) + for( PAD* pad: Pads() ) { - - if( pad->GetAttribute() != PAD_ATTRIB::PTH - && pad->GetAttribute() != PAD_ATTRIB::NPTH ) - continue; - - if( pad->GetDrillSizeX() < 1 || pad->GetDrillSizeY() < 1 ) + if( pad->GetAttribute() == PAD_ATTRIB::PTH || pad->GetAttribute() == PAD_ATTRIB::NPTH ) { - wxString msg; - msg.Printf( _( "(pad \"%s\")" ), pad->GetNumber() ); + if( pad->GetDrillSizeX() < 1 || pad->GetDrillSizeY() < 1 ) + (aErrorHandler)( pad, DRCE_PAD_TH_WITH_NO_HOLE, wxEmptyString ); + } - (*aErrorHandler)( msg, pad->GetPosition() ); + if( pad->GetAttribute() == PAD_ATTRIB::PTH ) + { + if( !pad->IsOnCopperLayer() ) + { + (aErrorHandler)( pad, DRCE_PADSTACK, _( "(PTH pad has no copper layers)" ) ); + } + else + { + LSET lset = pad->GetLayerSet() & LSET::AllCuMask(); + PCB_LAYER_ID layer = lset.Seq().at( 0 ); + SHAPE_POLY_SET padOutline; + + pad->TransformShapeWithClearanceToPolygon( padOutline, layer, 0, ARC_HIGH_DEF, + ERROR_LOC::ERROR_INSIDE ); + + const SHAPE_SEGMENT* drillShape = pad->GetEffectiveHoleShape(); + const SEG drillSeg = drillShape->GetSeg(); + SHAPE_POLY_SET drillOutline; + + TransformOvalToPolygon( drillOutline, drillSeg.A, drillSeg.B, + drillShape->GetWidth(), ARC_HIGH_DEF, + ERROR_LOC::ERROR_INSIDE ); + + padOutline.BooleanSubtract( drillOutline, SHAPE_POLY_SET::POLYGON_MODE::PM_FAST ); + + if( padOutline.IsEmpty() ) + { + (aErrorHandler)( pad, DRCE_PADSTACK, _( "(PTH pad's hole leaves no copper)" ) ); + } + } + } + } +} + + +void FOOTPRINT::CheckOverlappingPads( const std::function& aErrorHandler ) +{ + std::map< std::pair, int> checkedPairs; + + for( PAD* pad : Pads() ) + { + for( PAD* other : Pads() ) + { + if( other == pad || pad->SameLogicalPadAs( other ) ) + continue; + + // store canonical order so we don't collide in both directions + // (a:b and b:a) + if( static_cast( pad ) > static_cast( other ) ) + std::swap( pad, other ); + + if( checkedPairs.count( { pad, other } ) ) + { + continue; + } + else + { + checkedPairs[ { pad, other } ] = 1; + } + + VECTOR2I pos; + SHAPE* padShape = pad->GetEffectiveShape().get(); + SHAPE* otherShape = other->GetEffectiveShape().get(); + + if( padShape->Collide( otherShape, 0, nullptr, &pos ) ) + { + (aErrorHandler)( pad, other, pos ); + } } } } diff --git a/pcbnew/footprint.h b/pcbnew/footprint.h index 6b162a0b79..66e33a3409 100644 --- a/pcbnew/footprint.h +++ b/pcbnew/footprint.h @@ -339,27 +339,24 @@ public: /** * Test if footprint attributes for type (SMD/Through hole/Other) match the expected * type based on the pads in the footprint. - * Footprints with plated through-hole pads should usually be marked through hole even if they also - * have SMD because they might not be auto-placed. Exceptions to this might be shielded connectors - * Otherwise, footprints with SMD pads should be marked SMD + * Footprints with plated through-hole pads should usually be marked through hole even if they + * also have SMD because they might not be auto-placed. Exceptions to this might be shielded + * connectors. Otherwise, footprints with SMD pads should be marked SMD. * Footprints with no connecting pads should be marked "Other" * * @param aErrorHandler callback to handle the error messages generated */ - void CheckFootprintAttributes( const std::function* aErrorHandler ); + void CheckFootprintAttributes( const std::function& aErrorHandler ); /** - * Test if footprint attributes for type (SMD/Through hole/Other) match the expected - * type based on the pads in the footprint. - * Footprints with plated through-hole pads should usually be marked through hole even if they also - * have SMD because they might not be auto-placed. Exceptions to this might be shielded connectors - * Otherwise, footprints with SMD pads should be marked SMD - * Footprints with no connecting pads should be marked "Other" + * Run DRC checks on footprint's pads. * * @param aErrorHandler callback to handle the error messages generated */ - void CheckFootprintTHPadNoHoles( const std::function* - aErrorHandler ); + void CheckPads( const std::function& aErrorHandler ); + + void CheckOverlappingPads( const std::function& aErrorHandler ); /** * Generate pads shapes on layer \a aLayer as polygons and adds these polygons to diff --git a/pcbnew/pcb_base_frame.cpp b/pcbnew/pcb_base_frame.cpp index ebebfe2748..f4ecb3078b 100644 --- a/pcbnew/pcb_base_frame.cpp +++ b/pcbnew/pcb_base_frame.cpp @@ -335,7 +335,7 @@ void PCB_BASE_FRAME::FocusOnItem( BOARD_ITEM* aItem, PCB_LAYER_ID aLayer ) SHAPE_POLY_SET itemPoly, clippedPoly; if( aLayer == UNDEFINED_LAYER ) - aLayer = aItem->GetLayer(); + aLayer = aItem->GetLayerSet().Seq()[0]; switch( aItem->Type() ) {