improvements to is_active ping logic (spacetelescope#2450)

* add to skip list before running method - fixes case where method call taking longer than ping timeout results in flashing
* ping test coverage

CHANGES.rst

@@ -104,6 +104,8 @@ Specviz2d
 Other Changes and Additions
 ---------------------------
 
+- Improved logic for handling active state of plugins. [#2386, #2450]
+
 - API framework for batch aperture photometry. [#2401]
 
 3.6.3 (unreleased)

jdaviz/core/template_mixin.py

@@ -237,9 +237,11 @@ def wrapper(self, msg={}):
             # toggles before any *other* messages are received)
             # if the method returns False, then the method is not considered to have fully run
             # and so is NOT added to the skip list
-            ret_ = meth(self, msg)
-            if ret_ is not False and meth.__name__ not in self._methods_skip_since_last_active:  # noqa
+            if meth.__name__ not in self._methods_skip_since_last_active:
                 self._methods_skip_since_last_active.append(meth.__name__)
+            ret_ = meth(self, msg)
+            if ret_ is False:
+                self._methods_skip_since_last_active.remove(meth.__name__)
             return ret_
 
         return wrapper
@@ -262,6 +264,11 @@ def __init__(self, **kwargs):
         # _inactive_thread: thread checking for alive pings to control plugin_opened
         self._inactive_thread = None
         self._ping_timestamp = 0
+        # _ping_delay_ms should match value in setTimeout in tray_plugin.vue
+        # NOTE: could control with a traitlet, but then would need to pass through each
+        # <j-tray-plugin> component
+        self._ping_delay_ms = 200
+
         # _methods_skip_since_last_active: methods that should be skipped when is_active is next
         # set to True because no changes have been made.  This can be used to prevent queuing
         # of expensive method calls, especially when the browser throttles the ping resulting
@@ -298,16 +305,12 @@ def vue_plugin_ping(self, ping_timestamp):
         self._inactive_thread.start()
 
     def _watch_active(self):
-        # expected_delay_ms should match value in setTimeout in tray_plugin.vue
-        # NOTE: could control with a traitlet, but then would need to pass through each
-        # <j-tray-plugin> component
-        expected_delay_ms = 200
         # plugin_ping (ms) set by setTimeout in tray_plugin.vue
         # time.time() is in s, so need to convert to ms
-        while time.time()*1000 - self._ping_timestamp < 2 * expected_delay_ms:
+        while time.time()*1000 - self._ping_timestamp < 2 * self._ping_delay_ms:
             # at least one plugin has sent an "alive" ping within twice of the expected
             # interval, wait a full (double) interval and then check again
-            time.sleep(2 * expected_delay_ms / 1000)
+            time.sleep(2 * self._ping_delay_ms / 1000)
 
         # "alive" ping has not been received within the expected time, consider all instances
         # of the plugin to be closed

jdaviz/tests/test_plugin_is_active.py

@@ -0,0 +1,66 @@
+from traitlets import Bool, Float, observe
+from time import sleep
+
+from jdaviz import Cubeviz
+from jdaviz.app import Application
+from jdaviz.core.config import get_configuration
+from jdaviz.core.registries import tray_registry
+from jdaviz.core.template_mixin import PluginTemplateMixin, skip_if_no_updates_since_last_active
+
+
+@tray_registry('test-slow-plugin', label="Test Slow Plugin")
+class SlowPlugin(PluginTemplateMixin):
+    """
+    empty plugin with a simulated slow method
+    """
+    template = ''
+    uses_active_status = Bool(True).tag(sync=True)
+    sleep_time = Float(0).tag(sync=True)
+
+    def __init__(self, *args, **kwargs):
+        # reset this manually in the test to see if slow_method ran since last manual reset
+        self._method_ran = False
+        super().__init__(*args, **kwargs)
+
+    @observe('is_active', 'sleep_time')
+    @skip_if_no_updates_since_last_active()
+    def slow_method(self, *args):
+        self._method_ran = True
+        sleep(self.sleep_time)
+        return
+
+
+def test_is_active():
+    # create an instance of an app with just our fake "Slow" Plugin
+    config = get_configuration('cubeviz')
+    config['tray'] = ['test-slow-plugin']
+
+    cubeviz_app = Application(config)
+    cubeviz_helper = Cubeviz(cubeviz_app)
+
+    plg = cubeviz_helper.plugins['Test Slow Plugin']._obj
+    plg.open_in_tray()
+    assert plg.uses_active_status is True
+    # there is no actual frontend here to ping (this would be True in a notebook)
+    assert plg.is_active is False
+
+    # so we need to fake the pings
+    assert plg._ping_timestamp == 0
+    assert plg.sleep_time == 0
+    plg.vue_plugin_ping({})
+    # since the observe on is_active/sleep_time returns immediately, the plugin is active
+    assert plg.is_active is True
+    assert plg._ping_timestamp > 0
+    assert plg._method_ran is True
+
+    # if there are no pings then the plugin will reset to inactive
+    sleep(1)
+    assert plg.is_active is False
+
+    # if we set sleep_time while the plugin is inactive, the method should not fire
+    plg._method_ran = False
+    plg.sleep_time = 0.01
+    assert plg._method_ran is False
+    # but should immediately once it becomes active
+    plg.vue_plugin_ping({})
+    assert plg._method_ran is True