Repository: bluesky/bluesky (python)

Issue:
check_limits() should call objects' check_value() method
The `bluesky.simulators.check_limits()` needs to be revised.

## Expected Behavior

The `bluesky.simulators.check_limits()` acts on `set` messages in a plan, checking that the position will be within the limits allowed by the object.

## Current Behavior

The code now gets the `.limits` tuple from the object and does a comparison of position within those limits.  It raises an exception if there is a problem.  https://github.com/bluesky/bluesky/blob/09bd73bd05ac353ae9b2cc4ea4f015c8b934e658/bluesky/simulators.py#L83-L115

This is a problem for many objects, as noted by this GitHub search: https://github.com/search?p=1&q=org%3Abluesky+check_limits&type=Issues

Notably, the [*hklpy/diffract/Diffractometer*](https://github.com/bluesky/hklpy/pull/45) object has two types of positioner, pseudo and real.  The real positioners derive their limits from real positioners, such as EPICS motors.  The pseudo-positioners are reciprocal-space terms.  For these terms, the limits are computed dynamically, based on current values of all the real positioners.  Thus, the simple code implemented in  `bluesky.simulators.check_limits()` [fails](https://github.com/bluesky/hklpy/issues/13) for bluesky plans that position `Diffractometer` pseudo-axes.

## Possible Solution

The `check_limits` code should call each objects `check_value(pos)` method.  Objects with special needs should override the base class `check_value()` method and raise exceptions if there is a problem.

The new code will be much simpler:

```
def check_limits(plan):
    """
    Check that a bluesky plan will not move devices outside of their limits.

    Parameters
    ----------
    plan : iterable
        Must yield `Msg` objects
    """
    for msg in plan:
        if msg.command == 'set':
            msg.obj.check_value(msg.args[0])
```

## Steps to Reproduce (for bugs)
see various GH issues as noted: https://github.com/search?p=1&q=org%3Abluesky+check_limits&type=Issues

## Context

The impact of this change is important.  One of the selling points of the Bluesky framework is the ability to inspect a plan before executing it.  Some of the [issues noted above](https://github.com/search?p=1&q=org%3Abluesky+check_limits&type=Issues) propose adding a `check_limits()` call as a routine practice.

## Your Environment

```
In [25]: RE.md["versions"]                                                                                                                                    
Out[25]: 
{'apstools': '1.3.7+43.g59c4610',
 'bluesky': '1.6.5',
 'databroker': '1.0.6',
 'epics': '3.4.2',
 'event_model': '1.15.2',
 'matplotlib': '3.2.2',
 'numpy': '1.19.1',
 'ophyd': '1.5.2',
 'pyRestTable': '2020.0.2',
 'spec2nexus': '2021.1.7'}
```

Pull request:
check limits should call objects' check_value() method
Fixes #1411 which provides details.

## Description
Revise to call each object's `check_value(pos)` method.  Removed local check of `msg.obj.limits`.

Note that `check_limits()` cannot know if same limits are a problem.  Also it cannot know if limits are exceeded if the object does not raise.

## Motivation and Context
see #1411

## How Has This Been Tested?
Tested this code with the `hklpy.diffract.Diffractometer` class.  Other projects may also need to be tested with this code.


Patch:
diff --git a/bluesky/simulators.py b/bluesky/simulators.py
index e22eda4d..7a70e640 100644
--- a/bluesky/simulators.py
+++ b/bluesky/simulators.py
@@ -76,10 +76,6 @@ def summarize_plan(plan):
 print_summary = summarize_plan  # back-compat
 
 
-class LimitsExceeded(Exception):
-    ...
-
-
 def check_limits(plan):
     """
     Check that a plan will not move devices outside of their limits.
@@ -88,28 +84,13 @@ def check_limits(plan):
     ----------
     plan : iterable
         Must yield `Msg` objects
-
-    Raises
-    ------
-    LimitsExceeded
     """
     ignore = []
     for msg in plan:
-        if msg.command == 'set':
-            if msg.obj in ignore:
-                continue  # we have already warned about this device
-            try:
-                low, high = msg.obj.limits
-            except AttributeError:
-                warn("Limits of {} are unknown and can't be checked.".format(msg.obj.name))
-                ignore.append(msg.obj)
-                continue
-            if low == high:
-                warn("Limits are not set on {}".format(msg.obj.name))
+        if msg.command == 'set' and msg.obj not in ignore:
+            if hasattr(msg.obj, "check_value"):
+                msg.obj.check_value(msg.args[0])
+            else:
+                warn(f"{msg.obj.name} has no check_value() method"
+                     f" to check if {msg.args[0]} is within its limits.")
                 ignore.append(msg.obj)
-                continue
-            pos, = msg.args
-            if not (low < pos < high):
-                raise LimitsExceeded("This plan would put {} at {} "
-                                     "which is outside of its limits, {}."
-                                     "".format(msg.obj.name, pos, (low, high)))


Test patch:
diff --git a/bluesky/tests/test_simulators.py b/bluesky/tests/test_simulators.py
index c3aa7ada..dd033a3b 100644
--- a/bluesky/tests/test_simulators.py
+++ b/bluesky/tests/test_simulators.py
@@ -1,7 +1,7 @@
 from bluesky.plans import scan
 from bluesky.simulators import (print_summary, print_summary_wrapper,
                                 summarize_plan,
-                                check_limits, LimitsExceeded,
+                                check_limits,
                                 plot_raster_path)
 import pytest
 from bluesky.plans import grid_scan
@@ -38,23 +38,24 @@ def test_check_limits(hw):
     # Use an assert to help us out if this changes in the future.
     assert not hasattr(motor, 'limits')
 
-    # check_limits should warn if it can't find limits
-    with pytest.warns(UserWarning):
-        check_limits(scan([det], motor, -1, 1, 3))
+    # # check_limits should warn if it can't find check_value
+    # TODO: Is there _any_ object to test?
+    # with pytest.warns(UserWarning):
+    #     check_limits(scan([det], motor, -1, 1, 3))
 
     # monkey-patch some limits
     motor.limits = (-2, 2)
     # check_limits should do nothing here
     check_limits(scan([det], motor, -1, 1, 3))
 
-    # check_limits should error if limits are exceeded
-    with pytest.raises(LimitsExceeded):
-        check_limits(scan([det], motor, -3, 3, 3))
+    # check_limits should error if limits are exceeded only if object raises
+    # this object does not raise
+    check_limits(scan([det], motor, -3, 3, 3))
 
-    # check_limits should warn if limits are equal
+    # check_limits should raise if limits are equal only if object raises
+    # this object does not raise
     motor.limits = (2, 2)
-    with pytest.warns(UserWarning):
-        check_limits(scan([det], motor, -1, 1, 3))
+    check_limits(scan([det], motor, -1, 1, 3))
 
 
 def test_plot_raster_path(hw):