Review: Needs Fixing
Just a few minor things
Diff comments:
>
> === modified file 'openlp/core/api/websockets.py'
> --- openlp/core/api/websockets.py 2017-12-29 09:15:48 +0000
> +++ openlp/core/api/websockets.py 2018-01-07 05:37:57 +0000
> @@ -28,37 +28,88 @@
> import logging
> import time
>
> -import websockets
> -from PyQt5 import QtCore
> +from websockets import serve
>
> from openlp.core.common.mixins import LogMixin, RegistryProperties
> from openlp.core.common.registry import Registry
> from openlp.core.common.settings import Settings
> +from openlp.core.threading import ThreadWorker, run_thread
>
> log = logging.getLogger(__name__)
>
>
> -class WebSocketWorker(QtCore.QObject):
> +async def handle_websocket(request, path):
> + """
> + Handle web socket requests and return the poll information.
> + Check ever 0.2 seconds to get the latest position and send if changed.
typo in "ever
> + Only gets triggered when 1st client attaches
> +
> + :param request: request from client
> + :param path: determines the endpoints supported
> + :return:
Can you remove this ':return:' please
> + """
> + log.debug('WebSocket handler registered with client')
> + previous_poll = None
> + previous_main_poll = None
> + poller = Registry().get('poller')
> + if path == '/state':
> + while True:
> + current_poll = poller.poll()
> + if current_poll != previous_poll:
> + await request.send(json.dumps(current_poll).encode())
> + previous_poll = current_poll
> + await asyncio.sleep(0.2)
> + elif path == '/live_changed':
> + while True:
> + main_poll = poller.main_poll()
> + if main_poll != previous_main_poll:
> + await request.send(main_poll)
> + previous_main_poll = main_poll
> + await asyncio.sleep(0.2)
> +
> +
> +class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin):
> """
> A special Qt thread class to allow the WebSockets server to run at the
> same time as the UI.
> """
> - def __init__(self, server):
> - """
> - Constructor for the thread class.
> -
> - :param server: The http server class.
> - """
> - self.ws_server = server
> - super(WebSocketWorker, self).__init__()
> -
> - def run(self):
> - """
> - Run the thread.
> - """
> - self.ws_server.start_server()
> + def start(self):
> + """
> + Run the worker.
> + """
> + address = Settings().value('api/ip address')
> + port = Settings().value('api/websocket port')
> + # Start the event loop
> + self.event_loop = asyncio.new_event_loop()
> + asyncio.set_event_loop(self.event_loop)
> + # Create the websocker server
> + loop = 1
> + self.server = None
> + while not self.server:
> + try:
> + self.server = serve(handle_websocket, address, port)
> + log.debug('WebSocket server started on
> {addr}:{port}'.format(addr=address, port=port))
> + except Exception as e:
'e' unused
> + log.exception('Failed to start WebSocket server')
> + loop += 1
> + time.sleep(0.1)
> + if not self.server and loop > 3:
> + log.error('Unable to start WebSocket server {addr}:{port},
> giving up'.format(addr=address, port=port))
> + if self.server:
> + # If the websocket server exists, start listening
> + self.event_loop.run_until_complete(self.server)
> + self.event_loop.run_forever()
> + self.quit.emit()
>
> def stop(self):
> - self.ws_server.stop = True
> + """
> + Stop the websocket server
> + """
> + if hasattr(self.server, 'ws_server'):
> + self.server.ws_server.close()
> + elif hasattr(self.server, 'server'):
> + self.server.server.close()
> + self.event_loop.stop()
> + self.event_loop.close()
>
>
> class WebSocketServer(RegistryProperties, LogMixin):
>
> === modified file 'openlp/core/threading.py'
> --- openlp/core/threading.py 2017-12-29 09:15:48 +0000
> +++ openlp/core/threading.py 2018-01-07 05:37:57 +0000
> @@ -51,5 +66,46 @@
> worker.quit.connect(thread.quit)
> worker.quit.connect(worker.deleteLater)
> thread.finished.connect(thread.deleteLater)
> - if auto_start:
> + thread.finished.connect(make_remove_thread(thread_name))
> + if can_start:
> thread.start()
> +
> +
> +def get_thread_worker(thread_name):
> + """
> + Get the worker by the thread name
> +
> + :param str thread_name: The name of the thread
> + :returns ThreadWorker: The worker for this thread name
> + """
> + return Registry().get('main_window').threads.get(thread_name)
> +
> +
> +def is_thread_finished(thread_name):
> + """
> + Check if a thread is finished running.
> +
> + :param str thread_name: The name of the thread
> + :returns bool: True if the thread is finished, False if it is still
> running
Not sure this is valid. Think you need to use :rtype: see my irc comments
> + """
> + main_window = Registry().get('main_window')
> + return thread_name not in main_window.threads or
> main_window.threads[thread_name]['thread'].isFinished()
> +
> +
> +def make_remove_thread(thread_name):
> + """
> + Create a function to remove the thread once the thread is finished.
> +
> + :param str thread_name: The name of the thread which should be removed
> from the thread registry.
> + :returns function: A function which will remove the thread from the
> thread registry.
Not sure this is valid. Think you need to use :rtype: see my irc comments
> + """
> + def remove_thread():
> + """
> + Stop and remove a registered thread
> +
> + :param str thread_name: The name of the thread to stop and remove
> + """
> + main_window = Registry().get('main_window')
> + if thread_name in main_window.threads:
> + del main_window.threads[thread_name]
> + return remove_thread
--
https://code.launchpad.net/~raoul-snyman/openlp/better-threading/+merge/335801
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
