Improved thread handling in device_manager dynamic plugin methods. Improved smartdevice dialog box.

src/calibre/devices/interface.py

@@ -515,12 +515,15 @@ class DevicePlugin(Plugin):
         pass
 
     # Dynamic control interface
+    # All of these methods are called on the device_manager thread
 
     def is_dynamically_controllable(self):
         '''
         Called by the device manager when starting plugins. If this method returns
         a string, then a) it supports the device manager's dynamic control
-        interface, and b) that name is to be used when talking to the plugin
+        interface, and b) that name is to be used when talking to the plugin.
+
+        This method must be called from the device_manager thread.
         '''
         return None
 
@@ -529,6 +532,8 @@ class DevicePlugin(Plugin):
         This method is called to start the plugin. The plugin should begin
         to accept device connections however it does that. If the plugin is
         already accepting connections, then do nothing.
+
+        This method must be called from the device_manager thread.
         '''
         pass
 
@@ -538,27 +543,35 @@ class DevicePlugin(Plugin):
         accept connections, and should cleanup behind itself. It is likely that
         this method should call shutdown. If the plugin is already not accepting
         connections, then do nothing.
+
+        This method must be called from the device_manager thread.
         '''
         pass
 
-    def get_option(self, opt_string):
+    def get_option(self, opt_string, default=None):
         '''
         Return the value of the option indicated by opt_string. This method can
         be called when the plugin is not started. Return None if the option does
         not exist.
+
+        This method must be called from the device_manager thread.
         '''
-        return None
+        return default
 
     def set_option(self, opt_string, opt_value):
         '''
         Set the value of the option indicated by opt_string. This method can
         be called when the plugin is not started.
+
+        This method must be called from the device_manager thread.
         '''
         pass
 
     def is_running(self):
         '''
         Return True if the plugin is started, otherwise false
+
+        This method must be called from the device_manager thread.
         '''
         return False
 

src/calibre/gui2/device.py

@@ -4,7 +4,7 @@ __copyright__ = '2008, Kovid Goyal <kovid at kovidgoyal.net>'
 
 # Imports {{{
 import os, traceback, Queue, time, cStringIO, re, sys
-from threading import Thread
+from threading import Thread, Event
 
 from PyQt4.Qt import (QMenu, QAction, QActionGroup, QIcon, SIGNAL,
                      Qt, pyqtSignal, QDialog, QObject, QVBoxLayout,
@@ -30,6 +30,7 @@ from calibre.constants import DEBUG
 from calibre.utils.config import prefs, tweaks
 from calibre.utils.magick.draw import thumbnail
 from calibre.library.save_to_disk import find_plugboard
+from calibre.gui2 import is_gui_thread
 # }}}
 
 class DeviceJob(BaseJob): # {{{
@@ -145,8 +146,10 @@ class DeviceManager(Thread): # {{{
         self._device_information = None
         self.current_library_uuid = None
         self.call_shutdown_on_disconnect = False
-        self.devices_initialized = Queue.Queue(0)
+        self.devices_initialized = Event()
         self.dynamic_plugins = {}
+        self.dynamic_plugin_requests = Queue.Queue(0)
+        self.dynamic_plugin_responses = Queue.Queue(0)
 
     def report_progress(self, *args):
         pass
@@ -291,7 +294,7 @@ class DeviceManager(Thread): # {{{
             n = d.is_dynamically_controllable()
             if n:
                 self.dynamic_plugins[n] = d
-        self.devices_initialized.put(None)
+        self.devices_initialized.set()
 
         while self.keep_going:
             kls = None
@@ -315,6 +318,7 @@ class DeviceManager(Thread): # {{{
                     traceback.print_exc()
             else:
                 self.detect_device()
+
             while True:
                 job = self.next()
                 if job is not None:
@@ -325,8 +329,15 @@ class DeviceManager(Thread): # {{{
                     self.current_job = None
                 else:
                     break
-            time.sleep(self.sleep_time)
+            while True:
-
+                dynamic_method = None
+                try:
+                    (dynamic_method, args, kwargs) = \
+                        self.dynamic_plugin_requests.get(self.sleep_time)
+                    res = dynamic_method(*args, **kwargs)
+                    self.dynamic_plugin_responses.put(res)
+                except Queue.Empty:
+                    break
         # We are exiting. Call the shutdown method for each plugin
         for p in self.devices:
             try:
@@ -516,47 +527,36 @@ class DeviceManager(Thread): # {{{
 
     # dynamic plugin interface
 
-    def start_plugin(self, name):
+    # This is a helper function that handles queueing with the device manager
+    def _queue_request(self, name, method, *args, **kwargs):
+        if not is_gui_thread():
+            raise ValueError(
+                'The device_manager dynamic plugin methods must be called from the GUI thread')
         try:
             d = self.dynamic_plugins.get(name, None)
-            if d:
+            self.dynamic_plugin_requests.put((getattr(d, method), args, kwargs))
-                d.start_plugin()
+            return self.dynamic_plugin_responses.get()
         except:
-            pass
+            traceback.print_exc()
-
-    def stop_plugin(self, name):
-        try:
-            d = self.dynamic_plugins.get(name, None)
-            if d:
-                d.stop_plugin()
-        except:
-            pass
-
-    def get_option(self, name, opt_string):
-        try:
-            d = self.dynamic_plugins.get(name, None)
-            if d:
-                return d.get_option(opt_string)
-        except:
-            pass
         return None
 
+    # The dynamic plugin methods below must be called on the GUI thread. They
+    # will switch to the device thread before calling the plugin.
+
+    def start_plugin(self, name):
+        self._queue_request(name, 'start_plugin')
+
+    def stop_plugin(self, name):
+        self._queue_request(name, 'stop_plugin')
+
+    def get_option(self, name, opt_string, default=None):
+        return self._queue_request(name, 'get_option', opt_string, default=default)
+
     def set_option(self, name, opt_string, opt_value):
-        try:
+        self._queue_request(name, 'set_option', opt_string, opt_value)
-            d = self.dynamic_plugins.get(name, None)
-            if d:
-                d.set_option(opt_string, opt_value)
-        except:
-            pass
 
     def is_running(self, name):
-        try:
+        return self._queue_request(name, 'is_running')
-            d = self.dynamic_plugins.get(name, None)
-            if d:
-                return d.is_running()
-        except:
-            pass
-        return False
 
     def is_enabled(self, name):
         try:
@@ -767,7 +767,7 @@ class DeviceMixin(object): # {{{
                 self.job_manager, Dispatcher(self.status_bar.show_message),
                 Dispatcher(self.show_open_feedback))
         self.device_manager.start()
-        self.device_manager.devices_initialized.get()
+        self.device_manager.devices_initialized.wait()
         if tweaks['auto_connect_to_folder']:
             self.connect_to_folder_named(tweaks['auto_connect_to_folder'])