CADSTAR Parts: Fix parsing of hidden pins (possible for several pins per net!)

common/plugins/cadstar/cadstar_parts_lib_grammar.h

@@ -231,7 +231,7 @@ struct EQUIVALENT_PINS_GROUP :
                 <
                     plus<WHITESPACE_OR_CONTINUATION>,
                     EQUIVALENT_PIN,
-                    plus<one<'='>, EQUIVALENT_PIN>
+                    plus<one<'='>, star<WHITESPACE_OR_CONTINUATION>, EQUIVALENT_PIN>
                 >
 {};
 
@@ -489,25 +489,14 @@ struct PIN_ENTRY :
                 >
 {};
 
+struct PIN_LIST : plus<PIN_ENTRY, star<WHITESPACE>, opt<LINE_CONTINUATION>> {};
 
-struct SYMBOL_ENTRY :
+struct SYMBOL_ENTRY : seq<SCH_SYMBOL_LINE, PIN_LIST, opt<eol>>{};
-                seq
-                <
-                    SCH_SYMBOL_LINE,
-                    plus
-                    <
-                        PIN_ENTRY,
-                        star<WHITESPACE>,
-                        opt<LINE_CONTINUATION>
-                    >,
-                    opt<eol>
-                >
-{};
 
 
-///<Signame>_<PinIdentifier>[.<Position>][!<Pintype>][:<Loading>]
+// /<Signame>_<PinIdentifier>[.<Position>][!<Pintype>][:<Loading>]
 struct PIN_SIGNAL_NAME : seq<one<'/'>, STRING_EXCLUDING<WHITESPACE>> {};
-struct HIDDEN_PIN_ENTRY : seq<PIN_SIGNAL_NAME, plus<WHITESPACE>, PIN_ENTRY, opt<eol>>{};
+struct HIDDEN_PIN_ENTRY : seq<PIN_SIGNAL_NAME, plus<WHITESPACE>, PIN_LIST, opt<eol>>{};
 
 
 //******************
@@ -548,7 +537,6 @@ struct PART_ENTRY :
                 >
 {};
 
-struct UNMATCHED_CONTENT : STRING_EXCLUDING<FORMAT, PART_ENTRY> {}; //@todo remove once parser is complete
 
 /**
  * Grammar for CADSTAR Parts Library file format (*.lib)
@@ -561,7 +549,6 @@ struct GRAMMAR :
                         sor
                         <
                             PART_ENTRY,
-                            //UNMATCHED_CONTENT, //@todo remove once parser is complete
                             EMPTY_LINE // optional empty line
                         >,
                         opt<eol>

common/plugins/cadstar/cadstar_parts_lib_model.h

@@ -162,9 +162,12 @@ struct CADSTAR_PART_ENTRY
 
     /**
      * Pins with an implied electrical connection to a net, not part of any symbol
-     * (Note: we probably will need to import these into the first gate or something)
+     * (Note: we probably will need to import these into the first gate)
+     *
+     * First = name of net (e.g. VCC/GND)
+     * Second = list of pins of the part that are connected to this net
      */
-    std::vector<CADSTAR_PART_PIN> m_HiddenPins;
+    std::map<std::string, std::vector<CADSTAR_PART_PIN>> m_HiddenPins;
 };
 
 
@@ -191,24 +194,21 @@ struct CADSTAR_PART_PIN
             m_Identifier( 0 ),
             m_Position( CADSTAR_PIN_POSITION::TOP_RIGHT ),
             m_Type( CADSTAR_PIN_TYPE::UNCOMMITTED ),
-            m_Loading(),
+            m_Loading()
-            m_Signal()
     {};
 
     CADSTAR_PART_PIN( long aId, CADSTAR_PIN_POSITION aPos, CADSTAR_PIN_TYPE aType,
-                      std::optional<long> aLoading, std::optional<std::string> aSignal ) :
+                      std::optional<long> aLoading ) :
             m_Identifier( aId ),
             m_Position( aPos ),
             m_Type( aType ),
-            m_Loading( aLoading ),
+            m_Loading( aLoading )
-            m_Signal( aSignal )
     {};
 
     long                       m_Identifier;
     CADSTAR_PIN_POSITION       m_Position;
     CADSTAR_PIN_TYPE           m_Type;
     std::optional<long>        m_Loading;
-    std::optional<std::string> m_Signal; // e.g. GND or VCC
 };
 
 

common/plugins/cadstar/cadstar_parts_lib_parser.cpp

@@ -37,6 +37,7 @@ struct CADSTAR_LIB_PARSER_STATE
 {
     std::string                   m_CurrentString;
     std::string                   m_CurrentAttrName;
+    std::string                   m_CurrentSignalName;
     long                          m_CurrentLong = 0;
     std::vector<long>             m_CurrentPinEquivalenceGroup;
     std::set<std::string>         m_CurrentElementsParsed;
@@ -151,13 +152,13 @@ DEFINE_STRING_ACTION( STEM,                     m_CurrentPart.m_ComponentStem );
 DEFINE_STRING_ACTION( SYM_ELEMENT_NAME,         m_CurrentSwapGroup.m_Name );
 DEFINE_STRING_ACTION( USER_PART_ATTRIBUTE_NAME, m_CurrentAttrName );
 DEFINE_STRING_ACTION( ATTRIBUTE_NAME,           m_CurrentAttrName );
+DEFINE_STRING_ACTION( PIN_SIGNAL_NAME,          m_CurrentSignalName );
 DEFINE_STRING_ACTION( ACCEPTANCE_PART_NAME,     m_CurrentPart.m_AcceptancePartName );
 DEFINE_STRING_ACTION( ACCEPTANCE_TEXT,          m_CurrentPart.m_AcceptanceText );
 DEFINE_STRING_ACTION( SPICE_PART_NAME,          m_CurrentPart.m_SpicePartName );
 DEFINE_STRING_ACTION( SPICE_MODEL,              m_CurrentPart.m_SpiceModel );
 DEFINE_STRING_ACTION( SCH_NAME,                 m_CurrentSymbol.m_SymbolName );
 DEFINE_STRING_ACTION( SCH_ALTERNATE,            m_CurrentSymbol.m_SymbolAlternateName );
-DEFINE_STRING_ACTION( PIN_SIGNAL_NAME,          m_CurrentPin.m_Signal );
 
 
 // STRING SEGMENT action
@@ -239,14 +240,6 @@ DECLARE_SINGLE_MATCH_RULE( SPI_LINE, );
 DECLARE_SINGLE_MATCH_RULE( PAC_LINE, );
 
 
-//@todo remove once complete
-template <>
-struct CADSTAR_LIB_PARSER_ACTION<UNMATCHED_CONTENT>
-{
-    static void apply0( CADSTAR_LIB_PARSER_STATE& s ) { s.m_CurrentString = ""; }
-};
-
-
 template <>
 struct CADSTAR_LIB_PARSER_ACTION<PINNAME_ENTRY>
 {
@@ -412,7 +405,8 @@ struct CADSTAR_LIB_PARSER_ACTION<HIDDEN_PIN_ENTRY>
 {
     static void apply0( CADSTAR_LIB_PARSER_STATE& s )
     {
-        s.m_CurrentPart.m_HiddenPins.push_back( std::move( s.m_CurrentPinList[0] ) );
+        s.m_CurrentPart.m_HiddenPins.insert(
+                { std::move( s.m_CurrentSignalName ), std::move( s.m_CurrentPinList ) } );
         s.m_CurrentPinList.clear();
     }
 };

eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp

@@ -230,45 +230,37 @@ CADSTAR_SCH_ARCHIVE_LOADER::loadLibPart( const CADSTAR_PART_ENTRY& aPart )
             VECTOR2I direction( 0, -1 );
             int spacing = schIUScale.MilsToIU( 50 ); // for now, place on a 50mil grid
 
-            for( const CADSTAR_PART_PIN& csPin : aPart.m_HiddenPins )
+            for( auto& [signalName, csPinVector] : aPart.m_HiddenPins )
             {
-                std::unique_ptr<LIB_PIN> pin = std::make_unique<LIB_PIN>( retSym.get() );
+                for(  const CADSTAR_PART_PIN& csPin : csPinVector )
-
-                long pinNum = csPin.m_Identifier;
-                pin->SetNumber( wxString::Format( "%ld", pinNum ) );
-
-                if( csPin.m_Signal.has_value() )
                 {
-                    // Implied net connection
+                    std::unique_ptr<LIB_PIN> pin = std::make_unique<LIB_PIN>( retSym.get() );
-                    pin->SetName( csPin.m_Signal.value() );
+
+                    long pinNum = csPin.m_Identifier;
+                    pin->SetNumber( wxString::Format( "%ld", pinNum ) );
+                    pin->SetName( signalName );
                     pin->SetType( ELECTRICAL_PINTYPE::PT_POWER_IN );
-                }
-                else if( aPart.m_PinNamesMap.count( pinNum ) )
-                {
-                    pin->SetName( HandleTextOverbar( aPart.m_PinNamesMap.at( pinNum ) ) );
-                    pin->SetType( getKiCadPinType( csPin.m_Type ) );
-                }
 
-                pin->SetVisible( false );
+                    pin->SetVisible( false );
 
-                // Generate the coordinate for the pin. We don't want overlapping pins
+                    // Generate the coordinate for the pin. We don't want overlapping pins
-                // and ideally close to the center of the symbol, so we load pins sequentially
+                    // and ideally close to the center of the symbol, so we load pins
-                // in a spiral sequence
+                    // in a spiral sequence around the center
-                if( delta.x == delta.y
+                    if( delta.x == delta.y || ( delta.x < 0 && delta.x == -delta.y )
-                    || ( delta.x < 0 && delta.x == -delta.y )
+                        || ( delta.x > 0 && delta.x == 1 - delta.y ) )
-                    || ( delta.x > 0 && delta.x == 1 - delta.y ) )
+                    {
-                {
+                        // change direction
-                    // change direction
+                        direction = { -direction.y, direction.x };
-                    direction = { -direction.y, direction.x };
+                    }
+
+                    delta += direction;
+                    VECTOR2I offset = delta * spacing;
+                    pin->SetPosition( symCenter + offset );
+                    pin->SetLength( 0 ); //CADSTAR Pins are just a point (have no length)
+                    pin->SetShape( GRAPHIC_PINSHAPE::LINE );
+                    pin->SetUnit( unit );
+                    retSym->AddDrawItem( pin.release() );
                 }
-
-                delta += direction;
-                VECTOR2I offset = delta * spacing;
-                pin->SetPosition( symCenter + offset );
-                pin->SetLength( 0 ); //CADSTAR Pins are just a point (have no length)
-                pin->SetShape( GRAPHIC_PINSHAPE::LINE );
-                pin->SetUnit( unit );
-                retSym->AddDrawItem( pin.release() );
             }
         }
         else

qa/unittests/common/plugins/cadstar/test_cadstar_parts_parser.cpp

@@ -181,7 +181,7 @@ BOOST_AUTO_TEST_CASE( ReadFile )
 
         // Check hidden pins
         BOOST_REQUIRE_EQUAL( partEntry.m_HiddenPins.size(), 1 );
-        BOOST_CHECK_EQUAL( partEntry.m_HiddenPins[0].m_Signal, "GND" );
+        BOOST_CHECK_EQUAL( partEntry.m_HiddenPins.count( "GND" ), 1 );
         i++;
     }
 }
@@ -231,7 +231,7 @@ BOOST_AUTO_TEST_CASE( ReadContent )
             "<SCM Symbol Refname2>\r\n"
             "4.2!U:1000 5.1!I 6.3!Q\r\n"
             "/GND 7.0!G:2000\r\n"
-            "/VCC 8.0!P:2000\r\n";
+            "/VCC 8.0!P:2000 9.1 10.0\r\n";
     //"etc ...\r\n"
     //"/<Signame> <PinIdentifier>.<Position>!<Pintype>:<Loading>\r\n"
     //"/<Signame> <PinIdentifier>.<Position>!<Pintype>:<Loading>\r\n";
@@ -404,24 +404,31 @@ BOOST_AUTO_TEST_CASE( ReadContent )
    // Check symbols
     std::vector<CADSTAR_PART_SYMBOL_ENTRY> symbols = result.m_PartEntries[0].m_Symbols;
 
-    std::vector<CADSTAR_PART_SYMBOL_ENTRY> expectedSymbols = {
+    std::vector<CADSTAR_PART_SYMBOL_ENTRY> expectedSymbols =
-        { "<SCM Symbol Refname1>",
+    {
-          "<SCM Alternate Refname>",
+        {
-          { CADSTAR_PART_PIN( 1, CADSTAR_PIN_POSITION::TOP_RIGHT, CADSTAR_PIN_TYPE::TRISTATE_DRIVER,
+            "<SCM Symbol Refname1>",
-                              2000, std::optional<std::string>() ),
+            "<SCM Alternate Refname>",
-            CADSTAR_PART_PIN( 2, CADSTAR_PIN_POSITION::TOP_LEFT, CADSTAR_PIN_TYPE::TRISTATE_INPUT,
+            {
-                              std::optional<long>(), std::optional<std::string>() ),
+                { 1, CADSTAR_PIN_POSITION::TOP_RIGHT,   CADSTAR_PIN_TYPE::TRISTATE_DRIVER,
-            CADSTAR_PART_PIN( 3, CADSTAR_PIN_POSITION::BOTTOM_LEFT, CADSTAR_PIN_TYPE::TRISTATE_BIDIR,
+                    2000 },
-                              std::optional<long>(), std::optional<std::string>() ) }
+                { 2, CADSTAR_PIN_POSITION::TOP_LEFT,    CADSTAR_PIN_TYPE::TRISTATE_INPUT,
+                    std::nullopt },
+                { 3, CADSTAR_PIN_POSITION::BOTTOM_LEFT, CADSTAR_PIN_TYPE::TRISTATE_BIDIR,
+                    std::nullopt }
+            }
         },
-        { "<SCM Symbol Refname2>",
+        {
-          std::optional<std::string>(),
+            "<SCM Symbol Refname2>",
-          { CADSTAR_PART_PIN( 4, CADSTAR_PIN_POSITION::BOTTOM_LEFT, CADSTAR_PIN_TYPE::UNCOMMITTED,
+            std::nullopt,
-                              1000, std::optional<std::string>() ),
+            {
-            CADSTAR_PART_PIN( 5, CADSTAR_PIN_POSITION::TOP_LEFT, CADSTAR_PIN_TYPE::INPUT,
+                { 4, CADSTAR_PIN_POSITION::BOTTOM_LEFT,  CADSTAR_PIN_TYPE::UNCOMMITTED,
-                              std::optional<long>(), std::optional<std::string>() ),
+                    1000 },
-            CADSTAR_PART_PIN( 6, CADSTAR_PIN_POSITION::BOTTOM_RIGHT, CADSTAR_PIN_TYPE::OUTPUT_NOT_NORM_OR,
+                { 5, CADSTAR_PIN_POSITION::TOP_LEFT,     CADSTAR_PIN_TYPE::INPUT,
-                              std::optional<long>(), std::optional<std::string>() ) }
+                    std::nullopt },
+                { 6, CADSTAR_PIN_POSITION::BOTTOM_RIGHT, CADSTAR_PIN_TYPE::OUTPUT_NOT_NORM_OR,
+                    std::nullopt }
+            }
         }
     };
 
@@ -451,7 +458,6 @@ BOOST_AUTO_TEST_CASE( ReadContent )
                     BOOST_CHECK( itPinsA->m_Position == itPinsB->m_Position );
                     BOOST_CHECK( itPinsA->m_Type == itPinsB->m_Type );
                     BOOST_CHECK_EQUAL( itPinsA->m_Loading, itPinsB->m_Loading );
-                    BOOST_CHECK_EQUAL( itPinsA->m_Signal, itPinsB->m_Signal );
                 }
 
                 ++itPinsA;
@@ -464,30 +470,54 @@ BOOST_AUTO_TEST_CASE( ReadContent )
     }
 
     // Compare hidden pins
-    std::vector<CADSTAR_PART_PIN> expectedHiddenPins = {
+    std::map<std::string,std::vector<CADSTAR_PART_PIN>> expectedHiddenPins =
-        { 7, CADSTAR_PIN_POSITION::TOP_RIGHT, CADSTAR_PIN_TYPE::GROUND, 2000, "GND" },
+    {
-        { 8, CADSTAR_PIN_POSITION::TOP_RIGHT, CADSTAR_PIN_TYPE::POWER, 2000, "VCC" }
+        {
+            "GND",
+            {
+                { 7, CADSTAR_PIN_POSITION::TOP_RIGHT, CADSTAR_PIN_TYPE::GROUND, 2000 }
+            }
+        },
+        {
+            "VCC",
+            {
+               { 8,  CADSTAR_PIN_POSITION::TOP_RIGHT, CADSTAR_PIN_TYPE::POWER,       2000 },
+               { 9,  CADSTAR_PIN_POSITION::TOP_LEFT,  CADSTAR_PIN_TYPE::UNCOMMITTED, std::nullopt },
+               { 10, CADSTAR_PIN_POSITION::TOP_RIGHT, CADSTAR_PIN_TYPE::UNCOMMITTED, std::nullopt }
+            }
+        }
     };
 
     BOOST_REQUIRE_EQUAL( result.m_PartEntries[0].m_HiddenPins.size(), expectedHiddenPins.size() );
 
-    auto itPinsA = result.m_PartEntries[0].m_HiddenPins.begin();
+    auto itEntryA = result.m_PartEntries[0].m_HiddenPins.begin();
-    auto itPinsB = expectedHiddenPins.begin();
+    auto itEntryB = expectedHiddenPins.begin();
 
-    while( itPinsA != result.m_PartEntries[0].m_HiddenPins.end()
+    while( itEntryA != result.m_PartEntries[0].m_HiddenPins.end()
-           || itPinsB != expectedHiddenPins.end() )
+           || itEntryB != expectedHiddenPins.end() )
     {
-        BOOST_TEST_CONTEXT( "Pin Identifier = " << itPinsB->m_Signal )
+        BOOST_TEST_CONTEXT( "Check Hidden pins - Signal = " << itEntryB->first )
         {
-            BOOST_CHECK_EQUAL( itPinsA->m_Identifier, itPinsB->m_Identifier );
+            BOOST_CHECK_EQUAL( itEntryA->first, itEntryB->first );
-            BOOST_CHECK( itPinsA->m_Position == itPinsB->m_Position );
+            BOOST_REQUIRE_EQUAL( itEntryA->second.size(), itEntryB->second.size() );
-            BOOST_CHECK( itPinsA->m_Type == itPinsB->m_Type );
+
-            BOOST_CHECK_EQUAL( itPinsA->m_Loading, itPinsB->m_Loading );
+            auto itPinsA = itEntryA->second.begin();
-            BOOST_CHECK_EQUAL( itPinsA->m_Signal, itPinsB->m_Signal );
+            auto itPinsB = itEntryB->second.begin();
+
+            while( itPinsA != itEntryA->second.end() || itPinsB != itEntryB->second.end() )
+            {
+                BOOST_CHECK_EQUAL( itPinsA->m_Identifier, itPinsB->m_Identifier );
+                BOOST_CHECK( itPinsA->m_Position == itPinsB->m_Position );
+                BOOST_CHECK( itPinsA->m_Type == itPinsB->m_Type );
+                BOOST_CHECK_EQUAL( itPinsA->m_Loading, itPinsB->m_Loading );
+
+                ++itPinsA;
+                ++itPinsB;
+            }
         }
 
-        ++itPinsA;
+        ++itEntryA;
-        ++itPinsB;
+        ++itEntryB;
     }
 }