STAR-322: Fixing printer matching by network key

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py

@@ -18,7 +18,6 @@ from cura.PrinterOutput.PrinterOutputModel import PrinterOutputModel
 from ..MeshFormatHandler import MeshFormatHandler
 from ..UM3PrintJobOutputModel import UM3PrintJobOutputModel
 from .CloudApiClient import CloudApiClient
-from .Models.CloudErrorObject import CloudErrorObject
 from .Models.CloudClusterStatus import CloudClusterStatus
 from .Models.CloudJobUploadRequest import CloudJobUploadRequest
 from .Models.CloudPrintResponse import CloudPrintResponse
@@ -63,19 +62,21 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     # The interval with which the remote clusters are checked
     CHECK_CLUSTER_INTERVAL = 2.0  # seconds
 
-    # Signal triggered when the printers in the remote cluster were changed.
-    clusterPrintersChanged = pyqtSignal()
-
     # Signal triggered when the print jobs in the queue were changed.
     printJobsChanged = pyqtSignal()
 
+    # Notify can only use signals that are defined by the class that they are in, not inherited ones.
+    # Therefore we create a private signal used to trigger the printersChanged signal.
+    _clusterPrintersChanged = pyqtSignal()
+
     ## Creates a new cloud output device
     #  \param api_client: The client that will run the API calls
     #  \param device_id: The ID of the device (i.e. the cluster_id for the cloud API)
     #  \param parent: The optional parent of this output device.
-    def __init__(self, api_client: CloudApiClient, device_id: str, parent: QObject = None) -> None:
+    def __init__(self, api_client: CloudApiClient, device_id: str, host_name: str, parent: QObject = None) -> None:
         super().__init__(device_id = device_id, address = "", properties = {}, parent = parent)
         self._api = api_client
+        self._host_name = host_name
 
         self._setInterfaceElements()
         
@@ -88,6 +89,9 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         self._control_view_qml_path = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                                                    "../../resources/qml/ClusterControlItem.qml")
 
+        # trigger the printersChanged signal when the private signal is triggered
+        self.printersChanged.connect(self._clusterPrintersChanged)
+
         # Properties to populate later on with received cloud data.
         self._print_jobs = []  # type: List[UM3PrintJobOutputModel]
         self._number_of_extruders = 2  # All networked printers are dual-extrusion Ultimaker machines.
@@ -96,6 +100,22 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         self._sending_job = False
         self._progress_message = None  # type: Optional[Message]
 
+    ## Gets the host name of this device
+    @property
+    def host_name(self) -> str:
+        return self._host_name
+
+    ## Updates the host name of the output device
+    @host_name.setter
+    def host_name(self, value: str) -> None:
+        self._host_name = value
+
+    ## Checks whether the given network key is found in the cloud's host name
+    def matchesNetworkKey(self, network_key: str) -> bool:
+        # A network key looks like "ultimakersystem-aabbccdd0011._ultimaker._tcp.local."
+        # the host name should then be "ultimakersystem-aabbccdd0011"
+        return network_key.startswith(self._host_name)
+
     ##  Set all the interface elements and texts for this output device.
     def _setInterfaceElements(self):
         self.setPriority(2)  # make sure we end up below the local networking and above 'save to file'
@@ -133,7 +153,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         self._api.requestUpload(request, lambda response: self._onPrintJobCreated(mesh_bytes, response))
 
     ##  Get remote printers.
-    @pyqtProperty("QVariantList", notify = clusterPrintersChanged)
+    @pyqtProperty("QVariantList", notify = _clusterPrintersChanged)
     def printers(self):
         return self._printers
 
@@ -196,7 +216,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         for updated_printer_guid in current_printer_ids.intersection(remote_printer_ids):
             remote_printers[updated_printer_guid].updateOutputModel(current_printers[updated_printer_guid])
 
-        self.clusterPrintersChanged.emit()
+        self._clusterPrintersChanged.emit()
 
     def _updatePrintJobs(self, jobs: List[CloudClusterPrintJob]) -> None:
         remote_jobs = {j.uuid: j for j in jobs}  # type: Dict[str, CloudClusterPrintJob]
@@ -283,7 +303,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     ##  TODO: The following methods are required by the monitor page QML, but are not actually available using cloud.
     #   TODO: We fake the methods here to not break the monitor page.
 
-    @pyqtProperty(QObject, notify = clusterPrintersChanged)
+    @pyqtProperty(QObject, notify = _clusterPrintersChanged)
     def activePrinter(self) -> Optional[PrinterOutputModel]:
         if not self._printers:
             return None
@@ -293,7 +313,7 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     def setActivePrinter(self, printer: Optional[PrinterOutputModel]) -> None:
         pass
 
-    @pyqtProperty(QUrl, notify = clusterPrintersChanged)
+    @pyqtProperty(QUrl, notify = _clusterPrintersChanged)
     def activeCameraUrl(self) -> "QUrl":
         return QUrl()
 
@@ -304,6 +324,3 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     @pyqtProperty(bool, notify = printJobsChanged)
     def receivedPrintJobs(self) -> bool:
         return True
-
-    def _onApiError(self, errors: List[CloudErrorObject]) -> None:
-        pass  # TODO: Show errors...

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py

@@ -1,6 +1,6 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
-from typing import Dict, List, Optional
+from typing import Dict, List
 
 from PyQt5.QtCore import QTimer
 
@@ -8,10 +8,12 @@ from UM import i18nCatalog
 from UM.Logger import Logger
 from UM.Message import Message
 from cura.CuraApplication import CuraApplication
+from cura.Settings.GlobalStack import GlobalStack
 from .CloudApiClient import CloudApiClient
 from .CloudOutputDevice import CloudOutputDevice
 from .Models.CloudCluster import CloudCluster
 from .Models.CloudErrorObject import CloudErrorObject
+from .Utils import findChanges
 
 
 ##  The cloud output device manager is responsible for using the Ultimaker Cloud APIs to manage remote clusters.
@@ -19,7 +21,9 @@ from .Models.CloudErrorObject import CloudErrorObject
 #
 #   API spec is available on https://api.ultimaker.com/docs/connect/spec/.
 #
+
 class CloudOutputDeviceManager:
+    META_CLUSTER_ID = "um_cloud_cluster_id"
 
     # The interval with which the remote clusters are checked
     CHECK_CLUSTER_INTERVAL = 5.0  # seconds
@@ -43,57 +47,46 @@ class CloudOutputDeviceManager:
         # When switching machines we check if we have to activate a remote cluster.
         application.globalContainerStackChanged.connect(self._connectToActiveMachine)
 
-        self.update_timer = QTimer(CuraApplication.getInstance())
-        self.update_timer.setInterval(self.CHECK_CLUSTER_INTERVAL * 1000)
-        self.update_timer.setSingleShot(False)
-        self.update_timer.timeout.connect(self._getRemoteClusters)
+        # create a timer to update the remote cluster list
+        self._update_timer = QTimer(application)
+        self._update_timer.setInterval(self.CHECK_CLUSTER_INTERVAL * 1000)
+        self._update_timer.setSingleShot(False)
+        self._update_timer.timeout.connect(self._getRemoteClusters)
 
     ##  Gets all remote clusters from the API.
     def _getRemoteClusters(self) -> None:
         Logger.log("i", "Retrieving remote clusters")
         if self._account.isLoggedIn:
             self._api.getClusters(self._onGetRemoteClustersFinished)
-
             # Only start the polling timer after the user is authenticated
             # The first call to _getRemoteClusters comes from self._account.loginStateChanged
-        if not self.update_timer.isActive():
-            self.update_timer.start()
+            if not self._update_timer.isActive():
+                self._update_timer.start()
 
     ##  Callback for when the request for getting the clusters. is finished.
     def _onGetRemoteClustersFinished(self, clusters: List[CloudCluster]) -> None:
-        found_clusters = {c.cluster_id: c for c in clusters}
+        online_clusters = {c.cluster_id: c for c in clusters if c.is_online}  # type: Dict[str, CloudCluster]
 
-        Logger.log("i", "Parsed remote clusters to %s", found_clusters)
+        removed_devices, added_clusters, updates = findChanges(self._remote_clusters, online_clusters)
 
-        known_cluster_ids = set(self._remote_clusters.keys())
-        found_cluster_ids = set(found_clusters.keys())
+        Logger.log("i", "Parsed remote clusters to %s", online_clusters)
+
+        # Remove output devices that are gone
+        for removed_cluster in removed_devices:
+            self._output_device_manager.removeOutputDevice(removed_cluster.key)
+            del self._remote_clusters[removed_cluster.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 cluster_id in found_cluster_ids.difference(known_cluster_ids):
-            if found_clusters[cluster_id].is_online:
-                self._addCloudOutputDevice(found_clusters[cluster_id])
-
-        # Remove output devices that are gone
-        for cluster_id in known_cluster_ids.difference(found_cluster_ids):
-            self._removeCloudOutputDevice(found_clusters[cluster_id])
-
-        # TODO: not pass clusters that are not online?
-        self._connectToActiveMachine()
-
-    ##  Adds a CloudOutputDevice for each entry in the remote cluster list from the API.
-    #   \param cluster: The cluster that was added.
-    def _addCloudOutputDevice(self, cluster: CloudCluster):
-        device = CloudOutputDevice(self._api, cluster.cluster_id)
+        for added_cluster in added_clusters:
+            device = CloudOutputDevice(self._api, added_cluster.cluster_id, added_cluster.host_name)
             self._output_device_manager.addOutputDevice(device)
-        self._remote_clusters[cluster.cluster_id] = device
+            self._remote_clusters[added_cluster.cluster_id] = device
 
-    ##  Remove a CloudOutputDevice
-    #   \param cluster: The cluster that was removed
-    def _removeCloudOutputDevice(self, cluster: CloudCluster):
-        self._output_device_manager.removeOutputDevice(cluster.cluster_id)
-        if cluster.cluster_id in self._remote_clusters:
-            del self._remote_clusters[cluster.cluster_id]
+        for device, cluster in updates:
+            device.host_name = cluster.host_name
+
+        self._connectToActiveMachine()
 
     ##  Callback for when the active machine was changed by the user or a new remote cluster was found.
     def _connectToActiveMachine(self) -> None:
@@ -102,23 +95,27 @@ class CloudOutputDeviceManager:
             return
 
         # Check if the stored cluster_id for the active machine is in our list of remote clusters.
-        stored_cluster_id = active_machine.getMetaDataEntry("um_cloud_cluster_id")
-        if stored_cluster_id in self._remote_clusters.keys():
-            return self._remote_clusters.get(stored_cluster_id).connect()
+        stored_cluster_id = active_machine.getMetaDataEntry(self.META_CLUSTER_ID)
+        if stored_cluster_id in self._remote_clusters:
+            device = self._remote_clusters[stored_cluster_id]
+            if not device.isConnected():
+                device.connect()
+        else:
+            self._connectByNetworkKey(active_machine)
 
+    ##  Tries to match the
+    def _connectByNetworkKey(self, active_machine: GlobalStack) -> None:
         # Check if the active printer has a local network connection and match this key to the remote cluster.
-        # The local network key is formatted as ultimakersystem-xxxxxxxxxxxx._ultimaker._tcp.local.
-        # The optional remote host_name is formatted as ultimakersystem-xxxxxxxxxxxx.
-        # This means we can match the two by checking if the host_name is in the network key string.
-
         local_network_key = active_machine.getMetaDataEntry("um_network_key")
         if not local_network_key:
             return
 
-        # TODO: get host_name in the output device so we can iterate here
-        # cluster_id = next(local_network_key in cluster.host_name for cluster in self._remote_clusters.items())
-        # if cluster_id in self._remote_clusters.keys():
-        #     return self._remote_clusters.get(cluster_id).connect()
+        device = next((c for c in self._remote_clusters.values() if c.matchesNetworkKey(local_network_key)), None)
+        if not device:
+            return
+
+        active_machine.setMetaDataEntry(self.META_CLUSTER_ID, device.key)
+        return device.connect()
 
     ## Handles an API error received from the cloud.
     #  \param errors: The errors received

plugins/UM3NetworkPrinting/src/Cloud/Models/CloudCluster.py

@@ -11,7 +11,7 @@ class CloudCluster(BaseModel):
         self.host_name = None  # type: str
         self.host_version = None  # type: str
         self.status = None  # type: str
-        self.is_online = None  # type: bool
+        self.is_online = False  # type: bool
         super().__init__(**kwargs)
 
     # Validates the model, raising an exception if the model is invalid.

plugins/UM3NetworkPrinting/src/Cloud/Utils.py

@@ -0,0 +1,19 @@
+from typing import TypeVar, Dict, Tuple, List
+
+T = TypeVar("T")
+U = TypeVar("U")
+
+
+def findChanges(previous: Dict[str, T], received: Dict[str, U]) -> Tuple[List[T], List[U], List[Tuple[T, U]]]:
+    previous_ids = set(previous)
+    received_ids = set(received)
+
+    removed_ids = previous_ids.difference(received_ids)
+    new_ids = received_ids.difference(previous_ids)
+    updated_ids = received_ids.intersection(previous_ids)
+
+    removed = [previous[removed_id] for removed_id in removed_ids]
+    added = [received[new_id] for new_id in new_ids]
+    updated = [(previous[updated_id], received[updated_id]) for updated_id in updated_ids]
+
+    return removed, added, updated

plugins/UM3NetworkPrinting/src/ClusterUM3OutputDevice.py

@@ -45,8 +45,8 @@ class ClusterUM3OutputDevice(NetworkedPrinterOutputDevice):
     activeCameraUrlChanged = pyqtSignal()
     receivedPrintJobsChanged = pyqtSignal()
 
-    # This is a bit of a hack, as the notify can only use signals that are defined by the class that they are in.
-    # Inheritance doesn't seem to work. Tying them together does work, but i'm open for better suggestions.
+    # Notify can only use signals that are defined by the class that they are in, not inherited ones.
+    # Therefore we create a private signal used to trigger the printersChanged signal.
     _clusterPrintersChanged = pyqtSignal()
 
     def __init__(self, device_id, address, properties, parent = None) -> None:
@@ -62,7 +62,7 @@ class ClusterUM3OutputDevice(NetworkedPrinterOutputDevice):
 
         self._monitor_view_qml_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "../resources/qml/ClusterMonitorItem.qml")
 
-        # See comments about this hack with the clusterPrintersChanged signal
+        # trigger the printersChanged signal when the private signal is triggered
         self.printersChanged.connect(self._clusterPrintersChanged)
 
         self._accepts_commands = True  # type: bool