Merge lp:~trb143/openlp/bug-892571 into lp:openlp

[Tim Bentley]

Stop keyboard floods getting the slide controller and service manager out of set.
Key presses are only accepted when the previous updates have happened.

To Test set preview next song to on.
Add 5 songs to service manager
Select the first one.
Press the right arrow key fast.

[Andreas Preikschat (googol-deactivatedaccount)]

Now "right arrow", "right arrow" does not work (only the first "right arrow" is recognised). If I press the "right arrow" twice, I want to go to the song after the next one.

review: Needs Fixing

[Tim Bentley (trb143)]

Well If I use a mutex it locks up.
Without a major (2-3 month) re-write of the code path as it is very long and impacts plugins etc I am not sure what else to try.
The code
   looks up the current selected service item
   Selects the next one.
   Flushes the events to add a cursor
   Adds a service item
   This signals plugins twice to turn off and then hand new service item.
   selects the next service item to preview it.
   Flushes the events to remove the cursor

The Keyboard will start this flow BEFORE the previous has finished so you get a number of calling chains running at the same time.

[Andreas Preikschat (googol-deactivatedaccount)]

I just do not think this is the right attempt. I see that it is difficult (impossible ?) to fix this!?

[Jonathan Stafford (staffj01)]

I'm still new to this so sorry if my comments don't make much sense.

Rather than disabling keypresses while the slide controller is proccessing an action, would it be possible to somehow queue keypresses and then execute them one at a time afterwards?

Alternatively, could the code that looks up the currently selected service item somehow be delayed if another keypress is currently being processed? I guess that could still lead to multiple keypresses being process out of order though couldn't it?

Not sure if any of those ideas have any merit or not...

[Raoul Snyman (raoul-snyman)]

Tried to break it, and I couldn't.

review: Approve

=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py	2011-10-31 09:14:07 +0000
+++ openlp/core/ui/slidecontroller.py	2011-11-28 18:10:49 +0000
@@ -26,9 +26,9 @@
 ###############################################################################
 
 import logging
-import os
 import time
 import copy
+from collections import deque
 
 from PyQt4 import QtCore, QtGui
 from PyQt4.phonon import Phonon
@@ -91,6 +91,8 @@
             self.typeLabel.setText(UiStrings().Live)
             self.split = 1
             self.typePrefix = u'live'
+            self.keypress_queue = deque() 
+            self.keypress_loop = False           
         else:
             self.typeLabel.setText(UiStrings().Preview)
             self.split = 0
@@ -578,12 +580,34 @@
         self.display.videoStop()
 
     def servicePrevious(self):
-        time.sleep(0.1)
-        Receiver.send_message('servicemanager_previous_item')
+        """
+        Live event to select the previous service item from the service manager.
+        """
+        self.keypress_queue.append(u'previous')
+        self._process_queue()
+        
 
     def serviceNext(self):
-        time.sleep(0.1)
-        Receiver.send_message('servicemanager_next_item')
+        """
+        Live event to select the next service item from the service manager.
+        """
+        self.keypress_queue.append(u'next')
+        self._process_queue()
+        
+    def _process_queue(self):
+        """
+        Process the service item request queue.  The key presses can arrive 
+        faster than the processing so implement a FIFO queue. 
+        """
+        if len(self.keypress_queue):
+            while len(self.keypress_queue) and not self.keypress_loop:
+                self.keypress_loop = True                
+                if self.keypress_queue.popleft() == u'previous':
+                    Receiver.send_message('servicemanager_previous_item')                    
+                else:
+                    Receiver.send_message('servicemanager_next_item')
+            self.keypress_loop = False
+     
 
     def screenSizeChanged(self):
         """
@@ -771,7 +795,7 @@
         log.debug(u'processManagerItem live = %s' % self.isLive)
         self.onStopLoop()
         old_item = self.serviceItem
-        # take a copy not a link to the servicemeanager copy.
+        # take a copy not a link to the servicemanager copy.
         self.serviceItem = copy.copy(serviceItem)
         if old_item and self.isLive and old_item.is_capable(
             ItemCapabilities.ProvidesOwnDisplay):