Annotate schematic: fix annotation bug with multi-unit parts when these parts have a duplicate reference.

It happens only with option "Reset, but keep order of multi-unit parts, because the algo tried
to propagate the new reference to parts having the same old reference,
without testing if this old reference was duplicated.

Fixes: lp:1769250
https://bugs.launchpad.net/kicad/+bug/1769250

eeschema/component_references_lister.cpp

@@ -1,12 +1,12 @@
 /**
  * @file component_references_lister.cpp
- * @brief Code for creating a flat list of components needed for annotation and BOM.
+ * @brief functions to create a component flat list and to annotate schematic.
  */
 
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 1992-2011 jean-pierre Charras <jp.charras at wanadoo.fr>
+ * Copyright (C) 1992-2018 jean-pierre Charras <jp.charras at wanadoo.fr>
  * Copyright (C) 1992-2011 Wayne Stambaugh <stambaughw@verizon.net>
  * Copyright (C) 1992-2018 KiCad Developers, see AUTHORS.txt for contributors.
  *
@@ -32,6 +32,7 @@
 #include <wx/regex.h>
 #include <algorithm>
 #include <vector>
+#include <unordered_set>
 
 #include <fctsys.h>
 #include <kicad_string.h>
@@ -271,6 +272,17 @@ int SCH_REFERENCE_LIST::CreateFirstFreeRefId( std::vector<int>& aIdList, int aFi
 }
 
 
+// A helper function to build a full reference string of a SCH_REFERENCE item
+wxString buildFullReference( const SCH_REFERENCE& aItem )
+{
+    wxString fullref;
+    fullref = aItem.GetRef() + aItem.GetRefNumber();
+    fullref << ".." << aItem.GetUnit();
+
+    return fullref;
+}
+
+
 void SCH_REFERENCE_LIST::Annotate( bool aUseSheetNum, int aSheetIntervalId, int aStartNumber,
       SCH_MULTI_UNIT_REFERENCE_MAP aLockedUnitMap )
 {
@@ -305,6 +317,18 @@ void SCH_REFERENCE_LIST::Annotate( bool aUseSheetNum, int aSheetIntervalId, int
     else
         minRefId = aStartNumber + 1;
 
+    // For multi units components, when "keep order of multi unit" option is selected,
+    // store the list of already used full references.
+    // The algorithm try to allocate the new reference to components having the same
+    // old reference.
+    // This algo works fine as long as the previous annotation has no duplicates.
+    // But when a hierarchy is reannotated with this option, the previous anotation can
+    // have duplicate references, and obviously we must fix these duplicate.
+    // therefore do not try to allocate a full reference more than once when trying
+    // to keep this order of multi units.
+    // inUseRefs keep trace of previously allocated references
+    std::unordered_set<wxString> inUseRefs;
+
     // This is the list of all Id already in use for a given reference prefix.
     // Will be refilled for each new reference prefix.
     std::vector<int>idList;
@@ -345,7 +369,8 @@ void SCH_REFERENCE_LIST::Annotate( bool aUseSheetNum, int aSheetIntervalId, int
             if( aUseSheetNum )
                 minRefId = componentFlatList[ii].m_SheetNum * aSheetIntervalId;
 
-            LastReferenceNumber = componentFlatList.GetLastReference( ii, minRefId );
+            LastReferenceNumber = GetLastReference( ii, minRefId );
+
 #else
             // when using sheet number, ensure ref number >= sheet number* aSheetIntervalId
             if( aUseSheetNum )
@@ -399,27 +424,44 @@ void SCH_REFERENCE_LIST::Annotate( bool aUseSheetNum, int aSheetIntervalId, int
         if( lockedList != NULL )
         {
             unsigned n_refs = lockedList->GetCount();
+
             for( unsigned thisRefI = 0; thisRefI < n_refs; ++thisRefI )
             {
                 SCH_REFERENCE &thisRef = (*lockedList)[thisRefI];
+
                 if( thisRef.IsSameInstance( componentFlatList[ii] ) )
                 {
                     // This is the component we're currently annotating. Hold the unit!
                     componentFlatList[ii].m_Unit = thisRef.m_Unit;
                 }
 
-                if( thisRef.CompareValue( componentFlatList[ii] ) != 0 ) continue;
+                if( thisRef.CompareValue( componentFlatList[ii] ) != 0 )
-                if( thisRef.CompareLibName( componentFlatList[ii] ) != 0 ) continue;
+                    continue;
+
+                if( thisRef.CompareLibName( componentFlatList[ii] ) != 0 )
+                    continue;
 
                 // Find the matching component
                 for( unsigned jj = ii + 1; jj < componentFlatList.size(); jj++ )
                 {
-                    if( ! thisRef.IsSameInstance( componentFlatList[jj] ) ) continue;
+                    if( ! thisRef.IsSameInstance( componentFlatList[jj] ) )
-                    componentFlatList[jj].m_NumRef = componentFlatList[ii].m_NumRef;
+                        continue;
-                    componentFlatList[jj].m_Unit = thisRef.m_Unit;
+
-                    componentFlatList[jj].m_IsNew = false;
+                    wxString ref_candidate = buildFullReference( componentFlatList[ii] );
-                    componentFlatList[jj].m_Flag = 1;
+
-                    break;
+                    // propagate the new reference and unit selection to the "old" component,
+                    // if this new full reference is not already used (can happens when initial
+                    // multiunits components have duplicate references)
+                    if( inUseRefs.find( ref_candidate ) == inUseRefs.end() )
+                    {
+                        componentFlatList[jj].m_NumRef = componentFlatList[ii].m_NumRef;
+                        componentFlatList[jj].m_Unit = thisRef.m_Unit;
+                        componentFlatList[jj].m_IsNew = false;
+                        componentFlatList[jj].m_Flag = 1;
+                        // lock this new full reference
+                        inUseRefs.insert( ref_candidate );
+                        break;
+                    }
                 }
             }
         }

eeschema/sch_reference_list.h

@@ -3,7 +3,7 @@
  *
  * Copyright (C) 1992-2011 jean-pierre Charras <jean-pierre.charras@gipsa-lab.inpg.fr>
  * Copyright (C) 1992-2011 Wayne Stambaugh <stambaughw@verizon.net>
- * Copyright (C) 1992-2015 KiCad Developers, see authors.txt for contributors.
+ * Copyright (C) 1992-2018 KiCad Developers, see authors.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -416,7 +416,6 @@ public:
     }
 
     /**
-     * Function GetUnit
      * searches the sorted list of components for a another component with the same
      * reference and a given part unit.  Use this method to manage components with
      * multiple parts per package.