← Back to team overview

kicad-developers team mailing list archive

[PATCH] Add layer-selecting via actions to GAL

 

Hi,

These patches add the "Select Layer and Add (Through|Blind) Via"
actions to the GAL: https://bugs.launchpad.net/kicad/+bug/1675195

I think I have done the calls to AddLayerPair() sensibly, but perhaps
the GAL wizards could double check, as the router is new territory for
me!

It's actually a bit tricky to use these actions right now for two
reasons unrelated to this patch:

* There's a quirk where placing a via right on a track node ends the
routing: https://bugs.launchpad.net/kicad/+bug/1675195
* If activating by the menu, the cursor isn't warped to where the menu
was invoked, so the via starts where the action was in the context
menu, not where the user originally clicked. This can lead
to...extravagant deviations in the trace if there is something in the
way.

Cheers,

John
From d2b0cfa38b53a25292ac150f896b0774b1f0848b Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Thu, 23 Mar 2017 00:56:01 +0800
Subject: [PATCH 1/2] Signal via actions with flags, not event equality
 checking

This allows actions to encode both via type and other behaviours
intrisically witohut having to individually test for each action.
---
 pcbnew/router/router_tool.cpp | 49 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp
index de95967d4..476f0d88b 100644
--- a/pcbnew/router/router_tool.cpp
+++ b/pcbnew/router/router_tool.cpp
@@ -64,6 +64,20 @@ using namespace std::placeholders;
 using namespace KIGFX;
 using boost::optional;
 
+
+/**
+ * Flags used by via tool actions
+ */
+enum VIA_ACTION_FLAGS
+{
+    // Via type
+    VIA_MASK     = 0x03,
+    VIA          = 0x00,     ///> Normal via
+    BLIND_VIA    = 0x01,     ///> blind/buried via
+    MICROVIA     = 0x02,     ///> Microvia
+};
+
+
 TOOL_ACTION PCB_ACTIONS::routerActivateSingle( "pcbnew.InteractiveRouter.SingleTrack",
         AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_ADD_NEW_TRACK ),
         _( "Interactive Router (Single Tracks)" ),
@@ -115,18 +129,21 @@ static const TOOL_ACTION ACT_PlaceThroughVia( "pcbnew.InteractiveRouter.PlaceVia
     AS_CONTEXT, TOOL_ACTION::LegacyHotKey( HK_ADD_THROUGH_VIA ),
     _( "Place Through Via" ),
     _( "Adds a through-hole via at the end of currently routed track." ),
-    via_xpm );
+    via_xpm, AF_NONE,
+    (void*) VIA_ACTION_FLAGS::VIA );
 
 static const TOOL_ACTION ACT_PlaceBlindVia( "pcbnew.InteractiveRouter.PlaceBlindVia",
     AS_CONTEXT, TOOL_ACTION::LegacyHotKey( HK_ADD_BLIND_BURIED_VIA ),
     _( "Place Blind/Buried Via" ),
     _( "Adds a blind or buried via at the end of currently routed track."),
-    via_buried_xpm );
+    via_buried_xpm, AF_NONE,
+    (void*) VIA_ACTION_FLAGS::BLIND_VIA );
 
 static const TOOL_ACTION ACT_PlaceMicroVia( "pcbnew.InteractiveRouter.PlaceMicroVia",
     AS_CONTEXT, TOOL_ACTION::LegacyHotKey( HK_ADD_MICROVIA ),
     _( "Place Microvia" ), _( "Adds a microvia at the end of currently routed track." ),
-    via_microvia_xpm );
+    via_microvia_xpm, AF_NONE,
+    (void*) VIA_ACTION_FLAGS::MICROVIA );
 
 static const TOOL_ACTION ACT_CustomTrackWidth( "pcbnew.InteractiveRouter.CustomTrackViaSize",
     AS_CONTEXT, 'Q',
@@ -418,18 +435,34 @@ void ROUTER_TOOL::switchLayerOnViaPlacement()
 }
 
 
-int ROUTER_TOOL::onViaCommand( const TOOL_EVENT& aEvent )
+static VIATYPE_T getViaTypeFromFlags( int aFlags )
 {
     VIATYPE_T viaType = VIA_THROUGH;
 
-    if( aEvent.IsAction( &ACT_PlaceThroughVia ) )
+    switch( aFlags & VIA_ACTION_FLAGS::VIA_MASK )
+    {
+    case VIA_ACTION_FLAGS::VIA:
         viaType = VIA_THROUGH;
-    else if( aEvent.IsAction( &ACT_PlaceBlindVia ) )
+        break;
+    case VIA_ACTION_FLAGS::BLIND_VIA:
         viaType = VIA_BLIND_BURIED;
-    else if( aEvent.IsAction( &ACT_PlaceMicroVia ) )
+        break;
+    case VIA_ACTION_FLAGS::MICROVIA:
         viaType = VIA_MICROVIA;
-    else
+        break;
+    default:
         wxASSERT_MSG( false, "Unhandled via type" );
+    }
+
+    return viaType;
+}
+
+
+int ROUTER_TOOL::onViaCommand( const TOOL_EVENT& aEvent )
+{
+    const int actViaFlags = aEvent.Parameter<intptr_t>();
+
+    VIATYPE_T viaType = getViaTypeFromFlags( actViaFlags );
 
     BOARD_DESIGN_SETTINGS& bds = m_board->GetDesignSettings();
 
-- 
2.12.0

From 40242b6b6c55ae168e1d8b53a82f598a1ff99403 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Thu, 16 Mar 2017 19:20:18 +0800
Subject: [PATCH 2/2] Add Select Layer And Add Via actions to GAL router tool

On these actions, invoke the layer select dialog and set the via layer
pairs accordingly.

Fixes: lp:1672820
* https://bugs.launchpad.net/kicad/+bug/1672820
---
 pcbnew/router/router_tool.cpp | 83 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 5 deletions(-)

diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp
index 476f0d88b..51b2061ca 100644
--- a/pcbnew/router/router_tool.cpp
+++ b/pcbnew/router/router_tool.cpp
@@ -75,6 +75,9 @@ enum VIA_ACTION_FLAGS
     VIA          = 0x00,     ///> Normal via
     BLIND_VIA    = 0x01,     ///> blind/buried via
     MICROVIA     = 0x02,     ///> Microvia
+
+    // Select layer
+    SELECT_LAYER = VIA_MASK + 1,    ///> Ask user to select layer before adding via
 };
 
 
@@ -145,6 +148,22 @@ static const TOOL_ACTION ACT_PlaceMicroVia( "pcbnew.InteractiveRouter.PlaceMicro
     via_microvia_xpm, AF_NONE,
     (void*) VIA_ACTION_FLAGS::MICROVIA );
 
+static const TOOL_ACTION ACT_SelLayerAndPlaceThroughVia(
+    "pcbnew.InteractiveRouter.SelLayerAndPlaceVia",
+    AS_CONTEXT, TOOL_ACTION::LegacyHotKey( HK_SEL_LAYER_AND_ADD_THROUGH_VIA ),
+    _( "Select Layer and Place Through Via" ),
+    _( "Select a layer, then add a through-hole via at the end of currently routed track." ),
+    select_w_layer_xpm, AF_NONE,
+    (void*) ( VIA_ACTION_FLAGS::VIA | VIA_ACTION_FLAGS::SELECT_LAYER ) );
+
+static const TOOL_ACTION ACT_SelLayerAndPlaceBlindVia(
+    "pcbnew.InteractiveRouter.SelLayerAndPlaceBlindVia",
+    AS_CONTEXT, TOOL_ACTION::LegacyHotKey( HK_SEL_LAYER_AND_ADD_BLIND_BURIED_VIA ),
+    _( "Select Layer and Place Blind/Buried Via" ),
+    _( "Select a layer, then add a blind or buried via at the end of currently routed track."),
+    select_w_layer_xpm, AF_NONE,
+    (void*) ( VIA_ACTION_FLAGS::BLIND_VIA | VIA_ACTION_FLAGS::SELECT_LAYER ) );
+
 static const TOOL_ACTION ACT_CustomTrackWidth( "pcbnew.InteractiveRouter.CustomTrackViaSize",
     AS_CONTEXT, 'Q',
     _( "Custom Track/Via Size" ),
@@ -287,6 +306,8 @@ public:
         Add( ACT_PlaceThroughVia );
         Add( ACT_PlaceBlindVia );
         Add( ACT_PlaceMicroVia );
+        Add( ACT_SelLayerAndPlaceThroughVia );
+        Add( ACT_SelLayerAndPlaceBlindVia );
         Add( ACT_SwitchPosture );
 
         AppendSeparator();
@@ -454,6 +475,8 @@ static VIATYPE_T getViaTypeFromFlags( int aFlags )
         wxASSERT_MSG( false, "Unhandled via type" );
     }
 
+    wxLogDebug("via type %d", viaType );
+
     return viaType;
 }
 
@@ -463,6 +486,7 @@ int ROUTER_TOOL::onViaCommand( const TOOL_EVENT& aEvent )
     const int actViaFlags = aEvent.Parameter<intptr_t>();
 
     VIATYPE_T viaType = getViaTypeFromFlags( actViaFlags );
+    const bool selectLayer = actViaFlags & VIA_ACTION_FLAGS::SELECT_LAYER;
 
     BOARD_DESIGN_SETTINGS& bds = m_board->GetDesignSettings();
 
@@ -473,6 +497,17 @@ int ROUTER_TOOL::onViaCommand( const TOOL_EVENT& aEvent )
 
     PNS::SIZES_SETTINGS sizes = m_router->Sizes();
 
+    // ask the user for a target layer
+    LAYER_ID targetLayer = UNDEFINED_LAYER;
+
+    if( selectLayer )
+    {
+        wxPoint dlgPosition = wxGetMousePosition();
+
+        targetLayer = m_frame->SelectLayer( static_cast<LAYER_ID>( currentLayer ),
+                LSET::AllNonCuMask(), dlgPosition );
+    }
+
     // fixme: P&S supports more than one fixed layer pair. Update the dialog?
     sizes.ClearLayerPairs();
 
@@ -520,29 +555,65 @@ int ROUTER_TOOL::onViaCommand( const TOOL_EVENT& aEvent )
     case VIA_THROUGH:
         sizes.SetViaDiameter( bds.GetCurrentViaSize() );
         sizes.SetViaDrill( bds.GetCurrentViaDrill() );
-        sizes.AddLayerPair( pairTop, pairBottom );
+
+        if( targetLayer != UNDEFINED_LAYER )
+        {
+            // go from the current layer to the chosen layer
+            sizes.AddLayerPair( currentLayer, targetLayer );
+        }
+        else
+        {
+            // use the default layer pair
+            sizes.AddLayerPair( pairTop, pairBottom );
+        }
         break;
 
     case VIA_MICROVIA:
         sizes.SetViaDiameter( bds.GetCurrentMicroViaSize() );
         sizes.SetViaDrill( bds.GetCurrentMicroViaDrill() );
 
+        wxASSERT_MSG( !selectLayer, "Unexpected select layer for microvia (microvia layers are implicit)" );
+
         if( currentLayer == F_Cu || currentLayer == In1_Cu )
+        {
+            // front-side microvia
             sizes.AddLayerPair( F_Cu, In1_Cu );
+        }
         else if( currentLayer == B_Cu || currentLayer == layerCount - 2 )
+        {
+            // back-side microvia
             sizes.AddLayerPair( B_Cu, layerCount - 2 );
+        }
         else
-            wxASSERT( false );
+        {
+            wxASSERT_MSG( false, "Invalid layer pair for microvia (must be on or adjacent to an outer layer)" );
+        }
         break;
 
     case VIA_BLIND_BURIED:
         sizes.SetViaDiameter( bds.GetCurrentViaSize() );
         sizes.SetViaDrill( bds.GetCurrentViaDrill() );
 
-        if( currentLayer == pairTop || currentLayer == pairBottom )
-            sizes.AddLayerPair( pairTop, pairBottom );
+        if( targetLayer != UNDEFINED_LAYER )
+        {
+            // go directly to the user specified layer
+            sizes.AddLayerPair( currentLayer, targetLayer );
+        }
         else
-            sizes.AddLayerPair( pairTop, currentLayer );
+        {
+            if( currentLayer == pairTop || currentLayer == pairBottom )
+            {
+                // the current layer is on the defined layer pair,
+                // swap to the other side
+                sizes.AddLayerPair( pairTop, pairBottom );
+            }
+            else
+            {
+                // the current layer is not part of the current layer pair,
+                // so fallback and swap to the top layer of the pair by default
+                sizes.AddLayerPair( pairTop, currentLayer );
+            }
+        }
         break;
 
     default:
@@ -730,6 +801,8 @@ void ROUTER_TOOL::SetTransitions()
     Go( &ROUTER_TOOL::onViaCommand, ACT_PlaceThroughVia.MakeEvent() );
     Go( &ROUTER_TOOL::onViaCommand, ACT_PlaceBlindVia.MakeEvent() );
     Go( &ROUTER_TOOL::onViaCommand, ACT_PlaceMicroVia.MakeEvent() );
+    Go( &ROUTER_TOOL::onViaCommand, ACT_SelLayerAndPlaceThroughVia.MakeEvent() );
+    Go( &ROUTER_TOOL::onViaCommand, ACT_SelLayerAndPlaceBlindVia.MakeEvent() );
 
     // TODO is not this redundant? the same actions can be used for menus and hotkeys
     Go( &ROUTER_TOOL::SettingsDialog, ACT_RouterOptions.MakeEvent() );
-- 
2.12.0


Follow ups