From be0aad5984b81484278a9a799ae876dcfc82548e Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Sun, 30 Aug 2020 15:42:46 -0400 Subject: [PATCH] Move similar labels check to new connectivity engine --- eeschema/connection_graph.cpp | 19 ++- eeschema/dialogs/dialog_erc.cpp | 8 +- eeschema/erc.cpp | 223 ++++++-------------------------- eeschema/erc.h | 6 + eeschema/netlist_object.h | 9 -- 5 files changed, 59 insertions(+), 206 deletions(-) diff --git a/eeschema/connection_graph.cpp b/eeschema/connection_graph.cpp index a1b89fc62b..0aaaf8b2aa 100644 --- a/eeschema/connection_graph.cpp +++ b/eeschema/connection_graph.cpp @@ -2517,26 +2517,30 @@ bool CONNECTION_GRAPH::ercCheckLabels( const CONNECTION_SUBGRAPH* aSubgraph ) if( !text ) return true; - bool is_global = text->Type() == SCH_GLOBAL_LABEL_T; + bool isGlobal = text->Type() == SCH_GLOBAL_LABEL_T; wxCHECK_MSG( m_schematic, true, "Null m_schematic in CONNECTION_GRAPH::ercCheckLabels" ); // Global label check can be disabled independently - if( !m_schematic->ErcSettings().IsTestEnabled( ERCE_GLOBLABEL ) && is_global ) + if( !m_schematic->ErcSettings().IsTestEnabled( ERCE_GLOBLABEL ) && isGlobal ) return true; wxString name = EscapeString( text->GetShownText(), CTX_NETNAME ); - if( is_global ) + if( isGlobal ) { // This will be set to true if the global is connected to a pin above, but we // want to reset this to false so that globals get flagged if they only have a // single instance has_other_connections = false; - if( m_net_name_to_subgraphs_map.count( name ) - && m_net_name_to_subgraphs_map.at( name ).size() > 1 ) - has_other_connections = true; + if( m_net_name_to_subgraphs_map.count( name ) ) + { + if( m_net_name_to_subgraphs_map.at( name ).size() > 1 ) + has_other_connections = true; + else if( aSubgraph->m_multiple_drivers ) + has_other_connections = true; + } } else if( text->Type() == SCH_HIER_LABEL_T ) { @@ -2560,7 +2564,8 @@ bool CONNECTION_GRAPH::ercCheckLabels( const CONNECTION_SUBGRAPH* aSubgraph ) if( !has_other_connections ) { - std::shared_ptr ercItem = ERC_ITEM::Create( is_global ? ERCE_GLOBLABEL : ERCE_LABEL_NOT_CONNECTED ); + std::shared_ptr ercItem = ERC_ITEM::Create( isGlobal ? ERCE_GLOBLABEL + : ERCE_LABEL_NOT_CONNECTED ); ercItem->SetItems( text ); SCH_MARKER* marker = new SCH_MARKER( ercItem, text->GetPosition() ); diff --git a/eeschema/dialogs/dialog_erc.cpp b/eeschema/dialogs/dialog_erc.cpp index aecabff865..9f820da911 100644 --- a/eeschema/dialogs/dialog_erc.cpp +++ b/eeschema/dialogs/dialog_erc.cpp @@ -263,15 +263,9 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter ) if( settings.IsTestEnabled( ERCE_DIFFERENT_UNIT_FP ) ) { aReporter.ReportTail( _( "Checking footprints...\n" ), RPT_SEVERITY_INFO ); - tester.TestMultiunitFootprints(); } - std::unique_ptr objectsConnectedList( m_parent->BuildNetListBase() ); - - // Reset the connection type indicator - objectsConnectedList->ResetConnectionsType(); - aReporter.ReportTail( _( "Checking pins...\n" ), RPT_SEVERITY_INFO ); if( settings.IsTestEnabled( ERCE_DIFFERENT_UNIT_NET ) ) @@ -286,7 +280,7 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter ) if( settings.IsTestEnabled( ERCE_SIMILAR_LABELS ) ) { aReporter.ReportTail( _( "Checking labels...\n" ), RPT_SEVERITY_INFO ); - objectsConnectedList->TestforSimilarLabels(); + tester.TestSimilarLabels(); } if( settings.IsTestEnabled( ERCE_UNRESOLVED_VARIABLE ) ) diff --git a/eeschema/erc.cpp b/eeschema/erc.cpp index 611b1a58da..7b2c3e626b 100644 --- a/eeschema/erc.cpp +++ b/eeschema/erc.cpp @@ -713,198 +713,55 @@ int ERC_TESTER::TestMultUnitPinConflicts() } -// this code try to detect similar labels, i.e. labels which are identical -// when they are compared using case insensitive coparisons. - - -// A helper struct to compare NETLIST_OBJECT items by sheetpath and label texts -// for a std::set container -// the full text is "sheetpath+label" for local labels and "label" for global labels -struct compare_labels +int ERC_TESTER::TestSimilarLabels() { - bool operator() ( const NETLIST_OBJECT* lab1, const NETLIST_OBJECT* lab2 ) const + const NET_MAP& nets = m_schematic->ConnectionGraph()->GetNetMap(); + + int errors = 0; + + std::unordered_map labelMap; + + for( const std::pair> net : nets ) { - wxString str1 = lab1->m_SheetPath.PathAsString() + lab1->m_Label; - wxString str2 = lab2->m_SheetPath.PathAsString() + lab2->m_Label; + const wxString& netName = net.first.first; + std::vector pins; - return str1.Cmp( str2 ) < 0; - } -}; - -struct compare_label_names -{ - bool operator() ( const NETLIST_OBJECT* lab1, const NETLIST_OBJECT* lab2 ) const - { - return lab1->m_Label.Cmp( lab2->m_Label ) < 0; - } -}; - -struct compare_paths -{ - bool operator() ( const NETLIST_OBJECT* lab1, const NETLIST_OBJECT* lab2 ) const - { - return lab1->m_SheetPath.Path() < lab2->m_SheetPath.Path(); - } -}; - -// Helper functions to build the warning messages about Similar Labels: -static int countIndenticalLabels( std::vector& aList, NETLIST_OBJECT* aRef ); -static void SimilarLabelsDiagnose( NETLIST_OBJECT* aItemA, NETLIST_OBJECT* aItemB ); - - -void NETLIST_OBJECT_LIST::TestforSimilarLabels() -{ - // Similar labels which are different when using case sensitive comparisons - // but are equal when using case insensitive comparisons - - // list of all labels (used the better item to build diag messages) - std::vector fullLabelList; - // list of all labels , each label appears only once (used to to detect similar labels) - std::set uniqueLabelList; - wxString msg; - - // Build a list of differents labels. If inside a given sheet there are - // more than one given label, only one label is stored. - // not also the sheet labels are not taken in account for 2 reasons: - // * they are in the root sheet but they are seen only from the child sheet - // * any mismatch between child sheet hierarchical labels and the sheet label - // already detected by ERC - for( unsigned netItem = 0; netItem < size(); ++netItem ) - { - switch( GetItemType( netItem ) ) + for( CONNECTION_SUBGRAPH* subgraph : net.second ) { - case NETLIST_ITEM::LABEL: - case NETLIST_ITEM::BUSLABELMEMBER: - case NETLIST_ITEM::PINLABEL: - case NETLIST_ITEM::GLOBBUSLABELMEMBER: - case NETLIST_ITEM::HIERLABEL: - case NETLIST_ITEM::HIERBUSLABELMEMBER: - case NETLIST_ITEM::GLOBLABEL: - // add this label in lists - uniqueLabelList.insert( GetItem( netItem ) ); - fullLabelList.push_back( GetItem( netItem ) ); - break; - - case NETLIST_ITEM::SHEETLABEL: - case NETLIST_ITEM::SHEETBUSLABELMEMBER: - default: - break; - } - } - - // build global labels and compare - std::set loc_labelList; - - for( NETLIST_OBJECT* label : uniqueLabelList ) - { - if( label->IsLabelGlobal() ) - loc_labelList.insert( label ); - } - - // compare global labels (same label names appears only once in list) - for( auto it = loc_labelList.begin(); it != loc_labelList.end(); ++it ) - { - auto it_aux = it; - - for( ++it_aux; it_aux != loc_labelList.end(); ++it_aux ) - { - if( (*it)->m_Label.CmpNoCase( (*it_aux)->m_Label ) == 0 ) + for( EDA_ITEM* item : subgraph->m_items ) { - // Create new marker for ERC. - int cntA = countIndenticalLabels( fullLabelList, *it ); - int cntB = countIndenticalLabels( fullLabelList, *it_aux ); - - if( cntA <= cntB ) - SimilarLabelsDiagnose( (*it), (*it_aux) ); - else - SimilarLabelsDiagnose( (*it_aux), (*it) ); - } - } - } - - // Build paths list - std::set pathsList; - - for( NETLIST_OBJECT* label : uniqueLabelList ) - pathsList.insert( label ); - - // Examine each label inside a sheet path: - for( NETLIST_OBJECT* candidate : pathsList ) - { - loc_labelList.clear(); - - for( NETLIST_OBJECT* uniqueLabel : uniqueLabelList) - { - if( candidate->m_SheetPath.Path() == uniqueLabel->m_SheetPath.Path() ) - loc_labelList.insert( uniqueLabel ); - } - - // at this point, loc_labelList contains labels of the current sheet path. - // Detect similar labels (same label names appears only once in list) - - for( auto ref_it = loc_labelList.begin(); ref_it != loc_labelList.end(); ++ref_it ) - { - NETLIST_OBJECT* ref_item = *ref_it; - auto it_aux = ref_it; - - for( ++it_aux; it_aux != loc_labelList.end(); ++it_aux ) - { - // global label versus global label was already examined. - // here, at least one label must be local - if( ref_item->IsLabelGlobal() && ( *it_aux )->IsLabelGlobal() ) - continue; - - if( ref_item->m_Label.CmpNoCase( ( *it_aux )->m_Label ) == 0 ) + switch( item->Type() ) { - // Create new marker for ERC. - int cntA = countIndenticalLabels( fullLabelList, ref_item ); - int cntB = countIndenticalLabels( fullLabelList, *it_aux ); + case SCH_LABEL_T: + case SCH_HIER_LABEL_T: + case SCH_GLOBAL_LABEL_T: + { + SCH_TEXT* text = static_cast( item ); - if( cntA <= cntB ) - SimilarLabelsDiagnose( ref_item, ( *it_aux ) ); - else - SimilarLabelsDiagnose( ( *it_aux ), ref_item ); + wxString normalized = text->GetShownText().Lower(); + + if( !labelMap.count( normalized ) ) + { + labelMap[normalized] = text; + } + else if( labelMap.at( normalized )->GetShownText() != text->GetShownText() ) + { + std::shared_ptr ercItem = ERC_ITEM::Create( ERCE_SIMILAR_LABELS ); + ercItem->SetItems( text, labelMap.at( normalized ) ); + + SCH_MARKER* marker = new SCH_MARKER( ercItem, text->GetPosition() ); + subgraph->m_sheet.LastScreen()->Append( marker ); + } + + break; + } + + default: + break; } } } } -} - - -// Helper function: count the number of labels identical to aLabel -// for global label: global labels in the full project -// for local label: all labels in the current sheet -static int countIndenticalLabels( std::vector& aList, NETLIST_OBJECT* aRef ) -{ - int count = 0; - - if( aRef->IsLabelGlobal() ) - { - for( NETLIST_OBJECT* i : aList ) - { - if( i->IsLabelGlobal() && i->m_Label == aRef->m_Label ) - count++; - } - } - else - { - for( NETLIST_OBJECT* i : aList ) - { - if( i->m_Label == aRef->m_Label && i->m_SheetPath.Path() == aRef->m_SheetPath.Path() ) - count++; - } - } - - return count; -} - - -// Helper function: creates a marker for similar labels ERC warning -static void SimilarLabelsDiagnose( NETLIST_OBJECT* aItemA, NETLIST_OBJECT* aItemB ) -{ - std::shared_ptr ercItem = ERC_ITEM::Create( ERCE_SIMILAR_LABELS ); - ercItem->SetItems( aItemA->m_Comp, aItemB->m_Comp ); - - SCH_MARKER* marker = new SCH_MARKER( ercItem, aItemA->m_Start ); - aItemA->m_SheetPath.LastScreen()->Append( marker ); + + return errors; } diff --git a/eeschema/erc.h b/eeschema/erc.h index 273f818e55..e9d19cc428 100644 --- a/eeschema/erc.h +++ b/eeschema/erc.h @@ -118,6 +118,12 @@ public: */ int TestMultUnitPinConflicts(); + /** + * Checks for labels that differ only in capitalization + * @return the error count + */ + int TestSimilarLabels(); + private: /** * Performs ERC testing and creates an ERC marker to show the ERC problem for aNetItemRef diff --git a/eeschema/netlist_object.h b/eeschema/netlist_object.h index ed4c6ae123..a7f1f8a0de 100644 --- a/eeschema/netlist_object.h +++ b/eeschema/netlist_object.h @@ -392,15 +392,6 @@ public: */ void SortListbySheet(); - /** - * Function TestforSimilarLabels - * detects labels which are different when using case sensitive comparisons - * but are equal when using case insensitive comparisons - * It can be due to a mistake from designer, so this kind of labels - * is reported by TestforSimilarLabels - */ - void TestforSimilarLabels(); - #if defined(DEBUG) void DumpNetTable() {