Merge pull request #1219 from tangkong/bug_lp_thread_overschedule

BUG: Prevent lightpath logic from over-scheduling threads

docs/source/upcoming_release_notes/1219-bug_lp_thread_overschedule.rst

@@ -0,0 +1,33 @@
+1219 bug_lp_thread_overschedule
+###############################
+
+API Breaks
+----------
+- N/A
+
+Features
+--------
+- N/A
+
+Device Updates
+--------------
+- N/A
+
+New Devices
+-----------
+- N/A
+
+Bugfixes
+--------
+- Prevent some devices from creating threads at high frequency when
+  trying to get the lightpath state.  These devices classes include
+  `XOffsetMirrorXYState`, `AttenuatorSXR_Ladder`,
+  `AttenuatorSXR_LadderTwoBladeLBD`, `AT2L0`, `XCSLODCM`, and `XPPLODCM`
+
+Maintenance
+-----------
+- N/A
+
+Contributors
+------------
+- tangkong

pcdsdevices/attenuator.py

@@ -1078,14 +1078,18 @@ def calc_lightpath_state(self, **lightpath_kwargs) -> LightpathState:
             obj = getattr(self, sig_name).state
             if not obj._state_initialized:
                 # This would prevent make check_inserted, etc. fail
-                utils.schedule_task(self._calc_cache_lightpath_state,
-                                    delay=2.0)
+                if self._retry_lightpath:
+                    self._retry_lightpath = False
+                    utils.schedule_task(self._calc_cache_lightpath_state,
+                                        delay=2.0)
 
                 return LightpathState(
                     inserted=True,
                     removed=True,
                     output={self.output_branches[0]: 1}
                 )
+
+            self._retry_lightpath = True
             # get state of the InOutPositioner and check status
             in_check.append(obj.check_inserted(sig_value))
             out_check.append(obj.check_removed(sig_value))
@@ -1245,14 +1249,18 @@ def calc_lightpath_state(self, **lightpath_kwargs) -> LightpathState:
             obj = getattr(self, sig_name).state
             if not obj._state_initialized:
                 # This would prevent make check_inserted, etc. fail
-                utils.schedule_task(self._calc_cache_lightpath_state,
-                                    delay=2.0)
+                if self._retry_lightpath:
+                    self._retry_lightpath = False
+                    utils.schedule_task(self._calc_cache_lightpath_state,
+                                        delay=2.0)
 
                 return LightpathState(
                     inserted=True,
                     removed=True,
                     output={self.output_branches[0]: 1}
                 )
+
+            self._retry_lightpath = True
             # get state of the InOutPositioner and check status
             in_check.append(obj.check_inserted(sig_value))
             out_check.append(obj.check_removed(sig_value))
@@ -1593,14 +1601,18 @@ def calc_lightpath_state(self, **lightpath_kwargs) -> LightpathState:
             obj = getattr(self, sig_name).state
             if not obj._state_initialized:
                 # This would prevent make check_inserted, etc. fail
-                utils.schedule_task(self._calc_cache_lightpath_state,
-                                    delay=2.0)
+                if self._retry_lightpath:
+                    self._retry_lightpath = False
+                    utils.schedule_task(self._calc_cache_lightpath_state,
+                                        delay=2.0)
 
                 return LightpathState(
                     inserted=True,
                     removed=True,
                     output={self.output_branches[0]: 1}
                 )
+
+            self._retry_lightpath = True
             # get state of the InOutPositioner and check status
             in_check.append(obj.check_inserted(sig_value))
             out_check.append(obj.check_removed(sig_value))

pcdsdevices/interface.py

@@ -1700,7 +1700,7 @@ def calc_lightpath_state(self, sig1=None, sig2=None):
     def __init__(self, *args,
                  input_branches=[], output_branches=[], **kwargs):
         self._lightpath_ready = False
-        self._retry_lightpath = False
+        self._retry_lightpath = True
         self._summary_initialized = False
         self._cached_state = None
         self._md = None
@@ -1817,15 +1817,18 @@ def calc_lightpath_state(self, state) -> LightpathState:
             # and queue up the calculation for later
             self.log.debug('state not initialized, scheduling '
                            'lightpath calculations for later')
-            utils.schedule_task(self._calc_cache_lightpath_state,
-                                delay=self.retry_delay)
+            if self._retry_lightpath:
+                self._retry_lightpath = False
+                utils.schedule_task(self._calc_cache_lightpath_state,
+                                    delay=self.retry_delay)
 
             return LightpathState(
                 inserted=True,
                 removed=True,
                 output={self.output_branches[0]: 1}
             )
 
+        self._retry_lightpath = True
         self._inserted = self.check_inserted(state)
         self._removed = self.check_removed(state)
         self._transmission = self.check_transmission(state)
@@ -1886,15 +1889,19 @@ def calc_lightpath_state(self, **lightpath_kwargs):
                 # and queue up the calculation for later
                 self.log.debug('state not initialized, scheduling '
                                'lightpath calculations for later')
-                utils.schedule_task(self._calc_cache_lightpath_state,
-                                    delay=self.retry_delay)
+                if self._retry_lightpath:
+                    self._retry_lightpath = False
+                    utils.schedule_task(self._calc_cache_lightpath_state,
+                                        delay=self.retry_delay)
 
                 return LightpathState(
                     inserted=True,
                     removed=True,
                     output={self.output_branches[0]: 1}
                 )
 
+            self._retry_lightpath = True
+
             # get state of the InOutPositioner and check status
             in_check.append(obj.check_inserted(sig_value))
             out_check.append(obj.check_removed(sig_value))

pcdsdevices/lodcm.py

@@ -2110,12 +2110,17 @@ def calc_lightpath_state(
         if not self.tower1.h1n_state._state_initialized:
             self.log.debug('tower1 state not initialized, scheduling '
                            'lightpath calc for later')
-            schedule_task(self._calc_cache_lightpath_state, delay=2.0)
+            if self._retry_lightpath:
+                self._retry_lightpath = False
+                schedule_task(self._calc_cache_lightpath_state, delay=2.0)
+
             return LightpathState(
                 inserted=True,
                 removed=True,
                 output={'L0': 0}
             )
+
+        self._retry_lightpath = True
         # avoid extra get calls in this method
         state_label = self.tower1.h1n_state.get_state(h1n_state).name
 
@@ -2177,13 +2182,16 @@ def calc_lightpath_state(
         if not self.tower1.h1n_state._state_initialized:
             self.log.debug('tower1 state not initialized, scheduling '
                            'lightpath calc for later')
-            schedule_task(self._calc_cache_lightpath_state, delay=2.0)
+            if self._retry_lightpath:
+                self._retry_lightpath = False
+                schedule_task(self._calc_cache_lightpath_state, delay=2.0)
             return LightpathState(
                 inserted=True,
                 removed=True,
                 output={'L0': 0}
             )
 
+        self._retry_lightpath = True
         inserted = self.tower1.h1n_state.check_inserted(h1n_state)
         removed = self.tower1.h1n_state.check_removed(h1n_state)
 

pcdsdevices/mirror.py

@@ -1461,6 +1461,9 @@ class XOffsetMirrorXYState(XOffsetMirrorState):
     lightpath_cpts = ['insertion.state', 'coating.state',
                       'pitch.user_readback']
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     def _get_insertion_state(
         self,
         insertion_state: int
@@ -1487,10 +1490,12 @@ def _get_insertion_state(
         if not self.insertion._state_initialized:
             self.log.debug('insertion state not initialized, '
                            'scheduling lightpath calcs for later')
+            if self._retry_lightpath:
+                self._retry_lightpath = False
+                schedule_task(self._calc_cache_lightpath_state, delay=2.0)
+            return True, True
 
-            schedule_task(self._calc_cache_lightpath_state, delay=2.0)
-            raise MirrorLogicError('insertion state not initialized')
-
+        self._retry_lightpath = True
         x_in = self.insertion.check_inserted(insertion_state)
         x_out = self.insertion.check_removed(insertion_state)
 

pcdsdevices/tests/test_mirror.py

@@ -4,8 +4,10 @@
 import pytest
 from ophyd.sim import ReadOnlyError, make_fake_device
 
+from pcdsdevices import mirror
+
 from ..mirror import (KBOMirror, OffsetMirror, PointingMirror,
-                      XOffsetMirrorStateCool)
+                      XOffsetMirrorStateCool, XOffsetMirrorXYState)
 
 
 @pytest.fixture(scope='function')
@@ -145,6 +147,17 @@ def fake_offset_cooled_mirror():
     return FakeCooledOffsetMirror('TST:MR1', name="Test Mirror")
 
 
+@pytest.fixture(scope='function')
+def fake_xy_offset_mirror():
+    FakeCooledOffsetMirror = make_fake_device(XOffsetMirrorXYState)
+    fake_mirror = FakeCooledOffsetMirror('TST:MR1', name="Test Mirror")
+    # do lightpath setup
+    fake_mirror._init_summary_signal()
+    fake_mirror.output_branches = ['L0', 'L1']
+    fake_mirror.input_branches = ['L0']
+    return fake_mirror
+
+
 def test_kbomirror_lighpath(fake_kbo_mirror):
     km = fake_kbo_mirror
     lp_state = km.get_lightpath_state()
@@ -179,3 +192,27 @@ def test_mirror_cooling(fake_offset_cooled_mirror):
 
     with pytest.raises(ReadOnlyError):
         om.cool_press.put(0.34)
+
+
+def test_xy_mirror_lightpath(fake_xy_offset_mirror, monkeypatch):
+    xym = fake_xy_offset_mirror
+    mock_schedule = Mock()
+    monkeypatch.setattr(mirror, 'schedule_task', mock_schedule)
+
+    assert xym._retry_lightpath is True
+    xym.insertion._state_initialized = False
+    # try getting lightpath state once, which should fail and schedule
+    xym.get_lightpath_state(use_cache=False)
+    assert mock_schedule.call_count == 1
+    assert xym._retry_lightpath is False
+
+    # subsequent calls to get_lightpath_state should not schedule
+    xym.get_lightpath_state(use_cache=False)
+    assert mock_schedule.call_count == 1
+    assert xym._retry_lightpath is False
+
+    xym._retry_lightpath = True
+    # After reset has completed, can retry
+    xym.get_lightpath_state(use_cache=False)
+    assert mock_schedule.call_count == 2
+    assert xym._retry_lightpath is False