← Back to team overview

kicad-developers team mailing list archive

[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 700007f..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 wxArrayString& list, const BITMAP_DEF* aBitmaps );
-    void SetStyle( int style ) { m_choiceStyle->SetSelection( style ); }
-    int GetStyle( void ) { return m_choiceStyle->GetSelection(); }
+    void SetStyle( PinShape style ) { m_choiceStyle->SetSelection( style ); }
+    PinShape GetStyle( void ) { return m_choiceStyle->GetSelection(); }
 
     void SetPinName( const wxString& name ) { m_textPinName->SetValue( name ); }
     wxString GetPinName( void ) { return m_textPinName->GetValue(); }
diff --git a/eeschema/dialogs/dialog_lib_edit_pin_base.cpp b/eeschema/dialogs/dialog_lib_edit_pin_base.cpp
index 42d508a..5e7b345 100644
--- a/eeschema/dialogs/dialog_lib_edit_pin_base.cpp
+++ b/eeschema/dialogs/dialog_lib_edit_pin_base.cpp
@@ -5,6 +5,7 @@
 // PLEASE DO "NOT" EDIT THIS FILE!
 ///////////////////////////////////////////////////////////////////////////
 
+#include "pin_shape_combobox.h"
 #include "wx/bmpcbox.h"
 
 #include "dialog_lib_edit_pin_base.h"
@@ -68,7 +69,7 @@ DIALOG_LIB_EDIT_PIN_BASE::DIALOG_LIB_EDIT_PIN_BASE( wxWindow* parent, wxWindowID
 	m_staticTextGstyle->Wrap( -1 );
 	fgSizerPins->Add( m_staticTextGstyle, 0, wxALIGN_CENTER_VERTICAL|wxALL, 3 );
 	
-	m_choiceStyle = new wxBitmapComboBox( this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0, NULL, wxCB_READONLY ); 
+	m_choiceStyle = new PinShapeComboBox( this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0, NULL, wxCB_READONLY ); 
 	fgSizerPins->Add( m_choiceStyle, 0, wxEXPAND|wxTOP|wxBOTTOM, 5 );
 	
 	
diff --git a/eeschema/dialogs/dialog_lib_edit_pin_base.fbp b/eeschema/dialogs/dialog_lib_edit_pin_base.fbp
index f18240f..f78abb6 100644
--- a/eeschema/dialogs/dialog_lib_edit_pin_base.fbp
+++ b/eeschema/dialogs/dialog_lib_edit_pin_base.fbp
@@ -958,7 +958,7 @@
                                                 <property name="show">1</property>
                                                 <property name="size"></property>
                                                 <property name="style">wxCB_READONLY</property>
-                                                <property name="subclass">wxBitmapComboBox; wx/bmpcbox.h</property>
+                                                <property name="subclass">PinShapeComboBox; pin_shape_combobox.h</property>
                                                 <property name="toolbar_pane">0</property>
                                                 <property name="tooltip"></property>
                                                 <property name="validator_data_type"></property>
diff --git a/eeschema/dialogs/dialog_lib_edit_pin_base.h b/eeschema/dialogs/dialog_lib_edit_pin_base.h
index bac7325..db9a949 100644
--- a/eeschema/dialogs/dialog_lib_edit_pin_base.h
+++ b/eeschema/dialogs/dialog_lib_edit_pin_base.h
@@ -12,6 +12,7 @@
 #include <wx/xrc/xmlres.h>
 #include <wx/intl.h>
 class DIALOG_SHIM;
+class PinShapeComboBox;
 class wxBitmapComboBox;
 
 #include "dialog_shim.h"
@@ -66,7 +67,7 @@ class DIALOG_LIB_EDIT_PIN_BASE : public DIALOG_SHIM
 		wxStaticText* m_staticTextEType;
 		wxBitmapComboBox* m_choiceElectricalType;
 		wxStaticText* m_staticTextGstyle;
-		wxBitmapComboBox* m_choiceStyle;
+		PinShapeComboBox* m_choiceStyle;
 		wxCheckBox* m_checkApplyToAllParts;
 		wxCheckBox* m_checkApplyToAllConversions;
 		wxCheckBox* m_checkShow;
diff --git a/eeschema/lib_pin.cpp b/eeschema/lib_pin.cpp
index 501eb4e..125791f 100644
--- a/eeschema/lib_pin.cpp
+++ b/eeschema/lib_pin.cpp
@@ -47,7 +47,6 @@
 #include <transform.h>
 #include <sch_component.h>
 
-
 static const int pin_orientation_codes[] =
 {
     PIN_RIGHT,
@@ -70,38 +69,6 @@ static const BITMAP_DEF iconsPinsOrientations[] =
 };
 
 
-// bitmaps to show pins shapes in dialog editor
-// must have same order than pin_style_names
-static BITMAP_DEF iconsPinsShapes[] =
-{
-    pinshape_normal_xpm,
-    pinshape_invert_xpm,
-    pinshape_clock_normal_xpm,
-    pinshape_clock_invert_xpm,
-    pinshape_active_low_input_xpm,
-    pinshape_clock_active_low_xpm,
-    pinshape_active_low_output_xpm,
-    pinshape_clock_fall_xpm,
-    pinshape_nonlogic_xpm
-};
-
-
-
-static const int pin_style_codes[] =
-{
-    NONE,
-    INVERT,
-    CLOCK,
-    CLOCK | INVERT,
-    LOWLEVEL_IN,
-    LOWLEVEL_IN | CLOCK,
-    LOWLEVEL_OUT,
-    CLOCK_FALL,
-    NONLOGIC
-};
-
-#define PIN_STYLE_CNT DIM( pin_style_codes )
-
 // bitmaps to show pins electrical type in dialog editor
 // must have same order than enum ElectricPinType (see lib_pin.h)
 static const BITMAP_DEF iconsPinsElectricalType[] =
@@ -200,29 +167,6 @@ const wxString LIB_PIN::GetElectricalTypeName( int aPinsElectricalType )
     return pin_electrical_type_names[ aPinsElectricalType ];
 }
 
-static const wxString getPinStyleName( int aPinsStyle )
-{
-    const wxString pin_style_names[] =
-    {   // Keep these translated strings not static
-        _( "Line" ),
-        _( "Inverted" ),
-        _( "Clock" ),
-        _( "Inverted clock" ),
-        _( "Input low" ),
-        _( "Clock low" ),
-        _( "Output low" ),
-        _( "Falling edge clock" ),
-        _( "NonLogic" ),
-        wxT( "???" )
-    };
-
-    if( aPinsStyle < 0 || aPinsStyle > int( PIN_STYLE_CNT ) )
-        aPinsStyle = PIN_STYLE_CNT;
-
-    return pin_style_names[ aPinsStyle ];
-}
-
-
 /// Utility for getting the size of the 'internal' pin decorators (as a radius)
 // i.e. the clock symbols (falling clock is actually external but is of
 // the same kind)
@@ -241,11 +185,11 @@ static int ExternalPinDecoSize( const LIB_PIN &aPin )
 }
 
 LIB_PIN::LIB_PIN( LIB_PART*      aParent ) :
-    LIB_ITEM( LIB_PIN_T, aParent )
+    LIB_ITEM( LIB_PIN_T, aParent ),
+    m_shape( PINSHAPE_LINE )
 {
     m_length = LIB_EDIT_FRAME::GetDefaultPinLength();
     m_orientation = PIN_RIGHT;                  // Pin orient: Up, Down, Left, Right
-    m_shape = NONE;                             // Pin shape, bitwise.
     m_type = PIN_UNSPECIFIED;                   // electrical type of pin
     m_attributes = 0;                           // bit 0 != 0: pin invisible
     m_number = 0;                               // pin number (i.e. 4 ASCII chars)
@@ -378,7 +322,7 @@ void LIB_PIN::SetOrientation( int orientation )
 }
 
 
-void LIB_PIN::SetShape( int aShape )
+void LIB_PIN::SetShape( PinShape aShape )
 {
     if( m_shape != aShape )
     {
@@ -691,22 +635,7 @@ bool LIB_PIN::Save( OUTPUTFORMATTER& aFormatter )
     if( !IsVisible() && aFormatter.Print( 0, "N" ) < 0 )
         return false;
 
-    if( m_shape & INVERT && aFormatter.Print( 0, "I" ) < 0 )
-        return false;
-
-    if( m_shape & CLOCK && aFormatter.Print( 0, "C" ) < 0 )
-        return false;
-
-    if( m_shape & LOWLEVEL_IN && aFormatter.Print( 0, "L" ) < 0 )
-        return false;
-
-    if( m_shape & LOWLEVEL_OUT && aFormatter.Print( 0, "V" ) < 0 )
-        return false;
-
-    if( m_shape & CLOCK_FALL && aFormatter.Print( 0, "F" ) < 0 )
-        return false;
-
-    if( m_shape & NONLOGIC && aFormatter.Print( 0, "X" ) < 0 )
+    if( aFormatter.Print( 0, FormatSave( m_shape ) ) < 0 )
         return false;
 
     if( aFormatter.Print( 0, "\n" ) < 0 )
@@ -797,39 +726,30 @@ bool LIB_PIN::Load( LINE_READER& aLineReader, wxString& aErrorMsg )
 
     if( i == 12 )       /* Special Symbol defined */
     {
+        try
+        {
+            m_shape = ParseLoad( pinAttrs );
+        }
+        catch( ... )
+        {
+            aErrorMsg.Printf( wxT( "pin attributes do not give a valid pin shape [%s]" ), pinAttrs );
+            return false;
+        }
         for( j = strlen( pinAttrs ); j > 0; )
         {
             switch( pinAttrs[--j] )
             {
             case '~':
-                break;
-
-            case 'N':
-                m_attributes |= PIN_INVISIBLE;
-                break;
-
             case 'I':
-                m_shape |= INVERT;
-                break;
-
             case 'C':
-                m_shape |= CLOCK;
-                break;
-
             case 'L':
-                m_shape |= LOWLEVEL_IN;
-                break;
-
             case 'V':
-                m_shape |= LOWLEVEL_OUT;
-                break;
-
             case 'F':
-                m_shape |= CLOCK_FALL;
+            case 'X':
                 break;
 
-            case 'X':
-                m_shape |= NONLOGIC;
+            case 'N':
+                m_attributes |= PIN_INVISIBLE;
                 break;
 
             default:
@@ -964,7 +884,7 @@ void LIB_PIN::DrawPinSymbol( EDA_DRAW_PANEL* aPanel,
         break;
     }
 
-    if( m_shape & INVERT )
+    if( m_shape == PINSHAPE_INVERTED || m_shape == PINSHAPE_INVERTED_CLOCK )
     {
         const int radius = ExternalPinDecoSize( *this );
         GRCircle( clipbox, aDC, MapX1 * radius + x1,
@@ -975,7 +895,7 @@ void LIB_PIN::DrawPinSymbol( EDA_DRAW_PANEL* aPanel,
                   MapY1 * radius * 2 + y1 );
         GRLineTo( clipbox, aDC, posX, posY, width, color );
     }
-    else if( m_shape & CLOCK_FALL ) /* an alternative for Inverted Clock */
+    else if( m_shape == PINSHAPE_FALLING_EDGE_CLOCK ) /* an alternative for Inverted Clock */
     {
         const int clock_size = InternalPinDecoSize( *this );
         if( MapY1 == 0 ) /* MapX1 = +- 1 */
@@ -1020,7 +940,7 @@ void LIB_PIN::DrawPinSymbol( EDA_DRAW_PANEL* aPanel,
         GRLineTo( clipbox, aDC, posX, posY, width, color );
     }
 
-    if( m_shape & CLOCK )
+    if( m_shape == PINSHAPE_CLOCK || m_shape == PINSHAPE_INVERTED_CLOCK || m_shape == PINSHAPE_CLOCK_LOW )
     {
         const int clock_size = InternalPinDecoSize( *this );
         if( MapY1 == 0 ) /* MapX1 = +- 1 */
@@ -1057,7 +977,7 @@ void LIB_PIN::DrawPinSymbol( EDA_DRAW_PANEL* aPanel,
         }
     }
 
-    if( m_shape & LOWLEVEL_IN )     /* IEEE symbol "Active Low Input" */
+    if( m_shape == PINSHAPE_INPUT_LOW || m_shape == PINSHAPE_CLOCK_LOW )
     {
         const int symbol_size = ExternalPinDecoSize( *this );
         if( MapY1 == 0 )            /* MapX1 = +- 1 */
@@ -1081,7 +1001,7 @@ void LIB_PIN::DrawPinSymbol( EDA_DRAW_PANEL* aPanel,
     }
 
 
-    if( m_shape & LOWLEVEL_OUT )    /* IEEE symbol "Active Low Output" */
+    if( m_shape == PINSHAPE_OUTPUT_LOW )    /* IEEE symbol "Active Low Output" */
     {
         const int symbol_size = ExternalPinDecoSize( *this );
         if( MapY1 == 0 )            /* MapX1 = +- 1 */
@@ -1105,7 +1025,7 @@ void LIB_PIN::DrawPinSymbol( EDA_DRAW_PANEL* aPanel,
                       color );
         }
     }
-    else if( m_shape & NONLOGIC ) /* NonLogic pin symbol */
+    else if( m_shape == PINSHAPE_NONLOGIC ) /* NonLogic pin symbol */
     {
         const int symbol_size = ExternalPinDecoSize( *this );
         GRMoveTo( x1 - (MapX1 + MapY1) * symbol_size,
@@ -1391,7 +1311,7 @@ void LIB_PIN::PlotSymbol( PLOTTER* aPlotter, const wxPoint& aPosition, int aOrie
         break;
     }
 
-    if( m_shape & INVERT )
+    if( m_shape == PINSHAPE_INVERTED || m_shape == PINSHAPE_INVERTED_CLOCK )
     {
         const int radius = ExternalPinDecoSize( *this );
         aPlotter->Circle( wxPoint( MapX1 * radius + x1,
@@ -1404,7 +1324,7 @@ void LIB_PIN::PlotSymbol( PLOTTER* aPlotter, const wxPoint& aPosition, int aOrie
                                     MapY1 * radius * 2 + y1 ) );
         aPlotter->FinishTo( aPosition );
     }
-    else if( m_shape & CLOCK_FALL )
+    else if( m_shape == PINSHAPE_FALLING_EDGE_CLOCK )
     {
         const int clock_size = InternalPinDecoSize( *this );
         if( MapY1 == 0 ) /* MapX1 = +- 1 */
@@ -1430,7 +1350,8 @@ void LIB_PIN::PlotSymbol( PLOTTER* aPlotter, const wxPoint& aPosition, int aOrie
         aPlotter->FinishTo( aPosition );
     }
 
-    if( m_shape & CLOCK )
+    if( m_shape == PINSHAPE_CLOCK || m_shape == PINSHAPE_INVERTED_CLOCK ||
+        m_shape == PINSHAPE_CLOCK_LOW )
     {
         const int clock_size = InternalPinDecoSize( *this );
         if( MapY1 == 0 ) /* MapX1 = +- 1 */
@@ -1447,7 +1368,7 @@ void LIB_PIN::PlotSymbol( PLOTTER* aPlotter, const wxPoint& aPosition, int aOrie
         }
     }
 
-    if( m_shape & LOWLEVEL_IN )   /* IEEE symbol "Active Low Input" */
+    if( m_shape == PINSHAPE_INPUT_LOW || m_shape == PINSHAPE_CLOCK_LOW )    /* IEEE symbol "Active Low Input" */
     {
         const int symbol_size = ExternalPinDecoSize( *this );
         if( MapY1 == 0 )        /* MapX1 = +- 1 */
@@ -1467,7 +1388,7 @@ void LIB_PIN::PlotSymbol( PLOTTER* aPlotter, const wxPoint& aPosition, int aOrie
     }
 
 
-    if( m_shape & LOWLEVEL_OUT )  /* IEEE symbol "Active Low Output" */
+    if( m_shape == PINSHAPE_OUTPUT_LOW )    /* IEEE symbol "Active Low Output" */
     {
         const int symbol_size = ExternalPinDecoSize( *this );
         if( MapY1 == 0 )        /* MapX1 = +- 1 */
@@ -1481,7 +1402,7 @@ void LIB_PIN::PlotSymbol( PLOTTER* aPlotter, const wxPoint& aPosition, int aOrie
             aPlotter->FinishTo( wxPoint( x1, y1 + MapY1 * symbol_size * 2 ) );
         }
     }
-    else if( m_shape & NONLOGIC ) /* NonLogic pin symbol */
+    else if( m_shape == PINSHAPE_NONLOGIC ) /* NonLogic pin symbol */
     {
         const int symbol_size = ExternalPinDecoSize( *this );
         aPlotter->MoveTo( wxPoint( x1 - (MapX1 + MapY1) * symbol_size,
@@ -1978,11 +1899,7 @@ void LIB_PIN::GetMsgPanelInfo( MSG_PANEL_ITEMS& aList )
                                      LIB_PIN::GetElectricalTypeName( m_type ),
                                      RED ) );
 
-    int styleCodeIndex = GetStyleCodeIndex( m_shape );
-    if( styleCodeIndex >= 0 )
-        text = getPinStyleName( styleCodeIndex );
-    else
-        text = wxT( "?" );
+    text = GetText( m_shape );
 
     aList.push_back( MSG_PANEL_ITEM( _( "Style" ), text, BLUE ) );
 
@@ -2030,7 +1947,7 @@ const EDA_RECT LIB_PIN::GetBoundingBox() const
     // Actual text height is bigger than text size
     int numberTextHeight  = showNum ? KiROUND( m_numTextSize * 1.1 ) : 0;
 
-    if( m_shape & INVERT )
+    if( m_shape == PINSHAPE_INVERTED || m_shape == PINSHAPE_INVERTED_CLOCK )
         minsizeV = std::max( TARGET_PIN_RADIUS, ExternalPinDecoSize( *this ) );
 
     // calculate top left corner position
@@ -2177,40 +2094,6 @@ void LIB_PIN::Rotate()
 }
 
 
-wxArrayString LIB_PIN::GetStyleNames( void )
-{
-    wxArrayString tmp;
-
-    for( unsigned ii = 0; ii < PIN_STYLE_CNT; ii++ )
-        tmp.Add( getPinStyleName( ii ) );
-
-    return tmp;
-}
-
-
-int LIB_PIN::GetStyleCode( int index )
-{
-    if( index >= 0 && index < (int) PIN_STYLE_CNT )
-        return pin_style_codes[ index ];
-
-    return NONE;
-}
-
-
-int LIB_PIN::GetStyleCodeIndex( int code )
-{
-    size_t i;
-
-    for( i = 0; i < PIN_STYLE_CNT; i++ )
-    {
-        if( pin_style_codes[i] == code )
-            return (int) i;
-    }
-
-    return wxNOT_FOUND;
-}
-
-
 wxArrayString LIB_PIN::GetElectricalTypeNames( void )
 {
     wxArrayString tmp;
@@ -2234,12 +2117,6 @@ const BITMAP_DEF* LIB_PIN::GetOrientationSymbols()
 }
 
 
-const BITMAP_DEF* LIB_PIN::GetStyleSymbols()
-{
-    return iconsPinsShapes;
-}
-
-
 BITMAP_DEF LIB_PIN::GetMenuImage() const
 {
     return iconsPinsElectricalType[m_type];
@@ -2251,12 +2128,7 @@ wxString LIB_PIN::GetSelectMenuText() const
     wxString tmp;
     wxString style;
 
-    int styleCode = GetStyleCodeIndex( m_shape );
-
-    if( styleCode >= 0 )
-        style = getPinStyleName( styleCode );
-    else
-        style = wxT( "?" );
+    style = GetText( m_shape );
 
     tmp.Printf( _( "Pin %s, %s, %s" ),
                 GetChars( GetNumberString() ),
diff --git a/eeschema/lib_pin.h b/eeschema/lib_pin.h
index cc9f3d0..b4fd7c7 100644
--- a/eeschema/lib_pin.h
+++ b/eeschema/lib_pin.h
@@ -32,6 +32,7 @@
 
 #include <lib_draw_item.h>
 
+#include "pin_shape.h"
 
 #define TARGET_PIN_RADIUS   12  // Circle diameter drawn at the active end of pins
 
@@ -59,20 +60,6 @@ enum ElectricPinType {
 
 
 /**
- * The component library pin object drawing shapes.
- */
-enum DrawPinShape {
-    NONE         = 0,
-    INVERT       = 1,
-    CLOCK        = 2,
-    LOWLEVEL_IN  = 4,
-    LOWLEVEL_OUT = 8,
-    CLOCK_FALL   = 0x10, /* this is common form for inverted clock in Eastern Block */
-    NONLOGIC     = 0x20
-};
-
-
-/**
  *  The component library pin object orientations.
  */
 enum DrawPinOrient {
@@ -88,7 +75,7 @@ class LIB_PIN : public LIB_ITEM
     wxPoint  m_position;     ///< Position of the pin.
     int      m_length;       ///< Length of the pin.
     int      m_orientation;  ///< Pin orientation (Up, Down, Left, Right)
-    int      m_shape;        ///< Bitwise ORed of pin shapes (see enum DrawPinShape)
+    PinShape m_shape;        ///< Shape drawn around pin
     int      m_width;        ///< Line width of the pin.
     int      m_type;         ///< Electrical type of the pin.  See enum ElectricPinType.
     int      m_attributes;   ///< Set bit 0 to indicate pin is invisible.
@@ -248,16 +235,16 @@ public:
 
     void Rotate();
 
-    int GetShape() const { return m_shape; }
+    PinShape GetShape() const { return m_shape; }
 
     /**
      * Set the shape of the pin to \a aShape.
      *
      * This will also update the draw style of the pins marked by EnableEditMode().
      *
-     * @param aShape - The draw shape of the pin.  See enum DrawPinShape.
+     * @param aShape - The draw shape of the pin.  See enum PinShape.
      */
-    void SetShape( int aShape );
+    void SetShape( PinShape aShape );
 
     /**
      * Get the electrical type of the pin.
@@ -456,38 +443,6 @@ public:
     static int GetOrientationCodeIndex( int aCode );
 
     /**
-     * Get a list of pin draw style names.
-     *
-     * @return  List of valid pin draw style names.
-     */
-    static wxArrayString GetStyleNames();
-
-    /**
-     * Get a list of pin styles bitmaps for menus and dialogs.
-     *
-     * @return  List of valid pin electrical type bitmaps symbols in .xpm format.
-     */
-    static const BITMAP_DEF* GetStyleSymbols();
-
-    /**
-     * Get the pin draw style code by index used to set the pin draw style.
-     *
-     * @param aIndex - The index of the pin draw style code to look up.
-     * @return  Pin draw style code if index is valid.  Returns NONE
-     *          style on index error.
-     */
-    static int GetStyleCode( int aIndex );
-
-    /**
-     * Get the index of the pin draw style code.
-     *
-     * @param aCode - The pin draw style code to look up.
-     * @return The index of the pin draw style code if found.  Otherwise,
-     *         return wxNOT_FOUND.
-     */
-    static int GetStyleCodeIndex( int aCode );
-
-    /**
      * Get a list of pin electrical type names.
      *
      * @return  List of valid pin electrical type names.
diff --git a/eeschema/pin_shape.cpp b/eeschema/pin_shape.cpp
new file mode 100644
index 0000000..38c5fed
--- /dev/null
+++ b/eeschema/pin_shape.cpp
@@ -0,0 +1,194 @@
+#include "pin_shape.h"
+
+#include "macros.h"
+
+#include <stdexcept>
+
+const char* FormatSave( PinShape shape )
+{
+    switch( shape )
+    {
+    case PINSHAPE_LINE:
+        return "";
+
+    case PINSHAPE_INVERTED:
+        return "I";
+
+    case PINSHAPE_CLOCK:
+        return "C";
+
+    case PINSHAPE_INVERTED_CLOCK:
+        return "IC";
+
+    case PINSHAPE_INPUT_LOW:
+        return "L";
+
+    case PINSHAPE_CLOCK_LOW:
+        return "CL";
+
+    case PINSHAPE_OUTPUT_LOW:
+        return "V";
+
+    case PINSHAPE_FALLING_EDGE_CLOCK:
+        return "F";
+
+    case PINSHAPE_NONLOGIC:
+        return "X";
+    }
+
+    assert( !"Invalid pin shape" );
+    return "?";
+}
+
+
+PinShape ParseLoad( const char* str )
+{
+    enum
+    {
+        INVERTED        = 1 << 0,
+        CLOCK           = 1 << 1,
+        LOWLEVEL_IN     = 1 << 2,
+        LOWLEVEL_OUT    = 1 << 3,
+        FALLING_EDGE    = 1 << 4,
+        NONLOGIC        = 1 << 5
+    };
+
+    int flags = 0;
+
+    for( char c; (c = *str); ++str )
+    {
+        switch( c )
+        {
+        case 'I':
+            flags |= INVERTED;
+            break;
+
+        case 'C':
+            flags |= CLOCK;
+            break;
+
+        case 'L':
+            flags |= LOWLEVEL_IN;
+            break;
+
+        case 'V':
+            flags |= LOWLEVEL_OUT;
+            break;
+
+        case 'F':
+            flags |= FALLING_EDGE;
+            break;
+
+        case 'X':
+            flags |= NONLOGIC;
+            break;
+        }
+    }
+
+    switch( flags )
+    {
+    case 0:
+        return PINSHAPE_LINE;
+
+    case INVERTED:
+        return PINSHAPE_INVERTED;
+
+    case CLOCK:
+        return PINSHAPE_CLOCK;
+
+    case INVERTED | CLOCK:
+        return PINSHAPE_INVERTED_CLOCK;
+
+    case LOWLEVEL_IN:
+        return PINSHAPE_INPUT_LOW;
+
+    case LOWLEVEL_IN | CLOCK:
+        return PINSHAPE_CLOCK_LOW;
+
+    case LOWLEVEL_OUT:
+        return PINSHAPE_OUTPUT_LOW;
+
+    case FALLING_EDGE:
+        return PINSHAPE_FALLING_EDGE_CLOCK;
+
+    case NONLOGIC:
+        return PINSHAPE_NONLOGIC;
+
+    default:
+        throw std::logic_error( "Illegal combination of flags" );
+    }
+}
+
+
+wxString GetText( PinShape shape )
+{
+    switch( shape )
+    {
+    case PINSHAPE_LINE:
+        return _( "Line" );
+
+    case PINSHAPE_INVERTED:
+        return _( "Inverted" );
+
+    case PINSHAPE_CLOCK:
+        return _( "Clock" );
+
+    case PINSHAPE_INVERTED_CLOCK:
+        return _( "Inverted clock" );
+
+    case PINSHAPE_INPUT_LOW:
+        return _( "Input low" );
+
+    case PINSHAPE_CLOCK_LOW:
+        return _( "Clock low" );
+
+    case PINSHAPE_OUTPUT_LOW:
+        return _( "Output low" );
+
+    case PINSHAPE_FALLING_EDGE_CLOCK:
+        return _( "Falling edge clock" );
+
+    case PINSHAPE_NONLOGIC:
+        return _( "NonLogic" );
+    }
+
+    assert( !"Invalid pin shape" );
+    return wxT( "?" );
+}
+
+
+BITMAP_DEF GetBitmap( PinShape shape )
+{
+    switch( shape )
+    {
+    case PINSHAPE_LINE:
+        return pinshape_normal_xpm;
+
+    case PINSHAPE_INVERTED:
+        return pinshape_invert_xpm;
+
+    case PINSHAPE_CLOCK:
+        return pinshape_clock_normal_xpm;
+
+    case PINSHAPE_INVERTED_CLOCK:
+        return pinshape_clock_invert_xpm;
+
+    case PINSHAPE_INPUT_LOW:
+        return pinshape_active_low_input_xpm;
+
+    case PINSHAPE_CLOCK_LOW:
+        return pinshape_clock_active_low_xpm;
+
+    case PINSHAPE_OUTPUT_LOW:
+        return pinshape_active_low_output_xpm;
+
+    case PINSHAPE_FALLING_EDGE_CLOCK:
+        return pinshape_clock_fall_xpm;
+
+    case PINSHAPE_NONLOGIC:
+        return pinshape_nonlogic_xpm;
+    }
+
+    assert( !"Invalid pin shape" );
+    return 0;
+};
diff --git a/eeschema/pin_shape.h b/eeschema/pin_shape.h
new file mode 100644
index 0000000..bec3e1d
--- /dev/null
+++ b/eeschema/pin_shape.h
@@ -0,0 +1,33 @@
+#ifndef _PIN_SHAPE_H_
+#define _PIN_SHAPE_H_
+
+#include <wx/string.h>
+#include <bitmaps.h>
+
+enum PinShape
+{
+    PINSHAPE_LINE,
+    PINSHAPE_INVERTED,
+    PINSHAPE_CLOCK,
+    PINSHAPE_INVERTED_CLOCK,
+    PINSHAPE_INPUT_LOW,
+    PINSHAPE_CLOCK_LOW,
+    PINSHAPE_OUTPUT_LOW,
+    PINSHAPE_FALLING_EDGE_CLOCK,
+    PINSHAPE_NONLOGIC
+};
+
+enum
+{
+    PINSHAPE_COUNT = PINSHAPE_NONLOGIC + 1
+};
+
+// Load/Save
+const char* FormatSave( PinShape shape );
+PinShape    ParseLoad( const char* );
+
+// UI
+wxString    GetText( PinShape shape );
+BITMAP_DEF  GetBitmap( PinShape shape );
+
+#endif
diff --git a/eeschema/pinedit.cpp b/eeschema/pinedit.cpp
index 58c706a..7be4b3d 100644
--- a/eeschema/pinedit.cpp
+++ b/eeschema/pinedit.cpp
@@ -53,14 +53,14 @@ static void AbortPinMove( EDA_DRAW_PANEL* Panel, wxDC* DC );
 static void DrawMovePin( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& aPositon, bool aErase );
 
 
-static wxPoint OldPos;
-static wxPoint PinPreviousPos;
-static int     LastPinType          = PIN_INPUT;
-static int     LastPinOrient        = PIN_RIGHT;
-static int     LastPinShape         = NONE;
-static bool    LastPinCommonConvert = false;
-static bool    LastPinCommonUnit    = false;
-static bool    LastPinVisible       = true;
+static wxPoint  OldPos;
+static wxPoint  PinPreviousPos;
+static int      LastPinType = PIN_INPUT;
+static int      LastPinOrient = PIN_RIGHT;
+static PinShape LastPinShape = PINSHAPE_LINE;
+static bool     LastPinCommonConvert = false;
+static bool     LastPinCommonUnit = false;
+static bool     LastPinVisible = true;
 
 // The -1 is a non-valid value to trigger delayed initialization
 static int     LastPinLength        = -1;
@@ -104,8 +104,7 @@ void LIB_EDIT_FRAME::OnEditPin( wxCommandEvent& event )
     wxString units = GetUnitsLabel( g_UserUnit );
     dlg.SetOrientationList( LIB_PIN::GetOrientationNames(), LIB_PIN::GetOrientationSymbols() );
     dlg.SetOrientation( LIB_PIN::GetOrientationCodeIndex( pin->GetOrientation() ) );
-    dlg.SetStyleList( LIB_PIN::GetStyleNames(), LIB_PIN::GetStyleSymbols() );
-    dlg.SetStyle( LIB_PIN::GetStyleCodeIndex( pin->GetShape() ) );
+    dlg.SetStyle( pin->GetShape() );
     dlg.SetElectricalTypeList( LIB_PIN::GetElectricalTypeNames(),
                                LIB_PIN::GetElectricalTypeSymbols() );
     dlg.SetElectricalType( pin->GetType() );
@@ -147,7 +146,7 @@ void LIB_EDIT_FRAME::OnEditPin( wxCommandEvent& event )
     LastPinNumSize = ValueFromString( g_UserUnit, dlg.GetPadNameTextSize() );
     LastPinOrient = LIB_PIN::GetOrientationCode( dlg.GetOrientation() );
     LastPinLength = ValueFromString( g_UserUnit, dlg.GetLength() );
-    LastPinShape = LIB_PIN::GetStyleCode( dlg.GetStyle() );
+    LastPinShape = dlg.GetStyle();
     LastPinType = dlg.GetElectricalType();
     LastPinCommonConvert = dlg.GetAddToAllBodyStyles();
     LastPinCommonUnit = dlg.GetAddToAllParts();
diff --git a/eeschema/widgets/pin_shape_combobox.cpp b/eeschema/widgets/pin_shape_combobox.cpp
new file mode 100644
index 0000000..b2aff5c
--- /dev/null
+++ b/eeschema/widgets/pin_shape_combobox.cpp
@@ -0,0 +1,69 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2015 Simon Richter <Simon.Richter@xxxxxxxxxx>
+ *
+ * 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 pin_shape.cpp
+ * @brief ComboBox widget for pin shape
+ */
+
+#include "pin_shape_combobox.h"
+
+#include <bitmaps.h>
+
+PinShapeComboBox::PinShapeComboBox( wxWindow* parent,
+        wxWindowID id,
+        const wxString& value,
+        const wxPoint& pos,
+        const wxSize& size,
+        int n,
+        const wxString choices[],
+        long style,
+        const wxValidator& validator,
+        const wxString& name ) :
+    wxBitmapComboBox( parent, id, value, pos, size, n, choices, style, validator, name )
+{
+    for( unsigned int i = 0; i < PINSHAPE_COUNT; ++i )
+    {
+        PinShape shape = static_cast<PinShape>( i );
+
+        wxString text = GetText( shape );
+        BITMAP_DEF bitmap = GetBitmap( shape );
+
+        if( bitmap )
+            Insert( text, KiBitmap( bitmap ), i );
+        else
+            Append( text );
+    }
+}
+
+
+PinShape PinShapeComboBox::GetSelection()
+{
+    return PinShape( wxBitmapComboBox::GetSelection() );
+}
+
+
+void PinShapeComboBox::SetSelection( PinShape aShape )
+{
+    wxBitmapComboBox::SetSelection( aShape );
+}
diff --git a/eeschema/widgets/pin_shape_combobox.h b/eeschema/widgets/pin_shape_combobox.h
new file mode 100644
index 0000000..1784067
--- /dev/null
+++ b/eeschema/widgets/pin_shape_combobox.h
@@ -0,0 +1,51 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2015 Simon Richter <Simon.Richter@xxxxxxxxxx>
+ *
+ * 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 pin_shape.h
+ * @brief ComboBox widget for pin shape
+ */
+
+#include <wx/bmpcbox.h>
+
+#include <pin_shape.h>
+
+class PinShapeComboBox : public wxBitmapComboBox
+{
+public:
+    /// @todo C++11: replace with forwarder
+
+    PinShapeComboBox( wxWindow* parent,
+            wxWindowID id = wxID_ANY,
+            const wxString& value = wxEmptyString,
+            const wxPoint& pos = wxDefaultPosition,
+            const wxSize& size = wxDefaultSize,
+            int n = 0,
+            const wxString choices[] = NULL,
+            long style = 0,
+            const wxValidator& validator = wxDefaultValidator,
+            const wxString& name = wxBitmapComboBoxNameStr );
+
+    PinShape    GetSelection();
+    void        SetSelection( PinShape aShape );
+};
-- 
2.1.4



Follow ups