From 7badbca8cf96132c2e187654b1774ad32f82bcae Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 22 Nov 2017 20:21:57 +0000 Subject: [PATCH 01/20] Pathfixes --- openlp/plugins/media/forms/mediaclipselectorform.py | 3 ++- openlp/plugins/media/lib/mediaitem.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/media/forms/mediaclipselectorform.py b/openlp/plugins/media/forms/mediaclipselectorform.py index af08870df..2bbb5fcdf 100644 --- a/openlp/plugins/media/forms/mediaclipselectorform.py +++ b/openlp/plugins/media/forms/mediaclipselectorform.py @@ -28,6 +28,7 @@ from datetime import datetime from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import is_win, is_linux, is_macosx +from openlp.core.common.path import Path from openlp.core.common.i18n import translate from openlp.core.common.mixins import RegistryProperties from openlp.plugins.media.forms.mediaclipselectordialog import Ui_MediaClipSelector @@ -615,7 +616,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro break # Append the new name to the optical string and the path optical += new_optical_name + ':' + path - self.media_item.add_optical_clip(optical) + self.media_item.add_optical_clip(Path(optical)) def media_state_wait(self, media_state): """ diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index e47ed70e0..99454ff13 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -455,5 +455,5 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): return # Append the optical string to the media list file_paths.append(optical) - self.load_list([optical]) + self.load_list([str(optical)]) Settings().setValue(self.settings_section + '/media files', file_paths) From 29f7d8967f58c17adacc2c145e73066957b6900f Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 22 Nov 2017 21:39:40 +0000 Subject: [PATCH 02/20] service fixes --- openlp/core/ui/media/mediacontroller.py | 2 -- openlp/core/ui/servicemanager.py | 7 +++++-- openlp/plugins/media/lib/mediaitem.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index c72d6669d..9c8973ca8 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/openlp/core/ui/media/mediacontroller.py @@ -498,8 +498,6 @@ class MediaController(RegistryBase, LogMixin, RegistryProperties): :param controller: The media controller. :return: True if setup succeeded else False. """ - if controller is None: - controller = self.display_controllers[DisplayControllerType.Plugin] # stop running videos self.media_reset(controller) # Setup media info diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index e6fb34e2f..76d696ba0 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -350,7 +350,10 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi if modified: self.service_id += 1 self._modified = modified - service_file = self.short_file_name() or translate('OpenLP.ServiceManager', 'Untitled Service') + if self._service_path: + service_file = self._service_path.name + else: + service_file = translate('OpenLP.ServiceManager', 'Untitled Service') self.main_window.set_service_modified(modified, service_file) def is_modified(self): @@ -367,7 +370,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi :rtype: None """ self._service_path = file_path - self.main_window.set_service_modified(self.is_modified(), self.short_file_name()) + self.main_window.set_service_modified(self.is_modified(), file_path.name) Settings().setValue('servicemanager/last file', file_path) if file_path and file_path.suffix == '.oszl': self._save_lite = True diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index 99454ff13..8ca595040 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -269,7 +269,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): service_item.add_from_command(filename, name, CLAPPERBOARD) service_item.title = clip_name # Set the length - self.media_controller.media_setup_optical(name, title, audio_track, subtitle_track, start, end, None, None) + #self.media_controller.media_setup_optical(name, title, audio_track, subtitle_track, start, end, None, None) service_item.set_media_length((end - start) / 1000) service_item.start_time = start / 1000 service_item.end_time = end / 1000 From 572e1efb9c207498e6ce5a2831a401c12c64b47c Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 22 Nov 2017 21:56:56 +0000 Subject: [PATCH 03/20] Fix dvd clip selection. Pretty much coppied from lp:~mikey74/openlp/dvdplayerfix/ Fixes: https://launchpad.net/bugs/1514545 --- openlp/core/ui/media/mediacontroller.py | 6 +++--- openlp/core/ui/media/vlcplayer.py | 11 ++++++----- openlp/plugins/media/forms/mediaclipselectorform.py | 10 +++++----- openlp/plugins/media/lib/mediaitem.py | 7 +++---- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index 9c8973ca8..10c384b0c 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/openlp/core/ui/media/mediacontroller.py @@ -507,9 +507,9 @@ class MediaController(RegistryBase, LogMixin, RegistryProperties): controller.media_info.media_type = MediaType.CD else: controller.media_info.media_type = MediaType.DVD - controller.media_info.start_time = start // 1000 - controller.media_info.end_time = end // 1000 - controller.media_info.length = (end - start) // 1000 + controller.media_info.start_time = start + controller.media_info.end_time = end + controller.media_info.length = (end - start) controller.media_info.title_track = title controller.media_info.audio_track = audio_track controller.media_info.subtitle_track = subtitle_track diff --git a/openlp/core/ui/media/vlcplayer.py b/openlp/core/ui/media/vlcplayer.py index 605f6a8a1..4ad9d990f 100644 --- a/openlp/core/ui/media/vlcplayer.py +++ b/openlp/core/ui/media/vlcplayer.py @@ -280,7 +280,8 @@ class VlcPlayer(MediaPlayer): start_time = controller.media_info.start_time log.debug('mediatype: ' + str(controller.media_info.media_type)) # Set tracks for the optical device - if controller.media_info.media_type == MediaType.DVD: + if controller.media_info.media_type == MediaType.DVD and \ + self.get_live_state() != MediaState.Paused and self.get_preview_state() != MediaState.Paused: log.debug('vlc play, playing started') if controller.media_info.title_track > 0: log.debug('vlc play, title_track set: ' + str(controller.media_info.title_track)) @@ -350,7 +351,7 @@ class VlcPlayer(MediaPlayer): """ if display.controller.media_info.media_type == MediaType.CD \ or display.controller.media_info.media_type == MediaType.DVD: - seek_value += int(display.controller.media_info.start_time * 1000) + seek_value += int(display.controller.media_info.start_time) if display.vlc_media_player.is_seekable(): display.vlc_media_player.set_time(seek_value) @@ -386,15 +387,15 @@ class VlcPlayer(MediaPlayer): self.stop(display) controller = display.controller if controller.media_info.end_time > 0: - if display.vlc_media_player.get_time() > controller.media_info.end_time * 1000: + if display.vlc_media_player.get_time() > controller.media_info.end_time: self.stop(display) self.set_visible(display, False) if not controller.seek_slider.isSliderDown(): controller.seek_slider.blockSignals(True) if display.controller.media_info.media_type == MediaType.CD \ or display.controller.media_info.media_type == MediaType.DVD: - controller.seek_slider.setSliderPosition(display.vlc_media_player.get_time() - - int(display.controller.media_info.start_time * 1000)) + controller.seek_slider.setSliderPosition( + display.vlc_media_player.get_time() - int(display.controller.media_info.start_time)) else: controller.seek_slider.setSliderPosition(display.vlc_media_player.get_time()) controller.seek_slider.blockSignals(False) diff --git a/openlp/plugins/media/forms/mediaclipselectorform.py b/openlp/plugins/media/forms/mediaclipselectorform.py index 2bbb5fcdf..be9e69fbd 100644 --- a/openlp/plugins/media/forms/mediaclipselectorform.py +++ b/openlp/plugins/media/forms/mediaclipselectorform.py @@ -110,7 +110,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro self.subtitle_tracks_combobox.clear() self.audio_tracks_combobox.clear() self.titles_combo_box.clear() - time = QtCore.QTime() + time = QtCore.QTime(0, 0, 0) self.start_position_edit.setTime(time) self.end_timeedit.setTime(time) self.position_timeedit.setTime(time) @@ -295,7 +295,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro :param clicked: Given from signal, not used. """ vlc_ms_pos = self.vlc_media_player.get_time() - time = QtCore.QTime() + time = QtCore.QTime(0, 0, 0) new_pos_time = time.addMSecs(vlc_ms_pos) self.start_position_edit.setTime(new_pos_time) # If start time is after end time, update end time. @@ -311,7 +311,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro :param clicked: Given from signal, not used. """ vlc_ms_pos = self.vlc_media_player.get_time() - time = QtCore.QTime() + time = QtCore.QTime(0, 0, 0) new_pos_time = time.addMSecs(vlc_ms_pos) self.end_timeedit.setTime(new_pos_time) # If start time is after end time, update start time. @@ -448,7 +448,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro self.position_slider.setMaximum(self.playback_length) # setup start and end time rounded_vlc_ms_length = int(round(self.playback_length / 100.0) * 100.0) - time = QtCore.QTime() + time = QtCore.QTime(0, 0, 0) playback_length_time = time.addMSecs(rounded_vlc_ms_length) self.start_position_edit.setMaximumTime(playback_length_time) self.end_timeedit.setMaximumTime(playback_length_time) @@ -506,7 +506,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro if self.vlc_media_player: vlc_ms_pos = self.vlc_media_player.get_time() rounded_vlc_ms_pos = int(round(vlc_ms_pos / 100.0) * 100.0) - time = QtCore.QTime() + time = QtCore.QTime(0, 0, 0) new_pos_time = time.addMSecs(rounded_vlc_ms_pos) self.position_timeedit.setTime(new_pos_time) self.position_slider.setSliderPosition(vlc_ms_pos) diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index 8ca595040..1c7556aa9 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -269,10 +269,9 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): service_item.add_from_command(filename, name, CLAPPERBOARD) service_item.title = clip_name # Set the length - #self.media_controller.media_setup_optical(name, title, audio_track, subtitle_track, start, end, None, None) - service_item.set_media_length((end - start) / 1000) - service_item.start_time = start / 1000 - service_item.end_time = end / 1000 + service_item.set_media_length(end - start) + service_item.start_time = start + service_item.end_time = end service_item.add_capability(ItemCapabilities.IsOptical) else: if not os.path.exists(filename): From 8fa9bdcf580f5842b87199a2bdafc01831eb0162 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Thu, 30 Nov 2017 15:15:11 -0700 Subject: [PATCH 04/20] Skip the test if not on Linux --- .../functional/openlp_plugins/songs/test_openoffice.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_openoffice.py b/tests/functional/openlp_plugins/songs/test_openoffice.py index 4172a553c..82cce6e10 100644 --- a/tests/functional/openlp_plugins/songs/test_openoffice.py +++ b/tests/functional/openlp_plugins/songs/test_openoffice.py @@ -25,12 +25,14 @@ This module contains tests for the OpenOffice/LibreOffice importer. from unittest import TestCase, SkipTest from unittest.mock import MagicMock, patch -from openlp.core.common.registry import Registry -try: - from openlp.plugins.songs.lib.importers.openoffice import OpenOfficeImport -except ImportError: +from openlp.core.common import is_linux + +if not is_linux: raise SkipTest('Could not import OpenOfficeImport probably due to unavailability of uno') +from openlp.core.common.registry import Registry +from openlp.plugins.songs.lib.importers.openoffice import OpenOfficeImport + from tests.helpers.testmixin import TestMixin From d9e2994deb6ec562e0c91cb4128eb968b31645de Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Thu, 30 Nov 2017 15:29:41 -0700 Subject: [PATCH 05/20] Actually run the function, ID10T --- tests/functional/openlp_plugins/songs/test_openoffice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_openoffice.py b/tests/functional/openlp_plugins/songs/test_openoffice.py index 82cce6e10..e122da806 100644 --- a/tests/functional/openlp_plugins/songs/test_openoffice.py +++ b/tests/functional/openlp_plugins/songs/test_openoffice.py @@ -27,7 +27,7 @@ from unittest.mock import MagicMock, patch from openlp.core.common import is_linux -if not is_linux: +if not is_linux(): raise SkipTest('Could not import OpenOfficeImport probably due to unavailability of uno') from openlp.core.common.registry import Registry From b3669c4f5dd49c7b5ee2fc3033aedd7908854156 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Thu, 30 Nov 2017 15:46:03 -0700 Subject: [PATCH 06/20] Change things around a bit --- .../openlp_plugins/songs/test_openoffice.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_openoffice.py b/tests/functional/openlp_plugins/songs/test_openoffice.py index e122da806..45ef2acfd 100644 --- a/tests/functional/openlp_plugins/songs/test_openoffice.py +++ b/tests/functional/openlp_plugins/songs/test_openoffice.py @@ -22,20 +22,20 @@ """ This module contains tests for the OpenOffice/LibreOffice importer. """ -from unittest import TestCase, SkipTest +from unittest import TestCase, skipIf from unittest.mock import MagicMock, patch -from openlp.core.common import is_linux - -if not is_linux(): - raise SkipTest('Could not import OpenOfficeImport probably due to unavailability of uno') - from openlp.core.common.registry import Registry -from openlp.plugins.songs.lib.importers.openoffice import OpenOfficeImport from tests.helpers.testmixin import TestMixin +try: + from openlp.plugins.songs.lib.importers.openoffice import OpenOfficeImport +except ImportError: + OpenOfficeImport = None + +@skipIf(OpenOfficeImport is None, 'Could not import OpenOfficeImport probably due to unavailability of uno') class TestOpenOfficeImport(TestCase, TestMixin): """ Test the :class:`~openlp.plugins.songs.lib.importer.openoffice.OpenOfficeImport` class From ba392da66590265395a2632c02122df8e74f65c6 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Thu, 30 Nov 2017 17:31:48 -0700 Subject: [PATCH 07/20] Skip locale test on macOS until we can figure it out --- tests/functional/openlp_core/common/test_i18n.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/functional/openlp_core/common/test_i18n.py b/tests/functional/openlp_core/common/test_i18n.py index bffb819dc..4f4ca2aec 100644 --- a/tests/functional/openlp_core/common/test_i18n.py +++ b/tests/functional/openlp_core/common/test_i18n.py @@ -22,8 +22,10 @@ """ Package to test the openlp.core.lib.languages package. """ +from unittest import skipIf from unittest.mock import MagicMock, patch +from openlp.core.common import is_macosx from openlp.core.common.i18n import LANGUAGES, Language, UiStrings, get_language, get_locale_key, get_natural_key, \ translate @@ -110,6 +112,7 @@ def test_get_language_invalid_with_none(): assert language is None +@skipIf(is_macosx(), 'This test doesn\'t work on macOS currently') def test_get_locale_key(): """ Test the get_locale_key(string) function From e5d82de5dbd9dec461244de30bde170b14a2742d Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Thu, 30 Nov 2017 17:44:22 -0700 Subject: [PATCH 08/20] Add new macOS build to CI script --- scripts/jenkins_script.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/jenkins_script.py b/scripts/jenkins_script.py index a669de71e..f6d07236f 100755 --- a/scripts/jenkins_script.py +++ b/scripts/jenkins_script.py @@ -63,9 +63,10 @@ class OpenLPJobs(object): Branch_Coverage = 'Branch-04b-Test_Coverage' Branch_Pylint = 'Branch-04c-Code_Analysis2' Branch_AppVeyor = 'Branch-05-AppVeyor-Tests' + Branch_macOS = 'Branch-07-macOS-Tests' Jobs = [Branch_Pull, Branch_Functional, Branch_Interface, Branch_PEP, Branch_Coverage, Branch_Pylint, - Branch_AppVeyor] + Branch_AppVeyor, Branch_macOS] class Colour(object): From 287debf9cf6580859f7ec606a98e1b35e207e4e3 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Fri, 1 Dec 2017 00:26:51 -0700 Subject: [PATCH 09/20] Add a way to continue watching all jobs, even if one fails --- scripts/jenkins_script.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/jenkins_script.py b/scripts/jenkins_script.py index f6d07236f..c7ca693be 100755 --- a/scripts/jenkins_script.py +++ b/scripts/jenkins_script.py @@ -116,7 +116,7 @@ class JenkinsTrigger(object): self.fetch_jobs() self.server.build_job(OpenLPJobs.Branch_Pull, {'BRANCH_NAME': self.repo_name, 'cause': cause}) - def print_output(self): + def print_output(self, can_continue=False): """ Print the status information of the build triggered. """ @@ -127,13 +127,21 @@ class JenkinsTrigger(object): revno = raw_output.decode().strip() print('%s (revision %s)' % (get_repo_name(), revno)) + failed_builds = [] for job in OpenLPJobs.Jobs: if not self.__print_build_info(job): if self.current_build: - print('Stopping after failure, see {}console for more details'.format(self.current_build['url'])) - else: + failed_builds.append((self.current_build['fullDisplayName'], self.current_build['url'])) + if not can_continue: print('Stopping after failure') - break + break + print('') + if failed_builds: + print('Failed builds:') + for build_name, url in failed_builds: + print(' - {}: {}console'.format(build_name, url)) + else: + print('All builds passed') def open_browser(self): """ @@ -228,6 +236,7 @@ def main(): help='Disable coloured output (always disabled on Windows)') parser.add_argument('-u', '--username', required=True, help='Your Jenkins username') parser.add_argument('-p', '--password', required=True, help='Your Jenkins password or personal token') + parser.add_argument('-c', '--always-continue', action='store_true', default=False, help='Continue despite failure') args = parser.parse_args() if not get_repo_name(): @@ -239,7 +248,7 @@ def main(): if args.open_browser: jenkins_trigger.open_browser() if not args.disable_output: - jenkins_trigger.print_output() + jenkins_trigger.print_output(can_continue=args.always_continue) if __name__ == '__main__': From 90118af85c9f3aaf389de141d07b379cff266330 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 2 Dec 2017 09:11:22 +0000 Subject: [PATCH 10/20] fix abend --- openlp/core/api/http/server.py | 15 ++++++++------- openlp/core/api/websockets.py | 13 +++++++------ openlp/core/ui/mainwindow.py | 5 ++--- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py index 782940f2d..b17888ddb 100644 --- a/openlp/core/api/http/server.py +++ b/openlp/core/api/http/server.py @@ -82,13 +82,14 @@ class HttpServer(RegistryBase, RegistryProperties, LogMixin): Initialise the http server, and start the http server """ super(HttpServer, self).__init__(parent) - self.worker = HttpWorker() - self.thread = QtCore.QThread() - self.worker.moveToThread(self.thread) - self.thread.started.connect(self.worker.run) - self.thread.start() - Registry().register_function('download_website', self.first_time) - Registry().register_function('get_website_version', self.website_version) + if Registry().get_flag('no_web_server'): + self.worker = HttpWorker() + self.thread = QtCore.QThread() + self.worker.moveToThread(self.thread) + self.thread.started.connect(self.worker.run) + self.thread.start() + Registry().register_function('download_website', self.first_time) + Registry().register_function('get_website_version', self.website_version) Registry().set_flag('website_version', '0.0') def bootstrap_post_set_up(self): diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py index 90dca8208..3417eb74d 100644 --- a/openlp/core/api/websockets.py +++ b/openlp/core/api/websockets.py @@ -70,12 +70,13 @@ class WebSocketServer(RegistryProperties, LogMixin): Initialise and start the WebSockets server """ super(WebSocketServer, self).__init__() - self.settings_section = 'api' - self.worker = WebSocketWorker(self) - self.thread = QtCore.QThread() - self.worker.moveToThread(self.thread) - self.thread.started.connect(self.worker.run) - self.thread.start() + if Registry().get_flag('no_web_server'): + self.settings_section = 'api' + self.worker = WebSocketWorker(self) + self.thread = QtCore.QThread() + self.worker.moveToThread(self.thread) + self.thread.started.connect(self.worker.run) + self.thread.start() def start_server(self): """ diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 312e9d445..ee07cbd69 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -504,9 +504,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): Settings().set_up_default_values() self.about_form = AboutForm(self) MediaController() - if Registry().get_flag('no_web_server'): - websockets.WebSocketServer() - server.HttpServer() + websockets.WebSocketServer() + server.HttpServer() SettingsForm(self) self.formatting_tag_form = FormattingTagForm(self) self.shortcut_form = ShortcutListForm(self) From 4f882c460b7d631e2d1683c199233bd98caa70ca Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 2 Dec 2017 09:31:13 +0000 Subject: [PATCH 11/20] fix tests --- .../openlp_core/api/http/test_http.py | 18 +++++++++++++++++- .../openlp_core/api/test_websockets.py | 18 +++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/tests/functional/openlp_core/api/http/test_http.py b/tests/functional/openlp_core/api/http/test_http.py index d9002b2ec..b3a3f6625 100644 --- a/tests/functional/openlp_core/api/http/test_http.py +++ b/tests/functional/openlp_core/api/http/test_http.py @@ -45,12 +45,28 @@ class TestHttpServer(TestCase): @patch('openlp.core.api.http.server.QtCore.QThread') def test_server_start(self, mock_qthread, mock_thread): """ - Test the starting of the Waitress Server + Test the starting of the Waitress Server with the disable flag set off """ # GIVEN: A new httpserver # WHEN: I start the server + Registry().set_flag('no_web_server', True) HttpServer() # THEN: the api environment should have been created self.assertEquals(1, mock_qthread.call_count, 'The qthread should have been called once') self.assertEquals(1, mock_thread.call_count, 'The http thread should have been called once') + + @patch('openlp.core.api.http.server.HttpWorker') + @patch('openlp.core.api.http.server.QtCore.QThread') + def test_server_start(self, mock_qthread, mock_thread): + """ + Test the starting of the Waitress Server with the disable flag set off + """ + # GIVEN: A new httpserver + # WHEN: I start the server + Registry().set_flag('no_web_server', False) + HttpServer() + + # THEN: the api environment should have been created + self.assertEquals(0, mock_qthread.call_count, 'The qthread should not have have been called') + self.assertEquals(0, mock_thread.call_count, 'The http thread should not have been called') diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py index 99abcdbc0..f400c50a5 100644 --- a/tests/functional/openlp_core/api/test_websockets.py +++ b/tests/functional/openlp_core/api/test_websockets.py @@ -66,16 +66,32 @@ class TestWSServer(TestCase, TestMixin): @patch('openlp.core.api.websockets.QtCore.QThread') def test_serverstart(self, mock_qthread, mock_worker): """ - Test the starting of the WebSockets Server + Test the starting of the WebSockets Server with the disabled flag set on """ # GIVEN: A new httpserver # WHEN: I start the server + Registry().set_flag('no_web_server', True) WebSocketServer() # THEN: the api environment should have been created self.assertEquals(1, mock_qthread.call_count, 'The qthread should have been called once') self.assertEquals(1, mock_worker.call_count, 'The http thread should have been called once') + @patch('openlp.core.api.websockets.WebSocketWorker') + @patch('openlp.core.api.websockets.QtCore.QThread') + def test_serverstart_not_required(self, mock_qthread, mock_worker): + """ + Test the starting of the WebSockets Server with the disabled flag set off + """ + # GIVEN: A new httpserver and the server is not required + # WHEN: I start the server + Registry().set_flag('no_web_server', False) + WebSocketServer() + + # THEN: the api environment should have been created + self.assertEquals(0, mock_qthread.call_count, 'The qthread should not have been called') + self.assertEquals(0, mock_worker.call_count, 'The http thread should not have been called') + def test_main_poll(self): """ Test the main_poll function returns the correct JSON From 0a695ea7e650901eaec2637fafc8aa2f2e4610be Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 2 Dec 2017 09:37:55 +0000 Subject: [PATCH 12/20] fix tests --- tests/functional/openlp_core/ui/test_mainwindow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_core/ui/test_mainwindow.py b/tests/functional/openlp_core/ui/test_mainwindow.py index 5e1a69cbc..4a0404eab 100644 --- a/tests/functional/openlp_core/ui/test_mainwindow.py +++ b/tests/functional/openlp_core/ui/test_mainwindow.py @@ -155,7 +155,7 @@ class TestMainWindow(TestCase, TestMixin): # WHEN: you check the started functions # THEN: the following registry functions should have been registered - assert len(self.registry.service_list) == 12, \ + assert len(self.registry.service_list) == 13, \ 'The registry should have 12 services, got {}'.format(self.registry.service_list.keys()) assert len(self.registry.functions_list) == 19, \ 'The registry should have 19 functions, got {}'.format(self.registry.functions_list.keys()) From 70d2d73171842a1227687954b1372c0043a09e62 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 2 Dec 2017 10:52:13 +0000 Subject: [PATCH 13/20] missed test name --- tests/functional/openlp_core/api/http/test_http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_core/api/http/test_http.py b/tests/functional/openlp_core/api/http/test_http.py index b3a3f6625..ed584c1b9 100644 --- a/tests/functional/openlp_core/api/http/test_http.py +++ b/tests/functional/openlp_core/api/http/test_http.py @@ -58,7 +58,7 @@ class TestHttpServer(TestCase): @patch('openlp.core.api.http.server.HttpWorker') @patch('openlp.core.api.http.server.QtCore.QThread') - def test_server_start(self, mock_qthread, mock_thread): + def test_server_start_not_required(self, mock_qthread, mock_thread): """ Test the starting of the Waitress Server with the disable flag set off """ From 582e2e267a291a18609a108a6b5fbd7095ff55fa Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 2 Dec 2017 21:47:11 +0000 Subject: [PATCH 14/20] Minor fixes Fixes: https://launchpad.net/bugs/1735765 --- openlp/core/api/deploy.py | 2 ++ openlp/core/api/http/server.py | 5 ++++- openlp/core/app.py | 4 ++-- openlp/core/common/settings.py | 2 +- openlp/core/ui/servicemanager.py | 3 ++- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/openlp/core/api/deploy.py b/openlp/core/api/deploy.py index b64cc40d5..a42f83f0c 100644 --- a/openlp/core/api/deploy.py +++ b/openlp/core/api/deploy.py @@ -52,6 +52,8 @@ def download_sha256(): web_config = get_web_page('https://get.openlp.org/webclient/download.cfg', headers={'User-Agent': user_agent}) except ConnectionError: return False + if not web_config: + return None file_bits = web_config.split() return file_bits[0], file_bits[2] diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py index b17888ddb..c80275801 100644 --- a/openlp/core/api/http/server.py +++ b/openlp/core/api/http/server.py @@ -67,7 +67,10 @@ class HttpWorker(QtCore.QObject): address = Settings().value('api/ip address') port = Settings().value('api/port') Registry().execute('get_website_version') - serve(application, host=address, port=port) + try: + serve(application, host=address, port=port) + except OSError: + log.exception('An error occurred when serving the application.') def stop(self): pass diff --git a/openlp/core/app.py b/openlp/core/app.py index 19943e3f0..114a62807 100644 --- a/openlp/core/app.py +++ b/openlp/core/app.py @@ -403,8 +403,8 @@ def main(args=None): .format(back_up_path=back_up_path)) QtWidgets.QMessageBox.information( None, translate('OpenLP', 'Settings Upgrade'), - translate('OpenLP', 'Your settings are about to upgraded. A backup will be created at {back_up_path}') - .format(back_up_path=back_up_path)) + translate('OpenLP', 'Your settings are about to be upgraded. A backup will be created at ' + '{back_up_path}').format(back_up_path=back_up_path)) settings.export(back_up_path) settings.upgrade_settings() # First time checks in settings diff --git a/openlp/core/common/settings.py b/openlp/core/common/settings.py index 225feb4e1..54f1d9b2a 100644 --- a/openlp/core/common/settings.py +++ b/openlp/core/common/settings.py @@ -236,7 +236,7 @@ class Settings(QtCore.QSettings): ('bibles/last search type', '', []), ('custom/last search type', 'custom/last used search type', []), # The following changes are being made for the conversion to using Path objects made in 2.6 development - ('advanced/data path', 'advanced/data path', [(str_to_path, None)]), + ('advanced/data path', 'advanced/data path', [(lambda p: Path(p) if p is not None else None, None)]), ('crashreport/last directory', 'crashreport/last directory', [(str_to_path, None)]), ('servicemanager/last directory', 'servicemanager/last directory', [(str_to_path, None)]), ('servicemanager/last file', 'servicemanager/last file', [(str_to_path, None)]), diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 76d696ba0..29718e09a 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -389,7 +389,8 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi """ Return the current file name, excluding the path. """ - return self._service_path.name + if self._service_path: + return self._service_path.name def reset_supported_suffixes(self): """ From 7043a20530ba9ccf08ae60c41bac6a6942ef27ae Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 2 Dec 2017 22:10:22 +0000 Subject: [PATCH 15/20] Few test fixes --- .../openlp_core/lib/test_exceptions.py | 45 +++++++++++++++++++ .../openlp_core/lib/test_mediamanageritem.py | 18 ++++---- 2 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 tests/functional/openlp_core/lib/test_exceptions.py diff --git a/tests/functional/openlp_core/lib/test_exceptions.py b/tests/functional/openlp_core/lib/test_exceptions.py new file mode 100644 index 000000000..bf5efa949 --- /dev/null +++ b/tests/functional/openlp_core/lib/test_exceptions.py @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Package to test the openlp.core.lib.exceptions package. +""" +from unittest import TestCase + +from openlp.core.lib.exceptions import ValidationError + + +class TestValidationError(TestCase): + """ + Test the ValidationError Class + """ + def test_validation_error(self): + """ + Test the creation of a ValidationError + """ + # GIVEN: The ValidationError class + + # WHEN: Creating an instance of ValidationError + error = ValidationError('Test ValidationError') + + # THEN: Then calling str on the error should return the correct text and it should be an instance of `Exception` + assert str(error) == 'Test ValidationError' + assert isinstance(error, Exception) diff --git a/tests/functional/openlp_core/lib/test_mediamanageritem.py b/tests/functional/openlp_core/lib/test_mediamanageritem.py index fba8ce36b..a6152cbdf 100644 --- a/tests/functional/openlp_core/lib/test_mediamanageritem.py +++ b/tests/functional/openlp_core/lib/test_mediamanageritem.py @@ -42,8 +42,8 @@ class TestMediaManagerItem(TestCase, TestMixin): self.mocked_setup = self.setup_patcher.start() self.addCleanup(self.setup_patcher.stop) - @patch(u'openlp.core.lib.mediamanageritem.Settings') - @patch(u'openlp.core.lib.mediamanageritem.MediaManagerItem.on_preview_click') + @patch('openlp.core.lib.mediamanageritem.Settings') + @patch('openlp.core.lib.mediamanageritem.MediaManagerItem.on_preview_click') def test_on_double_clicked(self, mocked_on_preview_click, MockedSettings): """ Test that when an item is double-clicked then the item is previewed @@ -75,8 +75,8 @@ class TestMediaManagerItem(TestCase, TestMixin): self.assertTrue(mmi.has_delete_icon, 'By default a delete icon should be present') self.assertFalse(mmi.add_to_service_item, 'There should be no add_to_service icon by default') - @patch(u'openlp.core.lib.mediamanageritem.Settings') - @patch(u'openlp.core.lib.mediamanageritem.MediaManagerItem.on_live_click') + @patch('openlp.core.lib.mediamanageritem.Settings') + @patch('openlp.core.lib.mediamanageritem.MediaManagerItem.on_live_click') def test_on_double_clicked_go_live(self, mocked_on_live_click, MockedSettings): """ Test that when "Double-click to go live" is enabled that the item goes live @@ -93,9 +93,9 @@ class TestMediaManagerItem(TestCase, TestMixin): # THEN: on_live_click() should have been called mocked_on_live_click.assert_called_with() - @patch(u'openlp.core.lib.mediamanageritem.Settings') - @patch(u'openlp.core.lib.mediamanageritem.MediaManagerItem.on_live_click') - @patch(u'openlp.core.lib.mediamanageritem.MediaManagerItem.on_preview_click') + @patch('openlp.core.lib.mediamanageritem.Settings') + @patch('openlp.core.lib.mediamanageritem.MediaManagerItem.on_live_click') + @patch('openlp.core.lib.mediamanageritem.MediaManagerItem.on_preview_click') def test_on_double_clicked_single_click_preview(self, mocked_on_preview_click, mocked_on_live_click, MockedSettings): """ @@ -111,5 +111,5 @@ class TestMediaManagerItem(TestCase, TestMixin): mmi.on_double_clicked() # THEN: on_live_click() should have been called - self.assertEqual(0, mocked_on_live_click.call_count, u'on_live_click() should not have been called') - self.assertEqual(0, mocked_on_preview_click.call_count, u'on_preview_click() should not have been called') + self.assertEqual(0, mocked_on_live_click.call_count, 'on_live_click() should not have been called') + self.assertEqual(0, mocked_on_preview_click.call_count, 'on_preview_click() should not have been called') From 159056f06fbb49fc4aa2ecf8ecdc5884d2cb72a0 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Sun, 3 Dec 2017 16:24:47 -0800 Subject: [PATCH 16/20] PJLink2-M updates --- openlp/core/projectors/__init__.py | 2 - openlp/core/projectors/constants.py | 18 ++ openlp/core/projectors/db.py | 2 +- openlp/core/projectors/pjlink.py | 241 ++++++++++++------ .../projectors/test_projector_bugfixes_01.py | 43 +--- .../projectors/test_projector_pjlink_base.py | 80 +++--- .../test_projector_pjlink_cmd_routing.py | 56 ++-- ...y => test_projector_pjlink_commands_01.py} | 2 +- .../test_projector_pjlink_commands_02.py | 198 ++++++++++++++ 9 files changed, 449 insertions(+), 193 deletions(-) rename tests/functional/openlp_core/projectors/{test_projector_pjlink_commands.py => test_projector_pjlink_commands_01.py} (99%) create mode 100644 tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py diff --git a/openlp/core/projectors/__init__.py b/openlp/core/projectors/__init__.py index 396422902..66cfd6080 100644 --- a/openlp/core/projectors/__init__.py +++ b/openlp/core/projectors/__init__.py @@ -25,8 +25,6 @@ Initialization for the openlp.core.projectors modules. """ -from openlp.core.projectors.constants import PJLINK_PORT, ERROR_MSG, ERROR_STRING - class DialogSourceStyle(object): """ diff --git a/openlp/core/projectors/constants.py b/openlp/core/projectors/constants.py index 715896133..c700141b8 100644 --- a/openlp/core/projectors/constants.py +++ b/openlp/core/projectors/constants.py @@ -144,6 +144,24 @@ PJLINK_VALID_CMD = { } } +# QAbstractSocketState enums converted to string +S_QSOCKET_STATE = { + 0: 'QSocketState - UnconnectedState', + 1: 'QSocketState - HostLookupState', + 2: 'QSocketState - ConnectingState', + 3: 'QSocketState - ConnectedState', + 4: 'QSocketState - BoundState', + 5: 'QSocketState - ListeningState (internal use only)', + 6: 'QSocketState - ClosingState', + 'UnconnectedState': 0, + 'HostLookupState': 1, + 'ConnectingState': 2, + 'ConnectedState': 3, + 'BoundState': 4, + 'ListeningState': 5, + 'ClosingState': 6, +} + # Error and status codes S_OK = E_OK = 0 # E_OK included since I sometimes forget # Error codes. Start at 200 so we don't duplicate system error codes. diff --git a/openlp/core/projectors/db.py b/openlp/core/projectors/db.py index 99fe9515b..fe8785861 100644 --- a/openlp/core/projectors/db.py +++ b/openlp/core/projectors/db.py @@ -415,7 +415,7 @@ class ProjectorDB(Manager): for key in projector.source_available: item = self.get_object_filtered(ProjectorSource, and_(ProjectorSource.code == key, - ProjectorSource.projector_id == projector.dbid)) + ProjectorSource.projector_id == projector.id)) if item is None: source_dict[key] = PJLINK_DEFAULT_CODES[key] else: diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py index 16a65bd11..99fb6956c 100644 --- a/openlp/core/projectors/pjlink.py +++ b/openlp/core/projectors/pjlink.py @@ -58,8 +58,7 @@ from openlp.core.projectors.constants import CONNECTION_ERRORS, CR, ERROR_MSG, E E_AUTHENTICATION, E_CONNECTION_REFUSED, E_GENERAL, E_INVALID_DATA, E_NETWORK, E_NOT_CONNECTED, E_OK, \ E_PARAMETER, E_PROJECTOR, E_SOCKET_TIMEOUT, E_UNAVAILABLE, E_UNDEFINED, PJLINK_ERRORS, PJLINK_ERST_DATA, \ PJLINK_ERST_STATUS, PJLINK_MAX_PACKET, PJLINK_PORT, PJLINK_POWR_STATUS, PJLINK_VALID_CMD, \ - STATUS_STRING, S_CONNECTED, S_CONNECTING, S_INFO, S_NETWORK_RECEIVED, S_NETWORK_SENDING, \ - S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_STATUS + STATUS_STRING, S_CONNECTED, S_CONNECTING, S_INFO, S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_QSOCKET_STATE, S_STATUS log = logging.getLogger(__name__) log.debug('pjlink loaded') @@ -123,7 +122,8 @@ class PJLinkCommands(object): 'INST': self.process_inst, 'LAMP': self.process_lamp, 'NAME': self.process_name, - 'PJLINK': self.check_login, + 'PJLINK': self.process_pjlink, + # 'PJLINK': self.check_login, 'POWR': self.process_powr, 'SNUM': self.process_snum, 'SVER': self.process_sver, @@ -135,7 +135,8 @@ class PJLinkCommands(object): """ Initialize instance variables. Also used to reset projector-specific information to default. """ - log.debug('({ip}) reset_information() connect status is {state}'.format(ip=self.ip, state=self.state())) + log.debug('({ip}) reset_information() connect status is {state}'.format(ip=self.ip, + state=S_QSOCKET_STATE[self.state()])) self.fan = None # ERST self.filter_time = None # FILT self.lamp = None # LAMP @@ -165,6 +166,7 @@ class PJLinkCommands(object): self.socket_timer.stop() self.send_busy = False self.send_queue = [] + self.priority_queue = [] def process_command(self, cmd, data): """ @@ -176,18 +178,19 @@ class PJLinkCommands(object): log.debug('({ip}) Processing command "{cmd}" with data "{data}"'.format(ip=self.ip, cmd=cmd, data=data)) - # Check if we have a future command not available yet - _cmd = cmd.upper() + # cmd should already be in uppercase, but data may be in mixed-case. + # Due to some replies should stay as mixed-case, validate using separate uppercase check _data = data.upper() - if _cmd not in PJLINK_VALID_CMD: + # Check if we have a future command not available yet + if cmd not in PJLINK_VALID_CMD: log.error("({ip}) Ignoring command='{cmd}' (Invalid/Unknown)".format(ip=self.ip, cmd=cmd)) return elif _data == 'OK': - log.debug('({ip}) Command "{cmd}" returned OK'.format(ip=self.ip, cmd=cmd)) - # A command returned successfully, no further processing needed - return - elif _cmd not in self.pjlink_functions: - log.warning("({ip}) Unable to process command='{cmd}' (Future option)".format(ip=self.ip, cmd=cmd)) + log.debug("({ip}) Command '{cmd}' returned OK".format(ip=self.ip, cmd=cmd)) + # A command returned successfully, so do a query on command to verify status + return self.send_command(cmd=cmd) + elif cmd not in self.pjlink_functions: + log.warning("({ip}) Unable to process command='{cmd}' (Future option?)".format(ip=self.ip, cmd=cmd)) return elif _data in PJLINK_ERRORS: # Oops - projector error @@ -211,12 +214,10 @@ class PJLinkCommands(object): elif _data == PJLINK_ERRORS[E_PROJECTOR]: # Projector/display error self.change_status(E_PROJECTOR) - self.receive_data_signal() return # Command checks already passed log.debug('({ip}) Calling function for {cmd}'.format(ip=self.ip, cmd=cmd)) - self.receive_data_signal() - self.pjlink_functions[_cmd](data) + self.pjlink_functions[cmd](data) def process_avmt(self, data): """ @@ -430,6 +431,51 @@ class PJLinkCommands(object): log.debug('({ip}) Setting projector PJLink name to "{data}"'.format(ip=self.ip, data=self.pjlink_name)) return + def process_pjlink(self, data): + """ + Process initial socket connection to terminal. + + :param data: Initial packet with authentication scheme + """ + log.debug("({ip}) Processing PJLINK command".format(ip=self.ip)) + chk = data.split(" ") + if len(chk[0]) != 1: + # Invalid - after splitting, first field should be 1 character, either '0' or '1' only + log.error("({ip}) Invalid initial authentication scheme - aborting".format(ip=self.ip)) + return self.disconnect_from_host() + elif chk[0] == '0': + # Normal connection no authentication + if len(chk) > 1: + # Invalid data - there should be nothing after a normal authentication scheme + log.error("({ip}) Normal connection with extra information - aborting".format(ip=self.ip)) + return self.disconnect_from_host() + elif self.pin: + log.error("({ip}) Normal connection but PIN set - aborting".format(ip=self.ip)) + return self.disconnect_from_host() + else: + data_hash = None + elif chk[0] == '1': + if len(chk) < 2: + # Not enough information for authenticated connection + log.error("({ip}) Authenticated connection but not enough info - aborting".format(ip=self.ip)) + return self.disconnect_from_host() + elif not self.pin: + log.error("({ip}) Authenticate connection but no PIN - aborting".format(ip=self.ip)) + return self.disconnect_from_host() + else: + data_hash = str(qmd5_hash(salt=chk[1].encode('utf-8'), data=self.pin.encode('utf-8')), + encoding='ascii') + # Passed basic checks, so start connection + self.readyRead.connect(self.get_socket) + if not self.no_poll: + log.debug('({ip}) process_pjlink(): Starting timer'.format(ip=self.ip)) + self.timer.setInterval(2000) # Set 2 seconds for initial information + self.timer.start() + self.change_status(S_CONNECTED) + log.debug("({ip}) process_pjlink(): Sending 'CLSS' initial command'".format(ip=self.ip)) + # Since this is an initial connection, make it a priority just in case + return self.send_command(cmd="CLSS", salt=data_hash, priority=True) + def process_powr(self, data): """ Power status. See PJLink specification for format. @@ -573,6 +619,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.widget = None # QListBox entry self.timer = None # Timer that calls the poll_loop self.send_queue = [] + self.priority_queue = [] self.send_busy = False # Socket timer for some possible brain-dead projectors or network cable pulled self.socket_timer = None @@ -586,6 +633,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.connected.connect(self.check_login) self.disconnected.connect(self.disconnect_from_host) self.error.connect(self.get_error) + self.projectorReceivedData.connect(self._send_command) def thread_stopped(self): """ @@ -608,6 +656,11 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.projectorReceivedData.disconnect(self._send_command) except TypeError: pass + try: + self.readyRead.connect(self.get_socket) # Set in process_pjlink + except TypeError: + pass + self.disconnect_from_host() self.deleteLater() self.i_am_running = False @@ -625,10 +678,10 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): Retrieve information from projector that changes. Normally called by timer(). """ - if self.state() != self.ConnectedState: + if self.state() != S_QSOCKET_STATE['ConnectedState']: log.warning("({ip}) poll_loop(): Not connected - returning".format(ip=self.ip)) return - log.debug('({ip}) Updating projector status'.format(ip=self.ip)) + log.debug('({ip}) poll_loop(): Updating projector status'.format(ip=self.ip)) # Reset timer in case we were called from a set command if self.timer.interval() < self.poll_time: # Reset timer to 5 seconds @@ -640,28 +693,28 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): if self.pjlink_class == '2': check_list.extend(['FILT', 'FREZ']) for command in check_list: - self.send_command(command, queue=True) + self.send_command(command) # The following commands do not change, so only check them once if self.power == S_ON and self.source_available is None: - self.send_command('INST', queue=True) + self.send_command('INST') if self.other_info is None: - self.send_command('INFO', queue=True) + self.send_command('INFO') if self.manufacturer is None: - self.send_command('INF1', queue=True) + self.send_command('INF1') if self.model is None: - self.send_command('INF2', queue=True) + self.send_command('INF2') if self.pjlink_name is None: - self.send_command('NAME', queue=True) + self.send_command('NAME') if self.pjlink_class == '2': # Class 2 specific checks if self.serial_no is None: - self.send_command('SNUM', queue=True) + self.send_command('SNUM') if self.sw_version is None: - self.send_command('SVER', queue=True) + self.send_command('SVER') if self.model_filter is None: - self.send_command('RFIL', queue=True) + self.send_command('RFIL') if self.model_lamp is None: - self.send_command('RLMP', queue=True) + self.send_command('RLMP') def _get_status(self, status): """ @@ -713,14 +766,12 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): code=status_code, message=status_message if msg is None else msg)) self.changeStatus.emit(self.ip, status, message) + self.projectorUpdateIcons.emit() @QtCore.pyqtSlot() def check_login(self, data=None): """ - Processes the initial connection and authentication (if needed). - Starts poll timer if connection is established. - - NOTE: Qt md5 hash function doesn't work with projector authentication. Use the python md5 hash function. + Processes the initial connection and convert to a PJLink packet if valid initial connection :param data: Optional data if called from another routine """ @@ -733,12 +784,12 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.change_status(E_SOCKET_TIMEOUT) return read = self.readLine(self.max_size) - self.readLine(self.max_size) # Clean out the trailing \r\n + self.readLine(self.max_size) # Clean out any trailing whitespace if read is None: log.warning('({ip}) read is None - socket error?'.format(ip=self.ip)) return elif len(read) < 8: - log.warning('({ip}) Not enough data read)'.format(ip=self.ip)) + log.warning('({ip}) Not enough data read - skipping'.format(ip=self.ip)) return data = decode(read, 'utf-8') # Possibility of extraneous data on input when reading. @@ -750,9 +801,16 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): # PJLink initial login will be: # 'PJLink 0' - Unauthenticated login - no extra steps required. # 'PJLink 1 XXXXXX' Authenticated login - extra processing required. - if not data.upper().startswith('PJLINK'): - # Invalid response + if not data.startswith('PJLINK'): + # Invalid initial packet - close socket + log.error("({ip}) Invalid initial packet received - closing socket".format(ip=self.ip)) return self.disconnect_from_host() + log.debug("({ip}) check_login(): Formatting initial connection prompt to PJLink packet".format(ip=self.ip)) + return self.get_data("{start}{clss}{data}".format(start=PJLINK_PREFIX, + clss="1", + data=data.replace(" ", "=", 1)).encode('utf-8')) + # TODO: The below is replaced by process_pjlink() - remove when working properly + """ if '=' in data: # Processing a login reply data_check = data.strip().split('=') @@ -801,6 +859,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): log.debug('({ip}) Starting timer'.format(ip=self.ip)) self.timer.setInterval(2000) # Set 2 seconds for initial information self.timer.start() + """ def _trash_buffer(self, msg=None): """ @@ -848,32 +907,43 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): log.debug('({ip}) get_socket(): No data available (-1)'.format(ip=self.ip)) return self.receive_data_signal() self.socket_timer.stop() - return self.get_data(buff=read, ip=self.ip) + self.get_data(buff=read, ip=self.ip) + return self.receive_data_signal() - def get_data(self, buff, ip): + def get_data(self, buff, ip=None): """ Process received data :param buff: Data to process. :param ip: (optional) Destination IP. """ + ip = self.ip if ip is None else ip log.debug("({ip}) get_data(ip='{ip_in}' buffer='{buff}'".format(ip=self.ip, ip_in=ip, buff=buff)) # NOTE: Class2 has changed to some values being UTF-8 data_in = decode(buff, 'utf-8') data = data_in.strip() - if (len(data) < 7) or (not data.startswith(PJLINK_PREFIX)): - return self._trash_buffer(msg='get_data(): Invalid packet - length or prefix') + # Initial packet checks + if (len(data) < 7): + return self._trash_buffer(msg="get_data(): Invalid packet - length") elif len(data) > self.max_size: - return self._trash_buffer(msg='get_data(): Invalid packet - too long') + return self._trash_buffer(msg="get_data(): Invalid packet - too long") + elif not data.startswith(PJLINK_PREFIX): + return self._trash_buffer(msg="get_data(): Invalid packet - PJLink prefix missing") elif '=' not in data: - return self._trash_buffer(msg='get_data(): Invalid packet does not have equal') - log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.ip, data=data)) + return self._trash_buffer(msg="get_data(): Invalid packet - Does not have '='") + log.debug("({ip}) get_data(): Checking new data '{data}'".format(ip=self.ip, data=data)) header, data = data.split('=') + # At this point, the header should contain: + # "PVCCCC" + # Where: + # P = PJLINK_PREFIX + # V = PJLink class or version + # C = PJLink command try: - version, cmd = header[1], header[2:] + version, cmd = header[1], header[2:].upper() except ValueError as e: self.change_status(E_INVALID_DATA) - log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) + log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in)) return self._trash_buffer('get_data(): Expected header + command + data') if cmd not in PJLINK_VALID_CMD: log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd)) @@ -881,6 +951,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): if int(self.pjlink_class) < int(version): log.warning('({ip}) get_data(): Projector returned class reply higher ' 'than projector stated class'.format(ip=self.ip)) + self.send_busy = False return self.process_command(cmd, data) @QtCore.pyqtSlot(QtNetwork.QAbstractSocket.SocketError) @@ -910,19 +981,18 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.reset_information() return - def send_command(self, cmd, opts='?', salt=None, queue=False): + def send_command(self, cmd, opts='?', salt=None, priority=False): """ Add command to output queue if not already in queue. :param cmd: Command to send :param opts: Command option (if any) - defaults to '?' (get information) :param salt: Optional salt for md5 hash initial authentication - :param queue: Option to force add to queue rather than sending directly + :param priority: Option to send packet now rather than queue it up """ if self.state() != self.ConnectedState: log.warning('({ip}) send_command(): Not connected - returning'.format(ip=self.ip)) - self.send_queue = [] - return + return self.reset_information() if cmd not in PJLINK_VALID_CMD: log.error('({ip}) send_command(): Invalid command requested - ignoring.'.format(ip=self.ip)) return @@ -939,28 +1009,26 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): header = PJLINK_HEADER.format(linkclass=cmd_ver[0]) else: # NOTE: Once we get to version 3 then think about looping - log.error('({ip}): send_command(): PJLink class check issue? aborting'.format(ip=self.ip)) + log.error('({ip}): send_command(): PJLink class check issue? Aborting'.format(ip=self.ip)) return out = '{salt}{header}{command} {options}{suffix}'.format(salt="" if salt is None else salt, header=header, command=cmd, options=opts, suffix=CR) - if out in self.send_queue: - # Already there, so don't add - log.debug('({ip}) send_command(out="{data}") Already in queue - skipping'.format(ip=self.ip, - data=out.strip())) - elif not queue and len(self.send_queue) == 0: - # Nothing waiting to send, so just send it - log.debug('({ip}) send_command(out="{data}") Sending data'.format(ip=self.ip, data=out.strip())) - return self._send_command(data=out) + if out in self.priority_queue: + log.debug("({ip}) send_command(): Already in priority queue - skipping".format(ip=self.ip)) + elif out in self.send_queue: + log.debug("({ip}) send_command(): Already in normal queue - skipping".format(ip=self.ip)) else: - log.debug('({ip}) send_command(out="{data}") adding to queue'.format(ip=self.ip, data=out.strip())) - self.send_queue.append(out) - self.projectorReceivedData.emit() - log.debug('({ip}) send_command(): send_busy is {data}'.format(ip=self.ip, data=self.send_busy)) - if not self.send_busy: - log.debug('({ip}) send_command() calling _send_string()'.format(ip=self.ip)) + if priority: + log.debug("({ip}) send_command(): Adding to priority queue".format(ip=self.ip)) + self.priority_queue.append(out) + else: + log.debug("({ip}) send_command(): Adding to normal queue".format(ip=self.ip)) + self.send_queue.append(out) + if self.priority_queue or self.send_queue: + # May be some initial connection setup so make sure we send data self._send_command() @QtCore.pyqtSlot() @@ -971,43 +1039,53 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): :param data: Immediate data to send :param utf8: Send as UTF-8 string otherwise send as ASCII string """ - log.debug('({ip}) _send_string()'.format(ip=self.ip)) - log.debug('({ip}) _send_string(): Connection status: {data}'.format(ip=self.ip, data=self.state())) + # Funny looking data check, but it's a quick check for data=None + log.debug("({ip}) _send_command(data='{data}')".format(ip=self.ip, data=data.strip() if data else data)) + log.debug('({ip}) _send_command(): Connection status: {data}'.format(ip=self.ip, + data=S_QSOCKET_STATE[self.state()])) if self.state() != self.ConnectedState: - log.debug('({ip}) _send_string() Not connected - abort'.format(ip=self.ip)) - self.send_queue = [] + log.debug('({ip}) _send_command() Not connected - abort'.format(ip=self.ip)) self.send_busy = False - return + return self.disconnect_from_host() + if data and data not in self.priority_queue: + log.debug("({ip}) _send_command(): Priority packet - adding to priority queue".format(ip=self.ip)) + self.priority_queue.append(data) + if self.send_busy: # Still waiting for response from last command sent + log.debug("({ip}) _send_command(): Still busy, returning".format(ip=self.ip)) + log.debug('({ip}) _send_command(): Priority queue = {data}'.format(ip=self.ip, data=self.priority_queue)) + log.debug('({ip}) _send_command(): Normal queue = {data}'.format(ip=self.ip, data=self.send_queue)) return - if data is not None: - out = data - log.debug('({ip}) _send_string(data="{data}")'.format(ip=self.ip, data=out.strip())) + + if len(self.priority_queue) != 0: + out = self.priority_queue.pop(0) + log.debug("({ip}) _send_command(): Getting priority queued packet".format(ip=self.ip)) elif len(self.send_queue) != 0: out = self.send_queue.pop(0) - log.debug('({ip}) _send_string(queued data="{data}"%s)'.format(ip=self.ip, data=out.strip())) + log.debug('({ip}) _send_command(): Getting normal queued packet'.format(ip=self.ip)) else: # No data to send - log.debug('({ip}) _send_string(): No data to send'.format(ip=self.ip)) + log.debug('({ip}) _send_command(): No data to send'.format(ip=self.ip)) self.send_busy = False return self.send_busy = True - log.debug('({ip}) _send_string(): Sending "{data}"'.format(ip=self.ip, data=out.strip())) - log.debug('({ip}) _send_string(): Queue = {data}'.format(ip=self.ip, data=self.send_queue)) + log.debug('({ip}) _send_command(): Sending "{data}"'.format(ip=self.ip, data=out.strip())) self.socket_timer.start() sent = self.write(out.encode('{string_encoding}'.format(string_encoding='utf-8' if utf8 else 'ascii'))) self.waitForBytesWritten(2000) # 2 seconds should be enough if sent == -1: # Network error? - log.warning("({ip}) _send_command(): -1 received".format(ip=self.ip)) + log.warning("({ip}) _send_command(): -1 received - disconnecting from host".format(ip=self.ip)) self.change_status(E_NETWORK, translate('OpenLP.PJLink', 'Error while sending data to projector')) + self.disconnect_from_host() def connect_to_host(self): """ Initiate connection to projector. """ + log.debug("{ip}) connect_to_host(): Starting connection".format(ip=self.ip)) if self.state() == self.ConnectedState: log.warning('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip)) return @@ -1023,22 +1101,19 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): if abort: log.warning('({ip}) disconnect_from_host(): Aborting connection'.format(ip=self.ip)) else: - log.warning('({ip}) disconnect_from_host(): Not connected - returning'.format(ip=self.ip)) - self.reset_information() + log.warning('({ip}) disconnect_from_host(): Not connected'.format(ip=self.ip)) self.disconnectFromHost() try: self.readyRead.disconnect(self.get_socket) except TypeError: pass + log.debug('({ip}) disconnect_from_host() ' + 'Current status {data}'.format(ip=self.ip, data=self._get_status(self.status_connect)[0])) if abort: self.change_status(E_NOT_CONNECTED) else: - log.debug('({ip}) disconnect_from_host() ' - 'Current status {data}'.format(ip=self.ip, data=self._get_status(self.status_connect)[0])) - if self.status_connect != E_NOT_CONNECTED: - self.change_status(S_NOT_CONNECTED) + self.change_status(S_NOT_CONNECTED) self.reset_information() - self.projectorUpdateIcons.emit() def get_av_mute_status(self): """ diff --git a/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py b/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py index c33220d4a..da0aae47f 100644 --- a/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py +++ b/tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py @@ -23,12 +23,11 @@ Package to test the openlp.core.projectors.pjlink base package. """ from unittest import TestCase -from unittest.mock import patch from openlp.core.projectors.db import Projector from openlp.core.projectors.pjlink import PJLink -from tests.resources.projector.data import TEST_PIN, TEST_CONNECT_AUTHENTICATE, TEST_HASH, TEST1_DATA +from tests.resources.projector.data import TEST1_DATA class TestPJLinkBugs(TestCase): @@ -80,43 +79,17 @@ class TestPJLinkBugs(TestCase): """ Test bug 1593882 no pin and authenticated request exception """ - # GIVEN: Test object and mocks - mock_socket_timer = patch.object(self.pjlink_test, 'socket_timer').start() - mock_timer = patch.object(self.pjlink_test, 'timer').start() - mock_authentication = patch.object(self.pjlink_test, 'projectorAuthentication').start() - mock_ready_read = patch.object(self.pjlink_test, 'waitForReadyRead').start() - mock_send_command = patch.object(self.pjlink_test, 'send_command').start() - pjlink = self.pjlink_test - pjlink.pin = None - mock_ready_read.return_value = True - - # WHEN: call with authentication request and pin not set - pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE) - - # THEN: 'No Authentication' signal should have been sent - mock_authentication.emit.assert_called_with(pjlink.ip) + # Test now part of test_projector_pjlink_commands_02 + # Keeping here for bug reference + pass def test_bug_1593883_pjlink_authentication(self): """ - Test bugfix 1593883 pjlink authentication + Test bugfix 1593883 pjlink authentication and ticket 92187 """ - # GIVEN: Test object and data - mock_socket_timer = patch.object(self.pjlink_test, 'socket_timer').start() - mock_timer = patch.object(self.pjlink_test, 'timer').start() - mock_send_command = patch.object(self.pjlink_test, 'write').start() - mock_state = patch.object(self.pjlink_test, 'state').start() - mock_waitForReadyRead = patch.object(self.pjlink_test, 'waitForReadyRead').start() - pjlink = self.pjlink_test - pjlink.pin = TEST_PIN - mock_state.return_value = pjlink.ConnectedState - mock_waitForReadyRead.return_value = True - - # WHEN: Athenticated connection is called - pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE) - - # THEN: send_command should have the proper authentication - self.assertEqual("{test}".format(test=mock_send_command.call_args), - "call(b'{hash}%1CLSS ?\\r')".format(hash=TEST_HASH)) + # Test now part of test_projector_pjlink_commands_02 + # Keeping here for bug reference + pass def test_bug_1734275_process_lamp_nonstandard_reply(self): """ diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py index 7253df032..c86904bad 100644 --- a/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_base.py @@ -25,11 +25,11 @@ Package to test the openlp.core.projectors.pjlink base package. from unittest import TestCase from unittest.mock import call, patch, MagicMock -from openlp.core.projectors.constants import E_PARAMETER, ERROR_STRING, S_ON, S_CONNECTED +from openlp.core.projectors.constants import E_PARAMETER, ERROR_STRING, S_ON, S_CONNECTED, S_QSOCKET_STATE from openlp.core.projectors.db import Projector from openlp.core.projectors.pjlink import PJLink -from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE, TEST1_DATA +from tests.resources.projector.data import TEST1_DATA pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True) @@ -38,29 +38,17 @@ class TestPJLinkBase(TestCase): """ Tests for the PJLink module """ - @patch.object(pjlink_test, 'readyRead') - @patch.object(pjlink_test, 'send_command') - @patch.object(pjlink_test, 'waitForReadyRead') - @patch('openlp.core.common.qmd5_hash') - def test_authenticated_connection_call(self, - mock_qmd5_hash, - mock_waitForReadyRead, - mock_send_command, - mock_readyRead): - """ - Ticket 92187: Fix for projector connect with PJLink authentication exception. - """ - # GIVEN: Test object - pjlink = pjlink_test + def setUp(self): + ''' + TestPJLinkCommands part 2 initialization + ''' + self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True) - # WHEN: Calling check_login with authentication request: - pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE) - - # THEN: Should have called qmd5_hash - self.assertTrue(mock_qmd5_hash.called_with(TEST_SALT, - "Connection request should have been called with TEST_SALT")) - self.assertTrue(mock_qmd5_hash.called_with(TEST_PIN, - "Connection request should have been called with TEST_PIN")) + def tearDown(self): + ''' + TestPJLinkCommands part 2 cleanups + ''' + self.pjlink_test = None @patch.object(pjlink_test, 'change_status') def test_status_change(self, mock_change_status): @@ -110,18 +98,18 @@ class TestPJLinkBase(TestCase): # THEN: poll_loop should exit without calling any other method self.assertFalse(pjlink.timer.called, 'Should have returned without calling any other method') - @patch.object(pjlink_test, 'send_command') - def test_poll_loop_start(self, mock_send_command): + def test_poll_loop_start(self): """ Test PJLink.poll_loop makes correct calls """ - # GIVEN: test object and test data - pjlink = pjlink_test - pjlink.state = MagicMock() - pjlink.timer = MagicMock() - pjlink.timer.interval = MagicMock() - pjlink.timer.setInterval = MagicMock() - pjlink.timer.start = MagicMock() + # GIVEN: Mocks and test data + mock_state = patch.object(self.pjlink_test, 'state').start() + mock_state.return_value = S_QSOCKET_STATE['ConnectedState'] + mock_timer = patch.object(self.pjlink_test, 'timer').start() + mock_timer.interval.return_value = 10 + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + + pjlink = self.pjlink_test pjlink.poll_time = 20 pjlink.power = S_ON pjlink.source_available = None @@ -130,19 +118,17 @@ class TestPJLinkBase(TestCase): pjlink.model = None pjlink.pjlink_name = None pjlink.ConnectedState = S_CONNECTED - pjlink.timer.interval.return_value = 10 - pjlink.state.return_value = S_CONNECTED call_list = [ - call('POWR', queue=True), - call('ERST', queue=True), - call('LAMP', queue=True), - call('AVMT', queue=True), - call('INPT', queue=True), - call('INST', queue=True), - call('INFO', queue=True), - call('INF1', queue=True), - call('INF2', queue=True), - call('NAME', queue=True), + call('POWR'), + call('ERST'), + call('LAMP'), + call('AVMT'), + call('INPT'), + call('INST'), + call('INFO'), + call('INF1'), + call('INF2'), + call('NAME'), ] # WHEN: PJLink.poll_loop is called @@ -150,8 +136,8 @@ class TestPJLinkBase(TestCase): # THEN: proper calls were made to retrieve projector data # First, call to update the timer with the next interval - self.assertTrue(pjlink.timer.setInterval.called, 'Should have updated the timer') + self.assertTrue(mock_timer.setInterval.called) # Next, should have called the timer to start - self.assertTrue(pjlink.timer.start.called, 'Should have started the timer') + self.assertTrue(mock_timer.start.called, 'Should have started the timer') # Finally, should have called send_command with a list of projetctor status checks mock_send_command.assert_has_calls(call_list, 'Should have queued projector queries') diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py index 431da0606..a5a6eca40 100644 --- a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py @@ -46,6 +46,18 @@ class TestPJLinkRouting(TestCase): """ Tests for the PJLink module command routing """ + def setUp(self): + ''' + TestPJLinkCommands part 2 initialization + ''' + self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True) + + def tearDown(self): + ''' + TestPJLinkCommands part 2 cleanups + ''' + self.pjlink_test = None + @patch.object(openlp.core.projectors.pjlink, 'log') def test_process_command_call_clss(self, mock_log): """ @@ -163,21 +175,20 @@ class TestPJLinkRouting(TestCase): mock_change_status.assert_called_once_with(E_AUTHENTICATION) mock_log.error.assert_called_with(log_text) - @patch.object(openlp.core.projectors.pjlink, 'log') - def test_process_command_future(self, mock_log): + def test_process_command_future(self): """ Test command valid but no method to process yet """ - # GIVEN: Test object - pjlink = pjlink_test - log_text = "(127.0.0.1) Unable to process command='CLSS' (Future option)" - mock_log.reset_mock() - # Remove a valid command so we can test valid command but not available yet - pjlink.pjlink_functions.pop('CLSS') + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_functions = patch.object(self.pjlink_test, 'pjlink_functions').start() + mock_functions.return_value = [] + + pjlink = self.pjlink_test + log_text = "(111.111.111.111) Unable to process command='CLSS' (Future option?)" # WHEN: process_command called with an unknown command - with patch.object(pjlink, 'pjlink_functions') as mock_functions: - pjlink.process_command(cmd='CLSS', data='DONT CARE') + pjlink.process_command(cmd='CLSS', data='DONT CARE') # THEN: Error should be logged and no command called self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method') @@ -202,23 +213,20 @@ class TestPJLinkRouting(TestCase): self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method') mock_log.error.assert_called_once_with(log_text) - @patch.object(pjlink_test, 'pjlink_functions') - @patch.object(openlp.core.projectors.pjlink, 'log') - def test_process_command_ok(self, mock_log, mock_functions): + def test_process_command_ok(self): """ Test command returned success """ - # GIVEN: Test object - pjlink = pjlink_test - mock_functions.reset_mock() - mock_log.reset_mock() + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() - # WHEN: process_command called with an unknown command - pjlink.process_command(cmd='CLSS', data='OK') - log_text = '(127.0.0.1) Command "CLSS" returned OK' + pjlink = self.pjlink_test + log_text = "(111.111.111.111) Command 'POWR' returned OK" - # THEN: Error should be logged and no command called - self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method') - self.assertEqual(mock_log.debug.call_count, 2, 'log.debug() should have been called twice') - # Although we called it twice, only the last log entry is saved + # WHEN: process_command called with a command that returns OK + pjlink.process_command(cmd='POWR', data='OK') + + # THEN: Appropriate calls should have been made mock_log.debug.assert_called_with(log_text) + mock_send_command.assert_called_once_with(cmd='POWR') diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py similarity index 99% rename from tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py rename to tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py index 32544dd09..0917a9357 100644 --- a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py @@ -47,7 +47,7 @@ for pos in range(0, len(PJLINK_ERST_DATA)): class TestPJLinkCommands(TestCase): """ - Tests for the PJLink module + Tests for the PJLinkCommands class part 1 """ @patch.object(pjlink_test, 'changeStatus') @patch.object(openlp.core.projectors.pjlink, 'log') diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py new file mode 100644 index 000000000..08682ccb7 --- /dev/null +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py @@ -0,0 +1,198 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2015 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Package to test the openlp.core.projectors.pjlink commands package. +""" +from unittest import TestCase +from unittest.mock import patch, call + +import openlp.core.projectors.pjlink +from openlp.core.projectors.constants import S_CONNECTED +from openlp.core.projectors.db import Projector +from openlp.core.projectors.pjlink import PJLink + +from tests.resources.projector.data import TEST_HASH, TEST_PIN, TEST_SALT, TEST1_DATA + + +class TestPJLinkCommands(TestCase): + """ + Tests for the PJLinkCommands class part 2 + """ + def setUp(self): + ''' + TestPJLinkCommands part 2 initialization + ''' + self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True) + + def tearDown(self): + ''' + TestPJLinkCommands part 2 cleanups + ''' + self.pjlink_test = None + + def test_process_pjlink_normal(self): + """ + Test initial connection prompt with no authentication + """ + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, "log").start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + mock_readyRead = patch.object(self.pjlink_test, 'readyRead').start() + mock_change_status = patch.object(self.pjlink_test, 'change_status').start() + pjlink = self.pjlink_test + pjlink.pin = None + log_check = [call("({111.111.111.111}) process_pjlink(): Sending 'CLSS' initial command'"), ] + + # WHEN: process_pjlink called with no authentication required + pjlink.process_pjlink(data="0") + + # THEN: proper processing should have occured + mock_log.debug.has_calls(log_check) + mock_disconnect_from_host.assert_not_called() + mock_readyRead.connect.assert_called_once() + mock_change_status.assert_called_once_with(S_CONNECTED) + mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=None) + + def test_process_pjlink_authenticate(self): + """ + Test initial connection prompt with authentication + """ + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, "log").start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + mock_readyRead = patch.object(self.pjlink_test, 'readyRead').start() + mock_change_status = patch.object(self.pjlink_test, 'change_status').start() + pjlink = self.pjlink_test + pjlink.pin = TEST_PIN + log_check = [call("({111.111.111.111}) process_pjlink(): Sending 'CLSS' initial command'"), ] + + # WHEN: process_pjlink called with no authentication required + pjlink.process_pjlink(data='1 {salt}'.format(salt=TEST_SALT)) + + # THEN: proper processing should have occured + mock_log.debug.has_calls(log_check) + mock_disconnect_from_host.assert_not_called() + mock_readyRead.connect.assert_called_once() + mock_change_status.assert_called_once_with(S_CONNECTED) + mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=TEST_HASH) + + def test_process_pjlink_normal_pin_set_error(self): + """ + Test process_pjlinnk called with no authentication but pin is set + """ + # GIVEN: Initial mocks and data + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + pjlink = self.pjlink_test + pjlink.pin = TEST_PIN + log_check = [call('(111.111.111.111) Normal connection but PIN set - aborting'), ] + + # WHEN: process_pjlink called with invalid authentication scheme + pjlink.process_pjlink(data='0') + + # THEN: Proper calls should be made + mock_log.error.assert_has_calls(log_check) + mock_disconnect_from_host.assert_called_once() + mock_send_command.assert_not_called() + + def test_process_pjlink_normal_with_salt_error(self): + """ + Test process_pjlinnk called with no authentication but pin is set + """ + # GIVEN: Initial mocks and data + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + pjlink = self.pjlink_test + pjlink.pin = TEST_PIN + log_check = [call('(111.111.111.111) Normal connection with extra information - aborting'), ] + + # WHEN: process_pjlink called with invalid authentication scheme + pjlink.process_pjlink(data='0 {salt}'.format(salt=TEST_SALT)) + + # THEN: Proper calls should be made + mock_log.error.assert_has_calls(log_check) + mock_disconnect_from_host.assert_called_once() + mock_send_command.assert_not_called() + + def test_process_pjlink_invalid_authentication_scheme_length_error(self): + """ + Test initial connection prompt with authentication scheme longer than 1 character + """ + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + pjlink = self.pjlink_test + log_check = [call('(111.111.111.111) Invalid initial authentication scheme - aborting'), ] + + # WHEN: process_pjlink called with invalid authentication scheme + pjlink.process_pjlink(data='01') + + # THEN: socket should be closed and invalid data logged + mock_log.error.assert_has_calls(log_check) + mock_disconnect_from_host.assert_called_once() + mock_send_command.assert_not_called() + + def test_process_pjlink_invalid_authentication_data_length_error(self): + """ + Test initial connection prompt with authentication no salt + """ + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + log_check = [call('(111.111.111.111) Authenticated connection but not enough info - aborting'), ] + pjlink = self.pjlink_test + + # WHEN: process_pjlink called with no salt + pjlink.process_pjlink(data='1') + + # THEN: socket should be closed and invalid data logged + mock_log.error.assert_has_calls(log_check) + mock_disconnect_from_host.assert_called_once() + mock_send_command.assert_not_called() + + def test_process_pjlink_authenticate_pin_not_set_error(self): + """ + Test process_pjlink authentication but pin not set + """ + # GIVEN: Initial mocks and data + mock_log = patch.object(openlp.core.projectors.pjlink, 'log').start() + mock_disconnect_from_host = patch.object(self.pjlink_test, 'disconnect_from_host').start() + mock_send_command = patch.object(self.pjlink_test, 'send_command').start() + log_check = [call('(111.111.111.111) Authenticate connection but no PIN - aborting'), ] + pjlink = self.pjlink_test + pjlink.pin = None + + # WHEN: process_pjlink called with no salt + pjlink.process_pjlink(data='1 {salt}'.format(salt=TEST_SALT)) + + # THEN: socket should be closed and invalid data logged + mock_log.error.assert_has_calls(log_check) + mock_disconnect_from_host.assert_called_once() + mock_send_command.assert_not_called() From 4374200ab04c3f39e04b01d38978162adab94238 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Mon, 4 Dec 2017 20:32:02 +0000 Subject: [PATCH 17/20] Test fixes --- .../openlp_core/ui/media/test_vlcplayer.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/functional/openlp_core/ui/media/test_vlcplayer.py b/tests/functional/openlp_core/ui/media/test_vlcplayer.py index 6e2fe73c6..96ab629c6 100644 --- a/tests/functional/openlp_core/ui/media/test_vlcplayer.py +++ b/tests/functional/openlp_core/ui/media/test_vlcplayer.py @@ -693,9 +693,9 @@ class TestVLCPlayer(TestCase, TestMixin): vlc_player.set_state(MediaState.Paused, mocked_display) # WHEN: play() is called - with patch.object(vlc_player, 'media_state_wait') as mocked_media_state_wait, \ - patch.object(vlc_player, 'volume') as mocked_volume: - mocked_media_state_wait.return_value = True + with patch.object(vlc_player, 'media_state_wait', return_value=True) as mocked_media_state_wait, \ + patch.object(vlc_player, 'volume') as mocked_volume, \ + patch.object(vlc_player, 'get_live_state', return_value=MediaState.Loaded): result = vlc_player.play(mocked_display) # THEN: A bunch of things should happen to play the media @@ -872,7 +872,7 @@ class TestVLCPlayer(TestCase, TestMixin): mocked_display = MagicMock() mocked_display.controller.media_info.media_type = MediaType.DVD mocked_display.vlc_media_player.is_seekable.return_value = True - mocked_display.controller.media_info.start_time = 3 + mocked_display.controller.media_info.start_time = 3000 vlc_player = VlcPlayer(None) # WHEN: seek() is called @@ -976,7 +976,7 @@ class TestVLCPlayer(TestCase, TestMixin): mocked_display = MagicMock() mocked_display.controller = mocked_controller mocked_display.vlc_media.get_state.return_value = 1 - mocked_display.vlc_media_player.get_time.return_value = 400000 + mocked_display.vlc_media_player.get_time.return_value = 400 mocked_display.controller.media_info.media_type = MediaType.DVD vlc_player = VlcPlayer(None) @@ -990,7 +990,7 @@ class TestVLCPlayer(TestCase, TestMixin): self.assertEqual(2, mocked_stop.call_count) mocked_display.vlc_media_player.get_time.assert_called_with() mocked_set_visible.assert_called_with(mocked_display, False) - mocked_controller.seek_slider.setSliderPosition.assert_called_with(300000) + mocked_controller.seek_slider.setSliderPosition.assert_called_with(300) expected_calls = [call(True), call(False)] self.assertEqual(expected_calls, mocked_controller.seek_slider.blockSignals.call_args_list) From baed1934446668b0cd8b99151f3936d1a0266563 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Mon, 4 Dec 2017 20:49:59 +0000 Subject: [PATCH 18/20] PEP8 --- openlp/core/ui/media/vlcplayer.py | 2 +- tests/functional/openlp_core/lib/test_exceptions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/core/ui/media/vlcplayer.py b/openlp/core/ui/media/vlcplayer.py index 4ad9d990f..840471cfe 100644 --- a/openlp/core/ui/media/vlcplayer.py +++ b/openlp/core/ui/media/vlcplayer.py @@ -281,7 +281,7 @@ class VlcPlayer(MediaPlayer): log.debug('mediatype: ' + str(controller.media_info.media_type)) # Set tracks for the optical device if controller.media_info.media_type == MediaType.DVD and \ - self.get_live_state() != MediaState.Paused and self.get_preview_state() != MediaState.Paused: + self.get_live_state() != MediaState.Paused and self.get_preview_state() != MediaState.Paused: log.debug('vlc play, playing started') if controller.media_info.title_track > 0: log.debug('vlc play, title_track set: ' + str(controller.media_info.title_track)) diff --git a/tests/functional/openlp_core/lib/test_exceptions.py b/tests/functional/openlp_core/lib/test_exceptions.py index bf5efa949..c0de323b7 100644 --- a/tests/functional/openlp_core/lib/test_exceptions.py +++ b/tests/functional/openlp_core/lib/test_exceptions.py @@ -38,7 +38,7 @@ class TestValidationError(TestCase): # GIVEN: The ValidationError class # WHEN: Creating an instance of ValidationError - error = ValidationError('Test ValidationError') + error = ValidationError('Test ValidationError') # THEN: Then calling str on the error should return the correct text and it should be an instance of `Exception` assert str(error) == 'Test ValidationError' From 44b82d8ca292c7eab4f440e1860ffec99917ed5e Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Mon, 4 Dec 2017 16:54:15 -0800 Subject: [PATCH 19/20] Fix mocks to use correct python version tests --- openlp/core/projectors/pjlink.py | 2 +- .../test_projector_pjlink_commands_02.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py index 99fb6956c..0a18aef33 100644 --- a/openlp/core/projectors/pjlink.py +++ b/openlp/core/projectors/pjlink.py @@ -657,7 +657,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): except TypeError: pass try: - self.readyRead.connect(self.get_socket) # Set in process_pjlink + self.readyRead.disconnect(self.get_socket) # Set in process_pjlink except TypeError: pass diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py index 08682ccb7..62ba4ec76 100644 --- a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_02.py @@ -69,7 +69,7 @@ class TestPJLinkCommands(TestCase): # THEN: proper processing should have occured mock_log.debug.has_calls(log_check) mock_disconnect_from_host.assert_not_called() - mock_readyRead.connect.assert_called_once() + self.assertEqual(mock_readyRead.connect.call_count, 1, 'Should have only been called once') mock_change_status.assert_called_once_with(S_CONNECTED) mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=None) @@ -93,7 +93,7 @@ class TestPJLinkCommands(TestCase): # THEN: proper processing should have occured mock_log.debug.has_calls(log_check) mock_disconnect_from_host.assert_not_called() - mock_readyRead.connect.assert_called_once() + self.assertEqual(mock_readyRead.connect.call_count, 1, 'Should have only been called once') mock_change_status.assert_called_once_with(S_CONNECTED) mock_send_command.assert_called_with(cmd='CLSS', priority=True, salt=TEST_HASH) @@ -115,7 +115,7 @@ class TestPJLinkCommands(TestCase): # THEN: Proper calls should be made mock_log.error.assert_has_calls(log_check) - mock_disconnect_from_host.assert_called_once() + self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once') mock_send_command.assert_not_called() def test_process_pjlink_normal_with_salt_error(self): @@ -136,7 +136,7 @@ class TestPJLinkCommands(TestCase): # THEN: Proper calls should be made mock_log.error.assert_has_calls(log_check) - mock_disconnect_from_host.assert_called_once() + self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once') mock_send_command.assert_not_called() def test_process_pjlink_invalid_authentication_scheme_length_error(self): @@ -155,7 +155,7 @@ class TestPJLinkCommands(TestCase): # THEN: socket should be closed and invalid data logged mock_log.error.assert_has_calls(log_check) - mock_disconnect_from_host.assert_called_once() + self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once') mock_send_command.assert_not_called() def test_process_pjlink_invalid_authentication_data_length_error(self): @@ -174,7 +174,7 @@ class TestPJLinkCommands(TestCase): # THEN: socket should be closed and invalid data logged mock_log.error.assert_has_calls(log_check) - mock_disconnect_from_host.assert_called_once() + self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once') mock_send_command.assert_not_called() def test_process_pjlink_authenticate_pin_not_set_error(self): @@ -194,5 +194,5 @@ class TestPJLinkCommands(TestCase): # THEN: socket should be closed and invalid data logged mock_log.error.assert_has_calls(log_check) - mock_disconnect_from_host.assert_called_once() + self.assertEqual(mock_disconnect_from_host.call_count, 1, 'Should have only been called once') mock_send_command.assert_not_called() From eec0e325dfc1d61157706019d88493a8a5c2e253 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Sat, 9 Dec 2017 03:17:05 -0800 Subject: [PATCH 20/20] OLP Style cleanups --- openlp/core/projectors/constants.py | 2 +- openlp/core/projectors/pjlink.py | 141 +++++++++--------- .../test_projector_pjlink_cmd_routing.py | 6 +- .../test_projector_pjlink_commands_01.py | 24 +-- 4 files changed, 88 insertions(+), 85 deletions(-) diff --git a/openlp/core/projectors/constants.py b/openlp/core/projectors/constants.py index c700141b8..a9410d109 100644 --- a/openlp/core/projectors/constants.py +++ b/openlp/core/projectors/constants.py @@ -159,7 +159,7 @@ S_QSOCKET_STATE = { 'ConnectedState': 3, 'BoundState': 4, 'ListeningState': 5, - 'ClosingState': 6, + 'ClosingState': 6 } # Error and status codes diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py index 0a18aef33..12ae7e1d0 100644 --- a/openlp/core/projectors/pjlink.py +++ b/openlp/core/projectors/pjlink.py @@ -110,7 +110,7 @@ class PJLinkCommands(object): """ log.debug('PJlinkCommands(args={args} kwargs={kwargs})'.format(args=args, kwargs=kwargs)) super().__init__() - # Map command to function + # Map PJLink command to method self.pjlink_functions = { 'AVMT': self.process_avmt, 'CLSS': self.process_clss, @@ -123,6 +123,7 @@ class PJLinkCommands(object): 'LAMP': self.process_lamp, 'NAME': self.process_name, 'PJLINK': self.process_pjlink, + # TODO: Part of check_login refactor - remove when done # 'PJLINK': self.check_login, 'POWR': self.process_powr, 'SNUM': self.process_snum, @@ -183,14 +184,14 @@ class PJLinkCommands(object): _data = data.upper() # Check if we have a future command not available yet if cmd not in PJLINK_VALID_CMD: - log.error("({ip}) Ignoring command='{cmd}' (Invalid/Unknown)".format(ip=self.ip, cmd=cmd)) + log.error('({ip}) Ignoring command="{cmd}" (Invalid/Unknown)'.format(ip=self.ip, cmd=cmd)) return elif _data == 'OK': - log.debug("({ip}) Command '{cmd}' returned OK".format(ip=self.ip, cmd=cmd)) + log.debug('({ip}) Command "{cmd}" returned OK'.format(ip=self.ip, cmd=cmd)) # A command returned successfully, so do a query on command to verify status return self.send_command(cmd=cmd) elif cmd not in self.pjlink_functions: - log.warning("({ip}) Unable to process command='{cmd}' (Future option?)".format(ip=self.ip, cmd=cmd)) + log.warning('({ip}) Unable to process command="{cmd}" (Future option?)'.format(ip=self.ip, cmd=cmd)) return elif _data in PJLINK_ERRORS: # Oops - projector error @@ -260,19 +261,19 @@ class PJLinkCommands(object): # : Received: '%1CLSS=Class 1' (Optoma) # : Received: '%1CLSS=Version1' (BenQ) if len(data) > 1: - log.warning("({ip}) Non-standard CLSS reply: '{data}'".format(ip=self.ip, data=data)) + log.warning('({ip}) Non-standard CLSS reply: "{data}"'.format(ip=self.ip, data=data)) # Due to stupid projectors not following standards (Optoma, BenQ comes to mind), # AND the different responses that can be received, the semi-permanent way to # fix the class reply is to just remove all non-digit characters. try: clss = re.findall('\d', data)[0] # Should only be the first match except IndexError: - log.error("({ip}) No numbers found in class version reply '{data}' - " - "defaulting to class '1'".format(ip=self.ip, data=data)) + log.error('({ip}) No numbers found in class version reply "{data}" - ' + 'defaulting to class "1"'.format(ip=self.ip, data=data)) clss = '1' elif not data.isdigit(): - log.error("({ip}) NAN clss version reply '{data}' - " - "defaulting to class '1'".format(ip=self.ip, data=data)) + log.error('({ip}) NAN CLSS version reply "{data}" - ' + 'defaulting to class "1"'.format(ip=self.ip, data=data)) clss = '1' else: clss = data @@ -290,7 +291,7 @@ class PJLinkCommands(object): """ if len(data) != PJLINK_ERST_DATA['DATA_LENGTH']: count = PJLINK_ERST_DATA['DATA_LENGTH'] - log.warning("{ip}) Invalid error status response '{data}': length != {count}".format(ip=self.ip, + log.warning('{ip}) Invalid error status response "{data}": length != {count}'.format(ip=self.ip, data=data, count=count)) return @@ -298,7 +299,7 @@ class PJLinkCommands(object): datacheck = int(data) except ValueError: # Bad data - ignore - log.warning("({ip}) Invalid error status response '{data}'".format(ip=self.ip, data=data)) + log.warning('({ip}) Invalid error status response "{data}"'.format(ip=self.ip, data=data)) return if datacheck == 0: self.projector_errors = None @@ -437,30 +438,30 @@ class PJLinkCommands(object): :param data: Initial packet with authentication scheme """ - log.debug("({ip}) Processing PJLINK command".format(ip=self.ip)) - chk = data.split(" ") + log.debug('({ip}) Processing PJLINK command'.format(ip=self.ip)) + chk = data.split(' ') if len(chk[0]) != 1: # Invalid - after splitting, first field should be 1 character, either '0' or '1' only - log.error("({ip}) Invalid initial authentication scheme - aborting".format(ip=self.ip)) + log.error('({ip}) Invalid initial authentication scheme - aborting'.format(ip=self.ip)) return self.disconnect_from_host() elif chk[0] == '0': # Normal connection no authentication if len(chk) > 1: # Invalid data - there should be nothing after a normal authentication scheme - log.error("({ip}) Normal connection with extra information - aborting".format(ip=self.ip)) + log.error('({ip}) Normal connection with extra information - aborting'.format(ip=self.ip)) return self.disconnect_from_host() elif self.pin: - log.error("({ip}) Normal connection but PIN set - aborting".format(ip=self.ip)) + log.error('({ip}) Normal connection but PIN set - aborting'.format(ip=self.ip)) return self.disconnect_from_host() else: data_hash = None elif chk[0] == '1': if len(chk) < 2: # Not enough information for authenticated connection - log.error("({ip}) Authenticated connection but not enough info - aborting".format(ip=self.ip)) + log.error('({ip}) Authenticated connection but not enough info - aborting'.format(ip=self.ip)) return self.disconnect_from_host() elif not self.pin: - log.error("({ip}) Authenticate connection but no PIN - aborting".format(ip=self.ip)) + log.error('({ip}) Authenticate connection but no PIN - aborting'.format(ip=self.ip)) return self.disconnect_from_host() else: data_hash = str(qmd5_hash(salt=chk[1].encode('utf-8'), data=self.pin.encode('utf-8')), @@ -472,7 +473,7 @@ class PJLinkCommands(object): self.timer.setInterval(2000) # Set 2 seconds for initial information self.timer.start() self.change_status(S_CONNECTED) - log.debug("({ip}) process_pjlink(): Sending 'CLSS' initial command'".format(ip=self.ip)) + log.debug('({ip}) process_pjlink(): Sending "CLSS" initial command'.format(ip=self.ip)) # Since this is an initial connection, make it a priority just in case return self.send_command(cmd="CLSS", salt=data_hash, priority=True) @@ -496,7 +497,7 @@ class PJLinkCommands(object): self.send_command('INST') else: # Log unknown status response - log.warning('({ip}) Unknown power response: {data}'.format(ip=self.ip, data=data)) + log.warning('({ip}) Unknown power response: "{data}"'.format(ip=self.ip, data=data)) return def process_rfil(self, data): @@ -506,9 +507,9 @@ class PJLinkCommands(object): if self.model_filter is None: self.model_filter = data else: - log.warning("({ip}) Filter model already set".format(ip=self.ip)) - log.warning("({ip}) Saved model: '{old}'".format(ip=self.ip, old=self.model_filter)) - log.warning("({ip}) New model: '{new}'".format(ip=self.ip, new=data)) + log.warning('({ip}) Filter model already set'.format(ip=self.ip)) + log.warning('({ip}) Saved model: "{old}"'.format(ip=self.ip, old=self.model_filter)) + log.warning('({ip}) New model: "{new}"'.format(ip=self.ip, new=data)) def process_rlmp(self, data): """ @@ -517,9 +518,9 @@ class PJLinkCommands(object): if self.model_lamp is None: self.model_lamp = data else: - log.warning("({ip}) Lamp model already set".format(ip=self.ip)) - log.warning("({ip}) Saved lamp: '{old}'".format(ip=self.ip, old=self.model_lamp)) - log.warning("({ip}) New lamp: '{new}'".format(ip=self.ip, new=data)) + log.warning('({ip}) Lamp model already set'.format(ip=self.ip)) + log.warning('({ip}) Saved lamp: "{old}"'.format(ip=self.ip, old=self.model_lamp)) + log.warning('({ip}) New lamp: "{new}"'.format(ip=self.ip, new=data)) def process_snum(self, data): """ @@ -528,16 +529,16 @@ class PJLinkCommands(object): :param data: Serial number from projector. """ if self.serial_no is None: - log.debug("({ip}) Setting projector serial number to '{data}'".format(ip=self.ip, data=data)) + log.debug('({ip}) Setting projector serial number to "{data}"'.format(ip=self.ip, data=data)) self.serial_no = data self.db_update = False else: # Compare serial numbers and see if we got the same projector if self.serial_no != data: - log.warning("({ip}) Projector serial number does not match saved serial number".format(ip=self.ip)) - log.warning("({ip}) Saved: '{old}'".format(ip=self.ip, old=self.serial_no)) - log.warning("({ip}) Received: '{new}'".format(ip=self.ip, new=data)) - log.warning("({ip}) NOT saving serial number".format(ip=self.ip)) + log.warning('({ip}) Projector serial number does not match saved serial number'.format(ip=self.ip)) + log.warning('({ip}) Saved: "{old}"'.format(ip=self.ip, old=self.serial_no)) + log.warning('({ip}) Received: "{new}"'.format(ip=self.ip, new=data)) + log.warning('({ip}) NOT saving serial number'.format(ip=self.ip)) self.serial_no_received = data def process_sver(self, data): @@ -546,20 +547,20 @@ class PJLinkCommands(object): """ if len(data) > 32: # Defined in specs max version is 32 characters - log.warning("Invalid software version - too long") + log.warning('Invalid software version - too long') return elif self.sw_version is None: - log.debug("({ip}) Setting projector software version to '{data}'".format(ip=self.ip, data=data)) + log.debug('({ip}) Setting projector software version to "{data}"'.format(ip=self.ip, data=data)) self.sw_version = data self.db_update = True else: # Compare software version and see if we got the same projector if self.serial_no != data: - log.warning("({ip}) Projector software version does not match saved " - "software version".format(ip=self.ip)) - log.warning("({ip}) Saved: '{old}'".format(ip=self.ip, old=self.sw_version)) - log.warning("({ip}) Received: '{new}'".format(ip=self.ip, new=data)) - log.warning("({ip}) Saving new serial number as sw_version_received".format(ip=self.ip)) + log.warning('({ip}) Projector software version does not match saved ' + 'software version'.format(ip=self.ip)) + log.warning('({ip}) Saved: "{old}"'.format(ip=self.ip, old=self.sw_version)) + log.warning('({ip}) Received: "{new}"'.format(ip=self.ip, new=data)) + log.warning('({ip}) Saving new serial number as sw_version_received'.format(ip=self.ip)) self.sw_version_received = data @@ -586,9 +587,9 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): :param poll_time: Time (in seconds) to poll connected projector :param socket_timeout: Time (in seconds) to abort the connection if no response """ - log.debug('PJlink(projector={projector}, args={args} kwargs={kwargs})'.format(projector=projector, - args=args, - kwargs=kwargs)) + log.debug('PJlink(projector="{projector}", args="{args}" kwargs="{kwargs}")'.format(projector=projector, + args=args, + kwargs=kwargs)) super().__init__() self.entry = projector self.ip = self.entry.ip @@ -660,7 +661,6 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.readyRead.disconnect(self.get_socket) # Set in process_pjlink except TypeError: pass - self.disconnect_from_host() self.deleteLater() self.i_am_running = False @@ -679,7 +679,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): Normally called by timer(). """ if self.state() != S_QSOCKET_STATE['ConnectedState']: - log.warning("({ip}) poll_loop(): Not connected - returning".format(ip=self.ip)) + log.warning('({ip}) poll_loop(): Not connected - returning'.format(ip=self.ip)) return log.debug('({ip}) poll_loop(): Updating projector status'.format(ip=self.ip)) # Reset timer in case we were called from a set command @@ -803,12 +803,12 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): # 'PJLink 1 XXXXXX' Authenticated login - extra processing required. if not data.startswith('PJLINK'): # Invalid initial packet - close socket - log.error("({ip}) Invalid initial packet received - closing socket".format(ip=self.ip)) + log.error('({ip}) Invalid initial packet received - closing socket'.format(ip=self.ip)) return self.disconnect_from_host() - log.debug("({ip}) check_login(): Formatting initial connection prompt to PJLink packet".format(ip=self.ip)) - return self.get_data("{start}{clss}{data}".format(start=PJLINK_PREFIX, - clss="1", - data=data.replace(" ", "=", 1)).encode('utf-8')) + log.debug('({ip}) check_login(): Formatting initial connection prompt to PJLink packet'.format(ip=self.ip)) + return self.get_data('{start}{clss}{data}'.format(start=PJLINK_PREFIX, + clss='1', + data=data.replace(' ', '=', 1)).encode('utf-8')) # TODO: The below is replaced by process_pjlink() - remove when working properly """ if '=' in data: @@ -865,13 +865,13 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): """ Clean out extraneous stuff in the buffer. """ - log.warning("({ip}) {message}".format(ip=self.ip, message='Invalid packet' if msg is None else msg)) + log.warning('({ip}) {message}'.format(ip=self.ip, message='Invalid packet' if msg is None else msg)) self.send_busy = False trash_count = 0 while self.bytesAvailable() > 0: trash = self.read(self.max_size) trash_count += len(trash) - log.debug("({ip}) Finished cleaning buffer - {count} bytes dropped".format(ip=self.ip, + log.debug('({ip}) Finished cleaning buffer - {count} bytes dropped'.format(ip=self.ip, count=trash_count)) return @@ -883,7 +883,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): :param data: Data to process. buffer must be formatted as a proper PJLink packet. :param ip: Destination IP for buffer. """ - log.debug("({ip}) get_buffer(data='{buff}' ip='{ip_in}'".format(ip=self.ip, buff=data, ip_in=ip)) + log.debug('({ip}) get_buffer(data="{buff}" ip="{ip_in}"'.format(ip=self.ip, buff=data, ip_in=ip)) if ip is None: log.debug("({ip}) get_buffer() Don't know who data is for - exiting".format(ip=self.ip)) return @@ -901,7 +901,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): return # Although we have a packet length limit, go ahead and use a larger buffer read = self.readLine(1024) - log.debug("({ip}) get_socket(): '{buff}'".format(ip=self.ip, buff=read)) + log.debug('({ip}) get_socket(): "{buff}"'.format(ip=self.ip, buff=read)) if read == -1: # No data available log.debug('({ip}) get_socket(): No data available (-1)'.format(ip=self.ip)) @@ -917,21 +917,24 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): :param buff: Data to process. :param ip: (optional) Destination IP. """ - ip = self.ip if ip is None else ip - log.debug("({ip}) get_data(ip='{ip_in}' buffer='{buff}'".format(ip=self.ip, ip_in=ip, buff=buff)) + # Since "self" is not available to options and the "ip" keyword is a "maybe I'll use in the future", + # set to default here + if ip is None: + ip = self.ip + log.debug('({ip}) get_data(ip="{ip_in}" buffer="{buff}"'.format(ip=self.ip, ip_in=ip, buff=buff)) # NOTE: Class2 has changed to some values being UTF-8 data_in = decode(buff, 'utf-8') data = data_in.strip() # Initial packet checks if (len(data) < 7): - return self._trash_buffer(msg="get_data(): Invalid packet - length") + return self._trash_buffer(msg='get_data(): Invalid packet - length') elif len(data) > self.max_size: - return self._trash_buffer(msg="get_data(): Invalid packet - too long") + return self._trash_buffer(msg='get_data(): Invalid packet - too long') elif not data.startswith(PJLINK_PREFIX): - return self._trash_buffer(msg="get_data(): Invalid packet - PJLink prefix missing") + return self._trash_buffer(msg='get_data(): Invalid packet - PJLink prefix missing') elif '=' not in data: - return self._trash_buffer(msg="get_data(): Invalid packet - Does not have '='") - log.debug("({ip}) get_data(): Checking new data '{data}'".format(ip=self.ip, data=data)) + return self._trash_buffer(msg='get_data(): Invalid reply - Does not have "="') + log.debug('({ip}) get_data(): Checking new data "{data}"'.format(ip=self.ip, data=data)) header, data = data.split('=') # At this point, the header should contain: # "PVCCCC" @@ -1017,15 +1020,15 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): options=opts, suffix=CR) if out in self.priority_queue: - log.debug("({ip}) send_command(): Already in priority queue - skipping".format(ip=self.ip)) + log.debug('({ip}) send_command(): Already in priority queue - skipping'.format(ip=self.ip)) elif out in self.send_queue: - log.debug("({ip}) send_command(): Already in normal queue - skipping".format(ip=self.ip)) + log.debug('({ip}) send_command(): Already in normal queue - skipping'.format(ip=self.ip)) else: if priority: - log.debug("({ip}) send_command(): Adding to priority queue".format(ip=self.ip)) + log.debug('({ip}) send_command(): Adding to priority queue'.format(ip=self.ip)) self.priority_queue.append(out) else: - log.debug("({ip}) send_command(): Adding to normal queue".format(ip=self.ip)) + log.debug('({ip}) send_command(): Adding to normal queue'.format(ip=self.ip)) self.send_queue.append(out) if self.priority_queue or self.send_queue: # May be some initial connection setup so make sure we send data @@ -1040,7 +1043,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): :param utf8: Send as UTF-8 string otherwise send as ASCII string """ # Funny looking data check, but it's a quick check for data=None - log.debug("({ip}) _send_command(data='{data}')".format(ip=self.ip, data=data.strip() if data else data)) + log.debug('({ip}) _send_command(data="{data}")'.format(ip=self.ip, data=data.strip() if data else data)) log.debug('({ip}) _send_command(): Connection status: {data}'.format(ip=self.ip, data=S_QSOCKET_STATE[self.state()])) if self.state() != self.ConnectedState: @@ -1048,19 +1051,19 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.send_busy = False return self.disconnect_from_host() if data and data not in self.priority_queue: - log.debug("({ip}) _send_command(): Priority packet - adding to priority queue".format(ip=self.ip)) + log.debug('({ip}) _send_command(): Priority packet - adding to priority queue'.format(ip=self.ip)) self.priority_queue.append(data) if self.send_busy: # Still waiting for response from last command sent - log.debug("({ip}) _send_command(): Still busy, returning".format(ip=self.ip)) + log.debug('({ip}) _send_command(): Still busy, returning'.format(ip=self.ip)) log.debug('({ip}) _send_command(): Priority queue = {data}'.format(ip=self.ip, data=self.priority_queue)) log.debug('({ip}) _send_command(): Normal queue = {data}'.format(ip=self.ip, data=self.send_queue)) return if len(self.priority_queue) != 0: out = self.priority_queue.pop(0) - log.debug("({ip}) _send_command(): Getting priority queued packet".format(ip=self.ip)) + log.debug('({ip}) _send_command(): Getting priority queued packet'.format(ip=self.ip)) elif len(self.send_queue) != 0: out = self.send_queue.pop(0) log.debug('({ip}) _send_command(): Getting normal queued packet'.format(ip=self.ip)) @@ -1076,7 +1079,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): self.waitForBytesWritten(2000) # 2 seconds should be enough if sent == -1: # Network error? - log.warning("({ip}) _send_command(): -1 received - disconnecting from host".format(ip=self.ip)) + log.warning('({ip}) _send_command(): -1 received - disconnecting from host'.format(ip=self.ip)) self.change_status(E_NETWORK, translate('OpenLP.PJLink', 'Error while sending data to projector')) self.disconnect_from_host() @@ -1085,7 +1088,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): """ Initiate connection to projector. """ - log.debug("{ip}) connect_to_host(): Starting connection".format(ip=self.ip)) + log.debug('{ip}) connect_to_host(): Starting connection'.format(ip=self.ip)) if self.state() == self.ConnectedState: log.warning('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip)) return diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py index a5a6eca40..4ff133061 100644 --- a/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py @@ -185,7 +185,7 @@ class TestPJLinkRouting(TestCase): mock_functions.return_value = [] pjlink = self.pjlink_test - log_text = "(111.111.111.111) Unable to process command='CLSS' (Future option?)" + log_text = '(111.111.111.111) Unable to process command="CLSS" (Future option?)' # WHEN: process_command called with an unknown command pjlink.process_command(cmd='CLSS', data='DONT CARE') @@ -207,7 +207,7 @@ class TestPJLinkRouting(TestCase): # WHEN: process_command called with an unknown command pjlink.process_command(cmd='Unknown', data='Dont Care') - log_text = "(127.0.0.1) Ignoring command='Unknown' (Invalid/Unknown)" + log_text = '(127.0.0.1) Ignoring command="Unknown" (Invalid/Unknown)' # THEN: Error should be logged and no command called self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method') @@ -222,7 +222,7 @@ class TestPJLinkRouting(TestCase): mock_send_command = patch.object(self.pjlink_test, 'send_command').start() pjlink = self.pjlink_test - log_text = "(111.111.111.111) Command 'POWR' returned OK" + log_text = '(111.111.111.111) Command "POWR" returned OK' # WHEN: process_command called with a command that returns OK pjlink.process_command(cmd='POWR', data='OK') diff --git a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py index 0917a9357..8340a0fd0 100644 --- a/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py +++ b/tests/functional/openlp_core/projectors/test_projector_pjlink_commands_01.py @@ -580,7 +580,7 @@ class TestPJLinkCommands(TestCase): # WHEN: Process invalid reply pjlink.process_clss('Z') - log_text = "(127.0.0.1) NAN clss version reply 'Z' - defaulting to class '1'" + log_text = '(127.0.0.1) NAN CLSS version reply "Z" - defaulting to class "1"' # THEN: Projector class should be set with default value self.assertEqual(pjlink.pjlink_class, '1', @@ -597,7 +597,7 @@ class TestPJLinkCommands(TestCase): # WHEN: Process invalid reply pjlink.process_clss('Invalid') - log_text = "(127.0.0.1) No numbers found in class version reply 'Invalid' - defaulting to class '1'" + log_text = '(127.0.0.1) No numbers found in class version reply "Invalid" - defaulting to class "1"' # THEN: Projector class should be set with default value self.assertEqual(pjlink.pjlink_class, '1', @@ -627,7 +627,7 @@ class TestPJLinkCommands(TestCase): # GIVEN: Test object pjlink = pjlink_test pjlink.projector_errors = None - log_text = "127.0.0.1) Invalid error status response '11111111': length != 6" + log_text = '127.0.0.1) Invalid error status response "11111111": length != 6' # WHEN: process_erst called with invalid data (too many values pjlink.process_erst('11111111') @@ -645,7 +645,7 @@ class TestPJLinkCommands(TestCase): # GIVEN: Test object pjlink = pjlink_test pjlink.projector_errors = None - log_text = "(127.0.0.1) Invalid error status response '1111Z1'" + log_text = '(127.0.0.1) Invalid error status response "1111Z1"' # WHEN: process_erst called with invalid data (too many values pjlink.process_erst('1111Z1') @@ -671,8 +671,8 @@ class TestPJLinkCommands(TestCase): # THEN: PJLink instance errors should match chk_value for chk in pjlink.projector_errors: self.assertEqual(pjlink.projector_errors[chk], chk_string, - "projector_errors['{chk}'] should have been set to {err}".format(chk=chk, - err=chk_string)) + 'projector_errors["{chk}"] should have been set to "{err}"'.format(chk=chk, + err=chk_string)) def test_projector_process_erst_all_error(self): """ @@ -690,8 +690,8 @@ class TestPJLinkCommands(TestCase): # THEN: PJLink instance errors should match chk_value for chk in pjlink.projector_errors: self.assertEqual(pjlink.projector_errors[chk], chk_string, - "projector_errors['{chk}'] should have been set to {err}".format(chk=chk, - err=chk_string)) + 'projector_errors["{chk}"] should have been set to "{err}"'.format(chk=chk, + err=chk_string)) def test_projector_process_erst_warn_cover_only(self): """ @@ -744,9 +744,9 @@ class TestPJLinkCommands(TestCase): pjlink = pjlink_test pjlink.source_available = [] test_data = '21 10 30 31 11 20' - test_saved = ['10', '11', '20', '21', '30', '31'] - log_data = '(127.0.0.1) Setting projector sources_available to ' \ - '"[\'10\', \'11\', \'20\', \'21\', \'30\', \'31\']"' + test_saved = ["10", "11", "20", "21", "30", "31"] + log_data = "(127.0.0.1) Setting projector sources_available to " \ + "\"['10', '11', '20', '21', '30', '31']\"" mock_UpdateIcons.reset_mock() mock_log.reset_mock() @@ -1021,7 +1021,7 @@ class TestPJLinkCommands(TestCase): pjlink.sw_version = None pjlink.sw_version_received = None test_data = 'Test 1 Subtest 1' - test_log = "(127.0.0.1) Setting projector software version to 'Test 1 Subtest 1'" + test_log = '(127.0.0.1) Setting projector software version to "Test 1 Subtest 1"' mock_log.reset_mock() # WHEN: process_sver called with invalid data