Fix vast majority of outstanding Python 3 issues

.codespellignorelines

@@ -17,7 +17,6 @@
      /(?:([0-9]{1,3})(?:\s+THRU\s+([0-9]{0,3}))?)(?:\s+@\s+([0-9]{0,3}))?$/);
   str = str.replace('>', 'THRU');
                     ' THRU ' + ola.common.DmxConstants.MAX_CHANNEL_NUMBER);
-                    ' THRU ' + ola.common.DmxConstants.MAX_CHANNEL_NUMBER);
     // If it's the T or > keys, autocomplete 'THRU'
          case 'U': // THRU
   var values = ['7', '8', '9', ' THRU ', '4', '5', '6', ' @ ', '1', '2', '3',
@@ -118,9 +117,6 @@ class AsyncronousLibUsbAdaptor : public BaseLibUsbAdaptor {
   OLA_ASSERT_EQ(expected, JsonWriter::AsString(uint_value));
  * Test the uint item
     "  \"type\": \"uint\",\n"
-    "  \"type\": \"uint\",\n"
-    "  \"type\": \"uint\",\n"
-    "  \"type\": \"uint\",\n"
   std::map<std::string, UIntMap*> m_uint_map_variables;
     if (message.uint_offset < MAX_UINT_FIELDS) {
       message.uint16_fields[message.uint_offset++] = field->Value();
@@ -131,10 +127,8 @@ class AsyncronousLibUsbAdaptor : public BaseLibUsbAdaptor {
     status_message() : uint_offset(0), int_offset(0), status_type(0),
     std::string Type() const { return "uint"; }
       if (items[i]['type'] == 'uint') {
-      if (items[i]['type'] == 'uint') {
     if (type == 'string' || type == 'uint' || type == 'hidden') {
 const char RDMHTTPModule::GENERIC_UINT_FIELD[] = "int";
-  section.AddItem(new HiddenItem("1", GENERIC_UINT_FIELD));
   section.AddItem(new HiddenItem("1", GENERIC_UINT_FIELD));
   SelectItem *item = new SelectItem("Personality", GENERIC_UINT_FIELD);
   string personality_str = request->GetParameter(GENERIC_UINT_FIELD);
@@ -198,4 +192,6 @@ import java.nio.ByteOrder;
                      "{'a': 'caf\\\\xe9'}")
 #    self.assertEqual('%s' % rtf._EscapeData({"caf\xe9": "bar"}),
 #                     "{'caf\xe9': 'bar'}")
+    self.assertEqual('caf\\xe9', StringEscape(u'caf\xe9'))
+    self.assertEqual('caf\\xe9', ("%s" % StringEscape(u'caf\xe9')))
   "forin": true,

.gitignore

@@ -196,6 +196,8 @@ python/ola/PidStoreTest.sh
 python/ola/RDMTest.sh
 python/ola/Version.py
 python/ola/rpc/SimpleRpcControllerTest.sh
+python/ola/testing/
+python/ola/testing/__init__.py
 slp/slp_client
 slp/slp_client.exe
 slp/slp_server
@@ -228,6 +230,7 @@ tools/rdm/DataLocation.py
 tools/rdm/ExpectedResultsTest.sh
 tools/rdm/ResponderTestTest.sh
 tools/rdm/TestHelpersTest.sh
+tools/rdm/TestRunnerTest.sh
 tools/rdm/TestStateTest.sh
 tools/rdmpro/rdmpro_sniffer
 tools/rdmpro/rdmpro_sniffer.exe

NEWS

@@ -1,12 +1,14 @@
 x/y/2023 ola-0.10.9
  Features:
- * Further improvements on Python 3 compatibility #1506
+ * Python 3 compatibility across the board (including the RDM Responder Tests)!
+   #1506
  * Support for the JMS USB2DMX PRO V2.1 device via the FTDI plugin #1728
 
  API:
  * Python: Add a fetch current DMX example.
 
  RDM Tests:
+ * Python 3 compatibility of the RDM Tests #1599
  * Fix a longstanding bug in the GetMaxPacketSize RDM test around timeouts
 
  Bugs:

configure.ac

@@ -973,18 +973,27 @@ AM_CONDITIONAL([FOUND_CPPLINT], [test "x$cpplint" = xyes])
 # srcdir and set PYTHONPATH=${top_builddir}/python in data/rdm/Makefile.am
 AC_CONFIG_LINKS([python/ola/__init__.py:python/ola/__init__.py
                  python/ola/ClientWrapper.py:python/ola/ClientWrapper.py
+                 python/ola/DMXConstants.py:python/ola/DMXConstants.py
+                 python/ola/DUBDecoder.py:python/ola/DUBDecoder.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/StringUtils.py:python/ola/StringUtils.py
                  python/ola/TestUtils.py:python/ola/TestUtils.py
                  python/ola/UID.py:python/ola/UID.py
                  python/ola/rpc/__init__.py:python/ola/rpc/__init__.py
                  python/ola/rpc/SimpleRpcController.py:python/ola/rpc/SimpleRpcController.py
                  python/ola/rpc/StreamRpcChannel.py:python/ola/rpc/StreamRpcChannel.py
+                 tools/rdm/__init__.py:tools/rdm/__init__.py
                  tools/rdm/ExpectedResults.py:tools/rdm/ExpectedResults.py
+                 tools/rdm/ResponderTest.py:tools/rdm/ResponderTest.py
                  tools/rdm/TestCategory.py:tools/rdm/TestCategory.py
+                 tools/rdm/TestDefinitions.py:tools/rdm/TestDefinitions.py
+                 tools/rdm/TestHelpers.py:tools/rdm/TestHelpers.py
+                 tools/rdm/TestMixins.py:tools/rdm/TestMixins.py
+                 tools/rdm/TestRunner.py:tools/rdm/TestRunner.py
                  tools/rdm/TestState.py:tools/rdm/TestState.py
                  tools/rdm/TimingStats.py:tools/rdm/TimingStats.py])
 

python/ola/Makefile.mk

@@ -85,6 +85,7 @@ dist_check_SCRIPTS += \
     python/ola/OlaClientTest.py \
     python/ola/PidStoreTest.py \
     python/ola/RDMTest.py \
+    python/ola/StringUtilsTest.py \
     python/ola/TestUtils.py \
     python/ola/UIDTest.py
 
@@ -96,6 +97,7 @@ test_scripts += \
     python/ola/OlaClientTest.sh \
     python/ola/PidStoreTest.sh \
     python/ola/RDMTest.sh \
+    python/ola/StringUtilsTest.py \
     python/ola/UIDTest.py
 endif
 

python/ola/PidStore.py

@@ -45,7 +45,7 @@
 MAX_VALID_SUB_DEVICE = 0x0200
 ALL_SUB_DEVICES = 0xffff
 
-# The two types of commands classes
+# The different types of commands classes
 RDM_GET, RDM_SET, RDM_DISCOVERY = range(3)
 
 
@@ -637,6 +637,12 @@ def Pack(self, args):
     arg = args[0]
     arg_size = len(arg)
 
+    # Handle the fact a UTF-8 character could be multi-byte
+    if sys.version_info >= (3, 2):
+      arg_size = max(arg_size, len(bytes(arg, 'utf-8')))
+    else:
+      arg_size = max(arg_size, len(arg.encode('utf-8')))
+
     if self.max is not None and arg_size > self.max:
       raise ArgsValidationError('%s can be at most %d,' %
                                 (self.name, self.max))
@@ -647,9 +653,9 @@ def Pack(self, args):
 
     try:
       if sys.version_info >= (3, 2):
-        data = struct.unpack('%ds' % arg_size, bytes(arg, 'utf8'))
+        data = struct.unpack('%ds' % arg_size, bytes(arg, 'utf-8'))
       else:
-        data = struct.unpack('%ds' % arg_size, arg)
+        data = struct.unpack('%ds' % arg_size, arg.encode('utf-8'))
     except struct.error as e:
       raise ArgsValidationError("Can't pack data: %s" % e)
     return data[0], 1
@@ -669,10 +675,12 @@ def Unpack(self, data):
     except struct.error as e:
       raise UnpackException(e)
 
-    if sys.version_info >= (3, 2):
-      return value[0].rstrip(b'\x00').decode('utf-8')
-    else:
-      return value[0].rstrip(b'\x00')
+    try:
+      value = value[0].rstrip(b'\x00').decode('utf-8')
+    except UnicodeDecodeError as e:
+      raise UnpackException(e)
+
+    return value
 
   def GetDescription(self, indent=0):
     indent = ' ' * indent
@@ -878,7 +886,7 @@ def Unpack(self, data):
             'Too many repeated group_count for %s, limit is %d, found %d' %
             (self.name, self.max, group_count))
 
-      if self.max is not None and group_count < self.min:
+      if self.min is not None and group_count < self.min:
         raise UnpackException(
             'Too few repeated group_count for %s, limit is %d, found %d' %
             (self.name, self.min, group_count))

python/ola/PidStoreTest.py

@@ -222,6 +222,22 @@ def testPackUnpack(self):
     self.assertEqual(decoded['slots_required'], 7)
     self.assertEqual(decoded['name'], "UnpackTest")
 
+    # Test null handling, trailing null should be truncated on the way back in
+    args = ["42", "7", "Foo\0"]
+    blob = pid._responses.get(PidStore.RDM_GET).Pack(args)[0]
+    # Not truncated here
+    self.assertEqual(blob, binascii.unhexlify("2a0007466f6f00"))
+    decoded = pid.Unpack(blob, PidStore.RDM_GET)
+    self.assertEqual(decoded['personality'], 42)
+    self.assertEqual(decoded['slots_required'], 7)
+    self.assertEqual(decoded['name'], "Foo")
+
+    # Confirm we raise an error if we try and unpack a non-ASCII, non-UTF-8
+    # containing packet (0xc0)
+    with self.assertRaises(PidStore.UnpackException):
+      blob = binascii.unhexlify("2a0007556e7061636bc054657374")
+      decoded = pid.Unpack(blob, PidStore.RDM_GET)
+
   def testPackRanges(self):
     store = PidStore.PidStore()
     store.Load([os.path.join(path, "test_pids.proto")])
@@ -279,6 +295,28 @@ def testPackRanges(self):
       args = ["enx"]
       blob = pid._responses.get(PidStore.RDM_GET).Pack(args)[0]
 
+    # test packing some non-printable characters
+    args = ["\x0d\x7f"]
+    blob = pid._responses.get(PidStore.RDM_GET).Pack(args)[0]
+    self.assertEqual(blob, binascii.unhexlify("0d7f"))
+    decoded = pid.Unpack(blob, PidStore.RDM_GET)
+    self.assertEqual(decoded, {'languages': [{'language': '\x0d\x7f'}]})
+
+    # test packing some non-ascii characters, as the
+    # LATIN CAPITAL LETTER A WITH GRAVE, unicode U+00C0 gets encoded as two
+    # bytes (\xc3\x80) the total length is three bytes and it doesn't fit!
+    with self.assertRaises(PidStore.ArgsValidationError):
+      args = [u"\x0d\xc0"]
+      blob = pid._responses.get(PidStore.RDM_GET).Pack(args)[0]
+
+    # It works on it's own as it's short enough...
+    args = [u"\u00c0"]
+    blob = pid._responses.get(PidStore.RDM_GET).Pack(args)[0]
+    self.assertEqual(blob, binascii.unhexlify("c380"))
+    decoded = pid.Unpack(blob, PidStore.RDM_GET)
+    # This is the unicode code point for it
+    self.assertEqual(decoded, {'languages': [{'language': u'\u00c0'}]})
+
     # valid empty string
     pid = store.GetName("STATUS_ID_DESCRIPTION")
     args = [""]

python/ola/StringUtils.py

@@ -0,0 +1,45 @@
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# StringUtils.py
+# Copyright (C) 2022 Peter Newman
+
+"""Common utils for OLA Python string handling"""
+
+import sys
+
+if sys.version_info >= (3, 0):
+  try:
+    unicode
+  except NameError:
+    unicode = str
+
+__author__ = '[email protected] (Simon Newton)'
+
+
+def StringEscape(s):
+  """Escape unprintable characters in a string."""
+  # TODO(Peter): How does this interact with the E1.20 Unicode flag?
+  # We don't use sys.version_info.major to support Python 2.6.
+  if sys.version_info[0] == 2 and type(s) == str:
+    return s.encode('string-escape')
+  elif sys.version_info[0] == 2 and type(s) == unicode:
+    return s.encode('unicode-escape')
+  elif type(s) == str:
+    # All strings in Python 3 are unicode
+    # This encode/decode pair gets us an escaped string
+    return s.encode('unicode-escape').decode(encoding="ascii",
+                                             errors="backslashreplace")
+  else:
+    raise TypeError('Only strings are supported not %s' % type(s))

python/ola/StringUtilsTest.py

@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# StringUtilsTest.py
+# Copyright (C) 2022 Peter Newman
+
+import unittest
+
+from ola.StringUtils import StringEscape
+
+"""Test cases for StringUtils utilities."""
+
+__author__ = '[email protected] (Simon Newton)'
+
+
+class StringUtilsTest(unittest.TestCase):
+  def testStringEscape(self):
+    # Test we escape properly
+    self.assertEqual('foo', StringEscape("foo"))
+    self.assertEqual('bar', StringEscape("bar"))
+    self.assertEqual('bar[]', StringEscape("bar[]"))
+    self.assertEqual('foo-bar', StringEscape(u'foo-bar'))
+    self.assertEqual('foo\\x00bar', StringEscape("foo\x00bar"))
+    # TODO(Peter): How does this interact with the E1.20 Unicode flag?
+    self.assertEqual('caf\\xe9', StringEscape(u'caf\xe9'))
+    self.assertEqual('foo\\u2014bar', StringEscape(u'foo\u2014bar'))
+
+    # Test that we display nicely in a string
+    self.assertEqual('foo', ("%s" % StringEscape("foo")))
+    self.assertEqual('bar[]', ("%s" % StringEscape("bar[]")))
+    self.assertEqual('foo-bar', ("%s" % StringEscape(u'foo-bar')))
+    self.assertEqual('foo\\x00bar', ("%s" % StringEscape("foo\x00bar")))
+    # TODO(Peter): How does this interact with the E1.20 Unicode flag?
+    self.assertEqual('caf\\xe9', ("%s" % StringEscape(u'caf\xe9')))
+    self.assertEqual('foo\\u2014bar', ("%s" % StringEscape(u'foo\u2014bar')))
+
+    # Confirm we throw an exception if we pass in a number or something else
+    # that's not a string
+    with self.assertRaises(TypeError):
+      result = StringEscape(42)
+      self.assertNone(result)
+
+
+if __name__ == '__main__':
+  unittest.main()

tools/rdm/ExpectedResultsTest.py

@@ -19,6 +19,7 @@
 import unittest
 from collections import namedtuple
 
+# Keep this import relative to simplify the testing
 from ExpectedResults import (BroadcastResult, DUBResult, InvalidResponse,
                              SuccessfulResult, TimeoutResult,
                              UnsupportedResult)

tools/rdm/Makefile.mk

@@ -61,7 +61,10 @@ tools/rdm/ExpectedResultsTest.sh: tools/rdm/Makefile.mk
 	chmod +x $(top_builddir)/tools/rdm/ExpectedResultsTest.sh
 
 tools/rdm/ResponderTestTest.sh: tools/rdm/Makefile.mk
-	mkdir -p $(top_builddir)/python/ola
+	mkdir -p $(top_builddir)/python/ola/testing
+	touch $(top_builddir)/python/ola/testing/__init__.py
+        # This link is relative within the builddir
+	$(LN_S) -f ../../../tools/rdm $(top_builddir)/python/ola/testing/rdm
 	echo "PYTHONPATH=${top_builddir}/python $(PYTHON) ${srcdir}/tools/rdm/ResponderTestTest.py; exit \$$?" > $(top_builddir)/tools/rdm/ResponderTestTest.sh
 	chmod +x $(top_builddir)/tools/rdm/ResponderTestTest.sh
 
@@ -70,6 +73,14 @@ tools/rdm/TestHelpersTest.sh: tools/rdm/Makefile.mk
 	echo "PYTHONPATH=${top_builddir}/python $(PYTHON) ${srcdir}/tools/rdm/TestHelpersTest.py; exit \$$?" > $(top_builddir)/tools/rdm/TestHelpersTest.sh
 	chmod +x $(top_builddir)/tools/rdm/TestHelpersTest.sh
 
+tools/rdm/TestRunnerTest.sh: tools/rdm/Makefile.mk
+	mkdir -p $(top_builddir)/python/ola/testing
+	touch $(top_builddir)/python/ola/testing/__init__.py
+        # This link is relative within the builddir
+	$(LN_S) -f ../../../tools/rdm $(top_builddir)/python/ola/testing/rdm
+	echo "PYTHONPATH=${top_builddir}/python $(PYTHON) ${srcdir}/tools/rdm/TestRunnerTest.py; exit \$$?" > $(top_builddir)/tools/rdm/TestRunnerTest.sh
+	chmod +x $(top_builddir)/tools/rdm/TestRunnerTest.sh
+
 tools/rdm/TestStateTest.sh: tools/rdm/Makefile.mk
 	mkdir -p $(top_builddir)/python/ola
 	echo "PYTHONPATH=${top_builddir}/python $(PYTHON) ${srcdir}/tools/rdm/TestStateTest.py; exit \$$?" > $(top_builddir)/tools/rdm/TestStateTest.sh
@@ -79,21 +90,27 @@ dist_check_SCRIPTS += \
    tools/rdm/ExpectedResultsTest.py \
    tools/rdm/ResponderTestTest.py \
    tools/rdm/TestHelpersTest.py \
+   tools/rdm/TestRunnerTest.py \
    tools/rdm/TestStateTest.py
 
 if BUILD_PYTHON_LIBS
 test_scripts += \
    tools/rdm/ExpectedResultsTest.sh \
    tools/rdm/ResponderTestTest.sh \
    tools/rdm/TestHelpersTest.sh \
+   tools/rdm/TestRunnerTest.sh \
    tools/rdm/TestStateTest.sh
 endif
 
 CLEANFILES += \
+    python/ola/testing/*.pyc \
+    python/ola/testing/__pycache__/* \
+    python/ola/testing/__init__.py \
     tools/rdm/*.pyc \
     tools/rdm/ExpectedResultsTest.sh \
     tools/rdm/ResponderTestTest.sh \
     tools/rdm/TestHelpersTest.sh \
+    tools/rdm/TestRunnerTest.sh \
     tools/rdm/TestStateTest.sh \
     tools/rdm/__pycache__/*