openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Tomas Groth has proposed merging lp:~tomasgroth/openlp/bugfixes10 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #954613 in OpenLP: "screens sometimes skip when using down arrow"
  https://bugs.launchpad.net/openlp/+bug/954613
  Bug #1393186 in OpenLP: "Problems editing Start/Finish time of DVD clip in Service"
  https://bugs.launchpad.net/openlp/+bug/1393186
  Bug #1401234 in OpenLP: "If preview next item, focus on wrong service item"
  https://bugs.launchpad.net/openlp/+bug/1401234
  Bug #1404865 in OpenLP: "Traceback when adding verse to New King James Version from BibleGateway"
  https://bugs.launchpad.net/openlp/+bug/1404865
  Bug #1405133 in OpenLP: "Every second serviceitem is skipped when preview-next-item and move-to-next-item is enabled"
  https://bugs.launchpad.net/openlp/+bug/1405133
  Bug #1405476 in OpenLP: "Media finish time not defaulting correctly "
  https://bugs.launchpad.net/openlp/+bug/1405476

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/bugfixes10/+merge/245387

Fix bug 1405476 by checking if an end_time has been set.
Do not allow edit of start/end time for optical mediaclips in the servicemanager. Fixes bug 1393186 until re-edit of optical clips is added.
Make bible text parsing a bit more rebust, fixes bug 1404865.
Remove unneeded serviceitem shift when auto previewing. Fixes bug 1405133 and bug 1401234
[bug 954613] Introduce locking when updating slide display to avoid slidecontroller and display getting out of sync.

-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/bugfixes10 into lp:openlp.

=== modified file 'openlp/core/ui/media/mediacontroller.py'
--- openlp/core/ui/media/mediacontroller.py	2014-10-27 13:14:12 +0000
+++ openlp/core/ui/media/mediacontroller.py	2014-12-26 20:02:42 +0000
@@ -217,6 +217,8 @@
             if self.current_media_players[source].state != MediaState.Paused:
                 display = self._define_display(self.display_controllers[source])
                 display.controller.seek_slider.setSliderPosition(0)
+                display.controller.mediabar.actions['playbackPlay'].setVisible(True)
+                display.controller.mediabar.actions['playbackPause'].setVisible(False)
         self.timer.stop()
 
     def get_media_display_css(self):

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2014-12-06 20:08:42 +0000
+++ openlp/core/ui/servicemanager.py	2014-12-26 20:02:42 +0000
@@ -883,7 +883,8 @@
             # TODO for future: make group explains itself more visually
         else:
             self.auto_play_slides_menu.menuAction().setVisible(False)
-        if service_item['service_item'].is_capable(ItemCapabilities.HasVariableStartTime):
+        if service_item['service_item'].is_capable(ItemCapabilities.HasVariableStartTime) and \
+                not service_item['service_item'].is_capable(ItemCapabilities.IsOptical):
             self.time_action.setVisible(True)
         if service_item['service_item'].is_capable(ItemCapabilities.CanAutoStartForLive):
             self.auto_start_action.setVisible(True)
@@ -1478,8 +1479,6 @@
                 if self.service_items and item < len(self.service_items) and \
                         self.service_items[item]['service_item'].is_capable(ItemCapabilities.CanPreview):
                     self.preview_controller.add_service_manager_item(self.service_items[item]['service_item'], 0)
-                    next_item = self.service_manager_list.topLevelItem(item)
-                    self.service_manager_list.setCurrentItem(next_item)
                     self.live_controller.preview_widget.setFocus()
         else:
             critical_error_message_box(translate('OpenLP.ServiceManager', 'Missing Display Handler'),

=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py	2014-12-07 08:21:37 +0000
+++ openlp/core/ui/slidecontroller.py	2014-12-26 20:02:42 +0000
@@ -33,6 +33,7 @@
 import os
 import copy
 from collections import deque
+from threading import Lock
 
 from PyQt4 import QtCore, QtGui
 
@@ -131,6 +132,8 @@
             self.ratio = self.screens.current['size'].width() / self.screens.current['size'].height()
         except ZeroDivisionError:
             self.ratio = 1
+        self.process_queue_lock = Lock()
+        self.slide_selected_lock = Lock()
         self.timer_id = 0
         self.song_edit = False
         self.selected_row = 0
@@ -531,9 +534,9 @@
         Process the service item request queue.  The key presses can arrive
         faster than the processing so implement a FIFO queue.
         """
-        if self.keypress_queue:
-            while len(self.keypress_queue) and not self.keypress_loop:
-                self.keypress_loop = True
+        # Make sure only one thread get in here. Just return if already locked.
+        if self.keypress_queue and self.process_queue_lock.acquire(False):
+            while len(self.keypress_queue):
                 keypress_command = self.keypress_queue.popleft()
                 if keypress_command == ServiceItemAction.Previous:
                     self.service_manager.previous_item()
@@ -542,7 +545,7 @@
                     self.service_manager.previous_item(last_slide=True)
                 else:
                     self.service_manager.next_item()
-            self.keypress_loop = False
+            self.process_queue_lock.release()
 
     def screen_size_changed(self):
         """
@@ -1039,6 +1042,10 @@
 
         :param start:
         """
+        # Only one thread should be in here at the time. If already locked just skip, since the update will be
+        # done by the thread holding the lock. If it is a "start" slide, we must wait for the lock.
+        if not self.slide_selected_lock.acquire(start):
+            return
         row = self.preview_widget.current_slide_number()
         self.selected_row = 0
         if -1 < row < self.preview_widget.slide_count():
@@ -1061,6 +1068,8 @@
             self.update_preview()
             self.preview_widget.change_slide(row)
         self.display.setFocus()
+        # Release lock
+        self.slide_selected_lock.release()
 
     def on_slide_change(self, row):
         """
@@ -1405,7 +1414,6 @@
         self.split = 1
         self.type_prefix = 'live'
         self.keypress_queue = deque()
-        self.keypress_loop = False
         self.category = UiStrings().LiveToolbar
         ActionList.get_instance().add_category(str(self.category), CategoryOrder.standard_toolbar)
 

=== modified file 'openlp/core/ui/starttimeform.py'
--- openlp/core/ui/starttimeform.py	2014-03-20 19:10:31 +0000
+++ openlp/core/ui/starttimeform.py	2014-12-26 20:02:42 +0000
@@ -56,7 +56,9 @@
         self.hour_spin_box.setValue(hour)
         self.minute_spin_box.setValue(minutes)
         self.second_spin_box.setValue(seconds)
-        hours, minutes, seconds = self._time_split(self.item['service_item'].media_length)
+        hours, minutes, seconds = self._time_split(self.item['service_item'].end_time)
+        if hours == 0 and minutes == 0 and seconds == 0:
+            hours, minutes, seconds = self._time_split(self.item['service_item'].media_length)
         self.hour_finish_spin_box.setValue(hours)
         self.minute_finish_spin_box.setValue(minutes)
         self.second_finish_spin_box.setValue(seconds)

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2014-08-27 08:44:04 +0000
+++ openlp/plugins/bibles/lib/http.py	2014-12-26 20:02:42 +0000
@@ -93,10 +93,10 @@
         """
         if isinstance(tag, NavigableString):
             return None, str(tag)
-        elif tag.get('class')[0] == "versenum" or tag.get('class')[0] == 'versenum mid-line':
+        elif tag.get('class') and (tag.get('class')[0] == 'versenum' or tag.get('class')[0] == 'versenum mid-line'):
             verse = str(tag.string).replace('[', '').replace(']', '').strip()
             return verse, None
-        elif tag.get('class')[0] == 'chapternum':
+        elif tag.get('class') and tag.get('class')[0] == 'chapternum':
             verse = '1'
             return verse, None
         else:

=== modified file 'tests/interfaces/openlp_plugins/bibles/test_lib_http.py'
--- tests/interfaces/openlp_plugins/bibles/test_lib_http.py	2014-08-27 08:43:18 +0000
+++ tests/interfaces/openlp_plugins/bibles/test_lib_http.py	2014-12-26 20:02:42 +0000
@@ -85,6 +85,19 @@
         # THEN: We should get back a valid service item
         assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed'
 
+    def bible_gateway_extract_verse_nkjv_test(self):
+        """
+        Test the Bible Gateway retrieval of verse list for NKJV bible John 3
+        """
+        # GIVEN: A new Bible Gateway extraction class
+        handler = BGExtract()
+
+        # WHEN: The Books list is called
+        results = handler.get_bible_chapter('NKJV', 'John', 3)
+
+        # THEN: We should get back a valid service item
+        assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed'
+
     def crosswalk_extract_books_test(self):
         """
         Test Crosswalk retrieval of book list for NIV bible