sch groups: add load/save support

eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp

@@ -38,6 +38,7 @@
 #include <sch_bitmap.h>
 #include <sch_bus_entry.h>
 #include <sch_edit_frame.h>       // SYMBOL_ORIENTATION_T
+#include <sch_group.h>
 #include <sch_io/kicad_sexpr/sch_io_kicad_sexpr.h>
 #include <sch_io/kicad_sexpr/sch_io_kicad_sexpr_common.h>
 #include <sch_io/kicad_sexpr/sch_io_kicad_sexpr_lib_cache.h>
@@ -337,6 +338,20 @@ void SCH_IO_KICAD_SEXPR::SaveSchematicFile( const wxString& aFileName, SCH_SHEET
 
     LOCALE_IO   toggle;     // toggles on, then off, the C locale, to write floating point values.
 
+    wxString sanityResult = aSheet->GetScreen()->GroupsSanityCheck();
+
+    if( sanityResult != wxEmptyString && m_queryUserCallback )
+    {
+        if( !m_queryUserCallback( _( "Internal Group Data Error" ), wxICON_ERROR,
+                                  wxString::Format( _( "Please report this bug.  Error validating group "
+                                                       "structure: %s\n\nSave anyway?" ),
+                                                    sanityResult ),
+                                  _( "Save Anyway" ) ) )
+        {
+            return;
+        }
+    }
+
     init( aSchematic, aProperties );
 
     wxFileName fn = aFileName;
@@ -470,6 +485,10 @@ void SCH_IO_KICAD_SEXPR::Format( SCH_SHEET* aSheet )
             saveTable( static_cast<SCH_TABLE*>( item ) );
             break;
 
+        case SCH_GROUP_T:
+            saveGroup( static_cast<SCH_GROUP*>( item ) );
+            break;
+
         default:
             wxASSERT( "Unexpected schematic object type in SCH_IO_KICAD_SEXPR::Format()" );
         }
@@ -1443,6 +1462,36 @@ void SCH_IO_KICAD_SEXPR::saveTable( SCH_TABLE* aTable )
 }
 
 
+void SCH_IO_KICAD_SEXPR::saveGroup( SCH_GROUP* aGroup )
+{
+    // Don't write empty groups
+    if( aGroup->GetItems().empty() )
+        return;
+
+    m_out->Print( "(group %s", m_out->Quotew( aGroup->GetName() ).c_str() );
+
+    KICAD_FORMAT::FormatUuid( m_out, aGroup->m_Uuid );
+
+    if( aGroup->IsLocked() )
+        KICAD_FORMAT::FormatBool( m_out, "locked", true );
+
+    wxArrayString memberIds;
+
+    for( EDA_ITEM* member : aGroup->GetItems() )
+        memberIds.Add( member->m_Uuid.AsString() );
+
+    memberIds.Sort();
+
+    m_out->Print( "(members" );
+
+    for( const wxString& memberId : memberIds )
+        m_out->Print( " %s", m_out->Quotew( memberId ).c_str() );
+
+    m_out->Print( ")" ); // Close `members` token.
+    m_out->Print( ")" ); // Close `group` token.
+}
+
+
 void SCH_IO_KICAD_SEXPR::saveBusAlias( std::shared_ptr<BUS_ALIAS> aAlias )
 {
     wxCHECK_RET( aAlias != nullptr, "BUS_ALIAS* is NULL" );

eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.h

@@ -48,6 +48,7 @@ class SCH_BUS_ENTRY_BASE;
 class SCH_TEXT;
 class SCH_TEXTBOX;
 class SCH_TABLE;
+class SCH_GROUP;
 class SCH_SYMBOL;
 class SCH_FIELD;
 struct SCH_SYMBOL_INSTANCE;
@@ -158,6 +159,7 @@ private:
     void saveText( SCH_TEXT* aText );
     void saveTextBox( SCH_TEXTBOX* aText );
     void saveTable( SCH_TABLE* aTable );
+    void saveGroup( SCH_GROUP* aGroup );
     void saveBusAlias( std::shared_ptr<BUS_ALIAS> aAlias );
     void saveInstances( const std::vector<SCH_SHEET_INSTANCE>& aSheets );
 
@@ -180,6 +182,8 @@ protected:
 
     /// initialize PLUGIN like a constructor would.
     void init( SCHEMATIC* aSchematic, const std::map<std::string, UTF8>* aProperties = nullptr );
+
+    std::function<bool( wxString aTitle, int aIcon, wxString aMsg, wxString aAction )> m_queryUserCallback;
 };
 
 #endif  // SCH_IO_KICAD_SEXPR_H_

eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.cpp

@@ -50,6 +50,7 @@
 #include <sch_symbol.h>
 #include <sch_edit_frame.h>          // SYM_ORIENT_XXX
 #include <sch_field.h>
+#include <sch_group.h>
 #include <sch_line.h>
 #include <sch_rule_area.h>
 #include <sch_textbox.h>
@@ -2710,6 +2711,10 @@ void SCH_IO_KICAD_SEXPR_PARSER::ParseSchematic( SCH_SHEET* aSheet, bool aIsCopya
 
         switch( token )
         {
+        case T_group:
+            parseGroup();
+            break;
+
         case T_generator:
             // (generator "genname"); we don't care about it at the moment.
             NeedSYMBOL();
@@ -3000,6 +3005,8 @@ void SCH_IO_KICAD_SEXPR_PARSER::ParseSchematic( SCH_SHEET* aSheet, bool aIsCopya
     screen->UpdateLocalLibSymbolLinks();
     screen->FixupEmbeddedData();
 
+    resolveGroups( screen );
+
     SCHEMATIC* schematic = screen->Schematic();
 
     if( !schematic )
@@ -4797,3 +4804,127 @@ void SCH_IO_KICAD_SEXPR_PARSER::parseBusAlias( SCH_SCREEN* aScreen )
 
     aScreen->AddBusAlias( busAlias );
 }
+
+
+void SCH_IO_KICAD_SEXPR_PARSER::parseGroupMembers( GROUP_INFO& aGroupInfo )
+{
+    T token;
+
+    while( ( token = NextTok() ) != T_RIGHT )
+    {
+        // This token is the Uuid of the item in the group.
+        // Since groups are serialized at the end of the file/footprint, the Uuid should already
+        // have been seen and exist in the board.
+        KIID uuid( CurStr() );
+        aGroupInfo.memberUuids.push_back( uuid );
+    }
+}
+
+
+void SCH_IO_KICAD_SEXPR_PARSER::parseGroup()
+{
+    wxCHECK_RET( CurTok() == T_group,
+                 wxT( "Cannot parse " ) + GetTokenString( CurTok() ) + wxT( " as PCB_GROUP." ) );
+
+    T token;
+
+    m_groupInfos.push_back( GROUP_INFO() );
+    GROUP_INFO& groupInfo = m_groupInfos.back();
+
+    while( ( token = NextTok() ) != T_LEFT )
+    {
+        if( token == T_STRING )
+            groupInfo.name = FromUTF8();
+        else
+            Expecting( "group name or locked" );
+    }
+
+    for( ; token != T_RIGHT; token = NextTok() )
+    {
+        if( token != T_LEFT )
+            Expecting( T_LEFT );
+
+        token = NextTok();
+
+        switch( token )
+        {
+        case T_uuid:
+            NextTok();
+            groupInfo.uuid = parseKIID();
+            NeedRIGHT();
+            break;
+
+        case T_members:
+        {
+            parseGroupMembers( groupInfo );
+            break;
+        }
+
+        default:
+            Expecting( "uuid, members" );
+        }
+    }
+}
+
+
+void SCH_IO_KICAD_SEXPR_PARSER::resolveGroups( SCH_SCREEN* aParent )
+{
+    if( !aParent )
+        return;
+
+    auto getItem =
+            [&]( const KIID& aId )
+            {
+                SCH_ITEM* aItem = nullptr;
+
+                for( SCH_ITEM* item : aParent->Items() )
+                {
+                    if( item->m_Uuid == aId )
+                    {
+                        aItem = item;
+                        break;
+                    }
+                }
+
+                return aItem;
+            };
+
+    // Now that we've parsed the other Uuids in the file we can resolve the uuids referred
+    // to in the group declarations we saw.
+    //
+    // First add all group objects so subsequent GetItem() calls for nested groups work.
+    for( const GROUP_INFO& groupInfo : m_groupInfos )
+    {
+        SCH_GROUP* group = nullptr;
+
+        group = new SCH_GROUP( aParent );
+        group->SetName( groupInfo.name );
+
+        const_cast<KIID&>( group->m_Uuid ) = groupInfo.uuid;
+
+        aParent->Append( group );
+    }
+
+    for( const GROUP_INFO& groupInfo : m_groupInfos )
+    {
+        SCH_GROUP* group = static_cast<SCH_GROUP*>( getItem( groupInfo.uuid ) );
+
+        if( group && group->Type() == SCH_GROUP_T )
+        {
+            for( const KIID& aUuid : groupInfo.memberUuids )
+            {
+                SCH_ITEM* gItem = getItem( aUuid );
+
+                if( !gItem || gItem->Type() == NOT_USED )
+                {
+                    // This is the deleted item singleton, which means we didn't find the uuid.
+                    continue;
+                }
+
+                group->AddItem( gItem );
+            }
+        }
+    }
+
+    aParent->GroupsSanityCheck( true );
+}

eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.h

@@ -113,6 +113,19 @@ public:
     int GetParsedRequiredVersion() const { return m_requiredVersion; }
 
 private:
+    // Group membership info refers to other Uuids in the file.
+    // We don't want to rely on group declarations being last in the file, so
+    // we store info about the group declarations here during parsing and then resolve
+    // them into BOARD_ITEM* after we've parsed the rest of the file.
+    struct GROUP_INFO
+    {
+        virtual ~GROUP_INFO() = default; // Make polymorphic
+
+        wxString          name;
+        KIID              uuid;
+        std::vector<KIID> memberUuids;
+    };
+
     void checkpoint();
 
     KIID parseKIID();
@@ -210,6 +223,9 @@ private:
     void parseSchSymbolInstances( SCH_SCREEN* aScreen );
     void parseSchSheetInstances( SCH_SHEET* aRootSheet, SCH_SCREEN* aScreen );
 
+    void parseGroup();
+    void parseGroupMembers( GROUP_INFO& aGroupInfo );
+
     SCH_SHEET_PIN* parseSchSheetPin( SCH_SHEET* aSheet );
     SCH_FIELD* parseSchField( SCH_ITEM* aParent );
     SCH_SYMBOL* parseSchematicSymbol();
@@ -232,6 +248,8 @@ private:
     SCH_TABLE* parseSchTable();
     void parseBusAlias( SCH_SCREEN* aScreen );
 
+    void resolveGroups( SCH_SCREEN* aParent );
+
 private:
     int      m_requiredVersion;   ///< Set to the symbol library file version required.
     wxString m_generatorVersion;
@@ -251,6 +269,8 @@ private:
 
     /// The rootsheet for full project loads or null for importing a schematic.
     SCH_SHEET*         m_rootSheet;
+
+    std::vector<GROUP_INFO> m_groupInfos;
 };
 
 #endif    // SCH_IO_KICAD_SEXPR_PARSER_H_

eeschema/sch_screen.cpp

@@ -1627,6 +1627,86 @@ bool SCH_SCREEN::HasInstanceDataFromOtherProjects() const
 }
 
 
+wxString SCH_SCREEN::GroupsSanityCheck( bool repair )
+{
+    if( repair )
+    {
+        while( GroupsSanityCheckInternal( repair ) != wxEmptyString )
+        {
+        };
+
+        return wxEmptyString;
+    }
+    return GroupsSanityCheckInternal( repair );
+}
+
+
+wxString SCH_SCREEN::GroupsSanityCheckInternal( bool repair )
+{
+    // Cycle detection
+    //
+    // Each group has at most one parent group.
+    // So we start at group 0 and traverse the parent chain, marking groups seen along the way.
+    // If we ever see a group that we've already marked, that's a cycle.
+    // If we reach the end of the chain, we know all groups in that chain are not part of any cycle.
+    //
+    // Algorithm below is linear in the # of groups because each group is visited only once.
+    // There may be extra time taken due to the container access calls and iterators.
+    //
+    // Groups we know are cycle free
+    std::unordered_set<EDA_GROUP*> knownCycleFreeGroups;
+    // Groups in the current chain we're exploring.
+    std::unordered_set<EDA_GROUP*> currentChainGroups;
+    // Groups we haven't checked yet.
+    std::unordered_set<EDA_GROUP*> toCheckGroups;
+
+    // Initialize set of groups and generators to check that could participate in a cycle.
+    for( SCH_ITEM* item : Items().OfType( SCH_GROUP_T ) )
+        toCheckGroups.insert( static_cast<SCH_GROUP*>( item ) );
+
+    while( !toCheckGroups.empty() )
+    {
+        currentChainGroups.clear();
+        EDA_GROUP* group = *toCheckGroups.begin();
+
+        while( true )
+        {
+            if( currentChainGroups.find( group ) != currentChainGroups.end() )
+            {
+                if( repair )
+                    Remove( static_cast<SCH_ITEM*>( group->AsEdaItem() ) );
+
+                return "Cycle detected in group membership";
+            }
+            else if( knownCycleFreeGroups.find( group ) != knownCycleFreeGroups.end() )
+            {
+                // Parent is a group we know does not lead to a cycle
+                break;
+            }
+
+            currentChainGroups.insert( group );
+            // We haven't visited currIdx yet, so it must be in toCheckGroups
+            toCheckGroups.erase( group );
+
+            group = group->AsEdaItem()->GetParentGroup();
+
+            if( !group )
+            {
+                // end of chain and no cycles found in this chain
+                break;
+            }
+        }
+
+        // No cycles found in chain, so add it to set of groups we know don't participate
+        // in a cycle.
+        knownCycleFreeGroups.insert( currentChainGroups.begin(), currentChainGroups.end() );
+    }
+
+    // Success
+    return "";
+}
+
+
 bool SCH_SCREEN::InProjectPath() const
 {
     wxCHECK( Schematic() && !m_fileName.IsEmpty(), false );

eeschema/sch_screen.h

@@ -611,6 +611,21 @@ public:
      */
     bool HasInstanceDataFromOtherProjects() const;
 
+    /**
+     * Consistency check of internal m_groups structure.
+     *
+     * @param repair if true, modify groups structure until it passes the sanity check.
+     * @return empty string on success.  Or error description if there's a problem.
+     */
+    wxString GroupsSanityCheck( bool repair = false );
+
+    /**
+     * @param repair if true, make one modification to groups structure that brings it
+     *        closer to passing the sanity check.
+     * @return empty string on success.  Or error description if there's a problem.
+     */
+    wxString GroupsSanityCheckInternal( bool repair );
+
 private:
     friend SCH_EDIT_FRAME;     // Only to populate m_symbolInstances.
     friend SCH_IO_KICAD_SEXPR_PARSER;   // Only to load instance information from schematic file.

eeschema/schematic.keywords

@@ -58,6 +58,7 @@ generator
 generator_version
 global
 global_label
+group
 hatch
 header
 hide