From df600fc1a2f55c032e02c41fbdf1b2db412954cc Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Thu, 11 Apr 2019 21:22:19 -0400 Subject: [PATCH] Fix a few ERC issues with no-connects and power pins Fixes: lp:1824359 * https://bugs.launchpad.net/kicad/+bug/1824359 --- eeschema/connection_graph.cpp | 94 ++++++++++++++++++++++++++++++----- eeschema/connection_graph.h | 2 + eeschema/drc_erc_item.cpp | 14 ++++-- eeschema/erc.cpp | 59 +--------------------- eeschema/erc.h | 2 + 5 files changed, 95 insertions(+), 76 deletions(-) diff --git a/eeschema/connection_graph.cpp b/eeschema/connection_graph.cpp index 018d8bd00d..abe4634a40 100644 --- a/eeschema/connection_graph.cpp +++ b/eeschema/connection_graph.cpp @@ -1573,11 +1573,22 @@ int CONNECTION_GRAPH::RunERC( const ERC_SETTINGS& aSettings, bool aCreateMarkers { int error_count = 0; + std::map< wxString, std::vector< std::pair< SCH_ITEM*, CONNECTION_SUBGRAPH* > > > globals; + for( auto subgraph : m_subgraphs ) { // Graph is supposed to be up-to-date before calling RunERC() wxASSERT( !subgraph->m_dirty ); + for( const auto& item : subgraph->m_items ) + { + if( item->Type() == SCH_GLOBAL_LABEL_T ) + { + wxString key = static_cast( item )->GetText(); + globals[ key ].emplace_back( std::make_pair( item, subgraph ) ); + } + } + /** * NOTE: * @@ -1615,10 +1626,51 @@ int CONNECTION_GRAPH::RunERC( const ERC_SETTINGS& aSettings, bool aCreateMarkers error_count++; } + // Some checks are now run after processing every subgraph + + // Check for lonely global labels + if( aSettings.check_unique_global_labels ) + { + for( auto &it : globals ) + { + if( it.second.size() == 1 ) + { + ercReportIsolatedGlobalLabel( it.second.at( 0 ).second, it.second.at( 0 ).first ); + error_count++; + } + } + } + return error_count; } +void CONNECTION_GRAPH::ercReportIsolatedGlobalLabel( CONNECTION_SUBGRAPH* aSubgraph, + SCH_ITEM* aLabel ) +{ + wxString msg; + auto label = dynamic_cast( aLabel ); + + if( !label ) + return; + + msg.Printf( _( "Global label %s is not connected to any other global label." ), + label->GetShownText() ); + + auto marker = new SCH_MARKER(); + marker->SetTimeStamp( GetNewTimeStamp() ); + marker->SetMarkerType( MARKER_BASE::MARKER_ERC ); + marker->SetErrorLevel( MARKER_BASE::MARKER_SEVERITY_WARNING ); + marker->SetData( ERCE_GLOBLABEL, + label->GetPosition(), + msg, + label->GetPosition() ); + + SCH_SCREEN* screen = aSubgraph->m_sheet.LastScreen(); + screen->Append( marker ); +} + + bool CONNECTION_GRAPH::ercCheckBusToNetConflicts( CONNECTION_SUBGRAPH* aSubgraph, bool aCreateMarkers ) { @@ -1860,11 +1912,12 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( CONNECTION_SUBGRAPH* aSubgraph, if( aSubgraph->m_no_connect != nullptr ) { bool has_invalid_items = false; + bool has_other_items = false; SCH_PIN* pin = nullptr; std::vector invalid_items; // Any subgraph that contains both a pin and a no-connect should not - // contain any other connectable items. + // contain any other driving items. for( auto item : aSubgraph->m_items ) { @@ -1872,13 +1925,17 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( CONNECTION_SUBGRAPH* aSubgraph, { case SCH_PIN_T: pin = static_cast( item ); + has_other_items = true; break; + case SCH_LINE_T: + case SCH_JUNCTION_T: case SCH_NO_CONNECT_T: break; default: has_invalid_items = true; + has_other_items = true; invalid_items.push_back( item ); } } @@ -1887,9 +1944,7 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( CONNECTION_SUBGRAPH* aSubgraph, // (JEY) Yes, I think it should if( pin && has_invalid_items ) { - SCH_COMPONENT* comp = pin->GetParentComponent(); - wxPoint pos = comp->GetTransform().TransformCoordinate( pin->GetPosition() ) - + comp->GetPosition(); + wxPoint pos = pin->GetTransformedPosition(); msg.Printf( _( "Pin %s of component %s has a no-connect marker but is connected" ), GetChars( pin->GetName() ), @@ -1905,6 +1960,23 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( CONNECTION_SUBGRAPH* aSubgraph, return false; } + + if( !has_other_items ) + { + wxPoint pos = aSubgraph->m_no_connect->GetPosition(); + + msg.Printf( _( "No-connect marker is not connected to anything" ) ); + + auto marker = new SCH_MARKER(); + marker->SetTimeStamp( GetNewTimeStamp() ); + marker->SetMarkerType( MARKER_BASE::MARKER_ERC ); + marker->SetErrorLevel( MARKER_BASE::MARKER_SEVERITY_WARNING ); + marker->SetData( ERCE_NOCONNECT_NOT_CONNECTED, pos, msg, pos ); + + screen->Append( marker ); + + return false; + } } else { @@ -1941,22 +2013,18 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( CONNECTION_SUBGRAPH* aSubgraph, pin->IsPowerConnection() && !pin->IsVisible() ) { wxString name = pin->Connection( sheet )->Name(); + wxString local_name = pin->Connection( sheet )->Name( true ); - if( int code = m_net_name_to_code_map.count( name ) ) + if( m_global_label_cache.count( name ) || + ( m_local_label_cache.count( std::make_pair( sheet, local_name ) ) ) ) { - if( m_net_code_to_subgraphs_map.count( code ) ) - { - if( m_net_code_to_subgraphs_map.at( code ).size() > 1 ) - has_other_connections = true; - } + has_other_connections = true; } } if( pin && !has_other_connections && pin->GetType() != PIN_NC ) { - SCH_COMPONENT* comp = pin->GetParentComponent(); - wxPoint pos = comp->GetTransform().TransformCoordinate( pin->GetPosition() ) - + comp->GetPosition(); + wxPoint pos = pin->GetTransformedPosition(); msg.Printf( _( "Pin %s of component %s is unconnected." ), GetChars( pin->GetName() ), diff --git a/eeschema/connection_graph.h b/eeschema/connection_graph.h index dc62030cab..66199a8c2d 100644 --- a/eeschema/connection_graph.h +++ b/eeschema/connection_graph.h @@ -345,6 +345,8 @@ private: * @return true for no errors, false for errors */ bool ercCheckLabels( CONNECTION_SUBGRAPH* aSubgraph, bool aCreateMarkers ); + + void ercReportIsolatedGlobalLabel( CONNECTION_SUBGRAPH* aSubgraph, SCH_ITEM* aLabel ); }; #endif diff --git a/eeschema/drc_erc_item.cpp b/eeschema/drc_erc_item.cpp index 0dc3839c79..a258c05cb2 100644 --- a/eeschema/drc_erc_item.cpp +++ b/eeschema/drc_erc_item.cpp @@ -49,17 +49,19 @@ wxString DRC_ITEM::GetErrorText() const case ERCE_PIN_TO_PIN_ERROR: return wxString( _("Conflict problem between pins. Severity: error") ); case ERCE_HIERACHICAL_LABEL: - return wxString( _("Mismatch between hierarchical labels and pins sheets")); + return wxString( _("Mismatch between hierarchical labels and pins sheets") ); case ERCE_NOCONNECT_CONNECTED: - return wxString( _("A pin with a \"no connection\" flag is connected")); + return wxString( _("A pin with a \"no connection\" flag is connected") ); + case ERCE_NOCONNECT_NOT_CONNECTED: + return wxString( _("A \"no connection\" flag is not connected to anything") ); case ERCE_LABEL_NOT_CONNECTED: return wxString( _("A label not connected to any other item") ); case ERCE_SIMILAR_LABELS: - return wxString( _("Labels are similar (lower/upper case difference only)") ); + return wxString( _("Labels are similar (lower/upper case difference only)" ) ); case ERCE_SIMILAR_GLBL_LABELS: - return wxString( _("Global labels are similar (lower/upper case difference only)") ); + return wxString( _("Global labels are similar (lower/upper case difference only)" ) ); case ERCE_DIFFERENT_UNIT_FP: - return wxString( _("Different footprint assigned in another unit of the same component") ); + return wxString( _("Different footprint assigned in another unit of the same component" ) ); case ERCE_DIFFERENT_UNIT_NET: return wxString( _("Different net assigned to a shared pin in another unit of the same component" ) ); case ERCE_BUS_ALIAS_CONFLICT: @@ -74,6 +76,8 @@ wxString DRC_ITEM::GetErrorText() const return wxString( _( "No nets are shared between two bus items" ) ); case ERCE_BUS_TO_NET_CONFLICT: return wxString( _( "Invalid connection between bus and net items" ) ); + case ERCE_GLOBLABEL: + return wxString( _( "Invalid connection between bus and net items" ) ); default: wxFAIL_MSG( "Missing ERC error description" ); return wxString( wxT("Unknown.") ); diff --git a/eeschema/erc.cpp b/eeschema/erc.cpp index b96177033b..1b9c3118a7 100644 --- a/eeschema/erc.cpp +++ b/eeschema/erc.cpp @@ -369,7 +369,7 @@ void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, SCH_SCREEN* screen; ELECTRICAL_PINTYPE ii, jj; - if( aDiag == OK ) + if( aDiag == OK || aMinConn < 1 ) return; /* Create new marker for ERC error. */ @@ -383,40 +383,6 @@ void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, wxString msg; - if( aMinConn < 0 ) - { -#if 0 - if( aNetItemRef->m_Type == NET_HIERLABEL || aNetItemRef->m_Type == NET_HIERBUSLABELMEMBER ) - { - msg.Printf( _( "Hierarchical label %s is not connected to a sheet label." ), - GetChars( aNetItemRef->m_Label ) ); - marker->SetData( ERCE_HIERACHICAL_LABEL, - aNetItemRef->m_Start, - msg, - aNetItemRef->m_Start ); - } - else if( aNetItemRef->m_Type == NET_GLOBLABEL ) - { - msg.Printf( _( "Global label %s is not connected to any other global label." ), - GetChars( aNetItemRef->m_Label ) ); - marker->SetData( ERCE_GLOBLABEL, - aNetItemRef->m_Start, - msg, - aNetItemRef->m_Start ); - } - else - { - msg.Printf( _( "Sheet label %s is not connected to a hierarchical label." ), - GetChars( aNetItemRef->m_Label ) ); - marker->SetData( ERCE_HIERACHICAL_LABEL, - aNetItemRef->m_Start, - msg, - aNetItemRef->m_Start ); - } -#endif - return; - } - ii = aNetItemRef->m_ElectricalPinType; wxString cmp_ref( "?" ); @@ -426,19 +392,6 @@ void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, if( aNetItemTst == NULL ) { - if( aMinConn == NOC ) /* Only 1 element in the net. */ - { - msg.Printf( _( "Pin %s (%s) of component %s is unconnected." ), - aNetItemRef->m_PinNum, - GetChars( GetText( ii ) ), - GetChars( cmp_ref ) ); - marker->SetData( ERCE_PIN_NOT_CONNECTED, - aNetItemRef->m_Start, - msg, - aNetItemRef->m_Start ); - return; - } - if( aMinConn == NOD ) /* Nothing driving the net. */ { if( aNetItemRef->m_Type == NET_PIN && aNetItemRef->m_Link ) @@ -456,16 +409,6 @@ void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, aNetItemRef->m_Start ); return; } - - if( aDiag == UNC ) - { - msg.Printf( _( "More than 1 pin connected to an UnConnect symbol." ) ); - marker->SetData( ERCE_NOCONNECT_CONNECTED, - aNetItemRef->m_Start, - msg, - aNetItemRef->m_Start ); - return; - } } if( aNetItemTst ) /* Error between 2 pins */ diff --git a/eeschema/erc.h b/eeschema/erc.h index ea0b48a8e5..d7e96acdf2 100644 --- a/eeschema/erc.h +++ b/eeschema/erc.h @@ -59,6 +59,7 @@ enum ERCE_T ERCE_PIN_TO_PIN_ERROR, // pin connected to an other pin: error level ERCE_HIERACHICAL_LABEL, // mismatch between hierarchical labels and pins sheets ERCE_NOCONNECT_CONNECTED, // a no connect symbol is connected to more than 1 pin + ERCE_NOCONNECT_NOT_CONNECTED, // a no connect symbol is not connected to anything ERCE_LABEL_NOT_CONNECTED, // label not connected to anything ERCE_SIMILAR_LABELS, // 2 labels are equal fir case insensitive comparisons ERCE_SIMILAR_GLBL_LABELS, // 2 labels are equal fir case insensitive comparisons @@ -70,6 +71,7 @@ enum ERCE_T ERCE_BUS_LABEL_ERROR, // a label attached to a bus isn't in bus format ERCE_BUS_TO_BUS_CONFLICT, // a connection between bus objects doesn't share at least one net ERCE_BUS_TO_NET_CONFLICT, // a bus wire is graphically connected to a net port/pin (or vice versa) + ERCE_GLOBLABEL, // a global label is unique }; /* Minimal connection table */