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

@@ -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<void( const wxString& msg )> typeWarning =
+            [&]( const wxString& aMsg )
+            {
+                std::shared_ptr<DRC_ITEM> 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() ) );

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<std::reference_wrapper<RC_ITEM>> DRC_ITEM::allItemTypes( {
             DRC_ITEM::heading_electrical,
             DRC_ITEM::shortingItems,
@@ -248,6 +252,8 @@ std::vector<std::reference_wrapper<RC_ITEM>> DRC_ITEM::allItemTypes( {
             DRC_ITEM::npthInsideCourtyard,
             DRC_ITEM::itemOnDisabledLayer,
             DRC_ITEM::unresolvedVariable,
+
+            DRC_ITEM::footprintTypeMismatch
         } );
 
 
@@ -294,6 +300,7 @@ std::shared_ptr<DRC_ITEM> DRC_ITEM::Create( int aErrorCode )
     case DRCE_TOO_MANY_VIAS:            return std::make_shared<DRC_ITEM>( tooManyVias );
     case DRCE_DIFF_PAIR_GAP_OUT_OF_RANGE:          return std::make_shared<DRC_ITEM>( diffPairGapOutOfRange );
     case DRCE_DIFF_PAIR_UNCOUPLED_LENGTH_TOO_LONG: return std::make_shared<DRC_ITEM>( diffPairUncoupledLengthTooLong );
+    case DRCE_FOOTPRINT_TYPE_MISMATCH:  return std::make_shared<DRC_ITEM>( footprintTypeMismatch );
     default:
         wxFAIL_MSG( "Unknown DRC error code" );
         return nullptr;

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;

pcbnew/footprint.cpp

@@ -2072,6 +2072,38 @@ void FOOTPRINT::BuildPolyCourtyards( OUTLINE_ERROR_HANDLER* aErrorHandler )
 }
 
 
+void FOOTPRINT::CheckFootprintAttributes( const std::function<void( const wxString& msg )>* 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 );

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<void( const wxString& msg )>* aErrorHandler );
+
     /**
      * Generate pads shapes on layer \a aLayer as polygons and adds these polygons to
      * \a aCornerBuffer.

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 )
     {