Improved multi-device handling

Ask user which device to use when there are many.  If there
is only one skip the question.  We used to just pick the
first one we found; user had no way to switch.

We have to handle querying from the non-GUI thread.

gui/qt/main_window.py

@@ -1337,6 +1337,16 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         WaitingDialog(self, _('Broadcasting transaction...'),
                       broadcast_thread, broadcast_done, self.on_error)
 
+    def query_choice(self, msg, choices):
+        # Needed by QtHandler for hardware wallets
+        dialog = WindowModalDialog(self.top_level_window())
+        clayout = ChoicesLayout(msg, choices)
+        vbox = QVBoxLayout(dialog)
+        vbox.addLayout(clayout.layout())
+        vbox.addLayout(Buttons(OkButton(dialog)))
+        dialog.exec_()
+        return clayout.selected_index()
+
     def prepare_for_payment_request(self):
         self.tabs.setCurrentIndex(1)
         self.payto_e.is_pr = True

lib/plugins.py

@@ -228,6 +228,7 @@ class BasePlugin(PrintError):
         pass
 
 Device = namedtuple("Device", "path id_ product_key")
+DeviceInfo = namedtuple("DeviceInfo", "device description initialized")
 
 class DeviceMgr(PrintError):
     '''Manages hardware clients.  A client communicates over a hardware
@@ -328,10 +329,6 @@ class DeviceMgr(PrintError):
     def paired_wallets(self):
         return list(self.wallets.keys())
 
-    def unpaired_devices(self, handler):
-        devices = self.scan_devices(handler)
-        return [dev for dev in devices if not self.wallet_by_id(dev.id_)]
-
     def client_lookup(self, id_):
         with self.lock:
             for client, (path, client_id) in self.clients.items():
@@ -362,28 +359,56 @@ class DeviceMgr(PrintError):
 
         if force_pair:
             first_address, derivation = wallet.first_address()
-            # Wallets don't have a first address in the install wizard
+            assert first_address
-            # until account creation
-            if not first_address:
-                self.print_error("no first address for ", wallet)
-                return None
 
-            # The wallet has not been previously paired, so get the
+            # The wallet has not been previously paired, so let the user
-            # first address of all unpaired clients and compare.
+            # choose an unpaired device and compare its first address.
-            for device in devices:
+            info = self.select_device(wallet, plugin, devices)
-                # Skip already-paired devices
+            if info:
-                if self.wallet_by_id(device.id_):
+                client = self.client_lookup(info.device.id_)
-                    continue
-                client = self.create_client(device, wallet.handler, plugin)
                 if client and not client.features.bootloader_mode:
                     # This will trigger a PIN/passphrase entry request
                     client_first_address = client.first_address(derivation)
                     if client_first_address == first_address:
-                        self.pair_wallet(wallet, device.id_)
+                        self.pair_wallet(wallet, info.device.id_)
                         return client
 
         return None
 
+    def unpaired_device_infos(self, handler, plugin, devices=None):
+        '''Returns a list of DeviceInfo objects: one for each connected,
+        unpaired device accepted by the plugin.'''
+        if devices is None:
+            devices = self.scan_devices(handler)
+        devices = [dev for dev in devices if not self.wallet_by_id(dev.id_)]
+
+        states = [_("wiped"), _("initialized")]
+        infos = []
+        for device in devices:
+            if not device.product_key in plugin.DEVICE_IDS:
+                continue
+            client = self.create_client(device, handler, plugin)
+            if not client:
+                continue
+            state = states[client.is_initialized()]
+            label = client.label() or _("An unnamed %s") % plugin.device
+            descr = "%s (%s)" % (label, state)
+            infos.append(DeviceInfo(device, descr, client.is_initialized()))
+
+        return infos
+
+    def select_device(self, wallet, plugin, devices=None):
+        '''Ask the user to select a device to use if there is more than one,
+        and return the DeviceInfo for the device.'''
+        infos = self.unpaired_device_infos(wallet.handler, plugin, devices)
+        if not infos:
+            return None
+        if len(infos) == 1:
+            return infos[0]
+        msg = _("Please select which %s device to use:") % plugin.device
+        descriptions = [info.description for info in infos]
+        return infos[wallet.handler.query_choice(msg, descriptions)]
+
     def scan_devices(self, handler):
         # All currently supported hardware libraries use hid, so we
         # assume it here.  This can be easily abstracted if necessary.

plugins/trezor/plugin.py

@@ -25,9 +25,6 @@ TIM_NEW, TIM_RECOVER, TIM_MNEMONIC, TIM_PRIVKEY = range(0, 4)
 class DeviceDisconnectedError(Exception):
     pass
 
-class OutdatedFirmwareError(Exception):
-    pass
-
 class TrezorCompatibleWallet(BIP44_Wallet):
     # Extend BIP44 Wallet as required by hardware implementation.
     # Derived classes must set:
@@ -332,42 +329,15 @@ class TrezorCompatiblePlugin(BasePlugin, ThreadJob):
         '''Called when creating a new wallet.  Select the device to use.  If
         the device is uninitialized, go through the intialization
         process.  Then create the wallet accounts.'''
-        initialized = self.select_device(wallet)
+        devmgr = self.device_manager()
-        if initialized:
+        device_info = devmgr.select_device(wallet, self)
+        devmgr.pair_wallet(wallet, device_info.device.id_)
+        if device_info.initialized:
             task = partial(wallet.create_hd_account, None)
         else:
             task = self.initialize_device(wallet)
         wallet.thread.add(task, on_done=on_done, on_error=on_error)
 
-    def unpaired_devices(self, handler):
-        '''Returns all connected, unpaired devices as a list of clients and a
-        list of descriptions.'''
-        devmgr = self.device_manager()
-        devices = devmgr.unpaired_devices(handler)
-
-        states = [_("wiped"), _("initialized")]
-        infos = []
-        for device in devices:
-            if not device.product_key in self.DEVICE_IDS:
-                continue
-            client = self.device_manager().create_client(device, handler, self)
-            if not client:
-                continue
-            state = states[client.is_initialized()]
-            label = client.label() or _("An unnamed %s") % self.device
-            descr = "%s (%s)" % (label, state)
-            infos.append((device, descr, client.is_initialized()))
-
-        return infos
-
-    def select_device(self, wallet):
-        msg = _("Please select which %s device to use:") % self.device
-        infos = self.unpaired_devices(wallet.handler)
-        labels = [info[1] for info in infos]
-        device, descr, init = infos[wallet.handler.query_choice(msg, labels)]
-        self.device_manager().pair_wallet(wallet, device.id_)
-        return init
-
     def on_restore_wallet(self, wallet, wizard):
         assert isinstance(wallet, self.wallet_class)
 

plugins/trezor/qt_generic.py

@@ -134,6 +134,7 @@ class QtHandler(QObject, PrintError):
     Trezor protocol; derived classes can customize it.'''
 
     charSig = pyqtSignal(object)
+    qcSig = pyqtSignal(object, object)
 
     def __init__(self, win, pin_matrix_widget_class, device):
         super(QtHandler, self).__init__()
@@ -144,6 +145,7 @@ class QtHandler(QObject, PrintError):
         win.connect(win, SIGNAL('passphrase_dialog'), self.passphrase_dialog)
         win.connect(win, SIGNAL('word_dialog'), self.word_dialog)
         self.charSig.connect(self.update_character_dialog)
+        self.qcSig.connect(self.win_query_choice)
         self.win = win
         self.pin_matrix_widget_class = pin_matrix_widget_class
         self.device = device
@@ -157,6 +159,12 @@ class QtHandler(QObject, PrintError):
     def watching_only_changed(self):
         self.win.emit(SIGNAL('watching_only_changed'))
 
+    def query_choice(self, msg, labels):
+        self.done.clear()
+        self.qcSig.emit(msg, labels)
+        self.done.wait()
+        return self.choice
+
     def show_message(self, msg, on_cancel=None):
         self.win.emit(SIGNAL('message_dialog'), msg, on_cancel)
 
@@ -256,8 +264,9 @@ class QtHandler(QObject, PrintError):
             self.dialog.accept()
             self.dialog = None
 
-    def query_choice(self, msg, labels):
+    def win_query_choice(self, msg, labels):
-        return self.win.query_choice(msg, labels)
+        self.choice = self.win.query_choice(msg, labels)
+        self.done.set()
 
     def request_trezor_init_settings(self, method, device):
         wizard = self.win
@@ -399,18 +408,13 @@ def qt_plugin_class(base_plugin_class):
     def choose_device(self, window):
         '''This dialog box should be usable even if the user has
         forgotten their PIN or it is in bootloader mode.'''
-        handler = window.wallet.handler
         device_id = self.device_manager().wallet_id(window.wallet)
         if not device_id:
-            infos = self.unpaired_devices(handler)
+            info = self.device_manager().select_device(window.wallet, self)
-            if infos:
+            if info:
-                labels = [info[1] for info in infos]
+                device_id = info.device.id_
-                msg = _("Select a %s device:") % self.device
-                choice = self.query_choice(window, msg, labels)
-                if choice is not None:
-                    device_id = infos[choice][0].id_
             else:
-                handler.show_error(_("No devices found"))
+                window.wallet.handler.show_error(_("No devices found"))
         return device_id
 
     def query_choice(self, window, msg, choices):