From 9a801d8b72f24e297a7d9d6e8cee2eef6cab2988 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@craftyjon.com>
Date: Mon, 20 Jul 2020 20:41:56 -0400
Subject: [PATCH] Don't disrupt diff pairs when auto-renaming buses

With bus groups we can use the prefix to disambiguate

Fixes https://gitlab.com/kicad/code/kicad/-/issues/4916
---
 eeschema/connection_graph.cpp | 41 +++++++++++++++++++++++++++--------
 eeschema/sch_connection.cpp   |  5 ++++-
 eeschema/sch_connection.h     | 10 ++++++++-
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/eeschema/connection_graph.cpp b/eeschema/connection_graph.cpp
index 6a1412299f..aee6eebf85 100644
--- a/eeschema/connection_graph.cpp
+++ b/eeschema/connection_graph.cpp
@@ -977,13 +977,36 @@ void CONNECTION_GRAPH::buildConnectionGraph()
         // Test subgraphs with weak drivers for net name conflicts and fix them
         unsigned suffix = 1;
 
-        auto create_new_name = [&] ( SCH_CONNECTION* aConn, wxString aName ) -> wxString
-                               {
-                                   wxString new_name = wxString::Format( "%s_%u", aName, suffix );
-                                   aConn->SetSuffix( wxString::Format( "_%u", suffix ) );
-                                   suffix++;
-                                   return new_name;
-                               };
+        auto create_new_name =
+                [&suffix]( SCH_CONNECTION* aConn ) -> wxString
+                {
+                    wxString newName;
+
+                    // For group buses with a prefix, we can add the suffix to the prefix.
+                    // If they don't have a prefix, we force the creation of a prefix so that
+                    // two buses don't get inadvertently shorted together.
+                    if( aConn->Type() == CONNECTION_TYPE::BUS_GROUP )
+                    {
+                        wxString prefix = aConn->BusPrefix();
+
+                        if( prefix.empty() )
+                            prefix = wxT( "BUS" ); // So result will be "BUS_1{...}"
+
+                        wxString oldName = aConn->Name().AfterFirst( '{' );
+
+                        newName = wxString::Format( "%s_%u{%s", prefix, suffix, oldName );
+
+                        aConn->ConfigureFromLabel( newName );
+                    }
+                    else
+                    {
+                        newName = wxString::Format( "%s_%u", aConn->Name(), suffix );
+                        aConn->SetSuffix( wxString::Format( "_%u", suffix ) );
+                    }
+
+                    suffix++;
+                    return newName;
+                };
 
         if( !subgraph->m_strong_driver )
         {
@@ -991,10 +1014,10 @@ void CONNECTION_GRAPH::buildConnectionGraph()
 
             if( vec.size() > 1 )
             {
-                wxString new_name = create_new_name( connection, name );
+                wxString new_name = create_new_name( connection );
 
                 while( m_net_name_to_subgraphs_map.count( new_name ) )
-                    new_name = create_new_name( connection, name );
+                    new_name = create_new_name( connection );
 
                 wxLogTrace( "CONN", "%ld (%s) is weakly driven and not unique. Changing to %s.",
                             subgraph->m_code, name, new_name );
diff --git a/eeschema/sch_connection.cpp b/eeschema/sch_connection.cpp
index 2af98db3ad..b0aee5bf3a 100644
--- a/eeschema/sch_connection.cpp
+++ b/eeschema/sch_connection.cpp
@@ -152,7 +152,8 @@ void SCH_CONNECTION::ConfigureFromLabel( const wxString& aLabel )
     }
     else if( NET_SETTINGS::ParseBusGroup( unescaped, &prefix, &members ) )
     {
-        m_type = CONNECTION_TYPE::BUS_GROUP;
+        m_type       = CONNECTION_TYPE::BUS_GROUP;
+        m_bus_prefix = prefix;
 
         // Named bus groups generate a net prefix, unnamed ones don't
         if( !prefix.IsEmpty() )
@@ -199,6 +200,7 @@ void SCH_CONNECTION::Reset()
     m_cached_name.Empty();
     m_cached_name_with_path.Empty();
     m_prefix.Empty();
+    m_bus_prefix.Empty();
     m_suffix .Empty();
     m_driver = nullptr;
     m_members.clear();
@@ -222,6 +224,7 @@ void SCH_CONNECTION::Clone( SCH_CONNECTION& aOther )
     m_name = aOther.m_name;
     // Note: m_local_name is not cloned
     m_prefix = aOther.Prefix();
+    m_bus_prefix = aOther.BusPrefix();
     m_suffix = aOther.Suffix();
     m_members = aOther.Members();
     m_net_code = aOther.NetCode();
diff --git a/eeschema/sch_connection.h b/eeschema/sch_connection.h
index 6ff75bb938..cad56fa6f6 100644
--- a/eeschema/sch_connection.h
+++ b/eeschema/sch_connection.h
@@ -178,6 +178,11 @@ public:
         return m_prefix;
     }
 
+    wxString BusPrefix() const
+    {
+        return m_bus_prefix;
+    }
+
     wxString Suffix() const
     {
         return m_suffix;
@@ -325,9 +330,12 @@ private:
      */
     wxString m_local_name;
 
-    ///< Prefix if connection is member of a labeled bus group (or "" if not)
+    /// Prefix if connection is member of a labeled bus group (or "" if not)
     wxString m_prefix;
 
+    /// Optional prefix of a bux group (always empty for nets and vector buses)
+    wxString m_bus_prefix;
+
     wxString m_suffix;      ///< Name suffix (used only for disambiguation)
 
     int m_net_code;         // TODO(JE) remove if unused