From 3e7310a2198ce69776e9b662efecef1b84f827f9 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Sat, 25 Sep 2021 08:55:33 -0700 Subject: [PATCH] Move FP type check to FP DRC Checks for validity are better centrally located where they don't interrupt user actions --- pcbnew/dialogs/dialog_footprint_checker.cpp | 18 ++++++++++-- pcbnew/drc/drc_item.cpp | 7 +++++ pcbnew/drc/drc_item.h | 3 ++ pcbnew/footprint.cpp | 32 +++++++++++++++++++++ pcbnew/footprint.h | 12 ++++++++ pcbnew/footprint_libraries_utils.cpp | 32 --------------------- 6 files changed, 70 insertions(+), 34 deletions(-) diff --git a/pcbnew/dialogs/dialog_footprint_checker.cpp b/pcbnew/dialogs/dialog_footprint_checker.cpp index 5abd29829c..18e82ff614 100644 --- a/pcbnew/dialogs/dialog_footprint_checker.cpp +++ b/pcbnew/dialogs/dialog_footprint_checker.cpp @@ -51,6 +51,7 @@ DIALOG_FOOTPRINT_CHECKER::DIALOG_FOOTPRINT_CHECKER( FOOTPRINT_EDIT_FRAME* aParen m_sdbSizerCancel->SetLabel( _( "Close" ) ); m_sdbSizerOK->SetDefault(); + m_sdbSizer->Layout(); syncCheckboxes(); @@ -66,8 +67,6 @@ DIALOG_FOOTPRINT_CHECKER::~DIALOG_FOOTPRINT_CHECKER() bool DIALOG_FOOTPRINT_CHECKER::TransferDataToWindow() { - runChecks(); - return true; } @@ -120,6 +119,21 @@ void DIALOG_FOOTPRINT_CHECKER::runChecks() footprint->BuildPolyCourtyards( &errorHandler ); + + const std::function typeWarning = + [&]( const wxString& aMsg ) + { + 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 ); + }; + + footprint->CheckFootprintAttributes( &typeWarning ); m_checksRun = true; SetMarkersProvider( new BOARD_DRC_ITEMS_PROVIDER( m_frame->GetBoard() ) ); diff --git a/pcbnew/drc/drc_item.cpp b/pcbnew/drc/drc_item.cpp index 8e12e21330..ad0a63470a 100644 --- a/pcbnew/drc/drc_item.cpp +++ b/pcbnew/drc/drc_item.cpp @@ -202,6 +202,10 @@ DRC_ITEM DRC_ITEM::diffPairUncoupledLengthTooLong( DRCE_DIFF_PAIR_UNCOUPLED_LENG _( "Differential uncoupled length too long" ), wxT( "diff_pair_uncoupled_length_too_long" ) ); +DRC_ITEM DRC_ITEM::footprintTypeMismatch( DRCE_FOOTPRINT_TYPE_MISMATCH, + _( "Footprint type doesn't match footprint pads" ), + wxT( "footprint_type_mismatch" ) ); + std::vector> DRC_ITEM::allItemTypes( { DRC_ITEM::heading_electrical, DRC_ITEM::shortingItems, @@ -248,6 +252,8 @@ std::vector> DRC_ITEM::allItemTypes( { DRC_ITEM::npthInsideCourtyard, DRC_ITEM::itemOnDisabledLayer, DRC_ITEM::unresolvedVariable, + + DRC_ITEM::footprintTypeMismatch } ); @@ -294,6 +300,7 @@ std::shared_ptr DRC_ITEM::Create( int aErrorCode ) case DRCE_TOO_MANY_VIAS: return std::make_shared( tooManyVias ); 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 ); default: wxFAIL_MSG( "Unknown DRC error code" ); return nullptr; diff --git a/pcbnew/drc/drc_item.h b/pcbnew/drc/drc_item.h index 3c67ec4714..039b09f25a 100644 --- a/pcbnew/drc/drc_item.h +++ b/pcbnew/drc/drc_item.h @@ -67,6 +67,8 @@ enum PCB_DRC_CODE { DRCE_EXTRA_FOOTPRINT, // netlist item not found for footprint DRCE_NET_CONFLICT, // pad net doesn't match netlist + DRCE_FOOTPRINT_TYPE_MISMATCH, // footprint attribute does not match actual pads + DRCE_UNRESOLVED_VARIABLE, DRCE_SILK_MASK_CLEARANCE, // silkscreen clipped by mask (potentially leaving it // over pads, exposed copper, etc.) @@ -169,6 +171,7 @@ private: static DRC_ITEM tooManyVias; static DRC_ITEM diffPairGapOutOfRange; static DRC_ITEM diffPairUncoupledLengthTooLong; + static DRC_ITEM footprintTypeMismatch; private: DRC_RULE* m_violatingRule = nullptr; diff --git a/pcbnew/footprint.cpp b/pcbnew/footprint.cpp index 0725e1f343..27c1e33dd6 100644 --- a/pcbnew/footprint.cpp +++ b/pcbnew/footprint.cpp @@ -2072,6 +2072,38 @@ void FOOTPRINT::BuildPolyCourtyards( OUTLINE_ERROR_HANDLER* aErrorHandler ) } +void FOOTPRINT::CheckFootprintAttributes( const std::function* aErrorHandler ) +{ + + 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) + if( aErrorHandler && likelyAttr != setAttr ) + { + wxString msg; + + if( likelyAttr == FP_THROUGH_HOLE ) + { + msg.Printf( _( "Expected \"Through hole\" type but set to \"%s\"" ), GetTypeName() ); + } + else if( likelyAttr == FP_SMD ) + { + msg.Printf( _( "Expected \"SMD\" type but set to \"%s\"" ), GetTypeName() ); + } + else + { + msg.Printf( _( "Expected \"Other\" type but set to \"%s\"" ), GetTypeName() ); + } + + msg = "(" + msg + ")"; + + (*aErrorHandler)( msg ); + } +} + + void FOOTPRINT::SwapData( BOARD_ITEM* aImage ) { wxASSERT( aImage->Type() == PCB_FOOTPRINT_T ); diff --git a/pcbnew/footprint.h b/pcbnew/footprint.h index 5292f538fe..144608f6d5 100644 --- a/pcbnew/footprint.h +++ b/pcbnew/footprint.h @@ -342,6 +342,18 @@ public: void SetLastEditTime() { m_lastEditTime = time( nullptr ); } timestamp_t GetLastEditTime() const { return m_lastEditTime; } + /** + * 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" + * + * @param aErrorHandler callback to handle the error messages generated + */ + void CheckFootprintAttributes( const std::function* aErrorHandler ); + /** * Generate pads shapes on layer \a aLayer as polygons and adds these polygons to * \a aCornerBuffer. diff --git a/pcbnew/footprint_libraries_utils.cpp b/pcbnew/footprint_libraries_utils.cpp index a6cb2d3f9f..8f3cf2dfb9 100644 --- a/pcbnew/footprint_libraries_utils.cpp +++ b/pcbnew/footprint_libraries_utils.cpp @@ -784,38 +784,6 @@ bool FOOTPRINT_EDIT_FRAME::SaveFootprint( FOOTPRINT* aFootprint ) wxString libraryName = aFootprint->GetFPID().GetLibNickname(); wxString footprintName = aFootprint->GetFPID().GetLibItemName(); bool nameChanged = m_footprintNameWhenLoaded != footprintName; - int likelyAttr = aFootprint->GetLikelyAttribute(); - int setAttr = ( aFootprint->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) - if( likelyAttr != setAttr ) - { - wxString msg; - - if( likelyAttr == FP_THROUGH_HOLE ) - { - msg.Printf( _( "Your footprint has plated through hole pads but " - "its type is set to \"%s\"" ), aFootprint->GetTypeName() ); - } - else if( likelyAttr == FP_SMD ) - { - msg.Printf( _( "Your footprint has SMD pads but " - "its type is set to \"%s\"" ), aFootprint->GetTypeName() ); - } - else - { - msg.Printf( _( "Your footprint has no SMD or plated through hole pads but " - "its type is set to \"%s\"" ), aFootprint->GetTypeName() ); - } - - KIDIALOG errorDlg( this, msg, _( "Confirmation" ), wxOK | wxCANCEL | wxICON_WARNING ); - errorDlg.SetOKLabel( _( "Continue" ) ); - errorDlg.DoNotShowCheckbox( __FILE__, __LINE__ ); - - if( errorDlg.ShowModal() == wxID_CANCEL ) - return false; - } if( aFootprint->GetLink() != niluuid ) {