Fix Bug #1545708 (global bus labels ignored). This is a very old bug.

eeschema/class_netlist_object.cpp

@@ -1,9 +1,9 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2013 Jean-Pierre Charras, jp.charras at wanadoo.fr
+ * Copyright (C) 2016 Jean-Pierre Charras, jp.charras at wanadoo.fr
  * Copyright (C) 2013 Wayne Stambaugh <stambaughw@verizon.net>
- * Copyright (C) 1992-2013 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2016 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
@@ -315,6 +315,32 @@ void NETLIST_OBJECT::ConvertBusToNetListItems( NETLIST_OBJECT_LIST& aNetListItem
     }
 }
 
+
+bool NETLIST_OBJECT::IsLabelGlobal() const
+{
+    // return true if the object is a global label
+    // * a actual global label
+    // * a pin label coming from a invisible power pin
+    return ( m_Type == NET_PINLABEL ) ||
+           ( m_Type == NET_GLOBLABEL ) ||
+           ( m_Type == NET_GLOBBUSLABELMEMBER );
+}
+
+
+bool NETLIST_OBJECT::IsLabelBusMemberType() const
+{
+    // return true if the object is a bus label member build from a
+    // schematic bus label (like label[xx..yy)
+    // They are labels with very specific properties, especially for connection
+    // between them: 2 bus label members can be connected only
+    // if they have the same member value.
+    return ( m_Type == NET_SHEETBUSLABELMEMBER ) ||
+           ( m_Type == NET_BUSLABELMEMBER ) ||
+           ( m_Type == NET_HIERBUSLABELMEMBER ) ||
+           ( m_Type == NET_GLOBBUSLABELMEMBER );
+}
+
+
 /*
  * return the net name of the item
  */
@@ -373,7 +399,7 @@ wxString NETLIST_OBJECT::GetShortNetName() const
  * Set m_netNameCandidate to a connected item which will
  * be used to calcule the net name of the item
  * Obviously the candidate can be only a label
- * when there is no label on the net a pad which will
+ * If there is no label on the net, a pad name will be
  * used to build a net name (something like Cmp<REF>_Pad<PAD_NAME>
  * @param aCandidate = the connected item candidate
  */
@@ -385,6 +411,8 @@ void NETLIST_OBJECT::SetNetNameCandidate( NETLIST_OBJECT* aCandidate )
         case NET_LABEL:
         case NET_PINLABEL:
         case NET_GLOBLABEL:
+        case NET_GLOBBUSLABELMEMBER:
+        case NET_SHEETBUSLABELMEMBER:
         case NET_PIN:
             m_netNameCandidate = aCandidate;
             break;

eeschema/class_netlist_object.h

@@ -59,6 +59,8 @@ enum NETLIST_ITEM_T
                                      * the bus label (like DATA[0..7] is
                                      * converted to n single labels like
                                      * DATA0, DATA1 ...
+                                     * These objects are living only in the current
+                                     * NETLIST_OBJECT_LIST, not in shematic.
                                      */
     NET_GLOBBUSLABELMEMBER,         // see NET_BUSLABELMEMBER, used when a
                                     // global bus label is found
@@ -217,10 +219,19 @@ public:
      * (i.e. an real global label or a pin label coming
      * from a power pin invisible
      */
-    bool IsLabelGlobal() const
+    bool IsLabelGlobal() const;
-    {
+
-        return ( m_Type == NET_PINLABEL ) || ( m_Type == NET_GLOBLABEL );
+    /**
-    }
+     * Function IsLabelBusMemberType
+     * @return true if the object is a bus label member build from a
+     * schematic bus label (like label[xx..yy], xx and yy are the first and last
+     * bus member id)
+     * bus label members have specific properties:
+     * they do not live in schematic
+     * they have specific properties in connections:
+     * 2 bus label members can be connected connected only if they have the same member value.
+     */
+    bool IsLabelBusMemberType() const;
 
     /**
      * Function IsLabelType
@@ -436,7 +447,7 @@ private:
         return Objet1->m_SheetPath.Cmp( Objet2->m_SheetPath ) < 0;
     }
 
-    /*
+    /**
      * Propagate net codes from a parent sheet to an include sheet,
      * from a pin sheet connection
      */
@@ -444,8 +455,8 @@ private:
 
     void pointToPointConnect( NETLIST_OBJECT* aRef, bool aIsBus, int start );
 
-    /*
+    /**
-     * Search connections betweena junction and segments
+     * Search connections between a junction and segments
      * Propagate the junction net code to objects connected by this junction.
      * The junction must have a valid net code
      * The list of objects is expected sorted by sheets.
@@ -453,9 +464,16 @@ private:
      */
     void segmentToPointConnect( NETLIST_OBJECT* aJonction, bool aIsBus, int aIdxStart );
 
+
+    /**
+     * Function connectBusLabels
+     * Propagate the net code (and create it, if not yet existing) between
+     * all bus label member objects connected by they name.
+     * Search is done in the entire list
+     */
     void connectBusLabels();
 
-    /*
+    /**
      * Set the m_FlagOfConnection member of items in list
      * depending on the connection type:
      * UNCONNECTED, PAD_CONNECT or NOCONNECT_SYMBOL_PRESENT

eeschema/netlist.cpp

@@ -228,7 +228,7 @@ bool NETLIST_OBJECT_LIST::BuildNetListInfo( SCH_SHEET_LIST& aSheets )
         switch( net_item->m_Type )
         {
         case NET_ITEM_UNSPECIFIED:
-            wxMessageBox( wxT( "BuildNetListBase() error" ) );
+            wxMessageBox( wxT( "BuildNetListInfo() error" ) );
             break;
 
         case NET_PIN:
@@ -397,8 +397,8 @@ bool NETLIST_OBJECT_LIST::BuildNetListInfo( SCH_SHEET_LIST& aSheets )
 }
 
 // Helper function to give a priority to sort labels:
-// NET_PINLABEL and NET_GLOBLABEL are global labels
+// NET_PINLABEL, NET_GLOBBUSLABELMEMBER and NET_GLOBLABEL are global labels
-// and the priority is hight
+// and the priority is high
 static int getPriority( const NETLIST_OBJECT* Objet )
 {
     switch( Objet->m_Type )
@@ -407,7 +407,8 @@ static int getPriority( const NETLIST_OBJECT* Objet )
         case NET_LABEL: return 2;
         case NET_HIERLABEL: return 3;
         case NET_PINLABEL: return 4;
-        case NET_GLOBLABEL: return 5;
+        case NET_GLOBBUSLABELMEMBER: return 5;
+        case NET_GLOBLABEL: return 6;
         default: break;
     }
 
@@ -417,11 +418,24 @@ static int getPriority( const NETLIST_OBJECT* Objet )
 
 /* function evalLabelsPriority used by findBestNetNameForEachNet()
  * evalLabelsPriority calculates the priority of alabel1 and aLabel2
- * return true if alabel1 has a highter priority than aLabel2
+ * return true if alabel1 has a higher priority than aLabel2
  */
 static bool evalLabelsPriority( const NETLIST_OBJECT* aLabel1,
                                  const NETLIST_OBJECT* aLabel2 )
 {
+    // Global labels have the highest prioriy.
+    // For local labels: names are prefixed by their sheetpath
+    // use name defined in the more top level hierarchical sheet
+    // (i.e. shorter timestamp path because paths are /<timestamp1>/<timestamp2>/...
+    // and timestamp = 8 letters.
+    // Note: the final net name uses human sheetpath name, not timestamp sheetpath name
+    // They are equivalent, but not for human readers.
+    if( ! aLabel1->IsLabelGlobal() && ! aLabel2->IsLabelGlobal() )
+    {
+        if( aLabel1->m_SheetPath.Path().Length() != aLabel2->m_SheetPath.Path().Length() )
+            return aLabel1->m_SheetPath.Path().Length() < aLabel2->m_SheetPath.Path().Length();
+    }
+
     int priority1 = getPriority( aLabel1 );
     int priority2 = getPriority( aLabel2 );
 
@@ -429,29 +443,22 @@ static bool evalLabelsPriority( const NETLIST_OBJECT* aLabel1,
         return priority1 > priority2;
 
     // Objects have here the same priority, therefore they have the same type.
-
     // for global labels, we select the best candidate by alphabetic order
     // because they have no sheetpath as prefix name
     // for other labels, we select them before by sheet deep order
     // because the actual name is /sheetpath/label
     // and for a given path length, by alphabetic order
-
+    if( aLabel1->IsLabelGlobal() )
-    if( aLabel1->m_Type == NET_PINLABEL || aLabel1->m_Type == NET_GLOBLABEL )
         return aLabel1->m_Label.Cmp( aLabel2->m_Label ) < 0;
 
-    // not global: names are prefixed by their sheetpath
+    // Sheet paths have here the same length: use alphabetic label name order
-    // use name defined in higher hierarchical sheet
-    // (i.e. shorter path because paths are /<timestamp1>/<timestamp2>/...
-    // and timestamp = 8 letters.
-    if( aLabel1->m_SheetPath.Path().Length() != aLabel2->m_SheetPath.Path().Length() )
-        return aLabel1->m_SheetPath.Path().Length() < aLabel2->m_SheetPath.Path().Length();
-
-    // Sheet paths have the same length: use alphabetic label name order
     // For labels on sheets having an equivalent deep in hierarchy, use
     // alphabetic label name order:
     if( aLabel1->m_Label.Cmp( aLabel2->m_Label ) != 0 )
         return aLabel1->m_Label.Cmp( aLabel2->m_Label ) < 0;
 
+    // For identical labels having the same priority: choose the
+    // alphabetic label full name order
     return aLabel1->m_SheetPath.PathHumanReadable().Cmp(
                 aLabel2->m_SheetPath.PathHumanReadable() ) < 0;
 }
@@ -459,6 +466,12 @@ static bool evalLabelsPriority( const NETLIST_OBJECT* aLabel1,
 
 void NETLIST_OBJECT_LIST::findBestNetNameForEachNet()
 {
+    // Important note: NET_SHEETLABEL items of sheet items should *NOT* be considered,
+    // because they live in a sheet but their names are actually used in the subsheet.
+    // Moreover, in the parent sheet, the name of NET_SHEETLABEL can be not unique,
+    // ( for instance when 2 different sheets share the same schematic in complex hierarchies
+    // and 2 identical NET_SHEETLABEL labels can be connected to 2 different nets
+
     int netcode = 0;            // current netcode for tested items
     unsigned idxstart = 0;      // index of the first item of this net
     NETLIST_OBJECT* item;
@@ -481,9 +494,10 @@ void NETLIST_OBJECT_LIST::findBestNetNameForEachNet()
                     GetItem( jj )->SetNetNameCandidate( candidate );
             }
 
-            if( item == NULL )
+            if( item == NULL )  // End of list
                 break;
 
+            // Prepare next net analysis:
             netcode = item->GetNet();
             candidate = NULL;
             idxstart = ii;
@@ -495,6 +509,7 @@ void NETLIST_OBJECT_LIST::findBestNetNameForEachNet()
         case NET_LABEL:
         case NET_PINLABEL:
         case NET_GLOBLABEL:
+        case NET_GLOBBUSLABELMEMBER:
             // A candidate is found: select the better between the previous
             // and this one
             if( candidate == NULL )
@@ -566,7 +581,7 @@ void NETLIST_OBJECT_LIST::findBestNetNameForEachNet()
 
         // Examine all pins of the net to find the best candidate,
         // i.e. the first net name candidate, by alphabetic order
-        // the net names are names bu_ilt by GetShortNetName
+        // the net names are built by GetShortNetName
         // (Net-<{reference}-Pad{pad number}> like Net-<U3-Pad5>
         // Not named nets do not have usually a lot of members.
         // Many have only 2 members(a pad and a non connection symbol)
@@ -627,16 +642,18 @@ void NETLIST_OBJECT_LIST::sheetLabelConnect( NETLIST_OBJECT* SheetLabel )
 
 void NETLIST_OBJECT_LIST::connectBusLabels()
 {
+    // Propagate the net code between all bus label member objects connected by they name.
+    // If the net code is not yet existing, a new one is created
+    // Search is done in the entire list
     for( unsigned ii = 0; ii < size(); ii++ )
     {
         NETLIST_OBJECT* Label = GetItem( ii );
 
-        if(  (Label->m_Type == NET_SHEETBUSLABELMEMBER)
+        if( Label->IsLabelBusMemberType() )
-          || (Label->m_Type == NET_BUSLABELMEMBER)
-          || (Label->m_Type == NET_HIERBUSLABELMEMBER) )
         {
             if( Label->GetNet() == 0 )
             {
+                // Not yet existiing net code: create a new one.
                 Label->SetNet( m_lastNetCode );
                 m_lastNetCode++;
             }
@@ -644,9 +661,8 @@ void NETLIST_OBJECT_LIST::connectBusLabels()
             for( unsigned jj = ii + 1; jj < size(); jj++ )
             {
                 NETLIST_OBJECT* LabelInTst =  GetItem( jj );
-                if( (LabelInTst->m_Type == NET_SHEETBUSLABELMEMBER)
+
-                   || (LabelInTst->m_Type == NET_BUSLABELMEMBER)
+                if( LabelInTst->IsLabelBusMemberType() )
-                   || (LabelInTst->m_Type == NET_HIERBUSLABELMEMBER) )
                 {
                     if( LabelInTst->m_BusNetCode != Label->m_BusNetCode )
                         continue;
@@ -655,8 +671,10 @@ void NETLIST_OBJECT_LIST::connectBusLabels()
                         continue;
 
                     if( LabelInTst->GetNet() == 0 )
+                        // Append this object to the current net
                         LabelInTst->SetNet( Label->GetNet() );
                     else
+                        // Merge the 2 net codes, they are connected.
                         propageNetCode( LabelInTst->GetNet(), Label->GetNet(), IS_WIRE );
                 }
             }