Merge lp:~trb143/openlp/bug-850254 into lp:openlp
- bug-850254
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 1841 |
Proposed branch: | lp:~trb143/openlp/bug-850254 |
Merge into: | lp:openlp |
Diff against target: |
112 lines (+33/-10) 2 files modified
openlp/core/ui/slidecontroller.py (+24/-6) openlp/plugins/remotes/lib/httpserver.py (+9/-4) |
To merge this branch: | bzr merge lp:~trb143/openlp/bug-850254 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raoul Snyman | Approve | ||
Review via email: mp+85934@code.launchpad.net |
This proposal supersedes a proposal from 2011-12-15.
Commit message
Description of the change
This is the core changes to handle the remote events for hiding and showing screens
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal | # |
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal | # |
Oh, and you seem to be sneaking a feature in along with the fix...
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal | # |
81 + (r'^/api/status$', self.status),
82 + (r'^/api/
As Jonathan said, "status" is a little too generic...
(r'^/api/
(r'^/api/
That should work.
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal | # |
Can you also change the method names to make them a little more descriptive please? Like...
def displayStatus(
"""
Obtain the display status of system.
"""
def changeDisplaySt
"""
Toggle the display of the system including the status button.
"""
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal | # |
Just looking at httpserver.py, we already have a method /api/display
It currently only supports hide or show.
Can't we just extend it to support 'theme' and 'desktop' too?
I'm also not convinced about just toggling it, this could just cause chaos if both the PC user and remote user do it at the same time as it could just result in a flicker. I think the user should tell the app what he wants to do, and then the app does it, even if the end result is to do nothing.
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal | # |
Will fix up the comments.
The API needs changing but if we change it when have to update the clients at the same time which will be a problem. The old API should be deprecated and removed in March.
The current situation is worse as the when and android can set the screen to blank but then have no view of what it is and the display is just changed without the button being set.
This change will make it possible to make the button states consistent across all clients.
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal | # |
But if I write a client that wants to blank the screen, then I want to be able to tell OpenLP to blank the screen. Changing it to a 'toggle' means that I press the button, and the application has no clue as to what OpenLP will do.
I have no issue with a method to get the current display status and for this to then update the ui afterwards with the state, or even poll regularly. But the remote app needs to be able to tell OpenLP exactly what they want to do.
99% of the time the operator will be able to look up at the projector screen and see what the current state of play is if they really badly want to know.
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal | # |
In fact we could just change the existing 'poll' call to get the display status
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal | # |
I agree with Jonathan, I think we should probably move away from a "toggle" mentality, and move to a "hide|theme|
Then we can show 4 buttons in the toolbar, and using the poll event, indicate which button is active.
Further thoughts/
Raoul Snyman (raoul-snyman) : | # |
Preview Diff
1 | === modified file 'openlp/core/ui/slidecontroller.py' | |||
2 | --- openlp/core/ui/slidecontroller.py 2011-12-09 12:35:18 +0000 | |||
3 | +++ openlp/core/ui/slidecontroller.py 2011-12-15 19:01:50 +0000 | |||
4 | @@ -95,7 +95,7 @@ | |||
5 | 95 | u'Edit Song', | 95 | u'Edit Song', |
6 | 96 | ] | 96 | ] |
7 | 97 | self.nextPreviousList = [ | 97 | self.nextPreviousList = [ |
9 | 98 | u'Previous Slide', | 98 | u'Previous Slide', |
10 | 99 | u'Next Slide' | 99 | u'Next Slide' |
11 | 100 | ] | 100 | ] |
12 | 101 | self.timer_id = 0 | 101 | self.timer_id = 0 |
13 | @@ -114,8 +114,8 @@ | |||
14 | 114 | self.typeLabel.setText(UiStrings().Live) | 114 | self.typeLabel.setText(UiStrings().Live) |
15 | 115 | self.split = 1 | 115 | self.split = 1 |
16 | 116 | self.typePrefix = u'live' | 116 | self.typePrefix = u'live' |
19 | 117 | self.keypress_queue = deque() | 117 | self.keypress_queue = deque() |
20 | 118 | self.keypress_loop = False | 118 | self.keypress_loop = False |
21 | 119 | else: | 119 | else: |
22 | 120 | self.typeLabel.setText(UiStrings().Preview) | 120 | self.typeLabel.setText(UiStrings().Preview) |
23 | 121 | self.split = 0 | 121 | self.split = 0 |
24 | @@ -187,7 +187,7 @@ | |||
25 | 187 | translate('OpenLP.SlideController', 'Hide'), self.toolbar)) | 187 | translate('OpenLP.SlideController', 'Hide'), self.toolbar)) |
26 | 188 | self.blankScreen = shortcut_action(self.hideMenu, u'blankScreen', | 188 | self.blankScreen = shortcut_action(self.hideMenu, u'blankScreen', |
27 | 189 | [QtCore.Qt.Key_Period], self.onBlankDisplay, | 189 | [QtCore.Qt.Key_Period], self.onBlankDisplay, |
29 | 190 | u':/slides/slide_blank.png', False, | 190 | u':/slides/slide_blank.png', False, |
30 | 191 | unicode(UiStrings().LiveToolbar)) | 191 | unicode(UiStrings().LiveToolbar)) |
31 | 192 | self.blankScreen.setText( | 192 | self.blankScreen.setText( |
32 | 193 | translate('OpenLP.SlideController', 'Blank Screen')) | 193 | translate('OpenLP.SlideController', 'Blank Screen')) |
33 | @@ -412,6 +412,9 @@ | |||
34 | 412 | QtCore.QObject.connect(Receiver.get_receiver(), | 412 | QtCore.QObject.connect(Receiver.get_receiver(), |
35 | 413 | QtCore.SIGNAL(u'slidecontroller_live_spin_delay'), | 413 | QtCore.SIGNAL(u'slidecontroller_live_spin_delay'), |
36 | 414 | self.receiveSpinDelay) | 414 | self.receiveSpinDelay) |
37 | 415 | QtCore.QObject.connect(Receiver.get_receiver(), | ||
38 | 416 | QtCore.SIGNAL(u'slidecontroller_toggle_display'), | ||
39 | 417 | self.toggleDisplay) | ||
40 | 415 | self.toolbar.makeWidgetsInvisible(self.loopList) | 418 | self.toolbar.makeWidgetsInvisible(self.loopList) |
41 | 416 | else: | 419 | else: |
42 | 417 | QtCore.QObject.connect(self.previewListWidget, | 420 | QtCore.QObject.connect(self.previewListWidget, |
43 | @@ -570,6 +573,21 @@ | |||
44 | 570 | self.display.setVisible(False) | 573 | self.display.setVisible(False) |
45 | 571 | self.mediaController.video_stop([self]) | 574 | self.mediaController.video_stop([self]) |
46 | 572 | 575 | ||
47 | 576 | def toggleDisplay(self, action): | ||
48 | 577 | """ | ||
49 | 578 | Toggle the display settings triggered from remote messages. | ||
50 | 579 | """ | ||
51 | 580 | if action == u'blank' or action == u'hide': | ||
52 | 581 | self.onBlankDisplay(True) | ||
53 | 582 | elif action == u'theme': | ||
54 | 583 | self.onThemeDisplay(True) | ||
55 | 584 | elif action == u'desktop': | ||
56 | 585 | self.onHideDisplay(True) | ||
57 | 586 | elif action == u'show': | ||
58 | 587 | self.onBlankDisplay(False) | ||
59 | 588 | self.onThemeDisplay(False) | ||
60 | 589 | self.onHideDisplay(False) | ||
61 | 590 | |||
62 | 573 | def servicePrevious(self): | 591 | def servicePrevious(self): |
63 | 574 | """ | 592 | """ |
64 | 575 | Live event to select the previous service item from the service manager. | 593 | Live event to select the previous service item from the service manager. |
65 | @@ -618,8 +636,8 @@ | |||
66 | 618 | self.previewSizeChanged() | 636 | self.previewSizeChanged() |
67 | 619 | self.previewDisplay.setup() | 637 | self.previewDisplay.setup() |
68 | 620 | serviceItem = ServiceItem() | 638 | serviceItem = ServiceItem() |
71 | 621 | self.previewDisplay.webView.setHtml(build_html(serviceItem, | 639 | self.previewDisplay.webView.setHtml(build_html(serviceItem, |
72 | 622 | self.previewDisplay.screen, None, self.isLive, None, | 640 | self.previewDisplay.screen, None, self.isLive, None, |
73 | 623 | plugins=PluginManager.get_instance().plugins)) | 641 | plugins=PluginManager.get_instance().plugins)) |
74 | 624 | self.mediaController.setup_display(self.previewDisplay) | 642 | self.mediaController.setup_display(self.previewDisplay) |
75 | 625 | if self.serviceItem: | 643 | if self.serviceItem: |
76 | 626 | 644 | ||
77 | === modified file 'openlp/plugins/remotes/lib/httpserver.py' | |||
78 | --- openlp/plugins/remotes/lib/httpserver.py 2011-12-14 22:39:52 +0000 | |||
79 | +++ openlp/plugins/remotes/lib/httpserver.py 2011-12-15 19:01:50 +0000 | |||
80 | @@ -249,7 +249,7 @@ | |||
81 | 249 | (r'^/api/poll$', self.poll), | 249 | (r'^/api/poll$', self.poll), |
82 | 250 | (r'^/api/controller/(live|preview)/(.*)$', self.controller), | 250 | (r'^/api/controller/(live|preview)/(.*)$', self.controller), |
83 | 251 | (r'^/api/service/(.*)$', self.service), | 251 | (r'^/api/service/(.*)$', self.service), |
85 | 252 | (r'^/api/display/(hide|show)$', self.display), | 252 | (r'^/api/display/(hide|show|blank|theme|desktop)$', self.display), |
86 | 253 | (r'^/api/alert$', self.alert), | 253 | (r'^/api/alert$', self.alert), |
87 | 254 | (r'^/api/plugin/(search)$', self.pluginInfo), | 254 | (r'^/api/plugin/(search)$', self.pluginInfo), |
88 | 255 | (r'^/api/(.*)/search$', self.search), | 255 | (r'^/api/(.*)/search$', self.search), |
89 | @@ -401,7 +401,13 @@ | |||
90 | 401 | u'item': self.parent.current_item._uuid \ | 401 | u'item': self.parent.current_item._uuid \ |
91 | 402 | if self.parent.current_item else u'', | 402 | if self.parent.current_item else u'', |
92 | 403 | u'twelve':QtCore.QSettings().value( | 403 | u'twelve':QtCore.QSettings().value( |
94 | 404 | u'remotes/twelve hour', QtCore.QVariant(True)).toBool() | 404 | u'remotes/twelve hour', QtCore.QVariant(True)).toBool(), |
95 | 405 | u'blank': self.parent.plugin.liveController.blankScreen.\ | ||
96 | 406 | isChecked(), | ||
97 | 407 | u'theme': self.parent.plugin.liveController.themeScreen.\ | ||
98 | 408 | isChecked(), | ||
99 | 409 | u'display': self.parent.plugin.liveController.desktopScreen.\ | ||
100 | 410 | isChecked() | ||
101 | 405 | } | 411 | } |
102 | 406 | return HttpResponse(json.dumps({u'results': result}), | 412 | return HttpResponse(json.dumps({u'results': result}), |
103 | 407 | {u'Content-Type': u'application/json'}) | 413 | {u'Content-Type': u'application/json'}) |
104 | @@ -413,8 +419,7 @@ | |||
105 | 413 | ``action`` | 419 | ``action`` |
106 | 414 | This is the action, either ``hide`` or ``show``. | 420 | This is the action, either ``hide`` or ``show``. |
107 | 415 | """ | 421 | """ |
110 | 416 | event = u'live_display_%s' % action | 422 | Receiver.send_message(u'slidecontroller_toggle_display', action) |
109 | 417 | Receiver.send_message(event, HideMode.Blank) | ||
111 | 418 | return HttpResponse(json.dumps({u'results': {u'success': True}}), | 423 | return HttpResponse(json.dumps({u'results': {u'success': True}}), |
112 | 419 | {u'Content-Type': u'application/json'}) | 424 | {u'Content-Type': u'application/json'}) |
113 | 420 | 425 |
Too many blank lines 56-58
There is an elif on 53 but an if on 55.
I'm not sure about the term "status" in httpserver, it doesn't seem descriptive enough to me, as lots of things could have a status. Perhaps displaystatus or something?