Merge lp:~alisonken1/openlp/pjlink2-s into lp:openlp

[Ken Roberts]

PJLink2 update S

- Convert action.remove raise ValueError on missing action to log.warning
- Added 2-second status timer to update icons during projector state change
- Update set power methods to use status timer
- Update set shutter methods to use status timer
- Fix process_avmt to include missing valid status from projector
- Update projector info menu to include version 2 information
- Add E_SOCKET_TIMEOUT to STATUS_ICONS dictionary
- Change 'Action "{action}" does not exist' from raise ValueError to log.warning
- Update projector to include PJLink2 extras
- Fix version check exception if server returns status code != 200
- Fix version tests for status code
- Fix actions tests for remove action not in list
- Fix projector tests for changed calls
- Update setup.cfg to inlude pycodestyle (pep8 deprecaded)
- PEP8 fix for scripts/lp-merge.txt
- Updated fix for version check exception

--------------------------------------------------------------------------------
lp:~alisonken1/openlp/pjlink2-s (revision 2822)
https://ci.openlp.io/job/Branch-01-Pull/2538/ [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2436/ [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/225/ [FAILURE]
https://ci.openlp.io/job/Branch-03a-Build-Source/127/ [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/120/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1589/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1402/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/323/ [FAILURE]

[Tim Bentley (trb143)]

The fix for version is not correct see inline notes.

review: Needs Fixing

[Tim Bentley (trb143)]

review: Approve

[Tim Bentley (trb143)]

review: Approve

[Tomas Groth (tomasgroth)]

Looks good to me

review: Approve

=== modified file 'openlp/core/common/actions.py'
--- openlp/core/common/actions.py	2017-12-29 09:15:48 +0000
+++ openlp/core/common/actions.py	2018-06-28 17:08:08 +0000
@@ -113,7 +113,7 @@
             if item[1] == action:
                 self.actions.remove(item)
                 return
-        raise ValueError('Action "{action}" does not exist.'.format(action=action))
+        log.warning('Action "{action}" does not exist.'.format(action=action))
 
 
 class CategoryList(object):

=== modified file 'openlp/core/projectors/manager.py'
--- openlp/core/projectors/manager.py	2018-05-19 00:48:33 +0000
+++ openlp/core/projectors/manager.py	2018-06-28 17:08:08 +0000
@@ -36,8 +36,8 @@
 from openlp.core.lib.ui import create_widget_action
 from openlp.core.projectors import DialogSourceStyle
 from openlp.core.projectors.constants import E_AUTHENTICATION, E_ERROR, E_NETWORK, E_NOT_CONNECTED, \
-    E_UNKNOWN_SOCKET_ERROR, S_CONNECTED, S_CONNECTING, S_COOLDOWN, S_INITIALIZE, S_NOT_CONNECTED, S_OFF, S_ON, \
-    S_STANDBY, S_WARMUP, PJLINK_PORT, STATUS_CODE, STATUS_MSG, QSOCKET_STATE
+    E_SOCKET_TIMEOUT, E_UNKNOWN_SOCKET_ERROR, S_CONNECTED, S_CONNECTING, S_COOLDOWN, S_INITIALIZE, \
+    S_NOT_CONNECTED, S_OFF, S_ON, S_STANDBY, S_WARMUP, PJLINK_PORT, STATUS_CODE, STATUS_MSG, QSOCKET_STATE
 
 from openlp.core.projectors.db import ProjectorDB
 from openlp.core.projectors.editform import ProjectorEditForm
@@ -62,6 +62,7 @@
     S_COOLDOWN: ':/projector/projector_cooldown.png',
     E_ERROR: ':/projector/projector_error.png',
     E_NETWORK: ':/projector/projector_not_connected_error.png',
+    E_SOCKET_TIMEOUT: ':/projector/projector_not_connected_error.png',
     E_AUTHENTICATION: ':/projector/projector_not_connected_error.png',
     E_UNKNOWN_SOCKET_ERROR: ':/projector/projector_not_connected_error.png',
     E_NOT_CONNECTED: ':/projector/projector_not_connected_error.png'
@@ -360,22 +361,14 @@
         self.connect_action.setVisible(not visible)
         self.disconnect_action.setVisible(visible)
         self.status_action.setVisible(visible)
-        if visible:
-            self.select_input_action.setVisible(real_projector.link.power == S_ON)
-            self.edit_input_action.setVisible(real_projector.link.power == S_ON)
-            self.poweron_action.setVisible(real_projector.link.power == S_STANDBY)
-            self.poweroff_action.setVisible(real_projector.link.power == S_ON)
-            self.blank_action.setVisible(real_projector.link.power == S_ON and
-                                         not real_projector.link.shutter)
-            self.show_action.setVisible(real_projector.link.power == S_ON and
-                                        real_projector.link.shutter)
-        else:
-            self.select_input_action.setVisible(False)
-            self.edit_input_action.setVisible(False)
-            self.poweron_action.setVisible(False)
-            self.poweroff_action.setVisible(False)
-            self.blank_action.setVisible(False)
-            self.show_action.setVisible(False)
+        self.select_input_action.setVisible(visible and real_projector.link.power == S_ON)
+        self.edit_input_action.setVisible(visible and real_projector.link.power == S_ON)
+        self.poweron_action.setVisible(visible and real_projector.link.power == S_STANDBY)
+        self.poweroff_action.setVisible(visible and real_projector.link.power == S_ON)
+        self.blank_action.setVisible(visible and real_projector.link.power == S_ON and
+                                     not real_projector.link.shutter)
+        self.show_action.setVisible(visible and real_projector.link.power == S_ON and
+                                    real_projector.link.shutter)
         self.menu.projector = real_projector
         self.menu.exec(self.projector_list_widget.mapToGlobal(point))
 
@@ -516,11 +509,10 @@
         except (AttributeError, TypeError):
             pass
         # Disconnect signals from projector being deleted
-        if self.pjlink_udp[projector.port]:
-            try:
-                self.pjlink_udp[projector.port].data_received.disconnect(projector.get_buffer)
-            except (AttributeError, TypeError):
-                pass
+        try:
+            self.pjlink_udp[projector.link.port].data_received.disconnect(projector.link.get_buffer)
+        except (AttributeError, TypeError):
+            pass
 
         # Rebuild projector list
         new_list = []
@@ -650,6 +642,21 @@
                                                              data=projector.link.manufacturer)
             message += '<b>{title}</b>: {data}<br />'.format(title=translate('OpenLP.ProjectorManager', 'Model'),
                                                              data=projector.link.model)
+            message += '<b>{title}</b>: {data}<br />'.format(title=translate('OpenLP.ProjectorManager', 'PJLink Class'),
+                                                             data=projector.link.pjlink_class)
+            if projector.link.pjlink_class != 1:
+                message += '<b>{title}</b>: {data}<br />'.format(title=translate('OpenLP.ProjectorManager',
+                                                                                 'Software Version'),
+                                                                 data=projector.link.sw_version)
+                message += '<b>{title}</b>: {data}<br />'.format(title=translate('OpenLP.ProjectorManager',
+                                                                                 'Serial Number'),
+                                                                 data=projector.link.serial_no)
+                message += '<b>{title}</b>: {data}<br />'.format(title=translate('OpenLP.ProjectorManager',
+                                                                                 'Lamp Model Number'),
+                                                                 data=projector.link.model_lamp)
+                message += '<b>{title}</b>: {data}<br />'.format(title=translate('OpenLP.ProjectorManager',
+                                                                                 'Filter Model Number'),
+                                                                 data=projector.link.model_filter)
             message += '<b>{title}</b>: {data}<br /><br />'.format(title=translate('OpenLP.ProjectorManager',
                                                                                    'Other info'),
                                                                    data=projector.link.other_info)
@@ -663,20 +670,6 @@
                                                                       source=translate('OpenLP.ProjectorManager',
                                                                                        'Current source input is'),
                                                                       selected=projector.link.source)
-            if projector.link.pjlink_class == '2':
-                # Information only available for PJLink Class 2 projectors
-                message += '<b>{title}</b>: {data}<br /><br />'.format(title=translate('OpenLP.ProjectorManager',
-                                                                                       'Serial Number'),
-                                                                       data=projector.serial_no)
-                message += '<b>{title}</b>: {data}<br /><br />'.format(title=translate('OpenLP.ProjectorManager',
-                                                                                       'Software Version'),
-                                                                       data=projector.sw_version)
-                message += '<b>{title}</b>: {data}<br /><br />'.format(title=translate('OpenLP.ProjectorManager',
-                                                                                       'Lamp type'),
-                                                                       data=projector.model_lamp)
-                message += '<b>{title}</b>: {data}<br /><br />'.format(title=translate('OpenLP.ProjectorManager',
-                                                                                       'Filter type'),
-                                                                       data=projector.model_filter)
             count = 1
             for item in projector.link.lamp:
                 if item['On'] is None:
@@ -957,6 +950,10 @@
         self.poll_time = None
         self.socket_timeout = None
         self.status = S_NOT_CONNECTED
+        self.serial_no = None
+        self.sw_version = None
+        self.model_filter = None
+        self.model_lamp = None
         super().__init__()
 
 

=== modified file 'openlp/core/projectors/pjlink.py'
--- openlp/core/projectors/pjlink.py	2018-05-19 00:48:33 +0000
+++ openlp/core/projectors/pjlink.py	2018-06-28 17:08:08 +0000
@@ -59,7 +59,7 @@
     PJLINK_ERST_DATA, PJLINK_ERST_STATUS, PJLINK_MAX_PACKET, PJLINK_PREFIX, PJLINK_PORT, PJLINK_POWR_STATUS, \
     PJLINK_SUFFIX, PJLINK_VALID_CMD, PROJECTOR_STATE, STATUS_CODE, STATUS_MSG, QSOCKET_STATE, \
     E_AUTHENTICATION, E_CONNECTION_REFUSED, E_GENERAL, E_NETWORK, E_NOT_CONNECTED, E_SOCKET_TIMEOUT, \
-    S_CONNECTED, S_CONNECTING, S_NOT_CONNECTED, S_OFF, S_OK, S_ON
+    S_CONNECTED, S_CONNECTING, S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_STANDBY
 
 log = logging.getLogger(__name__)
 log.debug('pjlink loaded')
@@ -233,6 +233,10 @@
         if hasattr(self, 'socket_timer'):
             log.debug('({ip}): Calling socket_timer.stop()'.format(ip=self.entry.name))
             self.socket_timer.stop()
+        if hasattr(self, 'status_timer'):
+            log.debug('({ip}): Calling status_timer.stop()'.format(ip=self.entry.name))
+            self.status_timer.stop()
+        self.status_timer_checks = {}
         self.send_busy = False
         self.send_queue = []
         self.priority_queue = []
@@ -287,14 +291,18 @@
         """
         Process shutter and speaker status. See PJLink specification for format.
         Update self.mute (audio) and self.shutter (video shutter).
+        10 = Shutter open, audio unchanged
         11 = Shutter closed, audio unchanged
+        20 = Shutter unchanged, Audio normal
         21 = Shutter unchanged, Audio muted
-        30 = Shutter closed, audio muted
-        31 = Shutter open,  audio normal
+        30 = Shutter open, audio muted
+        31 = Shutter closed,  audio normal
 
         :param data: Shutter and audio status
         """
-        settings = {'11': {'shutter': True, 'mute': self.mute},
+        settings = {'10': {'shutter': False, 'mute': self.mute},
+                    '11': {'shutter': True, 'mute': self.mute},
+                    '20': {'shutter': self.shutter, 'mute': False},
                     '21': {'shutter': self.shutter, 'mute': True},
                     '30': {'shutter': False, 'mute': False},
                     '31': {'shutter': True, 'mute': True}
@@ -309,6 +317,8 @@
         self.shutter = shutter
         self.mute = mute
         if update_icons:
+            if 'AVMT' in self.status_timer_checks:
+                self.status_timer_delete('AVMT')
             self.projectorUpdateIcons.emit()
         return
 
@@ -592,6 +602,8 @@
         else:
             # Log unknown status response
             log.warning('({ip}) Unknown power response: "{data}"'.format(ip=self.entry.name, data=data))
+        if self.power in [S_ON, S_STANDBY, S_OFF] and 'POWR' in self.status_timer_checks:
+            self.status_timer_delete(cmd='POWR')
         return
 
     def process_rfil(self, data):
@@ -734,6 +746,11 @@
         self.socket_timer = QtCore.QTimer(self)
         self.socket_timer.setInterval(self.socket_timeout)
         self.socket_timer.timeout.connect(self.socket_abort)
+        # Timer for doing status updates for commands that change state and should update faster
+        self.status_timer_checks = {}  # Keep track of events for the status timer
+        self.status_timer = QtCore.QTimer(self)
+        self.status_timer.setInterval(2000)  # 2 second interval should be fast enough
+        self.status_timer.timeout.connect(self.status_timer_update)
         # Socket status signals
         self.connected.connect(self.check_login)
         self.disconnected.connect(self.disconnect_from_host)
@@ -1191,12 +1208,12 @@
             self.change_status(S_NOT_CONNECTED)
         self.reset_information()
 
-    def get_av_mute_status(self):
+    def get_av_mute_status(self, priority=False):
         """
         Send command to retrieve shutter status.
         """
         log.debug('({ip}) Sending AVMT command'.format(ip=self.entry.name))
-        return self.send_command(cmd='AVMT')
+        return self.send_command(cmd='AVMT', priority=priority)
 
     def get_available_inputs(self):
         """
@@ -1254,12 +1271,14 @@
         log.debug('({ip}) Sending INFO command'.format(ip=self.entry.name))
         return self.send_command(cmd='INFO')
 
-    def get_power_status(self):
+    def get_power_status(self, priority=False):
         """
         Send command to retrieve power status.
+
+        :param priority: (OPTIONAL) Send in priority queue
         """
         log.debug('({ip}) Sending POWR command'.format(ip=self.entry.name))
-        return self.send_command(cmd='POWR')
+        return self.send_command(cmd='POWR', priority=priority)
 
     def set_input_source(self, src=None):
         """
@@ -1283,6 +1302,7 @@
         """
         log.debug('({ip}) Setting POWR to 1 (on)'.format(ip=self.entry.name))
         self.send_command(cmd='POWR', opts='1', priority=True)
+        self.status_timer_add(cmd='POWR', callback=self.get_power_status)
         self.poll_loop()
 
     def set_power_off(self):
@@ -1291,6 +1311,7 @@
         """
         log.debug('({ip}) Setting POWR to 0 (standby)'.format(ip=self.entry.name))
         self.send_command(cmd='POWR', opts='0', priority=True)
+        self.status_timer_add(cmd='POWR', callback=self.get_power_status)
         self.poll_loop()
 
     def set_shutter_closed(self):
@@ -1299,6 +1320,7 @@
         """
         log.debug('({ip}) Setting AVMT to 11 (shutter closed)'.format(ip=self.entry.name))
         self.send_command(cmd='AVMT', opts='11', priority=True)
+        self.status_timer_add('AVMT', self.get_av_mute_status)
         self.poll_loop()
 
     def set_shutter_open(self):
@@ -1307,8 +1329,51 @@
         """
         log.debug('({ip}) Setting AVMT to "10" (shutter open)'.format(ip=self.entry.name))
         self.send_command(cmd='AVMT', opts='10', priority=True)
+        self.status_timer_add('AVMT', self.get_av_mute_status)
         self.poll_loop()
-        self.projectorUpdateIcons.emit()
+
+    def status_timer_add(self, cmd, callback):
+        """
+        Add a callback to the status timer.
+
+        :param cmd: PJLink command associated with callback
+        :param callback: Method to call
+        """
+        if cmd in self.status_timer_checks:
+            log.warning('({ip}) "{cmd}" already in checks - returning'.format(ip=self.entry.name, cmd=cmd))
+            return
+        log.debug('({ip}) Adding "{cmd}" callback for status timer'.format(ip=self.entry.name, cmd=cmd))
+        if not self.status_timer.isActive():
+            self.status_timer.start()
+        self.status_timer_checks[cmd] = callback
+
+    def status_timer_delete(self, cmd):
+        """
+        Delete a callback from the status timer.
+
+        :param cmd: PJLink command associated with callback
+        :param callback: Method to call
+        """
+        if cmd not in self.status_timer_checks:
+            log.warning('({ip}) "{cmd}" not listed in status timer - returning'.format(ip=self.entry.name, cmd=cmd))
+            return
+        log.debug('({ip}) Removing "{cmd}" from status timer'.format(ip=self.entry.name, cmd=cmd))
+        self.status_timer_checks.pop(cmd)
+        if not self.status_timer_checks:
+            self.status_timer.stop()
+
+    def status_timer_update(self):
+        """
+        Call methods defined in status_timer_checks for updates
+        """
+        if not self.status_timer_checks:
+            log.warning('({ip}) status_timer_update() called when no callbacks - '
+                        'Race condition?'.format(ip=self.entry.name))
+            self.status_timer.stop()
+            return
+        for cmd, callback in self.status_timer_checks.items():
+            log.debug('({ip}) Status update call for {cmd}'.format(ip=self.entry.name, cmd=cmd))
+            callback(priority=True)
 
     def receive_data_signal(self):
         """

=== modified file 'openlp/core/version.py'
--- openlp/core/version.py	2018-03-29 15:54:55 +0000
+++ openlp/core/version.py	2018-06-28 17:08:08 +0000
@@ -89,7 +89,8 @@
         while retries < 3:
             try:
                 response = requests.get(download_url, headers=headers)
-                remote_version = response.text.strip()
+                if response.status_code == 200:
+                    remote_version = response.text.strip()
                 log.debug('New version found: %s', remote_version)
                 break
             except OSError:

=== modified file 'scripts/lp-merge.py'
--- scripts/lp-merge.py	2018-05-04 21:14:04 +0000
+++ scripts/lp-merge.py	2018-06-28 17:08:08 +0000
@@ -103,8 +103,8 @@
     merge_info['branch_url'] = span_branch_url.contents[0]
     # Find the p tag that contains the commit message
     # <div id="commit-message">...<div id="edit-commit_message">...<div class="yui3-editable_text-text"><p>
-    commit_message = soup.find('div', id='commit-message').find('div', id='edit-commit_message')\
-            .find('div', 'yui3-editable_text-text').p
+    commit_message = soup.find('div', id='commit-message').find('div', id='edit-commit_message') \
+        .find('div', 'yui3-editable_text-text').p
     merge_info['commit_message'] = commit_message.string
     # Find all tr-tags with this class. Makes it possible to get bug numbers.
     # <tr class="bug-branch-summary"

=== modified file 'setup.cfg'
--- setup.cfg	2017-07-07 23:43:50 +0000
+++ setup.cfg	2018-06-28 17:08:08 +0000
@@ -12,3 +12,8 @@
 max-line-length = 120
 ignore = E402
 
+[pycodestyle]
+exclude = resources.py,vlc.py
+max-line-length = 120
+ignore = E402
+

=== modified file 'tests/functional/openlp_core/common/test_actions.py'
--- tests/functional/openlp_core/common/test_actions.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/common/test_actions.py	2018-06-28 17:08:08 +0000
@@ -23,10 +23,11 @@
 Package to test the openlp.core.common.actions package.
 """
 from unittest import TestCase
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, call, patch
 
 from PyQt5 import QtGui, QtCore, QtWidgets
 
+import openlp.core.common.actions
 from openlp.core.common.actions import CategoryActionList, ActionList
 from openlp.core.common.settings import Settings
 from tests.helpers.testmixin import TestMixin
@@ -139,8 +140,21 @@
         # THEN: Now the element should not be in the list anymore.
         assert self.action1 not in self.list
 
-        # THEN: Check if an exception is raised when trying to remove a not present action.
-        self.assertRaises(ValueError, self.list.remove, self.action2)
+    def test_remove_not_in_list(self):
+        """
+        Test the remove() method when action not in list
+        """
+        with patch.object(openlp.core.common.actions, 'log') as mock_log:
+            log_warn_calls = [call('Action "" does not exist.')]
+
+            # GIVEN: The list
+            self.list.append(self.action1)
+
+            # WHEN: Delete an item not in the list.
+            self.list.remove('')
+
+            # THEN: Warning should be logged
+            mock_log.warning.assert_has_calls(log_warn_calls)
 
 
 class TestActionList(TestCase, TestMixin):

=== modified file 'tests/functional/openlp_core/test_version.py'
--- tests/functional/openlp_core/test_version.py	2018-01-02 21:00:54 +0000
+++ tests/functional/openlp_core/test_version.py	2018-06-28 17:08:08 +0000
@@ -54,7 +54,7 @@
     current_version = {'full': '2.0', 'version': '2.0', 'build': None}
     mock_platform.system.return_value = 'Linux'
     mock_platform.release.return_value = '4.12.0-1-amd64'
-    mock_requests.get.return_value = MagicMock(text='2.4.6')
+    mock_requests.get.return_value = MagicMock(text='2.4.6', status_code=200)
     worker = VersionWorker(last_check_date, current_version)
 
     # WHEN: The worker is run
@@ -79,7 +79,7 @@
     current_version = {'full': '2.1.3', 'version': '2.1.3', 'build': None}
     mock_platform.system.return_value = 'Linux'
     mock_platform.release.return_value = '4.12.0-1-amd64'
-    mock_requests.get.return_value = MagicMock(text='2.4.6')
+    mock_requests.get.return_value = MagicMock(text='2.4.6', status_code=200)
     worker = VersionWorker(last_check_date, current_version)
 
     # WHEN: The worker is run
@@ -104,7 +104,7 @@
     current_version = {'full': '2.1-bzr2345', 'version': '2.1', 'build': '2345'}
     mock_platform.system.return_value = 'Linux'
     mock_platform.release.return_value = '4.12.0-1-amd64'
-    mock_requests.get.return_value = MagicMock(text='2.4.6')
+    mock_requests.get.return_value = MagicMock(text='2.4.6', status_code=200)
     worker = VersionWorker(last_check_date, current_version)
 
     # WHEN: The worker is run

=== modified file 'tests/openlp_core/projectors/test_projector_pjlink_base_01.py'
--- tests/openlp_core/projectors/test_projector_pjlink_base_01.py	2018-05-03 14:58:50 +0000
+++ tests/openlp_core/projectors/test_projector_pjlink_base_01.py	2018-06-28 17:08:08 +0000
@@ -290,7 +290,7 @@
 
             # THEN: log data and send_command should have been called
             mock_log.debug.assert_has_calls(log_debug_calls)
-            mock_send_command.assert_called_once_with(cmd=test_data)
+            mock_send_command.assert_called_once_with(cmd=test_data, priority=False)
 
     def test_projector_get_available_inputs(self):
         """
@@ -470,7 +470,7 @@
 
             # THEN: log data and send_command should have been called
             mock_log.debug.assert_has_calls(log_debug_calls)
-            mock_send_command.assert_called_once_with(cmd=test_data)
+            mock_send_command.assert_called_once_with(cmd=test_data, priority=False)
 
     def test_projector_get_status_invalid(self):
         """