GeezerVison

Coverage increased (+1.0%) to 78.881% when pulling 70ec8be on icam0:sisotool_final into 601b581 on python-control:master.

Coverage decreased (-0.05%) to 78.222% when pulling 17c4a95 on icam0:sisotool_final into 1254ccb on python-control:master.

Coverage increased (+1.0%) to 78.881% when pulling 70ec8be on icam0:sisotool_final into 601b581 on python-control:master.

I tested locally, and this works fine!

Very nice tool. I tried it out on Python 3.5 on MacOS 10.13 (High Sierra) and it worked well.

If someone can test this on Linux (and Windows?) and confirm that it works, we can incorporate into the next release.

I tried out the tool and it works most of the time. But there can be problems if changing the gain if the scale of the plot is too large. For example, try the following example:

import control
sys = control.tf([10,100], [1,5,9,5])
control.sisotool(sys)

This will generate a matplotlib window and you can click on the root locus branches on the right hand side to change the gain. But if you try to click on the left branch (on the negative real axis), it doesn't get recognized.

I think the problem is that the scaling parameters that are used to determine if you are close enough to the axis don't work for the scaling on this plot.

@icam0 If there is no easy fix, we can merge this change as is and update later. I'll wait for a few days to get your input.

I suspect the gain tolerance to be too small for some reason (lines 455 and 457 of the rlocus.py file). I will get back to you during the weekend when I take a closer look at this.

I have looked into it, what happens is that because this plot generates a very skewed aspect ratio due to the automatic matplotlib scaling of the plot the left side of the plot 'does not seem to work'. This is due to the fact that I made a decision to vary the gain tolerance according to the zoom and xlimit and ylimit of the plot. It selects the smallest of these 2 and bases the gain tolerance on the smallest of these 2 margins. When having a skewed aspect ratio it makes it hard for the user to click because the margin becomes very small. In the original implementation, there was a 'fixed' 0.04 factor in there which meant that if you would zoom in a lot, you could click very far away from the plot resulting in floating dots far away from the plot line. Using a scaled gain factor fixed this problem.

If you were to just increase the gain tolerance the behaviour becomes more unreliable and the point sometimes snaps to the incorrect line from then a user would intend. Therefore, for now, I'd like to propose to scale axes of the root locus plot to the same length of the smallest axis. Which makes sure the initial plot is always clickable. Something like this would then be added to the end of the rlocus.py plot (an edge case for handling when the user specifies the xlim or ylim would still need to be added):

x0, x1 = ax.get_xlim()
y0, y1 = ax.get_ylim()
if abs(x0-x1) >= abs(y0-y1):
    ax.set_xlim(0. - 0.5*abs(y0-y1) ,0. +0.5*abs(y0-y1))
else:
    ax.set_ylim(0. - 0.5 * abs(x0 - x1), 0. + 0.5 * abs(x0 - x1))

However, when the user zooms in with a very skewed aspect ratio clicking the line would still be very difficult which is why I think in the future a more sophisticated algorithm for click detection would be a worthwhile enhancement. I'd like to get your thoughts on this @murrayrm

@icam0 Can you rebase this against the current master and resolve the conflicts. I think this is ready to go into release 0.8.2 as soon as that is done.

Did an inline correction of the conflicts. Will merge if tests run correctly.

Fixed up remaining issues. Ready to merge.