Re: [Kicad-developers] [RFC] Refactor pin shapes
Hi, On 04.04.2015 17:23, Lorenzo Marcantonio wrote: On Sat, Apr 04, 2015 at 01:36:31AM +0200, Simon Richter wrote: 2. Keep strings/bitmaps outside of LIB_PIN These are UI elements. Also, AFAIK the bitmaps are only used in the pin dialog... So far. I plan to add the same widget into another dialog, so it makes sense to move the responsibility for initialization out of the dialog. I also need a simpler representation in the table, before entering edit mode. 3. Keep load/save outside of LIB_PIN These belong to the load/save routines, which may change when switching to s-expressions. *Could* be contested, depending on the OO school you follow: - You can say that serializing is responsibily of the pin (i.e. virtual methods) Yes, that is the current approach. As far as I've understood, there is a certain desire to switch the format, which means we will need two parsers at least, and ideally also two formatters so we can still save to the old format for some time. I think if I were to submit these changes, I'd inline the parsing/formatting bits back into lib_pin.cpp for the time being, and do the split separately. - You can say that the load/save is/has/uses (we could discuss days on this detail alone...) a pin factory (for load) and a pin user (for save) Well, I'm thinking about Model-View-Controller, because that would allow easy compartmentalization (legacy load/save, S-exp load/save, legacy rendering, GAL rendering, non-modal dialogs). 4. Introduce dedicated ComboBox for pin shapes This makes the pin shape selection widget reusable across different dialogs, and removes the responsibility of the user to initialize the available selections. Is there more than one dialog using these? Anyway it could be said that's a quite easy to do refactoring of the code. Soon. Also: pin type is more or less useless (like the related DRC), so I wouldn't worry too much about it. Right, I just had to start somewhere. Simon signature.asc Description: OpenPGP digital signature ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [RFC] Refactor pin shapes
On 4/4/2015 11:41 AM, Simon Richter wrote: Hi, On 04.04.2015 17:23, Lorenzo Marcantonio wrote: On Sat, Apr 04, 2015 at 01:36:31AM +0200, Simon Richter wrote: 2. Keep strings/bitmaps outside of LIB_PIN These are UI elements. Also, AFAIK the bitmaps are only used in the pin dialog... So far. I plan to add the same widget into another dialog, so it makes sense to move the responsibility for initialization out of the dialog. I also need a simpler representation in the table, before entering edit mode. 3. Keep load/save outside of LIB_PIN These belong to the load/save routines, which may change when switching to s-expressions. *Could* be contested, depending on the OO school you follow: - You can say that serializing is responsibily of the pin (i.e. virtual methods) Yes, that is the current approach. As far as I've understood, there is a certain desire to switch the format, which means we will need two parsers at least, and ideally also two formatters so we can still save to the old format for some time. I think if I were to submit these changes, I'd inline the parsing/formatting bits back into lib_pin.cpp for the time being, and do the split separately. Please do not make any changes to the schematic file parsing or formatting code at this time. I will be rewriting the entire schematic I/O handling code after the stable release and after I fix the underlying schematic data model. Expect the the schematic I/O handling to look very similar to the pcb I/O handling. - You can say that the load/save is/has/uses (we could discuss days on this detail alone...) a pin factory (for load) and a pin user (for save) Well, I'm thinking about Model-View-Controller, because that would allow easy compartmentalization (legacy load/save, S-exp load/save, legacy rendering, GAL rendering, non-modal dialogs). 4. Introduce dedicated ComboBox for pin shapes This makes the pin shape selection widget reusable across different dialogs, and removes the responsibility of the user to initialize the available selections. Is there more than one dialog using these? Anyway it could be said that's a quite easy to do refactoring of the code. Soon. Also: pin type is more or less useless (like the related DRC), so I wouldn't worry too much about it. Right, I just had to start somewhere. Simon ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
[Kicad-developers] [RFC] Refactor pin shapes
Hi,
this is not yet ready for inclusion, and should probably go into the
post-release branch anyway. I'd like to get some feedback on whether the
general direction is the right one.
I'd like to do a bit of refactoring over the next time to allow for better
reusability of code -- in this case, I'm planning to reuse the ComboBox
with the pin shapes in the pin table in the long run, so I thought I'd
factor this out into a separate widget class, and simplify some of the
other code while I'm at it.
The load/save code should probably at some point be moved to a legacy
parser when eeschema is switched to S-Expressions -- ideally I'd like to
see a clear separation between data model, load/save code and UI code.
The patch does the following:
1. Replace bitmask with enumeration of all valid values
Only a few of the values possible were actually sensibly supported anyway,
so this can be simplified by replacing the various integer representations
with a single enumeration.
2. Keep strings/bitmaps outside of LIB_PIN
These are UI elements.
3. Keep load/save outside of LIB_PIN
These belong to the load/save routines, which may change when switching to
s-expressions.
4. Introduce dedicated ComboBox for pin shapes
This makes the pin shape selection widget reusable across different
dialogs, and removes the responsibility of the user to initialize the
available selections.
5. Switch pin edit dialog to dedicated ComboBox
---
eeschema/CMakeLists.txt | 4 +
eeschema/dialogs/dialog_lib_edit_pin.cpp | 14 +-
eeschema/dialogs/dialog_lib_edit_pin.h| 6 +-
eeschema/dialogs/dialog_lib_edit_pin_base.cpp | 3 +-
eeschema/dialogs/dialog_lib_edit_pin_base.fbp | 2 +-
eeschema/dialogs/dialog_lib_edit_pin_base.h | 3 +-
eeschema/lib_pin.cpp | 192 +
eeschema/lib_pin.h| 55 +---
eeschema/pin_shape.cpp| 194 ++
eeschema/pin_shape.h | 33 +
eeschema/pinedit.cpp | 21 ++-
eeschema/widgets/pin_shape_combobox.cpp | 69 +
eeschema/widgets/pin_shape_combobox.h | 51 +++
13 files changed, 407 insertions(+), 240 deletions(-)
create mode 100644 eeschema/pin_shape.cpp
create mode 100644 eeschema/pin_shape.h
create mode 100644 eeschema/widgets/pin_shape_combobox.cpp
create mode 100644 eeschema/widgets/pin_shape_combobox.h
diff --git a/eeschema/CMakeLists.txt b/eeschema/CMakeLists.txt
index 77f..026ea06 100644
--- a/eeschema/CMakeLists.txt
+++ b/eeschema/CMakeLists.txt
@@ -13,6 +13,7 @@ include_directories( BEFORE ${INC_BEFORE} )
include_directories(
./dialogs
./netlist_exporters
+./widgets
../common
../common/dialogs
${INC_AFTER}
@@ -130,6 +131,7 @@ set( EESCHEMA_SRCS
onrightclick.cpp
operations_on_items_lists.cpp
pinedit.cpp
+pin_shape.cpp
plot_schematic_DXF.cpp
plot_schematic_HPGL.cpp
plot_schematic_PS.cpp
@@ -174,6 +176,8 @@ set( EESCHEMA_SRCS
netlist_exporters/netlist_exporter_kicad.cpp
netlist_exporters/netlist_exporter_orcadpcb2.cpp
netlist_exporters/netlist_exporter_pspice.cpp
+
+widgets/pin_shape_combobox.cpp
)
diff --git a/eeschema/dialogs/dialog_lib_edit_pin.cpp
b/eeschema/dialogs/dialog_lib_edit_pin.cpp
index 15954dd..6c205d1 100644
--- a/eeschema/dialogs/dialog_lib_edit_pin.cpp
+++ b/eeschema/dialogs/dialog_lib_edit_pin.cpp
@@ -129,7 +129,7 @@ void DIALOG_LIB_EDIT_PIN::OnPropertiesChange(
wxCommandEvent event )
int pinNumSize = ValueFromString( g_UserUnit, GetPadNameTextSize());
int pinOrient = LIB_PIN::GetOrientationCode( GetOrientation() );
int pinLength = ValueFromString( g_UserUnit, GetLength() );
-int pinShape = LIB_PIN::GetStyleCode( GetStyle() );
+PinShape pinShape = GetStyle();
int pinType = GetElectricalType();
m_dummyPin-SetName( GetPinName() );
@@ -170,15 +170,3 @@ void DIALOG_LIB_EDIT_PIN::SetElectricalTypeList( const
wxArrayString list,
m_choiceElectricalType-Insert( list[ii], KiBitmap( aBitmaps[ii]
), ii );
}
}
-
-
-void DIALOG_LIB_EDIT_PIN::SetStyleList( const wxArrayString list, const
BITMAP_DEF* aBitmaps )
-{
-for ( unsigned ii = 0; ii list.GetCount(); ii++ )
-{
-if( aBitmaps == NULL )
-m_choiceStyle-Append( list[ii] );
-else
-m_choiceStyle-Insert( list[ii], KiBitmap( aBitmaps[ii] ), ii );
-}
-}
diff --git a/eeschema/dialogs/dialog_lib_edit_pin.h
b/eeschema/dialogs/dialog_lib_edit_pin.h
index 948a32a..057e840 100644
--- a/eeschema/dialogs/dialog_lib_edit_pin.h
+++ b/eeschema/dialogs/dialog_lib_edit_pin.h
@@ -31,6 +31,7 @@
*/
#include wx/bmpcbox.h
+#include pin_shape_combobox.h
#include dialog_lib_edit_pin_base.h
@@ -67,9 +68,8 @@ public:
return m_choiceElectricalType-GetSelection();
}
-void SetStyleList( const
