From 5c07441e246633cc9d04c1da55de29a145592b9f Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 17 Sep 2021 21:25:05 +0100 Subject: [PATCH] Remove Setup Dialog assignment of netclasses to buses. Also removes the message bar display of assigned netclass for buses and bus-to-bus entries. Also fixes a bug where assigning a netclass via the canvas only looked at the first level of bus members (and not any nested members). Also fixes a bug where the bus name validator tried to validate a vector bus first -- which doesn't work as a vector bus may be nested in a group bus. Also fixes a bug where we were failing to check for illegal chars in bus definitions which otherwise passed the bus parsers. See additional comments in the bug report. Fixes https://gitlab.com/kicad/code/kicad/issues/9160 --- common/project/net_settings.cpp | 65 +++++++++++---------------- eeschema/cross-probing.cpp | 2 - eeschema/eeschema_config.cpp | 2 +- eeschema/sch_bus_entry.cpp | 15 ++++--- eeschema/sch_line.cpp | 15 ++++--- eeschema/sch_text.cpp | 10 ++--- eeschema/schematic.cpp | 5 ++- eeschema/tools/sch_editor_control.cpp | 15 +++++-- include/project/net_settings.h | 8 +--- pcbnew/files.cpp | 2 +- pcbnew/pcb_edit_frame.cpp | 2 +- 11 files changed, 69 insertions(+), 72 deletions(-) diff --git a/common/project/net_settings.cpp b/common/project/net_settings.cpp index 80da3cc0ac..79f884d0c1 100644 --- a/common/project/net_settings.cpp +++ b/common/project/net_settings.cpp @@ -31,7 +31,8 @@ // const int netSettingsSchemaVersion = 0; -const int netSettingsSchemaVersion = 1; // new overbar syntax +// const int netSettingsSchemaVersion = 1; // new overbar syntax +const int netSettingsSchemaVersion = 2; // exclude buses from netclass members static OPT getInPcbUnits( const nlohmann::json& aObj, const std::string& aKey, @@ -193,7 +194,26 @@ NET_SETTINGS::NET_SETTINGS( JSON_SETTINGS* aParent, const std::string& aPath ) : if( entry.contains( "nets" ) && entry["nets"].is_array() ) { for( const auto& net : entry["nets"].items() ) - nc->Add( net.value().get() ); + { + wxString netname = net.value().get(); + + if( m_schemaVersion < 2 ) + { + // Strip out buses from older 5.99 implementations. They were + // a world of hurt, never fully functional, and are functionally + // replaced by assigning a netclass to a bus on the canvas. + wxString unescaped = UnescapeString( netname ); + wxString prefix; + std::vector members; + + if( ParseBusVector( unescaped, &prefix, &members ) ) + continue; + else if( ParseBusGroup( unescaped, &prefix, &members ) ) + continue; + } + + nc->Add( netname ); + } } if( entry.contains( "pcb_color" ) && entry["pcb_color"].is_string() ) @@ -211,8 +231,6 @@ NET_SETTINGS::NET_SETTINGS( JSON_SETTINGS* aParent, const std::string& aPath ) : for( const wxString& net : *nc ) m_NetClassAssignments[ net ] = nc->GetName(); } - - ResolveNetClassAssignments(); }, {} ) ); @@ -519,44 +537,13 @@ bool NET_SETTINGS::ParseBusGroup( const wxString& aGroup, wxString* aName, } -void NET_SETTINGS::ResolveNetClassAssignments( bool aRebuildFromScratch ) +void NET_SETTINGS::RebuildNetClassAssignments() { - std::map baseList; - - if( aRebuildFromScratch ) - { - for( const std::pair& netclass : m_NetClasses ) - { - for( const wxString& net : *netclass.second ) - baseList[ net ] = netclass.second->GetName(); - } - } - else - { - baseList = m_NetClassAssignments; - } - m_NetClassAssignments.clear(); - for( const auto& ii : baseList ) + for( const std::pair& netclass : m_NetClasses ) { - m_NetClassAssignments[ ii.first ] = ii.second; - - wxString unescaped = UnescapeString( ii.first ); - wxString prefix; - std::vector members; - - if( ParseBusVector( unescaped, &prefix, &members ) ) - { - prefix = wxEmptyString; - } - else if( ParseBusGroup( unescaped, &prefix, &members ) ) - { - if( !prefix.IsEmpty() ) - prefix += wxT( "." ); - } - - for( wxString& member : members ) - m_NetClassAssignments[ prefix + member ] = ii.second; + for( const wxString& net : *netclass.second ) + m_NetClassAssignments[ net ] = netclass.second->GetName(); } } diff --git a/eeschema/cross-probing.cpp b/eeschema/cross-probing.cpp index fffed2a54a..10643bb437 100644 --- a/eeschema/cross-probing.cpp +++ b/eeschema/cross-probing.cpp @@ -658,8 +658,6 @@ void SCH_EDIT_FRAME::KiwayMailIn( KIWAY_EXPRESS& mail ) if( netclass ) netclass->Add( ii.first ); } - - netSettings.ResolveNetClassAssignments(); } break; diff --git a/eeschema/eeschema_config.cpp b/eeschema/eeschema_config.cpp index fe85873653..1dd711ae8f 100644 --- a/eeschema/eeschema_config.cpp +++ b/eeschema/eeschema_config.cpp @@ -115,7 +115,7 @@ void SCH_EDIT_FRAME::ShowSchematicSetupDialog( const wxString& aInitialPage ) if( dlg.ShowQuasiModal() == wxID_OK ) { - Prj().GetProjectFile().NetSettings().ResolveNetClassAssignments( true ); + Prj().GetProjectFile().NetSettings().RebuildNetClassAssignments(); SaveProjectSettings(); diff --git a/eeschema/sch_bus_entry.cpp b/eeschema/sch_bus_entry.cpp index 29cc26b92c..0e1ee1683c 100644 --- a/eeschema/sch_bus_entry.cpp +++ b/eeschema/sch_bus_entry.cpp @@ -501,14 +501,17 @@ void SCH_BUS_ENTRY_BASE::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, MSG_PANEL_ITEM { conn->AppendInfoToMsgPanel( aList ); - NET_SETTINGS& netSettings = Schematic()->Prj().GetProjectFile().NetSettings(); - wxString netname = conn->Name(); - wxString netclassName = netSettings.m_NetClasses.GetDefaultPtr()->GetName(); + if( !conn->IsBus() ) + { + NET_SETTINGS& netSettings = Schematic()->Prj().GetProjectFile().NetSettings(); + wxString netname = conn->Name(); + wxString netclassName = netSettings.m_NetClasses.GetDefaultPtr()->GetName(); - if( netSettings.m_NetClassAssignments.count( netname ) ) - netclassName = netSettings.m_NetClassAssignments[ netname ]; + if( netSettings.m_NetClassAssignments.count( netname ) ) + netclassName = netSettings.m_NetClassAssignments[ netname ]; - aList.push_back( MSG_PANEL_ITEM( _( "Assigned Netclass" ), netclassName ) ); + aList.push_back( MSG_PANEL_ITEM( _( "Assigned Netclass" ), netclassName ) ); + } } } diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp index af373692d6..37eb99039d 100644 --- a/eeschema/sch_line.cpp +++ b/eeschema/sch_line.cpp @@ -898,14 +898,17 @@ void SCH_LINE::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, MSG_PANEL_ITEMS& aList ) { conn->AppendInfoToMsgPanel( aList ); - NET_SETTINGS& netSettings = Schematic()->Prj().GetProjectFile().NetSettings(); - wxString netname = conn->Name(); - wxString netclassName = netSettings.m_NetClasses.GetDefaultPtr()->GetName(); + if( !conn->IsBus() ) + { + NET_SETTINGS& netSettings = Schematic()->Prj().GetProjectFile().NetSettings(); + wxString netname = conn->Name(); + wxString netclassName = netSettings.m_NetClasses.GetDefaultPtr()->GetName(); - if( netSettings.m_NetClassAssignments.count( netname ) ) - netclassName = netSettings.m_NetClassAssignments[ netname ]; + if( netSettings.m_NetClassAssignments.count( netname ) ) + netclassName = netSettings.m_NetClassAssignments[ netname ]; - aList.push_back( MSG_PANEL_ITEM( _( "Assigned Netclass" ), netclassName ) ); + aList.push_back( MSG_PANEL_ITEM( _( "Assigned Netclass" ), netclassName ) ); + } } } diff --git a/eeschema/sch_text.cpp b/eeschema/sch_text.cpp index 9e9a31cf52..6ea4acb714 100644 --- a/eeschema/sch_text.cpp +++ b/eeschema/sch_text.cpp @@ -791,14 +791,14 @@ void SCH_TEXT::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, MSG_PANEL_ITEMS& aList ) msg = MessageTextFromValue( aFrame->GetUserUnits(), GetTextWidth() ); aList.push_back( MSG_PANEL_ITEM( _( "Size" ), msg ) ); - SCH_EDIT_FRAME* frame = dynamic_cast( aFrame ); + SCH_CONNECTION* conn = dynamic_cast( aFrame ) ? Connection() : nullptr; - if( frame ) + if( conn ) { - if( SCH_CONNECTION* conn = Connection() ) - { - conn->AppendInfoToMsgPanel( aList ); + conn->AppendInfoToMsgPanel( aList ); + if( !conn->IsBus() ) + { NET_SETTINGS& netSettings = Schematic()->Prj().GetProjectFile().NetSettings(); const wxString& netname = conn->Name( true ); diff --git a/eeschema/schematic.cpp b/eeschema/schematic.cpp index a36d36b130..7b4a61d9fa 100644 --- a/eeschema/schematic.cpp +++ b/eeschema/schematic.cpp @@ -241,8 +241,11 @@ std::vector SCHEMATIC::GetNetClassAssignmentCandidates() { CONNECTION_SUBGRAPH* subgraph = pair.second[0]; - if( subgraph->GetDriverPriority() >= CONNECTION_SUBGRAPH::PRIORITY::PIN ) + if( !subgraph->m_driver_connection->IsBus() + && subgraph->GetDriverPriority() >= CONNECTION_SUBGRAPH::PRIORITY::PIN ) + { names.emplace_back( pair.first.first ); + } } return names; diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index 2ae20251d5..76501decfc 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -992,9 +992,17 @@ int SCH_EDITOR_CONTROL::AssignNetclass( const TOOL_EVENT& aEvent ) if( conn->IsBus() ) { for( const std::shared_ptr& member : conn->Members() ) - netNames.Add( member->Name() ); - - netNames.Add( conn->Name() ); + { + if( member->IsBus() ) + { + for( const std::shared_ptr& subMember : member->Members() ) + netNames.Add( subMember->Name() ); + } + else + { + netNames.Add( member->Name() ); + } + } } else { @@ -1046,7 +1054,6 @@ int SCH_EDITOR_CONTROL::AssignNetclass( const TOOL_EVENT& aEvent ) newNetclass->Add( netName ); netSettings.m_NetClassAssignments[netName] = netclassName; - netSettings.ResolveNetClassAssignments(); } } } diff --git a/include/project/net_settings.h b/include/project/net_settings.h index 6fd7484442..4200ddcf9d 100644 --- a/include/project/net_settings.h +++ b/include/project/net_settings.h @@ -79,13 +79,9 @@ public: std::vector* aMemberList ); /** - * Explode the list of netclass assignments to include atomic members of composite labels - * (buses). - * - * @param aRebuildFromScratch indicates the assignments should be rebuilt from the netclass - * membership lists before resolving. + * Rebuild netclass assignments from the netclass membership lists. */ - void ResolveNetClassAssignments( bool aRebuildFromScratch = false ); + void RebuildNetClassAssignments(); private: bool migrateSchema0to1(); diff --git a/pcbnew/files.cpp b/pcbnew/files.cpp index 818037bef6..ff6dbcb5fc 100644 --- a/pcbnew/files.cpp +++ b/pcbnew/files.cpp @@ -753,7 +753,7 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in { Prj().SetReadOnly( false ); - Prj().GetProjectFile().NetSettings().ResolveNetClassAssignments( true ); + Prj().GetProjectFile().NetSettings().RebuildNetClassAssignments(); // Before we had a copper edge clearance setting, the edge line widths could be used // as a kludge to control them. So if there's no setting then infer it from the diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index e19b1a09a0..928f0529c6 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -904,7 +904,7 @@ void PCB_EDIT_FRAME::ShowBoardSetupDialog( const wxString& aInitialPage ) if( dlg.ShowQuasiModal() == wxID_OK ) { - Prj().GetProjectFile().NetSettings().ResolveNetClassAssignments( true ); + Prj().GetProjectFile().NetSettings().RebuildNetClassAssignments(); GetBoard()->SynchronizeNetsAndNetClasses(); SaveProjectSettings();