From b51bbe5ea9ccafe14b09e180c5bf7bc987d10b44 Mon Sep 17 00:00:00 2001 From: appel_c Date: Thu, 20 Feb 2025 09:18:37 +0100 Subject: [PATCH] refactor: remove main, individual gui_ids for DockArea --- bec_widgets/cli/client_utils.py | 84 ++++++++++++------- bec_widgets/cli/rpc/rpc_base.py | 4 +- bec_widgets/cli/server.py | 27 ++++-- .../containers/main_window/main_window.py | 12 +-- tests/end-2-end/conftest.py | 15 ++-- tests/end-2-end/test_bec_dock_rpc_e2e.py | 26 +++--- tests/unit_tests/test_client_utils.py | 6 +- 7 files changed, 110 insertions(+), 64 deletions(-) diff --git a/bec_widgets/cli/client_utils.py b/bec_widgets/cli/client_utils.py index 9d043f6c..f987c0dc 100644 --- a/bec_widgets/cli/client_utils.py +++ b/bec_widgets/cli/client_utils.py @@ -8,7 +8,6 @@ import select import subprocess import threading from contextlib import contextmanager -from dataclasses import dataclass from typing import TYPE_CHECKING from bec_lib.endpoints import MessageEndpoints @@ -67,7 +66,9 @@ def _get_output(process, logger) -> None: logger.error(f"Error reading process output: {str(e)}") -def _start_plot_process(gui_id: str, gui_class: type, config: dict | str, logger=None) -> None: +def _start_plot_process( + gui_id: str, gui_class: type, gui_class_id: str, config: dict | str, logger=None +) -> None: """ Start the plot in a new process. @@ -76,7 +77,16 @@ def _start_plot_process(gui_id: str, gui_class: type, config: dict | str, logger process will not be captured. """ # pylint: disable=subprocess-run-check - command = ["bec-gui-server", "--id", gui_id, "--gui_class", gui_class.__name__, "--hide"] + command = [ + "bec-gui-server", + "--id", + gui_id, + "--gui_class", + gui_class.__name__, + "--gui_class_id", + gui_class_id, + "--hide", + ] if config: if isinstance(config, dict): config = json.dumps(config) @@ -120,7 +130,7 @@ class RepeatTimer(threading.Timer): def wait_for_server(client): timeout = client._startup_timeout if not timeout: - if client.gui_is_alive(): + if client._gui_is_alive(): # there is hope, let's wait a bit timeout = 1 else: @@ -147,8 +157,8 @@ def wait_for_server(client): ### is created, and client module is patched. class BECDockArea(client.BECDockArea): def delete(self): - if self is BECGuiClient._top_level["main"]: - raise RuntimeError("Cannot delete main window") + if self is BECGuiClient._top_level["bec"]: + raise RuntimeError("Cannot delete bec window") super().delete() try: del BECGuiClient._top_level[self._gui_id] @@ -166,6 +176,7 @@ class BECGuiClient(RPCBase): def __init__(self, **kwargs) -> None: super().__init__(**kwargs) + self._default_dock_name = "bec" self._auto_updates_enabled = True self._auto_updates = None self._startup_timeout = 0 @@ -179,6 +190,10 @@ class BECGuiClient(RPCBase): def windows(self): return self._top_level + @property + def window_list(self): + return list(self._top_level.values()) + @property def auto_updates(self): if self._auto_updates_enabled: @@ -235,20 +250,21 @@ class BECGuiClient(RPCBase): def _update_script_msg_parser(self, msg: messages.BECMessage) -> None: if isinstance(msg, messages.ScanStatusMessage): - if not self.gui_is_alive(): + if not self._gui_is_alive(): return if self._auto_updates_enabled: return self.auto_updates.do_update(msg) def _gui_post_startup(self): - self._top_level["main"] = BECDockArea(gui_id=self._gui_id) + widget = BECDockArea(gui_id=self._default_dock_name, parent=self) + self._add_widget_to_top_level(self._default_dock_name, widget) if self._auto_updates_enabled: if self._auto_updates is None: auto_updates = self._get_update_script() if auto_updates is None: AutoUpdates.create_default_dock = True AutoUpdates.enabled = True - auto_updates = AutoUpdates(self._top_level["main"]) + auto_updates = AutoUpdates(self._top_level[self._default_dock_name]) if auto_updates.create_default_dock: auto_updates.start_default_dock() self._start_update_script() @@ -265,7 +281,11 @@ class BECGuiClient(RPCBase): self._startup_timeout = 5 self._gui_started_event.clear() self._process, self._process_output_processing_thread = _start_plot_process( - self._gui_id, self.__class__, self._client._service_config.config, logger=logger + self._gui_id, + self.__class__, + gui_class_id=self._default_dock_name, + config=self._client._service_config.config, + logger=logger, ) def gui_started_callback(callback): @@ -276,7 +296,7 @@ class BECGuiClient(RPCBase): threading.current_thread().cancel() self._gui_started_timer = RepeatTimer( - 0.5, lambda: self.gui_is_alive() and gui_started_callback(self._gui_post_startup) + 0.5, lambda: self._gui_is_alive() and gui_started_callback(self._gui_post_startup) ) self._gui_started_timer.start() @@ -300,11 +320,11 @@ class BECGuiClient(RPCBase): for window in self._top_level.values(): window.show() - def show_all(self): + def _show_all(self): with wait_for_server(self): return self._do_show_all() - def hide_all(self): + def _hide_all(self): with wait_for_server(self): rpc_client = RPCBase(gui_id=f"{self._gui_id}:window", parent=self) rpc_client._run_rpc("hide") @@ -313,28 +333,30 @@ class BECGuiClient(RPCBase): def show(self): if self._process is not None: - return self.show_all() + return self._show_all() # backward compatibility: show() was also starting server return self._start_server(wait=True) def hide(self): - return self.hide_all() + return self._hide_all() - @property - def main(self): - """Return client to main dock area (in main window)""" - with wait_for_server(self): - return self._top_level["main"] + def new(self, title: str = None, wait: bool = True) -> BECDockArea: + """Create a new top-level dock area""" + if wait: + with wait_for_server(self): + rpc_client = RPCBase(gui_id=f"{self._gui_id}:window", parent=self) + widget = rpc_client._run_rpc("new_dock_area", title) + self._add_widget_to_top_level(widget._gui_id, widget) + return widget + rpc_client = RPCBase(gui_id=f"{self._gui_id}:window", parent=self) + widget = rpc_client._run_rpc("new_dock_area", title) + self._add_widget_to_top_level(widget._gui_id, widget) + return widget - def new(self, title: str = None) -> BECDockArea: - """Ask main window to create a new top-level dock area""" - with wait_for_server(self): - rpc_client = RPCBase(gui_id=f"{self._gui_id}:window", parent=self) - widget = rpc_client._run_rpc("new_dock_area", title) - self._top_level[widget._gui_id] = widget - setattr(self, widget._gui_id, widget) - self._exposed_widgets.append(widget._gui_id) - return widget + def _add_widget_to_top_level(self, widget_id: str, widget: BECDockArea) -> None: + self._top_level[widget_id] = widget + setattr(self, widget_id, widget) + self._exposed_widgets.append(widget_id) def _update_top_level_widgets(self): for widget_id in self._exposed_widgets: @@ -346,6 +368,10 @@ class BECGuiClient(RPCBase): self._exposed_widgets.append(widget_id) def close(self) -> None: + # FIXME: keeping backwards compatibility for now + self._close() + + def _close(self) -> None: """ Close the gui window. """ diff --git a/bec_widgets/cli/rpc/rpc_base.py b/bec_widgets/cli/rpc/rpc_base.py index 0401350e..cc688925 100644 --- a/bec_widgets/cli/rpc/rpc_base.py +++ b/bec_widgets/cli/rpc/rpc_base.py @@ -44,7 +44,7 @@ def rpc_call(func): for key, val in kwargs.items(): if hasattr(val, "name"): kwargs[key] = val.name - if not self.gui_is_alive(): + if not self._root._gui_is_alive(): raise RuntimeError("GUI is not alive") return self._run_rpc(func.__name__, *args, **kwargs) @@ -165,7 +165,7 @@ class RPCBase: return cls(parent=self, **msg_result) return msg_result - def gui_is_alive(self): + def _gui_is_alive(self): """ Check if the GUI is alive. """ diff --git a/bec_widgets/cli/server.py b/bec_widgets/cli/server.py index 59e98aa8..8d137cf9 100644 --- a/bec_widgets/cli/server.py +++ b/bec_widgets/cli/server.py @@ -56,14 +56,15 @@ class BECWidgetsCLIServer: dispatcher: BECDispatcher = None, client=None, config=None, - gui_class: Union[BECFigure, BECDockArea] = BECFigure, + gui_class: Union[BECFigure, BECDockArea] = BECDockArea, + gui_class_id: str = "bec", ) -> None: self.status = messages.BECStatus.BUSY self.dispatcher = BECDispatcher(config=config) if dispatcher is None else dispatcher self.client = self.dispatcher.client if client is None else client self.client.start() self.gui_id = gui_id - self.gui = gui_class(gui_id=self.gui_id) + self.gui = gui_class(gui_id=gui_class_id) self.rpc_register = RPCRegister() self.rpc_register.add_rpc(self.gui) @@ -197,7 +198,12 @@ class SimpleFileLikeFromLogOutputFunc: return -def _start_server(gui_id: str, gui_class: Union[BECFigure, BECDockArea], config: str | None = None): +def _start_server( + gui_id: str, + gui_class: Union[BECFigure, BECDockArea], + gui_class_id: str = "bec", + config: str | None = None, +): if config: try: config = json.loads(config) @@ -214,7 +220,9 @@ def _start_server(gui_id: str, gui_class: Union[BECFigure, BECDockArea], config: # service_name="BECWidgetsCLIServer", # service_config=service_config.service_config, # ) - server = BECWidgetsCLIServer(gui_id=gui_id, config=service_config, gui_class=gui_class) + server = BECWidgetsCLIServer( + gui_id=gui_id, config=service_config, gui_class=gui_class, gui_class_id=gui_class_id + ) return server @@ -235,6 +243,12 @@ def main(): type=str, help="Name of the gui class to be rendered. Possible values: \n- BECFigure\n- BECDockArea", ) + parser.add_argument( + "--gui_class_id", + type=str, + default="bec", + help="The id of the gui class that is added to the QApplication", + ) parser.add_argument("--config", type=str, help="Config file or config string.") parser.add_argument("--hide", action="store_true", help="Hide on startup") @@ -274,11 +288,12 @@ def main(): # store gui id within QApplication object, to make it available to all widgets app.gui_id = args.id - server = _start_server(args.id, gui_class, args.config) + # args.id = "52e70" + server = _start_server(args.id, gui_class, args.gui_class_id, args.config) win = BECMainWindow(gui_id=f"{server.gui_id}:window") win.setAttribute(Qt.WA_ShowWithoutActivating) - win.setWindowTitle("BEC Widgets") + win.setWindowTitle("BEC") RPCRegister().add_rpc(win) RPCRegister().add_callback(server.broadcast_registry_update) diff --git a/bec_widgets/widgets/containers/main_window/main_window.py b/bec_widgets/widgets/containers/main_window/main_window.py index 93eb2c34..75d0ff7e 100644 --- a/bec_widgets/widgets/containers/main_window/main_window.py +++ b/bec_widgets/widgets/containers/main_window/main_window.py @@ -34,15 +34,17 @@ class BECMainWindow(QMainWindow, BECConnector): } return info - def new_dock_area(self, name=None): - name = name or "BEC Widgets" + def new_dock_area(self, name: str | None = None): + if name is None: + name = "BEC" + else: + name = "BEC - " + name self.rpc_register = RPCRegister() - gui_id = name.replace(" ", "_") + gui_id = name.replace(" - ", "_").replace(" ", "_").lower() existing_widgets = self.rpc_register.get_rpc_by_type(gui_id) if existing_widgets: name = f"{name} {len(existing_widgets) + 1}" - - dock_area = BECDockArea(gui_id=name.replace(" ", "_")) + dock_area = BECDockArea(gui_id=name.replace(" - ", "_").replace(" ", "_").lower()) dock_area.resize(dock_area.minimumSizeHint()) dock_area.window().setWindowTitle(name) dock_area.show() diff --git a/tests/end-2-end/conftest.py b/tests/end-2-end/conftest.py index c8aa83dc..f4548f89 100644 --- a/tests/end-2-end/conftest.py +++ b/tests/end-2-end/conftest.py @@ -27,7 +27,9 @@ def gui_id(): @contextmanager def plot_server(gui_id, klass, client_lib): dispatcher = BECDispatcher(client=client_lib) # Has to init singleton with fixture client - process, _ = _start_plot_process(gui_id, klass, client_lib._client._service_config.config_path) + process, _ = _start_plot_process( + gui_id, klass, gui_class_id="bec", config=client_lib._client._service_config.config_path + ) try: while client_lib._client.connector.get(MessageEndpoints.gui_heartbeat(gui_id)) is None: time.sleep(0.3) @@ -52,7 +54,7 @@ def connected_client_gui_obj(gui_id, bec_client_lib): gui._start_server(wait=True) yield gui finally: - gui.close() + gui._close() @pytest.fixture @@ -61,9 +63,10 @@ def connected_client_dock(gui_id, bec_client_lib): gui._auto_updates_enabled = False try: gui._start_server(wait=True) - yield gui.main + gui.window_list[0] + yield gui.window_list[0] finally: - gui.close() + gui._close() @pytest.fixture @@ -71,6 +74,6 @@ def connected_client_dock_w_auto_updates(gui_id, bec_client_lib): gui = BECGuiClient(gui_id=gui_id) try: gui._start_server(wait=True) - yield gui, gui.main + yield gui, gui.window_list[0] finally: - gui.close() + gui._close() diff --git a/tests/end-2-end/test_bec_dock_rpc_e2e.py b/tests/end-2-end/test_bec_dock_rpc_e2e.py index 8ab6d73b..2b26ecd6 100644 --- a/tests/end-2-end/test_bec_dock_rpc_e2e.py +++ b/tests/end-2-end/test_bec_dock_rpc_e2e.py @@ -315,8 +315,8 @@ def test_rpc_gui_obj(connected_client_gui_obj, qtbot): assert gui.selected_device is None assert len(gui.windows) == 1 - assert gui.windows["main"] is gui.main - mw = gui.main + assert gui.windows["bec"] is gui.bec + mw = gui.bec assert mw.__class__.__name__ == "BECDockArea" xw = gui.new("X") @@ -325,10 +325,10 @@ def test_rpc_gui_obj(connected_client_gui_obj, qtbot): gui_info = gui._dump() mw_info = gui_info[mw._gui_id] - assert mw_info["title"] == "BEC Widgets" + assert mw_info["title"] == "BEC" assert mw_info["visible"] xw_info = gui_info[xw._gui_id] - assert xw_info["title"] == "X" + assert xw_info["title"] == "BEC - X" assert xw_info["visible"] gui.hide() @@ -339,23 +339,23 @@ def test_rpc_gui_obj(connected_client_gui_obj, qtbot): gui_info = gui._dump() assert all(windows["visible"] for windows in gui_info.values()) - assert gui.gui_is_alive() - gui.close() - assert not gui.gui_is_alive() + assert gui._gui_is_alive() + gui._close() + assert not gui._gui_is_alive() gui._start_server(wait=True) - assert gui.gui_is_alive() + assert gui._gui_is_alive() # calling start multiple times should not change anything gui._start_server(wait=True) - gui.start() - # gui.windows should have main, and main dock area should have same gui_id as before + gui._start() + # gui.windows should have bec with gui_id 'bec' assert len(gui.windows) == 1 - assert gui.windows["main"]._gui_id == mw._gui_id + assert gui.windows["bec"]._gui_id == mw._gui_id # communication should work, main dock area should have same id and be visible gui_info = gui._dump() assert gui_info[mw._gui_id]["visible"] with pytest.raises(RuntimeError): - gui.main.delete() + gui.bec.delete() yw = gui.new("Y") assert len(gui.windows) == 2 @@ -373,5 +373,5 @@ def test_rpc_call_with_exception_in_safeslot_error_popup(connected_client_gui_ob qtbot.waitUntil(lambda: len(gui.main.panels) == 2) # default_figure + test qtbot.wait(500) with pytest.raises(ValueError): - gui.main.add_dock("test") + gui.bec.add_dock("test") # time.sleep(0.1) diff --git a/tests/unit_tests/test_client_utils.py b/tests/unit_tests/test_client_utils.py index 4b2505ed..070cfcfb 100644 --- a/tests/unit_tests/test_client_utils.py +++ b/tests/unit_tests/test_client_utils.py @@ -66,13 +66,13 @@ def test_client_utils_passes_client_config_to_server(bec_dispatcher): mixin = BECGuiClient() mixin._client = bec_dispatcher.client mixin._gui_id = "gui_id" - mixin.gui_is_alive = mock.MagicMock() - mixin.gui_is_alive.side_effect = [True] + mixin._gui_is_alive = mock.MagicMock() + mixin._gui_is_alive.side_effect = [True] try: yield mixin finally: - mixin.close() + mixin._close() with bec_client_mixin() as mixin: with mock.patch("bec_widgets.cli.client_utils._start_plot_process") as mock_start_plot: