From deea793c21aa75cde15bf2ca26c1dae569a94ede Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Wed, 26 Feb 2014 11:30:22 +0100 Subject: [PATCH] 1) Change the new sync method to return a list of changed book ids instead of a simple boolean. 2) Make the wireless device driver synchronization more robust in the face of multiple libraries where the sync column might not always exist. --- src/calibre/devices/interface.py | 10 ++- .../devices/smart_device_app/driver.py | 90 ++++++++++++------- src/calibre/gui2/device.py | 48 ++++++---- 3 files changed, 97 insertions(+), 51 deletions(-) diff --git a/src/calibre/devices/interface.py b/src/calibre/devices/interface.py index 81c7e127c5..954f19560f 100644 --- a/src/calibre/devices/interface.py +++ b/src/calibre/devices/interface.py @@ -724,8 +724,12 @@ class DevicePlugin(Plugin): a book in calibre's db. The method is responsible for syncronizing data from the device to calibre's db (if needed). - The method must return True if either calibre's database or the device - book's metadata were changed, False otherwise. + The method must return a set of calibre book ids changed if calibre's + database was changed, None if the database was not changed. If the + method returns an empty set then the metadata for the book on the + device is updated with calibre's metadata and given back to the device, + but no GUI refresh of that book is done. This is useful when the calire + data is correct but must be sent to the device. Extremely important: this method is called on the GUI thread. It must be threadsafe with respect to the device manager's thread. @@ -733,7 +737,7 @@ class DevicePlugin(Plugin): book_id: the calibre id for the book in the database. book_metadata: the Metadata object for the book coming from the device. ''' - return False + return None class BookList(list): ''' diff --git a/src/calibre/devices/smart_device_app/driver.py b/src/calibre/devices/smart_device_app/driver.py index e0c3e637c9..8d26f87654 100644 --- a/src/calibre/devices/smart_device_app/driver.py +++ b/src/calibre/devices/smart_device_app/driver.py @@ -1295,7 +1295,8 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): # given back by "books", and one that has been plugboarded. books_to_send = [] for book in booklists[0]: - if not self._metadata_already_on_device(book): + if (book.get('_force_send_metadata_', None) or + not self._metadata_already_on_device(book)): books_to_send.append(book) count = len(books_to_send) @@ -1459,38 +1460,63 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin): @synchronous('sync_lock') def synchronize_with_db(self, db, id_, book): + if not (self.is_read_sync_col or self.is_read_date_sync_col): + # Not syncing + return None + is_changed = book.get('_is_read_changed_', None); - if is_changed: - made_changes = False - # is_read_changed == 1: standard sync. Update calibre with the new value - # is_read_changed == 2: special first-time sync after specifying the - # column. Update calibre if CC's value is not null, else update CC - # if calibre's value is not null - val = book.get('_is_read_', None) - if is_changed == 1 or (is_changed == 2 and val): - self._debug('standard update book', book.get('title', 'huh?'), 'to', val) - if self.is_read_sync_col: - db.new_api.set_field(self.is_read_sync_col, {id_: val}) - made_changes = True - if self.is_read_date_sync_col: - db.new_api.set_field(self.is_read_date_sync_col, - {id_: book.get('_last_read_date_', None)}) - made_changes = True - elif self.is_read_sync_col and is_changed == 2 and not val: - calibre_val = db.new_api.field_for(self.is_read_sync_col, - id_, default_value=None) - if calibre_val: - from calibre.utils.date import UNDEFINED_DATE - # This will force the metadata for the book to be sent even - # if the last_mod dates matched before. Note that because - # CC's last_read date is one-way sync, this could leave an - # empty date in CC. - self._debug('special update book', book.get('title', 'huh?'), - 'to', calibre_val) - book.set('last_modified', UNDEFINED_DATE) - book.set('_is_read_changed_', None) - return made_changes - return False + is_read = book.get('_is_read_', None) + + if is_changed == 2 and is_read is None: + # This is a special case where the user just set the sync column. In + # this case the device value wins if it is not None by falling + # through to the normal sync situation below, otherwise the calibre + # value wins. + calibre_val = db.new_api.field_for(self.is_read_sync_col, + id_, default_value=None) + if calibre_val is not None: + # This will force the metadata for the book to be sent . Note + # that because the devices last_read date is one-way sync, this + # could leave an empty date in the device. + book.set('_force_send_metadata_', True) + self._debug('special update book', book.get('title', 'huh?'), + 'to', calibre_val) + return set(id_) + # Both values are None. Do nothing + return None + + orig_is_read = book.get(self.is_read_sync_col, None) + if is_read != orig_is_read: + # The value in the device's is_read checkbox is not the same as the + # last one that came to the device from calibre during the last + # connect, meaning that the user changed it. Write the one from the + # checkbox to calibre's db. + changed_books = set() + is_read_date = book.get('_last_read_date_', None); + self._debug('standard update book', book.get('title', 'huh?'), 'to', + is_read, is_read_date) + if self.is_read_sync_col: + try: + changed_books = db.new_api.set_field(self.is_read_sync_col, + {id_: is_read}) + except: + self._debug('setting read sync col tossed exception', + self.is_read_sync_col) + if self.is_read_date_sync_col: + try: + changed_books |= db.new_api.set_field(self.is_read_date_sync_col, + {id_: is_read_date}) + except: + self._debug('setting read date sync col tossed exception', + self.is_read_date_sync_col) + return changed_books + + # The user might have changed the value in calibre. If so, that value + # will be sent to the device in the normal way. Note that because any + # updated value has already been synced and so will also be sent, the + # device should put the calibre value into its checkbox (or whatever it + # uses) + return None @synchronous('sync_lock') def startup(self): diff --git a/src/calibre/gui2/device.py b/src/calibre/gui2/device.py index 8dcf03dfec..843ccffaa5 100644 --- a/src/calibre/gui2/device.py +++ b/src/calibre/gui2/device.py @@ -1751,12 +1751,9 @@ class DeviceMixin(object): # {{{ self.db_book_title_cache = db_book_title_cache self.db_book_uuid_cache = db_book_uuid_cache - # Now iterate through all the books on the device, setting the - # in_library field. If the UUID matches a book in the library, then - # do not consider that book for other matching. In all cases set - # the application_id to the db_id of the matching book. This value - # will be used by books_on_device to indicate matches. While we are - # going by, update the metadata for a book if automatic management is on + # This is really a local variable, but it must be made into a class + # attribute so that the updateq function (below) can change it. + self.sbil_book_ids_to_refresh = set() def update_book(id_, book) : if not update_metadata: @@ -1768,21 +1765,31 @@ class DeviceMixin(object): # {{{ def updateq(id_, book): try: - return (update_metadata and - ( - (self.device_manager.device is not None and - self.device_manager.device.synchronize_with_db(db, id_, book)) or - (db.metadata_last_modified(id_, index_is_id=True) != - getattr(book, 'last_modified', None) or - (isinstance(getattr(book, 'thumbnail', None), (list, tuple)) - and max(book.thumbnail[0], book.thumbnail[1]) != desired_thumbnail_height - ) - ) + if not update_metadata: + return False + + if self.device_manager.device is not None: + set_of_ids = self.device_manager.device.synchronize_with_db(db, id_, book) + if set_of_ids is not None: + self.sbil_book_ids_to_refresh |= set_of_ids + return True + + return (db.metadata_last_modified(id_, index_is_id=True) != + getattr(book, 'last_modified', None) or + (isinstance(getattr(book, 'thumbnail', None), (list, tuple)) + and max(book.thumbnail[0], book.thumbnail[1]) != desired_thumbnail_height ) ) except: return True + # Now iterate through all the books on the device, setting the + # in_library field. If the UUID matches a book in the library, then + # do not consider that book for other matching. In all cases set + # the application_id to the db_id of the matching book. This value + # will be used by books_on_device to indicate matches. While we are + # going by, update the metadata for a book if automatic management is on + total_book_count = 0 for booklist in booklists: for book in booklist: @@ -1877,6 +1884,15 @@ class DeviceMixin(object): # {{{ FunctionDispatcher(self.metadata_synced), booklists, plugboards, add_as_step_to_job) + if self.sbil_book_ids_to_refresh: + try: + prints('DeviceJob: set_books_in_library refreshing GUI for ', + len(self.sbil_book_ids_to_refresh), 'books') + self.library_view.model().refresh_ids(self.sbil_book_ids_to_refresh) + except: + # This shouldn't ever happen, but just in case ... + pass + if DEBUG: prints('DeviceJob: set_books_in_library finished: time=', time.time() - start_time)