From 60408c14bcac5aac9ac8b623f1d455b06032cd09 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Sat, 13 Oct 2018 19:21:22 +0200 Subject: [PATCH] FirmwareUpdateChecker: Small refactors due to code review. --- .../FirmwareUpdateChecker.py | 33 ++++++++++--------- .../FirmwareUpdateCheckerJob.py | 6 ++-- .../FirmwareUpdateCheckerLookup.py | 10 +++--- .../resources/machines.json | 2 +- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/plugins/FirmwareUpdateChecker/FirmwareUpdateChecker.py b/plugins/FirmwareUpdateChecker/FirmwareUpdateChecker.py index e030d8f796..61604ff78b 100644 --- a/plugins/FirmwareUpdateChecker/FirmwareUpdateChecker.py +++ b/plugins/FirmwareUpdateChecker/FirmwareUpdateChecker.py @@ -5,19 +5,20 @@ import os from PyQt5.QtCore import QUrl from PyQt5.QtGui import QDesktopServices -from typing import List +from typing import Set from UM.Extension import Extension from UM.Application import Application from UM.Logger import Logger from UM.PluginRegistry import PluginRegistry +from UM.Qt.QtApplication import QtApplication from UM.i18n import i18nCatalog from UM.Settings.ContainerRegistry import ContainerRegistry from cura.Settings.GlobalStack import GlobalStack from .FirmwareUpdateCheckerJob import FirmwareUpdateCheckerJob -from .FirmwareUpdateCheckerLookup import FirmwareUpdateCheckerLookup, get_settings_key_for_machine +from .FirmwareUpdateCheckerLookup import FirmwareUpdateCheckerLookup, getSettingsKeyForMachine i18n_catalog = i18nCatalog("cura") @@ -36,22 +37,23 @@ class FirmwareUpdateChecker(Extension): if Application.getInstance().getPreferences().getValue("info/automatic_update_check"): ContainerRegistry.getInstance().containerAdded.connect(self._onContainerAdded) - self._late_init = True # Init some things after creation, since we need the path from the plugin-mgr. + # Partly initialize after creation, since we need our own path from the plugin-manager. self._download_url = None self._check_job = None - self._name_cache = [] # type: List[str] + self._checked_printer_names = [] # type: Set[str] self._lookups = None + QtApplication.pluginsLoaded.connect(self._onPluginsLoaded) ## Callback for the message that is spawned when there is a new version. def _onActionTriggered(self, message, action): - try: download_url = self._lookups.getRedirectUserFor(int(action)) if download_url is not None: - QDesktopServices.openUrl(QUrl(download_url)) + if QDesktopServices.openUrl(QUrl(download_url)): + Logger.log("i", "Redirected browser to {0} to show newly available firmware.".format(download_url)) + else: + Logger.log("e", "Can't reach URL: {0}".format(download_url)) else: Logger.log("e", "Can't find URL for {0}".format(action)) - except Exception as ex: - Logger.log("e", "Don't know what to do with '{0}' because {1}".format(action, ex)) def _onContainerAdded(self, container): # Only take care when a new GlobalStack was added @@ -61,8 +63,9 @@ class FirmwareUpdateChecker(Extension): def _onJobFinished(self, *args, **kwargs): self._check_job = None - def doLateInit(self): - self._late_init = False + def _onPluginsLoaded(self): + if self._lookups is not None: + return self._lookups = FirmwareUpdateCheckerLookup(os.path.join(PluginRegistry.getInstance().getPluginPath( "FirmwareUpdateChecker"), "resources/machines.json")) @@ -70,7 +73,7 @@ class FirmwareUpdateChecker(Extension): # Initialize the Preference called `latest_checked_firmware` that stores the last version # checked for each printer. for machine_id in self._lookups.getMachineIds(): - Application.getInstance().getPreferences().addPreference(get_settings_key_for_machine(machine_id), "") + Application.getInstance().getPreferences().addPreference(getSettingsKeyForMachine(machine_id), "") ## Connect with software.ultimaker.com, load latest.version and check version info. # If the version info is different from the current version, spawn a message to @@ -79,13 +82,13 @@ class FirmwareUpdateChecker(Extension): # \param silent type(boolean) Suppresses messages other than "new version found" messages. # This is used when checking for a new firmware version at startup. def checkFirmwareVersion(self, container = None, silent = False): - if self._late_init: - self.doLateInit() + if self._lookups is None: + self._onPluginsLoaded() container_name = container.definition.getName() - if container_name in self._name_cache: + if container_name in self._checked_printer_names: return - self._name_cache.append(container_name) + self._checked_printer_names.append(container_name) self._check_job = FirmwareUpdateCheckerJob(container = container, silent = silent, lookups = self._lookups, diff --git a/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerJob.py b/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerJob.py index ee5eaac25b..5bb9d076b6 100644 --- a/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerJob.py +++ b/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerJob.py @@ -12,7 +12,7 @@ from urllib.error import URLError from typing import Dict import codecs -from .FirmwareUpdateCheckerLookup import FirmwareUpdateCheckerLookup, get_settings_key_for_machine +from .FirmwareUpdateCheckerLookup import FirmwareUpdateCheckerLookup, getSettingsKeyForMachine from UM.i18n import i18nCatalog i18n_catalog = i18nCatalog("cura") @@ -78,12 +78,12 @@ class FirmwareUpdateCheckerJob(Job): # If it is not None, then we compare between the checked_version and the current_version machine_id = self._lookups.getMachineByName(machine_name.lower()) if machine_id is not None: - Logger.log("i", "You have a {0} in the printer list. Let's check the firmware!".format(machine_name)) + Logger.log("i", "You have a(n) {0} in the printer list. Let's check the firmware!".format(machine_name)) current_version = self.getCurrentVersionForMachine(machine_id) # If it is the first time the version is checked, the checked_version is "" - setting_key_str = get_settings_key_for_machine(machine_id) + setting_key_str = getSettingsKeyForMachine(machine_id) checked_version = Version(Application.getInstance().getPreferences().getValue(setting_key_str)) # If the checked_version is "", it's because is the first time we check firmware and in this case diff --git a/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerLookup.py b/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerLookup.py index 6d96ee36bb..ceecef61ba 100644 --- a/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerLookup.py +++ b/plugins/FirmwareUpdateChecker/FirmwareUpdateCheckerLookup.py @@ -12,17 +12,17 @@ from UM.i18n import i18nCatalog i18n_catalog = i18nCatalog("cura") -def get_settings_key_for_machine(machine_id: int) -> str: +def getSettingsKeyForMachine(machine_id: int) -> str: return "info/latest_checked_firmware_for_{0}".format(machine_id) -def default_parse_version_response(response: str) -> Version: +def defaultParseVersionResponse(response: str) -> Version: raw_str = response.split("\n", 1)[0].rstrip() - return Version(raw_str.split(".")) # Split it into a list; the default parsing of "single string" is different. + return Version(raw_str) class FirmwareUpdateCheckerLookup: - JSON_NAME_TO_VERSION_PARSE_FUNCTION = {"default": default_parse_version_response} + JSON_NAME_TO_VERSION_PARSE_FUNCTION = {"default": defaultParseVersionResponse} def __init__(self, json_path) -> None: # Open the .json file with the needed lookup-lists for each machine(/model) and retrieve "raw" json. @@ -48,7 +48,7 @@ class FirmwareUpdateCheckerLookup: self.JSON_NAME_TO_VERSION_PARSE_FUNCTION.get(machine_json.get("version_parser")) if version_parse_function is None: Logger.log("w", "No version-parse-function specified for machine {0}.".format(machine_name)) - version_parse_function = default_parse_version_response # Use default instead if nothing is found. + version_parse_function = defaultParseVersionResponse # Use default instead if nothing is found. self._parse_version_url_per_machine[machine_id] = version_parse_function self._check_urls_per_machine[machine_id] = [] # Multiple check-urls: see "_comment" in the .json file. for check_url in machine_json.get("check_urls"): diff --git a/plugins/FirmwareUpdateChecker/resources/machines.json b/plugins/FirmwareUpdateChecker/resources/machines.json index 5dc9aadbbf..ee072f75c3 100644 --- a/plugins/FirmwareUpdateChecker/resources/machines.json +++ b/plugins/FirmwareUpdateChecker/resources/machines.json @@ -1,5 +1,5 @@ { - "_comment": "Multiple 'update_urls': The 'new'-style URL is the only way to check for S5 firmware, and in the future, the UM3 line will also switch over, but for now the old 'JEDI'-style URL is still needed.", + "_comment": "There are multiple 'check_urls', because sometimes an URL is about to be phased out, and it's useful to have a new 'future-proof' one at the ready.", "machines": [