kicad-developers team mailing list archive

[Jon Evans <jon@xxxxxxxxxxxxx>]

Relating to https://bugs.launchpad.net/kicad/+bug/1680638

I figured Wayne and perhaps some others would like to check this out before
merging to make sure it properly addresses the problem.

-Jon

From 210eae84bd55854323939f179abe2c8927fe020c Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Thu, 8 Mar 2018 20:52:03 -0500
Subject: [PATCH 2/2] Add ERC check in component editor for conflicting
 multi-unit pins

Fixes: lp:1680638
* https://bugs.launchpad.net/kicad/+bug/1680638
---
 eeschema/pinedit.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/eeschema/pinedit.cpp b/eeschema/pinedit.cpp
index aeeee6279..ff39bcff2 100644
--- a/eeschema/pinedit.cpp
+++ b/eeschema/pinedit.cpp
@@ -722,8 +722,7 @@ void LIB_EDIT_FRAME::OnCheckComponent( wxCommandEvent& event )
         LIB_PIN* pin      = pinList[ii - 1];
 
         if( pin->GetNumber() != curr_pin->GetNumber()
-            || pin->GetConvert() != curr_pin->GetConvert()
-            || pin->GetUnit() != curr_pin->GetUnit() )
+            || pin->GetConvert() != curr_pin->GetConvert() )
             continue;
 
         dup_error++;
@@ -746,7 +745,9 @@ void LIB_EDIT_FRAME::OnCheckComponent( wxCommandEvent& event )
 
         if( part->GetUnitCount() > 1 )
         {
-            msg += wxString::Format( _( " in symbol %c" ), 'A' + curr_pin->GetUnit() - 1 );
+            msg += wxString::Format( _( " in units %c and %c" ),
+                                     'A' + curr_pin->GetUnit() - 1,
+                                     'A' + pin->GetUnit() - 1 );
         }
 
         if( m_showDeMorgan )
-- 
2.14.1

From e5b4a1d07a5b3dff305c90a7d17995010a0d9d24 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Thu, 8 Mar 2018 20:42:06 -0500
Subject: [PATCH 1/2] Add ERC check for conflicting pins on multi-unit
 components

Fixes: lp:1680638
* https://bugs.launchpad.net/kicad/+bug/1680638
---
 eeschema/dialogs/dialog_erc.cpp |  3 +++
 eeschema/drc_erc_item.cpp       |  2 ++
 eeschema/erc.cpp                | 60 +++++++++++++++++++++++++++++++++++++++++
 eeschema/erc.h                  | 12 +++++++++
 4 files changed, 77 insertions(+)

diff --git a/eeschema/dialogs/dialog_erc.cpp b/eeschema/dialogs/dialog_erc.cpp
index 4a257a443..2bd1540dd 100644
--- a/eeschema/dialogs/dialog_erc.cpp
+++ b/eeschema/dialogs/dialog_erc.cpp
@@ -482,6 +482,9 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter )
      */
     TestMultiunitFootprints( sheets );
 
+    // Test multi-unit components for conflicting pin numbers
+    TestMultiunitPinConflicts( sheets );
+
     std::unique_ptr<NETLIST_OBJECT_LIST> objectsConnectedList( m_parent->BuildNetListBase() );
 
     // Reset the connection type indicator
diff --git a/eeschema/drc_erc_item.cpp b/eeschema/drc_erc_item.cpp
index 15a893cca..9739a07cf 100644
--- a/eeschema/drc_erc_item.cpp
+++ b/eeschema/drc_erc_item.cpp
@@ -60,6 +60,8 @@ wxString DRC_ITEM::GetErrorText() const
         return wxString( _("Global labels are similar (lower/upper case difference only)") );
     case ERCE_DIFFERENT_UNIT_FP:
         return wxString( _("Different footprint assigned in another unit of the same component") );
+    case ERCE_DIFFERENT_UNIT_PIN:
+        return wxString( _( "Conflicting pin in another unit of the same component" ) );
 
     default:
         wxFAIL_MSG( "Missing ERC error description" );
diff --git a/eeschema/erc.cpp b/eeschema/erc.cpp
index 8fdee6f15..4ad86da06 100644
--- a/eeschema/erc.cpp
+++ b/eeschema/erc.cpp
@@ -28,6 +28,7 @@
  * @brief Electrical Rules Check implementation.
  */
 
+#include <unordered_set>
 #include <fctsys.h>
 #include <class_drawpanel.h>
 #include <kicad_string.h>
@@ -291,6 +292,65 @@ int TestMultiunitFootprints( SCH_SHEET_LIST& aSheetList )
 }
 
 
+int TestMultiunitPinConflicts( SCH_SHEET_LIST& aSheetList )
+{
+    int errors = 0;
+    std::map<wxString, LIB_ID> footprints;
+    SCH_MULTI_UNIT_REFERENCE_MAP refMap;
+    aSheetList.GetMultiUnitComponents( refMap, true );
+
+    for( auto& component : refMap )
+    {
+        auto& refList = component.second;
+
+        if( refList.GetCount() == 0 )
+        {
+            wxFAIL;   // it should not happen
+            continue;
+        }
+
+        std::unordered_set<wxString> seen_pins;
+
+        for( unsigned i = 0; i < component.second.GetCount(); ++i )
+        {
+            SCH_REFERENCE& ref = refList.GetItem( i );
+            SCH_COMPONENT* cmp = ref.GetComp();
+            SCH_SHEET_PATH sheetPath = ref.GetSheetPath();
+
+            std::vector<LIB_PIN*> pins;
+            cmp->GetPins( pins );
+
+            for( auto pin : pins )
+            {
+                if( seen_pins.find( pin->GetNumber() ) != seen_pins.end() )
+                {
+                    wxString unitName = cmp->GetRef( &sheetPath )
+                        + LIB_PART::SubReference( cmp->GetUnit(), false );
+
+                    SCH_MARKER* marker = new SCH_MARKER();
+
+                    marker->SetTimeStamp( GetNewTimeStamp() );
+                    marker->SetData( ERCE_DIFFERENT_UNIT_FP, cmp->GetPosition(),
+                        wxString::Format( _( "Unit %s has pin '%s', which also "
+                            "exists on another unit of the same component." ),
+                            unitName, pin->GetNumber() ), cmp->GetPosition() );
+                    marker->SetMarkerType( MARKER_BASE::MARKER_ERC );
+                    marker->SetErrorLevel( MARKER_BASE::MARKER_SEVERITY_ERROR );
+
+                    ref.GetSheetPath().LastScreen()->Append( marker );
+
+                    ++errors;
+                }
+
+                seen_pins.insert( pin->GetNumber() );
+            }
+        }
+    }
+
+    return errors;
+}
+
+
 void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst,
                int aMinConn, int aDiag )
 {
diff --git a/eeschema/erc.h b/eeschema/erc.h
index a50fa4327..a1dd3482c 100644
--- a/eeschema/erc.h
+++ b/eeschema/erc.h
@@ -62,6 +62,7 @@ extern const wxString CommentERC_V[];
 #define ERCE_SIMILAR_GLBL_LABELS  10   // 2 labels are equal fir case insensitive comparisons
 #define ERCE_DIFFERENT_UNIT_FP    11   // different units of the same component have different
                                        // footprints assigned
+#define ERCE_DIFFERENT_UNIT_PIN   12   // different units of the same component have a shared pin
 
 /* Minimal connection table */
 #define NPI    4  // Net with Pin isolated, this pin has type Not Connected and must be left N.C.
@@ -117,5 +118,16 @@ int TestDuplicateSheetNames( bool aCreateMarker );
  */
 int TestMultiunitFootprints( SCH_SHEET_LIST& aSheetList );
 
+/**
+ * Test if there are any duplicate pins in differing units of a multiunit component.
+ *
+ * This is not a supported way to create components, but some old library
+ * components were created this way, which can cause improper annotation.
+ *
+ * @param aSheetList contains the sheets to be tested
+ * @return the error count
+ */
+int TestMultiunitPinConflicts( SCH_SHEET_LIST& aSheetList );
+
 
 #endif  // _ERC_H
-- 
2.14.1