From 4e8534b93b1be06d25f647a1f6da1032d71379dc Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Fri, 17 Jan 2020 11:10:29 +0100 Subject: [PATCH 1/7] Unsubscribe from package when a license is declined (cloud flow) CURA-6984 --- plugins/Toolbox/src/CloudApiModel.py | 8 ++++++++ plugins/Toolbox/src/CloudSync/CloudPackageManager.py | 5 +++++ plugins/Toolbox/src/CloudSync/SyncOrchestrator.py | 3 +-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/plugins/Toolbox/src/CloudApiModel.py b/plugins/Toolbox/src/CloudApiModel.py index 31c3139262..556d54cf88 100644 --- a/plugins/Toolbox/src/CloudApiModel.py +++ b/plugins/Toolbox/src/CloudApiModel.py @@ -18,3 +18,11 @@ class CloudApiModel: cloud_api_root=cloud_api_root, cloud_api_version=cloud_api_version, ) + + ## https://api.ultimaker.com/cura-packages/v1/user/packages/{package_id} + @classmethod + def userPackageUrl(cls, package_id: str) -> str: + + return (CloudApiModel.api_url_user_packages + "/{package_id}").format( + package_id=package_id + ) diff --git a/plugins/Toolbox/src/CloudSync/CloudPackageManager.py b/plugins/Toolbox/src/CloudSync/CloudPackageManager.py index ee57a1b90d..0cbc9eaa7a 100644 --- a/plugins/Toolbox/src/CloudSync/CloudPackageManager.py +++ b/plugins/Toolbox/src/CloudSync/CloudPackageManager.py @@ -16,3 +16,8 @@ class CloudPackageManager: data=data.encode(), scope=self._scope ) + + def unsubscribe(self, package_id: str) -> None: + url = CloudApiModel.userPackageUrl(package_id) + self._request_manager.delete(url=url, scope=self._scope) + diff --git a/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py b/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py index 674fb68729..abde4e4072 100644 --- a/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py +++ b/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py @@ -83,8 +83,7 @@ class SyncOrchestrator(Extension): self._cloud_package_manager.subscribe(item["package_id"]) has_changes = True else: - # todo unsubscribe declined packages - pass + self._cloud_package_manager.unsubscribe(item["package_id"]) # delete temp file os.remove(item["package_path"]) From 9dd50c88046128bd77dcd4cd7a02b7180a4fd580 Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Fri, 17 Jan 2020 11:44:57 +0100 Subject: [PATCH 2/7] Replace buttons by PrimaryButtons for cloud sync dialogs CURA-6984 --- .../qml/dialogs/CompatibilityDialog.qml | 4 +++- .../qml/dialogs/ToolboxLicenseDialog.qml | 21 ++++++++++++------- resources/themes/cura-light/theme.json | 1 + 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/plugins/Toolbox/resources/qml/dialogs/CompatibilityDialog.qml b/plugins/Toolbox/resources/qml/dialogs/CompatibilityDialog.qml index 06c1102811..32b4da4823 100644 --- a/plugins/Toolbox/resources/qml/dialogs/CompatibilityDialog.qml +++ b/plugins/Toolbox/resources/qml/dialogs/CompatibilityDialog.qml @@ -152,7 +152,7 @@ UM.Dialog{ } // End of ScrollView - Cura.ActionButton + Cura.PrimaryButton { id: nextButton anchors.bottom: parent.bottom @@ -160,6 +160,8 @@ UM.Dialog{ anchors.margins: UM.Theme.getSize("default_margin").height text: catalog.i18nc("@button", "Next") onClicked: accept() + leftPadding: UM.Theme.getSize("dialog_primary_button_padding").width + rightPadding: UM.Theme.getSize("dialog_primary_button_padding").width } } } diff --git a/plugins/Toolbox/resources/qml/dialogs/ToolboxLicenseDialog.qml b/plugins/Toolbox/resources/qml/dialogs/ToolboxLicenseDialog.qml index 2c88ac6d5f..3e7cdc9df8 100644 --- a/plugins/Toolbox/resources/qml/dialogs/ToolboxLicenseDialog.qml +++ b/plugins/Toolbox/resources/qml/dialogs/ToolboxLicenseDialog.qml @@ -10,6 +10,7 @@ import QtQuick.Controls.Styles 1.4 // TODO: Switch to QtQuick.Controls 2.x and remove QtQuick.Controls.Styles import UM 1.1 as UM +import Cura 1.6 as Cura UM.Dialog { @@ -51,18 +52,22 @@ UM.Dialog } rightButtons: [ - Button + Cura.PrimaryButton { - id: acceptButton - anchors.margins: UM.Theme.getSize("default_margin").width - text: catalog.i18nc("@action:button", "Accept") + leftPadding: UM.Theme.getSize("dialog_primary_button_padding").width + rightPadding: UM.Theme.getSize("dialog_primary_button_padding").width + + text: catalog.i18nc("@button", "Agree") onClicked: { handler.onLicenseAccepted() } - }, - Button + } + ] + + leftButtons: + [ + Cura.SecondaryButton { id: declineButton - anchors.margins: UM.Theme.getSize("default_margin").width - text: catalog.i18nc("@action:button", "Decline") + text: catalog.i18nc("@button", "Decline and remove from account") onClicked: { handler.onLicenseDeclined() } } ] diff --git a/resources/themes/cura-light/theme.json b/resources/themes/cura-light/theme.json index e5009d8633..de4c9ccb42 100644 --- a/resources/themes/cura-light/theme.json +++ b/resources/themes/cura-light/theme.json @@ -520,6 +520,7 @@ "action_button": [15.0, 2.5], "action_button_icon": [1.0, 1.0], "action_button_radius": [0.15, 0.15], + "dialog_primary_button_padding": [3.0, 0], "radio_button": [1.3, 1.3], From b03be75a1338cab6ef0ce79a88b01199c4fe977b Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Fri, 17 Jan 2020 12:21:18 +0100 Subject: [PATCH 3/7] Show error messages for cloud flow errors - failed downloads - failed installs CURA-6984 --- plugins/Toolbox/src/CloudSync/SyncOrchestrator.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py b/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py index abde4e4072..e97bdbcbc4 100644 --- a/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py +++ b/plugins/Toolbox/src/CloudSync/SyncOrchestrator.py @@ -1,8 +1,10 @@ import os from typing import List, Dict, Any, cast +from UM import i18n_catalog from UM.Extension import Extension from UM.Logger import Logger +from UM.Message import Message from UM.PluginRegistry import PluginRegistry from cura.CuraApplication import CuraApplication from .CloudPackageChecker import CloudPackageChecker @@ -64,7 +66,10 @@ class SyncOrchestrator(Extension): # \param success_items: Dict[package_id, file_path] # \param error_items: List[package_id] def _onDownloadFinished(self, success_items: Dict[str, str], error_items: List[str]) -> None: - # todo handle error items + if error_items: + message = i18n_catalog.i18nc("@info:generic", "{} plugins failed to download".format(len(error_items))) + self._showErrorMessage(message) + plugin_path = cast(str, PluginRegistry.getInstance().getPluginPath(self.getPluginId())) self._license_presenter.present(plugin_path, success_items) @@ -78,7 +83,8 @@ class SyncOrchestrator(Extension): if item["accepted"]: # install and subscribe packages if not self._package_manager.installPackage(item["package_path"]): - Logger.error("could not install {}".format(item["package_id"])) + message = "Could not install {}".format(item["package_id"]) + self._showErrorMessage(message) continue self._cloud_package_manager.subscribe(item["package_id"]) has_changes = True @@ -89,3 +95,8 @@ class SyncOrchestrator(Extension): if has_changes: self._restart_presenter.present() + + ## Logs an error and shows it to the user + def _showErrorMessage(self, text: str): + Logger.error(text) + Message(text, lifetime=0).show() From 66105e8a3a8cfc54410d57a48eb0bfc81715417f Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Mon, 20 Jan 2020 09:48:23 +0100 Subject: [PATCH 4/7] Add invalid imports checker Since plugins.* is not available on the PATH for some builds, they should not be used. Relative imports are preferred --- docker/build.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docker/build.sh b/docker/build.sh index 5b035ca08a..9bff78c2a3 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -13,6 +13,19 @@ export PKG_CONFIG_PATH="${CURA_BUILD_ENV_PATH}/lib/pkgconfig:${PKG_CONFIG_PATH}" cd "${PROJECT_DIR}" +# Check for plugins.* import statements. These imports may work when running from source, +# but will fail in some build types (linux and mac) +GREP_OUTPUT=$(grep -Ern "^\s*(from plugins|import plugins)" --include \*.py "${PROJECT_DIR}" || true) +echo "$GREP_OUTPUT" + +if [ -z "$GREP_OUTPUT" ] +then + echo "invalid imports checker: OK" +else + echo "error: sources contain invalid imports. Use relative imports when referencing plugin source files" + exit 1 +fi + # # Clone Uranium and set PYTHONPATH first # From b830a6faa3f83814b795fdca41bb73b90e48ac68 Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Mon, 20 Jan 2020 16:22:02 +0100 Subject: [PATCH 5/7] Rewrite invalid imports checker to Python Makes it consistent with other checkers we already have --- cmake/CuraTests.cmake | 7 ++++ docker/build.sh | 11 ------ scripts/check_invalid_imports.py | 60 ++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 scripts/check_invalid_imports.py diff --git a/cmake/CuraTests.cmake b/cmake/CuraTests.cmake index b1d3e0ddc4..c76019d310 100644 --- a/cmake/CuraTests.cmake +++ b/cmake/CuraTests.cmake @@ -56,6 +56,13 @@ function(cura_add_test) endif() endfunction() +#Add test for whether the shortcut alt-keys are unique in every translation. +add_test( + NAME "invalid-imports" + COMMAND ${Python3_EXECUTABLE} scripts/check_invalid_imports.py + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} +) + cura_add_test(NAME pytest-main DIRECTORY ${CMAKE_SOURCE_DIR}/tests PYTHONPATH "${CMAKE_SOURCE_DIR}|${URANIUM_DIR}") file(GLOB_RECURSE _plugins plugins/*/__init__.py) diff --git a/docker/build.sh b/docker/build.sh index 9bff78c2a3..a500663c64 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -13,18 +13,7 @@ export PKG_CONFIG_PATH="${CURA_BUILD_ENV_PATH}/lib/pkgconfig:${PKG_CONFIG_PATH}" cd "${PROJECT_DIR}" -# Check for plugins.* import statements. These imports may work when running from source, -# but will fail in some build types (linux and mac) -GREP_OUTPUT=$(grep -Ern "^\s*(from plugins|import plugins)" --include \*.py "${PROJECT_DIR}" || true) -echo "$GREP_OUTPUT" -if [ -z "$GREP_OUTPUT" ] -then - echo "invalid imports checker: OK" -else - echo "error: sources contain invalid imports. Use relative imports when referencing plugin source files" - exit 1 -fi # # Clone Uranium and set PYTHONPATH first diff --git a/scripts/check_invalid_imports.py b/scripts/check_invalid_imports.py new file mode 100644 index 0000000000..121184e739 --- /dev/null +++ b/scripts/check_invalid_imports.py @@ -0,0 +1,60 @@ +import os +import re +import sys +from pathlib import Path + +""" +Run this file with the Cura project root as the working directory +""" + +class InvalidImportsChecker: + # compile regex + REGEX = re.compile(r"^\s*(from plugins|import plugins)") + + def check(self): + """ Checks for invalid imports + + :return: True if checks passed, False when the test fails + """ + cwd = os.getcwd() + cura_result = checker.check_dir(os.path.join(cwd, "cura")) + plugins_result = checker.check_dir(os.path.join(cwd, "plugins")) + result = cura_result and plugins_result + if not result: + print("error: sources contain invalid imports. Use relative imports when referencing plugin source files") + + return result + + def check_dir(self, root_dir: str) -> bool: + """ Checks a directory for invalid imports + + :return: True if checks passed, False when the test fails + """ + passed = True + for path_like in Path(root_dir).rglob('*.py'): + if not self.check_file(str(path_like)): + passed = False + + return passed + + def check_file(self, file_path): + """ Checks a file for invalid imports + + :return: True if checks passed, False when the test fails + """ + passed = True + with open(file_path, 'r', encoding = "utf-8") as inputFile: + # loop through each line in file + for line_i, line in enumerate(inputFile, 1): + # check if we have a regex match + match = self.REGEX.search(line) + if match: + path = os.path.relpath(file_path) + print("{path}:{line_i}:{match}".format(path=path, line_i=line_i, match=match.group(1))) + passed = False + return passed + + +if __name__ == "__main__": + checker = InvalidImportsChecker() + sys.exit(0 if checker.check() else 1) From 2e20bf6a983dfd4fcfad56aecb3ce41f53621b31 Mon Sep 17 00:00:00 2001 From: Nino van Hooff Date: Mon, 20 Jan 2020 16:33:03 +0100 Subject: [PATCH 6/7] Update invalid imports checker documentation Makes it consistent with other checkers we already have --- cmake/CuraTests.cmake | 2 +- scripts/check_invalid_imports.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmake/CuraTests.cmake b/cmake/CuraTests.cmake index c76019d310..251bec5781 100644 --- a/cmake/CuraTests.cmake +++ b/cmake/CuraTests.cmake @@ -56,7 +56,7 @@ function(cura_add_test) endif() endfunction() -#Add test for whether the shortcut alt-keys are unique in every translation. +#Add test for import statements which are not compatible with all builds add_test( NAME "invalid-imports" COMMAND ${Python3_EXECUTABLE} scripts/check_invalid_imports.py diff --git a/scripts/check_invalid_imports.py b/scripts/check_invalid_imports.py index 121184e739..ba21b9f822 100644 --- a/scripts/check_invalid_imports.py +++ b/scripts/check_invalid_imports.py @@ -5,8 +5,13 @@ from pathlib import Path """ Run this file with the Cura project root as the working directory +Checks for invalid imports. When importing from plugins, there will be no problems when running from source, +but for some build types the plugins dir is not on the path, so relative imports should be used instead. eg: +from ..UltimakerCloudScope import UltimakerCloudScope <-- OK +import plugins.Toolbox.src ... <-- NOT OK """ + class InvalidImportsChecker: # compile regex REGEX = re.compile(r"^\s*(from plugins|import plugins)") From 5d21872e50e99f99076b7799cc72ad8f66eb87a3 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 21 Jan 2020 10:31:47 +0100 Subject: [PATCH 7/7] Take nozzle offsets into account when placing prime tower extruderValues('machine_nozzle_offset_x') := [0, 20, -18] map(abs(extruderValues('machine_nozzle_offset_x') := [0, 20, 18] max(map(abs(extruderValues('machine_nozzle_offset_x') := 20 So we take the highest offset of all extruders to get the area that can be reached by all extruders. And we take the abs() of all extruder values because positive or negative only means that the other extruders get offset in the same direction. Fixes #6997. --- resources/definitions/fdmprinter.def.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/definitions/fdmprinter.def.json b/resources/definitions/fdmprinter.def.json index ca70f0d7de..b5d423c28b 100644 --- a/resources/definitions/fdmprinter.def.json +++ b/resources/definitions/fdmprinter.def.json @@ -5688,7 +5688,7 @@ "unit": "mm", "enabled": "resolveOrValue('prime_tower_enable')", "default_value": 200, - "value": "machine_width - max(extruderValue(adhesion_extruder_nr, 'brim_width') * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 if adhesion_type == 'brim' or (prime_tower_brim_enable and adhesion_type != 'raft') else (extruderValue(adhesion_extruder_nr, 'raft_margin') if adhesion_type == 'raft' else (extruderValue(adhesion_extruder_nr, 'skirt_gap') if adhesion_type == 'skirt' else 0)), max(extruderValues('travel_avoid_distance'))) - max(extruderValues('support_offset')) - sum(extruderValues('skirt_brim_line_width')) * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 - (resolveOrValue('draft_shield_dist') if resolveOrValue('draft_shield_enabled') else 0) - 1", + "value": "machine_width - max(extruderValue(adhesion_extruder_nr, 'brim_width') * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 if adhesion_type == 'brim' or (prime_tower_brim_enable and adhesion_type != 'raft') else (extruderValue(adhesion_extruder_nr, 'raft_margin') if adhesion_type == 'raft' else (extruderValue(adhesion_extruder_nr, 'skirt_gap') if adhesion_type == 'skirt' else 0)), max(extruderValues('travel_avoid_distance'))) - max(extruderValues('support_offset')) - sum(extruderValues('skirt_brim_line_width')) * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 - (resolveOrValue('draft_shield_dist') if resolveOrValue('draft_shield_enabled') else 0) - max(map(abs, extruderValues('machine_nozzle_offset_x'))) - 1", "maximum_value": "machine_width / 2 if machine_center_is_zero else machine_width", "minimum_value": "resolveOrValue('prime_tower_size') - machine_width / 2 if machine_center_is_zero else resolveOrValue('prime_tower_size')", "settable_per_mesh": false, @@ -5702,7 +5702,7 @@ "unit": "mm", "enabled": "resolveOrValue('prime_tower_enable')", "default_value": 200, - "value": "machine_depth - prime_tower_size - max(extruderValue(adhesion_extruder_nr, 'brim_width') * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 if adhesion_type == 'brim' or (prime_tower_brim_enable and adhesion_type != 'raft') else (extruderValue(adhesion_extruder_nr, 'raft_margin') if adhesion_type == 'raft' else (extruderValue(adhesion_extruder_nr, 'skirt_gap') if adhesion_type == 'skirt' else 0)), max(extruderValues('travel_avoid_distance'))) - max(extruderValues('support_offset')) - sum(extruderValues('skirt_brim_line_width')) * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 - (resolveOrValue('draft_shield_dist') if resolveOrValue('draft_shield_enabled') else 0) - 1", + "value": "machine_depth - prime_tower_size - max(extruderValue(adhesion_extruder_nr, 'brim_width') * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 if adhesion_type == 'brim' or (prime_tower_brim_enable and adhesion_type != 'raft') else (extruderValue(adhesion_extruder_nr, 'raft_margin') if adhesion_type == 'raft' else (extruderValue(adhesion_extruder_nr, 'skirt_gap') if adhesion_type == 'skirt' else 0)), max(extruderValues('travel_avoid_distance'))) - max(extruderValues('support_offset')) - sum(extruderValues('skirt_brim_line_width')) * extruderValue(adhesion_extruder_nr, 'initial_layer_line_width_factor') / 100 - (resolveOrValue('draft_shield_dist') if resolveOrValue('draft_shield_enabled') else 0) - max(map(abs, extruderValues('machine_nozzle_offset_y'))) - 1", "maximum_value": "machine_depth / 2 - resolveOrValue('prime_tower_size') if machine_center_is_zero else machine_depth - resolveOrValue('prime_tower_size')", "minimum_value": "machine_depth / -2 if machine_center_is_zero else 0", "settable_per_mesh": false,