mirror of
https://github.com/bec-project/bec_widgets.git
synced 2026-11-10 19:44:04 +01:00
fix(plots): drop deferred timers of closed plot widgets
This commit is contained in:
1 parent
929657538a
commit
a1f3730c60
14 files changed
+250
-10
No files matched your search
@@ -977,6 +977,14 @@ class Crosshair(QObject):
|
||||
self.clear_markers()
|
||||
|
||||
def cleanup(self):
|
||||
# The plot's scene outlives the crosshair: detach from its mouse signals first, so
|
||||
# no mouse move queued in the rate-limiting proxy, and no click, reaches it afterwards.
|
||||
if self.proxy is not None: # None once an earlier cleanup() detached it
|
||||
self.proxy.disconnect()
|
||||
self.proxy = None
|
||||
scene = self.plot_item.scene()
|
||||
if scene is not None:
|
||||
scene.sigMouseClicked.disconnect(self.mouse_clicked)
|
||||
self.reset()
|
||||
self.clear_pin()
|
||||
self.plot_item.removeItem(self.v_line)
|
||||
|
||||
@@ -7,7 +7,6 @@ import numpy as np
|
||||
from bec_lib import bec_logger
|
||||
from bec_lib.endpoints import MessageEndpoints
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
from qtpy.QtCore import QTimer
|
||||
from qtpy.QtWidgets import QWidget
|
||||
|
||||
from bec_widgets.utils.bec_connector import ConnectionConfig
|
||||
@@ -172,12 +171,12 @@ class Image(ImageBase):
|
||||
]
|
||||
)
|
||||
|
||||
QTimer.singleShot(0, self._adjust_and_connect)
|
||||
self._call_later(0, self._adjust_and_connect)
|
||||
|
||||
def _adjust_and_connect(self):
|
||||
"""
|
||||
Sync the device selection toolbar with current properties.
|
||||
Has to be done with QTimer.singleShot to ensure the UI is fully initialized, needed for testing.
|
||||
Has to be deferred to the event loop to ensure the UI is fully initialized, needed for testing.
|
||||
|
||||
Note: DeviceComboBox and SignalComboBox auto-populate themselves, no manual population needed.
|
||||
"""
|
||||
|
||||
@@ -854,6 +854,20 @@ class MotorMap(PlotBase):
|
||||
data = {"x": self._buffer["x"], "y": self._buffer["y"]}
|
||||
return data
|
||||
|
||||
################################################################################
|
||||
# Cleanup
|
||||
################################################################################
|
||||
def cleanup(self):
|
||||
"""
|
||||
Cleanup the widget by detaching the plot update proxy and closing the settings popup.
|
||||
"""
|
||||
# Detach the update proxy first, so no queued update reaches the closed plot.
|
||||
self.proxy_update_plot.disconnect()
|
||||
if self.motor_map_settings is not None:
|
||||
self.motor_map_settings.reject()
|
||||
self.motor_map_settings = None
|
||||
super().cleanup()
|
||||
|
||||
|
||||
class DemoApp(QMainWindow): # pragma: no cover
|
||||
def __init__(self):
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Iterable, Mapping
|
||||
from collections.abc import Callable, Iterable, Mapping
|
||||
from enum import Enum
|
||||
from typing import Literal, TypeAlias
|
||||
|
||||
@@ -9,7 +9,7 @@ import pyqtgraph as pg
|
||||
from bec_lib import bec_logger
|
||||
from bec_lib.scan_data_container import ScanDataContainer
|
||||
from bec_lib.scan_items import ScanItem
|
||||
from qtpy.QtCore import QPoint, QPointF, Qt, Signal
|
||||
from qtpy.QtCore import QPoint, QPointF, Qt, QTimer, Signal
|
||||
from qtpy.QtWidgets import QHBoxLayout, QLabel, QMainWindow, QVBoxLayout, QWidget
|
||||
|
||||
from bec_widgets.utils.bec_connector import ConnectionConfig
|
||||
@@ -1375,6 +1375,27 @@ class PlotBase(BECWidget, QWidget):
|
||||
self.crosshair.clear_markers()
|
||||
self.crosshair.update_markers()
|
||||
|
||||
def _call_later(self, msec: int, callback: Callable[[], object]) -> None:
|
||||
"""
|
||||
Call ``callback`` once after ``msec`` milliseconds, but only while the widget is in use.
|
||||
|
||||
Use this instead of a bare ``QTimer.singleShot`` for deferred work that touches the
|
||||
widget: the timer is bound to the widget, so the call is dropped if the widget is
|
||||
deleted first, and it is skipped if the widget has been closed (cleaned up) but not
|
||||
yet deleted.
|
||||
|
||||
Args:
|
||||
msec(int): Delay in milliseconds.
|
||||
callback(Callable[[], object]): Called without arguments.
|
||||
"""
|
||||
|
||||
def _call_unless_cleaned_up():
|
||||
if self._destroyed:
|
||||
return
|
||||
callback()
|
||||
|
||||
QTimer.singleShot(msec, self, _call_unless_cleaned_up)
|
||||
|
||||
def cleanup(self):
|
||||
self.toolbar.cleanup()
|
||||
self.unhook_crosshair(preserve_pin=False)
|
||||
|
||||
@@ -5,7 +5,7 @@ import pyqtgraph as pg
|
||||
from bec_lib import bec_logger
|
||||
from bec_lib.endpoints import MessageEndpoints
|
||||
from pydantic import Field, ValidationError, field_validator
|
||||
from qtpy.QtCore import QTimer, Signal
|
||||
from qtpy.QtCore import Signal
|
||||
from qtpy.QtWidgets import QHBoxLayout, QMainWindow, QWidget
|
||||
|
||||
from bec_widgets.utils.bec_connector import ConnectionConfig
|
||||
@@ -335,8 +335,9 @@ class ScatterWaveform(PlotBase):
|
||||
self.sync_signal_update.emit()
|
||||
status = msg.get("done")
|
||||
if status:
|
||||
QTimer.singleShot(100, self.update_sync_curves)
|
||||
QTimer.singleShot(300, self.update_sync_curves)
|
||||
# Final refreshes for late scan data; skipped if the widget is closed meanwhile.
|
||||
self._call_later(100, self.update_sync_curves)
|
||||
self._call_later(300, self.update_sync_curves)
|
||||
|
||||
@SafeSlot()
|
||||
def update_sync_curves(self, _=None):
|
||||
|
||||
@@ -1629,8 +1629,9 @@ class Waveform(PlotBase):
|
||||
self.sync_signal_update.emit()
|
||||
self._scan_done = msg.get("done")
|
||||
if self._scan_done:
|
||||
QTimer.singleShot(100, self.update_sync_curves)
|
||||
QTimer.singleShot(300, self.update_sync_curves)
|
||||
# Final refreshes for late scan data; skipped if the widget is closed meanwhile.
|
||||
self._call_later(100, self.update_sync_curves)
|
||||
self._call_later(300, self.update_sync_curves)
|
||||
|
||||
def _fetch_scan_data_and_access(self) -> tuple[dict, str] | tuple[None, None]:
|
||||
"""
|
||||
|
||||
@@ -199,6 +199,8 @@ class DeviceBrowser(BECWidget, QWidget):
|
||||
self._config_helper.save_current_session(file_path)
|
||||
|
||||
def cleanup(self):
|
||||
# Detach the filter proxy first, so no queued filter change reaches the closed browser.
|
||||
self.proxy_device_update.disconnect()
|
||||
super().cleanup()
|
||||
self.bec_dispatcher.client.callbacks.remove(self._scan_status_callback_id)
|
||||
self.bec_dispatcher.client.callbacks.remove(self._device_update_callback_id)
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import warnings
|
||||
|
||||
import numpy as np
|
||||
import pyqtgraph as pg
|
||||
import pytest
|
||||
@@ -643,3 +645,38 @@ def test_reset_preserves_pin_cleanup_removes_it(image_widget_with_crosshair):
|
||||
crosshair.cleanup()
|
||||
assert crosshair.pinned_point is None
|
||||
assert crosshair.pinned_pos is None
|
||||
|
||||
|
||||
def test_cleanup_detaches_crosshair_from_scene_mouse_signals(qtbot, plot_widget_with_crosshair):
|
||||
"""
|
||||
The plot's scene outlives the crosshair. After cleanup() neither mouse moves (through
|
||||
the rate-limited SignalProxy) nor clicks on the scene may reach the crosshair, e.g.
|
||||
re-creating a pin on the plot after teardown.
|
||||
"""
|
||||
crosshair, plot_item = plot_widget_with_crosshair
|
||||
scene = plot_item.scene()
|
||||
scene_pos = plot_item.vb.mapViewToScene(QPointF(2, 5))
|
||||
moved, clicked = [], []
|
||||
crosshair.positionChanged.connect(moved.append)
|
||||
crosshair.positionClicked.connect(clicked.append)
|
||||
|
||||
# Sanity check: while active, both scene signals reach the crosshair.
|
||||
scene.sigMouseMoved.emit(scene_pos)
|
||||
scene.sigMouseClicked.emit(_FakeClickEvent(scene_pos))
|
||||
qtbot.waitUntil(lambda: bool(moved and clicked), timeout=1000)
|
||||
assert crosshair.pinned_point is not None
|
||||
|
||||
crosshair.cleanup()
|
||||
moved.clear()
|
||||
clicked.clear()
|
||||
scene.sigMouseMoved.emit(scene_pos)
|
||||
scene.sigMouseClicked.emit(_FakeClickEvent(scene_pos))
|
||||
qtbot.wait(100) # well past the proxy's 60 Hz rate limit
|
||||
|
||||
assert moved == []
|
||||
assert clicked == []
|
||||
assert crosshair.pinned_point is None
|
||||
|
||||
with warnings.catch_warnings():
|
||||
warnings.simplefilter("error", RuntimeWarning) # e.g. libpyside "Failed to disconnect"
|
||||
crosshair.cleanup() # a second cleanup() leaves the scene connections alone
|
||||
@@ -181,3 +181,16 @@ def test_signal_display_omitted_not_added(mocked_client, qtbot):
|
||||
|
||||
qtbot.addWidget(signal_display)
|
||||
assert signal_display._content_layout.itemAt(1).widget() is None
|
||||
|
||||
|
||||
def test_device_browser_cleanup_detaches_filter_proxy(device_browser: DeviceBrowser, qtbot):
|
||||
"""
|
||||
After cleanup() the rate-limited filter proxy must no longer forward filter_input
|
||||
changes to update_device_list of the closed browser.
|
||||
"""
|
||||
with mock.patch.object(device_browser.dev_list, "update_filter") as update_filter:
|
||||
device_browser.close() # closeEvent -> cleanup()
|
||||
device_browser.ui.filter_input.setText("samx")
|
||||
|
||||
qtbot.wait(600)
|
||||
update_filter.assert_not_called()
|
||||
@@ -1,3 +1,5 @@
|
||||
from unittest import mock
|
||||
|
||||
import numpy as np
|
||||
import pyqtgraph as pg
|
||||
import pytest
|
||||
@@ -1676,3 +1678,25 @@ def test_layer_accessors_safe_after_teardown(qtbot, mocked_client):
|
||||
view.autorange_mode = "max"
|
||||
view._sync_autorange_switch()
|
||||
view._sync_colorbar_levels()
|
||||
|
||||
|
||||
def test_deferred_toolbar_sync_skipped_after_close(qtbot, mocked_client):
|
||||
"""
|
||||
The toolbar sync that __init__ defers to the event loop must not run into an Image
|
||||
that was closed (cleaned up) before the event loop got to it.
|
||||
"""
|
||||
with mock.patch.object(Image, "_sync_device_selection") as sync_device_selection:
|
||||
view = Image(client=mocked_client)
|
||||
qtbot.addWidget(view)
|
||||
view.close() # closeEvent -> cleanup() before the deferred sync ran
|
||||
|
||||
qtbot.wait(100)
|
||||
sync_device_selection.assert_not_called()
|
||||
|
||||
|
||||
def test_deferred_toolbar_sync_runs_while_open(qtbot, mocked_client):
|
||||
"""The toolbar sync deferred from __init__ still runs once for an open Image."""
|
||||
with mock.patch.object(Image, "_sync_device_selection") as sync_device_selection:
|
||||
create_widget(qtbot, Image, client=mocked_client)
|
||||
|
||||
qtbot.waitUntil(lambda: sync_device_selection.call_count == 1, timeout=1000)
|
||||
@@ -1,3 +1,6 @@
|
||||
from unittest import mock
|
||||
|
||||
import shiboken6
|
||||
from qtpy.QtTest import QSignalSpy
|
||||
|
||||
from bec_widgets.tests.client_mocks import mocked_client
|
||||
@@ -380,3 +383,41 @@ def test_motor_map_settings_dialog(qtbot, mocked_client):
|
||||
mm.motor_map_settings.close()
|
||||
qtbot.wait(200)
|
||||
assert mm.motor_map_settings is None
|
||||
|
||||
|
||||
def test_motor_map_cleanup_detaches_update_proxy(qtbot, mocked_client):
|
||||
"""
|
||||
A plot update still queued in proxy_update_plot when the MotorMap is cleaned up, or
|
||||
requested afterwards, must not reach _update_plot of the closed widget.
|
||||
"""
|
||||
with mock.patch.object(MotorMap, "_update_plot") as update_plot:
|
||||
mm = create_widget(qtbot, MotorMap, client=mocked_client)
|
||||
qtbot.wait(100) # let anything queued during construction drain
|
||||
update_plot.reset_mock()
|
||||
|
||||
mm.update_signal.emit() # queued in the proxy
|
||||
mm.close() # closeEvent -> cleanup()
|
||||
mm.update_signal.emit() # requested after cleanup
|
||||
|
||||
qtbot.wait(200)
|
||||
update_plot.assert_not_called()
|
||||
|
||||
|
||||
def test_motor_map_cleanup_closes_settings_popup(qtbot, mocked_client):
|
||||
"""
|
||||
Closing the MotorMap must also close its settings popup instead of leaving it open
|
||||
on top of the removed widget.
|
||||
"""
|
||||
mm = create_widget(qtbot, MotorMap, client=mocked_client, popups=True)
|
||||
mm.map(device_x="samx", device_y="samy")
|
||||
mm.show_motor_map_settings()
|
||||
dialog = mm.motor_map_settings
|
||||
assert dialog.isVisible()
|
||||
|
||||
mm.close() # closeEvent -> cleanup()
|
||||
try:
|
||||
assert mm.motor_map_settings is None
|
||||
assert not dialog.isVisible()
|
||||
finally:
|
||||
if shiboken6.isValid(dialog) and dialog.isVisible():
|
||||
dialog.reject() # keep a failure on main from leaking the popup into later tests
|
||||
@@ -792,3 +792,20 @@ def test_limits_accept_fractional_values(qtbot, mocked_client):
|
||||
pb.y_limits = (-1.75, 3.5)
|
||||
assert (pb.x_limits.x(), pb.x_limits.y()) == (0.5, 9.25)
|
||||
assert (pb.y_limits.x(), pb.y_limits.y()) == (-1.75, 3.5)
|
||||
|
||||
|
||||
def test_call_later_skips_callback_once_widget_is_closed(qtbot, mocked_client):
|
||||
"""
|
||||
PlotBase._call_later runs the deferred callback while the widget is open and skips it
|
||||
when the widget has been closed (cleaned up) before the delay elapsed.
|
||||
"""
|
||||
open_pb = create_widget(qtbot, PlotBase, client=mocked_client)
|
||||
closed_pb = create_widget(qtbot, PlotBase, client=mocked_client)
|
||||
calls = []
|
||||
|
||||
open_pb._call_later(10, lambda: calls.append("open"))
|
||||
closed_pb._call_later(10, lambda: calls.append("closed"))
|
||||
closed_pb.close() # closeEvent -> cleanup() before the callback is due
|
||||
|
||||
qtbot.wait(100)
|
||||
assert calls == ["open"]
|
||||
@@ -682,3 +682,37 @@ def test_scatter_waveform_cleanup_detaches_update_proxy(qtbot, mocked_client):
|
||||
|
||||
qtbot.wait(200)
|
||||
update_sync_curves.assert_not_called()
|
||||
|
||||
|
||||
def test_scatter_waveform_scan_done_runs_delayed_sync_refreshes(qtbot, mocked_client):
|
||||
"""
|
||||
A finished scan triggers one proxied sync update plus the two delayed final refreshes
|
||||
(100 ms and 300 ms) while the ScatterWaveform is open.
|
||||
"""
|
||||
with patch.object(ScatterWaveform, "update_sync_curves") as update_sync_curves:
|
||||
swf = create_widget(qtbot, ScatterWaveform, client=mocked_client)
|
||||
qtbot.wait(100) # let anything queued during construction drain
|
||||
update_sync_curves.reset_mock()
|
||||
|
||||
swf.on_scan_progress({"done": True}, {})
|
||||
|
||||
qtbot.waitUntil(lambda: update_sync_curves.call_count == 3, timeout=2000)
|
||||
qtbot.wait(100)
|
||||
assert update_sync_curves.call_count == 3
|
||||
|
||||
|
||||
def test_scatter_waveform_scan_done_refreshes_skipped_after_cleanup(qtbot, mocked_client):
|
||||
"""
|
||||
The delayed final refreshes scheduled when a scan is done must not run into a
|
||||
ScatterWaveform that has been closed (cleaned up) before they fire.
|
||||
"""
|
||||
with patch.object(ScatterWaveform, "update_sync_curves") as update_sync_curves:
|
||||
swf = create_widget(qtbot, ScatterWaveform, client=mocked_client)
|
||||
qtbot.wait(100) # let anything queued during construction drain
|
||||
update_sync_curves.reset_mock()
|
||||
|
||||
swf.on_scan_progress({"done": True}, {}) # schedules the refreshes in 100 ms and 300 ms
|
||||
swf.close() # closeEvent -> cleanup() before they fire
|
||||
|
||||
qtbot.wait(450)
|
||||
update_sync_curves.assert_not_called()
|
||||
@@ -678,6 +678,34 @@ def test_waveform_cleanup_detaches_proxies_from_update_signals(
|
||||
}
|
||||
|
||||
|
||||
def test_waveform_scan_done_runs_delayed_sync_refreshes(qtbot, waveform_with_proxy_slot_spies):
|
||||
"""
|
||||
A finished scan triggers one proxied sync update plus the two delayed final refreshes
|
||||
(100 ms and 300 ms) while the Waveform is open.
|
||||
"""
|
||||
wf, slots = waveform_with_proxy_slot_spies
|
||||
|
||||
wf.on_scan_progress({"done": True}, {})
|
||||
|
||||
qtbot.waitUntil(lambda: slots["sync"].call_count == 3, timeout=2000)
|
||||
qtbot.wait(100)
|
||||
assert slots["sync"].call_count == 3
|
||||
|
||||
|
||||
def test_waveform_scan_done_refreshes_skipped_after_cleanup(qtbot, waveform_with_proxy_slot_spies):
|
||||
"""
|
||||
The delayed final refreshes scheduled when a scan is done must not run into a Waveform
|
||||
that has been closed (cleaned up) before they fire.
|
||||
"""
|
||||
wf, slots = waveform_with_proxy_slot_spies
|
||||
|
||||
wf.on_scan_progress({"done": True}, {}) # schedules the refreshes in 100 ms and 300 ms
|
||||
wf.close() # closeEvent -> cleanup() before they fire
|
||||
|
||||
qtbot.wait(450)
|
||||
assert slots["sync"].call_count == 0
|
||||
|
||||
|
||||
def test_request_dap_always_resubmits_device_parent(qtbot, mocked_client_with_dap, monkeypatch):
|
||||
"""
|
||||
DAP curves attached to device curves keep the resubmit-on-every-update behavior,
|
||||
|
||||
Reference in new issue
Block a user