python3 rdm compat work from sdbbs 1615

.gitignore

@@ -186,11 +186,13 @@ protoc/ola_protoc
 protoc/ola_protoc.exe
 protoc/ola_protoc_plugin
 protoc/ola_protoc_plugin.exe
+python/PyCompileTest.sh
 python/examples/ola_rdm_set.py
 python/ola/ClientWrapperTest.sh
 python/ola/OlaClientTest.sh
 python/ola/PidStoreLocation.py
 python/ola/PidStoreTest.sh
+python/ola/RDMTest.sh
 python/ola/Version.py
 python/ola/rpc/SimpleRpcControllerTest.sh
 slp/slp_client

.travis.yml

@@ -90,8 +90,9 @@ matrix:
     - os: linux
       dist: xenial
       compiler: clang
-      env: TASK='compile'
-      python: '2.7'
+      env:
+      - TASK='compile'
+      - PYTHON='python3'
       addons:
         apt:
           packages:
@@ -104,7 +105,6 @@ matrix:
       dist: xenial
       compiler: gcc
       env: TASK='compile'
-      python: '2.7'
       addons:
         apt:
           packages:
@@ -178,7 +178,7 @@ matrix:
       dist: xenial
       env:
        - TASK='codespell'
-       - PATH=/opt/python/3.7.1/bin:$PATH
+       - PYTHON='python3'
       addons:
         apt:
           packages:
@@ -314,10 +314,11 @@ install:
   - if [ "$TRAVIS_OS_NAME" == "linux" ]; then pip install --user numpy; fi
   - if [ "$TASK" = "coverage" ]; then pip install --user cpp-coveralls; fi
   - if [ "$TASK" = "flake8" ]; then pip install --user flake8; fi
-  - if [ "$TASK" = "codespell" ]; then pip3 install --user git+https://github.com/codespell-project/codespell.git; fi
+  - if [ "$TASK" = "codespell" ]; then pip install --user git+https://github.com/codespell-project/codespell.git; fi
   - if [ "$TASK" = "jshint" ]; then npm install -g grunt-cli; fi
 
 before_install:
+ - if [ "$PYTHON" == "python3" ]; then pyenv global 3.7.1 ; fi
 #Fix permissions for unbound (and possibly others)
  - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then if [ ! -d /usr/local/sbin ]; then sudo mkdir -p /usr/local/sbin && sudo chown -R $(whoami) /usr/local/sbin; fi; fi
 #Add a missing gnupg folder
@@ -355,12 +356,11 @@ before_install:
  - if [ "$TASK" == "spellintian" -o "$TASK" == "spellintian-duplicates" ]; then wget "http://old-releases.ubuntu.com/ubuntu/pool/main/l/lintian/lintian_2.5.104_all.deb"; sudo dpkg -i lintian_*.deb; sudo apt-get install -f -y; fi # Install a later lintian
 
 after_failure:
-# Disabled as otherwise the logfile is too big
-#  - if [ -f ${TRAVIS_BUILD_DIR}/config.log ]; then cat ${TRAVIS_BUILD_DIR}/config.log; fi
-  - if [ -f ${TRAVIS_BUILD_DIR}/ola-*/_build/config.log ]; then cat ${TRAVIS_BUILD_DIR}/ola-*/_build/config.log; fi
-  - if [ -f ${TRAVIS_BUILD_DIR}/ola-*/_build/sub/config.log ]; then cat ${TRAVIS_BUILD_DIR}/ola-*/_build/sub/config.log; fi
   - if [ -f ${TRAVIS_BUILD_DIR}/ola-*/_build/test-suite.log ]; then cat ${TRAVIS_BUILD_DIR}/ola-*/_build/test-suite.log; fi
   - if [ -f ${TRAVIS_BUILD_DIR}/ola-*/_build/sub/test-suite.log ]; then cat ${TRAVIS_BUILD_DIR}/ola-*/_build/sub/test-suite.log; fi
+  - if [ -f ${TRAVIS_BUILD_DIR}/config.log ]; then cat ${TRAVIS_BUILD_DIR}/config.log; fi
+  - if [ -f ${TRAVIS_BUILD_DIR}/ola-*/_build/config.log ]; then cat ${TRAVIS_BUILD_DIR}/ola-*/_build/config.log; fi
+  - if [ -f ${TRAVIS_BUILD_DIR}/ola-*/_build/sub/config.log ]; then cat ${TRAVIS_BUILD_DIR}/ola-*/_build/sub/config.log; fi
 
 after_success:
   - if [ "$TASK" = "coverage" ]; then coveralls --gcov /usr/bin/gcov-8 -b . -E '.*Test\.cpp$' -E '.*\.pb\.cc$' -E '.*\.pb\.cpp$' -E '.*\.pb\.h$' -E '.*\.yy\.cpp$' -E '.*\.tab\.cpp$' -E '.*\.tab\.h$' -E '.*/doxygen/examples.*$' --gcov-options '\-lp' > /dev/null; fi

Makefile.am

@@ -149,6 +149,9 @@ built_sources =
 # Test scripts are run if BUILD_TESTS is true.
 test_scripts =
 
+# directories with python code that should be test compiled during the build
+PYTHON_BUILD_DIRS =
+
 # The includes
 # -----------------------------------------------------------------------------
 
@@ -172,6 +175,7 @@ include plugins/Makefile.mk
 include olad/Makefile.mk
 include protoc/Makefile.mk
 include python/Makefile.mk
+include scripts/Makefile.mk
 include tools/Makefile.mk
 
 # -----------------------------------------------------------------------------

configure.ac

@@ -965,6 +965,7 @@ AC_CONFIG_LINKS([python/ola/__init__.py:python/ola/__init__.py
                  python/ola/MACAddress.py:python/ola/MACAddress.py
                  python/ola/OlaClient.py:python/ola/OlaClient.py
                  python/ola/PidStore.py:python/ola/PidStore.py
+                 python/ola/RDMAPI.py:python/ola/RDMAPI.py
                  python/ola/RDMConstants.py:python/ola/RDMConstants.py
                  python/ola/TestUtils.py:python/ola/TestUtils.py
                  python/ola/UID.py:python/ola/UID.py

data/Makefile.mk

@@ -1 +1,3 @@
 include data/rdm/Makefile.mk
+
+PYTHON_BUILD_DIRS += data

include/Makefile.mk

@@ -1,2 +1,6 @@
 include include/ola/Makefile.mk
 include include/olad/Makefile.mk
+
+# uncomment when include is py3 compatible
+# PYTHON_BUILD_DIRS += include
+

python/Makefile.mk

@@ -1,2 +1,17 @@
 include python/examples/Makefile.mk
 include python/ola/Makefile.mk
+
+python/PyCompileTest.sh: python/Makefile.mk
+	mkdir -p $(top_builddir)/python
+	echo "$(PYTHON) -m compileall $(PYTHON_BUILD_DIRS); exit \$$?" > $(top_builddir)/python/PyCompileTest.sh
+	chmod +x $(top_builddir)/python/PyCompileTest.sh
+
+PYTHON_BUILD_DIRS += python
+
+if BUILD_PYTHON_LIBS
+test_scripts += \
+    python/PyCompileTest.sh
+endif
+
+CLEANFILES += \
+    python/PyCompileTest.sh

python/examples/ola_rdm_get.py

@@ -18,6 +18,7 @@
 
 '''Get a PID from a UID.'''
 
+from __future__ import print_function
 import cmd
 import getopt
 import os.path
@@ -459,7 +460,7 @@ def main():
   try:
     PidStore.GetStore(pid_location)
   except PidStore.MissingPLASAPIDs as e:
-    print e
+    print(e)
     sys.exit()
 
   controller = InteractiveModeController(universe,

python/ola/ClientWrapperTest.py

@@ -17,6 +17,7 @@
 # Copyright (C) 2019 Bruce Lowekamp
 
 import array
+import binascii
 import datetime
 import socket
 # import timeout_decorator
@@ -208,7 +209,16 @@ class results:
 
     def DataCallback(self):
       data = sockets[1].recv(4096)
-      self.assertTrue(len(data) > 100)
+      expected = binascii.unhexlify(
+        "7d000010080110001a0d557064617465446d784461746122680801126400000"
+        "000000000000000000000000000000000000000000000000000000000000000"
+        "000000000000000000000000000000000000000000000000000000000000000"
+        "000000000000000000000000000000000000000000000000000000000000000"
+        "000000")
+      self.assertEqual(data, expected,
+                       msg="Regression check failed. If protocol change "
+                       "was intended set expected to: " +
+                       str(binascii.hexlify(data)))
       results.gotdata = True
       wrapper.AddEvent(0, wrapper.Stop)
 

python/ola/Makefile.mk

@@ -73,12 +73,18 @@ python/ola/PidStoreTest.sh: python/ola/Makefile.mk
 	echo "PYTHONPATH=${top_builddir}/python TESTDATADIR=$(srcdir)/common/rdm/testdata $(PYTHON) ${srcdir}/python/ola/PidStoreTest.py; exit \$$?" > $(top_builddir)/python/ola/PidStoreTest.sh
 	chmod +x $(top_builddir)/python/ola/PidStoreTest.sh
 
+python/ola/RDMTest.sh: python/ola/Makefile.mk
+	mkdir -p $(top_builddir)/python/ola
+	echo "PYTHONPATH=${top_builddir}/python PIDSTOREDIR=$(srcdir)/data/rdm $(PYTHON) ${srcdir}/python/ola/RDMTest.py; exit \$$?" > $(top_builddir)/python/ola/RDMTest.sh
+	chmod +x $(top_builddir)/python/ola/RDMTest.sh
+
 dist_check_SCRIPTS += \
     python/ola/DUBDecoderTest.py \
     python/ola/ClientWrapperTest.py \
     python/ola/MACAddressTest.py \
     python/ola/OlaClientTest.py \
     python/ola/PidStoreTest.py \
+    python/ola/RDMTest.py \
     python/ola/TestUtils.py \
     python/ola/UIDTest.py
 
@@ -89,11 +95,14 @@ test_scripts += \
     python/ola/MACAddressTest.py \
     python/ola/OlaClientTest.sh \
     python/ola/PidStoreTest.sh \
+    python/ola/RDMTest.sh \
     python/ola/UIDTest.py
 endif
 
 CLEANFILES += \
     python/ola/*.pyc \
     python/ola/ClientWrapperTest.sh \
     python/ola/OlaClientTest.sh \
-    python/ola/PidStoreTest.sh
+    python/ola/PidStoreTest.sh \
+    python/ola/RDMTest.sh \
+    python/ola/__pycache__/*

python/ola/OlaClient.py

@@ -429,6 +429,12 @@ def message(self):
 
 
 class RDMNack(object):
+  """Nack response to a request.
+
+      Individual NACK response reasons can be access as attrs, e.g.
+      RMDNack.NR_FORMAT_ERROR
+      """
+
   NACK_SYMBOLS_TO_VALUES = {
     'NR_UNKNOWN_PID': (0, 'Unknown PID'),
     'NR_FORMAT_ERROR': (1, 'Format Error'),
@@ -1224,7 +1230,7 @@ def RunRDMDiscovery(self, universe, full, callback):
       raise OLADNotRunningException()
     return True
 
-  def RDMGet(self, universe, uid, sub_device, param_id, callback, data='',
+  def RDMGet(self, universe, uid, sub_device, param_id, callback, data=b'',
              include_frames=False):
     """Send an RDM get command.
 
@@ -1246,7 +1252,7 @@ def RDMGet(self, universe, uid, sub_device, param_id, callback, data='',
     return self._RDMMessage(universe, uid, sub_device, param_id, callback,
                             data, include_frames)
 
-  def RDMSet(self, universe, uid, sub_device, param_id, callback, data='',
+  def RDMSet(self, universe, uid, sub_device, param_id, callback, data=b'',
              include_frames=False):
     """Send an RDM set command.
 
@@ -1274,7 +1280,7 @@ def SendRawRDMDiscovery(self,
                           sub_device,
                           param_id,
                           callback,
-                          data='',
+                          data=b'',
                           include_frames=False):
     """Send an RDM Discovery command. Unless you're writing RDM tests you
       shouldn't need to use this.

python/ola/PidStore.py

@@ -53,7 +53,7 @@ class Error(Exception):
 
 
 class InvalidPidFormat(Error):
-  "Indicates the PID data file was invalid."""
+  """Indicates the PID data file was invalid."""
 
 
 class PidStructureException(Error):
@@ -458,7 +458,7 @@ def _AccountForMultiplierPack(self, value):
         raise ArgsValidationError(
             'Conversion will lose data: %d -> %d' %
             (new_value, (new_value / multiplier * multiplier)))
-      new_value = new_value / multiplier
+      new_value = int(new_value / multiplier)
 
     else:
       try:
@@ -645,7 +645,10 @@ def Pack(self, args):
                                 (self.name, self.min))
 
     try:
-      data = struct.unpack('%ds' % arg_size, arg)
+      if sys.version >= '3.2':
+        data = struct.unpack('%ds' % arg_size, bytes(arg, 'utf8'))
+      else:
+        data = struct.unpack('%ds' % arg_size, arg)
     except struct.error as e:
       raise ArgsValidationError("Can't pack data: %s" % e)
     return data[0], 1
@@ -665,7 +668,10 @@ def Unpack(self, data):
     except struct.error as e:
       raise UnpackException(e)
 
-    return value[0].rstrip('\x00')
+    if sys.version >= '3.2':
+      return value[0].rstrip(b'\x00').decode('utf-8')
+    else:
+      return value[0].rstrip(b'\x00')
 
   def GetDescription(self, indent=0):
     indent = ' ' * indent
@@ -807,10 +813,10 @@ def Pack(self, args):
         raise ArgsValidationError('Too many arguments, expected %d, got %d' %
                                   (arg_offset, len(args)))
 
-      return ''.join(data), arg_offset
+      return b''.join(data), arg_offset
 
     elif self._group_size == 0:
-      return '', 0
+      return b'', 0
     else:
       # this could be groups of fields, but we don't support that yet
       data = []
@@ -823,7 +829,7 @@ def Pack(self, args):
       if arg_offset < len(args):
         raise ArgsValidationError('Too many arguments, expected %d, got %d' %
                                   (arg_offset, len(args)))
-      return ''.join(data), arg_offset
+      return b''.join(data), arg_offset
 
   def Unpack(self, data):
     """Unpack binary data.