From 49947a2d214d8fcf0c8bad6797f88687fd385255 Mon Sep 17 00:00:00 2001 From: miro Date: Wed, 18 Sep 2024 23:18:37 +0100 Subject: [PATCH] refactor!:deprecate QML upload from bus never worked right, causes more issues than it helps --- README.md | 7 -- ovos_gui/constants.py | 4 ++ ovos_gui/gui_file_server.py | 1 - ovos_gui/namespace.py | 111 +++---------------------------- ovos_gui/page.py | 46 +++++-------- test/unittests/test_namespace.py | 4 +- 6 files changed, 33 insertions(+), 140 deletions(-) create mode 100644 ovos_gui/constants.py diff --git a/README.md b/README.md index 45de849..c374dff 100644 --- a/README.md +++ b/README.md @@ -40,13 +40,6 @@ under mycroft.conf // "gui_file_server": true, // "file_server_port": 8000, - // Optional support for collecting GUI files for container support - // The ovos-gui container path for these files will be {XDG_CACHE_HOME}/ovos_gui_file_server. - // With the below configuration, the GUI client will have files prefixed with the configured host path, - // so the example below describes a situation where `{XDG_CACHE_HOME}/ovos_gui_file_server` maps - // to `/tmp/gui_files` on the filesystem where the GUI client is running. - // "gui_file_host_path": "/tmp/gui_files", - // Optionally specify a default qt version for connected clients that don't report it "default_qt_version": 5 }, diff --git a/ovos_gui/constants.py b/ovos_gui/constants.py new file mode 100644 index 0000000..c3551f1 --- /dev/null +++ b/ovos_gui/constants.py @@ -0,0 +1,4 @@ +from ovos_config.locations import get_xdg_cache_save_path + +GUI_CACHE_PATH = get_xdg_cache_save_path('ovos_gui') + diff --git a/ovos_gui/gui_file_server.py b/ovos_gui/gui_file_server.py index 8b352c1..869fc8d 100644 --- a/ovos_gui/gui_file_server.py +++ b/ovos_gui/gui_file_server.py @@ -5,7 +5,6 @@ from threading import Thread, Event from ovos_config import Configuration -from ovos_utils.file_utils import get_temp_path from ovos_utils.log import LOG _HTTP_SERVER = None diff --git a/ovos_gui/namespace.py b/ovos_gui/namespace.py index 0c4ca5e..47fd625 100644 --- a/ovos_gui/namespace.py +++ b/ovos_gui/namespace.py @@ -40,13 +40,12 @@ over the GUI message bus. """ import shutil -from os import makedirs from os.path import join, dirname, isfile, exists from threading import Event, Lock, Timer from typing import List, Union, Optional, Dict from ovos_config.config import Configuration -from ovos_utils.log import LOG, log_deprecation +from ovos_utils.log import LOG from ovos_bus_client import Message, MessageBusClient from ovos_gui.bus import ( @@ -57,7 +56,7 @@ ) from ovos_gui.gui_file_server import start_gui_http_server from ovos_gui.page import GuiPage - +from ovos_gui.constants import GUI_CACHE_PATH namespace_lock = Lock() RESERVED_KEYS = ['__from', '__idle'] @@ -433,9 +432,6 @@ def __init__(self, core_bus: MessageBusClient): self._system_res_dir = join(dirname(__file__), "res", "gui") self._ready_event = Event() self.gui_file_server = None - self.gui_file_path = None # HTTP Server local path - self.gui_file_host_path = None # Docker host path - self._connected_frameworks: List[str] = list() self._init_gui_file_share() self._define_message_handlers() @@ -450,20 +446,10 @@ def _init_gui_file_share(self): If `gui_file_server` is defined, resources will be served via HTTP """ config = Configuration().get("gui", {}) - self.gui_file_host_path = config.get("gui_file_host_path") - - # Check for GUI file sharing via HTTP server or mounted host path - if config.get("gui_file_server") or self.gui_file_host_path: - from ovos_utils.xdg_utils import xdg_cache_home - if config.get("server_path"): - log_deprecation("`server_path` configuration is deprecated. " - "Files will always be saved to " - "XDG_CACHE_HOME/ovos_gui_file_server", "0.1.0") - self.gui_file_path = config.get("server_path") or \ - join(xdg_cache_home(), "ovos_gui_file_server") - if config.get("gui_file_server"): - self.gui_file_server = start_gui_http_server(self.gui_file_path) - self._upload_system_resources() + # Check for GUI file sharing via HTTP server + if config.get("gui_file_server"): + self.gui_file_server = start_gui_http_server(GUI_CACHE_PATH) + self._cache_system_resources() def _define_message_handlers(self): """ @@ -474,7 +460,6 @@ def _define_message_handlers(self): self.core_bus.on("gui.page.delete", self.handle_delete_page) self.core_bus.on("gui.page.delete.all", self.handle_delete_all_pages) self.core_bus.on("gui.page.show", self.handle_show_page) - self.core_bus.on("gui.page.upload", self.handle_receive_gui_pages) self.core_bus.on("gui.status.request", self.handle_status_request) self.core_bus.on("gui.value.set", self.handle_set_value) self.core_bus.on("mycroft.gui.connected", self.handle_client_connected) @@ -485,68 +470,6 @@ def _define_message_handlers(self): def handle_ready(self, message): self._ready_event.set() - self.core_bus.on("gui.volunteer_page_upload", - self.handle_gui_pages_available) - - def handle_gui_pages_available(self, message: Message): - """ - Handle a skill or plugin advertising that it has GUI pages available to - upload. If there are connected clients, request pages for each connected - GUI framework. - @param message: `gui.volunteer_page_upload` message - """ - if not any((self.gui_file_host_path, self.gui_file_server)): - LOG.debug("No GUI file server running or host path configured") - return - - LOG.debug(f"Requesting resources for {self._connected_frameworks}") - for framework in self._connected_frameworks: - skill_id = message.data.get("skill_id") - self.core_bus.emit(message.reply("gui.request_page_upload", - {'skill_id': skill_id, - 'framework': framework}, - {"source": "gui", - "destination": ["skills", - "PHAL"]})) - - def handle_receive_gui_pages(self, message: Message): - """ - Handle GUI resources from a skill or plugin. Pages are written to - `self.server_path` which is accessible via a lightweight HTTP server and - may additionally be mounted to a host path/volume in container setups. - @param message: Message containing UI resource file contents and meta - message.data: - pages: dict page_filename to encoded bytes content; - paths are relative to the `framework` directory, so a page - for framework `all` could be `qt5/subdir/file.qml` and the - equivalent page for framework `qt5` would be - `subdir/file.qml` - framework: `all` if all GUI resources are included, else the - specific GUI framework (i.e. `qt5`, `qt6`) - __from: skill_id of module uploading GUI resources - """ - for page, contents in message.data["pages"].items(): - try: - if message.data.get("framework") == "all": - # All GUI resources are uploaded - resource_base_path = join(self.gui_file_path, - message.data['__from']) - else: - resource_base_path = join(self.gui_file_path, - message.data['__from'], - message.data.get('framework') or - "qt5") - byte_contents = bytes.fromhex(contents) - file_path = join(resource_base_path, page) - LOG.debug(f"writing UI file: {file_path}") - makedirs(dirname(file_path), exist_ok=True) - with open(file_path, 'wb+') as f: - f.write(byte_contents) - except Exception as e: - LOG.exception(f"Failed to write {page}: {e}") - if message.data["__from"] == self._active_homescreen: - # Configured home screen skill just uploaded pages, show it again - self.core_bus.emit(message.forward("homescreen.manager.show_active")) def handle_clear_namespace(self, message: Message): """ @@ -952,23 +875,9 @@ def handle_client_connected(self, message: Message): websocket_config = get_gui_websocket_config() port = websocket_config["base_port"] message = message.forward("mycroft.gui.port", - dict(port=port, gui_id=gui_id)) + dict(port=port, gui_id=gui_id, framework=framework)) self.core_bus.emit(message) - if self.gui_file_path or self.gui_file_host_path: - if not self._ready_event.wait(90): - LOG.warning("Not reported ready after 90s") - if framework not in self._connected_frameworks: - LOG.debug(f"Requesting page upload for {framework}") - self.core_bus.emit(Message("gui.request_page_upload", - {'framework': framework}, - {"source": "gui", - "destination": ["skills", "PHAL"]})) - - if framework not in self._connected_frameworks: - LOG.debug(f"Connecting framework: {framework}") - self._connected_frameworks.append(framework) - def handle_page_interaction(self, message: Message): """ Handles an event from the GUI indicating a page has been interacted with. @@ -1037,13 +946,13 @@ def _del_namespace_in_remove_timers(self, namespace_name: str): if namespace_name in self.remove_namespace_timers: del self.remove_namespace_timers[namespace_name] - def _upload_system_resources(self): + def _cache_system_resources(self): """ Copy system GUI resources to the served file path """ - output_path = join(self.gui_file_path, "system") + output_path = f"{GUI_CACHE_PATH}/system" if exists(output_path): LOG.info(f"Removing existing system resources before updating") shutil.rmtree(output_path) shutil.copytree(self._system_res_dir, output_path) - LOG.debug(f"Copied system resources to {self.gui_file_path}") + LOG.debug(f"Copied system resources from {self._system_res_dir} to {output_path}") diff --git a/ovos_gui/page.py b/ovos_gui/page.py index b6eeef7..de1e24e 100644 --- a/ovos_gui/page.py +++ b/ovos_gui/page.py @@ -2,6 +2,7 @@ from typing import Union, Optional from dataclasses import dataclass from ovos_utils.log import LOG +from ovos_gui.constants import GUI_CACHE_PATH @dataclass @@ -44,12 +45,18 @@ def get_file_extension(framework: str) -> str: return "qml" return "" - def get_uri(self, framework: str = "qt5", server_url: str = None) -> str: + @property + def _gui_cache(self): + return f"{GUI_CACHE_PATH}]/{self.res_namespace}" + + @property + def res_namespace(self): + return "system" if self.page_id.startswith("SYSTEM") else self.namespace + + def get_uri(self, framework: str = "qt5") -> str: """ Get a valid URI for this Page. - @param framework: String GUI framework to get resources for - @param server_url: String server URL if available; this could be for a - web server (http://), or a container host path (file://) + @param framework: String GUI framework to get resources for (currently only 'qt5') @return: Absolute path to the requested resource """ if self.url: @@ -57,32 +64,13 @@ def get_uri(self, framework: str = "qt5", server_url: str = None) -> str: return self.url res_filename = f"{self.page_id}.{self.get_file_extension(framework)}" - res_namespace = "system" if self.page_id.startswith("SYSTEM") else \ - self.namespace - if server_url: - if "://" not in server_url: - if server_url.startswith("/"): - LOG.debug(f"No schema in server_url, assuming 'file'") - server_url = f"file://{server_url}" - else: - LOG.debug(f"No schema in server_url, assuming 'http'") - server_url = f"http://{server_url}" - path = f"{server_url}/{res_namespace}/{framework}/{res_filename}" - LOG.info(f"Resolved server URI: {path}") - return path - base_path = self.resource_dirs.get(framework) - if not base_path and self.resource_dirs.get("all"): - file_path = join(self.resource_dirs.get('all'), framework, - res_filename) + if self.res_namespace == "system": + path = join(dirname(__file__), "res", "gui", framework, res_filename) else: - file_path = join(base_path, res_filename) - if isfile(file_path): - return file_path - # Check system resources - file_path = join(dirname(__file__), "res", "gui", framework, - res_filename) - if isfile(file_path): - return file_path + path = f"{self._gui_cache}/{framework}/{res_filename}" + LOG.debug(f"Resolved page URI: {path}") + if isfile(path): + return path raise FileNotFoundError(f"Unable to resolve resource file for " f"resource {res_filename} for framework " f"{framework}") diff --git a/test/unittests/test_namespace.py b/test/unittests/test_namespace.py index a4a33a4..a84db5b 100644 --- a/test/unittests/test_namespace.py +++ b/test/unittests/test_namespace.py @@ -467,12 +467,12 @@ def test_upload_system_resources(self): test_dir = join(dirname(__file__), "upload_test") makedirs(test_dir, exist_ok=True) self.namespace_manager.gui_file_path = test_dir - self.namespace_manager._upload_system_resources() + self.namespace_manager._cache_system_resources() self.assertTrue(isdir(join(test_dir, "system", "qt5"))) self.assertTrue(isfile(join(test_dir, "system", "qt5", "SYSTEM_TextFrame.qml"))) # Test repeated copy doesn't raise any exception - self.namespace_manager._upload_system_resources() + self.namespace_manager._cache_system_resources() self.assertTrue(isdir(join(test_dir, "system", "qt5"))) self.assertTrue(isfile(join(test_dir, "system", "qt5", "SYSTEM_TextFrame.qml")))