From a9f35ba42e82b0c3c8a68d42cf89bbc91f8df6c6 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 --- 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 5daf8244b4..6e092a5e4c 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 @@ -635,6 +637,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 @@ -714,37 +731,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 ); } } @@ -774,6 +830,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; @@ -932,12 +989,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 ) { @@ -945,7 +1020,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 ); @@ -957,23 +1032,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 @@ -998,7 +1060,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: @@ -1079,9 +1144,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; @@ -1115,7 +1177,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 ); } } @@ -1217,7 +1279,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 ) { @@ -1252,6 +1314,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 @@ -1382,7 +1446,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 ) { @@ -1560,14 +1624,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 ) @@ -2832,8 +2902,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 ed44bcc82b..cf8bbf9124 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 cf2397a5c3..108a189ce8 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -270,6 +270,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 c8b1ec4f06..8ba88c7817 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -1779,12 +1779,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(); @@ -1802,7 +1806,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 ); } @@ -1821,21 +1824,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 8143656761..d5a2262ef7 100644 --- a/eeschema/sch_item.cpp +++ b/eeschema/sch_item.cpp @@ -257,7 +257,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 ]; } @@ -265,13 +274,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 00c967140c..310b51bdc9 100644 --- a/eeschema/sch_item.h +++ b/eeschema/sch_item.h @@ -158,7 +158,7 @@ public: std::vector& aItemListByPos ); }; -typedef std::vector SCH_ITEM_SET; +typedef std::vector SCH_ITEM_VEC; /** @@ -487,13 +487,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. * @@ -709,7 +714,7 @@ protected: // an initial position 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 1fd2d41829..d8f51d3b2b 100644 --- a/eeschema/sch_label.cpp +++ b/eeschema/sch_label.cpp @@ -275,7 +275,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 cc501e8aa4..468082c858 100644 --- a/eeschema/sch_symbol.cpp +++ b/eeschema/sch_symbol.cpp @@ -755,7 +755,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..e3ab106dce --- /dev/null +++ b/qa/data/eeschema/incremental_test.kicad_sch @@ -0,0 +1,478 @@ +(kicad_sch + (version 20240101) + (generator "eeschema") + (generator_version "8.99") + (uuid "9c07925a-2c60-43f0-9bac-88be11f9747d") + (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 "power:GND") + (at 74.93 80.01 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "082ecf0a-cced-43cc-8685-ec8f3fcd6b19") + (property "Reference" "#PWR03" + (at 74.93 86.36 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Value" "GND" + (at 74.93 85.09 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Footprint" "" + (at 74.93 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "" + (at 74.93 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "Power symbol creates a global label with name \"GND\" , ground" + (at 74.93 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "57103d9d-4b3c-48c2-b977-d64a9baf31c7") + ) + (instances + (project "incremental_test" + (path "/9c07925a-2c60-43f0-9bac-88be11f9747d" + (reference "#PWR03") + (unit 1) + ) + ) + ) + ) + (symbol + (lib_id "Connector:TestPoint") + (at 74.93 80.01 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "490aaa68-2909-49c0-8165-f5e94f598219") + (property "Reference" "TP2" + (at 77.47 75.4379 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Value" "TestPoint" + (at 77.47 77.9779 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Footprint" "" + (at 80.01 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "~" + (at 80.01 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "test point" + (at 74.93 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "359df8f3-a746-432e-ad86-18319a2f1165") + ) + (instances + (project "incremental_test" + (path "/9c07925a-2c60-43f0-9bac-88be11f9747d" + (reference "TP2") + (unit 1) + ) + ) + ) + ) + (symbol + (lib_id "Connector:TestPoint") + (at 88.9 80.01 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "605caf70-d9c1-4338-a34f-cf180570b0fc") + (property "Reference" "TP1" + (at 91.44 75.4379 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Value" "TestPoint" + (at 91.44 77.9779 0) + (effects + (font + (size 1.27 1.27) + ) + (justify left) + ) + ) + (property "Footprint" "" + (at 93.98 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "~" + (at 93.98 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "test point" + (at 88.9 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "394b88b9-3d9c-4d41-a369-21858416767b") + ) + (instances + (project "incremental_test" + (path "/9c07925a-2c60-43f0-9bac-88be11f9747d" + (reference "TP1") + (unit 1) + ) + ) + ) + ) + (symbol + (lib_id "power:GND") + (at 88.9 80.01 0) + (unit 1) + (exclude_from_sim no) + (in_bom yes) + (on_board yes) + (dnp no) + (fields_autoplaced yes) + (uuid "8b732294-87ef-4a3e-b789-48fe9ba4760a") + (property "Reference" "#PWR01" + (at 88.9 86.36 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Value" "GND" + (at 88.9 85.09 0) + (effects + (font + (size 1.27 1.27) + ) + ) + ) + (property "Footprint" "" + (at 88.9 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Datasheet" "" + (at 88.9 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (property "Description" "Power symbol creates a global label with name \"GND\" , ground" + (at 88.9 80.01 0) + (effects + (font + (size 1.27 1.27) + ) + (hide yes) + ) + ) + (pin "1" + (uuid "82cebcce-03b4-4c68-bb1c-0460161b3f56") + ) + (instances + (project "incremental_test" + (path "/9c07925a-2c60-43f0-9bac-88be11f9747d" + (reference "#PWR01") + (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 c9763bd1b6..9ffbf77171 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