From 26d010d31580e7397b45005fa171dfa249332dd9 Mon Sep 17 00:00:00 2001 From: Charles Haley <> Date: Tue, 24 Jul 2012 19:23:45 +0200 Subject: [PATCH] Improved thread handling in device_manager dynamic plugin methods. Improved smartdevice dialog box. --- src/calibre/devices/interface.py | 19 ++++++-- src/calibre/gui2/device.py | 78 ++++++++++++++++---------------- 2 files changed, 55 insertions(+), 42 deletions(-) diff --git a/src/calibre/devices/interface.py b/src/calibre/devices/interface.py index 0f2027065e..1466732169 100644 --- a/src/calibre/devices/interface.py +++ b/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 diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 14a9093bd4..63d7a03220 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -4,7 +4,7 @@ __copyright__ = '2008, Kovid Goyal ' # 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: - d.start_plugin() + self.dynamic_plugin_requests.put((getattr(d, method), args, kwargs)) + return self.dynamic_plugin_responses.get() except: - pass - - 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 + traceback.print_exc() 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: - d = self.dynamic_plugins.get(name, None) - if d: - d.set_option(opt_string, opt_value) - except: - pass + self._queue_request(name, 'set_option', opt_string, opt_value) def is_running(self, name): - try: - d = self.dynamic_plugins.get(name, None) - if d: - return d.is_running() - except: - pass - return False + return self._queue_request(name, 'is_running') 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'])