Don't use FindNamedPad for net-tie logic. It only reutrns the *first*

pad of a given number.

Also improves other DRC logic to tighten up the net-tie rules now that
we know which pads are allowed to short with which other pads.

Also removes the "Overlapping pads" DRC violation now that we know
whether or not overlapping pads in a net-tie footprint constitute a
short.

Fixes https://gitlab.com/kicad/code/kicad/issues/12506
This commit is contained in:
Jeff Young 2022-09-25 17:23:14 +01:00
parent 05a8650158
commit 503385f52e
13 changed files with 151 additions and 163 deletions

View File

@ -168,7 +168,6 @@ BOARD_DESIGN_SETTINGS::BOARD_DESIGN_SETTINGS( JSON_SETTINGS* aParent, const std:
m_DRCSeverities[ DRCE_ISOLATED_COPPER ] = RPT_SEVERITY_WARNING;
m_DRCSeverities[ DRCE_PADSTACK ] = RPT_SEVERITY_WARNING;
m_DRCSeverities[ DRCE_OVERLAPPING_PADS ] = RPT_SEVERITY_WARNING;
m_DRCSeverities[ DRCE_MISSING_FOOTPRINT ] = RPT_SEVERITY_WARNING;
m_DRCSeverities[ DRCE_DUPLICATE_FOOTPRINT ] = RPT_SEVERITY_WARNING;

View File

@ -154,10 +154,10 @@ void DIALOG_FOOTPRINT_CHECKER::runChecks()
errorHandler( aPad, nullptr, nullptr, aErrorCode, aMsg, aPad->GetPosition() );
} );
footprint->CheckOverlappingPads(
footprint->CheckShortingPads(
[&]( const PAD* aPadA, const PAD* aPadB, const VECTOR2I& aPosition )
{
errorHandler( aPadA, aPadB, nullptr, DRCE_OVERLAPPING_PADS, wxEmptyString,
errorHandler( aPadA, aPadB, nullptr, DRCE_SHORTING_ITEMS, wxEmptyString,
aPosition );
} );

View File

@ -1757,6 +1757,10 @@ bool DRC_ENGINE::IsNetADiffPair( BOARD* aBoard, NETINFO_ITEM* aNet, int& aNetP,
}
/**
* Check if the given collision between a track and another item occurs during the track's entry
* into a net-tie pad.
*/
bool DRC_ENGINE::IsNetTieExclusion( int aTrackNetCode, PCB_LAYER_ID aTrackLayer,
const VECTOR2I& aCollisionPos, BOARD_ITEM* aCollidingItem )
{
@ -1764,13 +1768,13 @@ bool DRC_ENGINE::IsNetTieExclusion( int aTrackNetCode, PCB_LAYER_ID aTrackLayer,
if( parentFootprint && parentFootprint->IsNetTie() )
{
std::map<wxString, int> padToNetTieGroupMap = parentFootprint->MapPadNumbersToNetTieGroups();
for( PAD* pad : parentFootprint->Pads() )
{
if( aTrackNetCode == pad->GetNetCode() )
if( padToNetTieGroupMap[ pad->GetNumber() ] >= 0 && aTrackNetCode == pad->GetNetCode() )
{
std::shared_ptr<SHAPE> otherShape = pad->GetEffectiveShape( aTrackLayer );
if( otherShape->Collide( aCollisionPos, 0 ) )
if( pad->GetEffectiveShape( aTrackLayer )->Collide( aCollisionPos, 0 ) )
return true;
}
}

View File

@ -257,10 +257,6 @@ 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<std::reference_wrapper<RC_ITEM>> DRC_ITEM::allItemTypes( {
DRC_ITEM::heading_electrical,
@ -322,8 +318,7 @@ std::vector<std::reference_wrapper<RC_ITEM>> DRC_ITEM::allItemTypes( {
DRC_ITEM::footprintTypeMismatch,
DRC_ITEM::libFootprintIssues,
DRC_ITEM::libFootprintMismatch,
DRC_ITEM::footprintTHPadhasNoHole,
DRC_ITEM::footprintOverlappingPads
DRC_ITEM::footprintTHPadhasNoHole
} );
@ -383,7 +378,6 @@ std::shared_ptr<DRC_ITEM> DRC_ITEM::Create( int aErrorCode )
case DRCE_FOOTPRINT: return std::make_shared<DRC_ITEM>( footprint );
case DRCE_FOOTPRINT_TYPE_MISMATCH: return std::make_shared<DRC_ITEM>( footprintTypeMismatch );
case DRCE_PAD_TH_WITH_NO_HOLE: return std::make_shared<DRC_ITEM>( footprintTHPadhasNoHole );
case DRCE_OVERLAPPING_PADS: return std::make_shared<DRC_ITEM>( footprintOverlappingPads );
default:
wxFAIL_MSG( wxT( "Unknown DRC error code" ) );
return nullptr;

View File

@ -76,7 +76,6 @@ 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_FOOTPRINT, // error in footprint definition
DRCE_UNRESOLVED_VARIABLE,
@ -207,7 +206,6 @@ private:
static DRC_ITEM footprint;
static DRC_ITEM footprintTypeMismatch;
static DRC_ITEM footprintTHPadhasNoHole;
static DRC_ITEM footprintOverlappingPads;
private:
DRC_RULE* m_violatingRule = nullptr;

View File

@ -511,17 +511,26 @@ bool DRC_TEST_PROVIDER_COPPER_CLEARANCE::testPadAgainstItem( PAD* pad, SHAPE* pa
bool testShorting = !m_drcEngine->IsErrorLimitExceeded( DRCE_SHORTING_ITEMS );
bool testHoles = !m_drcEngine->IsErrorLimitExceeded( DRCE_HOLE_CLEARANCE );
// Disable some tests *within* a single footprint
// Disable some tests for net-tie objects in a footprint
if( other->GetParent() == pad->GetParent() )
{
FOOTPRINT* fp = static_cast<FOOTPRINT*>( pad->GetParent() );
FOOTPRINT* fp = static_cast<FOOTPRINT*>( pad->GetParent() );
std::map<wxString, int> padToNetTieGroupMap = fp->MapPadNumbersToNetTieGroups();
int padGroupIdx = padToNetTieGroupMap[ pad->GetNumber() ];
// Graphic items are allowed to act as net-ties within their own footprint
if( fp->IsNetTie() && ( other->Type() == PCB_FP_SHAPE_T || other->Type() == PCB_PAD_T ) )
if( other->Type() == PCB_PAD_T )
{
PAD* otherPad = static_cast<PAD*>( other );
if( padGroupIdx >= 0 && padGroupIdx == padToNetTieGroupMap[ otherPad->GetNumber() ] )
testClearance = false;
if( pad->SameLogicalPadAs( otherPad ) )
testHoles = false;
}
if( other->Type() == PCB_FP_SHAPE_T && padGroupIdx >= 0 )
testClearance = false;
if( other->Type() == PCB_PAD_T && pad->SameLogicalPadAs( static_cast<PAD*>( other ) ) )
testHoles = false;
}
PAD* otherPad = nullptr;

View File

@ -108,12 +108,12 @@ bool DRC_TEST_PROVIDER_FOOTPRINT_CHECKS::Run()
} );
}
if( !m_drcEngine->IsErrorLimitExceeded( DRCE_OVERLAPPING_PADS ) )
if( !m_drcEngine->IsErrorLimitExceeded( DRCE_SHORTING_ITEMS ) )
{
footprint->CheckOverlappingPads(
footprint->CheckShortingPads(
[&]( const PAD* aPadA, const PAD* aPadB, const VECTOR2I& aPosition )
{
errorHandler( aPadA, aPadB, nullptr, DRCE_OVERLAPPING_PADS, wxEmptyString,
errorHandler( aPadA, aPadB, nullptr, DRCE_SHORTING_ITEMS, wxEmptyString,
aPosition, aPadA->GetPrincipalLayer() );
} );
}

View File

@ -386,18 +386,10 @@ bool DRC_TEST_PROVIDER_SOLDER_MASK::checkItemMask( BOARD_ITEM* aMaskItem, int aT
// footprint. They must be allowed to intrude into their pad's mask aperture.
if( aTestNet < 0 && aMaskItem->Type() == PCB_PAD_T && fp->IsNetTie() )
{
wxString padNumber = static_cast<PAD*>( aMaskItem )->GetNumber();
std::map<wxString, int> padNumberToGroupIdxMap = fp->MapPadNumbersToNetTieGroups();
for( const wxString& group : fp->GetNetTiePadGroups() )
{
wxStringTokenizer groupParser( group, "," );
while( groupParser.HasMoreTokens() )
{
if( groupParser.GetNextToken().Trim( false ).Trim( true ) == padNumber )
return false;
}
}
if( padNumberToGroupIdxMap[ static_cast<PAD*>( aMaskItem )->GetNumber() ] >= 0 )
return false;
}
return true;

View File

@ -43,7 +43,6 @@
#include <footprint.h>
#include <zone.h>
#include <view/view.h>
#include <geometry/shape_null.h>
#include <i18n_utility.h>
#include <drc/drc_item.h>
#include <geometry/shape_segment.h>
@ -1185,25 +1184,6 @@ PAD* FOOTPRINT::GetPad( const VECTOR2I& aPosition, LSET aLayerMask )
}
PAD* FOOTPRINT::GetTopLeftPad()
{
PAD* topLeftPad = m_pads.front();
for( PAD* p : m_pads )
{
VECTOR2I pnt = p->GetPosition(); // GetPosition() returns the center of the pad
if( ( pnt.x < topLeftPad->GetPosition().x ) ||
( topLeftPad->GetPosition().x == pnt.x && pnt.y < topLeftPad->GetPosition().y ) )
{
topLeftPad = p;
}
}
return topLeftPad;
}
unsigned FOOTPRINT::GetPadCount( INCLUDE_NPTH_T aIncludeNPTH ) const
{
if( aIncludeNPTH )
@ -1399,33 +1379,6 @@ void FOOTPRINT::RunOnChildren( const std::function<void ( BOARD_ITEM*)>& aFuncti
}
void FOOTPRINT::GetAllDrawingLayers( int aLayers[], int& aCount, bool aIncludePads ) const
{
std::unordered_set<int> layers;
for( BOARD_ITEM* item : m_drawings )
layers.insert( static_cast<int>( item->GetLayer() ) );
if( aIncludePads )
{
for( PAD* pad : m_pads )
{
int pad_layers[KIGFX::VIEW::VIEW_MAX_LAYERS], pad_layers_count;
pad->ViewGetLayers( pad_layers, pad_layers_count );
for( int i = 0; i < pad_layers_count; i++ )
layers.insert( pad_layers[i] );
}
}
aCount = layers.size();
int i = 0;
for( int layer : layers )
aLayers[i++] = layer;
}
void FOOTPRINT::ViewGetLayers( int aLayers[], int& aCount ) const
{
aCount = 2;
@ -2296,6 +2249,48 @@ void FOOTPRINT::BuildCourtyardCaches( OUTLINE_ERROR_HANDLER* aErrorHandler )
}
std::map<wxString, int> FOOTPRINT::MapPadNumbersToNetTieGroups() const
{
std::map<wxString, int> padNumberToGroupIdxMap;
for( const PAD* pad : m_pads )
padNumberToGroupIdxMap[ pad->GetNumber() ] = -1;
for( size_t ii = 0; ii < m_netTiePadGroups.size(); ++ii )
{
wxStringTokenizer groupParser( m_netTiePadGroups[ ii ], "," );
std::vector<wxString> numbersInGroup;
while( groupParser.HasMoreTokens() )
padNumberToGroupIdxMap[ groupParser.GetNextToken().Trim( false ).Trim( true ) ] = ii;
}
return padNumberToGroupIdxMap;
}
std::vector<PAD*> FOOTPRINT::GetNetTiePads( PAD* aPad ) const
{
// First build a map from pad numbers to allowed-shorting-group indexes. This ends up being
// something like O(3n), but it still beats O(n^2) for large numbers of pads.
std::map<wxString, int> padToNetTieGroupMap = MapPadNumbersToNetTieGroups();
int groupIdx = padToNetTieGroupMap[ aPad->GetNumber() ];
std::vector<PAD*> otherPads;
if( groupIdx >= 0 )
{
for( PAD* pad : m_pads )
{
if( padToNetTieGroupMap[ pad->GetNumber() ] == groupIdx )
otherPads.push_back( pad );
}
}
return otherPads;
}
void FOOTPRINT::CheckFootprintAttributes( const std::function<void( const wxString& )>& aErrorHandler )
{
int likelyAttr = GetLikelyAttribute();
@ -2405,19 +2400,24 @@ void FOOTPRINT::CheckPads( const std::function<void( const PAD*, int,
}
void FOOTPRINT::CheckOverlappingPads( const std::function<void( const PAD*, const PAD*,
const VECTOR2I& )>& aErrorHandler )
void FOOTPRINT::CheckShortingPads( const std::function<void( const PAD*, const PAD*,
const VECTOR2I& )>& aErrorHandler )
{
std::unordered_map<PTR_PTR_CACHE_KEY, int> checkedPairs;
for( PAD* pad : Pads() )
{
std::vector<PAD*> netTiePads = GetNetTiePads( pad );
for( PAD* other : Pads() )
{
if( other == pad || pad->SameLogicalPadAs( other ) )
continue;
if( !( pad->GetLayerSet() & other->GetLayerSet() ).any() )
if( alg::contains( netTiePads, other ) )
continue;
if( !( ( pad->GetLayerSet() & other->GetLayerSet() ) & LSET::AllCuMask() ).any() )
continue;
// store canonical order so we don't collide in both directions (a:b and b:a)
@ -2451,26 +2451,10 @@ void FOOTPRINT::CheckNetTies( const std::function<void( const BOARD_ITEM* aItem,
const BOARD_ITEM* cItem,
const VECTOR2I& )>& aErrorHandler )
{
// First build a map from pads to allowed-shorting-group indexes. This ends up being
// First build a map from pad numbers to allowed-shorting-group indexes. This ends up being
// something like O(3n), but it still beats O(n^2) for large numbers of pads.
std::unordered_map<const PAD*, int> padToGroupIdxMap;
for( const PAD* pad : m_pads )
padToGroupIdxMap[ pad ] = -1;
for( size_t ii = 0; ii < m_netTiePadGroups.size(); ++ii )
{
wxStringTokenizer groupParser( m_netTiePadGroups[ ii ], "," );
while( groupParser.HasMoreTokens() )
{
PAD* pad = FindPadByNumber( groupParser.GetNextToken().Trim( false ).Trim( true ) );
if( pad )
padToGroupIdxMap[ pad ] = ii;
}
}
std::map<wxString, int> padNumberToGroupIdxMap = MapPadNumbersToNetTieGroups();
// Now collect all the footprint items which are on copper layers
@ -2533,12 +2517,12 @@ void FOOTPRINT::CheckNetTies( const std::function<void( const BOARD_ITEM* aItem,
if( pads.size() > 1 )
{
const PAD* firstPad = pads[0];
int firstGroupIdx = padToGroupIdxMap[ firstPad ];
int firstGroupIdx = padNumberToGroupIdxMap[ firstPad->GetNumber() ];
for( size_t ii = 1; ii < pads.size(); ++ii )
{
const PAD* thisPad = pads[ii];
int thisGroupIdx = padToGroupIdxMap[ thisPad ];
int thisGroupIdx = padNumberToGroupIdxMap[ thisPad->GetNumber() ];
if( thisGroupIdx < 0 || thisGroupIdx != firstGroupIdx )
{
@ -2571,6 +2555,7 @@ void FOOTPRINT::CheckNetTies( const std::function<void( const BOARD_ITEM* aItem,
void FOOTPRINT::CheckNetTiePadGroups( const std::function<void( const wxString& )>& aErrorHandler )
{
std::set<wxString> padNumbers;
wxString msg;
for( size_t ii = 0; ii < m_netTiePadGroups.size(); ++ii )
{
@ -2583,15 +2568,13 @@ void FOOTPRINT::CheckNetTiePadGroups( const std::function<void( const wxString&
if( !pad )
{
aErrorHandler( wxString::Format( _( "(net-tie pad group contains unknown pad "
"number %s)" ),
padNumber ) );
msg.Printf( _( "(net-tie pad group contains unknown pad number %s)" ), padNumber );
aErrorHandler( msg );
}
else if( !padNumbers.insert( padNumber ).second )
else if( !padNumbers.insert( pad->GetNumber() ).second )
{
aErrorHandler( wxString::Format( _( "(pad %s appears in more than one net-tie "
"pad group)" ),
padNumber ) );
msg.Printf( _( "(pad %s appears in more than one net-tie pad group)" ), padNumber );
aErrorHandler( msg );
}
}
}

View File

@ -261,8 +261,8 @@ public:
}
/**
* Return a list of pad groups, each of which is allowed to short nets within their group.
* A pad group is a comma-separated list of pad numbers.
* @return a list of pad groups, each of which is allowed to short nets within their group.
* A pad group is a comma-separated list of pad numbers.
*/
const std::vector<wxString>& GetNetTiePadGroups() const { return m_netTiePadGroups; }
@ -276,6 +276,17 @@ public:
m_netTiePadGroups.emplace_back( aGroup );
}
/**
* @return a map from pad numbers to net-tie group indicies. If a pad is not a member of
* a net-tie group its index will be -1.
*/
std::map<wxString, int> MapPadNumbersToNetTieGroups() const;
/**
* @return a list of pads that appear in \a aPad's net-tie pad group.
*/
std::vector<PAD*> GetNetTiePads( PAD* aPad ) const;
/**
* Returns the most likely attribute based on pads
* Either FP_THROUGH_HOLE/FP_SMD/OTHER(0)
@ -385,13 +396,13 @@ public:
void CheckPads( const std::function<void( const PAD*, int, const wxString& )>& aErrorHandler );
/**
* Check for overlapping, different-numbered pads.
* Check for overlapping, different-numbered, non-net-tie pads.
*
* @param aErrorHandler callback to handle the error messages generated
*/
void CheckOverlappingPads( const std::function<void( const PAD*,
const PAD*,
const VECTOR2I& )>& aErrorHandler );
void CheckShortingPads( const std::function<void( const PAD*,
const PAD*,
const VECTOR2I& )>& aErrorHandler );
/**
* Check for un-allowed shorting of pads in net-tie footprints. If two pads are shorted,
@ -577,8 +588,6 @@ public:
*/
PAD* GetPad( const VECTOR2I& aPosition, LSET aLayerMask = LSET::AllLayersMask() );
PAD* GetTopLeftPad();
/**
* Return the number of pads.
*
@ -659,15 +668,6 @@ public:
*/
void RunOnChildren( const std::function<void (BOARD_ITEM*)>& aFunction ) const;
/**
* Return a set of all layers that this footprint has drawings on similar to ViewGetLayers().
*
* @param aLayers is an array to store layer ids.
* @param aCount is the number of layers stored in the array.
* @param aIncludePads controls whether to also include pad layers.
*/
void GetAllDrawingLayers( int aLayers[], int& aCount, bool aIncludePads = true ) const;
virtual void ViewGetLayers( int aLayers[], int& aCount ) const override;
double ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const override;

View File

@ -351,30 +351,13 @@ void GRAPHICS_CLEANER::mergePads()
{
wxCHECK_MSG( m_parentFootprint, /*void*/, wxT( "mergePads() is FootprintEditor only" ) );
PAD_TOOL* padTool = m_toolMgr->GetTool<PAD_TOOL>();
for( PAD* pad : m_parentFootprint->Pads() )
pad->SetFlags( CANDIDATE );
if( m_parentFootprint->IsNetTie() )
{
for( const wxString& group : m_parentFootprint->GetNetTiePadGroups() )
{
wxStringTokenizer groupParser( group, "," );
while( groupParser.HasMoreTokens() )
{
wxString number = groupParser.GetNextToken().Trim( false ).Trim( true );
if( PAD* pad = m_parentFootprint->FindPadByNumber( number ) )
pad->ClearFlags( CANDIDATE );
}
}
}
PAD_TOOL* padTool = m_toolMgr->GetTool<PAD_TOOL>();
std::map<wxString, int> padToNetTieGroupMap = m_parentFootprint->MapPadNumbersToNetTieGroups();
for( PAD* pad : m_parentFootprint->Pads() )
{
if( !( pad->GetFlags() & CANDIDATE ) )
// Don't merge a pad that's in a net-tie pad group. (We don't care which group.)
if( padToNetTieGroupMap[ pad->GetNumber() ] >= 0 )
continue;
std::vector<FP_SHAPE*> shapes = padTool->RecombinePad( pad, m_dryRun, m_commit );

View File

@ -874,33 +874,60 @@ void ZONE_FILLER::buildCopperItemClearances( const ZONE* aZone, PCB_LAYER_ID aLa
for( FOOTPRINT* footprint : m_board->Footprints() )
{
bool skipFootprint = false;
knockoutGraphicClearance( &footprint->Reference() );
knockoutGraphicClearance( &footprint->Value() );
// Don't knock out holes in zones that share a net with a nettie footprint
std::set<PAD*> allowedNetTiePads;
// Don't knock out holes for graphic items which implement a net-tie to the zone's net
// on the layer being filled.
if( footprint->IsNetTie() )
{
for( PAD* pad : footprint->Pads() )
{
if( aZone->GetNetCode() == pad->GetNetCode() )
if( pad->GetNetCode() == aZone->GetNetCode() )
{
skipFootprint = true;
break;
if( pad->IsOnLayer( aLayer ) )
allowedNetTiePads.insert( pad );
for( PAD* other : footprint->GetNetTiePads( pad ) )
{
if( other->IsOnLayer( aLayer ) )
allowedNetTiePads.insert( other );
}
}
}
}
if( skipFootprint )
continue;
for( BOARD_ITEM* item : footprint->GraphicalItems() )
{
if( checkForCancel( m_progressReporter ) )
return;
knockoutGraphicClearance( item );
BOX2I itemBBox = item->GetBoundingBox();
if( !zone_boundingbox.Intersects( itemBBox ) )
continue;
bool skipItem = false;
if( item->IsOnLayer( aLayer ) )
{
std::shared_ptr<SHAPE> itemShape = item->GetEffectiveShape();
for( PAD* pad : allowedNetTiePads )
{
if( pad->GetBoundingBox().Intersects( itemBBox )
&& pad->GetEffectiveShape()->Collide( itemShape.get() ) )
{
skipItem = true;
break;
}
}
}
if( !skipItem )
knockoutGraphicClearance( item );
}
}

View File

@ -60,7 +60,6 @@ BOOST_FIXTURE_TEST_CASE( DRCCustomRuleSeverityTest, DRC_REGRESSION_TEST_FIXTURE
bds.m_DRCSeverities[ DRCE_LIB_FOOTPRINT_MISMATCH ] = SEVERITY::RPT_SEVERITY_IGNORE;
// Also disable a couple of footprint checks which throw up errors on this board
bds.m_DRCSeverities[ DRCE_OVERLAPPING_PADS ] = SEVERITY::RPT_SEVERITY_IGNORE;
bds.m_DRCSeverities[ DRCE_FOOTPRINT_TYPE_MISMATCH ] = SEVERITY::RPT_SEVERITY_IGNORE;
bds.m_DRCEngine->SetViolationHandler(