Pcbnew: ignore start values in "first available" grid numbering

The using the "first available" numbering option, force an
artifical linear numbering scheme starting at '1'.

Start the pad name provider at the 0'th pad index.

Also adds a few related tests and adjusts tests affected by
this change to reflect that the offset still applies.

This can be expanded in future to be more general by skipping
pad (which would allow a custom start and numbering scheme
while still avoiding duplication), but for now,
this does what the UI says and avoids string changes in 5.1rc.

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

Fixes: lp:1814917
* https://bugs.launchpad.net/kicad/+bug/1814917
This commit is contained in:
John Beard 2019-02-07 12:33:29 +00:00
parent 95af750fc9
commit edc73de023
4 changed files with 144 additions and 43 deletions

View File

@ -31,7 +31,7 @@ ARRAY_PAD_NAME_PROVIDER::ARRAY_PAD_NAME_PROVIDER(
: m_arrayOpts( aArrayOpts )
{
// start by numbering the first new item
m_current_pad_index = 1;
m_current_pad_index = 0;
// construct the set of existing pad numbers
if( aArrayOpts.GetNumberingStartIsSpecified() )

View File

@ -1,8 +1,7 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2014 John Beard, john.j.beard@gmail.com
* Copyright (C) 1992-2014 KiCad Developers, see AUTHORS.txt for contributors.
* Copyright (C) 1992-2019 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
@ -303,25 +302,35 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow()
newGrid->SetShouldNumber( m_numberingEnabled );
if ( m_numberingEnabled )
{
newGrid->SetNumberingStartIsSpecified( m_rbGridStartNumberingOpt->GetSelection() == 1 );
if( newGrid->GetNumberingStartIsSpecified() )
{
newGrid->m_2dArrayNumbering = m_radioBoxGridNumberingScheme->GetSelection() != 0;
bool numOk = validateNumberingTypeAndOffset(
*m_entryGridPriNumberingOffset, *m_choicePriAxisNumbering,
newGrid->m_priAxisNumType, newGrid->m_numberingOffsetX,
errors );
// validate from the input fields
bool numOk = validateNumberingTypeAndOffset( *m_entryGridPriNumberingOffset,
*m_choicePriAxisNumbering, newGrid->m_priAxisNumType,
newGrid->m_numberingOffsetX, errors );
if( newGrid->m_2dArrayNumbering )
{
numOk = validateNumberingTypeAndOffset(
*m_entryGridSecNumberingOffset, *m_choiceSecAxisNumbering,
newGrid->m_secAxisNumType, newGrid->m_numberingOffsetY,
errors ) && numOk;
numOk = validateNumberingTypeAndOffset( *m_entryGridSecNumberingOffset,
*m_choiceSecAxisNumbering, newGrid->m_secAxisNumType,
newGrid->m_numberingOffsetY, errors )
&& numOk;
}
ok = ok && numOk;
newGrid->SetNumberingStartIsSpecified( m_rbGridStartNumberingOpt->GetSelection() == 1 );
}
else
{
// artificial linear numeric scheme from 1
newGrid->m_2dArrayNumbering = false;
newGrid->m_priAxisNumType = ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC;
newGrid->m_numberingOffsetX = 1; // Start at "1"
}
}
// Only use settings if all values are good
@ -347,10 +356,20 @@ bool DIALOG_CREATE_ARRAY::TransferDataFromWindow()
if ( m_numberingEnabled )
{
newCirc->SetNumberingStartIsSpecified( m_rbCircStartNumberingOpt->GetSelection() == 1 );
if( newCirc->GetNumberingStartIsSpecified() )
{
newCirc->m_numberingType = ARRAY_OPTIONS::NUMBERING_NUMERIC;
ok = ok && validateLongEntry(*m_entryCircNumberingStart, newCirc->m_numberingOffset,
_("numbering start"), errors);
ok = ok
&& validateLongEntry( *m_entryCircNumberingStart, newCirc->m_numberingOffset,
_( "numbering start" ), errors );
}
else
{
// artificial linear numeric scheme from 1
newCirc->m_numberingOffset = 1; // Start at "1"
}
}
// Only use settings if all values are good
@ -390,18 +409,20 @@ void DIALOG_CREATE_ARRAY::setControlEnablement()
{
if ( m_numberingEnabled )
{
// If we set the start number, we can set the other options,
// otherwise it's a hardcoded linear array
const bool use_set_start_grid = m_rbGridStartNumberingOpt->GetSelection() == 1;
m_radioBoxGridNumberingScheme->Enable( true );
m_labelPriAxisNumbering->Enable( true );
m_choicePriAxisNumbering->Enable( true );
m_radioBoxGridNumberingScheme->Enable( use_set_start_grid );
m_labelPriAxisNumbering->Enable( use_set_start_grid );
m_choicePriAxisNumbering->Enable( use_set_start_grid );
// Disable the secondary axis numbering option if the
// numbering scheme doesn't have two axes
const bool num2d = m_radioBoxGridNumberingScheme->GetSelection() != 0;
m_labelSecAxisNumbering->Enable( true && num2d );
m_choiceSecAxisNumbering->Enable( true && num2d );
m_labelSecAxisNumbering->Enable( use_set_start_grid && num2d );
m_choiceSecAxisNumbering->Enable( use_set_start_grid && num2d );
// We can only set an offset if we're setting the start number
m_labelGridNumberingOffset->Enable( use_set_start_grid );
@ -417,8 +438,14 @@ void DIALOG_CREATE_ARRAY::setControlEnablement()
// grid
m_rbGridStartNumberingOpt->Enable( false );
m_radioBoxGridNumberingScheme->Enable( false );
m_labelPriAxisNumbering->Enable( false );
m_labelSecAxisNumbering->Enable( false );
m_choiceSecAxisNumbering->Enable( false );
m_choicePriAxisNumbering->Enable( false );
m_labelGridNumberingOffset->Enable( false );
m_entryGridPriNumberingOffset->Enable( false );
m_entryGridSecNumberingOffset->Enable( false );

View File

@ -397,6 +397,7 @@ struct GRID_ARRAY_NAMING_PARAMS
std::string m_start_at_x;
std::string m_start_at_y;
bool m_2d_numbering;
bool m_h_then_v;
int m_nx;
int m_ny;
};
@ -420,20 +421,54 @@ static const std::vector<GRID_ARRAY_NAMING_CASE> grid_name_cases = {
"1",
"2",
false,
false, // doesn't matter
2,
3,
},
{ "1", "2", "3", "4", "5", "6" },
},
{
// Tests a 2d grid
"2D grid A1",
{
ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_ALPHA_FULL,
ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC,
"A",
"1",
true,
true,
2,
3,
},
{ "A1", "B1", "A2", "B2", "A3", "B3" },
},
{
// Tests a 2d grid
"2D grid 11",
{
ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC,
ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC,
"1",
"1",
true,
false,
2,
3,
},
// moving down the "long axis" first
// so the first coordinate has a range of 1-3, the second 1-2
{ "11", "21", "31", "12", "22", "32" },
},
{
// Tests a 2d grid, with different types and offsets (and alphabet wrap)
"2D grid",
"2D grid offsets",
{
ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC,
ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_ALPHA_FULL,
"5",
"Z",
true,
true,
2,
3,
},
@ -457,6 +492,8 @@ BOOST_AUTO_TEST_CASE( GridNaming )
grid_opts.m_nx = c.m_prms.m_nx;
grid_opts.m_ny = c.m_prms.m_ny;
grid_opts.m_horizontalThenVertical = c.m_prms.m_h_then_v;
ARRAY_OPTIONS::GetNumberingOffset(
c.m_prms.m_start_at_x, c.m_prms.m_pri_type, grid_opts.m_numberingOffsetX );
ARRAY_OPTIONS::GetNumberingOffset(

View File

@ -63,13 +63,18 @@ BOOST_AUTO_TEST_SUITE( ArrayPadNameProv )
struct APNP_CASE
{
std::string m_case_name;
bool m_using_module;
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()
/**
* Get Array Pad Name Provider cases when a module is looked
* at to determine what names are available.
*/
std::vector<APNP_CASE> GetModuleAPNPCases()
{
std::vector<APNP_CASE> cases;
@ -77,43 +82,75 @@ std::vector<APNP_CASE> GetAPNPCases()
// simple linear numbering
opts->m_2dArrayNumbering = false;
opts->m_numberingOffsetX = 0;
opts->m_numberingOffsetX = 1;
opts->m_priAxisNumType = ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC;
cases.push_back( {
"Simple linear, skip some",
true,
{ "1", "3" },
std::move( opts ),
{ "2", "4", "5", "6", "7" },
} );
// one without a module
opts = std::make_unique<ARRAY_GRID_OPTIONS>();
// simple linear numbering (again)
opts->m_2dArrayNumbering = false;
opts->m_numberingOffsetX = 1;
opts->m_priAxisNumType = ARRAY_OPTIONS::NUMBERING_TYPE_T::NUMBERING_NUMERIC;
cases.push_back( {
"Simple linear, no module",
false,
{}, // not used
std::move( opts ),
{ "1", "2", "3", "4", "5" },
} );
// 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 )
/**
* Check that an #ARRAY_PAD_NAME_PROVIDER provides the right names
* @param aProvider the provider
* @param aExpNames ordered list of expected names
*/
void CheckPadNameProvider( ARRAY_PAD_NAME_PROVIDER& aProvider, std::vector<wxString> aExpNames )
{
for( const auto& c : GetAPNPCases() )
std::vector<wxString> got_names;
for( unsigned i = 0; i < aExpNames.size(); ++i )
{
got_names.push_back( aProvider.GetNextPadName() );
}
BOOST_CHECK_EQUAL_COLLECTIONS(
aExpNames.begin(), aExpNames.end(), got_names.begin(), got_names.end() );
}
BOOST_AUTO_TEST_CASE( ModuleCases )
{
for( const auto& c : GetModuleAPNPCases() )
{
BOOST_TEST_CONTEXT( c.m_case_name )
{
auto module = ModuleWithPads( c.m_existing_pads );
std::unique_ptr<MODULE> module;
if( c.m_using_module )
{
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() );
CheckPadNameProvider( apnp, c.m_exp_arr_names );
}
}
}