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

[Ken Roberts]

PJLink 2 update H

- Restructured AVMT to shortcut return on invalid input
- Added AVMT bad data test
- Fix AVMT tests
- Added extra logging information for CLSS errors
- Added CLSS failure tests
- Restructure ERST to not use hard-coded error breakout
- Added several ERST tests
- Fix ERST tests
- Added tests for pjlink.process_command

--------------------------------
lp:~alisonken1/openlp/pjlink2-h (revision 2757)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2130/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/2037/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1941/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1318/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1158/
[SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/288/
[FAILURE] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/133/

[Tim Bentley (trb143)]

review: Approve

[Tomas Groth (tomasgroth)]

review: Approve

=== modified file 'openlp/core/lib/projector/constants.py'
--- openlp/core/lib/projector/constants.py	2017-08-06 07:23:26 +0000
+++ openlp/core/lib/projector/constants.py	2017-08-11 11:14:30 +0000
@@ -46,7 +46,7 @@
            'S_NOT_CONNECTED', 'S_CONNECTING', 'S_CONNECTED',
            'S_STATUS', 'S_OFF', 'S_INITIALIZE', 'S_STANDBY', 'S_WARMUP', 'S_ON', 'S_COOLDOWN',
            'S_INFO', 'S_NETWORK_SENDING', 'S_NETWORK_RECEIVED',
-           'ERROR_STRING', 'CR', 'LF', 'PJLINK_ERST_STATUS', 'PJLINK_POWR_STATUS',
+           'ERROR_STRING', 'CR', 'LF', 'PJLINK_ERST_DATA', 'PJLINK_ERST_STATUS', 'PJLINK_POWR_STATUS',
            'PJLINK_PORT', 'PJLINK_MAX_PACKET', 'TIMEOUT', 'ERROR_MSG', 'PJLINK_ERRORS',
            'STATUS_STRING', 'PJLINK_VALID_CMD', 'CONNECTION_ERRORS',
            'PJLINK_DEFAULT_SOURCES', 'PJLINK_DEFAULT_CODES', 'PJLINK_DEFAULT_ITEMS']
@@ -393,11 +393,32 @@
     S_NETWORK_RECEIVED: translate('OpenLP.ProjectorConstants', 'Received data')
 }
 
+# Map ERST return code positions to equipment
+PJLINK_ERST_DATA = {
+    'DATA_LENGTH': 6,
+    0: 'FAN',
+    1: 'LAMP',
+    2: 'TEMP',
+    3: 'COVER',
+    4: 'FILTER',
+    5: 'OTHER',
+    'FAN': 0,
+    'LAMP': 1,
+    'TEMP': 2,
+    'COVER': 3,
+    'FILTER': 4,
+    'OTHER': 5
+}
+
 # Map for ERST return codes to string
 PJLINK_ERST_STATUS = {
-    '0': ERROR_STRING[E_OK],
+    '0': 'OK',
     '1': ERROR_STRING[E_WARN],
-    '2': ERROR_STRING[E_ERROR]
+    '2': ERROR_STRING[E_ERROR],
+    'OK': '0',
+    E_OK: '0',
+    E_WARN: '1',
+    E_ERROR: '2'
 }
 
 # Map for POWR return codes to status code

=== modified file 'openlp/core/lib/projector/pjlink.py'
--- openlp/core/lib/projector/pjlink.py	2017-08-06 23:33:53 +0000
+++ openlp/core/lib/projector/pjlink.py	2017-08-11 11:14:30 +0000
@@ -54,8 +54,8 @@
 
 from openlp.core.common import translate, qmd5_hash
 from openlp.core.lib.projector.constants import CONNECTION_ERRORS, CR, ERROR_MSG, ERROR_STRING, \
-    E_AUTHENTICATION, E_CONNECTION_REFUSED, E_GENERAL, E_INVALID_DATA, E_NETWORK, E_NOT_CONNECTED, \
-    E_PARAMETER, E_PROJECTOR, E_SOCKET_TIMEOUT, E_UNAVAILABLE, E_UNDEFINED, PJLINK_ERRORS, \
+    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_NETWORK_RECEIVED, S_NETWORK_SENDING, \
     S_NOT_CONNECTED, S_OFF, S_OK, S_ON, S_STATUS
@@ -154,39 +154,37 @@
         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.warn("({ip}) Unable to process command='{cmd}' (Future option)".format(ip=self.ip, cmd=cmd))
             return
         elif _data in PJLINK_ERRORS:
             # Oops - projector error
             log.error('({ip}) Projector returned error "{data}"'.format(ip=self.ip, data=data))
-            if _data == 'ERRA':
+            if _data == PJLINK_ERRORS[E_AUTHENTICATION]:
                 # Authentication error
                 self.disconnect_from_host()
                 self.change_status(E_AUTHENTICATION)
                 log.debug('({ip}) emitting projectorAuthentication() signal'.format(ip=self.ip))
                 self.projectorAuthentication.emit(self.name)
-            elif _data == 'ERR1':
-                # Undefined command
+            elif _data == PJLINK_ERRORS[E_UNDEFINED]:
+                # Projector does not recognize command
                 self.change_status(E_UNDEFINED, '{error}: "{data}"'.format(error=ERROR_MSG[E_UNDEFINED],
                                                                            data=cmd))
-            elif _data == 'ERR2':
+            elif _data == PJLINK_ERRORS[E_PARAMETER]:
                 # Invalid parameter
                 self.change_status(E_PARAMETER)
-            elif _data == 'ERR3':
+            elif _data == PJLINK_ERRORS[E_UNAVAILABLE]:
                 # Projector busy
                 self.change_status(E_UNAVAILABLE)
-            elif _data == 'ERR4':
+            elif _data == PJLINK_ERRORS[E_PROJECTOR]:
                 # Projector/display error
                 self.change_status(E_PROJECTOR)
             self.receive_data_signal()
             return
-        # Command succeeded - no extra information
-        elif _data == 'OK':
-            log.debug('({ip}) Command returned OK'.format(ip=self.ip))
-            # A command returned successfully
-            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()
@@ -196,6 +194,10 @@
         """
         Process shutter and speaker status. See PJLink specification for format.
         Update self.mute (audio) and self.shutter (video shutter).
+        11 = Shutter closed, audio unchanged
+        21 = Shutter unchanged, Audio muted
+        30 = Shutter closed, audio muted
+        31 = Shutter open,  audio normal
 
         :param data: Shutter and audio status
         """
@@ -204,15 +206,15 @@
                     '30': {'shutter': False, 'mute': False},
                     '31': {'shutter': True, 'mute': True}
                     }
-        if data in settings:
-            shutter = settings[data]['shutter']
-            mute = settings[data]['mute']
-            # Check if we need to update the icons
-            update_icons = (shutter != self.shutter) or (mute != self.mute)
-            self.shutter = shutter
-            self.mute = mute
-        else:
-            log.warning('({ip}) Unknown shutter response: {data}'.format(ip=self.ip, data=data))
+        if data not in settings:
+            log.warning('({ip}) Invalid shutter response: {data}'.format(ip=self.ip, data=data))
+            return
+        shutter = settings[data]['shutter']
+        mute = settings[data]['mute']
+        # Check if we need to update the icons
+        update_icons = (shutter != self.shutter) or (mute != self.mute)
+        self.shutter = shutter
+        self.mute = mute
         if update_icons:
             self.projectorUpdateIcons.emit()
         return
@@ -236,10 +238,12 @@
             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 - defaulting to class '1'".format(ip=self.ip))
+                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 class version reply - defaulting to class '1'".format(ip=self.ip))
+            log.error("({ip}) NAN clss version reply '{data}' - "
+                      "defaulting to class '1'".format(ip=self.ip, data=data))
             clss = '1'
         else:
             clss = data
@@ -253,41 +257,50 @@
         Error status. See PJLink Specifications for format.
         Updates self.projector_errors
 
-\        :param data: Error status
+        :param data: Error status
         """
+        if len(data) != PJLINK_ERST_DATA['DATA_LENGTH']:
+            count = PJLINK_ERST_DATA['DATA_LENGTH']
+            log.warn("{ip}) Invalid error status response '{data}': length != {count}".format(ip=self.ip,
+                                                                                              data=data,
+                                                                                              count=count))
+            return
         try:
             datacheck = int(data)
         except ValueError:
             # Bad data - ignore
+            log.warn("({ip}) Invalid error status response '{data}'".format(ip=self.ip, data=data))
             return
         if datacheck == 0:
             self.projector_errors = None
-        else:
-            self.projector_errors = {}
-            # Fan
-            if data[0] != '0':
-                self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Fan')] = \
-                    PJLINK_ERST_STATUS[data[0]]
-            # Lamp
-            if data[1] != '0':
-                self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Lamp')] =  \
-                    PJLINK_ERST_STATUS[data[1]]
-            # Temp
-            if data[2] != '0':
-                self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Temperature')] =  \
-                    PJLINK_ERST_STATUS[data[2]]
-            # Cover
-            if data[3] != '0':
-                self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Cover')] =  \
-                    PJLINK_ERST_STATUS[data[3]]
-            # Filter
-            if data[4] != '0':
-                self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Filter')] =  \
-                    PJLINK_ERST_STATUS[data[4]]
-            # Other
-            if data[5] != '0':
-                self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Other')] =  \
-                    PJLINK_ERST_STATUS[data[5]]
+            # No errors
+            return
+        # We have some sort of status error, so check out what it/they are
+        self.projector_errors = {}
+        fan, lamp, temp, cover, filt, other = (data[PJLINK_ERST_DATA['FAN']],
+                                               data[PJLINK_ERST_DATA['LAMP']],
+                                               data[PJLINK_ERST_DATA['TEMP']],
+                                               data[PJLINK_ERST_DATA['COVER']],
+                                               data[PJLINK_ERST_DATA['FILTER']],
+                                               data[PJLINK_ERST_DATA['OTHER']])
+        if fan != PJLINK_ERST_STATUS[E_OK]:
+            self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Fan')] = \
+                PJLINK_ERST_STATUS[fan]
+        if lamp != PJLINK_ERST_STATUS[E_OK]:
+            self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Lamp')] =  \
+                PJLINK_ERST_STATUS[lamp]
+        if temp != PJLINK_ERST_STATUS[E_OK]:
+            self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Temperature')] =  \
+                PJLINK_ERST_STATUS[temp]
+        if cover != PJLINK_ERST_STATUS[E_OK]:
+            self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Cover')] =  \
+                PJLINK_ERST_STATUS[cover]
+        if filt != PJLINK_ERST_STATUS[E_OK]:
+            self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Filter')] =  \
+                PJLINK_ERST_STATUS[filt]
+        if other != PJLINK_ERST_STATUS[E_OK]:
+            self.projector_errors[translate('OpenLP.ProjectorPJLink', 'Other')] =  \
+                PJLINK_ERST_STATUS[other]
         return
 
     def process_inf1(self, data):

=== added file 'tests/functional/openlp_core_lib/test_projector_pjlink_cmd_routing.py'
--- tests/functional/openlp_core_lib/test_projector_pjlink_cmd_routing.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_core_lib/test_projector_pjlink_cmd_routing.py	2017-08-11 11:14:30 +0000
@@ -0,0 +1,222 @@
+# -*- 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.lib.projector.pjlink class command routing.
+"""
+
+from unittest import TestCase
+from unittest.mock import patch, MagicMock
+
+import openlp.core.lib.projector.pjlink
+from openlp.core.lib.projector.pjlink import PJLink
+from openlp.core.lib.projector.constants import PJLINK_ERRORS, \
+    E_AUTHENTICATION, E_PARAMETER, E_PROJECTOR, E_UNAVAILABLE, E_UNDEFINED
+
+'''
+from openlp.core.lib.projector.constants import ERROR_STRING, PJLINK_ERST_DATA, PJLINK_ERST_STATUS, \
+    PJLINK_POWR_STATUS, PJLINK_VALID_CMD, E_WARN, E_ERROR, S_OFF, S_STANDBY, S_ON
+'''
+from tests.resources.projector.data import TEST_PIN
+
+pjlink_test = PJLink(name='test', ip='127.0.0.1', pin=TEST_PIN, no_poll=True)
+
+
+class TestPJLinkRouting(TestCase):
+    """
+    Tests for the PJLink module command routing
+    """
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_call_clss(self, mock_log):
+        """
+        Test process_command calls proper function
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        log_text = '(127.0.0.1) Calling function for CLSS'
+        mock_log.reset_mock()
+        mock_process_clss = MagicMock()
+        pjlink.pjlink_functions['CLSS'] = mock_process_clss
+
+        # WHEN: process_command is called with valid function and data
+        pjlink.process_command(cmd='CLSS', data='1')
+
+        # THEN: Process method should have been called properly
+        mock_log.debug.assert_called_with(log_text)
+        mock_process_clss.assert_called_with('1')
+
+    @patch.object(pjlink_test, 'change_status')
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_err1(self, mock_log, mock_change_status):
+        """
+        Test ERR1 - Undefined projector function
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        log_text = '(127.0.0.1) Projector returned error "ERR1"'
+        mock_change_status.reset_mock()
+        mock_log.reset_mock()
+
+        # WHEN: process_command called with ERR1
+        pjlink.process_command(cmd='CLSS', data=PJLINK_ERRORS[E_UNDEFINED])
+
+        # THEN: Error should be logged and status_change should be called
+        mock_change_status.assert_called_once_with(E_UNDEFINED, 'Undefined Command: "CLSS"')
+        mock_log.error.assert_called_with(log_text)
+
+    @patch.object(pjlink_test, 'change_status')
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_err2(self, mock_log, mock_change_status):
+        """
+        Test ERR2 - Parameter Error
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        log_text = '(127.0.0.1) Projector returned error "ERR2"'
+        mock_change_status.reset_mock()
+        mock_log.reset_mock()
+
+        # WHEN: process_command called with ERR2
+        pjlink.process_command(cmd='CLSS', data=PJLINK_ERRORS[E_PARAMETER])
+
+        # THEN: Error should be logged and status_change should be called
+        mock_change_status.assert_called_once_with(E_PARAMETER)
+        mock_log.error.assert_called_with(log_text)
+
+    @patch.object(pjlink_test, 'change_status')
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_err3(self, mock_log, mock_change_status):
+        """
+        Test ERR3 - Unavailable error
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        log_text = '(127.0.0.1) Projector returned error "ERR3"'
+        mock_change_status.reset_mock()
+        mock_log.reset_mock()
+
+        # WHEN: process_command called with ERR3
+        pjlink.process_command(cmd='CLSS', data=PJLINK_ERRORS[E_UNAVAILABLE])
+
+        # THEN: Error should be logged and status_change should be called
+        mock_change_status.assert_called_once_with(E_UNAVAILABLE)
+        mock_log.error.assert_called_with(log_text)
+
+    @patch.object(pjlink_test, 'change_status')
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_err4(self, mock_log, mock_change_status):
+        """
+        Test ERR3 - Unavailable error
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        log_text = '(127.0.0.1) Projector returned error "ERR4"'
+        mock_change_status.reset_mock()
+        mock_log.reset_mock()
+
+        # WHEN: process_command called with ERR3
+        pjlink.process_command(cmd='CLSS', data=PJLINK_ERRORS[E_PROJECTOR])
+
+        # THEN: Error should be logged and status_change should be called
+        mock_change_status.assert_called_once_with(E_PROJECTOR)
+        mock_log.error.assert_called_with(log_text)
+
+    @patch.object(pjlink_test, 'projectorAuthentication')
+    @patch.object(pjlink_test, 'change_status')
+    @patch.object(pjlink_test, 'disconnect_from_host')
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_erra(self, mock_log, mock_disconnect, mock_change_status, mock_err_authenticate):
+        """
+        Test ERRA - Authentication Error
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        log_text = '(127.0.0.1) Projector returned error "ERRA"'
+        mock_change_status.reset_mock()
+        mock_log.reset_mock()
+
+        # WHEN: process_command called with ERRA
+        pjlink.process_command(cmd='CLSS', data=PJLINK_ERRORS[E_AUTHENTICATION])
+
+        # THEN: Error should be logged and several methods should be called
+        self.assertTrue(mock_disconnect.called, 'disconnect_from_host should have been called')
+        mock_change_status.assert_called_once_with(E_AUTHENTICATION)
+        mock_log.error.assert_called_with(log_text)
+
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_future(self, mock_log):
+        """
+        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')
+
+        # 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')
+
+        # THEN: Error should be logged and no command called
+        self.assertFalse(mock_functions.called, 'Should not have gotten to the end of the method')
+        mock_log.warn.assert_called_once_with(log_text)
+
+    @patch.object(pjlink_test, 'pjlink_functions')
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_process_command_invalid(self, mock_log, mock_functions):
+        """
+        Test not a valid command
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        mock_functions.reset_mock()
+        mock_log.reset_mock()
+
+        # 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)"
+
+        # THEN: Error should be logged and no command called
+        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.lib.projector.pjlink, 'log')
+    def test_process_command_ok(self, mock_log, mock_functions):
+        """
+        Test command returned success
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        mock_functions.reset_mock()
+        mock_log.reset_mock()
+
+        # 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'
+
+        # 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
+        mock_log.debug.assert_called_with(log_text)

=== modified file 'tests/functional/openlp_core_lib/test_projector_pjlink_commands.py'
--- tests/functional/openlp_core_lib/test_projector_pjlink_commands.py	2017-08-07 00:08:41 +0000
+++ tests/functional/openlp_core_lib/test_projector_pjlink_commands.py	2017-08-11 11:14:30 +0000
@@ -25,16 +25,20 @@
 from unittest import TestCase
 from unittest.mock import patch, MagicMock
 
+import openlp.core.lib.projector.pjlink
 from openlp.core.lib.projector.pjlink import PJLink
-from openlp.core.lib.projector.constants import PJLINK_ERST_STATUS, PJLINK_POWR_STATUS, \
-    S_OFF, S_STANDBY, S_ON
+from openlp.core.lib.projector.constants import ERROR_STRING, PJLINK_ERST_DATA, PJLINK_ERST_STATUS, \
+    PJLINK_POWR_STATUS, E_WARN, E_ERROR, S_OFF, S_STANDBY, S_ON
 
 from tests.resources.projector.data import TEST_PIN
 
 pjlink_test = PJLink(name='test', ip='127.0.0.1', pin=TEST_PIN, no_poll=True)
 
-# ERST status codes
-ERST_OK, ERST_WARN, ERST_ERR = '0', '1', '2'
+# Create a list of ERST positional data so we don't have to redo the same buildup multiple times
+PJLINK_ERST_POSITIONS = []
+for pos in range(0, len(PJLINK_ERST_DATA)):
+    if pos in PJLINK_ERST_DATA:
+        PJLINK_ERST_POSITIONS.append(PJLINK_ERST_DATA[pos])
 
 
 class TestPJLinkCommands(TestCase):
@@ -85,7 +89,25 @@
         self.assertTrue(mock_socket_timer.called, 'Projector socket_timer.stop() should have been called')
 
     @patch.object(pjlink_test, 'projectorUpdateIcons')
-    def test_projector_process_avmt_closed_muted(self, mock_projectorReceivedData):
+    def test_projector_process_avmt_bad_data(self, mock_UpdateIcons):
+        """
+        Test avmt bad data fail
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        pjlink.shutter = True
+        pjlink.mute = True
+
+        # WHEN: Called with an invalid setting
+        pjlink.process_avmt('36')
+
+        # THEN: Shutter should be closed and mute should be True
+        self.assertTrue(pjlink.shutter, 'Shutter should changed')
+        self.assertTrue(pjlink.mute, 'Audio should not have changed')
+        self.assertFalse(mock_UpdateIcons.emit.called, 'Update icons should NOT have been called')
+
+    @patch.object(pjlink_test, 'projectorUpdateIcons')
+    def test_projector_process_avmt_closed_muted(self, mock_UpdateIcons):
         """
         Test avmt status shutter closed and mute off
         """
@@ -99,12 +121,13 @@
 
         # THEN: Shutter should be closed and mute should be True
         self.assertTrue(pjlink.shutter, 'Shutter should have been set to closed')
-        self.assertTrue(pjlink.mute, 'Audio should be off')
+        self.assertTrue(pjlink.mute, 'Audio should be muted')
+        self.assertTrue(mock_UpdateIcons.emit.called, 'Update icons should have been called')
 
     @patch.object(pjlink_test, 'projectorUpdateIcons')
-    def test_projector_process_avmt_shutter_closed(self, mock_projectorReceivedData):
+    def test_projector_process_avmt_shutter_closed(self, mock_UpdateIcons):
         """
-        Test avmt status shutter closed and audio muted
+        Test avmt status shutter closed and audio unchanged
         """
         # GIVEN: Test object
         pjlink = pjlink_test
@@ -117,11 +140,12 @@
         # THEN: Shutter should be True and mute should be False
         self.assertTrue(pjlink.shutter, 'Shutter should have been set to closed')
         self.assertTrue(pjlink.mute, 'Audio should not have changed')
+        self.assertTrue(mock_UpdateIcons.emit.called, 'Update icons should have been called')
 
     @patch.object(pjlink_test, 'projectorUpdateIcons')
-    def test_projector_process_avmt_audio_muted(self, mock_projectorReceivedData):
+    def test_projector_process_avmt_audio_muted(self, mock_UpdateIcons):
         """
-        Test avmt status shutter open and mute on
+        Test avmt status shutter unchanged and mute on
         """
         # GIVEN: Test object
         pjlink = pjlink_test
@@ -134,11 +158,12 @@
         # THEN: Shutter should be closed and mute should be True
         self.assertTrue(pjlink.shutter, 'Shutter should not have changed')
         self.assertTrue(pjlink.mute, 'Audio should be off')
+        self.assertTrue(mock_UpdateIcons.emit.called, 'Update icons should have been called')
 
     @patch.object(pjlink_test, 'projectorUpdateIcons')
-    def test_projector_process_avmt_open_unmuted(self, mock_projectorReceivedData):
+    def test_projector_process_avmt_open_unmuted(self, mock_UpdateIcons):
         """
-        Test avmt status shutter open and mute off off
+        Test avmt status shutter open and mute off
         """
         # GIVEN: Test object
         pjlink = pjlink_test
@@ -151,6 +176,7 @@
         # THEN: Shutter should be closed and mute should be True
         self.assertFalse(pjlink.shutter, 'Shutter should have been set to open')
         self.assertFalse(pjlink.mute, 'Audio should be on')
+        self.assertTrue(mock_UpdateIcons.emit.called, 'Update icons should have been called')
 
     def test_projector_process_clss_one(self):
         """
@@ -164,7 +190,7 @@
 
         # THEN: Projector class should be set to 1
         self.assertEqual(pjlink.pjlink_class, '1',
-                         'Projector should have returned class=1')
+                         'Projector should have set class=1')
 
     def test_projector_process_clss_two(self):
         """
@@ -178,11 +204,11 @@
 
         # THEN: Projector class should be set to 1
         self.assertEqual(pjlink.pjlink_class, '2',
-                         'Projector should have returned class=2')
+                         'Projector should have set class=2')
 
-    def test_projector_process_clss_nonstandard_reply(self):
+    def test_projector_process_clss_nonstandard_reply_optoma(self):
         """
-        Bugfix 1550891: CLSS request returns non-standard reply
+        Bugfix 1550891: CLSS request returns non-standard reply with Optoma projector
         """
         # GIVEN: Test object
         pjlink = pjlink_test
@@ -194,52 +220,124 @@
         self.assertEqual(pjlink.pjlink_class, '1',
                          'Non-standard class reply should have set class=1')
 
+    def test_projector_process_clss_nonstandard_reply_benq(self):
+        """
+        Bugfix 1550891: CLSS request returns non-standard reply with BenQ projector
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+
+        # WHEN: Process non-standard reply
+        pjlink.process_clss('Version2')
+
+        # THEN: Projector class should be set with proper value
+        # NOTE: At this time BenQ is Class 1, but we're trying a different value to verify
+        self.assertEqual(pjlink.pjlink_class, '2',
+                         'Non-standard class reply should have set class=2')
+
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_projector_process_clss_invalid_nan(self, mock_log):
+        """
+        Test CLSS reply has no class number
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+
+        # WHEN: Process invalid reply
+        pjlink.process_clss('Z')
+        log_warn_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',
+                         'Non-standard class reply should have set class=1')
+        mock_log.error.assert_called_once_with(log_warn_text)
+
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_projector_process_clss_invalid_no_version(self, mock_log):
+        """
+        Test CLSS reply has no class number
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+
+        # WHEN: Process invalid reply
+        pjlink.process_clss('Invalid')
+        log_warn_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',
+                         'Non-standard class reply should have set class=1')
+        mock_log.error.assert_called_once_with(log_warn_text)
+
     def test_projector_process_erst_all_ok(self):
         """
         Test test_projector_process_erst_all_ok
         """
         # GIVEN: Test object
         pjlink = pjlink_test
-        chk_test = ERST_OK
+        chk_test = PJLINK_ERST_STATUS['OK']
+        chk_param = chk_test * len(PJLINK_ERST_POSITIONS)
 
         # WHEN: process_erst with no errors
-        pjlink.process_erst('{fan}{lamp}{temp}{cover}{filter}{other}'.format(fan=chk_test,
-                                                                             lamp=chk_test,
-                                                                             temp=chk_test,
-                                                                             cover=chk_test,
-                                                                             filter=chk_test,
-                                                                             other=chk_test))
+        pjlink.process_erst(chk_param)
 
-        # PJLink instance errors should be None
+        # THEN: PJLink instance errors should be None
         self.assertIsNone(pjlink.projector_errors, 'projector_errors should have been set to None')
 
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_projector_process_erst_data_invalid_length(self, mock_log):
+        """
+        Test test_projector_process_erst_data_invalid_length
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        pjlink.projector_errors = None
+        log_warn_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')
+
+        # THEN: pjlink.projector_errors should be empty and warning logged
+        self.assertIsNone(pjlink.projector_errors, 'There should be no errors')
+        self.assertTrue(mock_log.warn.called, 'Warning should have been logged')
+        mock_log.warn.assert_called_once_with(log_warn_text)
+
+    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    def test_projector_process_erst_data_invalid_nan(self, mock_log):
+        """
+        Test test_projector_process_erst_data_invalid_nan
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        pjlink.projector_errors = None
+        log_warn_text = "(127.0.0.1) Invalid error status response '1111Z1'"
+
+        # WHEN: process_erst called with invalid data (too many values
+        pjlink.process_erst('1111Z1')
+
+        # THEN: pjlink.projector_errors should be empty and warning logged
+        self.assertIsNone(pjlink.projector_errors, 'There should be no errors')
+        self.assertTrue(mock_log.warn.called, 'Warning should have been logged')
+        mock_log.warn.assert_called_once_with(log_warn_text)
+
     def test_projector_process_erst_all_warn(self):
         """
         Test test_projector_process_erst_all_warn
         """
         # GIVEN: Test object
         pjlink = pjlink_test
-        chk_test = ERST_WARN
-        chk_code = PJLINK_ERST_STATUS[chk_test]
-        chk_value = {'Fan': chk_code,
-                     'Lamp': chk_code,
-                     'Temperature': chk_code,
-                     'Cover': chk_code,
-                     'Filter': chk_code,
-                     'Other': chk_code
-                     }
+        chk_test = PJLINK_ERST_STATUS[E_WARN]
+        chk_string = ERROR_STRING[E_WARN]
+        chk_param = chk_test * len(PJLINK_ERST_POSITIONS)
 
         # WHEN: process_erst with status set to WARN
-        pjlink.process_erst('{fan}{lamp}{temp}{cover}{filter}{other}'.format(fan=chk_test,
-                                                                             lamp=chk_test,
-                                                                             temp=chk_test,
-                                                                             cover=chk_test,
-                                                                             filter=chk_test,
-                                                                             other=chk_test))
+        pjlink.process_erst(chk_param)
 
-        # PJLink instance errors should match chk_value
-        self.assertEqual(pjlink.projector_errors, chk_value,
-                         'projector_errors should have been set to all {err}'.format(err=chk_code))
+        # 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))
 
     def test_projector_process_erst_all_error(self):
         """
@@ -247,27 +345,45 @@
         """
         # GIVEN: Test object
         pjlink = pjlink_test
-        chk_test = ERST_ERR
-        chk_code = PJLINK_ERST_STATUS[chk_test]
-        chk_value = {'Fan': chk_code,
-                     'Lamp': chk_code,
-                     'Temperature': chk_code,
-                     'Cover': chk_code,
-                     'Filter': chk_code,
-                     'Other': chk_code
-                     }
-
-        # WHEN: process_erst with status set to ERROR
-        pjlink.process_erst('{fan}{lamp}{temp}{cover}{filter}{other}'.format(fan=chk_test,
-                                                                             lamp=chk_test,
-                                                                             temp=chk_test,
-                                                                             cover=chk_test,
-                                                                             filter=chk_test,
-                                                                             other=chk_test))
-
-        # PJLink instance errors should be set to chk_value
-        self.assertEqual(pjlink.projector_errors, chk_value,
-                         'projector_errors should have been set to all {err}'.format(err=chk_code))
+        chk_test = PJLINK_ERST_STATUS[E_ERROR]
+        chk_string = ERROR_STRING[E_ERROR]
+        chk_param = chk_test * len(PJLINK_ERST_POSITIONS)
+
+        # WHEN: process_erst with status set to WARN
+        pjlink.process_erst(chk_param)
+
+        # 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))
+
+    def test_projector_process_erst_warn_cover_only(self):
+        """
+        Test test_projector_process_erst_warn_cover_only
+        """
+        # GIVEN: Test object
+        pjlink = pjlink_test
+        chk_test = PJLINK_ERST_STATUS[E_WARN]
+        chk_string = ERROR_STRING[E_WARN]
+        pos = PJLINK_ERST_DATA['COVER']
+        build_chk = []
+        for check in range(0, len(PJLINK_ERST_POSITIONS)):
+            if check == pos:
+                build_chk.append(chk_test)
+            else:
+                build_chk.append(PJLINK_ERST_STATUS['OK'])
+        chk_param = ''.join(build_chk)
+
+        # WHEN: process_erst with cover only set to WARN and all others set to OK
+        pjlink.process_erst(chk_param)
+
+        # THEN: Only COVER should have an error
+        self.assertEqual(len(pjlink.projector_errors), 1, 'projector_errors should only have 1 error')
+        self.assertTrue(('Cover' in pjlink.projector_errors), 'projector_errors should have an error for "Cover"')
+        self.assertEqual(pjlink.projector_errors['Cover'],
+                         chk_string,
+                         'projector_errors["Cover"] should have error "{err}"'.format(err=chk_string))
 
     def test_projector_process_inpt(self):
         """