Move add by ip device discovery into DiscoveredPrintersModel

CURA-6483

cura/CuraApplication.py

@@ -216,7 +216,7 @@ class CuraApplication(QtApplication):
 
         self._machine_settings_manager = MachineSettingsManager(self, parent = self)
 
-        self._discovered_printer_model = DiscoveredPrintersModel(parent = self)
+        self._discovered_printer_model = DiscoveredPrintersModel(self, 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)

cura/Machines/Models/DiscoveredPrintersModel.py

@@ -8,10 +8,11 @@ from PyQt5.QtCore import pyqtSlot, pyqtProperty, pyqtSignal, QObject
 from UM.i18n import i18nCatalog
 from UM.Logger import Logger
 from UM.Util import parseBool
+from UM.OutputDevice.OutputDeviceManager import ManualDeviceAdditionAttempt
 
 if TYPE_CHECKING:
     from PyQt5.QtCore import QObject
-
+    from cura.CuraApplication import CuraApplication
     from cura.PrinterOutput.NetworkedPrinterOutputDevice import NetworkedPrinterOutputDevice
 
 
@@ -45,6 +46,10 @@ class DiscoveredPrinter(QObject):
             self._name = name
             self.nameChanged.emit()
 
+    @pyqtProperty(str, constant = True)
+    def address(self) -> str:
+        return self._ip_address
+
     machineTypeChanged = pyqtSignal()
 
     @pyqtProperty(str, notify = machineTypeChanged)
@@ -94,13 +99,72 @@ class DiscoveredPrinter(QObject):
 #
 class DiscoveredPrintersModel(QObject):
 
-    def __init__(self, parent: Optional["QObject"] = None) -> None:
+    def __init__(self, application: "CuraApplication", parent: Optional["QObject"] = None) -> None:
         super().__init__(parent)
 
+        self._application = application
         self._discovered_printer_by_ip_dict = dict()  # type: Dict[str, DiscoveredPrinter]
 
+        self._plugin_for_manual_device = None
+        self._manual_device_address = ""
+
     discoveredPrintersChanged = pyqtSignal()
 
+    @pyqtSlot(str)
+    def checkManualDevice(self, address: str) -> None:
+        if self.hasManualDeviceRequestInProgress:
+            Logger.log("i", "A manual device request for address [%s] is still in progress, do nothing",
+                       self._manual_device_address)
+            return
+
+        priority_order = [
+            ManualDeviceAdditionAttempt.PRIORITY,
+            ManualDeviceAdditionAttempt.POSSIBLE,
+        ]  # type: List[ManualDeviceAdditionAttempt]
+
+        all_plugins_dict = self._application.getOutputDeviceManager().getAllOutputDevicePlugins()
+
+        can_add_manual_plugins = [item for item in filter(
+            lambda plugin_item: plugin_item.canAddManualDevice(address) in priority_order,
+            all_plugins_dict.values())]
+
+        if not can_add_manual_plugins:
+            Logger.log("d", "Could not find a plugin to accept adding %s manually via address.", address)
+            return
+
+        plugin = max(can_add_manual_plugins, key = lambda p: priority_order.index(p.canAddManualDevice(address)))
+        self._plugin_for_manual_device = plugin
+        self._plugin_for_manual_device.addManualDevice(address, callback = self._onManualDeviceRequestFinished)
+        self._manual_device_address = address
+        self.hasManualDeviceRequestInProgressChanged.emit()
+
+    @pyqtSlot()
+    def cancelCurrentManualDeviceRequest(self) -> None:
+        if self._manual_device_address:
+            self._plugin_for_manual_device.removeManualDevice(self._manual_device_address, address = self._manual_device_address)
+            self._manual_device_address = ""
+            self._plugin_for_manual_device = None
+            self.hasManualDeviceRequestInProgressChanged.emit()
+            self.manualDeviceRequestFinished.emit(False)
+
+    hasManualDeviceRequestInProgressChanged = pyqtSignal()
+
+    @pyqtProperty(bool, notify = hasManualDeviceRequestInProgressChanged)
+    def hasManualDeviceRequestInProgress(self) -> bool:
+        return self._manual_device_address != ""
+
+    manualDeviceRequestFinished = pyqtSignal(bool, arguments = ["success"])
+
+    def _onManualDeviceRequestFinished(self, success: bool, address: str) -> None:
+        if address == self._manual_device_address:
+            self._manual_device_address = ""
+            self.hasManualDeviceRequestInProgressChanged.emit()
+            self.manualDeviceRequestFinished.emit(success)
+
+    @pyqtProperty("QVariantMap", notify = discoveredPrintersChanged)
+    def discoveredPrintersByAddress(self) -> Dict[str, DiscoveredPrinter]:
+        return self._discovered_printer_by_ip_dict
+
     @pyqtProperty(list, notify = discoveredPrintersChanged)
     def discoveredPrinters(self) -> List["DiscoveredPrinter"]:
         item_list = list(
@@ -157,11 +221,3 @@ class DiscoveredPrintersModel(QObject):
     @pyqtSlot("QVariant")
     def createMachineFromDiscoveredPrinter(self, discovered_printer: "DiscoveredPrinter") -> None:
         discovered_printer.create_callback(discovered_printer.getKey())
-
-    @pyqtSlot(str)
-    def createMachineFromDiscoveredPrinterAddress(self, ip_address: str) -> None:
-        if ip_address not in self._discovered_printer_by_ip_dict:
-            Logger.log("i", "Key [%s] does not exist in the discovered printers list.", ip_address)
-            return
-
-        self.createMachineFromDiscoveredPrinter(self._discovered_printer_by_ip_dict[ip_address])

plugins/UM3NetworkPrinting/src/UM3OutputDevicePlugin.py

@@ -5,7 +5,7 @@ import os
 from queue import Queue
 from threading import Event, Thread
 from time import time
-from typing import Optional, TYPE_CHECKING, Dict
+from typing import Optional, TYPE_CHECKING, Dict, Callable
 
 from zeroconf import Zeroconf, ServiceBrowser, ServiceStateChange, ServiceInfo
 
@@ -39,6 +39,19 @@ if TYPE_CHECKING:
 i18n_catalog = i18nCatalog("cura")
 
 
+#
+# 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
+
+
 ##      This plugin handles the connection detection & creation of output device objects for the UM3 printer.
 #       Zero-Conf is used to detect printers, which are saved in a dict.
 #       If we discover a printer that has the same key as the active machine instance a connection is made.
@@ -84,10 +97,12 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
         self._preferences.addPreference("um3networkprinting/manual_instances",
                                         "")  # A comma-separated list of ip adresses or hostnames
 
-        self._manual_instances = self._preferences.getValue("um3networkprinting/manual_instances").split(",")
+        manual_instances = self._preferences.getValue("um3networkprinting/manual_instances").split(",")
+        self._manual_instances = {address: ManualPrinterRequest(address)
+                                  for address in manual_instances}  # type: Dict[str, ManualPrinterRequest]
 
         # Store the last manual entry key
-        self._last_manual_entry_key = "" # type: str
+        self._last_manual_entry_key = ""  # type: str
 
         # The zero-conf service changed requests are handled in a separate thread, so we can re-schedule the requests
         # which fail to get detailed service info.
@@ -185,8 +200,6 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
                 self.checkCloudFlowIsPossible(None)
         else:
             self.getOutputDeviceManager().removeOutputDevice(key)
-            if key.startswith("manual:"):
-                self.removeManualDeviceSignal.emit(self.getPluginId(), key, self._discovered_devices[key].address)
 
     def stop(self):
         if self._zero_conf is not None:
@@ -198,7 +211,10 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
         # This plugin should always be the fallback option (at least try it):
         return ManualDeviceAdditionAttempt.POSSIBLE
 
-    def removeManualDevice(self, key, address = None):
+    def removeManualDevice(self, key: str, address: Optional[str] = None) -> None:
+        if key not in self._discovered_devices and address is not None:
+            key = "manual:%s" % address
+
         if key in self._discovered_devices:
             if not address:
                 address = self._discovered_devices[key].ipAddress
@@ -206,15 +222,19 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
             self.resetLastManualDevice()
 
         if address in self._manual_instances:
-            self._manual_instances.remove(address)
+            manual_printer_request = self._manual_instances.pop(address)
-            self._preferences.setValue("um3networkprinting/manual_instances", ",".join(self._manual_instances))
+            self._preferences.setValue("um3networkprinting/manual_instances", ",".join(self._manual_instances.keys()))
 
-        self.removeManualDeviceSignal.emit(self.getPluginId(), key, address)
+            if manual_printer_request.callback is not None:
+                self._application.callLater(manual_printer_request.callback, False, address)
 
-    def addManualDevice(self, address):
+    def addManualDevice(self, address: str, callback: Optional[Callable[[bool, str], None]] = None) -> None:
-        if address not in self._manual_instances:
+        if address in self._manual_instances:
-            self._manual_instances.append(address)
+            Logger.log("i", "Manual printer with address [%s] has already been added, do nothing", address)
-            self._preferences.setValue("um3networkprinting/manual_instances", ",".join(self._manual_instances))
+            return
+
+        self._manual_instances[address] = ManualPrinterRequest(address, callback = callback)
+        self._preferences.setValue("um3networkprinting/manual_instances", ",".join(self._manual_instances.keys()))
 
         instance_name = "manual:%s" % address
         properties = {
@@ -319,6 +339,11 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
                 Logger.log("e", "Something went wrong converting the JSON.")
                 return
 
+            if address in self._manual_instances:
+                manual_printer_request = self._manual_instances[address]
+                if manual_printer_request.callback is not None:
+                    self._application.callLater(manual_printer_request.callback, True, address)
+
             has_cluster_capable_firmware = Version(system_info["firmware"]) > self._min_cluster_version
             instance_name = "manual:%s" % address
             properties = {
@@ -362,10 +387,6 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
                 self._onRemoveDevice(instance_name)
                 self._onAddDevice(instance_name, address, properties)
 
-        if device and address in self._manual_instances:
-            self.getOutputDeviceManager().addOutputDevice(device)
-            self.addManualDeviceSignal.emit(self.getPluginId(), device.getId(), address, properties)
-
     def _onRemoveDevice(self, device_id: str) -> None:
         device = self._discovered_devices.pop(device_id, None)
         if device:
@@ -401,7 +422,9 @@ class UM3OutputDevicePlugin(OutputDevicePlugin):
             device = ClusterUM3OutputDevice.ClusterUM3OutputDevice(name, address, properties)
         else:
             device = LegacyUM3OutputDevice.LegacyUM3OutputDevice(name, address, properties)
-        self._application.getDiscoveredPrintersModel().addDiscoveredPrinter(address, device.getId(), name, self._createMachineFromDiscoveredPrinter, properties[b"printer_type"].decode("utf-8"), device)
+        self._application.getDiscoveredPrintersModel().addDiscoveredPrinter(
+            address, device.getId(), properties[b"name"].decode("utf-8"), self._createMachineFromDiscoveredPrinter,
+            properties[b"printer_type"].decode("utf-8"), device)
         self._discovered_devices[device.getId()] = device
         self.discoveredDevicesChanged.emit()
 

resources/qml/WelcomePages/AddPrinterByIpContent.qml

@@ -18,12 +18,13 @@ Item
 
     id: addPrinterByIpScreen
 
-    // Whether an IP address is currently being resolved.
+    // If there's a manual address resolve request in progress.
-    property bool hasSentRequest: false
+    property bool hasRequestInProgress: CuraApplication.getDiscoveredPrintersModel().hasManualDeviceRequestInProgress
-    // Whether the IP address user entered can be resolved as a recognizable printer.
+    // Indicates if a request has finished.
-    property bool haveConnection: false
+    property bool hasRequestFinished: false
-    // True when a request comes back, but the device hasn't responded.
+
-    property bool deviceUnresponsive: false
+    property var discoveredPrinter: null
+    property var isPrinterDiscovered: discoveredPrinter != null
 
     Label
     {
@@ -88,7 +89,7 @@ Item
                         regExp: /[a-fA-F0-9\.\:]*/
                     }
 
-                    enabled: { ! (addPrinterByIpScreen.hasSentRequest || addPrinterByIpScreen.haveConnection) }
+                    enabled: { ! (addPrinterByIpScreen.hasRequestInProgress || addPrinterByIpScreen.isPrinterDiscovered) }
                     onAccepted: addPrinterButton.clicked()
                 }
 
@@ -99,22 +100,25 @@ Item
                     anchors.left: hostnameField.right
                     anchors.leftMargin: UM.Theme.getSize("default_margin").width
                     text: catalog.i18nc("@button", "Add")
+                    enabled: !addPrinterByIpScreen.hasRequestInProgress && !addPrinterByIpScreen.hasRequestFinished
                     onClicked:
                     {
-                        if (hostnameField.text.trim() != "")
+                        const address = hostnameField.text.trim()
+                        if (address == "")
                         {
-                            enabled = false;
+                            return
-                            addPrinterByIpScreen.deviceUnresponsive = false;
-                            UM.OutputDeviceManager.addManualDevice(hostnameField.text, hostnameField.text);
                         }
-                    }
-                    busy: !enabled && !addPrinterByIpScreen.hasSentRequest && !addPrinterByIpScreen.haveConnection
 
-                    Connections
+                        // This address is already in the discovered printer model, no need to add a manual discovery.
-                    {
+                        if (CuraApplication.getDiscoveredPrintersModel().discoveredPrintersByAddress[address])
-                        target: UM.OutputDeviceManager
+                        {
-                        onManualDeviceChanged: { addPrinterButton.enabled = ! UM.OutputDeviceManager.hasManualDevice }
+                            addPrinterByIpScreen.discoveredPrinter = CuraApplication.getDiscoveredPrintersModel().discoveredPrintersByAddress[address]
+                            return
+                        }
+
+                        CuraApplication.getDiscoveredPrintersModel().checkManualDevice(address)
                     }
+                    busy: addPrinterByIpScreen.hasRequestInProgress
                 }
             }
 
@@ -133,14 +137,10 @@ Item
                     color: UM.Theme.getColor("text")
                     renderType: Text.NativeRendering
 
-                    visible:
+                    visible: addPrinterByIpScreen.hasRequestInProgress || (addPrinterByIpScreen.hasRequestFinished && !addPrinterByIpScreen.isPrinterDiscovered)
-                    {
-                        (addPrinterByIpScreen.hasSentRequest && ! addPrinterByIpScreen.haveConnection)
-                            || addPrinterByIpScreen.deviceUnresponsive
-                    }
                     text:
                     {
-                        if (addPrinterByIpScreen.deviceUnresponsive)
+                        if (addPrinterByIpScreen.hasRequestFinished)
                         {
                             catalog.i18nc("@label", "Could not connect to device.")
                         }
@@ -157,7 +157,7 @@ Item
                     anchors.top: parent.top
                     anchors.margins: UM.Theme.getSize("default_margin").width
 
-                    visible: addPrinterByIpScreen.haveConnection && ! addPrinterByIpScreen.deviceUnresponsive
+                    visible: addPrinterByIpScreen.isPrinterDiscovered
 
                     Label
                     {
@@ -167,7 +167,7 @@ Item
                         color: UM.Theme.getColor("text")
                         renderType: Text.NativeRendering
 
-                        text: "???"
+                        text: !addPrinterByIpScreen.isPrinterDiscovered ? "???" : addPrinterByIpScreen.discoveredPrinter.name
                     }
 
                     GridLayout
@@ -188,7 +188,7 @@ Item
                         Label
                         {
                             id: typeText
-                            text: "?"
+                            text: !addPrinterByIpScreen.isPrinterDiscovered ? "?" : addPrinterByIpScreen.discoveredPrinter.readableMachineType
                             font: UM.Theme.getFont("default")
                             color: UM.Theme.getColor("text")
                             renderType: Text.NativeRendering
@@ -204,7 +204,7 @@ Item
                         Label
                         {
                             id: firmwareText
-                            text: "0.0.0.0"
+                            text: !addPrinterByIpScreen.isPrinterDiscovered ? "0.0.0.0" : addPrinterByIpScreen.discoveredPrinter.device.getProperty("firmware_version")
                             font: UM.Theme.getFont("default")
                             color: UM.Theme.getColor("text")
                             renderType: Text.NativeRendering
@@ -220,52 +220,25 @@ Item
                         Label
                         {
                             id: addressText
-                            text: "0.0.0.0"
+                            text: !addPrinterByIpScreen.isPrinterDiscovered ? "0.0.0.0" : addPrinterByIpScreen.discoveredPrinter.address
                             font: UM.Theme.getFont("default")
                             color: UM.Theme.getColor("text")
                             renderType: Text.NativeRendering
                         }
-
-                        Connections
-                        {
-                            target: UM.OutputDeviceManager
-                            onManualDeviceChanged:
-                            {
-                                if (UM.OutputDeviceManager.hasManualDevice)
-                                {
-                                    const type_id = UM.OutputDeviceManager.manualDeviceProperty("printer_type")
-                                    var readable_type = Cura.MachineManager.getMachineTypeNameFromId(type_id)
-                                    readable_type = (readable_type != "") ? readable_type : catalog.i18nc("@label", "Unknown")
-                                    typeText.text = readable_type
-                                    firmwareText.text = UM.OutputDeviceManager.manualDeviceProperty("firmware_version")
-                                    addressText.text = UM.OutputDeviceManager.manualDeviceProperty("address")
-                                }
-                                else
-                                {
-                                    typeText.text = ""
-                                    firmwareText.text = ""
-                                    addressText.text = ""
-                                }
-                            }
-                        }
                     }
 
                     Connections
                     {
-                        target: UM.OutputDeviceManager
+                        target: CuraApplication.getDiscoveredPrintersModel()
-                        onManualDeviceChanged:
+                        onManualDeviceRequestFinished:
                         {
-                            if (UM.OutputDeviceManager.hasManualDevice)
+                            var discovered_printers_model = CuraApplication.getDiscoveredPrintersModel()
+                            var printer = discovered_printers_model.discoveredPrintersByAddress[hostnameField.text]
+                            if (printer)
                             {
-                                printerNameLabel.text = UM.OutputDeviceManager.manualDeviceProperty("name")
+                                addPrinterByIpScreen.discoveredPrinter = printer
-                                addPrinterByIpScreen.haveConnection = true
-                            }
-                            else
-                            {
-                                addPrinterByIpScreen.hasSentRequest = false
-                                addPrinterByIpScreen.haveConnection = false
-                                addPrinterByIpScreen.deviceUnresponsive = true
                             }
+                            addPrinterByIpScreen.hasRequestFinished = true
                         }
                     }
                 }
@@ -279,7 +252,11 @@ Item
         anchors.left: parent.left
         anchors.bottom: parent.bottom
         text: catalog.i18nc("@button", "Back")
-        onClicked: base.showPreviousPage()
+        onClicked:
+        {
+            CuraApplication.getDiscoveredPrintersModel().cancelCurrentManualDeviceRequest()
+            base.showPreviousPage()
+        }
     }
 
     Cura.PrimaryButton
@@ -290,12 +267,10 @@ Item
         text: catalog.i18nc("@button", "Connect")
         onClicked:
         {
-            CuraApplication.getDiscoveredPrintersModel().createMachineFromDiscoveredPrinterAddress(
+            CuraApplication.getDiscoveredPrintersModel().createMachineFromDiscoveredPrinter(discoveredPrinter)
-                UM.OutputDeviceManager.manualDeviceProperty("address"))
-            UM.OutputDeviceManager.setActiveDevice(UM.OutputDeviceManager.manualDeviceProperty("device_id"))
             base.showNextPage()
         }
 
-        enabled: addPrinterByIpScreen.haveConnection
+        enabled: addPrinterByIpScreen.hasRequestFinished && addPrinterByIpScreen.isPrinterDiscovered
     }
 }