From 55c864ec453936d22908113fa5d9011fa6f27bf9 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Wed, 10 Apr 2024 16:40:43 -0700 Subject: [PATCH] Fix incremental connectivity The connectivity routine would consider symbols, overwriting unused subgraphs for pins that were not in the change list. This is resolved by updating the full connectivity to only use pins in the graph since symbols are not connected independently. In the process of adding QA tests for this change, additional issues with the schematic QA were discovered. Specifically, we were not properly setting the root sheet UUID. This was partially masked by a const_cast setting of the RefDes in sch_symbol when called the RefDes getter. This exposed the fact that our QA ERC numbers did not match the schematic editor stand alone ERC numbers. So the test value for one check needed to be updated Fixes https://gitlab.com/kicad/code/kicad/-/issues/17528 (cherry picked from commit a9f35ba42e82b0c3c8a68d42cf89bbc91f8df6c6) --- eeschema/connection_graph.cpp | 176 +++++-- eeschema/connection_graph.h | 2 + eeschema/files-io.cpp | 1 + eeschema/sch_edit_frame.cpp | 23 +- eeschema/sch_item.cpp | 23 +- eeschema/sch_item.h | 11 +- eeschema/sch_label.cpp | 2 +- eeschema/sch_symbol.cpp | 1 - qa/data/eeschema/incremental_test.kicad_sch | 478 ++++++++++++++++++ qa/data/eeschema/issue13591.kicad_pro | 99 +++- qa/schematic_utils/schematic_file_util.cpp | 2 +- qa/tests/eeschema/CMakeLists.txt | 1 + .../erc/test_erc_hierarchical_schematics.cpp | 2 +- .../eeschema/test_incremental_netlister.cpp | 164 ++++++ 14 files changed, 898 insertions(+), 87 deletions(-) create mode 100644 qa/data/eeschema/incremental_test.kicad_sch create mode 100644 qa/tests/eeschema/test_incremental_netlister.cpp diff --git a/eeschema/connection_graph.cpp b/eeschema/connection_graph.cpp index f4cea812f5..99b24bc707 100644 --- a/eeschema/connection_graph.cpp +++ b/eeschema/connection_graph.cpp @@ -87,6 +87,8 @@ void CONNECTION_SUBGRAPH::RemoveItem( SCH_ITEM* aItem ) bool CONNECTION_SUBGRAPH::ResolveDrivers( bool aCheckMultipleDrivers ) { + std::lock_guard lock( m_driver_mutex ); + auto candidate_cmp = [&]( SCH_ITEM* a, SCH_ITEM* b ) -> bool { // meet irreflexive requirements of std::sort @@ -631,6 +633,21 @@ void CONNECTION_GRAPH::Recalculate( const SCH_SHEET_LIST& aSheetList, bool aUnco items.push_back( item ); dirty_items.insert( item ); } + // If the symbol isn't dirty, look at the pins + // TODO: remove symbols from connectivity graph and only use pins + else if( item->Type() == SCH_SYMBOL_T ) + { + SCH_SYMBOL* symbol = static_cast( item ); + + for( SCH_PIN* pin : symbol->GetPins( &sheet ) ) + { + if( pin->IsConnectivityDirty() ) + { + items.push_back( pin ); + dirty_items.insert( pin ); + } + } + } // Ensure the hierarchy info stored in the SCH_SCREEN (such as symbol units) reflects // the current SCH_SHEET_PATH @@ -710,37 +727,76 @@ std::set> CONNECTION_GRAPH::ExtractAffected aSubgraph->getAllConnectedItems( retvals, subgraphs ); }; + auto extract_element = [&]( SCH_ITEM* aItem ) + { + CONNECTION_SUBGRAPH* item_sg = GetSubgraphForItem( aItem ); + + if( !item_sg ) + { + wxLogTrace( ConnTrace, wxT( "Item %s not found in connection graph" ), aItem->GetTypeDesc() ); + return; + } + if( !item_sg->ResolveDrivers( true ) ) + { + wxLogTrace( ConnTrace, wxT( "Item %s in subgraph %ld (%p) has no driver" ), + aItem->GetTypeDesc(), item_sg->m_code, item_sg ); + } + + std::vector sg_to_scan = GetAllSubgraphs( item_sg->GetNetName() ); + + if( sg_to_scan.empty() ) + { + wxLogTrace( ConnTrace, wxT( "Item %s in subgraph %ld with net %s has no neighbors" ), + aItem->GetTypeDesc(), item_sg->m_code, item_sg->GetNetName() ); + sg_to_scan.push_back( item_sg ); + } + + wxLogTrace( ConnTrace, + wxT( "Removing all item %s connections from subgraph %ld with net %s: Found " + "%zu subgraphs" ), + aItem->GetTypeDesc(), item_sg->m_code, item_sg->GetNetName(), + sg_to_scan.size() ); + + for( CONNECTION_SUBGRAPH* sg : sg_to_scan ) + + { + traverse_subgraph( sg ); + + for( auto& bus_it : sg->m_bus_neighbors ) + { + for( CONNECTION_SUBGRAPH* bus_sg : bus_it.second ) + traverse_subgraph( bus_sg ); + } + + for( auto& bus_it : sg->m_bus_parents ) + { + for( CONNECTION_SUBGRAPH* bus_sg : bus_it.second ) + traverse_subgraph( bus_sg ); + } + } + + alg::delete_matching( m_items, aItem ); + }; + for( SCH_ITEM* item : aItems ) { - auto it = m_item_to_subgraph_map.find( item ); - - if( it == m_item_to_subgraph_map.end() ) - continue; - - CONNECTION_SUBGRAPH* sg = it->second; - - traverse_subgraph( sg ); - - for( auto& bus_it : sg->m_bus_neighbors ) + if( item->Type() == SCH_SHEET_T ) { - for( CONNECTION_SUBGRAPH* bus_sg : bus_it.second ) - traverse_subgraph( bus_sg ); + SCH_SHEET* sheet = static_cast( item ); + + for( SCH_SHEET_PIN* pin : sheet->GetPins() ) + extract_element( pin ); } - - for( auto& bus_it : sg->m_bus_parents ) - { - for( CONNECTION_SUBGRAPH* bus_sg : bus_it.second ) - traverse_subgraph( bus_sg ); - } - - alg::delete_matching( m_items, item ); - - if( item->Type() == SCH_SYMBOL_T ) + else if ( item->Type() == SCH_SYMBOL_T ) { SCH_SYMBOL* symbol = static_cast( item ); - for( SCH_PIN* pin : symbol->GetPins( &sg->m_sheet ) ) - alg::delete_matching( m_items, pin ); + for( SCH_PIN* pin : symbol->GetPins( &m_schematic->CurrentSheet() ) ) + extract_element( pin ); + } + else + { + extract_element( item ); } } @@ -770,6 +826,7 @@ void CONNECTION_GRAPH::RemoveItem( SCH_ITEM* aItem ) void CONNECTION_GRAPH::removeSubgraphs( std::set& aSubgraphs ) { + wxLogTrace( ConnTrace, wxT( "Removing %zu subgraphs" ), aSubgraphs.size() ); std::sort( m_driver_subgraphs.begin(), m_driver_subgraphs.end() ); std::sort( m_subgraphs.begin(), m_subgraphs.end() ); std::set codes_to_remove; @@ -928,12 +985,30 @@ void CONNECTION_GRAPH::removeSubgraphs( std::set& aSubgrap void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet, const std::vector& aItemList ) { + wxLogTrace( wxT( "Updating connectivity for sheet %s with %zu items" ), + aSheet.Last()->GetFileName(), aItemList.size() ); std::map> connection_map; + auto updatePin = [&]( SCH_PIN* aPin, SCH_CONNECTION* aConn ) + { + aConn->SetType( CONNECTION_TYPE::NET ); + + // because calling the first time is not thread-safe + wxString name = aPin->GetDefaultNetName( aSheet ); + aPin->ClearConnectedItems( aSheet ); + + // power symbol pins need to be post-processed later + if( aPin->IsGlobalPower() ) + { + aConn->SetName( name ); + m_global_power_pins.emplace_back( std::make_pair( aSheet, aPin ) ); + } + }; + for( SCH_ITEM* item : aItemList ) { std::vector points = item->GetConnectionPoints(); - item->ConnectedItems( aSheet ).clear(); + item->ClearConnectedItems( aSheet ); if( item->Type() == SCH_SHEET_T ) { @@ -941,7 +1016,7 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet, { pin->InitializeConnection( aSheet, this ); - pin->ConnectedItems( aSheet ).clear(); + pin->ClearConnectedItems( aSheet ); connection_map[ pin->GetTextPos() ].push_back( pin ); m_items.emplace_back( pin ); @@ -953,23 +1028,10 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet, for( SCH_PIN* pin : symbol->GetPins( &aSheet ) ) { - SCH_CONNECTION* conn = pin->InitializeConnection( aSheet, this ); - - VECTOR2I pos = pin->GetPosition(); - - // because calling the first time is not thread-safe - wxString name = pin->GetDefaultNetName( aSheet ); - pin->ConnectedItems( aSheet ).clear(); - - // power symbol pins need to be post-processed later - if( pin->IsGlobalPower() ) - { - conn->SetName( name ); - m_global_power_pins.emplace_back( std::make_pair( aSheet, pin ) ); - } - - connection_map[ pos ].push_back( pin ); m_items.emplace_back( pin ); + SCH_CONNECTION* conn = pin->InitializeConnection( aSheet, this ); + updatePin( pin, conn ); + connection_map[ pin->GetPosition() ].push_back( pin ); } } else @@ -994,7 +1056,10 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet, break; case SCH_PIN_T: - conn->SetType( CONNECTION_TYPE::NET ); + if( points.empty() ) + points = { static_cast( item )->GetPosition() }; + + updatePin( static_cast( item ), conn ); break; case SCH_BUS_WIRE_ENTRY_T: @@ -1075,9 +1140,6 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet, connected_item->SetLayer( busLine ? LAYER_BUS_JUNCTION : LAYER_JUNCTION ); } - SCH_ITEM_SET& connected_set = connected_item->ConnectedItems( aSheet ); - connected_set.reserve( connection_vec.size() ); - for( SCH_ITEM* test_item : connection_vec ) { bool bus_connection_ok = true; @@ -1111,7 +1173,7 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet, test_item->ConnectionPropagatesTo( connected_item ) && bus_connection_ok ) { - connected_set.push_back( test_item ); + connected_item->AddConnectionTo( aSheet, test_item ); } } @@ -1213,7 +1275,7 @@ void CONNECTION_GRAPH::buildItemSubGraphs() connected_conn->SetSubgraphCode( subgraph->m_code ); m_item_to_subgraph_map[connected_item] = subgraph; subgraph->AddItem( connected_item ); - SCH_ITEM_SET& citemset = connected_item->ConnectedItems( sheet ); + const SCH_ITEM_VEC& citemset = connected_item->ConnectedItems( sheet ); for( SCH_ITEM* citem : citemset ) { @@ -1248,6 +1310,8 @@ void CONNECTION_GRAPH::resolveAllDrivers() return candidate->m_dirty; } ); + wxLogTrace( ConnTrace, wxT( "Resolving drivers for %zu subgraphs" ), dirty_graphs.size() ); + std::vector> returns( dirty_graphs.size() ); auto update_lambda = []( CONNECTION_SUBGRAPH* subgraph ) -> size_t @@ -1378,7 +1442,7 @@ void CONNECTION_GRAPH::generateBusAliasMembers() for( CONNECTION_SUBGRAPH* subgraph : m_driver_subgraphs ) { - SCH_ITEM_SET vec = subgraph->GetAllBusLabels(); + SCH_ITEM_VEC vec = subgraph->GetAllBusLabels(); for( SCH_ITEM* item : vec ) { @@ -1556,14 +1620,20 @@ void CONNECTION_GRAPH::processSubGraphs() if( !subgraph->m_strong_driver ) { - std::vector* vec = &m_net_name_to_subgraphs_map.at( name ); + std::vector vec_empty; + std::vector* vec = &vec_empty; + + if( m_net_name_to_subgraphs_map.count( name ) ) + vec = &m_net_name_to_subgraphs_map.at( name ); // If we are a unique bus vector, check if we aren't actually unique because of another // subgraph with a similar bus vector if( vec->size() <= 1 && subgraph->m_driver_connection->Type() == CONNECTION_TYPE::BUS ) { wxString prefixOnly = name.BeforeFirst( '[' ) + wxT( "[]" ); - vec = &m_net_name_to_subgraphs_map.at( prefixOnly ); + + if( m_net_name_to_subgraphs_map.count( prefixOnly ) ) + vec = &m_net_name_to_subgraphs_map.at( prefixOnly ); } if( vec->size() > 1 ) @@ -2831,8 +2901,8 @@ CONNECTION_SUBGRAPH* CONNECTION_GRAPH::GetSubgraphForItem( SCH_ITEM* aItem ) } -const std::vector CONNECTION_GRAPH::GetAllSubgraphs( - const wxString& aNetName ) const +const std::vector +CONNECTION_GRAPH::GetAllSubgraphs( const wxString& aNetName ) const { std::vector subgraphs; diff --git a/eeschema/connection_graph.h b/eeschema/connection_graph.h index f0ab387f4c..e025a8937d 100644 --- a/eeschema/connection_graph.h +++ b/eeschema/connection_graph.h @@ -301,6 +301,8 @@ private: /// A cache of connections that are part of this subgraph but that don't have /// an owning element (i.e. bus members) std::set m_bus_element_connections; + + std::mutex m_driver_mutex; }; struct NET_NAME_CODE_CACHE_KEY diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index 3358ae0610..4cbc613ab4 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -267,6 +267,7 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in // Make ${SHEETNAME} work on the root sheet until we properly support // naming the root sheet Schematic().Root().SetName( _( "Root" ) ); + wxLogDebug( "Loaded schematic with root sheet UUID %s", Schematic().Root().m_Uuid.AsString() ); } if( !pi->GetError().IsEmpty() ) diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 1c2adc4bec..d20e7a3f44 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -1744,12 +1744,16 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL { for( SCH_ITEM* item : GetScreen()->Items().Overlapping( pt ) ) { + // Leave this check in place. Overlapping items are not necessarily connectable. + if( !item->IsConnectable() ) + continue; + if( item->Type() == SCH_LINE_T ) { if( item->HitTest( pt ) ) changed_items.insert( item ); } - else if( item->Type() == SCH_SYMBOL_T ) + else if( item->Type() == SCH_SYMBOL_T && item->IsConnected( pt ) ) { SCH_SYMBOL* symbol = static_cast( item ); std::vector pins = symbol->GetPins(); @@ -1767,7 +1771,6 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL } else { - // Non-connectable objects have already been pruned. if( item->IsConnected( pt ) ) changed_items.insert( item ); } @@ -1786,21 +1789,7 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL for( auto&[ path, item ] : all_items ) { wxCHECK2( item, continue ); - - switch( item->Type() ) - { - case SCH_FIELD_T: - case SCH_PIN_T: - { - SCH_ITEM* parent = static_cast( item->GetParent() ); - wxCHECK2( parent, continue ); - parent->SetConnectivityDirty(); - break; - } - - default: - item->SetConnectivityDirty(); - } + item->SetConnectivityDirty(); } new_graph.Recalculate( list, false, &changeHandler ); diff --git a/eeschema/sch_item.cpp b/eeschema/sch_item.cpp index 0add74bb8d..e45862857e 100644 --- a/eeschema/sch_item.cpp +++ b/eeschema/sch_item.cpp @@ -194,7 +194,16 @@ std::shared_ptr SCH_ITEM::GetEffectiveNetClass( const SCH_SHEET_PATH* } -SCH_ITEM_SET& SCH_ITEM::ConnectedItems( const SCH_SHEET_PATH& aSheet ) +void SCH_ITEM::ClearConnectedItems( const SCH_SHEET_PATH& aSheet ) +{ + auto it = m_connected_items.find( aSheet ); + + if( it != m_connected_items.end() ) + it->second.clear(); +} + + +const SCH_ITEM_VEC& SCH_ITEM::ConnectedItems( const SCH_SHEET_PATH& aSheet ) { return m_connected_items[ aSheet ]; } @@ -202,13 +211,17 @@ SCH_ITEM_SET& SCH_ITEM::ConnectedItems( const SCH_SHEET_PATH& aSheet ) void SCH_ITEM::AddConnectionTo( const SCH_SHEET_PATH& aSheet, SCH_ITEM* aItem ) { - SCH_ITEM_SET& set = m_connected_items[ aSheet ]; + SCH_ITEM_VEC& vec = m_connected_items[ aSheet ]; // The vector elements are small, so reserve 1k at a time to prevent re-allocations - if( set.size() == set.capacity() ) - set.reserve( set.size() + 4096 ); + if( vec.size() == vec.capacity() ) + vec.reserve( vec.size() + 4096 ); - set.emplace_back( aItem ); + // Add item to the correct place in the sorted vector if it is not already there + auto it = std::lower_bound( vec.begin(), vec.end(), aItem ); + + if( it == vec.end() || *it != aItem ) + vec.insert( it, aItem ); } diff --git a/eeschema/sch_item.h b/eeschema/sch_item.h index 03d4066503..e52f4e7248 100644 --- a/eeschema/sch_item.h +++ b/eeschema/sch_item.h @@ -151,7 +151,7 @@ public: std::vector& aItemListByPos ); }; -typedef std::vector SCH_ITEM_SET; +typedef std::vector SCH_ITEM_VEC; /** @@ -429,13 +429,18 @@ public: /** * Retrieve the set of items connected to this item on the given sheet. */ - SCH_ITEM_SET& ConnectedItems( const SCH_SHEET_PATH& aPath ); + const SCH_ITEM_VEC& ConnectedItems( const SCH_SHEET_PATH& aPath ); /** * Add a connection link between this item and another. */ void AddConnectionTo( const SCH_SHEET_PATH& aPath, SCH_ITEM* aItem ); + /** + * Clear all connections to this item. + */ + void ClearConnectedItems( const SCH_SHEET_PATH& aPath ); + /** * Create a new connection object associated with this object. * @@ -563,7 +568,7 @@ protected: // to store a initial pos of the item or mouse cursor /// Store pointers to other items that are connected to this one, per sheet. - std::map m_connected_items; + std::map m_connected_items; /// Store connectivity information, per sheet. std::unordered_map m_connection_map; diff --git a/eeschema/sch_label.cpp b/eeschema/sch_label.cpp index 1cfde709a8..6e933f48fe 100644 --- a/eeschema/sch_label.cpp +++ b/eeschema/sch_label.cpp @@ -274,7 +274,7 @@ bool SCH_LABEL_BASE::IsType( const std::vector& aScanTypes ) const if( m_connected_items.find( Schematic()->CurrentSheet() ) == m_connected_items.end() ) return false; - const SCH_ITEM_SET& item_set = m_connected_items.at( Schematic()->CurrentSheet() ); + const SCH_ITEM_VEC& item_set = m_connected_items.at( Schematic()->CurrentSheet() ); for( KICAD_T scanType : aScanTypes ) { diff --git a/eeschema/sch_symbol.cpp b/eeschema/sch_symbol.cpp index 38f4c88d0b..ce0e559e8b 100644 --- a/eeschema/sch_symbol.cpp +++ b/eeschema/sch_symbol.cpp @@ -770,7 +770,6 @@ const wxString SCH_SYMBOL::GetRef( const SCH_SHEET_PATH* sheet, bool aIncludeUni // the same symbol references, but perhaps this is best. if( ref.IsEmpty() && !GetField( REFERENCE_FIELD )->GetText().IsEmpty() ) { - const_cast( this )->SetRef( sheet, GetField( REFERENCE_FIELD )->GetText() ); ref = GetField( REFERENCE_FIELD )->GetText(); } diff --git a/qa/data/eeschema/incremental_test.kicad_sch b/qa/data/eeschema/incremental_test.kicad_sch new file mode 100644 index 0000000000..2125f8e3a1 --- /dev/null +++ b/qa/data/eeschema/incremental_test.kicad_sch @@ -0,0 +1,478 @@ +(kicad_sch + (version 20231120) + (generator "eeschema") + (generator_version "8.0") + (uuid "5e4124ac-877c-4d6f-a16b-822110ac68a1") + (paper "A4") + (lib_symbols + (symbol "Connector:TestPoint" + (pin_numbers hide) + (pin_names + (offset 0.762) hide) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (property "Reference" "TP" + (at 0 6.858 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Value" "TestPoint" + (at 0 5.08 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Footprint" "" + (at 5.08 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "~" + (at 5.08 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "test point" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "ki_keywords" "test point tp" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "ki_fp_filters" "Pin* Test*" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (symbol "TestPoint_0_1" + (circle + (center 0 3.302) + (radius 0.762) + (stroke + (width 0) + (type default) + ) + (fill + (type none) + ) + ) + ) + (symbol "TestPoint_1_1" + (pin passive line + (at 0 0 90) + (length 2.54) + (name "1" + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (number "1" + (effects + (font + (size 1.27 1.27) + ) + ) + ) + ) + ) + ) + (symbol "power:GND" + (power) + (pin_numbers hide) + (pin_names + (offset 0) hide) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (property "Reference" "#PWR" + (at 0 -6.35 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Value" "GND" + (at 0 -3.81 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Footprint" "" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "Power symbol creates a global label with name \"GND\" , ground" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "ki_keywords" "global power" + (at 0 0 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (symbol "GND_0_1" + (polyline + (pts + (xy 0 0) (xy 0 -1.27) (xy 1.27 -1.27) (xy 0 -2.54) (xy -1.27 -1.27) (xy 0 -1.27) + ) + (stroke + (width 0) + (type default) + ) + (fill + (type none) + ) + ) + ) + (symbol "GND_1_1" + (pin power_in line + (at 0 0 270) + (length 0) + (name "~" + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (number "1" + (effects + (font + (size 1.27 1.27) + ) + ) + ) + ) + ) + ) + ) + (symbol + (lib_id "Connector:TestPoint") + (at 82.55 88.9 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "86891145-73fb-48da-aca6-492a4fb35190") + (property "Reference" "TP1" + (at 85.09 84.328 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Value" "TestPoint" + (at 85.09 86.868 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Footprint" "" + (at 87.63 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "~" + (at 87.63 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "test point" + (at 82.55 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "2bf2be02-e7d9-4e42-8e57-404647a392eb") + ) + (instances + (project "" + (path "/5e4124ac-877c-4d6f-a16b-822110ac68a1" + (reference "TP1") + (unit 1) + ) + ) + ) + ) + (symbol + (lib_id "power:GND") + (at 82.55 88.9 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "b9e595cb-65cf-4cf6-b2cb-772d3688abf0") + (property "Reference" "#PWR01" + (at 82.55 95.25 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Value" "GND" + (at 82.55 93.98 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Footprint" "" + (at 82.55 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "" + (at 82.55 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "Power symbol creates a global label with name \"GND\" , ground" + (at 82.55 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "e434aab5-d764-4bd8-a761-696fb56ace19") + ) + (instances + (project "" + (path "/5e4124ac-877c-4d6f-a16b-822110ac68a1" + (reference "#PWR01") + (unit 1) + ) + ) + ) + ) + (symbol + (lib_id "Connector:TestPoint") + (at 99.06 88.9 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "d4520136-e0ab-467d-b271-26033a078cb6") + (property "Reference" "TP2" + (at 101.6 84.328 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Value" "TestPoint" + (at 101.6 86.868 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Footprint" "" + (at 104.14 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "~" + (at 104.14 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "test point" + (at 99.06 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "2bf2be02-e7d9-4e42-8e57-404647a392eb") + ) + (instances + (project "" + (path "/5e4124ac-877c-4d6f-a16b-822110ac68a1" + (reference "TP2") + (unit 1) + ) + ) + ) + ) + (symbol + (lib_id "power:GND") + (at 99.06 88.9 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "fa30dd75-0d78-423e-a304-836357e80e2f") + (property "Reference" "#PWR02" + (at 99.06 95.25 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Value" "GND" + (at 99.06 93.98 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Footprint" "" + (at 99.06 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "" + (at 99.06 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "Power symbol creates a global label with name \"GND\" , ground" + (at 99.06 88.9 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "e434aab5-d764-4bd8-a761-696fb56ace19") + ) + (instances + (project "" + (path "/5e4124ac-877c-4d6f-a16b-822110ac68a1" + (reference "#PWR02") + (unit 1) + ) + ) + ) + ) + (sheet_instances + (path "/" + (page "1") + ) + ) +) diff --git a/qa/data/eeschema/issue13591.kicad_pro b/qa/data/eeschema/issue13591.kicad_pro index 202b3dfb72..2f285a7439 100644 --- a/qa/data/eeschema/issue13591.kicad_pro +++ b/qa/data/eeschema/issue13591.kicad_pro @@ -24,6 +24,13 @@ "track_widths": [], "via_dimensions": [] }, + "ipc2581": { + "dist": "", + "distpn": "", + "internal_id": "", + "mfg": "", + "mpn": "" + }, "layer_presets": [], "viewports": [] }, @@ -212,15 +219,23 @@ "bus_label_syntax": "error", "bus_to_bus_conflict": "error", "bus_to_net_conflict": "error", + "conflicting_netclasses": "error", "different_unit_footprint": "error", "different_unit_net": "error", "duplicate_reference": "error", "duplicate_sheet_names": "error", + "endpoint_off_grid": "warning", "extra_units": "error", + "footprint_link_issues": "warning", "global_label_dangling": "warning", "hier_label_mismatch": "error", "label_dangling": "error", "lib_symbol_issues": "warning", + "lib_symbol_mismatch": "warning", + "missing_bidi_pin": "warning", + "missing_input_pin": "warning", + "missing_power_pin": "error", + "missing_unit": "warning", "multiple_net_names": "warning", "net_not_bus_member": "warning", "no_connect_connected": "warning", @@ -230,6 +245,8 @@ "pin_to_pin": "warning", "power_pin_not_driven": "error", "similar_labels": "warning", + "simulation_model_issue": "ignore", + "single_global_label": "ignore", "unannotated": "error", "unit_value_mismatch": "error", "unresolved_variable": "error", @@ -241,13 +258,13 @@ "pinned_symbol_libs": [] }, "meta": { - "filename": "sim2_V6.kicad_pro", + "filename": "issue13591.kicad_pro", "version": 1 }, "net_settings": { "classes": [ { - "bus_width": 12.0, + "bus_width": 12, "clearance": 0.2, "diff_pair_gap": 0.25, "diff_pair_via_gap": 0.25, @@ -261,14 +278,17 @@ "track_width": 0.25, "via_diameter": 0.8, "via_drill": 0.4, - "wire_width": 6.0 + "wire_width": 6 } ], "meta": { - "version": 2 + "version": 3 }, "net_colors": null, - "netclass_assignments": null, + "netclass_assignments": { + "Net-(D1-A)": "", + "Net-(R1-Pad1)": "" + }, "netclass_patterns": [] }, "pcbnew": { @@ -276,14 +296,75 @@ "gencad": "", "idf": "", "netlist": "", + "plot": "", + "pos_files": "", "specctra_dsn": "", "step": "", + "svg": "", "vrml": "" }, "page_layout_descr_file": "" }, "schematic": { "annotate_start_num": 0, + "bom_fmt_presets": [], + "bom_fmt_settings": { + "field_delimiter": ",", + "keep_line_breaks": false, + "keep_tabs": false, + "name": "CSV", + "ref_delimiter": ",", + "ref_range_delimiter": "", + "string_delimiter": "\"" + }, + "bom_presets": [], + "bom_settings": { + "exclude_dnp": false, + "fields_ordered": [ + { + "group_by": false, + "label": "Reference", + "name": "Reference", + "show": true + }, + { + "group_by": true, + "label": "Value", + "name": "Value", + "show": true + }, + { + "group_by": false, + "label": "Datasheet", + "name": "Datasheet", + "show": true + }, + { + "group_by": false, + "label": "Footprint", + "name": "Footprint", + "show": true + }, + { + "group_by": false, + "label": "Qty", + "name": "${QUANTITY}", + "show": true + }, + { + "group_by": true, + "label": "DNP", + "name": "${DNP}", + "show": true + } + ], + "filter_string": "", + "group_symbols": true, + "name": "Grouped By Value", + "sort_asc": true, + "sort_field": "Reference" + }, + "connection_grid_size": 50.0, "drawing": { "dashed_lines_dash_length_ratio": 12.0, "dashed_lines_gap_length_ratio": 3.0, @@ -297,6 +378,11 @@ "intersheets_ref_suffix": "", "junction_size_choice": 3, "label_size_ratio": 0.375, + "operating_point_overlay_i_precision": 3, + "operating_point_overlay_i_range": "~A", + "operating_point_overlay_v_precision": 3, + "operating_point_overlay_v_range": "~V", + "overbar_offset_ratio": 1.23, "pin_symbol_size": 25.0, "text_offset_ratio": 0.15 }, @@ -318,8 +404,11 @@ "page_layout_descr_file": "", "plot_directory": "", "spice_adjust_passive_values": false, + "spice_current_sheet_as_root": false, "spice_external_command": "spice \"%I\"", + "spice_model_current_sheet_as_root": true, "spice_save_all_currents": false, + "spice_save_all_dissipations": false, "spice_save_all_voltages": false, "subpart_first_id": 65, "subpart_id_separator": 0 diff --git a/qa/schematic_utils/schematic_file_util.cpp b/qa/schematic_utils/schematic_file_util.cpp index 609487fd74..84953e055c 100644 --- a/qa/schematic_utils/schematic_file_util.cpp +++ b/qa/schematic_utils/schematic_file_util.cpp @@ -51,7 +51,7 @@ void LoadSheetSchematicContents( const std::string& fileName, SCH_SHEET* sheet ) wxASSERT( fileStream.is_open() ); STDISTREAM_LINE_READER reader; reader.SetStream( fileStream ); - SCH_IO_KICAD_SEXPR_PARSER parser( &reader ); + SCH_IO_KICAD_SEXPR_PARSER parser( &reader, nullptr, 0, sheet ); parser.ParseSchematic( sheet ); } diff --git a/qa/tests/eeschema/CMakeLists.txt b/qa/tests/eeschema/CMakeLists.txt index add6ac0f3b..0a752bd38d 100644 --- a/qa/tests/eeschema/CMakeLists.txt +++ b/qa/tests/eeschema/CMakeLists.txt @@ -61,6 +61,7 @@ set( QA_EESCHEMA_SRCS test_lib_part.cpp test_netlist_exporter_kicad.cpp test_ee_item.cpp + test_incremental_netlister.cpp test_legacy_power_symbols.cpp test_pin_numbers.cpp test_sch_netclass.cpp diff --git a/qa/tests/eeschema/erc/test_erc_hierarchical_schematics.cpp b/qa/tests/eeschema/erc/test_erc_hierarchical_schematics.cpp index 313f1ed79b..85ff244012 100644 --- a/qa/tests/eeschema/erc/test_erc_hierarchical_schematics.cpp +++ b/qa/tests/eeschema/erc/test_erc_hierarchical_schematics.cpp @@ -49,7 +49,7 @@ BOOST_FIXTURE_TEST_CASE( ERCHierarchicalSchematics, ERC_REGRESSION_TEST_FIXTURE { { "issue10926_1", 3 }, { "issue12814", 0 }, - { "ERC_dynamic_power_symbol_test", 5 } + { "ERC_dynamic_power_symbol_test", 2 } }; for( const std::pair& test : tests ) diff --git a/qa/tests/eeschema/test_incremental_netlister.cpp b/qa/tests/eeschema/test_incremental_netlister.cpp new file mode 100644 index 0000000000..0fa9662a02 --- /dev/null +++ b/qa/tests/eeschema/test_incremental_netlister.cpp @@ -0,0 +1,164 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2024 KiCad Developers, see AUTHORS.TXT for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 3 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * https://www.gnu.org/licenses/gpl-3.0.en.html + * or you may search the http://www.gnu.org website for the version 32 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + + +#include +#include + +#include +#include +#include +#include +#include +#include + +struct INCREMENTAL_NETLIST_TEST_FIXTURE +{ + INCREMENTAL_NETLIST_TEST_FIXTURE() : + m_settingsManager( true /* headless */ ) + { } + + SETTINGS_MANAGER m_settingsManager; + std::unique_ptr m_schematic; +}; + +BOOST_FIXTURE_TEST_CASE( RemoveAddItems, INCREMENTAL_NETLIST_TEST_FIXTURE ) +{ + LOCALE_IO dummy; + + // Check for Errors when using global labels + std::vector tests = {// "incremental_test", + // "issue10430", + // "issue10926_1", + // "issue11926", + // "issue12505", + // "issue12814", + // "issue13112", + // "issue13162", + // "issue13212", + // "issue13431", + // "issue13591", + // "issue16223", + // "issue6588", + "issue7203"};//, + // "issue9367"}; + + for( const wxString& test : tests ) + { + KI_TEST::LoadSchematic( m_settingsManager, test, m_schematic ); + + SCH_SHEET_LIST sheets = m_schematic->GetSheets(); + + for( const SCH_SHEET_PATH& path : sheets ) + { + for( size_t ii = 0; ii < path.size(); ++ii ) + { + const SCH_SHEET* sheet = path.GetSheet( ii ); + const SCH_SCREEN* screen = sheet->GetScreen(); + std::vector items; + + for( SCH_ITEM* item : screen->Items() ) + { + if( !item->IsConnectable() ) + { + continue; + } + + if( item->Type() == SCH_SYMBOL_T ) + { + for( SCH_PIN* pin : static_cast( item )->GetPins() ) + { + items.push_back( pin ); + } + } + else + { + items.push_back( item ); + } + } + + for( SCH_ITEM* item : items ) + { + for( SCH_ITEM* check_item : items ) + { + auto& conn_items = check_item->ConnectedItems( path ); + auto conn = check_item->Connection(); + std::string netname = conn ? conn->GetNetName().ToStdString() : "NoNet"; + int subgraph = conn ? conn->SubgraphCode() : -1; + BOOST_TEST_MESSAGE( test.ToStdString() << ": Item " + << check_item->GetFriendlyName().ToStdString() + << " in net " << netname << " subgraph " << subgraph + << " has " << conn_items.size() << " connections" ); + } + SCH_CONNECTION* connection = item->Connection(); + + if( !connection ) + continue; + + wxString netname = connection->GetNetName(); + + if( !item->IsConnectable() ) + continue; + + SCH_ITEM_VEC prev_items = item->ConnectedItems( path ); + std::sort( prev_items.begin(), prev_items.end() ); + alg::remove_duplicates( prev_items ); + + + std::set> all_items = + m_schematic->ConnectionGraph()->ExtractAffectedItems( { item } ); + all_items.insert( { path, item } ); + BOOST_TEST_MESSAGE( test.ToStdString() << ": Item " + << item->GetFriendlyName().ToStdString() + << " in net " << netname.ToStdString() + << " has " << all_items.size() << " affected items" ); + + CONNECTION_GRAPH new_graph( m_schematic.get() ); + + new_graph.SetLastCodes( m_schematic->ConnectionGraph() ); + + for( auto&[ path, item ] : all_items ) + { + wxCHECK2( item, continue ); + item->SetConnectivityDirty(); + } + + new_graph.Recalculate( sheets, false ); + m_schematic->ConnectionGraph()->Merge( new_graph ); + + SCH_ITEM_VEC curr_items = item->ConnectedItems( path ); + std::sort( curr_items.begin(), curr_items.end() ); + alg::remove_duplicates( curr_items ); + + BOOST_CHECK_MESSAGE( prev_items == curr_items, + test.ToStdString() << ": Item " + << item->GetFriendlyName().ToStdString() + << " in net " << netname.ToStdString() + << " changed from " << prev_items.size() + << " to " << curr_items.size() << " Location:" << item->GetPosition().x << "," << item->GetPosition().y ); + } + + } + } + } +} \ No newline at end of file