Add a less-intrusive validation system to board setup

For some kinds of issues, we want a way to point them out but
we don't need to keep the user from being able to close the
dialog.  A separate Validate button lets the user check for
issues if desired, but these issues are not "fatal" and can
be ignored if the user wishes.
This commit is contained in:
Jon Evans 2021-02-11 22:19:46 -05:00
parent f54ab830f6
commit ab34d94f36
10 changed files with 75 additions and 34 deletions

View File

@ -357,7 +357,7 @@ static void gridRowToNetclass( EDA_UNITS aUnits, wxGrid* grid, int row, const NE
bool PANEL_SETUP_NETCLASSES::TransferDataFromWindow() bool PANEL_SETUP_NETCLASSES::TransferDataFromWindow()
{ {
if( !validateData() ) if( !Validate() )
return false; return false;
m_netclasses->Clear(); m_netclasses->Clear();
@ -696,7 +696,7 @@ void PANEL_SETUP_NETCLASSES::OnUpdateUI( wxUpdateUIEvent& event )
} }
bool PANEL_SETUP_NETCLASSES::validateData() bool PANEL_SETUP_NETCLASSES::Validate()
{ {
if( !m_netclassGrid->CommitPendingChanges() || !m_membershipGrid->CommitPendingChanges() ) if( !m_netclassGrid->CommitPendingChanges() || !m_membershipGrid->CommitPendingChanges() )
return false; return false;

View File

@ -62,21 +62,21 @@ PAGED_DIALOG::PAGED_DIALOG( wxWindow* aParent, const wxString& aTitle, bool aUse
wxLI_HORIZONTAL ); wxLI_HORIZONTAL );
mainSizer->Add( line, 0, wxEXPAND|wxLEFT|wxTOP|wxRIGHT, 10 ); mainSizer->Add( line, 0, wxEXPAND|wxLEFT|wxTOP|wxRIGHT, 10 );
auto buttonsSizer = new wxBoxSizer( wxHORIZONTAL ); m_buttonsSizer = new wxBoxSizer( wxHORIZONTAL );
if( aUseReset ) if( aUseReset )
{ {
m_resetButton = new wxButton( this, wxID_ANY, _( "Reset to Defaults" ) ); m_resetButton = new wxButton( this, wxID_ANY, _( "Reset to Defaults" ) );
buttonsSizer->Add( m_resetButton, 0, wxRIGHT|wxLEFT, 5 ); m_buttonsSizer->Add( m_resetButton, 0, wxRIGHT|wxLEFT, 5 );
} }
if( !aAuxiliaryAction.IsEmpty() ) if( !aAuxiliaryAction.IsEmpty() )
{ {
m_auxiliaryButton = new wxButton( this, wxID_ANY, aAuxiliaryAction ); m_auxiliaryButton = new wxButton( this, wxID_ANY, aAuxiliaryAction );
buttonsSizer->Add( m_auxiliaryButton, 0, wxRIGHT|wxLEFT, 5 ); m_buttonsSizer->Add( m_auxiliaryButton, 0, wxRIGHT|wxLEFT, 5 );
} }
buttonsSizer->AddStretchSpacer(); m_buttonsSizer->AddStretchSpacer();
auto sdbSizer = new wxStdDialogButtonSizer(); auto sdbSizer = new wxStdDialogButtonSizer();
wxButton* sdbSizerOK = new wxButton( this, wxID_OK ); wxButton* sdbSizerOK = new wxButton( this, wxID_OK );
@ -85,8 +85,8 @@ PAGED_DIALOG::PAGED_DIALOG( wxWindow* aParent, const wxString& aTitle, bool aUse
sdbSizer->AddButton( sdbSizerCancel ); sdbSizer->AddButton( sdbSizerCancel );
sdbSizer->Realize(); sdbSizer->Realize();
buttonsSizer->Add( sdbSizer, 1, 0, 5 ); m_buttonsSizer->Add( sdbSizer, 1, 0, 5 );
mainSizer->Add( buttonsSizer, 0, wxALL|wxEXPAND, 5 ); mainSizer->Add( m_buttonsSizer, 0, wxALL|wxEXPAND, 5 );
sdbSizerOK->SetDefault(); sdbSizerOK->SetDefault();
@ -282,6 +282,35 @@ void PAGED_DIALOG::SetError( const wxString& aMessage, wxWindow* aPage, wxWindow
} }
void PAGED_DIALOG::AddAuxiliaryAction( const wxString& aTitle, const wxString& aTooltip,
std::function<void( wxCommandEvent& )> aHandler )
{
// Insert before standard button sizer and flex spacer
const int idx = m_buttonsSizer->GetItemCount() - 2;
wxButton* button = new wxButton( this, wxID_ANY, aTitle );
button->SetToolTip( aTooltip );
button->Bind( wxEVT_BUTTON, aHandler );
m_buttonsSizer->Insert( idx, button, 0, wxRIGHT | wxLEFT, 5 );
}
void PAGED_DIALOG::OnValidate( wxCommandEvent& aEvent )
{
for( size_t i = 0; i < m_treebook->GetPageCount(); ++i )
{
wxWindow* page = m_treebook->GetPage( i );
// Display first warning
if( !page->Validate() )
break;
}
if( !m_errorMessage.IsEmpty() )
m_infoBar->ShowMessage( m_errorMessage, wxICON_WARNING );
}
void PAGED_DIALOG::OnUpdateUI( wxUpdateUIEvent& event ) void PAGED_DIALOG::OnUpdateUI( wxUpdateUIEvent& event )
{ {
// Handle an error. This is delayed to OnUpdateUI so that we can change the focus // Handle an error. This is delayed to OnUpdateUI so that we can change the focus
@ -295,8 +324,6 @@ void PAGED_DIALOG::OnUpdateUI( wxUpdateUIEvent& event )
wxWindow* ctrl = m_errorCtrl; wxWindow* ctrl = m_errorCtrl;
m_errorCtrl = nullptr; m_errorCtrl = nullptr;
DisplayErrorMessage( ctrl, m_errorMessage );
if( wxTextCtrl* textCtrl = dynamic_cast<wxTextCtrl*>( ctrl ) ) if( wxTextCtrl* textCtrl = dynamic_cast<wxTextCtrl*>( ctrl ) )
{ {
textCtrl->SetSelection( -1, -1 ); textCtrl->SetSelection( -1, -1 );

View File

@ -59,7 +59,6 @@ private:
void OnAssignSelected( wxCommandEvent& event ) override { doAssignments( false ); } void OnAssignSelected( wxCommandEvent& event ) override { doAssignments( false ); }
bool validateNetclassName( int aRow, wxString aName, bool focusFirst = true ); bool validateNetclassName( int aRow, wxString aName, bool focusFirst = true );
bool validateData();
void rebuildNetclassDropdowns(); void rebuildNetclassDropdowns();
@ -78,6 +77,8 @@ public:
bool TransferDataToWindow() override; bool TransferDataToWindow() override;
bool TransferDataFromWindow() override; bool TransferDataFromWindow() override;
bool Validate() override;
void ImportSettingsFrom( NETCLASSES* aBoard ); void ImportSettingsFrom( NETCLASSES* aBoard );
}; };

View File

@ -40,6 +40,8 @@ private:
int m_errorRow; // the row if m_errorCtrl is a grid int m_errorRow; // the row if m_errorCtrl is a grid
int m_errorCol; // the column if m_errorCtrl is a grid int m_errorCol; // the column if m_errorCtrl is a grid
wxBoxSizer* m_buttonsSizer;
std::vector<bool> m_macHack; std::vector<bool> m_macHack;
public: public:
@ -59,6 +61,9 @@ public:
void SetError( const wxString& aMessage, wxWindow* aPage, wxWindow* aCtrl, int aRow = -1, void SetError( const wxString& aMessage, wxWindow* aPage, wxWindow* aCtrl, int aRow = -1,
int aCol = -1 ); int aCol = -1 );
void AddAuxiliaryAction( const wxString& aTitle, const wxString& aTooltip,
std::function<void( wxCommandEvent& )> aHandler );
protected: protected:
void finishInitialization(); void finishInitialization();
@ -71,6 +76,7 @@ protected:
void OnResetButton( wxCommandEvent& aEvent ); void OnResetButton( wxCommandEvent& aEvent );
void OnUpdateUI( wxUpdateUIEvent& event ); void OnUpdateUI( wxUpdateUIEvent& event );
void OnPageChange( wxBookCtrlEvent& event ); void OnPageChange( wxBookCtrlEvent& event );
void OnValidate( wxCommandEvent& aEvent );
wxTreebook* m_treebook; wxTreebook* m_treebook;
wxButton* m_auxiliaryButton; wxButton* m_auxiliaryButton;

View File

@ -25,7 +25,7 @@
class PAGED_DIALOG; class PAGED_DIALOG;
/** /**
* A wxPanel that is designed to be reset in a standard manor. * A wxPanel that is designed to be reset in a standard manner.
*/ */
class RESETTABLE_PANEL : public wxPanel class RESETTABLE_PANEL : public wxPanel
{ {

View File

@ -35,11 +35,14 @@
#include <project/project_file.h> #include <project/project_file.h>
#include <settings/settings_manager.h> #include <settings/settings_manager.h>
#include <widgets/infobar.h> #include <widgets/infobar.h>
#include <widgets/resettable_panel.h>
#include <wildcards_and_files_ext.h> #include <wildcards_and_files_ext.h>
#include "dialog_board_setup.h" #include "dialog_board_setup.h"
#include "panel_setup_rules.h" #include "panel_setup_rules.h"
using std::placeholders::_1;
DIALOG_BOARD_SETUP::DIALOG_BOARD_SETUP( PCB_EDIT_FRAME* aFrame ) : DIALOG_BOARD_SETUP::DIALOG_BOARD_SETUP( PCB_EDIT_FRAME* aFrame ) :
PAGED_DIALOG( aFrame, _( "Board Setup" ), false, PAGED_DIALOG( aFrame, _( "Board Setup" ), false,
_( "Import Settings from Another Board..." ) ), _( "Import Settings from Another Board..." ) ),
@ -89,6 +92,9 @@ DIALOG_BOARD_SETUP::DIALOG_BOARD_SETUP( PCB_EDIT_FRAME* aFrame ) :
for( size_t i = 0; i < m_treebook->GetPageCount(); ++i ) for( size_t i = 0; i < m_treebook->GetPageCount(); ++i )
m_macHack.push_back( true ); m_macHack.push_back( true );
AddAuxiliaryAction( "Validate Settings", "Check for problems with board settings",
std::bind( &DIALOG_BOARD_SETUP::OnValidate, this, _1 ) );
// Connect Events // Connect Events
m_treebook->Connect( wxEVT_TREEBOOK_PAGE_CHANGED, m_treebook->Connect( wxEVT_TREEBOOK_PAGE_CHANGED,
wxBookCtrlEventHandler( DIALOG_BOARD_SETUP::OnPageChange ), NULL, this ); wxBookCtrlEventHandler( DIALOG_BOARD_SETUP::OnPageChange ), NULL, this );

View File

@ -162,7 +162,7 @@ int PANEL_SETUP_TEXT_AND_GRAPHICS::getGridValue( int aRow, int aCol )
} }
bool PANEL_SETUP_TEXT_AND_GRAPHICS::validateData() bool PANEL_SETUP_TEXT_AND_GRAPHICS::Validate()
{ {
if( !m_grid->CommitPendingChanges() ) if( !m_grid->CommitPendingChanges() )
return false; return false;
@ -175,7 +175,7 @@ bool PANEL_SETUP_TEXT_AND_GRAPHICS::validateData()
if( getGridValue( row, COL_TEXT_THICKNESS ) > textSize / 4 ) if( getGridValue( row, COL_TEXT_THICKNESS ) > textSize / 4 )
{ {
wxString msg = _( "Text will not be readable with a thickness greater than\n" wxString msg = _( "Text will not be readable with a thickness greater than "
"1/4 its width or height." ); "1/4 its width or height." );
m_Parent->SetError( msg, this, m_grid, row, COL_TEXT_THICKNESS ); m_Parent->SetError( msg, this, m_grid, row, COL_TEXT_THICKNESS );
return false; return false;
@ -188,9 +188,6 @@ bool PANEL_SETUP_TEXT_AND_GRAPHICS::validateData()
bool PANEL_SETUP_TEXT_AND_GRAPHICS::TransferDataFromWindow() bool PANEL_SETUP_TEXT_AND_GRAPHICS::TransferDataFromWindow()
{ {
if( !validateData() )
return false;
for( int i = 0; i < ROW_COUNT; ++i ) for( int i = 0; i < ROW_COUNT; ++i )
{ {
m_BrdSettings->m_LineThickness[ i ] = getGridValue( i, COL_LINE_THICKNESS ); m_BrdSettings->m_LineThickness[ i ] = getGridValue( i, COL_LINE_THICKNESS );

View File

@ -45,8 +45,6 @@ private:
UNIT_BINDER m_extensionOffset; UNIT_BINDER m_extensionOffset;
private: private:
bool validateData();
int getGridValue( int aRow, int aCol ); int getGridValue( int aRow, int aCol );
public: public:
@ -57,6 +55,8 @@ public:
bool TransferDataFromWindow() override; bool TransferDataFromWindow() override;
void ImportSettingsFrom( BOARD* aBoard ); void ImportSettingsFrom( BOARD* aBoard );
bool Validate() override;
}; };
#endif //PANEL_SETUP_TEXT_AND_GRAPHICS_H #endif //PANEL_SETUP_TEXT_AND_GRAPHICS_H

View File

@ -130,8 +130,12 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::TransferDataToWindow()
bool PANEL_SETUP_TRACKS_AND_VIAS::TransferDataFromWindow() bool PANEL_SETUP_TRACKS_AND_VIAS::TransferDataFromWindow()
{ {
if( !validateData() ) if( !m_trackWidthsGrid->CommitPendingChanges()
|| !m_viaSizesGrid->CommitPendingChanges()
|| !m_diffPairsGrid->CommitPendingChanges() )
{
return false; return false;
}
wxString msg; wxString msg;
std::vector<int> trackWidths; std::vector<int> trackWidths;
@ -212,7 +216,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::TransferDataFromWindow()
} }
bool PANEL_SETUP_TRACKS_AND_VIAS::validateData() bool PANEL_SETUP_TRACKS_AND_VIAS::Validate()
{ {
if( !m_trackWidthsGrid->CommitPendingChanges() if( !m_trackWidthsGrid->CommitPendingChanges()
|| !m_viaSizesGrid->CommitPendingChanges() || !m_viaSizesGrid->CommitPendingChanges()
@ -239,7 +243,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ValueFromString( m_Frame->GetUserUnits(), tvalue ) < minTrackWidth ) if( ValueFromString( m_Frame->GetUserUnits(), tvalue ) < minTrackWidth )
{ {
msg.Printf( _( "Track width less than minimum track width (%s)." ), msg.Printf( _( "Track width less than minimum track width (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minTrackWidth ) ); StringFromValue( m_Frame->GetUserUnits(), minTrackWidth, true ) );
m_Parent->SetError( msg, this, m_trackWidthsGrid, row, TR_WIDTH_COL ); m_Parent->SetError( msg, this, m_trackWidthsGrid, row, TR_WIDTH_COL );
return false; return false;
} }
@ -256,7 +260,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ValueFromString( m_Frame->GetUserUnits(), viaDia ) < minViaDia ) if( ValueFromString( m_Frame->GetUserUnits(), viaDia ) < minViaDia )
{ {
msg.Printf( _( "Via diameter less than minimum via diameter (%s)." ), msg.Printf( _( "Via diameter less than minimum via diameter (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minViaDia ) ); StringFromValue( m_Frame->GetUserUnits(), minViaDia, true ) );
m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_SIZE_COL ); m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_SIZE_COL );
return false; return false;
} }
@ -265,15 +269,15 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( viaDrill.IsEmpty() ) if( viaDrill.IsEmpty() )
{ {
msg = _( "No via drill defined." ); msg = _( "No via hole size defined." );
m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_DRILL_COL ); m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_DRILL_COL );
return false; return false;
} }
if( ValueFromString( m_Frame->GetUserUnits(), viaDrill ) < minThroughHole ) if( ValueFromString( m_Frame->GetUserUnits(), viaDrill ) < minThroughHole )
{ {
msg.Printf( _( "Via drill less than minimum through hole (%s)." ), msg.Printf( _( "Via hole diameter less than minimum through hole diameter (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minThroughHole ) ); StringFromValue( m_Frame->GetUserUnits(), minThroughHole, true ) );
m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_DRILL_COL ); m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_DRILL_COL );
return false; return false;
} }
@ -281,7 +285,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ValueFromString( m_Frame->GetUserUnits(), viaDrill ) if( ValueFromString( m_Frame->GetUserUnits(), viaDrill )
>= ValueFromString( m_Frame->GetUserUnits(), viaDia ) ) >= ValueFromString( m_Frame->GetUserUnits(), viaDia ) )
{ {
msg = _( "Via drill larger than via diameter." ); msg = _( "Via hole diameter larger than via diameter." );
m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_DRILL_COL ); m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_DRILL_COL );
return false; return false;
} }
@ -289,8 +293,8 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ( ValueFromString( m_Frame->GetUserUnits(), viaDia ) if( ( ValueFromString( m_Frame->GetUserUnits(), viaDia )
- ValueFromString( m_Frame->GetUserUnits(), viaDrill ) ) / 2 < minViaAnnulus ) - ValueFromString( m_Frame->GetUserUnits(), viaDrill ) ) / 2 < minViaAnnulus )
{ {
msg.Printf( _( "Diameter and drill leave via annulus less than minimum (%s)." ), msg.Printf( _( "Diameter and hole leave via annulus less than minimum (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minViaAnnulus ) ); StringFromValue( m_Frame->GetUserUnits(), minViaAnnulus, true ) );
m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_SIZE_COL ); m_Parent->SetError( msg, this, m_viaSizesGrid, row, VIA_SIZE_COL );
return false; return false;
} }
@ -307,7 +311,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ValueFromString( m_Frame->GetUserUnits(), tvalue ) < minTrackWidth ) if( ValueFromString( m_Frame->GetUserUnits(), tvalue ) < minTrackWidth )
{ {
msg.Printf( _( "Differential pair track width less than minimum track width (%s)." ), msg.Printf( _( "Differential pair track width less than minimum track width (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minTrackWidth ) ); StringFromValue( m_Frame->GetUserUnits(), minTrackWidth, true ) );
m_Parent->SetError( msg, this, m_diffPairsGrid, row, 0 ); m_Parent->SetError( msg, this, m_diffPairsGrid, row, 0 );
return false; return false;
} }
@ -324,7 +328,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ValueFromString( m_Frame->GetUserUnits(), gap ) < minClearance ) if( ValueFromString( m_Frame->GetUserUnits(), gap ) < minClearance )
{ {
msg.Printf( _( "Differential pair gap less than minimum clearance (%s)." ), msg.Printf( _( "Differential pair gap less than minimum clearance (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minClearance ) ); StringFromValue( m_Frame->GetUserUnits(), minClearance, true ) );
m_Parent->SetError( msg, this, m_diffPairsGrid, row, 1 ); m_Parent->SetError( msg, this, m_diffPairsGrid, row, 1 );
return false; return false;
} }
@ -337,7 +341,7 @@ bool PANEL_SETUP_TRACKS_AND_VIAS::validateData()
if( ValueFromString( m_Frame->GetUserUnits(), viaGap ) < minClearance ) if( ValueFromString( m_Frame->GetUserUnits(), viaGap ) < minClearance )
{ {
msg.Printf( _( "Differential pair via gap less than minimum clearance (%s)." ), msg.Printf( _( "Differential pair via gap less than minimum clearance (%s)." ),
StringFromValue( m_Frame->GetUserUnits(), minClearance ) ); StringFromValue( m_Frame->GetUserUnits(), minClearance, true ) );
m_Parent->SetError( msg, this, m_diffPairsGrid, row, 2 ); m_Parent->SetError( msg, this, m_diffPairsGrid, row, 2 );
return false; return false;
} }

View File

@ -48,8 +48,6 @@ private:
// changed but not yet committed. Fetch them from the constraints panel. // changed but not yet committed. Fetch them from the constraints panel.
PANEL_SETUP_CONSTRAINTS* m_ConstraintsPanel; PANEL_SETUP_CONSTRAINTS* m_ConstraintsPanel;
bool validateData();
protected: protected:
void OnAddTrackWidthsClick( wxCommandEvent& event ) override; void OnAddTrackWidthsClick( wxCommandEvent& event ) override;
void OnRemoveTrackWidthsClick( wxCommandEvent& event ) override; void OnRemoveTrackWidthsClick( wxCommandEvent& event ) override;
@ -70,6 +68,8 @@ public:
bool TransferDataToWindow() override; bool TransferDataToWindow() override;
bool TransferDataFromWindow() override; bool TransferDataFromWindow() override;
bool Validate() override;
void ImportSettingsFrom( BOARD* aBoard ); void ImportSettingsFrom( BOARD* aBoard );
}; };