From fef3274e8e35d9dc2f7b88b81375f6a83564ec9b Mon Sep 17 00:00:00 2001 From: JamesJCode <13408010-JamesJCode@users.noreply.gitlab.com> Date: Sat, 21 Jan 2023 23:25:40 +0000 Subject: [PATCH] Eeschema: ERC checks handle connections between a common sub-circuit Fixes #10926 Contains the following changes: - Adds a new ERC_SCH_PIN_CONTEXT class which is used to provide deterministic comparison between items causing ERC violations (e.g. pins) when associated with a SCH_SHEET_PATH context. - Adds association of SCH_SHEET_PATHs for ERC_ITEMs and the sub-schematic items which caused an ERC violation. This allows correct display of markers on the sheets of interest only, and allows correct naming resolution and cross-probing from the ERC dialog. - Adds a new ERC_TREE_MODEL class, derived from RC_TREE_MODEL, which correctly resolves component references across heirarchical sheets using the associated SCH_SHEET_PATHs. This allows sheet-specific component references to be displayed correctly in the ERC results tree. - Updates SCH_MARKER to only draw sheet-specific markers on the sheet causing an ERC violation. - Increments the schematic file version. - When loading a schematic with legacy ERC exclusions, discards those of type ERCE_PIN_TO_PIN_WARNING, ERCE_PIN_TO_PIN_ERROR, ERCE_HIERACHICAL_LABEL, and ERCE_DIFFERENT_UNIT_NET as there is no safe way to automatically infer the information which is now stored with these exclusions (sheet paths for error location and related items). Requiring users to (once) re-add exclusions is preferable to silently incorrectly matching new ERC issues to legacy exclusions. --- eeschema/CMakeLists.txt | 1 + eeschema/connection_graph.cpp | 9 +- eeschema/dialogs/dialog_erc.cpp | 26 ++- eeschema/erc.cpp | 76 +++++---- eeschema/erc_item.cpp | 113 ++++++++++++- eeschema/erc_item.h | 121 +++++++++++++- eeschema/erc_sch_pin_context.cpp | 73 +++++++++ eeschema/erc_sch_pin_context.h | 82 ++++++++++ eeschema/erc_settings.cpp | 15 +- eeschema/sch_file_versions.h | 3 +- eeschema/sch_marker.cpp | 93 +++++++++-- eeschema/sch_marker.h | 16 +- eeschema/schematic.cpp | 35 +++- include/rc_item.h | 2 +- qa/data/eeschema/issue10926_1.kicad_sch | 72 +++++++++ .../issue10926_1_subsheet_1.kicad_sch | 149 ++++++++++++++++++ .../issue10926_1_subsheet_1_1.kicad_sch | 94 +++++++++++ qa/unittests/eeschema/CMakeLists.txt | 1 + .../erc/test_erc_hierarchical_schematics.cpp | 75 +++++++++ 19 files changed, 993 insertions(+), 63 deletions(-) create mode 100644 eeschema/erc_sch_pin_context.cpp create mode 100644 eeschema/erc_sch_pin_context.h create mode 100644 qa/data/eeschema/issue10926_1.kicad_sch create mode 100644 qa/data/eeschema/issue10926_1_subsheet_1.kicad_sch create mode 100644 qa/data/eeschema/issue10926_1_subsheet_1_1.kicad_sch create mode 100644 qa/unittests/eeschema/erc/test_erc_hierarchical_schematics.cpp diff --git a/eeschema/CMakeLists.txt b/eeschema/CMakeLists.txt index 8a7cdb37c3..92fcfd145f 100644 --- a/eeschema/CMakeLists.txt +++ b/eeschema/CMakeLists.txt @@ -196,6 +196,7 @@ set( EESCHEMA_SRCS eeschema_settings.cpp erc.cpp erc_item.cpp + erc_sch_pin_context.cpp erc_settings.cpp fields_grid_table.cpp files-io.cpp diff --git a/eeschema/connection_graph.cpp b/eeschema/connection_graph.cpp index c851249447..e5d3f48a1f 100644 --- a/eeschema/connection_graph.cpp +++ b/eeschema/connection_graph.cpp @@ -3266,7 +3266,8 @@ int CONNECTION_GRAPH::ercCheckHierSheets() std::shared_ptr ercItem = ERC_ITEM::Create( ERCE_HIERACHICAL_LABEL ); ercItem->SetItems( unmatched.second ); ercItem->SetErrorMessage( msg ); - ercItem->SetIsSheetSpecific(); + ercItem->SetSheetSpecificPath( sheet ); + ercItem->SetItemsSheetPaths( sheet ); SCH_MARKER* marker = new SCH_MARKER( ercItem, unmatched.second->GetPosition() ); sheet.LastScreen()->Append( marker ); @@ -3280,10 +3281,14 @@ int CONNECTION_GRAPH::ercCheckHierSheets() "sheet pin in the parent sheet" ), UnescapeString( unmatched.first ) ); + SCH_SHEET_PATH parentSheetPath = sheet; + parentSheetPath.push_back( parentSheet ); + std::shared_ptr ercItem = ERC_ITEM::Create( ERCE_HIERACHICAL_LABEL ); ercItem->SetItems( unmatched.second ); ercItem->SetErrorMessage( msg ); - ercItem->SetIsSheetSpecific(); + ercItem->SetSheetSpecificPath( parentSheetPath ); + ercItem->SetItemsSheetPaths( parentSheetPath ); SCH_MARKER* marker = new SCH_MARKER( ercItem, unmatched.second->GetPosition() ); parentSheet->GetScreen()->Append( marker ); diff --git a/eeschema/dialogs/dialog_erc.cpp b/eeschema/dialogs/dialog_erc.cpp index 22ba1d1fdc..dd23f2a1a5 100644 --- a/eeschema/dialogs/dialog_erc.cpp +++ b/eeschema/dialogs/dialog_erc.cpp @@ -80,7 +80,7 @@ DIALOG_ERC::DIALOG_ERC( SCH_EDIT_FRAME* parent ) : m_markerProvider = std::make_shared( &m_parent->Schematic() ); - m_markerTreeModel = new RC_TREE_MODEL( parent, m_markerDataView ); + m_markerTreeModel = new ERC_TREE_MODEL( parent, m_markerDataView ); m_markerDataView->AssociateModel( m_markerTreeModel ); m_markerTreeModel->Update( m_markerProvider, m_severities ); @@ -582,6 +582,30 @@ void DIALOG_ERC::OnERCItemSelected( wxDataViewEvent& aEvent ) } else if( item && item->GetClass() != wxT( "DELETED_SHEET_ITEM" ) ) { + const RC_TREE_NODE* node = RC_TREE_MODEL::ToNode( aEvent.GetItem() ); + + if( node ) + { + // Determine the owning sheet for sheet-specific items + std::shared_ptr ercItem = + std::static_pointer_cast( node->m_RcItem ); + switch( node->m_Type ) + { + case RC_TREE_NODE::MARKER: + if( ercItem->IsSheetSpecific() ) + sheet = ercItem->GetSpecificSheetPath(); + break; + case RC_TREE_NODE::MAIN_ITEM: + if( ercItem->MainItemHasSheetPath() ) + sheet = ercItem->GetMainItemSheetPath(); + break; + case RC_TREE_NODE::AUX_ITEM: + if( ercItem->AuxItemHasSheetPath() ) + sheet = ercItem->GetAuxItemSheetPath(); + break; + } + } + WINDOW_THAWER thawer( m_parent ); if( !sheet.empty() && sheet != m_parent->GetCurrentSheet() ) diff --git a/eeschema/erc.cpp b/eeschema/erc.cpp index 34a460f888..e136448bb8 100644 --- a/eeschema/erc.cpp +++ b/eeschema/erc.cpp @@ -29,6 +29,7 @@ #include "connection_graph.h" #include // for ExpandEnvVarSubstitutions #include +#include #include #include #include @@ -38,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -600,7 +600,7 @@ int ERC_TESTER::TestPinToPin() for( const std::pair> net : nets ) { - std::vector pins; + std::vector pins; std::unordered_map pinToScreenMap; bool has_noconnect = false; @@ -613,44 +613,46 @@ int ERC_TESTER::TestPinToPin() { if( item->Type() == SCH_PIN_T ) { - pins.emplace_back( static_cast( item ) ); + pins.emplace_back( static_cast( item ), subgraph->m_sheet ); pinToScreenMap[item] = subgraph->m_sheet.LastScreen(); } } } - std::set> tested; + std::set> tested; - SCH_PIN* needsDriver = nullptr; - bool hasDriver = false; + ERC_SCH_PIN_CONTEXT needsDriver; + bool hasDriver = false; // We need different drivers for power nets and normal nets. // A power net has at least one pin having the ELECTRICAL_PINTYPE::PT_POWER_IN // and power nets can be driven only by ELECTRICAL_PINTYPE::PT_POWER_OUT pins bool ispowerNet = false; - for( SCH_PIN* refPin : pins ) + for( ERC_SCH_PIN_CONTEXT& refPin : pins ) { - if( refPin->GetType() == ELECTRICAL_PINTYPE::PT_POWER_IN ) + if( refPin.Pin()->GetType() == ELECTRICAL_PINTYPE::PT_POWER_IN ) { ispowerNet = true; break; } } - for( SCH_PIN* refPin : pins ) + for( ERC_SCH_PIN_CONTEXT& refPin : pins ) { - ELECTRICAL_PINTYPE refType = refPin->GetType(); + ELECTRICAL_PINTYPE refType = refPin.Pin()->GetType(); if( DrivenPinTypes.count( refType ) ) { // needsDriver will be the pin shown in the error report eventually, so try to // upgrade to a "better" pin if possible: something visible and only a power symbol // if this net needs a power driver - if( !needsDriver || - ( !needsDriver->IsVisible() && refPin->IsVisible() ) || - ( ispowerNet != ( needsDriver->GetType() == ELECTRICAL_PINTYPE::PT_POWER_IN ) && - ispowerNet == ( refType == ELECTRICAL_PINTYPE::PT_POWER_IN ) ) ) + if( !needsDriver.Pin() + || ( !needsDriver.Pin()->IsVisible() && refPin.Pin()->IsVisible() ) + || ( ispowerNet + != ( needsDriver.Pin()->GetType() + == ELECTRICAL_PINTYPE::PT_POWER_IN ) + && ispowerNet == ( refType == ELECTRICAL_PINTYPE::PT_POWER_IN ) ) ) { needsDriver = refPin; } @@ -661,18 +663,19 @@ int ERC_TESTER::TestPinToPin() else hasDriver |= ( DrivingPinTypes.count( refType ) != 0 ); - for( SCH_PIN* testPin : pins ) + for( ERC_SCH_PIN_CONTEXT& testPin : pins ) { if( testPin == refPin ) continue; - SCH_PIN* first_pin = refPin; - SCH_PIN* second_pin = testPin; + ERC_SCH_PIN_CONTEXT first_pin = refPin; + ERC_SCH_PIN_CONTEXT second_pin = testPin; - if( first_pin > second_pin ) + if( second_pin < first_pin ) std::swap( first_pin, second_pin ); - std::pair pair = std::make_pair( first_pin, second_pin ); + std::pair pair = + std::make_pair( first_pin, second_pin ); if( auto [ins_pin, inserted ] = tested.insert( pair ); !inserted ) continue; @@ -680,13 +683,14 @@ int ERC_TESTER::TestPinToPin() // Multiple pins in the same symbol that share a type, // name and position are considered // "stacked" and shouldn't trigger ERC errors - if( refPin->GetParent() == testPin->GetParent() && - refPin->GetPosition() == testPin->GetPosition() && - refPin->GetName() == testPin->GetName() && - refPin->GetType() == testPin->GetType() ) + if( refPin.Pin()->GetParent() == testPin.Pin()->GetParent() + && refPin.Pin()->GetPosition() == testPin.Pin()->GetPosition() + && refPin.Pin()->GetName() == testPin.Pin()->GetName() + && refPin.Pin()->GetType() == testPin.Pin()->GetType() + && refPin.Sheet() == testPin.Sheet() ) continue; - ELECTRICAL_PINTYPE testType = testPin->GetType(); + ELECTRICAL_PINTYPE testType = testPin.Pin()->GetType(); if( ispowerNet ) hasDriver |= ( DrivingPowerPinTypes.count( testType ) != 0 ); @@ -700,23 +704,24 @@ int ERC_TESTER::TestPinToPin() std::shared_ptr ercItem = ERC_ITEM::Create( erc == PIN_ERROR::WARNING ? ERCE_PIN_TO_PIN_WARNING : ERCE_PIN_TO_PIN_ERROR ); - ercItem->SetItems( refPin, testPin ); - ercItem->SetIsSheetSpecific(); + ercItem->SetItems( refPin.Pin(), testPin.Pin() ); + ercItem->SetSheetSpecificPath( refPin.Sheet() ); + ercItem->SetItemsSheetPaths( refPin.Sheet(), testPin.Sheet() ); ercItem->SetErrorMessage( wxString::Format( _( "Pins of type %s and %s are connected" ), ElectricalPinTypeGetText( refType ), ElectricalPinTypeGetText( testType ) ) ); - SCH_MARKER* marker = new SCH_MARKER( ercItem, - refPin->GetTransformedPosition() ); - pinToScreenMap[refPin]->Append( marker ); + SCH_MARKER* marker = + new SCH_MARKER( ercItem, refPin.Pin()->GetTransformedPosition() ); + pinToScreenMap[refPin.Pin()]->Append( marker ); errors++; } } } - if( needsDriver && !hasDriver && !has_noconnect ) + if( needsDriver.Pin() && !hasDriver && !has_noconnect ) { int err_code = ispowerNet ? ERCE_POWERPIN_NOT_DRIVEN : ERCE_PIN_NOT_DRIVEN; @@ -724,11 +729,11 @@ int ERC_TESTER::TestPinToPin() { std::shared_ptr ercItem = ERC_ITEM::Create( err_code ); - ercItem->SetItems( needsDriver ); + ercItem->SetItems( needsDriver.Pin() ); - SCH_MARKER* marker = new SCH_MARKER( ercItem, - needsDriver->GetTransformedPosition() ); - pinToScreenMap[needsDriver]->Append( marker ); + SCH_MARKER* marker = + new SCH_MARKER( ercItem, needsDriver.Pin()->GetTransformedPosition() ); + pinToScreenMap[needsDriver.Pin()]->Append( marker ); errors++; } } @@ -780,7 +785,8 @@ int ERC_TESTER::TestMultUnitPinConflicts() pinToNetMap[name].first ) ); ercItem->SetItems( pin, pinToNetMap[name].second ); - ercItem->SetIsSheetSpecific(); + ercItem->SetSheetSpecificPath( subgraph->m_sheet ); + ercItem->SetItemsSheetPaths( subgraph->m_sheet, subgraph->m_sheet ); SCH_MARKER* marker = new SCH_MARKER( ercItem, pin->GetTransformedPosition() ); diff --git a/eeschema/erc_item.cpp b/eeschema/erc_item.cpp index e84b76e19e..7905118a8b 100644 --- a/eeschema/erc_item.cpp +++ b/eeschema/erc_item.cpp @@ -25,8 +25,9 @@ #include "wx/html/m_templ.h" #include "wx/html/styleparams.h" #include -#include -#include +#include +#include +#include // These, being statically-defined, require specialized I18N handling. We continue to @@ -275,3 +276,111 @@ std::shared_ptr ERC_ITEM::Create( int aErrorCode ) return nullptr; } + +/** + * Override of RC_TREE_MODEL::GetValue which returns item descriptions in a specific + * SCH_SHEET_PATH context, if a context is available on the given SCH_MARKER or ERC_ITEM + * targets. + */ +void ERC_TREE_MODEL::GetValue( wxVariant& aVariant, wxDataViewItem const& aItem, + unsigned int aCol ) const +{ + const RC_TREE_NODE* node = ToNode( aItem ); + const std::shared_ptr rcItem = node->m_RcItem; + + switch( node->m_Type ) + { + case RC_TREE_NODE::MARKER: + { + wxString prefix; + + if( rcItem->GetParent() ) + { + SEVERITY severity = rcItem->GetParent()->GetSeverity(); + + if( severity == RPT_SEVERITY_EXCLUSION ) + { + if( m_editFrame->GetSeverity( rcItem->GetErrorCode() ) == RPT_SEVERITY_WARNING ) + prefix = _( "Excluded warning: " ); + else + prefix = _( "Excluded error: " ); + } + else if( severity == RPT_SEVERITY_WARNING ) + { + prefix = _( "Warning: " ); + } + else + { + prefix = _( "Error: " ); + } + } + + aVariant = prefix + rcItem->GetErrorMessage(); + } + break; + + case RC_TREE_NODE::MAIN_ITEM: + if( rcItem->GetParent() + && rcItem->GetParent()->GetMarkerType() == MARKER_BASE::MARKER_DRAWING_SHEET ) + { + aVariant = _( "Drawing Sheet" ); + break; + } + else + { + std::shared_ptr ercItem = std::static_pointer_cast( rcItem ); + SCH_EDIT_FRAME* schEditFrame = static_cast( m_editFrame ); + SCH_SHEET_PATH curPath; + + // Update the target ERC item reference field to a specific sheet context if present + curPath = schEditFrame->GetCurrentSheet(); + + if( ercItem->MainItemHasSheetPath() ) + ercItem->GetMainItemSheetPath().UpdateAllScreenReferences(); + + EDA_ITEM* item = m_editFrame->GetItem( rcItem->GetMainItemID() ); + aVariant = item->GetItemDescription( m_editFrame ); + + // Reset reference fields to current visible sheet + if( ercItem->MainItemHasSheetPath() ) + curPath.UpdateAllScreenReferences(); + } + + break; + + case RC_TREE_NODE::AUX_ITEM: + { + std::shared_ptr ercItem = std::static_pointer_cast( rcItem ); + SCH_EDIT_FRAME* schEditFrame = static_cast( m_editFrame ); + SCH_SHEET_PATH curPath; + + // Update the target ERC item reference field to a specific sheet context if present + curPath = schEditFrame->GetCurrentSheet(); + + if( ercItem->AuxItemHasSheetPath() ) + ercItem->GetAuxItemSheetPath().UpdateAllScreenReferences(); + + EDA_ITEM* item = m_editFrame->GetItem( rcItem->GetMainItemID() ); + aVariant = item->GetItemDescription( m_editFrame ); + + // Reset reference fields to current visible sheet + if( ercItem->AuxItemHasSheetPath() ) + curPath.UpdateAllScreenReferences(); + } + break; + + case RC_TREE_NODE::AUX_ITEM2: + { + EDA_ITEM* item = m_editFrame->GetItem( rcItem->GetAuxItem2ID() ); + aVariant = item->GetItemDescription( m_editFrame ); + } + break; + + case RC_TREE_NODE::AUX_ITEM3: + { + EDA_ITEM* item = m_editFrame->GetItem( rcItem->GetAuxItem3ID() ); + aVariant = item->GetItemDescription( m_editFrame ); + } + break; + } +} diff --git a/eeschema/erc_item.h b/eeschema/erc_item.h index 59eb8f7925..a94088d443 100644 --- a/eeschema/erc_item.h +++ b/eeschema/erc_item.h @@ -24,8 +24,33 @@ #ifndef ERC_ITEM_H #define ERC_ITEM_H +#include #include +#include "sch_sheet_path.h" +/** + * A specialisation of the RC_TREE_MODEL class to enable ERC errors / warnings to be resolved in + * a specific sheet path context. This allows the displayed ERC descriptions in the ERC dialog to + * reflect component references on a per-sheet basis in hierarchical schematics. + * @see RC_TREE_MODEL + */ +class ERC_TREE_MODEL : public RC_TREE_MODEL +{ +public: + ERC_TREE_MODEL( EDA_DRAW_FRAME* aParentFrame, wxDataViewCtrl* aView ) : + RC_TREE_MODEL( aParentFrame, aView ) + { + } + + ~ERC_TREE_MODEL() {} + + /** + * Override of RC_TREE_MODEL::GetValue which returns item descriptions in a specific + * SCH_SHEET_PATH context, if a context is available on the given SCH_MARKER or ERC_ITEM + * targets. + */ + void GetValue( wxVariant& aVariant, wxDataViewItem const& aItem, unsigned int aCol ) const; +}; class ERC_ITEM : public RC_ITEM { @@ -52,18 +77,104 @@ public: return allItemTypes; } - bool IsSheetSpecific() const { return m_sheetSpecific; } - void SetIsSheetSpecific( bool aSpecific = true ) { m_sheetSpecific = aSpecific; } + /** + * Determines whether the ERC item is bound to a specific sheet, or is common across multiple + * sheets (e.g. whether the error is internal to a hierarchical sheet, or is due to an enclosing + * context interacting with the hierarchical sheet) + * @return true if ERC applies to a specific sheet, otherwise false + */ + bool IsSheetSpecific() const { return m_sheetSpecificPath.has_value(); } + + /** + * Sets the SCH_SHEET_PATH this ERC item is bound to + * @param aSpecificSheet The SCH_SHEET_PATH containing the ERC violation + */ + void SetSheetSpecificPath( const SCH_SHEET_PATH& aSpecificSheet ) + { + m_sheetSpecificPath = aSpecificSheet; + } + + /** + * Gets the SCH_SHEET_PATH this ERC item is bound to. Throws std::bad_optional_access if there + * is no specific sheet path binding + * @return the SCH_SHEET_PATH containing the ERC violation + */ + const SCH_SHEET_PATH& GetSpecificSheetPath() const + { + wxASSERT( m_sheetSpecificPath.has_value() ); + return m_sheetSpecificPath.value(); + } + + /** + * Sets the SCH_SHEET_PATH of the main item causing this ERC violation to (e.g. a schematic + * pin). This allows violations to be specific to particular uses of shared hierarchical + * schematics. + * @param mainItemSheet the SCH_SHEET_PATH of the item causing the ERC violation + */ + void SetItemsSheetPaths( const SCH_SHEET_PATH& mainItemSheet ) + { + m_mainItemSheet = mainItemSheet; + } + + /** + * Set the SCH_SHEET PATHs of the main and auxiliary items causing this ERC violation to (e.g. + * two schematic pins which have a mutual connection violation). This allows violations to be + * specific to particular uses of shared hierarchical schematics. + * @param mainItemSheet the SCH_SHEET_PATH of the first item causing the ERC violation + * @param auxItemSheet the SCH_SHEET_PATH of the second item causing the ERC violation + */ + void SetItemsSheetPaths( const SCH_SHEET_PATH& mainItemSheet, + const SCH_SHEET_PATH& auxItemSheet ) + { + m_mainItemSheet = mainItemSheet; + m_auxItemSheet = auxItemSheet; + } + + /** + * Gets the SCH_SHEET_PATH of the main item causing this ERC violation + * @return SCH_SHEET_PATH containing the main item + */ + SCH_SHEET_PATH& GetMainItemSheetPath() + { + wxASSERT( MainItemHasSheetPath() ); + return m_mainItemSheet.value(); + } + + /** + * Gets the SCH_SHEET_PATH of the auxillary item causing this ERC violation + * @return SCH_SHEET_PATH containing the auxillary item + */ + SCH_SHEET_PATH& GetAuxItemSheetPath() + { + wxASSERT( AuxItemHasSheetPath() ); + return m_auxItemSheet.value(); + } + + /** + * Determines whether the main item causing this ERC violation has a specific + * SCH_SHEET_PATH binding. + * @return true if the item ERC violation is specific to a sheet, false otherwise + */ + bool MainItemHasSheetPath() { return m_mainItemSheet.has_value(); } + + /** + * Determines whether the auxiliary item causing this ERC violation has a specific + * SCH_SHEET_PATH binding. + * @return true if the item ERC violation is specific to a sheet, false otherwise + */ + bool AuxItemHasSheetPath() { return m_auxItemSheet.has_value(); } private: ERC_ITEM( int aErrorCode = 0, const wxString& aTitle = "", const wxString& aSettingsKey = "" ) { m_errorCode = aErrorCode; m_errorTitle = aTitle; - m_settingsKey = aSettingsKey; - m_sheetSpecific = false; + m_settingsKey = aSettingsKey; } + std::optional m_mainItemSheet; + std::optional m_auxItemSheet; + /// A list of all ERC_ITEM types which are valid error codes static std::vector> allItemTypes; @@ -108,7 +219,7 @@ private: static ERC_ITEM busEntryNeeded; /// True if this item is specific to a sheet instance (as opposed to applying to all instances) - bool m_sheetSpecific; + std::optional m_sheetSpecificPath; }; diff --git a/eeschema/erc_sch_pin_context.cpp b/eeschema/erc_sch_pin_context.cpp new file mode 100644 index 0000000000..8ee55eadda --- /dev/null +++ b/eeschema/erc_sch_pin_context.cpp @@ -0,0 +1,73 @@ +/* +* This program source code file is part of KiCad, a free EDA CAD application. +* +* Copyright (C) 2015 Jean-Pierre Charras, jp.charras at wanadoo.fr +* Copyright (C) 2011 Wayne Stambaugh +* Copyright (C) 1992-2023 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 2 +* 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: +* http://www.gnu.org/licenses/old-licenses/gpl-2.0.html +* or you may search the http://www.gnu.org website for the version 2 license, +* or you may write to the Free Software Foundation, Inc., +* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA +*/ + +/** + * @file erc_sch_pin_context.cpp + */ + +#include + +/** + * Gets the SCH_PIN for this context + */ +SCH_PIN* ERC_SCH_PIN_CONTEXT::Pin() +{ + return m_pin; +} + +/** + * Gets the SCH_SHEET_PATH context for the paired SCH_PIN + */ +SCH_SHEET_PATH& ERC_SCH_PIN_CONTEXT::Sheet() +{ + return m_sheet; +} + +/** + * Tests two pin contexts for equality based on the deterministic hash +*/ +bool ERC_SCH_PIN_CONTEXT::operator==( const ERC_SCH_PIN_CONTEXT& other ) const +{ + return m_hash == other.m_hash; +} + +/** + * Provides a deterministic ordering for item contexts based on hash value + */ +bool ERC_SCH_PIN_CONTEXT::operator<( const ERC_SCH_PIN_CONTEXT& other ) const +{ + return m_hash < other.m_hash; +} + +/** + * Calculates the deterministic hash for this pin / sheet context + */ +void ERC_SCH_PIN_CONTEXT::rehash() +{ + m_hash = 0; + + boost::hash_combine( m_hash, m_pin ); + boost::hash_combine( m_hash, m_sheet.GetCurrentHash() ); +} diff --git a/eeschema/erc_sch_pin_context.h b/eeschema/erc_sch_pin_context.h new file mode 100644 index 0000000000..bb3b0d494c --- /dev/null +++ b/eeschema/erc_sch_pin_context.h @@ -0,0 +1,82 @@ +/* +* This program source code file is part of KiCad, a free EDA CAD application. +* +* Copyright (C) 2009 Jean-Pierre Charras, jp.charras at wanadoo.fr +* Copyright (C) 2011 Wayne Stambaugh +* Copyright (C) 2009-2023 KiCad Developers, see change_log.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 2 +* 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: +* http://www.gnu.org/licenses/old-licenses/gpl-2.0.html +* or you may search the http://www.gnu.org website for the version 2 license, +* or you may write to the Free Software Foundation, Inc., +* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA +*/ + +/** + * @file erc_sch_pin_context.h + */ + +#ifndef _ERC_ITEM_CONTEXT_H +#define _ERC_ITEM_CONTEXT_H + +#include +#include + +/** + * A class used to associate a SCH_PIN with its owning SCH_SHEET_PATH, in order to handle ERC checks + * across connected common hierarchical schematics + */ +class ERC_SCH_PIN_CONTEXT +{ +protected: + SCH_PIN* m_pin; + SCH_SHEET_PATH m_sheet; + size_t m_hash; + +public: + ERC_SCH_PIN_CONTEXT() : m_pin( nullptr ), m_sheet(), m_hash( 0 ) {} + + ERC_SCH_PIN_CONTEXT( SCH_PIN* pin, SCH_SHEET_PATH sheet ) : m_pin( pin ), m_sheet( sheet ) + { + rehash(); + } + + ERC_SCH_PIN_CONTEXT( const ERC_SCH_PIN_CONTEXT& other ) = default; + + ERC_SCH_PIN_CONTEXT& operator=( const ERC_SCH_PIN_CONTEXT& other ) = default; + + ~ERC_SCH_PIN_CONTEXT() = default; + + /** + * Gets the SCH_PIN for this context + */ + SCH_PIN* Pin(); + + /** + * Gets the SCH_SHEET_PATH context for the paired SCH_PIN + */ + SCH_SHEET_PATH& Sheet(); + + bool operator==( const ERC_SCH_PIN_CONTEXT& other ) const; + + bool operator<( const ERC_SCH_PIN_CONTEXT& other ) const; + +protected: + /** + * Calculates the deterministic hash for this context + */ + void rehash(); +}; + +#endif //_ERC_ITEM_CONTEXT_H diff --git a/eeschema/erc_settings.cpp b/eeschema/erc_settings.cpp index 15e5ff0038..1a76178d29 100644 --- a/eeschema/erc_settings.cpp +++ b/eeschema/erc_settings.cpp @@ -291,13 +291,20 @@ void SHEETLIST_ERC_ITEMS_PROVIDER::visitMarkers( std::functionGetMarkerType() != MARKER_BASE::MARKER_ERC ) continue; - // Don't show non-specific markers more than once - if( !firstTime && - !static_cast( marker->GetRCItem().get() )->IsSheetSpecific() ) + std::shared_ptr ercItem = + std::static_pointer_cast( marker->GetRCItem() ); + + // Only show sheet-specific markers on the owning sheet + if( ercItem->IsSheetSpecific() ) { - continue; + if( ercItem->GetSpecificSheetPath() != sheetList[i] ) + continue; } + // Don't show non-specific markers more than once + if( !firstTime && !ercItem->IsSheetSpecific() ) + continue; + aVisitor( marker ); } } diff --git a/eeschema/sch_file_versions.h b/eeschema/sch_file_versions.h index 711538cb76..2b3902caeb 100644 --- a/eeschema/sch_file_versions.h +++ b/eeschema/sch_file_versions.h @@ -93,4 +93,5 @@ //#define SEXPR_SCHEMATIC_FILE_VERSION 20221004 // Move instance data back into symbol definition. //#define SEXPR_SCHEMATIC_FILE_VERSION 20221110 // Move sheet instance data to sheet definition. //#define SEXPR_SCHEMATIC_FILE_VERSION 20221126 // Remove value and footprint from instance data. -#define SEXPR_SCHEMATIC_FILE_VERSION 20221206 // Simulation model fields V6 -> V7 +//#define SEXPR_SCHEMATIC_FILE_VERSION 20221206 // Simulation model fields V6 -> V7 +#define SEXPR_SCHEMATIC_FILE_VERSION 20230121 // SCH_MARKER specific sheet path serialisation diff --git a/eeschema/sch_marker.cpp b/eeschema/sch_marker.cpp index 5095045e87..13810a59b5 100644 --- a/eeschema/sch_marker.cpp +++ b/eeschema/sch_marker.cpp @@ -35,6 +35,9 @@ #include #include #include +#include +#include +#include /// Factor to convert the maker unit shape to internal units: #define SCALING_FACTOR schIUScale.mmToIU( 0.15 ) @@ -48,6 +51,8 @@ SCH_MARKER::SCH_MARKER( std::shared_ptr aItem, const VECTOR2I& aPos ) m_rcItem->SetParent( this ); m_Pos = aPos; + + m_isLegacyMarker = false; } @@ -72,16 +77,26 @@ void SCH_MARKER::SwapData( SCH_ITEM* aItem ) wxString SCH_MARKER::Serialize() const { - return wxString::Format( wxT( "%s|%d|%d|%s|%s" ), - m_rcItem->GetSettingsKey(), - m_Pos.x, - m_Pos.y, - m_rcItem->GetMainItemID().AsString(), - m_rcItem->GetAuxItemID().AsString() ); + std::shared_ptr erc = std::static_pointer_cast( m_rcItem ); + wxString sheetSpecificPath, mainItemPath, auxItemPath; + + if( erc->IsSheetSpecific() ) + sheetSpecificPath = erc->GetSpecificSheetPath().Path().AsString(); + + if( erc->MainItemHasSheetPath() ) + mainItemPath = erc->GetMainItemSheetPath().Path().AsString(); + + if( erc->AuxItemHasSheetPath() ) + auxItemPath = erc->GetAuxItemSheetPath().Path().AsString(); + + return wxString::Format( wxT( "%s|%d|%d|%s|%s|%s|%s|%s" ), m_rcItem->GetSettingsKey(), m_Pos.x, + m_Pos.y, m_rcItem->GetMainItemID().AsString(), + m_rcItem->GetAuxItemID().AsString(), sheetSpecificPath, mainItemPath, + auxItemPath ); } -SCH_MARKER* SCH_MARKER::Deserialize( const wxString& data ) +SCH_MARKER* SCH_MARKER::Deserialize( SCHEMATIC* schematic, const wxString& data ) { wxArrayString props = wxSplit( data, '|' ); VECTOR2I markerPos( (int) strtol( props[1].c_str(), nullptr, 10 ), @@ -94,7 +109,55 @@ SCH_MARKER* SCH_MARKER::Deserialize( const wxString& data ) ercItem->SetItems( KIID( props[3] ), KIID( props[4] ) ); - return new SCH_MARKER( ercItem, markerPos ); + bool isLegacyMarker = true; + + // Deserialize sheet / item specific paths - we are not able to use the file version to determine + // if markers are legacy as there could be a file opened with a prior version but which has + // new markers - this code is called not just during schematic load, but also to match new + // ERC exceptions to exclusions. + if( props.size() == 8 ) + { + isLegacyMarker = false; + + SCH_SHEET_LIST sheetPaths = schematic->GetSheets(); + + if( !props[5].IsEmpty() ) + { + KIID_PATH sheetSpecificKiidPath( props[5] ); + std::optional sheetSpecificPath = + sheetPaths.GetSheetPathByKIIDPath( sheetSpecificKiidPath, true ); + if( sheetSpecificPath.has_value() ) + ercItem->SetSheetSpecificPath( sheetSpecificPath.value() ); + } + + if( !props[6].IsEmpty() ) + { + KIID_PATH mainItemKiidPath( props[6] ); + std::optional mainItemPath = + sheetPaths.GetSheetPathByKIIDPath( mainItemKiidPath, true ); + if( mainItemPath.has_value() ) + { + if( props[7].IsEmpty() ) + { + ercItem->SetItemsSheetPaths( mainItemPath.value() ); + } + else + { + KIID_PATH auxItemKiidPath( props[7] ); + std::optional auxItemPath = + sheetPaths.GetSheetPathByKIIDPath( auxItemKiidPath, true ); + + if( auxItemPath.has_value() ) + ercItem->SetItemsSheetPaths( mainItemPath.value(), auxItemPath.value() ); + } + } + } + } + + SCH_MARKER* marker = new SCH_MARKER( ercItem, markerPos ); + marker->SetIsLegacyMarker( isLegacyMarker ); + + return marker; } @@ -111,10 +174,20 @@ void SCH_MARKER::Show( int nestLevel, std::ostream& os ) const void SCH_MARKER::ViewGetLayers( int aLayers[], int& aCount ) const { - aCount = 2; - wxCHECK_RET( Schematic(), "No SCHEMATIC set for SCH_MARKER!" ); + // Don't display sheet-specific markers when SCH_SHEET_PATHs do not match + std::shared_ptr ercItem = std::static_pointer_cast( GetRCItem() ); + + if( ercItem->IsSheetSpecific() + && ( ercItem->GetSpecificSheetPath() != Schematic()->CurrentSheet() ) ) + { + aCount = 0; + return; + } + + aCount = 2; + if( IsExcluded() ) { aLayers[0] = LAYER_ERC_EXCLUSION; diff --git a/eeschema/sch_marker.h b/eeschema/sch_marker.h index dc78c284e8..891738ac15 100644 --- a/eeschema/sch_marker.h +++ b/eeschema/sch_marker.h @@ -54,7 +54,7 @@ public: void SwapData( SCH_ITEM* aItem ) override; wxString Serialize() const; - static SCH_MARKER* Deserialize( const wxString& data ); + static SCH_MARKER* Deserialize( SCHEMATIC* schematic, const wxString& data ); void ViewGetLayers( int aLayers[], int& aCount ) const override; @@ -108,12 +108,26 @@ public: EDA_ITEM* Clone() const override; + /** + * Sets this marker as a legacy artifact. Legacy markers are those deserialized from a file + * version < 20230121 + */ + void SetIsLegacyMarker( bool isLegacyMarker = true ) { m_isLegacyMarker = isLegacyMarker; } + + /** + * Determines if this marker is legacy (i.e. does not store sheet paths for specific errors) + * @return True if marker deserialized from a file version < 20230121 + */ + bool IsLegacyMarker() const { return m_isLegacyMarker; } + #if defined(DEBUG) void Show( int nestLevel, std::ostream& os ) const override; #endif protected: KIGFX::COLOR4D getColor() const override; + + bool m_isLegacyMarker; /// True if marker was deserialized from a file version < 20230121 }; #endif // TYPE_SCH_MARKER_H_ diff --git a/eeschema/schematic.cpp b/eeschema/schematic.cpp index 92c375c6fe..a2635b026b 100644 --- a/eeschema/schematic.cpp +++ b/eeschema/schematic.cpp @@ -194,6 +194,39 @@ std::vector SCHEMATIC::ResolveERCExclusions() SCH_SHEET_LIST sheetList = GetSheets(); ERC_SETTINGS& settings = ErcSettings(); + // Migrate legacy marker exclusions to new format to ensure exclusion matching functions across + // file versions. Silently drops any legacy exclusions which can not be mapped to the new format + // without risking an incorrect exclusion - this is preferable to silently dropping + // new ERC errors / warnings due to an incorrect match between a legacy and new + // marker serialization format + std::set migratedExclusions; + + for( auto it = settings.m_ErcExclusions.begin(); it != settings.m_ErcExclusions.end(); ) + { + SCH_MARKER* testMarker = SCH_MARKER::Deserialize( this, *it ); + if( testMarker->IsLegacyMarker() ) + { + const wxString settingsKey = testMarker->GetRCItem()->GetSettingsKey(); + + if( settingsKey != wxT( "pin_to_pin" ) && settingsKey != wxT( "hier_label_mismatch" ) + && settingsKey != wxT( "different_unit_net" ) ) + { + migratedExclusions.insert( testMarker->Serialize() ); + } + + it = settings.m_ErcExclusions.erase( it ); + } + else + { + ++it; + } + delete testMarker; + } + + settings.m_ErcExclusions.insert( migratedExclusions.begin(), migratedExclusions.end() ); + + // End of legacy exclusion removal / migrations + for( const SCH_SHEET_PATH& sheet : sheetList ) { for( SCH_ITEM* item : sheet.LastScreen()->Items().OfType( SCH_MARKER_T ) ) @@ -213,7 +246,7 @@ std::vector SCHEMATIC::ResolveERCExclusions() for( const wxString& exclusionData : settings.m_ErcExclusions ) { - SCH_MARKER* marker = SCH_MARKER::Deserialize( exclusionData ); + SCH_MARKER* marker = SCH_MARKER::Deserialize( this, exclusionData ); if( marker ) { diff --git a/include/rc_item.h b/include/rc_item.h index 7c85e882e1..4cc53c2e1c 100644 --- a/include/rc_item.h +++ b/include/rc_item.h @@ -271,7 +271,7 @@ public: */ void DeleteItems( bool aCurrentOnly, bool aIncludeExclusions, bool aDeep ); -private: +protected: void rebuildModel( std::shared_ptr aProvider, int aSeverities ); void onSizeView( wxSizeEvent& aEvent ); diff --git a/qa/data/eeschema/issue10926_1.kicad_sch b/qa/data/eeschema/issue10926_1.kicad_sch new file mode 100644 index 0000000000..5e382115a9 --- /dev/null +++ b/qa/data/eeschema/issue10926_1.kicad_sch @@ -0,0 +1,72 @@ +(kicad_sch (version 20221206) (generator eeschema) + + (uuid 30037e9f-fb8a-4b65-9af4-98689a2fae61) + + (paper "A4") + + (lib_symbols + ) + + + (no_connect (at 218.44 62.23) (uuid 083eb137-b86e-4300-a846-2bb596205673)) + (no_connect (at 140.97 62.23) (uuid 4be53f71-3455-49bb-bde8-59378e524b0f)) + + (wire (pts (xy 165.1 62.23) (xy 194.31 62.23)) + (stroke (width 0) (type default)) + (uuid ad8a2a0f-07e5-4bda-adf7-ee4963f2c46d) + ) + + (sheet (at 140.97 59.69) (size 24.13 5.08) (fields_autoplaced) + (stroke (width 0.1524) (type solid)) + (fill (color 0 0 0 0.0000)) + (uuid 48e5c29d-dae7-410f-a801-6c402cc42990) + (property "Sheetname" "SubSheet1" (at 140.97 58.9784 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + ) + (property "Sheetfile" "issue10926_1_subsheet_1.kicad_sch" (at 140.97 65.3546 0) + (effects (font (size 1.27 1.27)) (justify left top)) + ) + (pin "OUT" output (at 165.1 62.23 0) + (effects (font (size 1.27 1.27)) (justify right)) + (uuid 1f5f62db-c203-4919-ba5b-ade4e9ea2a6b) + ) + (pin "IN" input (at 140.97 62.23 180) + (effects (font (size 1.27 1.27)) (justify left)) + (uuid 56de26b5-1deb-4848-83c8-22baf301b552) + ) + (instances + (project "issue10926_1" + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61" (page "3")) + ) + ) + ) + + (sheet (at 194.31 59.69) (size 24.13 5.08) (fields_autoplaced) + (stroke (width 0.1524) (type solid)) + (fill (color 0 0 0 0.0000)) + (uuid 745d7512-5b57-4355-bb9c-332f95478133) + (property "Sheetname" "SubSheet2" (at 194.31 58.9784 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + ) + (property "Sheetfile" "issue10926_1_subsheet_1.kicad_sch" (at 194.31 65.3546 0) + (effects (font (size 1.27 1.27)) (justify left top)) + ) + (pin "OUT" output (at 194.31 62.23 180) + (effects (font (size 1.27 1.27)) (justify left)) + (uuid faa919ea-e1e6-48fa-a852-eca05c09b10a) + ) + (pin "IN" input (at 218.44 62.23 0) + (effects (font (size 1.27 1.27)) (justify right)) + (uuid c3b43390-ad2c-4f18-8b8b-2ecd0ba9205a) + ) + (instances + (project "issue10926_1" + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61" (page "4")) + ) + ) + ) + + (sheet_instances + (path "/" (page "1")) + ) +) diff --git a/qa/data/eeschema/issue10926_1_subsheet_1.kicad_sch b/qa/data/eeschema/issue10926_1_subsheet_1.kicad_sch new file mode 100644 index 0000000000..416ef2d1f8 --- /dev/null +++ b/qa/data/eeschema/issue10926_1_subsheet_1.kicad_sch @@ -0,0 +1,149 @@ +(kicad_sch (version 20221206) (generator eeschema) + + (uuid be526097-dcfd-4f9b-a181-67db2fe0ba1c) + + (paper "A4") + + (lib_symbols + (symbol "Test-Library:TEST" (in_bom yes) (on_board yes) + (property "Reference" "U" (at 0 1.27 0) + (effects (font (size 1.27 1.27))) + ) + (property "Value" "" (at 0 0 0) + (effects (font (size 1.27 1.27))) + ) + (property "Footprint" "" (at 0 0 0) + (effects (font (size 1.27 1.27)) hide) + ) + (property "Datasheet" "" (at 0 0 0) + (effects (font (size 1.27 1.27)) hide) + ) + (symbol "TEST_0_1" + (rectangle (start -3.81 0) (end 5.08 -5.08) + (stroke (width 0) (type default)) + (fill (type background)) + ) + ) + (symbol "TEST_1_1" + (pin input line (at -6.35 -2.54 0) (length 2.54) + (name "IN" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27)))) + ) + (pin output line (at 7.62 -2.54 180) (length 2.54) + (name "OUT" (effects (font (size 1.27 1.27)))) + (number "2" (effects (font (size 1.27 1.27)))) + ) + ) + ) + ) + + + + (no_connect (at 67.31 38.1) (uuid 1f9f1969-c5f8-40e8-8246-b977b20daba4)) + (no_connect (at 167.64 38.1) (uuid 989a1368-c3cd-49db-a907-c5445ec2477d)) + + (wire (pts (xy 143.51 69.85) (xy 146.05 69.85)) + (stroke (width 0) (type default)) + (uuid 01a5e094-9958-4828-a629-41d9f5c6527d) + ) + (wire (pts (xy 113.03 38.1) (xy 121.92 38.1)) + (stroke (width 0) (type default)) + (uuid 133145c3-6dac-4b3a-920d-9ebc2100325f) + ) + (wire (pts (xy 127 69.85) (xy 129.54 69.85)) + (stroke (width 0) (type default)) + (uuid 15845972-e785-4b64-b81a-c5b6b9aa406c) + ) + + (hierarchical_label "OUT" (shape output) (at 146.05 69.85 0) (fields_autoplaced) + (effects (font (size 1.27 1.27)) (justify left)) + (uuid c12ca71e-05e9-4686-b95e-26d43be31c39) + ) + (hierarchical_label "IN" (shape input) (at 127 69.85 180) (fields_autoplaced) + (effects (font (size 1.27 1.27)) (justify right)) + (uuid f0727efc-5d42-4816-b517-a7fda806c9a6) + ) + + (symbol (lib_id "Test-Library:TEST") (at 135.89 67.31 0) (unit 1) + (in_bom yes) (on_board yes) (dnp no) (fields_autoplaced) + (uuid 6f4fe901-859a-4888-8767-d39c882b4837) + (property "Reference" "U?" (at 136.525 66.04 0) + (effects (font (size 1.27 1.27))) + ) + (property "Value" "~" (at 135.89 67.31 0) + (effects (font (size 1.27 1.27))) + ) + (property "Footprint" "" (at 135.89 67.31 0) + (effects (font (size 1.27 1.27)) hide) + ) + (property "Datasheet" "" (at 135.89 67.31 0) + (effects (font (size 1.27 1.27)) hide) + ) + (pin "1" (uuid 481c2304-5900-4cb7-8847-6d645570be60)) + (pin "2" (uuid 59429fd6-6091-44c9-9523-a8adc4ba1bbe)) + (instances + (project "issue10926_1" + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61" + (reference "U?") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/745d7512-5b57-4355-bb9c-332f95478133" + (reference "U3") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/48e5c29d-dae7-410f-a801-6c402cc42990" + (reference "U2") (unit 1) + ) + ) + ) + ) + + (sheet (at 121.92 30.48) (size 45.72 17.78) (fields_autoplaced) + (stroke (width 0.1524) (type solid)) + (fill (color 0 0 0 0.0000)) + (uuid 0203a5e0-a22b-467e-bbd1-13728dd7050e) + (property "Sheetname" "Subsheet_1_1" (at 121.92 29.7684 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + ) + (property "Sheetfile" "issue10926_1_subsheet_1_1.kicad_sch" (at 121.92 48.8446 0) + (effects (font (size 1.27 1.27)) (justify left top)) + ) + (pin "OUT_A" output (at 121.92 38.1 180) + (effects (font (size 1.27 1.27)) (justify left)) + (uuid a136add9-7292-4943-9c47-7d95d0dd20f0) + ) + (pin "IN_A" input (at 167.64 38.1 0) + (effects (font (size 1.27 1.27)) (justify right)) + (uuid 9e034dc1-322e-4e11-ab3e-a70310826b7f) + ) + (instances + (project "issue10926_1" + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/48e5c29d-dae7-410f-a801-6c402cc42990" (page "4")) + ) + ) + ) + + (sheet (at 67.31 30.48) (size 45.72 17.78) (fields_autoplaced) + (stroke (width 0.1524) (type solid)) + (fill (color 0 0 0 0.0000)) + (uuid efd6cf1d-46e4-43a3-833d-ac2e75973904) + (property "Sheetname" "Subsheet_1_2" (at 67.31 29.7684 0) + (effects (font (size 1.27 1.27)) (justify left bottom)) + ) + (property "Sheetfile" "issue10926_1_subsheet_1_1.kicad_sch" (at 67.31 48.8446 0) + (effects (font (size 1.27 1.27)) (justify left top)) + ) + (pin "OUT_A" output (at 113.03 38.1 0) + (effects (font (size 1.27 1.27)) (justify right)) + (uuid 54a0a920-a43e-4508-aa6d-ecfc08fd7b62) + ) + (pin "IN_A" input (at 67.31 38.1 180) + (effects (font (size 1.27 1.27)) (justify left)) + (uuid 1fb4f480-e9eb-4522-9b4f-b793583c5265) + ) + (instances + (project "issue10926_1" + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/48e5c29d-dae7-410f-a801-6c402cc42990" (page "2")) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/745d7512-5b57-4355-bb9c-332f95478133" (page "5")) + ) + ) + ) +) diff --git a/qa/data/eeschema/issue10926_1_subsheet_1_1.kicad_sch b/qa/data/eeschema/issue10926_1_subsheet_1_1.kicad_sch new file mode 100644 index 0000000000..0089999362 --- /dev/null +++ b/qa/data/eeschema/issue10926_1_subsheet_1_1.kicad_sch @@ -0,0 +1,94 @@ +(kicad_sch (version 20221206) (generator eeschema) + + (uuid 45062e6c-d9e3-47ef-9773-cf9ed241f3ea) + + (paper "A4") + + (lib_symbols + (symbol "Test-Library:TEST" (in_bom yes) (on_board yes) + (property "Reference" "U" (at 0 1.27 0) + (effects (font (size 1.27 1.27))) + ) + (property "Value" "" (at 0 0 0) + (effects (font (size 1.27 1.27))) + ) + (property "Footprint" "" (at 0 0 0) + (effects (font (size 1.27 1.27)) hide) + ) + (property "Datasheet" "" (at 0 0 0) + (effects (font (size 1.27 1.27)) hide) + ) + (symbol "TEST_0_1" + (rectangle (start -3.81 0) (end 5.08 -5.08) + (stroke (width 0) (type default)) + (fill (type background)) + ) + ) + (symbol "TEST_1_1" + (pin input line (at -6.35 -2.54 0) (length 2.54) + (name "IN" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27)))) + ) + (pin output line (at 7.62 -2.54 180) (length 2.54) + (name "OUT" (effects (font (size 1.27 1.27)))) + (number "2" (effects (font (size 1.27 1.27)))) + ) + ) + ) + ) + + + + (hierarchical_label "OUT_A" (shape output) (at 105.41 46.99 0) (fields_autoplaced) + (effects (font (size 1.27 1.27)) (justify left)) + (uuid 13b0c889-1ff2-41ce-b523-62a03c25b660) + ) + (hierarchical_label "IN_A" (shape input) (at 91.44 46.99 180) (fields_autoplaced) + (effects (font (size 1.27 1.27)) (justify right)) + (uuid 829b3ae8-cd09-4eec-8fb1-2248807b71c5) + ) + + (symbol (lib_id "Test-Library:TEST") (at 97.79 44.45 0) (unit 1) + (in_bom yes) (on_board yes) (dnp no) (fields_autoplaced) + (uuid ad556e4a-3e61-4db6-b04a-45e4e2ee1dfb) + (property "Reference" "U?" (at 98.425 43.18 0) + (effects (font (size 1.27 1.27))) + ) + (property "Value" "~" (at 97.79 44.45 0) + (effects (font (size 1.27 1.27))) + ) + (property "Footprint" "" (at 97.79 44.45 0) + (effects (font (size 1.27 1.27)) hide) + ) + (property "Datasheet" "" (at 97.79 44.45 0) + (effects (font (size 1.27 1.27)) hide) + ) + (pin "1" (uuid add2ba25-6d53-4e93-8c77-301f68c1eb6d)) + (pin "2" (uuid e707f010-d34a-44d3-8c9b-00dd66344fb6)) + (instances + (project "issue10926_1" + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61" + (reference "U?") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/745d7512-5b57-4355-bb9c-332f95478133" + (reference "U3") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/48e5c29d-dae7-410f-a801-6c402cc42990" + (reference "U2") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/48e5c29d-dae7-410f-a801-6c402cc42990/0203a5e0-a22b-467e-bbd1-13728dd7050e" + (reference "U1") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/745d7512-5b57-4355-bb9c-332f95478133/0203a5e0-a22b-467e-bbd1-13728dd7050e" + (reference "U4") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/48e5c29d-dae7-410f-a801-6c402cc42990/efd6cf1d-46e4-43a3-833d-ac2e75973904" + (reference "U5") (unit 1) + ) + (path "/30037e9f-fb8a-4b65-9af4-98689a2fae61/745d7512-5b57-4355-bb9c-332f95478133/efd6cf1d-46e4-43a3-833d-ac2e75973904" + (reference "U6") (unit 1) + ) + ) + ) + ) +) diff --git a/qa/unittests/eeschema/CMakeLists.txt b/qa/unittests/eeschema/CMakeLists.txt index 97a19c094c..e7678fe369 100644 --- a/qa/unittests/eeschema/CMakeLists.txt +++ b/qa/unittests/eeschema/CMakeLists.txt @@ -71,6 +71,7 @@ set( QA_EESCHEMA_SRCS erc/test_erc_stacking_pins.cpp erc/test_erc_global_labels.cpp erc/test_erc_no_connect.cpp + erc/test_erc_hierarchical_schematics.cpp test_eagle_plugin.cpp test_lib_part.cpp diff --git a/qa/unittests/eeschema/erc/test_erc_hierarchical_schematics.cpp b/qa/unittests/eeschema/erc/test_erc_hierarchical_schematics.cpp new file mode 100644 index 0000000000..d5f144f4ae --- /dev/null +++ b/qa/unittests/eeschema/erc/test_erc_hierarchical_schematics.cpp @@ -0,0 +1,75 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2022 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 at + * http://www.gnu.org/licenses/ + */ + +#include +#include + +#include +#include +#include +#include +#include +#include + + +struct ERC_REGRESSION_TEST_FIXTURE +{ + ERC_REGRESSION_TEST_FIXTURE() : m_settingsManager( true /* headless */ ) {} + + SETTINGS_MANAGER m_settingsManager; + std::unique_ptr m_schematic; +}; + + +BOOST_FIXTURE_TEST_CASE( ERCHierarchicalSchematics, ERC_REGRESSION_TEST_FIXTURE ) +{ + LOCALE_IO dummy; + + // Check not-connected ERC errors + + std::vector> tests = { { "issue10926_1", 3 } }; + + for( const std::pair& test : tests ) + { + KI_TEST::LoadSchematic( m_settingsManager, test.first, m_schematic ); + + ERC_SETTINGS& settings = m_schematic->ErcSettings(); + SHEETLIST_ERC_ITEMS_PROVIDER errors( m_schematic.get() ); + + // Skip the "Modified symbol" warning + settings.m_ERCSeverities[ERCE_LIB_SYMBOL_ISSUES] = RPT_SEVERITY_IGNORE; + + m_schematic->ConnectionGraph()->RunERC(); + + ERC_TESTER tester( m_schematic.get() ); + tester.TestConflictingBusAliases(); + tester.TestMultUnitPinConflicts(); + tester.TestMultiunitFootprints(); + tester.TestNoConnectPins(); + tester.TestPinToPin(); + tester.TestSimilarLabels(); + + errors.SetSeverities( RPT_SEVERITY_ERROR | RPT_SEVERITY_WARNING ); + + BOOST_CHECK_MESSAGE( errors.GetCount() == test.second, + "Expected " << test.second << " errors in " << test.first.ToStdString() + << " but got " << errors.GetCount() ); + } +}