Fix re-entrancy problem in cross-probing.

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

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

eeschema/cross-probing.cpp

@@ -45,10 +45,10 @@
 #include <tools/sch_editor_control.h>
 
 
-SCH_ITEM* SCH_EDIT_FRAME::FindComponentAndItem( const wxString& aReference,
+SCH_ITEM* SCH_EDITOR_CONTROL::FindComponentAndItem( const wxString& aReference,
-                                                bool            aSearchHierarchy,
+                                                    bool            aSearchHierarchy,
-                                                SCH_SEARCH_T    aSearchType,
+                                                    SCH_SEARCH_T    aSearchType,
-                                                const wxString& aSearchText )
+                                                    const wxString& aSearchText )
 {
     SCH_SHEET_PATH* sheetWithComponentFound = NULL;
     SCH_ITEM*       item = NULL;
@@ -107,9 +107,9 @@ SCH_ITEM* SCH_EDIT_FRAME::FindComponentAndItem( const wxString& aReference,
     {
         if( *sheetWithComponentFound != *g_CurrentSheet )
         {
-            sheetWithComponentFound->LastScreen()->SetZoom( GetScreen()->GetZoom() );
+            sheetWithComponentFound->LastScreen()->SetZoom( m_frame->GetScreen()->GetZoom() );
             *g_CurrentSheet = *sheetWithComponentFound;
-            DisplayCurrentSheet();
+            m_frame->DisplayCurrentSheet();
         }
 
         wxPoint delta;
@@ -117,8 +117,8 @@ SCH_ITEM* SCH_EDIT_FRAME::FindComponentAndItem( const wxString& aReference,
         delta = Component->GetTransform().TransformCoordinate( pos );
         pos   = delta + Component->GetPosition();
 
-        GetCanvas()->GetViewControls()->SetCrossHairCursorPosition( pos, false );
+        m_frame->GetCanvas()->GetViewControls()->SetCrossHairCursorPosition( pos, false );
-        CenterScreen( pos, false );
+        m_frame->CenterScreen( pos, false );
     }
 
     /* Print diag */
@@ -140,16 +140,20 @@ SCH_ITEM* SCH_EDIT_FRAME::FindComponentAndItem( const wxString& aReference,
     else
         msg.Printf( _( "Component %s not found" ), aReference );
 
-    SetStatusText( msg );
+    m_frame->SetStatusText( msg );
 
-    // Clear any existing highlighting
+    m_probingPcbToSch = true;   // recursion guard
-    GetToolManager()->RunAction( EE_ACTIONS::clearSelection, true );
+    {
-    GetCanvas()->GetView()->HighlightItem( nullptr, nullptr );
+        // Clear any existing highlighting
+        m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
+        m_frame->GetCanvas()->GetView()->HighlightItem( nullptr, nullptr );
 
-    if( foundItem )
+        if( foundItem )
-        GetCanvas()->GetView()->HighlightItem( foundItem, pin );
+            m_frame->GetCanvas()->GetView()->HighlightItem( foundItem, pin );
+    }
+    m_probingPcbToSch = false;
 
-    GetCanvas()->Refresh();
+    m_frame->GetCanvas()->Refresh();
 
     return item;
 }
@@ -175,7 +179,8 @@ SCH_ITEM* SCH_EDIT_FRAME::FindComponentAndItem( const wxString& aReference,
  */
 void SCH_EDIT_FRAME::ExecuteRemoteCommand( const char* cmdline )
 {
-    char     line[1024];
+    SCH_EDITOR_CONTROL* editor = m_toolManager->GetTool<SCH_EDITOR_CONTROL>();
+    char                line[1024];
 
     strncpy( line, cmdline, sizeof(line) - 1 );
     line[ sizeof(line) - 1 ] = '\0';
@@ -220,7 +225,7 @@ void SCH_EDIT_FRAME::ExecuteRemoteCommand( const char* cmdline )
     if( idcmd == NULL )    // Highlight component only (from Cvpcb or Pcbnew)
     {
         // Highlight component part_ref, or clear Highlight, if part_ref is not existing
-        FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, wxEmptyString );
+        editor->FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, wxEmptyString );
         return;
     }
 
@@ -235,21 +240,21 @@ void SCH_EDIT_FRAME::ExecuteRemoteCommand( const char* cmdline )
     {
         // Highlighting the reference itself isn't actually that useful, and it's harder to
         // see.  Highlight the parent and display the message.
-        FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, msg );
+        editor->FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, msg );
     }
     else if( strcmp( idcmd, "$VAL:" ) == 0 )
     {
         // Highlighting the value itself isn't actually that useful, and it's harder to see.
         // Highlight the parent and display the message.
-        FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, msg );
+        editor->FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, msg );
     }
     else if( strcmp( idcmd, "$PAD:" ) == 0 )
     {
-        FindComponentAndItem( part_ref, true, HIGHLIGHT_PIN, msg );
+        editor->FindComponentAndItem( part_ref, true, HIGHLIGHT_PIN, msg );
     }
     else
     {
-        FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, wxEmptyString );
+        editor->FindComponentAndItem( part_ref, true, HIGHLIGHT_COMPONENT, wxEmptyString );
     }
 }
 

eeschema/dialogs/dialog_fields_editor_global.cpp

@@ -38,6 +38,8 @@
 #include <class_library.h>
 #include <sch_edit_frame.h>
 #include <sch_reference_list.h>
+#include <tool/tool_manager.h>
+#include <tools/sch_editor_control.h>
 #include <kiface_i.h>
 #include <eda_doc.h>
 #include <widgets/grid_text_button_helpers.h>
@@ -1035,8 +1037,10 @@ void DIALOG_FIELDS_EDITOR_GLOBAL::OnTableCellClick( wxGridEvent& event )
         // Focus Eeschema view on the component selected in the dialog
         if( refs.size() == 1 )
         {
-            m_parent->FindComponentAndItem( refs[0].GetRef() + refs[0].GetRefNumber(),
+            SCH_EDITOR_CONTROL* editor = m_parent->GetToolManager()->GetTool<SCH_EDITOR_CONTROL>();
-                                            true, HIGHLIGHT_COMPONENT, wxEmptyString );
+
+            editor->FindComponentAndItem( refs[0].GetRef() + refs[0].GetRefNumber(), true,
+                                          HIGHLIGHT_COMPONENT, wxEmptyString );
         }
     }
     else

eeschema/sch_edit_frame.h

@@ -323,20 +323,6 @@ public:
      */
     void AddItemToScreenAndUndoList( SCH_ITEM* aItem, bool aUndoAppend = false );
 
-    /**
-     * Finds a component in the schematic and an item in this component.
-     *
-     * @param aReference The component reference designator to find.
-     * @param aSearchHierarchy If false, search the current sheet only.  Otherwise,
-     *                         the entire hierarchy
-     * @param aSearchType A #SCH_SEARCH_T value used to determine what to search for.
-     * @param aSearchText The text to search for, either in value, reference or elsewhere.
-     */
-    SCH_ITEM* FindComponentAndItem( const wxString& aReference,
-                                    bool            aSearchHierarchy,
-                                    SCH_SEARCH_T    aSearchType,
-                                    const wxString& aSearchText );
-
     /**
      * Run the Find or Find & Replace dialog.
      */

eeschema/tools/sch_editor_control.cpp

@@ -373,11 +373,8 @@ int SCH_EDITOR_CONTROL::ExplicitCrossProbeToPcb( const TOOL_EVENT& aEvent )
 void SCH_EDITOR_CONTROL::doCrossProbeSchToPcb( const TOOL_EVENT& aEvent, bool aForce )
 {
     // Don't get in an infinite loop SCH -> PCB -> SCH -> PCB -> SCH -> ...
-    if( m_probingSchToPcb )
+    if( m_probingPcbToSch )
-    {
-        m_probingSchToPcb = false;
         return;
-    }
 
     EE_SELECTION_TOOL* selTool = m_toolMgr->GetTool<EE_SELECTION_TOOL>();
     SCH_ITEM*          item = nullptr;

eeschema/tools/sch_editor_control.h

@@ -40,8 +40,8 @@ class SCH_EDITOR_CONTROL : public wxEvtHandler, public EE_TOOL_BASE<SCH_EDIT_FRA
 {
 public:
     SCH_EDITOR_CONTROL()  :
-        EE_TOOL_BASE<SCH_EDIT_FRAME>( "eeschema.EditorControl" ),
+            EE_TOOL_BASE<SCH_EDIT_FRAME>( "eeschema.EditorControl" ),
-        m_probingSchToPcb( false )
+            m_probingPcbToSch( false )
     { }
 
     ~SCH_EDITOR_CONTROL() { }
@@ -124,6 +124,20 @@ public:
 
     void BackAnnotateFootprints( const std::string& aChangedSetOfReferences );
 
+    /**
+     * Finds a component in the schematic and an item in this component.
+     *
+     * @param aReference The component reference designator to find.
+     * @param aSearchHierarchy If false, search the current sheet only.  Otherwise,
+     *                         the entire hierarchy
+     * @param aSearchType A #SCH_SEARCH_T value used to determine what to search for.
+     * @param aSearchText The text to search for, either in value, reference or elsewhere.
+     */
+    SCH_ITEM* FindComponentAndItem( const wxString& aReference,
+                                    bool            aSearchHierarchy,
+                                    SCH_SEARCH_T    aSearchType,
+                                    const wxString& aSearchText );
+
 private:
     ///> copy selection to clipboard
     bool doCopy();
@@ -166,7 +180,7 @@ private:
     void setTransitions() override;
 
 private:
-    bool m_probingSchToPcb;     ///> Recursion guard when cross-probing to PCBNew
+    bool m_probingPcbToSch;     ///> Recursion guard when cross-probing to PCBNew
 };
 
 

pcbnew/cross-probing.cpp

@@ -24,10 +24,7 @@
 
 /**
  * @file pcbnew/cross-probing.cpp
- * @brief Cross probing functions to handle communication to andfrom Eeschema.
+ * @brief Cross probing functions to handle communication to and from Eeschema.
- */
-
-/**
  * Handle messages between Pcbnew and Eeschema via a socket, the port numbers are
  * KICAD_PCB_PORT_SERVICE_NUMBER (currently 4242) (Eeschema to Pcbnew)
  * KICAD_SCH_PORT_SERVICE_NUMBER (currently 4243) (Pcbnew to Eeschema)
@@ -41,7 +38,6 @@
 #include <pcb_edit_frame.h>
 #include <eda_dde.h>
 #include <macros.h>
-
 #include <pcbnew_id.h>
 #include <class_board.h>
 #include <class_module.h>
@@ -53,7 +49,6 @@
 #include <tools/pcb_actions.h>
 #include <tool/tool_manager.h>
 #include <tools/selection_tool.h>
-#include <pcb_draw_panel_gal.h>
 #include <pcb_painter.h>
 
 /* Execute a remote command send by Eeschema via a socket,
@@ -173,8 +168,7 @@ void PCB_EDIT_FRAME::ExecuteRemoteCommand( const char* cmdline )
 
     if( module )
     {
-        m_toolManager->RunAction( PCB_ACTIONS::selectionClear, true );
+        m_toolManager->RunAction( PCB_ACTIONS::highlightItem, true, (void*) module );
-        m_toolManager->RunAction( PCB_ACTIONS::selectItem, true, (void*) module );
         bbox = module->GetBoundingBox();
     }
     else if( netcode > 0 )
@@ -297,7 +291,7 @@ void PCB_EDIT_FRAME::SendMessageToEESCHEMA( BOARD_ITEM* aSyncItem )
 {
     std::string packet = FormatProbeItem( aSyncItem );
 
-    if( packet.size() )
+    if( !packet.empty() )
     {
         if( Kiface().IsSingle() )
             SendCommand( MSG_TO_SCH, packet.c_str() );
@@ -316,7 +310,7 @@ void PCB_EDIT_FRAME::SendCrossProbeNetName( const wxString& aNetName )
 {
     std::string packet = StrPrintf( "$NET: \"%s\"", TO_UTF8( aNetName ) );
 
-    if( packet.size() )
+    if( !packet.empty() )
     {
         if( Kiface().IsSingle() )
             SendCommand( MSG_TO_SCH, packet.c_str() );

pcbnew/tools/pcb_actions.cpp

@@ -594,6 +594,9 @@ TOOL_ACTION PCB_ACTIONS::highlightNetSelection( "pcbnew.EditorControl.highlightN
         _( "Highlight Net" ), _( "Highlight all copper items of a net" ),
         net_highlight_xpm );
 
+TOOL_ACTION PCB_ACTIONS::highlightItem( "pcbnew.EditorControl.highlightItem",
+        AS_GLOBAL );
+
 TOOL_ACTION PCB_ACTIONS::showEeschema( "pcbnew.EditorControl.showEeschema",
         AS_GLOBAL, 0, "",
         _( "Switch to Schematic Editor" ), _( "Open schematic in Eeschema" ),

pcbnew/tools/pcb_actions.h

@@ -411,6 +411,7 @@ public:
     static TOOL_ACTION toggleLastNetHighlight;
     static TOOL_ACTION highlightNetTool;
     static TOOL_ACTION highlightNetSelection;
+    static TOOL_ACTION highlightItem;
     static TOOL_ACTION drillOrigin;
     static TOOL_ACTION appendBoard;
     static TOOL_ACTION showEeschema;

pcbnew/tools/pcb_editor_control.cpp

@@ -1084,10 +1084,7 @@ int PCB_EDITOR_CONTROL::CrossProbePcbToSch( const TOOL_EVENT& aEvent )
 {
     // Don't get in an infinite loop PCB -> SCH -> PCB -> SCH -> ...
     if( m_probingSchToPcb )
-    {
-        m_probingSchToPcb = false;
         return 0;
-    }
 
     SELECTION_TOOL*         selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
     const PCBNEW_SELECTION& selection = selTool->GetSelection();
@@ -1101,6 +1098,23 @@ int PCB_EDITOR_CONTROL::CrossProbePcbToSch( const TOOL_EVENT& aEvent )
 }
 
 
+int PCB_EDITOR_CONTROL::HighlightItem( const TOOL_EVENT& aEvent )
+{
+    BOARD_ITEM* item = aEvent.Parameter<BOARD_ITEM*>();
+
+    m_probingSchToPcb = true;   // recursion guard
+    {
+        m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
+
+        if( item )
+            m_toolMgr->RunAction( PCB_ACTIONS::selectItem, true, (void*) item );
+    }
+    m_probingSchToPcb = false;
+
+    return 0;
+}
+
+
 void PCB_EDITOR_CONTROL::DoSetDrillOrigin( KIGFX::VIEW* aView, PCB_BASE_FRAME* aFrame,
                                            BOARD_ITEM* originViewItem, const VECTOR2D& aPosition )
 {
@@ -1532,6 +1546,7 @@ void PCB_EDITOR_CONTROL::setTransitions()
     Go( &PCB_EDITOR_CONTROL::ClearHighlight,         PCB_ACTIONS::clearHighlight.MakeEvent() );
     Go( &PCB_EDITOR_CONTROL::HighlightNetTool,       PCB_ACTIONS::highlightNetTool.MakeEvent() );
     Go( &PCB_EDITOR_CONTROL::ClearHighlight,         ACTIONS::cancelInteractive.MakeEvent() );
+    Go( &PCB_EDITOR_CONTROL::HighlightItem,          PCB_ACTIONS::highlightItem.MakeEvent() );
 
     Go( &PCB_EDITOR_CONTROL::LocalRatsnestTool,      PCB_ACTIONS::localRatsnestTool.MakeEvent() );
     Go( &PCB_EDITOR_CONTROL::HideDynamicRatsnest,    PCB_ACTIONS::hideDynamicRatsnest.MakeEvent() );

pcbnew/tools/pcb_editor_control.h

@@ -125,6 +125,9 @@ public:
     ///> Launches a tool to pick the item whose net is going to be highlighted.
     int HighlightNetTool( const TOOL_EVENT& aEvent );
 
+    ///> Performs the appropriate action in response to an eeschema cross-probe.
+    int HighlightItem( const TOOL_EVENT& aEvent );
+
     ///> Updates ratsnest for selected items.
     int UpdateSelectionRatsnest( const TOOL_EVENT& aEvent );