openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

At first look not bad.
Needs tests but a new feature for 2.4 nice one.

Diff comments:

> === modified file 'openlp/plugins/remotes/html/stage.js'
> --- openlp/plugins/remotes/html/stage.js	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/remotes/html/stage.js	2015-02-08 20:35:01 +0000
> @@ -96,6 +96,10 @@
>      $("#tag" + OpenLP.currentTags[OpenLP.currentSlide]).addClass("currenttag");
>      var slide = OpenLP.currentSlides[OpenLP.currentSlide];
>      var text = "";
> +    if (!slide) {

What problem is here.  Should this be a fix for 2.2?

> +        // This will happen if no live view ist started
> +        return;
> +    }
>      // use title if available
>      if (slide["title"]) {
>          text = slide["title"];
> @@ -137,14 +141,26 @@
>    },
>    updateClock: function(data) {
>      var div = $("#clock");
> -    var t = new Date();
> -    var h = t.getHours();
> -    if (data.results.twelve && h > 12)
> -      h = h - 12;
> -    var m = t.getMinutes();
> -    if (m < 10)
> -      m = '0' + m + '';
> -    div.html(h + ":" + m);
> +    switch (data.results.timeSource) {
> +        case 0: // use client's time
> +            var t = new Date();
> +            var h = t.getHours();
> +            if (data.results.twelve && h > 12)
> +            h = h - 12;
> +            var m = t.getMinutes();
> +            if (m < 10)
> +            m = '0' + m + '';
> +            div.html(h + ":" + m);
> +            div.show();
> +            break;
> +        case 1: // use server time
> +            div.html(data.results.serverTime);
> +            div.show();
> +            break;
> +        default: // no time display
> +            div.html("");
> +            div.hide();
> +    }
>    },
>    pollServer: function () {
>      $.getJSON(
> 
> === modified file 'openlp/plugins/remotes/lib/__init__.py'
> --- openlp/plugins/remotes/lib/__init__.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/remotes/lib/__init__.py	2015-02-08 20:35:01 +0000
> @@ -20,8 +20,17 @@
>  # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
>  ###############################################################################
>  
> +class TimeSourceType(object):
> +    """
> +    An enumeration for the time source
> +    """
> +    Client = 0
> +    Server = 1
> +    NoTime = 2
> +
>  from .remotetab import RemoteTab
>  from .httprouter import HttpRouter
>  from .httpserver import OpenLPServer
>  
>  __all__ = ['RemoteTab', 'OpenLPServer', 'HttpRouter']
> +
> 
> === modified file 'openlp/plugins/remotes/lib/httprouter.py'
> --- openlp/plugins/remotes/lib/httprouter.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/remotes/lib/httprouter.py	2015-02-08 20:35:01 +0000
> @@ -112,6 +112,7 @@
>  import re
>  import urllib.request
>  import urllib.error
> +import time
>  from urllib.parse import urlparse, parse_qs
>  
>  from mako.template import Template
> @@ -415,15 +416,22 @@
>          """
>          Poll OpenLP to determine the current slide number and item name.
>          """
> +        twelve = Settings().value('remotes/twelve hour');
> +        if twelve:
> +            time_string = time.strftime("%I:%M")
> +        else:
> +            time_string = time.strftime("%H:%M")
>          result = {
>              'service': self.service_manager.service_id,
>              'slide': self.live_controller.selected_row or 0,
>              'item': self.live_controller.service_item.unique_identifier if self.live_controller.service_item else '',
> -            'twelve': Settings().value('remotes/twelve hour'),
> +            'twelve': twelve,
> +            'timeSource': Settings().value('remotes/time source'),
> +            'serverTime' : time_string,
>              'blank': self.live_controller.blank_screen.isChecked(),
>              'theme': self.live_controller.theme_screen.isChecked(),
>              'display': self.live_controller.desktop_screen.isChecked(),
> -            'version': 2,
> +            'version': 3,
>              'isSecure': Settings().value(self.settings_section + '/authentication enabled'),
>              'isAuthorised': self.authorised
>          }
> 
> === modified file 'openlp/plugins/remotes/lib/remotetab.py'
> --- openlp/plugins/remotes/lib/remotetab.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/remotes/lib/remotetab.py	2015-02-08 20:35:01 +0000
> @@ -26,6 +26,7 @@
>  
>  from openlp.core.common import AppLocation, Settings, translate
>  from openlp.core.lib import SettingsTab, build_icon
> +from openlp.plugins.remotes.lib import TimeSourceType
>  
>  ZERO_URL = '0.0.0.0'
>  
> @@ -52,6 +53,12 @@
>                                         self))
>          self.address_edit.setObjectName('address_edit')
>          self.server_settings_layout.addRow(self.address_label, self.address_edit)
> +        self.time_source_label = QtGui.QLabel(self.server_settings_group_box)
> +        self.time_source_label.setObjectName('time_source_label')
> +        self.time_source_combo_box = QtGui.QComboBox(self.server_settings_group_box)
> +        self.time_source_combo_box.addItems(['', '', ''])
> +        self.time_source_combo_box.setObjectName('time_source_combo_box')
> +        self.server_settings_layout.addRow(self.time_source_label, self.time_source_combo_box)
>          self.twelve_hour_check_box = QtGui.QCheckBox(self.server_settings_group_box)
>          self.twelve_hour_check_box.setObjectName('twelve_hour_check_box')
>          self.server_settings_layout.addRow(self.twelve_hour_check_box)
> @@ -158,6 +165,7 @@
>          self.qr_layout.addWidget(self.qr_description_label)
>          self.left_layout.addStretch()
>          self.right_layout.addStretch()
> +        self.time_source_combo_box.activated.connect(self.on_time_source_combo_box_changed)
>          self.twelve_hour_check_box.stateChanged.connect(self.on_twelve_hour_check_box_changed)
>          self.thumbnails_check_box.stateChanged.connect(self.on_thumbnails_check_box_changed)
>          self.address_edit.textChanged.connect(self.set_urls)
> @@ -172,6 +180,10 @@
>          self.remote_url_label.setText(translate('RemotePlugin.RemoteTab', 'Remote URL:'))
>          self.stage_url_label.setText(translate('RemotePlugin.RemoteTab', 'Stage view URL:'))
>          self.live_url_label.setText(translate('RemotePlugin.RemoteTab', 'Live view URL:'))
> +        self.time_source_label.setText(translate('RemotePlugin.RemoteTab', 'Time source:'))

Client
Server
None

A thought

> +        self.time_source_combo_box.setItemText(TimeSourceType.Client, translate('RemotePlugin.RemoteTab', 'Client'))
> +        self.time_source_combo_box.setItemText(TimeSourceType.Server, translate('RemotePlugin.RemoteTab', 'Server'))
> +        self.time_source_combo_box.setItemText(TimeSourceType.NoTime, translate('RemotePlugin.RemoteTab', 'No time view'))
>          self.twelve_hour_check_box.setText(translate('RemotePlugin.RemoteTab', 'Display stage time in 12h format'))
>          self.thumbnails_check_box.setText(translate('RemotePlugin.RemoteTab',
>                                                      'Show thumbnails of non-text slides in remote and stage view.'))
> @@ -237,6 +249,8 @@
>          self.port_spin_box.setValue(Settings().value(self.settings_section + '/port'))
>          self.https_port_spin_box.setValue(Settings().value(self.settings_section + '/https port'))
>          self.address_edit.setText(Settings().value(self.settings_section + '/ip address'))
> +        self.time_source = Settings().value(self.settings_section + '/time source')
> +        self.time_source_combo_box.setCurrentIndex(self.time_source)
>          self.twelve_hour = Settings().value(self.settings_section + '/twelve hour')
>          self.twelve_hour_check_box.setChecked(self.twelve_hour)
>          self.thumbnails = Settings().value(self.settings_section + '/thumbnails')
> @@ -271,6 +285,7 @@
>          Settings().setValue(self.settings_section + '/https port', self.https_port_spin_box.value())
>          Settings().setValue(self.settings_section + '/https enabled', self.https_settings_group_box.isChecked())
>          Settings().setValue(self.settings_section + '/ip address', self.address_edit.text())
> +        Settings().setValue(self.settings_section + '/time source', self.time_source)
>          Settings().setValue(self.settings_section + '/twelve hour', self.twelve_hour)
>          Settings().setValue(self.settings_section + '/thumbnails', self.thumbnails)
>          Settings().setValue(self.settings_section + '/authentication enabled', self.user_login_group_box.isChecked())
> @@ -278,6 +293,9 @@
>          Settings().setValue(self.settings_section + '/password', self.password.text())
>          self.generate_icon()
>  
> +    def on_time_source_combo_box_changed(self):
> +        self.time_source = self.time_source_combo_box.currentIndex();
> +        
>      def on_twelve_hour_check_box_changed(self, check_state):
>          """
>          Toggle the 12 hour check box.
> 
> === modified file 'openlp/plugins/remotes/remoteplugin.py'
> --- openlp/plugins/remotes/remoteplugin.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/remotes/remoteplugin.py	2015-02-08 20:35:01 +0000
> @@ -25,11 +25,12 @@
>  from PyQt4 import QtGui
>  
>  from openlp.core.lib import Plugin, StringContent, translate, build_icon
> -from openlp.plugins.remotes.lib import RemoteTab, OpenLPServer
> +from openlp.plugins.remotes.lib import RemoteTab, OpenLPServer, TimeSourceType
>  
>  log = logging.getLogger(__name__)
>  
>  __default_settings__ = {
> +    'remotes/time source': TimeSourceType.Client,
>      'remotes/twelve hour': True,
>      'remotes/port': 4316,
>      'remotes/https port': 4317,
> @@ -41,7 +42,6 @@
>      'remotes/thumbnails': True
>  }
>  
> -
>  class RemotesPlugin(Plugin):
>      log.info('Remote Plugin loaded')
>  
> 
> === modified file 'tests/functional/openlp_plugins/remotes/test_remotetab.py'
> --- tests/functional/openlp_plugins/remotes/test_remotetab.py	2015-01-18 13:39:21 +0000
> +++ tests/functional/openlp_plugins/remotes/test_remotetab.py	2015-02-08 20:35:01 +0000
> @@ -30,10 +30,12 @@
>  
>  from openlp.core.common import Settings
>  from openlp.plugins.remotes.lib.remotetab import RemoteTab
> +from openlp.plugins.remotes.lib import TimeSourceType
>  from tests.functional import patch
>  from tests.helpers.testmixin import TestMixin
>  
>  __default_settings__ = {
> +    'remotes/time source': TimeSourceType.Client,
>      'remotes/twelve hour': True,
>      'remotes/port': 4316,
>      'remotes/https port': 4317,
> 


-- 
https://code.launchpad.net/~oliwee/openlp/selecttimesource/+merge/249019
Your team OpenLP Core is subscribed to branch lp:openlp.