remove sisotool dependencies in rlocus

murrayrm · murrayrm · commit 1390e3771f76 · 2023-12-28T12:41:09.000-08:00
diff --git a/control/pzmap.py b/control/pzmap.py
@@ -273,8 +273,12 @@ def pole_zero_plot(
                     label=response.sysname)
 
         # Compute the axis limits to use
-        xlim = (min(xlim[0], response.xlim[0]), max(xlim[1], response.xlim[1]))
-        ylim = (min(ylim[0], response.ylim[0]), max(ylim[1], response.ylim[1]))
+        if response.xlim is not None:
+            xlim = (min(xlim[0], response.xlim[0]),
+                    max(xlim[1], response.xlim[1]))
+        if response.ylim is not None:
+            ylim = (min(ylim[0], response.ylim[0]),
+                    max(ylim[1], response.ylim[1]))
 
     # Set up the limits for the plot
     ax.set_xlim(xlim if xlim_user is None else xlim_user)
diff --git a/control/rlocus.py b/control/rlocus.py
@@ -25,7 +25,6 @@
 from .iosys import isdtime
 from .xferfcn import _convert_to_transfer_function
 from .exception import ControlMIMONotImplemented
-from .sisotool import _SisotoolUpdate
 from .grid import sgrid, zgrid
 from . import config
 import warnings
@@ -76,7 +75,7 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
     ax : :class:`matplotlib.axes.Axes`
         Axes on which to create root locus plot
     initial_gain : float, optional
-        Used by :func:`sisotool` to indicate initial gain.
+        Specify the initial gain to use when marking current gain. [TODO: update]
 
     Returns
     -------
@@ -100,17 +99,12 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
     print_gain = config._get_param(
         'rlocus', 'print_gain', print_gain, _rlocus_defaults)
 
-    # Check for sisotool mode
-    sisotool = kwargs.get('sisotool', False)
-
-    # make sure siso. sisotool has different requirements
-    if not sys.issiso() and not sisotool:
+    if not sys.issiso():
         raise ControlMIMONotImplemented(
             'sys must be single-input single-output (SISO)')
 
-    sys_loop = sys[0,0]
     # Convert numerator and denominator to polynomials if they aren't
-    (nump, denp) = _systopoly1d(sys_loop)
+    nump, denp = _systopoly1d(sys)
 
     # if discrete-time system and if xlim and ylim are not given,
     #  that we a view of the unit circle
@@ -128,31 +122,30 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
         root_array = _RLSortRoots(root_array)
         recompute_on_zoom = False
 
-    if sisotool:
-        start_roots = _RLFindRoots(nump, denp, initial_gain)
-
     # Make sure there were no extraneous keywords
-    if not sisotool and kwargs:
+    if kwargs:
         raise TypeError("unrecognized keywords: ", str(kwargs))
 
     # Create the Plot
     if plot:
-        if sisotool:
-            fig = kwargs['fig']
-            ax = fig.axes[1]
-        else:
-            if ax is None:
-                ax = plt.gca()
-            fig = ax.figure
+        if ax is None:
+            ax = plt.gca()
             ax.set_title('Root Locus')
+        fig = ax.figure
+
+        # TODO: get rid of extra variable start_roots
+        if initial_gain is not None:
+            start_roots = _RLFindRoots(nump, denp, initial_gain)
+        else:
+            start_roots = None
 
-        if print_gain and not sisotool:
+        if print_gain and start_roots is None:
             fig.canvas.mpl_connect(
                 'button_release_event',
                 partial(_RLClickDispatcher, sys=sys, fig=fig,
-                        ax_rlocus=fig.axes[0], plotstr=plotstr))
-        elif sisotool:
-            fig.axes[1].plot(
+                        ax_rlocus=ax, plotstr=plotstr))
+        elif start_roots is not None:
+            ax.plot(
                 [root.real for root in start_roots],
                 [root.imag for root in start_roots],
                 marker='s', markersize=6, zorder=20, color='k', label='gain_point')
@@ -165,14 +158,6 @@ def root_locus(sys, kvect=None, xlim=None, ylim=None,
                 "Clicked at: %10.4g%+10.4gj  gain: %10.4g  damp: %10.4g" %
                 (s.real, s.imag, initial_gain, zeta),
                 fontsize=12 if int(mpl.__version__[0]) == 1 else 10)
-            fig.canvas.mpl_connect(
-                'button_release_event',
-                partial(_RLClickDispatcher, sys=sys, fig=fig,
-                        ax_rlocus=fig.axes[1], plotstr=plotstr,
-                        sisotool=sisotool,
-                        bode_plot_params=kwargs['bode_plot_params'],
-                        tvect=kwargs['tvect']))
-
 
         if recompute_on_zoom:
             # update gains and roots when xlim/ylim change. Only then are
@@ -526,7 +511,7 @@ def _RLZoomDispatcher(event, sys, ax_rlocus, plotstr):
                        scalex=False, scaley=False)
 
 
-def _RLClickDispatcher(event, sys, fig, ax_rlocus, plotstr, sisotool=False,
+def _RLClickDispatcher(event, sys, fig, ax_rlocus, plotstr,
                        bode_plot_params=None, tvect=None):
     """Rootlocus plot click dispatcher"""
 
@@ -536,15 +521,14 @@ def _RLClickDispatcher(event, sys, fig, ax_rlocus, plotstr, sisotool=False,
        {'zoom rect', 'pan/zoom'}:
 
         # if a point is clicked on the rootlocus plot visually emphasize it
-        K = _RLFeedbackClicksPoint(event, sys, fig, ax_rlocus, sisotool)
-        if sisotool and K is not None:
-            _SisotoolUpdate(sys, fig, K, bode_plot_params, tvect)
+        K = _RLFeedbackClicksPoint(
+            event, sys, fig, ax_rlocus, show_clicked=False)
 
     # Update the canvas
     fig.canvas.draw()
 
 
-def _RLFeedbackClicksPoint(event, sys, fig, ax_rlocus, sisotool=False):
+def _RLFeedbackClicksPoint(event, sys, fig, ax_rlocus, show_clicked=False):
     """Display root-locus gain feedback point for clicks on root-locus plot"""
     sys_loop = sys[0,0]
     (nump, denp) = _systopoly1d(sys_loop)
@@ -591,16 +575,20 @@ def _RLFeedbackClicksPoint(event, sys, fig, ax_rlocus, sisotool=False):
         # Remove the previous line
         _removeLine(label='gain_point', ax=ax_rlocus)
 
-        # Visualise clicked point, display all roots for sisotool mode
-        if sisotool:
+        if show_clicked:
+            # Visualise clicked point, display all roots
             root_array = _RLFindRoots(nump, denp, K.real)
             ax_rlocus.plot(
                 [root.real for root in root_array],
                 [root.imag for root in root_array],
-                marker='s', markersize=6, zorder=20, label='gain_point', color='k')
+                marker='s', markersize=6, zorder=20, label='gain_point',
+                color='k')
         else:
-            ax_rlocus.plot(s.real, s.imag, 'k.', marker='s', markersize=8,
-                           zorder=20, label='gain_point')
+            # Just show the clicked point
+            # TODO: should we keep this?
+            ax_rlocus.plot(
+                s.real, s.imag, 'k.', marker='s', markersize=8, zorder=20,
+                label='gain_point')
 
         return K.real
 
diff --git a/control/sisotool.py b/control/sisotool.py
@@ -1,5 +1,10 @@
 __all__ = ['sisotool', 'rootlocus_pid_designer']
 
+import numpy as np
+import matplotlib.pyplot as plt
+import warnings
+from functools import partial
+
 from control.exception import ControlMIMONotImplemented
 from .freqplot import bode_plot
 from .timeresp import step_response
@@ -11,9 +16,6 @@
 from .nlsys import interconnect
 from control.statesp import _convert_to_statespace
 from . import config
-import numpy as np
-import matplotlib.pyplot as plt
-import warnings
 
 _sisotool_defaults = {
     'sisotool.initial_gain': 1
@@ -123,13 +125,39 @@ def sisotool(sys, initial_gain=None, xlim_rlocus=None, ylim_rlocus=None,
     initial_gain = config._get_param('sisotool', 'initial_gain',
             initial_gain, _sisotool_defaults)
 
-    # First time call to setup the bode and step response plots
+    # First time call to setup the Bode and step response plots
     _SisotoolUpdate(sys, fig, initial_gain, bode_plot_params)
 
-    # Setup the root-locus plot window
-    root_locus(sys, initial_gain=initial_gain, xlim=xlim_rlocus,
+    root_locus(
+        sys[0, 0], initial_gain=initial_gain, xlim=xlim_rlocus,
         ylim=ylim_rlocus, plotstr=plotstr_rlocus, grid=rlocus_grid,
-        fig=fig, bode_plot_params=bode_plot_params, tvect=tvect, sisotool=True)
+        ax=fig.axes[1])
+
+    # Reset the button release callback so that we can update all plots
+    fig.canvas.mpl_connect(
+        'button_release_event',
+        partial(_click_dispatcher, sys=sys, fig=fig,
+                ax_rlocus=fig.axes[1], plotstr=plotstr_rlocus,
+                bode_plot_params=bode_plot_params, tvect=tvect))
+
+
+def _click_dispatcher(event, sys, fig, ax_rlocus, plotstr,
+                      bode_plot_params=None, tvect=None):
+    from .rlocus import _RLFeedbackClicksPoint
+
+    # Zoom is handled by specialized callback in rlocus, only handle gain plot
+    if event.inaxes == ax_rlocus.axes and \
+       plt.get_current_fig_manager().toolbar.mode not in \
+       {'zoom rect', 'pan/zoom'}:
+        # if a point is clicked on the rootlocus plot visually emphasize it
+        K = _RLFeedbackClicksPoint(
+            event, sys, fig, ax_rlocus, show_clicked=True)
+        if K is not None:
+            _SisotoolUpdate(sys, fig, K, bode_plot_params, tvect)
+
+    # Update the canvas
+    fig.canvas.draw()
+
 
 def _SisotoolUpdate(sys, fig, K, bode_plot_params, tvect=None):
 
diff --git a/control/tests/pzmap_test.py b/control/tests/pzmap_test.py
@@ -60,7 +60,7 @@ def test_pzmap(kwargs, setdefaults, dt, editsdefaults, mplcleanup):
         fig, ax = plt.gcf(), plt.gca()
 
         assert fig._suptitle.get_text().startswith(
-            kwargs.get('title', 'Pole/zero map'))
+            kwargs.get('title', 'Pole/zero plot'))
 
         # FIXME: This won't work when zgrid and sgrid are unified
         children = ax.get_children()
diff --git a/control/tests/sisotool_test.py b/control/tests/sisotool_test.py
@@ -7,7 +7,7 @@
 import pytest
 
 from control.sisotool import sisotool, rootlocus_pid_designer
-from control.rlocus import _RLClickDispatcher
+from control.sisotool import _click_dispatcher
 from control.xferfcn import TransferFunction
 from control.statesp import StateSpace
 from control import c2d
@@ -93,8 +93,8 @@ def test_sisotool(self, tsys):
         event = type('test', (object,), {'xdata': 2.31206868287,
                                          'ydata': 15.5983051046,
                                          'inaxes': ax_rlocus.axes})()
-        _RLClickDispatcher(event=event, sys=tsys, fig=fig,
-                           ax_rlocus=ax_rlocus, sisotool=True, plotstr='-',
+        _click_dispatcher(event=event, sys=tsys, fig=fig,
+                           ax_rlocus=ax_rlocus, plotstr='-',
                            bode_plot_params=bode_plot_params, tvect=None)
 
         # Check the moved root locus plot points
@@ -143,8 +143,8 @@ def test_sisotool_tvect(self, tsys):
         event = type('test', (object,), {'xdata': 2.31206868287,
                                          'ydata': 15.5983051046,
                                          'inaxes': ax_rlocus.axes})()
-        _RLClickDispatcher(event=event, sys=tsys, fig=fig,
-                           ax_rlocus=ax_rlocus, sisotool=True, plotstr='-',
+        _click_dispatcher(event=event, sys=tsys, fig=fig,
+                           ax_rlocus=ax_rlocus, plotstr='-',
                            bode_plot_params=dict(), tvect=tvect)
         assert_array_almost_equal(tvect, ax_step.lines[0].get_data()[0])
 
