From 1629b038e11e5f62f8c08108b056801304df256a Mon Sep 17 00:00:00 2001
From: Roberto Fernandez Bautista <roberto.fer.bau@gmail.com>
Date: Sun, 28 Mar 2021 19:47:48 +0100
Subject: [PATCH] CADSTAR Schematic: Fix loading of symbol instances differing
 from part The previous assumption was that a part definition would always
 hold the information about the symbol. However some designs have symbol
 instances that refer to a different symbol definition than that specified in
 the part.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/7808
---
 .../cadstar/cadstar_sch_archive_loader.cpp    | 44 +++++++++++++++----
 .../cadstar/cadstar_sch_archive_loader.h      |  3 ++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp b/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp
index cbe04442e8..f26a68dd9d 100644
--- a/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp
+++ b/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp
@@ -328,7 +328,7 @@ void CADSTAR_SCH_ARCHIVE_LOADER::loadPartsLibrary()
 {
     for( std::pair<PART_ID, PART> partPair : Parts.PartDefinitions )
     {
-        PART_ID key  = partPair.first;
+        PART_ID partID  = partPair.first;
         PART    part = partPair.second;
 
         if( part.Definition.GateSymbols.size() == 0 )
@@ -337,12 +337,14 @@ void CADSTAR_SCH_ARCHIVE_LOADER::loadPartsLibrary()
         LIB_PART* kiPart = new LIB_PART( part.Name );
 
         kiPart->SetUnitCount( part.Definition.GateSymbols.size() );
+        bool ok = true;
 
         for( std::pair<GATE_ID, PART::DEFINITION::GATE> gatePair : part.Definition.GateSymbols )
         {
             GATE_ID                gateID   = gatePair.first;
             PART::DEFINITION::GATE gate     = gatePair.second;
             SYMDEF_ID              symbolID = getSymDefFromName( gate.Name, gate.Alternate );
+            m_partSymbolsMap.insert( { { partID, gateID }, symbolID } );
 
             if( symbolID.IsEmpty() )
             {
@@ -352,18 +354,28 @@ void CADSTAR_SCH_ARCHIVE_LOADER::loadPartsLibrary()
                            "been loaded into the KiCad library." ),
                         part.Name, gate.Name, gate.Alternate ) );
 
-                continue;
+                ok = false;
+                break;
             }
 
             loadSymDefIntoLibrary( symbolID, &part, gateID, kiPart );
         }
 
-        ( *m_plugin )->SaveSymbol( m_libraryFileName.GetFullPath(), kiPart );
+        if( ok )
+        {
+            ( *m_plugin )->SaveSymbol( m_libraryFileName.GetFullPath(), kiPart );
 
-        LIB_PART* loadedPart =
-                ( *m_plugin )->LoadSymbol( m_libraryFileName.GetFullPath(), kiPart->GetName() );
+            LIB_PART* loadedPart =
+                    ( *m_plugin )->LoadSymbol( m_libraryFileName.GetFullPath(), kiPart->GetName() );
 
-        m_partMap.insert( { key, loadedPart } );
+            m_partMap.insert( { partID, loadedPart } );
+        }
+        else
+        {
+            // Don't save in the library, but still keep it cached as some of the units might have
+            // been loaded correctly (saving us time later on)
+            m_partMap.insert( { partID, kiPart } );
+        }
     }
 }
 
@@ -389,11 +401,27 @@ void CADSTAR_SCH_ARCHIVE_LOADER::loadSchematicSymbolInstances()
                 continue;
             }
 
-            LIB_PART* kiPart = m_partMap.at( sym.PartRef.RefID );
-            double    symOrientDeciDeg = 0.0;
+            PART_GATE_ID partSymbolID = { sym.PartRef.RefID, sym.GateID };
+            LIB_PART*    kiPart = m_partMap.at( sym.PartRef.RefID );
+            bool         copy = false;
 
+            // The symbol definition in the part either does not exist for this gate number
+            // or is different to the symbol instance. We need to load a new symbol
+            if( m_partSymbolsMap.find( partSymbolID ) == m_partSymbolsMap.end()
+                || m_partSymbolsMap.at( partSymbolID ) != sym.SymdefID )
+            {
+                kiPart = new LIB_PART( *kiPart ); // Make a copy
+                copy = true;
+                const PART& part = Parts.PartDefinitions.at( sym.PartRef.RefID );
+                loadSymDefIntoLibrary( sym.SymdefID, &part, sym.GateID, kiPart );
+            }
+
+            double         symOrientDeciDeg = 0.0;
             SCH_COMPONENT* component = loadSchematicSymbol( sym, *kiPart, symOrientDeciDeg );
 
+            if( copy )
+                delete kiPart;
+
             SCH_FIELD* refField = component->GetField( REFERENCE_FIELD );
 
             sym.ComponentRef.Designator.Replace( wxT( "\n" ), wxT( "\\n" ) );
diff --git a/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.h b/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.h
index 850ecfe1cf..6f5729a515 100644
--- a/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.h
+++ b/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.h
@@ -78,6 +78,7 @@ public:
 
 private:
     typedef std::pair<BLOCK_ID, TERMINAL_ID> BLOCK_PIN_ID;
+    typedef std::pair<PART_ID, GATE_ID> PART_GATE_ID;
 
     /**
      * Map between a terminal ID in a symbol definition to the pin number that should
@@ -96,6 +97,8 @@ private:
     std::map<BLOCK_PIN_ID, SCH_HIERLABEL*>
                                  m_sheetPinMap; ///< Map between Cadstar and KiCad Sheets Pins
     std::map<PART_ID, LIB_PART*> m_partMap;     ///< Map between Cadstar and KiCad Parts
+    std::map<PART_GATE_ID, SYMDEF_ID> m_partSymbolsMap; ///< Map holding the symbols loaded so far
+                                                        ///  for a particular PART_ID and GATE_ID
     std::map<PART_ID, TERMINAL_TO_PINNUM_MAP> m_pinNumsMap; ///< Map of pin numbers in CADSTAR parts
     std::map<wxString, LIB_PART*> m_powerSymLibMap; ///< Map of KiCad Power Symbol Library items
     std::map<SYMBOL_ID, SCH_COMPONENT*>