Pcbnew: arrays skip existing names

The current module cannot be queried for existing pads as we
add them (because we never commit, until we finish the whole array).

Instead, pre-gather the names and check as we add, skipping any existing
names.

Note: this will not prevent arrays becoming "mis-ordered", but there
is not a lot we can do to prevent all possible errors.

Fixes: 1808706
* https://bugs.launchpad.net/kicad/+bug/1808706

The same principle could be used to skip existing ref-des'es on PCBs.
This commit is contained in:
John Beard 2019-01-25 16:00:29 +00:00 committed by Seth Hillbrand
parent 5504981d00
commit 453a91f661
6 changed files with 272 additions and 6 deletions

View File

@ -232,6 +232,7 @@ set( PCBNEW_CLASS_SRCS
action_plugin.cpp
append_board_to_current.cpp
array_creator.cpp
array_pad_name_provider.cpp
attribut.cpp
block.cpp
block_footprint_editor.cpp

View File

@ -28,12 +28,12 @@
#include "array_creator.h"
#include <array_pad_name_provider.h>
#include <board_commit.h>
#include <pad_naming.h>
#include <dialogs/dialog_create_array.h>
/**
* Transform a #BOARD_ITEM from the given #ARRAY_OPTIONS and an index into the array.
*
@ -74,6 +74,8 @@ void ARRAY_CREATOR::Invoke()
BOARD_COMMIT commit( &m_parent );
ARRAY_PAD_NAME_PROVIDER pad_name_provider( module, *array_opts );
for ( int i = 0; i < numItems; ++i )
{
BOARD_ITEM* item = getNthItemToArray( i );
@ -91,9 +93,9 @@ void ARRAY_CREATOR::Invoke()
if( isModuleEditor )
{
// increment pad numbers if do any renumbering
// (we will number again later according to the numbering scheme if set)
new_item = module->Duplicate( item, array_opts->ShouldNumberItems() );
// Don't bother incrementing pads: the module won't update
// until commit, so we can only do this once
new_item = module->Duplicate( item, false );
}
else
{
@ -119,7 +121,7 @@ void ARRAY_CREATOR::Invoke()
// attempt to renumber items if the array parameters define
// a complete numbering scheme to number by (as opposed to
// implicit numbering by incrementing the items during creation
if( new_item && array_opts->GetNumberingStartIsSpecified() )
if( new_item && array_opts->ShouldNumberItems() )
{
// Renumber non-aperture pads.
if( new_item->Type() == PCB_PAD_T )
@ -127,7 +129,10 @@ void ARRAY_CREATOR::Invoke()
D_PAD* pad = static_cast<D_PAD*>( new_item );
if( PAD_NAMING::PadCanHaveName( *pad ) )
pad->SetName( array_opts->GetItemNumber( ptN ) );
{
wxString newName = pad_name_provider.GetNextPadName();
pad->SetName( newName );
}
}
}
}

View File

@ -0,0 +1,74 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2019 KiCad Developers, see CHANGELOG.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
*/
#include <array_pad_name_provider.h>
#include <class_pad.h>
ARRAY_PAD_NAME_PROVIDER::ARRAY_PAD_NAME_PROVIDER(
const MODULE* aMod, const ARRAY_OPTIONS& aArrayOpts )
: m_arrayOpts( aArrayOpts )
{
// start by numbering the first new item
m_current_pad_index = 1;
// construct the set of existing pad numbers
if( aArrayOpts.GetNumberingStartIsSpecified() )
{
// if we start from a specified point, we don't look at existing
// names, so it's just an empty "reserved" set
}
else
{
// no module, no reserved names either
if( aMod )
{
// reserve the name of each existing pad
for( D_PAD* pad = aMod->PadsList(); pad; pad = pad->Next() )
{
m_existing_pad_names.insert( pad->GetName() );
}
}
}
}
wxString ARRAY_PAD_NAME_PROVIDER::GetNextPadName()
{
return getNextName( m_current_pad_index, m_existing_pad_names );
}
wxString ARRAY_PAD_NAME_PROVIDER::getNextName( int& aIndex, const std::set<wxString>& aExisting )
{
wxString next_name;
do
{
next_name = m_arrayOpts.GetItemNumber( aIndex );
aIndex++;
} while( aExisting.count( next_name ) != 0 );
return next_name;
}

View File

@ -0,0 +1,64 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2019 KiCad Developers, see CHANGELOG.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
*/
#ifndef PCBNEW_ARRAY_PAD_NAME_PROVIDER__H
#define PCBNEW_ARRAY_PAD_NAME_PROVIDER__H
#include <array_options.h>
#include <class_module.h>
/**
* Simple class that sequentially provides names from an #ARRAY_OPTIONS
* object, making sure that they do not conflict with names already existing
* in a #MODULE.
*/
class ARRAY_PAD_NAME_PROVIDER
{
public:
/**
* @param aMod the module to gather existing names from (nullptr for no module)
* @oaram aArrayOpts the array options that provide the candidate names
*/
ARRAY_PAD_NAME_PROVIDER( const MODULE* aMod, const ARRAY_OPTIONS& aArrayOpts );
/**
* Get the next available pad name.
*/
wxString GetNextPadName();
private:
/**
* Get the next name from a given index/list combo
* @param aIndex index to start at, will be updated
* @param aExisting the set of existing names to skip
* @return the first name found that's not in aExisting
*/
wxString getNextName( int& aIndex, const std::set<wxString>& aExisting );
const ARRAY_OPTIONS& m_arrayOpts;
std::set<wxString> m_existing_pad_names;
int m_current_pad_index;
};
#endif // PCBNEW_ARRAY_PAD_NAME_PROVIDER__H

View File

@ -42,6 +42,7 @@ add_executable( qa_pcbnew
drc/drc_test_utils.cpp
# test compilation units (start test_)
test_array_pad_name_provider.cpp
test_graphics_import_mgr.cpp
test_pad_naming.cpp

View File

@ -0,0 +1,121 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2018 KiCad Developers, see CHANGELOG.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 test_array_pad_name_provider.cpp
* Test suite for the #ARRAY_PAD_NAME_PROVIDER class
*/
#include <unit_test_utils/unit_test_utils.h>
#include <array_pad_name_provider.h> // UUT
#include <common.h> // make_unique
#include <class_module.h>
#include <class_pad.h>
/**
* Make a module with a given list of named pads
*/
static std::unique_ptr<MODULE> ModuleWithPads( const std::vector<wxString> aNames )
{
auto module = std::make_unique<MODULE>( nullptr );
for( const auto& name : aNames )
{
auto pad = std::make_unique<D_PAD>( module.get() );
pad->SetName( name );
module->PadsList().PushBack( pad.release() );
}
return module;
}
/**
* Declare the test suite
*/
BOOST_AUTO_TEST_SUITE( ArrayPadNameProv )
struct APNP_CASE
{
std::string m_case_name;
std::vector<wxString> m_existing_pads;
std::unique_ptr<ARRAY_OPTIONS> m_arr_opts;
std::vector<wxString> m_exp_arr_names;
};
std::vector<APNP_CASE> GetAPNPCases()
{
std::vector<APNP_CASE> cases;
auto opts = std::make_unique<ARRAY_GRID_OPTIONS>();
// simple linear numbering
opts->m_2dArrayNumbering = false;
opts->m_numberingOffsetX = 0;
opts->m_priAxisNumType = ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC;
cases.push_back( {
"Simple linear, skip some",
{ "1", "3" },
std::move( opts ),
{ "2", "4", "5", "6", "7" },
} );
opts = std::make_unique<ARRAY_GRID_OPTIONS>();
// Grid numberings with skips don't make a lot of sense, there is
// no particular contract made for them
return cases;
}
BOOST_AUTO_TEST_CASE( Cases )
{
for( const auto& c : GetAPNPCases() )
{
BOOST_TEST_CONTEXT( c.m_case_name )
{
auto module = ModuleWithPads( c.m_existing_pads );
ARRAY_PAD_NAME_PROVIDER apnp( module.get(), *c.m_arr_opts );
std::vector<wxString> got_names;
for( unsigned i = 0; i < c.m_exp_arr_names.size(); ++i )
{
got_names.push_back( apnp.GetNextPadName() );
}
BOOST_CHECK_EQUAL_COLLECTIONS( c.m_exp_arr_names.begin(), c.m_exp_arr_names.end(),
got_names.begin(), got_names.end() );
}
}
}
BOOST_AUTO_TEST_SUITE_END()