Cleanup and small enhancements

Restructured code to simplify maintenance and building
Fixed message delay calculation
Switched to kv language for interface description
Enhanced logging handling of modules with Kivy
Add application icon
Streamlined windows binary

Author: rdoursenaud

.gitignore

@@ -7,4 +7,4 @@ build
 dist
 __pycache__
 # Asset sources
-sources
+assets

README.md

@@ -1,7 +1,7 @@
 Denon Remote
 ============
 
-![Screenshot](screenshot-v0.2.0.png)
+![Screenshot](screenshot-v0.3.0.png)
 
 Author: Raphael Doursenaud <rdoursenaud@gmail.com>
 
@@ -85,6 +85,11 @@ Dependencies:
     - [ ] The Pythonic Way
 - [ ] Handle shutdown to power off the device
 - [x] PyInstaller
+    - [x] Generate icon with [IconMaker](https://github.com/Inedo/iconmaker)
+    - [x] [UPX](https://upx.github.io/) support
+    - How to build:
+        - Review [denonremote.spec](denonremote.spec)
+        - Use `python -m PyInstaller denonremote.spec --upx-dir=c:\upx-3.96-win64`
 - [ ] VST plugin? (Not required if MIDI input is implemented but would be neat to have in the monitoring section of a
   DAW)
     - [ ] See [PyVST](https://pypi.org/project/pyvst/)

constraints.txt

@@ -2,28 +2,29 @@
 
 from kivy_deps import sdl2, glew
 
+# Minimize dependencies bundling
+from kivy.tools.packaging.pyinstaller_hooks import get_deps_minimal, get_deps_all, hookspath, runtime_hooks
+
 block_cipher = None
 
 added_files = [
-    ( 'Unicode_IEC_symbol.ttf', '.' ),
-    ('assets', 'assets')
+    ('denonremote\\fonts', 'fonts'),
+    ('denonremote\\images', 'images')
 ]
 
-a = Analysis(['main.py'],
-             pathex=['G:\\raph\\Documents\\GitHub\\denonremote'],
-             binaries=[],
+a = Analysis(['denonremote\\main.py'],
+             pathex=['denonremote'],
              datas=added_files,
-             hiddenimports=[],
-             hookspath=[],
+             hookspath=hookspath(),
              runtime_hooks=[],
-             excludes=[],
              win_no_prefer_redirects=False,
              win_private_assemblies=False,
              cipher=block_cipher,
-             noarchive=False)
+             noarchive=False,
+             **get_deps_all())
 pyz = PYZ(a.pure, a.zipped_data,
              cipher=block_cipher)
-exe = EXE(pyz, Tree('G:\\raph\\Documents\\GitHub\\denonremote\\'),
+exe = EXE(pyz, Tree('denonremote'),
           a.scripts,
           a.binaries,
           a.zipfiles,
@@ -37,4 +38,5 @@ exe = EXE(pyz, Tree('G:\\raph\\Documents\\GitHub\\denonremote\\'),
           upx=True,
           upx_exclude=[],
           runtime_tmpdir=None,
-          console=False)
+          console=False,
+          icon='icon.ico')

denonremote.spec

@@ -6,13 +6,9 @@
 from twisted.internet.protocol import ClientFactory
 from twisted.protocols.basic import LineOnlyReceiver
 
-from config import GUI
-from denon.dn500av import DN500AVMessages, DN500AVFormat
+from denon.dn500av import DN500AVMessage, DN500AVFormat
 
-if GUI:
-    from kivy import Logger
-
-    logger = Logger
+logger = logging.getLogger(__name__)
 
 
 # TODO: Implement Serial ?
@@ -24,40 +20,35 @@ class DenonProtocol(LineOnlyReceiver):
     MAX_LENGTH = 135
     DELAY = 0.04  # in seconds. The documentation requires 200 ms. 40 ms seems safe.
     delimiter = b'\r'
-    ongoing_calls = -1  # Delay handling. FIXME: should timeout after 200 ms.
-
-    logger = None
+    ongoing_calls = 0  # Delay handling. FIXME: should timeout after 200 ms.
 
     def sendLine(self, line):
         if b'?' in line:
             # A request is made. We need to delay the next calls
             self.ongoing_calls += 1
-            self.logger.debug("Ongoing calls for delay: %s", self.ongoing_calls)
-        self.logger.debug("Will send line: %s", line)
-        return task.deferLater(reactor, delay=self.DELAY * self.ongoing_calls, callable=super().sendLine, line=line)
+            logger.debug("Ongoing calls for delay: %s", self.ongoing_calls)
+        logger.debug("Will send line: %s", line)
+        if self.ongoing_calls:
+            delay = self.DELAY * (self.ongoing_calls - 1)
+        else:
+            delay = self.DELAY
+        return task.deferLater(reactor, delay=delay,
+                               callable=super().sendLine, line=line)
 
     def lineReceived(self, line):
         if self.ongoing_calls:
             # We received a reply
             self.ongoing_calls -= 1
-            self.logger.debug("Ongoing calls for delay: %s", self.ongoing_calls)
-        receiver = DN500AVMessages(logger=self.logger)
+            logger.debug("Ongoing calls for delay: %s", self.ongoing_calls)
+        receiver = DN500AVMessage()
         receiver.parse_response(line)
-        self.logger.info("Received line: %s", receiver.response)
-        if self.factory.gui:
-            self.factory.app.print_message(receiver.response)
-            # FIXME: abstract
-            # MUTE
-            if receiver.command_code == 'MU':
-                state = False
-                if receiver.parameter_code == 'ON':
-                    state = True
-                self.factory.app.update_volume_mute(state)
+        logger.info("Received line: %s", receiver.response)
 
-            # VOLUME
-            if receiver.command_code == 'MV':
-                if receiver.subcommand_code is None:
-                    self.factory.app.update_volume(receiver.parameter_label)
+        # FIXME: parse message into state
+
+        # FIXME: abstract away with a callback to the factory
+        if self.factory.gui:
+            self.factory.app.print_debug(receiver.response)
 
             # POWER
             if receiver.command_code == 'PW':
@@ -66,10 +57,22 @@ def lineReceived(self, line):
                     state = False
                 self.factory.app.update_power(state)
 
+            # VOLUME
+            if receiver.command_code == 'MV':
+                if receiver.subcommand_code is None:
+                    self.factory.app.update_volume(receiver.parameter_label)
+
+            # MUTE
+            if receiver.command_code == 'MU':
+                state = False
+                if receiver.parameter_code == 'ON':
+                    state = True
+                self.factory.app.set_volume_mute(state)
+
             # SOURCE
             if receiver.command_code == 'SI':
                 source = receiver.parameter_code
-                self.factory.app.update_source(source)
+                self.factory.app.set_sources(source)
 
     def connectionMade(self):
         if self.factory.gui:
@@ -79,7 +82,7 @@ def get_power(self):
         self.sendLine('PW?'.encode('ASCII'))
 
     def set_power(self, state):
-        self.logger.debug("Entering power callback")
+        logger.debug("Entering power callback")
         if state:
             self.sendLine('PWON'.encode('ASCII'))
         else:
@@ -91,7 +94,7 @@ def get_volume(self):
     def set_volume(self, value):
         rawvalue = DN500AVFormat().mv_reverse_params.get(value)
         if rawvalue is None:
-            self.logger.warning("Set volume value %s is invalid.", value)
+            logger.warning("Set volume value %s is invalid.", value)
         else:
             message = 'MV' + rawvalue
             self.sendLine(message.encode('ASCII'))
@@ -118,7 +121,6 @@ class DenonClientFactory(ClientFactory):
 
     def __init__(self):
         self.gui = False
-        self.protocol.logger = logging.getLogger(__name__)
 
 
 class DenonClientGUIFactory(ClientFactory):
@@ -127,4 +129,6 @@ class DenonClientGUIFactory(ClientFactory):
     def __init__(self, app):
         self.gui = True
         self.app = app
-        self.protocol.logger = Logger
+        import kivy.logger
+        global logger
+        logger = kivy.logger.Logger

cli.py denonremote/cli.pycli.py renamed to denonremote/cli.py

@@ -1,10 +1,11 @@
 # -*- coding: utf-8 -*-
-from config import GUI
+"""
+Denon DN-500AV serial and IP communication protocol description
+"""
 
-if GUI:
-    from kivy import Logger
+import logging
 
-    logger = Logger
+logger = logging.getLogger(__name__)
 
 commands = {
     'PW': "Power",
@@ -722,12 +723,10 @@ def compute_lfe_volume_label(value):
 # TODO: abstract device
 
 
-class DN500AVMessages():
+class DN500AVMessage:
     # From DN-500 manual (DN-500AVEM_ENG_CD-ROM_v00.pdf)
     # Pages 93-101 (99-107 in PDF form)
 
-    logger = None
-
     command_code = None
     command_label = None
     subcommand_code = None
@@ -736,15 +735,15 @@ class DN500AVMessages():
     parameter_label = None
     response = None
 
-    def __init__(self, logger=None):
-        self.logger = logger
+    def __init__(self):
+        pass
 
     def parse_response(self, status_command):
         # Handle strings and bytes
         if type(status_command) is bytes:
             # FIXME: some parts can be UTF-8 encoded
             status_command = status_command.decode('ASCII')
-        self.logger.debug("Received status command: %s", status_command)
+        logger.debug("Received status command: %s", status_command)
 
         # Commands are of known sizes. Try the largest first.
         for i in range(commands_max_size, commands_min_size - 1, -1):
@@ -754,19 +753,19 @@ def parse_response(self, status_command):
                 break
 
         if self.command_label is None:
-            self.logger.error("Command unknown: %s", status_command)
+            logger.error("Command unknown: %s", status_command)
             return
         else:
-            self.logger.info("Parsed command %s: %s", self.command_code, self.command_label)
+            logger.info("Parsed command %s: %s", self.command_code, self.command_label)
 
         # Trim command from status command stream
         status_command = status_command[len(self.command_code):]
 
         # Handle subcommands
         if commands_subcommands.get(self.command_code) is None:
-            self.logger.debug("The command %s doesn't have any known subcommands.", self.command_code)
+            logger.debug("The command %s doesn't have any known subcommands.", self.command_code)
         else:
-            self.logger.debug("Searching for subcommands in: %s", status_command)
+            logger.debug("Searching for subcommands in: %s", status_command)
 
             # Subcommands are of known sizes. Try the largest first.
             for i in range(commands_subcommands_max_size[self.command_code],
@@ -777,49 +776,46 @@ def parse_response(self, status_command):
                     break
 
             if self.subcommand_label is None:
-                self.logger.debug("Subcommand unknown. Probably a parameter: %s", status_command)
+                logger.debug("Subcommand unknown. Probably a parameter: %s", status_command)
                 self.subcommand_code = None
             else:
-                self.logger.info("Parsed subcommand %s: %s", self.subcommand_code, self.subcommand_label)
+                logger.info("Parsed subcommand %s: %s", self.subcommand_code, self.subcommand_label)
                 # Trim subcommand from status command stream
                 status_command = status_command[
                                  len(self.subcommand_code) + 1:]  # Subcommands have a space before the parameter
 
         # Handle parameters
-        self.logger.debug("Searching for parameters in: %s", status_command)
+        logger.debug("Searching for parameters in: %s", status_command)
         self.parameter_code = status_command
         if self.command_code == 'PS':
             self.parameter_label = commands_params[self.command_code][self.subcommand_code].get(self.parameter_code)
         else:
             self.parameter_label = commands_params[self.command_code].get(self.parameter_code)
         if self.parameter_label is None:
-            self.logger.error("Parameter unknown: %s", status_command)
+            logger.error("Parameter unknown: %s", status_command)
             self.parameter_code = None
         else:
             # Trim parameters from status command stream
             status_command = status_command[len(status_command):]
 
         # Handle unexpected leftovers
         if status_command:
-            self.logger.error("Unexpected unparsed data found: %s", status_command)
+            logger.error("Unexpected unparsed data found: %s", status_command)
 
         if self.subcommand_label:
             self.response = "%s, %s: %s" % (self.command_label, self.subcommand_label, self.parameter_label)
         else:
             self.response = "%s: %s" % (self.command_label, self.parameter_label)
 
 
-class DN500AVFormat():
-    logger = None
-
+class DN500AVFormat:
     mv_reverse_params = {}
 
-    def __init__(self, logger=None):
-        self.logger = logger
+    def __init__(self):
         self.mv_reverse_params = dict([(value, key) for key, value in mv_params.items()])
 
     def get_raw_volume_value_from_db_value(self, value):
-        self.logger.debug('value: %s', value)
+        logger.debug('value: %s', value)
         raw_value = self.mv_reverse_params['value']
-        self.logger.debug('rawvalue: %s', raw_value)
+        logger.debug('rawvalue: %s', raw_value)
         return raw_value