From 3cbd8a94a91de50565f0f6756e8ab0a33604f587 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Thu, 18 Apr 2019 00:19:12 +0200 Subject: [PATCH 1/7] Add minimal support for discovering cloud printers outside of LAN --- .../src/Cloud/CloudOutputDevice.py | 35 ++++++++--- .../src/Cloud/CloudOutputDeviceManager.py | 60 +++++++++++++++---- .../src/Cloud/Models/CloudClusterResponse.py | 4 +- 3 files changed, 79 insertions(+), 20 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index 688538522e..fb8e9d9408 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -58,6 +58,14 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): # Therefore we create a private signal used to trigger the printersChanged signal. _clusterPrintersChanged = pyqtSignal() + # Map of Cura Connect machine_variant field to Cura machine types. + # Needed for printer discovery stack creation. + _host_machine_variant_to_machine_type_map = { + "Ultimaker 3": "ultimaker3", + "Ultimaker 3 Extended": "ultimaker3_extended", + "Ultimaker S5": "ultimaker_s5" + } + ## Creates a new cloud output device # \param api_client: The client that will run the API calls # \param cluster: The device response received from the cloud API. @@ -68,10 +76,10 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): # Because the cloud connection does not off all of these, we manually construct this version here. # An example of why this is needed is the selection of the compatible file type when exporting the tool path. properties = { - b"address": b"", - b"name": cluster.host_name.encode() if cluster.host_name else b"", + b"address": cluster.host_internal_ip.encode() if cluster.host_internal_ip else b"", + b"name": cluster.friendly_name.encode() if cluster.friendly_name else b"", b"firmware_version": cluster.host_version.encode() if cluster.host_version else b"", - b"printer_type": b"" + b"cluster_size": 1 # cloud devices are always clusters of at least one } super().__init__(device_id = cluster.cluster_id, address = "", @@ -95,6 +103,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): # We keep track of which printer is visible in the monitor page. self._active_printer = None # type: Optional[PrinterOutputModel] + self._host_machine_type = "" # Properties to populate later on with received cloud data. self._print_jobs = [] # type: List[UM3PrintJobOutputModel] @@ -234,6 +243,9 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): def _updatePrinters(self, printers: List[CloudClusterPrinterStatus]) -> None: previous = {p.key: p for p in self._printers} # type: Dict[str, PrinterOutputModel] received = {p.uuid: p for p in printers} # type: Dict[str, CloudClusterPrinterStatus] + + # We need the machine type of the host (1st list entry) to allow discovery to work. + self._host_machine_type = printers[0].machine_variant removed_printers, added_printers, updated_printers = findChanges(previous, received) @@ -358,6 +370,19 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): ).show() self.writeFinished.emit() + ## Gets the printer type of the cluster host. Falls back to the printer type in the device properties. + @pyqtProperty(str, notify=_clusterPrintersChanged) + def printerType(self) -> str: + if self._printers and self._host_machine_type in self._host_machine_variant_to_machine_type_map: + return self._host_machine_variant_to_machine_type_map[self._host_machine_type] + return super().printerType + + ## Gets the number of printers in the cluster. + # We use a minimum of 1 because cloud devices are always a cluster and printer discovery needs it. + @pyqtProperty(int, notify = _clusterPrintersChanged) + def clusterSize(self) -> int: + return max(1, len(self._printers)) + ## Gets the remote printers. @pyqtProperty("QVariantList", notify=_clusterPrintersChanged) def printers(self) -> List[PrinterOutputModel]: @@ -375,10 +400,6 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): self._active_printer = printer self.activePrinterChanged.emit() - @pyqtProperty(int, notify = _clusterPrintersChanged) - def clusterSize(self) -> int: - return len(self._printers) - ## Get remote print jobs. @pyqtProperty("QVariantList", notify = printJobsChanged) def printJobs(self) -> List[UM3PrintJobOutputModel]: diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py index 680caa568a..19ec34a6bb 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py @@ -7,7 +7,7 @@ from PyQt5.QtCore import QTimer from UM import i18nCatalog from UM.Logger import Logger from UM.Message import Message -from UM.Signal import Signal, signalemitter +from UM.Signal import Signal from cura.API import Account from cura.CuraApplication import CuraApplication from cura.Settings.GlobalStack import GlobalStack @@ -81,25 +81,61 @@ class CloudOutputDeviceManager: Logger.log("d", "Removed: %s, added: %s, updates: %s", len(removed_devices), len(added_clusters), len(updates)) # Remove output devices that are gone - for removed_cluster in removed_devices: - if removed_cluster.isConnected(): - removed_cluster.disconnect() - removed_cluster.close() - self._output_device_manager.removeOutputDevice(removed_cluster.key) - self.removedCloudCluster.emit(removed_cluster) - del self._remote_clusters[removed_cluster.key] + for device in removed_devices: + if device.isConnected(): + device.disconnect() + device.close() + self._output_device_manager.removeOutputDevice(device.key) + self._application.getDiscoveredPrintersModel().removeDiscoveredPrinter(device.key) + self.removedCloudCluster.emit(device) + del self._remote_clusters[device.key] # Add an output device for each new remote cluster. # We only add when is_online as we don't want the option in the drop down if the cluster is not online. - for added_cluster in added_clusters: - device = CloudOutputDevice(self._api, added_cluster) - self._remote_clusters[added_cluster.cluster_id] = device - self.addedCloudCluster.emit(added_cluster) + for cluster in added_clusters: + device = CloudOutputDevice(self._api, cluster) + self._remote_clusters[cluster.cluster_id] = device + self._application.getDiscoveredPrintersModel().addDiscoveredPrinter( + cluster.cluster_id, + device.key, + cluster.friendly_name, + self._createMachineFromDiscoveredPrinter, + device.printerType, + device + ) + self.addedCloudCluster.emit(cluster) + # Update the output devices for device, cluster in updates: device.clusterData = cluster + self._application.getDiscoveredPrintersModel().updateDiscoveredPrinter( + cluster.cluster_id, + cluster.friendly_name, + device.printerType, + ) self._connectToActiveMachine() + + def _createMachineFromDiscoveredPrinter(self, key: str) -> None: + device = self._remote_clusters[key] # type: CloudOutputDevice + if not device: + Logger.log("e", "Could not find discovered device with key [%s]", key) + return + + group_name = device.clusterData.friendly_name + machine_type_id = device.printerType + + Logger.log("i", "Creating machine from cloud device with key = [%s], group name = [%s], printer type = [%s]", + key, group_name, machine_type_id) + + # The newly added machine is automatically activated. + self._application.getMachineManager().addMachine(machine_type_id, group_name) + active_machine = CuraApplication.getInstance().getGlobalContainerStack() + if not active_machine: + return + + active_machine.setMetaDataEntry(self.META_CLUSTER_ID, device.key) + self._connectToOutputDevice(device, active_machine) ## Callback for when the active machine was changed by the user or a new remote cluster was found. def _connectToActiveMachine(self) -> None: diff --git a/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudClusterResponse.py b/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudClusterResponse.py index 48a4d5f031..5549da02aa 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudClusterResponse.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/Models/CloudClusterResponse.py @@ -16,7 +16,8 @@ class CloudClusterResponse(BaseCloudModel): # \param status: The status of the cluster authentication (active or inactive). # \param host_version: The firmware version of the cluster host. This is where the Stardust client is running on. def __init__(self, cluster_id: str, host_guid: str, host_name: str, is_online: bool, status: str, - host_internal_ip: Optional[str] = None, host_version: Optional[str] = None, **kwargs) -> None: + host_internal_ip: Optional[str] = None, host_version: Optional[str] = None, + friendly_name: Optional[str] = None, **kwargs) -> None: self.cluster_id = cluster_id self.host_guid = host_guid self.host_name = host_name @@ -24,6 +25,7 @@ class CloudClusterResponse(BaseCloudModel): self.is_online = is_online self.host_version = host_version self.host_internal_ip = host_internal_ip + self.friendly_name = friendly_name super().__init__(**kwargs) # Validates the model, raising an exception if the model is invalid. From 0028ec674637347c7c20c89847b44e165b46cd6b Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Thu, 18 Apr 2019 14:22:05 +0200 Subject: [PATCH 2/7] Ensure all property values are of type bytes --- plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index 43f0a71f6a..58005a7cc8 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -79,7 +79,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): b"address": cluster.host_internal_ip.encode() if cluster.host_internal_ip else b"", b"name": cluster.friendly_name.encode() if cluster.friendly_name else b"", b"firmware_version": cluster.host_version.encode() if cluster.host_version else b"", - b"cluster_size": 1 # cloud devices are always clusters of at least one + b"cluster_size": b"1" # cloud devices are always clusters of at least one } super().__init__(device_id = cluster.cluster_id, address = "", From 76d0b5f1980356e0ea4f616be244b18b94ee1c22 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Thu, 18 Apr 2019 14:26:38 +0200 Subject: [PATCH 3/7] Ensure printers length is larger than 0 before accessing index --- plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py index 58005a7cc8..4f89513e1e 100644 --- a/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py @@ -244,9 +244,10 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice): def _updatePrinters(self, printers: List[CloudClusterPrinterStatus]) -> None: previous = {p.key: p for p in self._printers} # type: Dict[str, PrinterOutputModel] received = {p.uuid: p for p in printers} # type: Dict[str, CloudClusterPrinterStatus] - - # We need the machine type of the host (1st list entry) to allow discovery to work. - self._host_machine_type = printers[0].machine_variant + + if len(printers) > 0: + # We need the machine type of the host (1st list entry) to allow discovery to work. + self._host_machine_type = printers[0].machine_variant removed_printers, added_printers, updated_printers = findChanges(previous, received) From 3b0cfc270f10fb01f9e60624668c560cdd19f212 Mon Sep 17 00:00:00 2001 From: ChrisTerBeke Date: Thu, 18 Apr 2019 14:43:53 +0200 Subject: [PATCH 4/7] Fix unit test for device name --- .../UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py b/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py index 2c401fab25..d11cfa8a0e 100644 --- a/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py +++ b/plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py @@ -22,6 +22,7 @@ class TestCloudOutputDevice(TestCase): HOST_NAME = "ultimakersystem-ccbdd30044ec" HOST_GUID = "e90ae0ac-1257-4403-91ee-a44c9b7e8050" HOST_VERSION = "5.2.0" + FRIENDLY_NAME = "My Friendly Printer" STATUS_URL = "{}/connect/v1/clusters/{}/status".format(CuraCloudAPIRoot, CLUSTER_ID) PRINT_URL = "{}/connect/v1/clusters/{}/print/{}".format(CuraCloudAPIRoot, CLUSTER_ID, JOB_ID) @@ -37,7 +38,8 @@ class TestCloudOutputDevice(TestCase): patched_method.start() self.cluster = CloudClusterResponse(self.CLUSTER_ID, self.HOST_GUID, self.HOST_NAME, is_online=True, - status="active", host_version=self.HOST_VERSION) + status="active", host_version=self.HOST_VERSION, + friendly_name=self.FRIENDLY_NAME) self.network = NetworkManagerMock() self.account = MagicMock(isLoggedIn=True, accessToken="TestAccessToken") @@ -60,7 +62,7 @@ class TestCloudOutputDevice(TestCase): # We test for these in order to make sure the correct file type is selected depending on the firmware version. def test_properties(self): self.assertEqual(self.device.firmwareVersion, self.HOST_VERSION) - self.assertEqual(self.device.name, self.HOST_NAME) + self.assertEqual(self.device.name, self.FRIENDLY_NAME) def test_status(self): self.device._update() From ba9c38a4a6cefdf764949fac5d8694d2ec7957ca Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 18 Apr 2019 14:43:39 +0200 Subject: [PATCH 5/7] Revert "Give more verbose output if a test failed (or succeeded)" This reverts commit 36cefcf0a313bc0db207539ff9b29d1aef03446a. Apparently it was already working correctly and I just wasn't looking properly. --- docker/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/build.sh b/docker/build.sh index 88fec8b869..eb20b18c0d 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -40,4 +40,4 @@ cmake3 \ -DBUILD_TESTS=ON \ .. make -ctest3 --verbose --output-on-failure -T Test +ctest3 --output-on-failure -T Test From f5668df1274c61f4036b20dba94873dec979eee2 Mon Sep 17 00:00:00 2001 From: Lipu Fei Date: Thu, 18 Apr 2019 15:54:21 +0200 Subject: [PATCH 6/7] Add parent to models so QML dont take ownership --- cura/CuraApplication.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index 0310526c2e..67ff984e03 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -214,14 +214,14 @@ class CuraApplication(QtApplication): self._cura_scene_controller = None self._machine_error_checker = None - self._machine_settings_manager = MachineSettingsManager(self) + self._machine_settings_manager = MachineSettingsManager(self, parent = self) - self._discovered_printer_model = DiscoveredPrintersModel(self) - self._first_start_machine_actions_model = FirstStartMachineActionsModel(self) - self._welcome_pages_model = WelcomePagesModel(self) - self._add_printer_pages_model = AddPrinterPagesModel(self) - self._whats_new_pages_model = WhatsNewPagesModel(self) - self._text_manager = TextManager(self) + self._discovered_printer_model = DiscoveredPrintersModel(parent = self) + self._first_start_machine_actions_model = FirstStartMachineActionsModel(self, parent = self) + self._welcome_pages_model = WelcomePagesModel(self, parent = self) + self._add_printer_pages_model = AddPrinterPagesModel(self, parent = self) + self._whats_new_pages_model = WhatsNewPagesModel(self, parent = self) + self._text_manager = TextManager(parent = self) self._quality_profile_drop_down_menu_model = None self._custom_quality_profile_drop_down_menu_model = None From a713cdc6bd65c7c78f72bb44842a5d4dd90f4c35 Mon Sep 17 00:00:00 2001 From: Diego Prado Gesto Date: Thu, 18 Apr 2019 17:26:01 +0200 Subject: [PATCH 7/7] Add new values for the resolution and deviation settings According to the experts --- resources/definitions/fdmprinter.def.json | 16 ++++++++-------- resources/definitions/ultimaker3.def.json | 1 - resources/definitions/ultimaker_s5.def.json | 3 ++- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/resources/definitions/fdmprinter.def.json b/resources/definitions/fdmprinter.def.json index b64d72d78d..9585e34476 100644 --- a/resources/definitions/fdmprinter.def.json +++ b/resources/definitions/fdmprinter.def.json @@ -5853,10 +5853,10 @@ "description": "The minimum size of a line segment after slicing. If you increase this, the mesh will have a lower resolution. This may allow the printer to keep up with the speed it has to process g-code and will increase slice speed by removing details of the mesh that it can't process anyway.", "type": "float", "unit": "mm", - "default_value": 0.20, + "default_value": 0.5, "minimum_value": "0.001", - "minimum_value_warning": "0.02", - "maximum_value_warning": "2", + "minimum_value_warning": "0.01", + "maximum_value_warning": "3", "settable_per_mesh": true }, "meshfix_maximum_travel_resolution": @@ -5865,8 +5865,8 @@ "description": "The minimum size of a travel line segment after slicing. If you increase this, the travel moves will have less smooth corners. This may allow the printer to keep up with the speed it has to process g-code, but it may cause model avoidance to become less accurate.", "type": "float", "unit": "mm", - "default_value": 0.5, - "value": "meshfix_maximum_resolution * speed_travel / speed_print", + "default_value": 1.0, + "value": "min(meshfix_maximum_resolution * speed_travel / speed_print, 2 * line_width)", "minimum_value": "0.001", "minimum_value_warning": "0.05", "maximum_value_warning": "10", @@ -5879,10 +5879,10 @@ "description": "The maximum deviation allowed when reducing the resolution for the Maximum Resolution setting. If you increase this, the print will be less accurate, but the g-code will be smaller.", "type": "float", "unit": "mm", - "default_value": 0.005, + "default_value": 0.05, "minimum_value": "0.001", - "minimum_value_warning": "0.003", - "maximum_value_warning": "0.1", + "minimum_value_warning": "0.01", + "maximum_value_warning": "0.3", "settable_per_mesh": true }, "support_skip_some_zags": diff --git a/resources/definitions/ultimaker3.def.json b/resources/definitions/ultimaker3.def.json index 2b5a5e6dc5..368a41e0a9 100644 --- a/resources/definitions/ultimaker3.def.json +++ b/resources/definitions/ultimaker3.def.json @@ -118,7 +118,6 @@ "material_bed_temperature": { "maximum_value": "115" }, "material_bed_temperature_layer_0": { "maximum_value": "115" }, "material_standby_temperature": { "value": "100" }, - "meshfix_maximum_resolution": { "value": "0.04" }, "multiple_mesh_overlap": { "value": "0" }, "optimize_wall_printing_order": { "value": "True" }, "prime_tower_enable": { "default_value": true }, diff --git a/resources/definitions/ultimaker_s5.def.json b/resources/definitions/ultimaker_s5.def.json index 3f24673e6d..f76ce77619 100644 --- a/resources/definitions/ultimaker_s5.def.json +++ b/resources/definitions/ultimaker_s5.def.json @@ -156,7 +156,8 @@ "wall_0_inset": { "value": "0" }, "wall_line_width_x": { "value": "round(line_width * 0.3 / 0.35, 2)" }, "wall_thickness": { "value": "1" }, - "meshfix_maximum_resolution": { "value": "0.04" }, + "meshfix_maximum_resolution": { "value": "(speed_wall_0 + speed_wall_x) / 60" }, + "meshfix_maximum_deviation": { "value": "layer_height / 2" }, "optimize_wall_printing_order": { "value": "True" }, "retraction_combing": { "default_value": "all" }, "initial_layer_line_width_factor": { "value": "120" },