Simply manual device checking

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py

@@ -112,6 +112,8 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice):
 
     ## Disconnects the device
     def disconnect(self) -> None:
+        if not self.isConnected():
+            return
         super().disconnect()
         Logger.log("i", "Disconnected from cluster %s", self.key)
         CuraApplication.getInstance().getBackend().backendStateChange.disconnect(self._onBackendStateChange)
@@ -201,7 +203,6 @@ class CloudOutputDevice(UltimakerNetworkedPrinterOutputDevice):
         if self._received_printers != status.printers:
             self._received_printers = status.printers
             self._updatePrinters(status.printers)
-
         if status.print_jobs != self._received_print_jobs:
             self._received_print_jobs = status.print_jobs
             self._updatePrintJobs(status.print_jobs)

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py

@@ -15,7 +15,6 @@ from .CloudApiClient import CloudApiClient
 from .CloudOutputDevice import CloudOutputDevice
 from ..Models.Http.CloudClusterResponse import CloudClusterResponse
 from ..Models.Http.CloudError import CloudError
-from ..Utils import findChanges
 
 
 ##  The cloud output device manager is responsible for using the Ultimaker Cloud APIs to manage remote clusters.
@@ -33,8 +32,8 @@ class CloudOutputDeviceManager:
     # The translation catalog for this device.
     I18N_CATALOG = i18nCatalog("cura")
 
-    addedCloudCluster = Signal()
+    # Signal emitted when the list of discovered devices changed.
-    removedCloudCluster = Signal()
+    discoveredDevicesChanged = Signal()
 
     def __init__(self) -> None:
         # Persistent dict containing the remote clusters for the authenticated user.
@@ -79,7 +78,6 @@ class CloudOutputDeviceManager:
         else:
             if self._update_timer.isActive():
                 self._update_timer.stop()
-
             # Notify that all clusters have disappeared
             self._onGetRemoteClustersFinished([])
 
@@ -87,45 +85,39 @@ class CloudOutputDeviceManager:
     def _getRemoteClusters(self) -> None:
         self._api.getClusters(self._onGetRemoteClustersFinished)
 
-    ## Callback for when the request for getting the clusters. is finished.
+    ## 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]
-        removed_devices, added_clusters, updates = findChanges(self._remote_clusters, online_clusters)
 
-        # Remove output devices that are gone
+        # 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 not device:
+                device = CloudOutputDevice(self._api, cluster_data)
+                discovery.addDiscoveredPrinter(device.key, device.key, cluster_data.friendly_name,
+                                               self._createMachineFromDiscoveredPrinter, device.printerType, device)
+            else:
+                discovery.updateDiscoveredPrinter(device.key, cluster_data.friendly_name, device.printerType)
+            new_devices[device.key] = device
+
+        # Remove output devices that disappeared.
+        remote_cluster_keys = self._remote_clusters.keys()
+        removed_devices = [cluster for cluster in self._remote_clusters.values() if cluster.key not in remote_cluster_keys]
         for device in removed_devices:
-            if device.isConnected():
-                device.disconnect()
-                device.close()
             CuraApplication.getInstance().getOutputDeviceManager().removeOutputDevice(device.key)
-            CuraApplication.getInstance().getDiscoveredPrintersModel().removeDiscoveredPrinter(device.key)
+            discovery.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 cluster in added_clusters:
-            device = CloudOutputDevice(self._api, cluster)
-            self._remote_clusters[cluster.cluster_id] = device
-            CuraApplication.getInstance().getDiscoveredPrintersModel().addDiscoveredPrinter(
-                    device.key,
-                    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
-            CuraApplication.getInstance().getDiscoveredPrintersModel().updateDiscoveredPrinter(
-                    device.key,
-                    cluster.friendly_name,
-                    device.printerType,
-            )
 
+        self._remote_clusters = new_devices
+        self.discoveredDevicesChanged.emit()
         self._connectToActiveMachine()
 
     def _createMachineFromDiscoveredPrinter(self, key: str) -> None:
@@ -151,10 +143,9 @@ class CloudOutputDeviceManager:
             return
 
         # Remove all output devices that we have registered.
-        # This is needed because when we switch machines we can only leave
+        # This is needed because when we switch we can only leave output devices that are meant for that machine.
-        # output devices that are meant for that machine.
+        for device_id in self._remote_clusters:
-        for stored_cluster_id in self._remote_clusters:
+            CuraApplication.getInstance().getOutputDeviceManager().removeOutputDevice(device_id)
-            CuraApplication.getInstance().getOutputDeviceManager().removeOutputDevice(stored_cluster_id)
 
         # Check if the stored cluster_id for the active machine is in our list of remote clusters.
         stored_cluster_id = active_machine.getMetaDataEntry(self.META_CLUSTER_ID)
@@ -191,4 +182,5 @@ class CloudOutputDeviceManager:
     #  \param errors: The errors received
     @staticmethod
     def _onApiError(errors: List[CloudError] = None) -> None:
-        Logger.log("w", str(errors))
+        for error in errors:
+            Logger.log("w", str(error.toDict()))

plugins/UM3NetworkPrinting/src/Network/ManualPrinterRequest.py

@@ -1,13 +0,0 @@
-from typing import Optional, Callable
-
-
-## Represents a request for adding a manual printer. It has the following fields:
-#  - address: The string of the (IP) address of the manual printer
-#  - callback: (Optional) Once the HTTP request to the printer to get printer information is done, whether successful
-#              or not, this callback will be invoked to notify about the result. The callback must have a signature of
-#                  func(success: bool, address: str) -> None
-class ManualPrinterRequest:
-
-    def __init__(self, address: str, callback: Optional[Callable[[bool, str], None]] = None) -> None:
-        self.address = address
-        self.callback = callback

plugins/UM3NetworkPrinting/src/Network/NetworkOutputDeviceManager.py

@@ -9,7 +9,6 @@ from zeroconf import Zeroconf, ServiceBrowser, ServiceStateChange, ServiceInfo
 
 from UM import i18nCatalog
 from UM.Logger import Logger
-from UM.Message import Message
 from UM.Signal import Signal
 from UM.Version import Version
 
@@ -19,7 +18,6 @@ from cura.Settings.GlobalStack import GlobalStack
 
 from .ClusterApiClient import ClusterApiClient
 from .NetworkOutputDevice import NetworkOutputDevice
-from .ManualPrinterRequest import ManualPrinterRequest
 
 
 ## The NetworkOutputDeviceManager is responsible for discovering and managing local networked clusters.
@@ -32,7 +30,10 @@ class NetworkOutputDeviceManager:
     # The translation catalog for this device.
     I18N_CATALOG = i18nCatalog("cura")
 
+    # Signal emitted when the list of discovered devices changed.
     discoveredDevicesChanged = Signal()
+
+    # Signals emitted when new services were discovered or removed on the network.
     addedNetworkCluster = Signal()
     removedNetworkCluster = Signal()
 
@@ -51,8 +52,7 @@ class NetworkOutputDeviceManager:
 
         # TODO: move manual device stuff to own class?
         # Persistent dict containing manually connected clusters.
-        self._manual_instances = {}  # type: Dict[str, ManualPrinterRequest]
+        self._manual_instances = {}  # type: Dict[str, Callable]
-        self._last_manual_entry_key = None  # type: Optional[str]
 
         # Hook up the signals for discovery.
         self.addedNetworkCluster.connect(self._onAddDevice)
@@ -78,8 +78,7 @@ class NetworkOutputDeviceManager:
         # Load all manual devices.
         self._manual_instances = self._getStoredManualInstances()
         for address in self._manual_instances:
-            if address:
+            self.addManualDevice(address)
-                self.addManualDevice(address)
 
     ## Stop network discovery and clean up discovered devices.
     def stop(self):
@@ -97,7 +96,7 @@ class NetworkOutputDeviceManager:
 
     ## Add a networked printer manually by address.
     def addManualDevice(self, address: str, callback: Optional[Callable[[bool, str], None]] = None) -> None:
-        self._manual_instances[address] = ManualPrinterRequest(address, callback=callback)
+        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)
 
@@ -111,7 +110,6 @@ class NetworkOutputDeviceManager:
                 b"temporary": b"true"
             })
 
-        self._last_manual_entry_key = key
         response_callback = lambda status_code, response: self._onCheckManualDeviceResponse(status_code, address)
         self._checkManualDevice(address, response_callback)
 
@@ -126,12 +124,12 @@ class NetworkOutputDeviceManager:
             self._onRemoveDevice(key)
 
         if address in self._manual_instances:
-            manual_printer_request = self._manual_instances.pop(address)
+            manual_instance_callback = self._manual_instances.pop(address)
             new_manual_devices = ",".join(self._manual_instances.keys())
             CuraApplication.getInstance().getPreferences().setValue(self.MANUAL_DEVICES_PREFERENCE_KEY,
                                                                     new_manual_devices)
-            if manual_printer_request.callback is not None:
+            if manual_instance_callback:
-                CuraApplication.getInstance().callLater(manual_printer_request.callback, False, address)
+                CuraApplication.getInstance().callLater(manual_instance_callback, False, address)
 
     ## Force reset all network device connections.
     def refreshConnections(self):
@@ -143,13 +141,17 @@ class NetworkOutputDeviceManager:
         if not active_machine:
             return
 
+        # Remove all output devices that we have registered.
+        # This is needed because when we switch we can only leave output devices that are meant for that machine.
         for device_id in self._discovered_devices:
             CuraApplication.getInstance().getOutputDeviceManager().removeOutputDevice(device_id)
 
+        # Check if the stored network key for the active machine is in our list of discovered devices.
         stored_network_key = active_machine.getMetaDataEntry("um_network_key")
         if stored_network_key in self._discovered_devices:
             device = self._discovered_devices[stored_network_key]
             self._connectToOutputDevice(device, active_machine)
+            Logger.log("d", "Device connected by metadata network key %s", stored_network_key)
 
     ## Add a device to the current active machine.
     @staticmethod
@@ -160,17 +162,6 @@ class NetworkOutputDeviceManager:
         active_machine.addConfiguredConnectionType(device.connectionType.value)
         CuraApplication.getInstance().getOutputDeviceManager().addOutputDevice(device)
 
-    ## Handles an API error received from the cloud.
-    #  \param errors: The errors received
-    def _onApiError(self, errors) -> None:
-        Logger.log("w", str(errors))
-        message = Message(
-            text=self.I18N_CATALOG.i18nc("@info:description", "There was an error connecting to the printer."),
-            title=self.I18N_CATALOG.i18nc("@info:title", "Error"),
-            lifetime=10
-        )
-        message.show()
-
     ## 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:
@@ -181,12 +172,12 @@ class NetworkOutputDeviceManager:
     def _onCheckManualDeviceResponse(self, status_code: int, address: str) -> None:
         Logger.log("d", "manual device check response: {} {}".format(status_code, address))
         if address in self._manual_instances:
-            callback = self._manual_instances[address].callback
+            callback = self._manual_instances[address]
-            if callback:
+            if callback is not None:
                 CuraApplication.getInstance().callLater(callback, status_code == 200, address)
 
-    ##  Returns a dict of printer BOM numbers to machine types.
+    ## 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.
+    #  These numbers are available in the machine definition already so we just search for them here.
     @staticmethod
     def _getPrinterTypeIdentifiers() -> Dict[str, str]:
         container_registry = CuraApplication.getInstance().getContainerRegistry()
@@ -218,16 +209,14 @@ class NetworkOutputDeviceManager:
             return
 
         device = NetworkOutputDevice(key, address, properties)
-
         CuraApplication.getInstance().getDiscoveredPrintersModel().addDiscoveredPrinter(
             ip_address=address,
             key=device.getId(),
-            name=properties[b"name"].decode("utf-8"),
+            name=device.getName(),
             create_callback=self._createMachineFromDiscoveredPrinter,
-            machine_type=properties[b"printer_type"].decode("utf-8"),
+            machine_type=device.printerType,
             device=device
         )
-
         self._discovered_devices[device.getId()] = device
         self.discoveredDevicesChanged.emit()
         self._connectToActiveMachine()
@@ -237,11 +226,35 @@ class NetworkOutputDeviceManager:
         device = self._discovered_devices.pop(device_id, None)
         if not device:
             return
-        if device.isConnected():
-            device.disconnect()
         CuraApplication.getInstance().getDiscoveredPrintersModel().removeDiscoveredPrinter(device.address)
         self.discoveredDevicesChanged.emit()
 
+    ## Create a machine instance based on the discovered network printer.
+    def _createMachineFromDiscoveredPrinter(self, key: str) -> None:
+        discovered_device = self._discovered_devices.get(key)
+        if discovered_device is None:
+            Logger.log("e", "Could not find discovered device with key [%s]", key)
+            return
+        group_name = discovered_device.getProperty("name")
+        machine_type_id = discovered_device.getProperty("printer_type")
+        Logger.log("i", "Creating machine from network device with key = [%s], group name = [%s],  printer type = [%s]",
+                   key, group_name, machine_type_id)
+        CuraApplication.getInstance().getMachineManager().addMachine(machine_type_id, group_name)
+        self._connectToActiveMachine()
+
+    ## Load the user-configured manual devices from Cura preferences.
+    def _getStoredManualInstances(self) -> Dict[str, Optional[Callable]]:
+        preferences = CuraApplication.getInstance().getPreferences()
+        preferences.addPreference(self.MANUAL_DEVICES_PREFERENCE_KEY, "")
+        manual_instances = preferences.getValue(self.MANUAL_DEVICES_PREFERENCE_KEY).split(",")
+        return {address: None for address in manual_instances}
+
+    ## Handles an API error received from the cloud.
+    #  \param errors: The errors received
+    @staticmethod
+    def _onApiError(errors) -> None:
+        Logger.log("w", str(errors))
+
     ## Appends a service changed request so later the handling thread will pick it up and processes it.
     def _appendServiceChangedRequest(self, zeroconf: Zeroconf, service_type, name: str,
                                      state_change: ServiceStateChange) -> None:
@@ -323,26 +336,6 @@ class NetworkOutputDeviceManager:
 
     ## Handler for when a ZeroConf service was removed.
     def _onServiceRemoved(self, name: str) -> bool:
-        Logger.log("d", "Bonjour service removed: %s" % name)
+        Logger.log("d", "ZeroConf service removed: %s" % name)
         self.removedNetworkCluster.emit(str(name))
         return True
-
-    ## Create a machine instance based on the discovered network printer.
-    def _createMachineFromDiscoveredPrinter(self, key: str) -> None:
-        discovered_device = self._discovered_devices.get(key)
-        if discovered_device is None:
-            Logger.log("e", "Could not find discovered device with key [%s]", key)
-            return
-        group_name = discovered_device.getProperty("name")
-        machine_type_id = discovered_device.getProperty("printer_type")
-        Logger.log("i", "Creating machine from network device with key = [%s], group name = [%s],  printer type = [%s]",
-                   key, group_name, machine_type_id)
-        CuraApplication.getInstance().getMachineManager().addMachine(machine_type_id, group_name)
-        self._connectToActiveMachine()
-
-    ## Load the user-configured manual devices from Cura preferences.
-    def _getStoredManualInstances(self) -> Dict[str, ManualPrinterRequest]:
-        preferences = CuraApplication.getInstance().getPreferences()
-        preferences.addPreference(self.MANUAL_DEVICES_PREFERENCE_KEY, "")
-        manual_instances = preferences.getValue(self.MANUAL_DEVICES_PREFERENCE_KEY).split(",")
-        return {address: ManualPrinterRequest(address) for address in manual_instances}

plugins/UM3NetworkPrinting/src/Utils.py

@@ -1,33 +1,7 @@
 from datetime import datetime, timedelta
-from typing import TypeVar, Dict, Tuple, List
 
 from UM import i18nCatalog
 
-T = TypeVar("T")
-U = TypeVar("U")
-
-
-## Splits the given dictionaries into three lists (in a tuple):
-#       - `removed`: Items that were in the first argument but removed in the second one.
-#       - `added`: Items that were not in the first argument but were included in the second one.
-#       - `updated`: Items that were in both dictionaries. Both values are given in a tuple.
-#  \param previous: The previous items
-#  \param received: The received items
-#  \return: The tuple (removed, added, updated) as explained above.
-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
-
 
 def formatTimeCompleted(seconds_remaining: int) -> str:
     completed = datetime.now() + timedelta(seconds=seconds_remaining)