Cleanup

plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py

@@ -169,14 +169,16 @@ class CloudApiClient:
                                         Callable[[List[CloudApiClientModel]], Any]],
                      model: Type[CloudApiClientModel],
                      ) -> None:
+        
         def parse() -> None:
+            self._anti_gc_callbacks.remove(parse)
+            
             # Don't try to parse the reply if we didn't get one
             if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) is None:
                 return
+            
             status_code, response = self._parseReply(reply)
-            self._anti_gc_callbacks.remove(parse)
             self._parseModels(response, on_finished, model)
-            return
 
         self._anti_gc_callbacks.append(parse)
         reply.finished.connect(parse)

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py

@@ -1,11 +1,10 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
-from typing import Dict, List
+from typing import Dict, List, Optional
 
 from PyQt5.QtCore import QTimer
 
 from UM import i18nCatalog
-from UM.Logger import Logger
 from UM.Signal import Signal
 from cura.API import Account
 from cura.CuraApplication import CuraApplication
@@ -14,14 +13,11 @@ from cura.Settings.GlobalStack import GlobalStack
 from .CloudApiClient import CloudApiClient
 from .CloudOutputDevice import CloudOutputDevice
 from ..Models.Http.CloudClusterResponse import CloudClusterResponse
-from ..Models.Http.CloudError import CloudError
 
 
-##  The cloud output device manager is responsible for using the Ultimaker Cloud APIs to manage remote clusters.
+## The cloud output device manager is responsible for using the Ultimaker Cloud APIs to manage remote clusters.
-#   Keeping all cloud related logic in this class instead of the UM3OutputDevicePlugin results in more readable code.
+#  Keeping all cloud related logic in this class instead of the UM3OutputDevicePlugin results in more readable code.
-#
+#  API spec is available on https://api.ultimaker.com/docs/connect/spec/.
-#   API spec is available on https://api.ultimaker.com/docs/connect/spec/.
-#
 class CloudOutputDeviceManager:
 
     META_CLUSTER_ID = "um_cloud_cluster_id"
@@ -40,7 +36,7 @@ class CloudOutputDeviceManager:
         # Persistent dict containing the remote clusters for the authenticated user.
         self._remote_clusters = {}  # type: Dict[str, CloudOutputDevice]
         self._account = CuraApplication.getInstance().getCuraAPI().account  # type: Account
-        self._api = CloudApiClient(self._account, self._onApiError)
+        self._api = CloudApiClient(self._account, on_error=lambda error: print(error))
         self._account.loginStateChanged.connect(self._onLoginStateChanged)
 
         # Create a timer to update the remote cluster list
@@ -90,41 +86,51 @@ class CloudOutputDeviceManager:
 
     ## Callback for when the request for getting the clusters is finished.
     def _onGetRemoteClustersFinished(self, clusters: List[CloudClusterResponse]) -> None:
-
-        # Filter on clusters that are currently online.
         online_clusters = {c.cluster_id: c for c in clusters if c.is_online}  # type: Dict[str, CloudClusterResponse]
-
-        # Keep track of the new cloud clusters to show.
-        # We create a new list instead of changing the existing one to prevent issues with ordering.
-        new_devices = {}  # type: Dict[str, CloudOutputDevice]
-
-        # Get the discovery mechanism of Cura.
-        discovery = CuraApplication.getInstance().getDiscoveredPrintersModel()
-
-        # Check which devices need to be created or updated.
         for device_id, cluster_data in online_clusters.items():
-            device = next(iter(device for device in self._remote_clusters.values() if device.key == device_id), None)
+            if device_id not in self._remote_clusters:
-            if not device:
+                self._onDeviceDiscovered(cluster_data)
-                device = CloudOutputDevice(self._api, cluster_data)
-                discovery.addDiscoveredPrinter(device.key, device.key, cluster_data.friendly_name,
-                                               self._createMachineFromDiscoveredDevice, device.printerType, device)
             else:
-                discovery.updateDiscoveredPrinter(device.key, cluster_data.friendly_name, device.printerType)
+                self._onDiscoveredDeviceUpdated(cluster_data)
-            new_devices[device.key] = device
 
-        # Remove output devices that disappeared.
+        removed_device_keys = set(self._remote_clusters.keys()) - set(online_clusters.keys())
-        keys = new_devices.keys()
+        for device_id in removed_device_keys:
-        removed_devices = [cluster for cluster in self._remote_clusters.values() if cluster.key not in keys]
+            self._onDiscoveredDeviceRemoved(device_id)
-        for device in removed_devices:
-            device.disconnect()
-            device.close()
-            CuraApplication.getInstance().getOutputDeviceManager().removeOutputDevice(device.key)
-            discovery.removeDiscoveredPrinter(device.key)
 
-        self._remote_clusters = new_devices
+    def _onDeviceDiscovered(self, cluster_data: CloudClusterResponse) -> None:
+        device = CloudOutputDevice(self._api, cluster_data)
+        CuraApplication.getInstance().getDiscoveredPrintersModel().addDiscoveredPrinter(
+            ip_address=device.key,
+            key=device.getId(),
+            name=device.getName(),
+            create_callback=self._createMachineFromDiscoveredDevice,
+            machine_type=device.printerType,
+            device=device
+        )
+        self._remote_clusters[device.getId()] = device
         self.discoveredDevicesChanged.emit()
         self._connectToActiveMachine()
 
+    def _onDiscoveredDeviceUpdated(self, cluster_data: CloudClusterResponse) -> None:
+        device = self._remote_clusters.get(cluster_data.cluster_id)
+        if not device:
+            return
+        CuraApplication.getInstance().getDiscoveredPrintersModel().updateDiscoveredPrinter(
+            ip_address=device.key,
+            name=cluster_data.friendly_name,
+            machine_type=device.printerType
+        )
+        self.discoveredDevicesChanged.emit()
+
+    def _onDiscoveredDeviceRemoved(self, device_id: str) -> None:
+        device = self._remote_clusters.pop(device_id, None)  # type: Optional[CloudOutputDevice]
+        if not device:
+            return
+        device.disconnect()
+        device.close()
+        CuraApplication.getInstance().getDiscoveredPrintersModel().removeDiscoveredPrinter(device.key)
+        self.discoveredDevicesChanged.emit()
+
     def _createMachineFromDiscoveredDevice(self, key: str) -> None:
         device = self._remote_clusters[key]
         if not device:
@@ -165,10 +171,3 @@ class CloudOutputDeviceManager:
         device.connect()
         active_machine.addConfiguredConnectionType(device.connectionType.value)
         CuraApplication.getInstance().getOutputDeviceManager().addOutputDevice(device)
-
-    ## Handles an API error received from the cloud.
-    #  \param errors: The errors received
-    @staticmethod
-    def _onApiError(errors: List[CloudError] = None) -> None:
-        for error in errors:
-            Logger.log("w", str(error.toDict()))

plugins/UM3NetworkPrinting/src/Network/ClusterApiClient.py

@@ -5,13 +5,14 @@ from json import JSONDecodeError
 from typing import Callable, List, Optional, Dict, Union, Any, Type, cast, TypeVar, Tuple
 
 from PyQt5.QtCore import QUrl
-from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkRequest, QNetworkReply
+from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkRequest, QNetworkReply, QNetworkConfiguration
 
 from UM.Logger import Logger
 
 from ..Models.BaseModel import BaseModel
 from ..Models.Http.ClusterPrintJobStatus import ClusterPrintJobStatus
 from ..Models.Http.ClusterPrinterStatus import ClusterPrinterStatus
+from ..Models.Http.PrinterSystemStatus import PrinterSystemStatus
 
 
 ## The generic type variable used to document the methods below.
@@ -21,11 +22,8 @@ ClusterApiClientModel = TypeVar("ClusterApiClientModel", bound=BaseModel)
 ## The ClusterApiClient is responsible for all network calls to local network clusters.
 class ClusterApiClient:
 
-    PRINTER_API_VERSION = "1"
+    PRINTER_API_PREFIX = "/api/v1"
-    PRINTER_API_PREFIX = "/api/v" + PRINTER_API_VERSION
+    CLUSTER_API_PREFIX = "/cluster-api/v1"
-
-    CLUSTER_API_VERSION = "1"
-    CLUSTER_API_PREFIX = "/cluster-api/v" + CLUSTER_API_VERSION
 
     ## Initializes a new cluster API client.
     #  \param address: The network address of the cluster to call.
@@ -43,7 +41,8 @@ class ClusterApiClient:
     #  \param on_finished: The callback in case the response is successful.
     def getSystem(self, on_finished: Callable) -> None:
         url = "{}/system/".format(self.PRINTER_API_PREFIX)
-        self._manager.get(self._createEmptyRequest(url))
+        reply = self._manager.get(self._createEmptyRequest(url))
+        self._addCallback(reply, on_finished, PrinterSystemStatus)
 
     ## Get the printers in the cluster.
     #  \param on_finished: The callback in case the response is successful.
@@ -132,12 +131,17 @@ class ClusterApiClient:
                                         Callable[[List[ClusterApiClientModel]], Any]],
                      model: Optional[Type[ClusterApiClientModel]] = None,
                      ) -> None:
+        
         def parse() -> None:
+            self._anti_gc_callbacks.remove(parse)
+            
             # Don't try to parse the reply if we didn't get one
             if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) is None:
                 return
-
+            
-            self._anti_gc_callbacks.remove(parse)
+            if reply.error() > 0:
+                self._on_error(reply.errorString())
+                return
 
             # If no parse model is given, simply return the raw data in the callback.
             if not model:

plugins/UM3NetworkPrinting/src/Network/NetworkOutputDevice.py

@@ -1,14 +1,13 @@
 # Copyright (c) 2019 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
-from typing import Optional, Dict, List, Any
+from typing import Optional, Dict, List
 
-from PyQt5.QtGui import QDesktopServices, QImage
+from PyQt5.QtGui import QDesktopServices
 from PyQt5.QtCore import pyqtSlot, QUrl, pyqtSignal, pyqtProperty
 from PyQt5.QtNetwork import QNetworkReply
 
 from UM.FileHandler.FileHandler import FileHandler
 from UM.FileHandler.WriteFileJob import WriteFileJob
-from UM.Logger import Logger
 from UM.Message import Message
 from UM.i18n import i18nCatalog
 from UM.Scene.SceneNode import SceneNode
@@ -39,7 +38,7 @@ class NetworkOutputDevice(UltimakerNetworkedPrinterOutputDevice):
         )
 
         # API client for making requests to the print cluster.
-        self._cluster_api = ClusterApiClient(address, on_error=self._onNetworkError)
+        self._cluster_api = ClusterApiClient(address, on_error=lambda error: print(error))
         # We don't have authentication over local networking, so we're always authenticated.
         self.setAuthenticationState(AuthState.Authenticated)
         self._setInterfaceElements()
@@ -95,11 +94,6 @@ class NetworkOutputDevice(UltimakerNetworkedPrinterOutputDevice):
     def setJobState(self, print_job_uuid: str, action: str) -> None:
         self._cluster_api.setPrintJobState(print_job_uuid, action)
 
-    ## Handle network errors.
-    @staticmethod
-    def _onNetworkError(errors: Dict[str, Any]):
-        Logger.log("w", str(errors))
-
     def _update(self) -> None:
         super()._update()
         self._cluster_api.getPrinters(self._updatePrinters)
@@ -152,7 +146,7 @@ class NetworkOutputDevice(UltimakerNetworkedPrinterOutputDevice):
         self.writeProgress.emit()
 
     ## Handler for when the print job was fully uploaded to the cluster.
-    def _onPrintUploadCompleted(self, reply: QNetworkReply) -> None:
+    def _onPrintUploadCompleted(self, _: QNetworkReply) -> None:
         self._progress.hide()
         Message(
             text=I18N_CATALOG.i18nc("@info:status", "Print job was successfully sent to the printer."),

plugins/UM3NetworkPrinting/src/Network/NetworkOutputDeviceManager.py

@@ -14,6 +14,7 @@ from cura.Settings.GlobalStack import GlobalStack
 from .ZeroConfClient import ZeroConfClient
 from .ClusterApiClient import ClusterApiClient
 from .NetworkOutputDevice import NetworkOutputDevice
+from ..Models.Http.PrinterSystemStatus import PrinterSystemStatus
 
 
 ## The NetworkOutputDeviceManager is responsible for discovering and managing local networked clusters.
@@ -64,18 +65,8 @@ class NetworkOutputDeviceManager:
         self._manual_instances[address] = callback
         new_manual_devices = ",".join(self._manual_instances.keys())
         CuraApplication.getInstance().getPreferences().setValue(self.MANUAL_DEVICES_PREFERENCE_KEY, new_manual_devices)
-
+        api_client = ClusterApiClient(address, self._onApiError)
-        device_id = "manual:{}".format(address)
+        api_client.getSystem(lambda status: self._onCheckManualDeviceResponse(address, status))
-        if device_id not in self._discovered_devices:
-            self._onDeviceDiscovered(device_id, address, {
-                b"name": address.encode("utf-8"),
-                b"address": address.encode("utf-8"),
-                b"manual": b"true",
-                b"incomplete": b"true",
-                b"temporary": b"true"
-            })
-        self._checkManualDevice(address, lambda status_code, response: self._onCheckManualDeviceResponse(
-                status_code, address))
 
     ## Remove a manually added networked printer.
     def removeManualDevice(self, device_id: str, address: Optional[str] = None) -> None:
@@ -119,19 +110,19 @@ class NetworkOutputDeviceManager:
         active_machine.addConfiguredConnectionType(device.connectionType.value)
         CuraApplication.getInstance().getOutputDeviceManager().addOutputDevice(device)
 
-    ## Checks if a networked printer exists at the given address.
-    #  If the printer responds it will replace the preliminary printer created from the stored manual instances.
-    def _checkManualDevice(self, address: str, on_finished: Callable) -> None:
-        api_client = ClusterApiClient(address, self._onApiError)
-        api_client.getSystem(on_finished)
-
     ## Callback for when a manual device check request was responded to.
-    def _onCheckManualDeviceResponse(self, status_code: int, address: str) -> None:
+    def _onCheckManualDeviceResponse(self, address: str, status: PrinterSystemStatus) -> None:
-        Logger.log("d", "manual device check response: {} {}".format(status_code, address))
+        callback = self._manual_instances.get(address, None)
-        if address in self._manual_instances:
+        if callback is None:
-            callback = self._manual_instances[address]
+            return
-            if callback is not None:
+        self._onDeviceDiscovered("manual:{}".format(address), address, {
-                CuraApplication.getInstance().callLater(callback, status_code == 200, address)
+            b"name": status.name.encode("utf-8"),
+            b"address": address.encode("utf-8"),
+            b"manual": b"true",
+            b"incomplete": b"true",
+            b"temporary": b"true"
+        })
+        CuraApplication.getInstance().callLater(callback, True, address)
 
     ## Returns a dict of printer BOM numbers to machine types.
     #  These numbers are available in the machine definition already so we just search for them here.
@@ -179,9 +170,10 @@ class NetworkOutputDeviceManager:
 
     ## Remove a device.
     def _onDiscoveredDeviceRemoved(self, device_id: str) -> None:
-        device = self._discovered_devices.pop(device_id, None)
+        device = self._discovered_devices.pop(device_id, None)  # type: Optional[NetworkOutputDevice]
         if not device:
             return
+        device.close()
         CuraApplication.getInstance().getDiscoveredPrintersModel().removeDiscoveredPrinter(device.address)
         self.discoveredDevicesChanged.emit()
 

plugins/UM3NetworkPrinting/src/UM3OutputDevicePlugin.py

@@ -44,7 +44,7 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
 
     ## Indicate that this plugin supports adding networked printers manually.
     def canAddManualDevice(self, address: str = "") -> ManualDeviceAdditionAttempt:
-        return ManualDeviceAdditionAttempt.POSSIBLE
+        return ManualDeviceAdditionAttempt.PRIORITY
 
     ## Add a networked printer manually based on its network address.
     def addManualDevice(self, address: str, callback: Optional[Callable[[bool, str], None]] = None) -> None:

plugins/UM3NetworkPrinting/src/UltimakerNetworkedPrinterOutputDevice.py

@@ -4,7 +4,6 @@ import os
 from typing import List, Optional, Dict
 
 from PyQt5.QtCore import pyqtProperty, pyqtSignal, QObject, pyqtSlot, QUrl
-from PyQt5.QtGui import QImage
 
 from UM.Logger import Logger
 from UM.Qt.Duration import Duration, DurationFormat