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

[Ken Roberts]

PJLink2 Update U

Honest - last merge request for this series
(found oops in running code not found during test)

- Fix projector S_ON status icon selection
- Remove unused status items (status icons changed to FA)
- Fix projector icon status update for network errors
    (keep error icon until network reconnects rather than disconnect icon)
- Fix colors to projector toolbar connect/power icons
- Fix toolbar select input icon not always enabled after selecting/connecting projector
- Add max-line-length option for flake8 tests in setup.cfg
- Fix projector source select check for db id in projector object
- Fix projector tests

--------------------------------------------------------------------------------
lp:~alisonken1/openlp/pjlink2-u (revision 2849)
https://ci.openlp.io/job/Branch-01-Pull/2699/ [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2593/ [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/363/ [FAILURE]
https://ci.openlp.io/job/Branch-03a-Build-Source/192/ [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/171/ [FAILURE]
https://ci.openlp.io/job/Branch-04a-Code-Lint/1654/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1467/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/354/ [FAILURE]

[Ken Roberts (alisonken1)]

--------------------------------------------------------------------------------
lp:~alisonken1/openlp/pjlink2-u (revision 2849)
https://ci.openlp.io/job/Branch-01-Pull/2696/ [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2590/ [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/360/ [FAILURE]
https://ci.openlp.io/job/Branch-03a-Build-Source/190/ [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/169/ [FAILURE]
https://ci.openlp.io/job/Branch-04a-Code-Lint/1652/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1465/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/352/ [FAILURE]

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting passed!

[Raoul Snyman (raoul-snyman)]

macOS tests failed, please see https://ci.openlp.io/job/MP-04-macOS-Tests/42/ for more details

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting passed!

[Raoul Snyman (raoul-snyman)]

macOS tests failed, please see https://ci.openlp.io/job/MP-04-macOS-Tests/43/ for more details

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting passed!

[Raoul Snyman (raoul-snyman)]

macOS tests failed, please see https://ci.openlp.io/job/MP-04-macOS-Tests/44/ for more details

[Phill (phill-ridout)]

Few minor inlines.

review: Needs Fixing

[lp:~alisonken1/openlp/pjlink2-u]

2850. By Ken Roberts on 2019-03-09

Minor cleanups

[Tim Bentley (trb143)]

review: Approve

Unmerged revisions

=== modified file 'openlp/core/projectors/db.py'
--- openlp/core/projectors/db.py	2019-02-14 15:09:09 +0000
+++ openlp/core/projectors/db.py	2019-03-09 03:53:55 +0000
@@ -417,11 +417,17 @@
                   value: (str) From ProjectorSource, Sources tables or PJLink default code list
         """
         source_dict = {}
+        # Apparently, there was a change to the projector object. Test for which object has db id
+        if hasattr(projector, "id"):
+            chk = projector.id
+        elif hasattr(projector.entry, "id"):
+            chk = projector.entry.id
+
         # Get default list first
         for key in projector.source_available:
             item = self.get_object_filtered(ProjectorSource,
                                             and_(ProjectorSource.code == key,
-                                                 ProjectorSource.projector_id == projector.id))
+                                                 ProjectorSource.projector_id == chk))
             if item is None:
                 source_dict[key] = PJLINK_DEFAULT_CODES[key]
             else:
 
=== modified file 'openlp/core/projectors/manager.py'
--- openlp/core/projectors/manager.py	2019-02-14 15:09:09 +0000
+++ openlp/core/projectors/manager.py	2019-03-09 03:53:55 +0000
@@ -46,31 +46,10 @@
 from openlp.core.ui.icons import UiIcons
 from openlp.core.widgets.toolbar import OpenLPToolbar
 
-
 log = logging.getLogger(__name__)
 log.debug('projectormanager loaded')
 
 
-# Dict for matching projector status to display icon
-STATUS_ICONS = {
-    S_NOT_CONNECTED: ':/projector/projector_item_disconnect.png',
-    S_CONNECTING: ':/projector/projector_item_connect.png',
-    S_CONNECTED: ':/projector/projector_off.png',
-    S_OFF: ':/projector/projector_off.png',
-    S_INITIALIZE: ':/projector/projector_off.png',
-    S_STANDBY: ':/projector/projector_off.png',
-    S_WARMUP: ':/projector/projector_warmup.png',
-    S_ON: ':/projector/projector_on.png',
-    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'
-}
-
-
 class UiProjectorManager(object):
     """
     UI part of the Projector Manager
@@ -122,7 +101,7 @@
         self.one_toolbar.add_toolbar_action('connect_projector',
                                             text=translate('OpenLP.ProjectorManager',
                                                            'Connect to selected projector.'),
-                                            icon=UiIcons().projector_connect,
+                                            icon=UiIcons().projector_select_connect,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Connect to selected projector.'),
                                             triggers=self.on_connect_projector)
@@ -136,7 +115,7 @@
         self.one_toolbar.add_toolbar_action('disconnect_projector',
                                             text=translate('OpenLP.ProjectorManager',
                                                            'Disconnect from selected projectors'),
-                                            icon=UiIcons().projector_disconnect,
+                                            icon=UiIcons().projector_select_disconnect,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Disconnect from selected projector.'),
                                             triggers=self.on_disconnect_projector)
@@ -151,7 +130,7 @@
         self.one_toolbar.add_toolbar_action('poweron_projector',
                                             text=translate('OpenLP.ProjectorManager',
                                                            'Power on selected projector'),
-                                            icon=UiIcons().projector_on,
+                                            icon=UiIcons().projector_power_on,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Power on selected projector.'),
                                             triggers=self.on_poweron_projector)
@@ -164,7 +143,7 @@
                                             triggers=self.on_poweron_projector)
         self.one_toolbar.add_toolbar_action('poweroff_projector',
                                             text=translate('OpenLP.ProjectorManager', 'Standby selected projector'),
-                                            icon=UiIcons().projector_off,
+                                            icon=UiIcons().projector_power_off,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Put selected projector in standby.'),
                                             triggers=self.on_poweroff_projector)
@@ -309,7 +288,7 @@
             S_INITIALIZE: UiIcons().projector_on,
             S_STANDBY: UiIcons().projector_off,
             S_WARMUP: UiIcons().projector_warmup,
-            S_ON: UiIcons().projector_off,
+            S_ON: UiIcons().projector_on,
             S_COOLDOWN: UiIcons().projector_cooldown,
             E_ERROR: UiIcons().projector_error,
             E_NETWORK: UiIcons().error,
@@ -880,6 +859,7 @@
         """
         Update the icons when the selected projectors change
         """
+        log.debug('update_icons(): Checking for selected projector items in list')
         count = len(self.projector_list_widget.selectedItems())
         projector = None
         if count == 0:
@@ -900,6 +880,7 @@
             self.get_toolbar_item('blank_projector_multiple', hidden=True)
             self.get_toolbar_item('show_projector_multiple', hidden=True)
         elif count == 1:
+            log.debug('update_icons(): Found one item selected')
             projector = self.projector_list_widget.selectedItems()[0].data(QtCore.Qt.UserRole)
             connected = QSOCKET_STATE[projector.link.state()] == S_CONNECTED
             power = projector.link.power == S_ON
@@ -910,12 +891,14 @@
             self.get_toolbar_item('blank_projector_multiple', hidden=True)
             self.get_toolbar_item('show_projector_multiple', hidden=True)
             if connected:
+                log.debug('update_icons(): Updating icons for connected state')
                 self.get_toolbar_item('view_projector', enabled=True)
                 self.get_toolbar_item('source_view_projector',
-                                      enabled=connected and power and projector.link.source_available is not None)
+                                      enabled=projector.link.source_available is not None and connected and power)
                 self.get_toolbar_item('edit_projector', hidden=True)
                 self.get_toolbar_item('delete_projector', hidden=True)
             else:
+                log.debug('update_icons(): Updating for not connected state')
                 self.get_toolbar_item('view_projector', hidden=True)
                 self.get_toolbar_item('source_view_projector', hidden=True)
                 self.get_toolbar_item('edit_projector', enabled=True)
@@ -931,6 +914,7 @@
                 self.get_toolbar_item('blank_projector', enabled=False)
                 self.get_toolbar_item('show_projector', enabled=False)
         else:
+            log.debug('update_icons(): Updating for multiple items selected')
             self.get_toolbar_item('edit_projector', enabled=False)
             self.get_toolbar_item('delete_projector', enabled=False)
             self.get_toolbar_item('view_projector', hidden=True)
 
=== modified file 'openlp/core/projectors/pjlink.py'
--- openlp/core/projectors/pjlink.py	2019-02-14 15:09:09 +0000
+++ openlp/core/projectors/pjlink.py	2019-03-09 03:53:55 +0000
@@ -528,8 +528,9 @@
             sources.append(source)
         sources.sort()
         self.source_available = sources
-        log.debug('({ip}) Setting projector sources_available to "{data}"'.format(ip=self.entry.name,
-                                                                                  data=self.source_available))
+        log.debug('({ip}) Setting projector source_available to "{data}"'.format(ip=self.entry.name,
+                                                                                 data=self.source_available))
+        self.projectorUpdateIcons.emit()
         return
 
     def process_lamp(self, data):
@@ -886,6 +887,9 @@
         elif status >= S_NOT_CONNECTED and status in QSOCKET_STATE:
             # Socket connection status update
             self.status_connect = status
+            # Check if we need to update error state as well
+            if self.error_status != S_OK and status != S_NOT_CONNECTED:
+                self.error_status = S_OK
         elif status >= S_NOT_CONNECTED and status in PROJECTOR_STATE:
             # Only affects the projector status
             self.projector_status = status
@@ -905,7 +909,14 @@
                                                                     message=status_message if msg is None else msg))
 
         # Now that we logged extra information for debugging, broadcast the original change/message
-        (code, message) = self._get_status(status)
+        # Check for connection errors first
+        if self.error_status != S_OK:
+            log.debug('({ip}) Signalling error code'.format(ip=self.entry.name))
+            code, message = self._get_status(self.error_status)
+            status = self.error_status
+        else:
+            log.debug('({ip}) Signalling status code'.format(ip=self.entry.name))
+            code, message = self._get_status(status)
         if msg is not None:
             message = msg
         elif message is None:
@@ -953,7 +964,7 @@
             log.error('({ip}) Invalid initial packet received - closing socket'.format(ip=self.entry.name))
             return self.disconnect_from_host()
         # Convert the initial login prompt with the expected PJLink normal command format for processing
-        log.debug('({ip}) check_login(): Formatting initial connection prompt'
+        log.debug('({ip}) check_login(): Formatting initial connection prompt '
                   'to PJLink packet'.format(ip=self.entry.name))
         return self.get_data('{start}{clss}{data}'.format(start=PJLINK_PREFIX,
                                                           clss='1',
 
=== modified file 'openlp/core/ui/advancedtab.py'
--- openlp/core/ui/advancedtab.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/advancedtab.py	2019-03-09 03:53:55 +0000
@@ -455,7 +455,7 @@
         Service Name options changed
         """
         self.service_name_day.setEnabled(default_service_enabled)
-        time_enabled = default_service_enabled and self.service_name_day.currentIndex() is not 7
+        time_enabled = default_service_enabled and self.service_name_day.currentIndex() != 7
         self.service_name_time.setEnabled(time_enabled)
         self.service_name_edit.setEnabled(default_service_enabled)
         self.service_name_revert_button.setEnabled(default_service_enabled)
@@ -497,7 +497,7 @@
         """
         React to the day of the service name changing.
         """
-        self.service_name_time.setEnabled(service_day is not 7)
+        self.service_name_time.setEnabled(service_day != 7)
         self.update_service_name_example(None)
 
     def on_service_name_revert_button_clicked(self):
 
=== modified file 'openlp/core/ui/icons.py'
--- openlp/core/ui/icons.py	2019-02-15 22:34:53 +0000
+++ openlp/core/ui/icons.py	2019-03-09 03:53:55 +0000
@@ -117,13 +117,17 @@
             'presentation': {'icon': 'fa.bar-chart'},
             'preview': {'icon': 'fa.laptop'},
             'projector': {'icon': 'op.video'},
-            'projector_connect': {'icon': 'fa.plug'},
+            'projector_connect': {'icon': 'fa.plug'},  # Projector connect
             'projector_cooldown': {'icon': 'fa.video-camera', 'attr': 'blue'},
-            'projector_disconnect': {'icon': 'fa.plug', 'attr': 'lightGray'},
+            'projector_disconnect': {'icon': 'fa.plug', 'attr': 'lightGray'},  # Projector disconnect
             'projector_error': {'icon': 'fa.video-camera', 'attr': 'red'},
             'projector_hdmi': {'icon': 'op.hdmi'},
-            'projector_off': {'icon': 'fa.video-camera', 'attr': 'black'},
-            'projector_on': {'icon': 'fa.video-camera', 'attr': 'green'},
+            'projector_power_off': {'icon': 'fa.video-camera', 'attr': 'red'},  # Toolbar power off
+            'projector_power_on': {'icon': 'fa.video-camera', 'attr': 'green'},  # Toolbar power on
+            'projector_off': {'icon': 'fa.video-camera', 'attr': 'black'},  # Projector off
+            'projector_on': {'icon': 'fa.video-camera', 'attr': 'green'},  # Projector on
+            'projector_select_connect': {'icon': 'fa.plug', 'attr': 'green'},  # Toolbar connect
+            'projector_select_disconnect': {'icon': 'fa.plug', 'attr': 'red'},  # Toolbar disconnect
             'projector_warmup': {'icon': 'fa.video-camera', 'attr': 'yellow'},
             'picture': {'icon': 'fa.picture-o'},
             'print': {'icon': 'fa.print'},
 
=== modified file 'openlp/core/ui/themestab.py'
--- openlp/core/ui/themestab.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/themestab.py	2019-03-09 03:53:55 +0000
@@ -206,7 +206,7 @@
         find_and_set_in_combo_box(self.default_combo_box, self.global_theme)
         # self.renderer.set_global_theme()
         self.renderer.set_theme_level(self.theme_level)
-        if self.global_theme is not '':
+        if self.global_theme != '':
             self._preview_global_theme()
 
     def _preview_global_theme(self):
 
=== modified file 'setup.cfg'
--- setup.cfg	2018-10-30 19:46:55 +0000
+++ setup.cfg	2019-03-09 03:53:55 +0000
@@ -9,6 +9,7 @@
 
 [flake8]
 exclude=resources.py,vlc.py
+max-line-length = 120
 ignore = E402,W503,W504,D
 
 [pycodestyle]
 
=== modified file 'tests/functional/openlp_core/lib/test_image_manager.py'
--- tests/functional/openlp_core/lib/test_image_manager.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_core/lib/test_image_manager.py	2019-03-09 03:53:55 +0000
@@ -178,7 +178,7 @@
         # THEN a KeyError is thrown
         with self.assertRaises(KeyError) as context:
             self.image_manager.get_image(TEST_PATH, 'church1.jpg')
-        assert context.exception is not '', 'KeyError exception should have been thrown for missing image'
+        assert context.exception != '', 'KeyError exception should have been thrown for missing image'
 
     @patch('openlp.core.lib.imagemanager.run_thread')
     def test_different_dimension_image(self, mocked_run_thread):
@@ -211,7 +211,7 @@
         # WHEN: calling with correct image, but wrong dimensions
         with self.assertRaises(KeyError) as context:
             self.image_manager.get_image(full_path, 'church.jpg', 120, 120)
-        assert context.exception is not '', 'KeyError exception should have been thrown for missing dimension'
+        assert context.exception != '', 'KeyError exception should have been thrown for missing dimension'
 
     @patch('openlp.core.lib.imagemanager.resize_image')
     @patch('openlp.core.lib.imagemanager.image_to_byte')
 
=== modified file 'tests/functional/openlp_plugins/media/test_mediaplugin.py'
--- tests/functional/openlp_plugins/media/test_mediaplugin.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_plugins/media/test_mediaplugin.py	2019-03-09 03:53:55 +0000
@@ -57,4 +57,4 @@
         # THEN: about() should return a string object
         assert isinstance(MediaPlugin.about(), str)
         # THEN: about() should return a non-empty string
-        assert len(MediaPlugin.about()) is not 0
+        assert len(MediaPlugin.about()) != 0
 
=== modified file 'tests/functional/openlp_plugins/songs/test_lib.py'
--- tests/functional/openlp_plugins/songs/test_lib.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_plugins/songs/test_lib.py	2019-03-09 03:53:55 +0000
@@ -199,7 +199,7 @@
         result = _remove_typos(diff)
 
         # THEN: There should be no typos in the middle anymore. The remaining equals should have been merged.
-        assert len(result) is 1, 'The result should contain only one element.'
+        assert len(result) == 1, 'The result should contain only one element.'
         assert result[0][0] == 'equal', 'The result should contain an equal element.'
         assert result[0][1] == 0, 'The start indices should be kept.'
         assert result[0][2] == 22, 'The stop indices should be kept.'
 
=== modified file 'tests/functional/openlp_plugins/songusage/test_songusage.py'
--- tests/functional/openlp_plugins/songusage/test_songusage.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_plugins/songusage/test_songusage.py	2019-03-09 03:53:55 +0000
@@ -45,8 +45,8 @@
         # THEN: about() should return a string object
         assert isinstance(SongUsagePlugin.about(), str)
         # THEN: about() should return a non-empty string
-        assert len(SongUsagePlugin.about()) is not 0
-        assert len(SongUsagePlugin.about()) is not 0
+        assert len(SongUsagePlugin.about()) != 0
+        assert len(SongUsagePlugin.about()) != 0
 
     @patch('openlp.plugins.songusage.songusageplugin.Manager')
     def test_song_usage_init(self, MockedManager):
 
=== modified file 'tests/openlp_core/projectors/test_projector_db.py'
--- tests/openlp_core/projectors/test_projector_db.py	2019-02-14 15:09:09 +0000
+++ tests/openlp_core/projectors/test_projector_db.py	2019-03-09 03:53:55 +0000
@@ -134,6 +134,7 @@
         """
         Set up anything necessary for all tests
         """
+        self.tmp_folder = mkdtemp(prefix='openlp_')
         # Create a test app to keep from segfaulting
         Registry.create()
         self.registry = Registry()
@@ -153,11 +154,11 @@
                 patch('openlp.core.ui.mainwindow.ThemeManager'), \
                 patch('openlp.core.ui.mainwindow.ProjectorManager'), \
                 patch('openlp.core.ui.mainwindow.websockets.WebSocketServer'), \
-                patch('openlp.core.ui.mainwindow.server.HttpServer'):
+                patch('openlp.core.ui.mainwindow.server.HttpServer'), \
+                patch('openlp.core.state.State.list_plugins') as mock_plugins:
+            mock_plugins.return_value = []
             self.main_window = MainWindow()
 
-        # Create a temporary database directory and database
-        self.tmp_folder = mkdtemp(prefix='openlp_')
         tmpdb_url = 'sqlite:///{db}'.format(db=os.path.join(self.tmp_folder, TEST_DB))
         mocked_init_url.return_value = tmpdb_url
         self.projector = ProjectorDB()
 
=== modified file 'tests/openlp_core/projectors/test_projector_pjlink_commands_01.py'
--- tests/openlp_core/projectors/test_projector_pjlink_commands_01.py	2019-02-14 15:09:09 +0000
+++ tests/openlp_core/projectors/test_projector_pjlink_commands_01.py	2019-03-09 03:53:55 +0000
@@ -452,12 +452,16 @@
         """
         Test saving video source available information
         """
+
         # GIVEN: Test object
         with patch.object(openlp.core.projectors.pjlink, 'log') as mock_log:
             pjlink = PJLink(Projector(**TEST1_DATA), no_poll=True)
             pjlink.source_available = []
-            log_debug_calls = [call('({ip}) Setting projector sources_available to '
+            log_debug_calls = [call('({ip}) reset_information() connect status is '
+                                    'S_NOT_CONNECTED'.format(ip=pjlink.name)),
+                               call('({ip}) Setting projector source_available to '
                                     '"[\'11\', \'12\', \'21\', \'22\', \'31\', \'32\']"'.format(ip=pjlink.name))]
+
             chk_data = '21 12 11 22 32 31'  # Although they should already be sorted, use unsorted to verify method
             chk_test = ['11', '12', '21', '22', '31', '32']