Skip to content

Commit

Permalink
Merge pull request #1761 from peternewman/0.10-c11-compat
Browse files Browse the repository at this point in the history
Fix vast majority of outstanding Python 3 issues
  • Loading branch information
peternewman authored Feb 26, 2023
2 parents e523002 + f5be4e7 commit bb3fb8b
Show file tree
Hide file tree
Showing 23 changed files with 453 additions and 137 deletions.
8 changes: 2 additions & 6 deletions .codespellignorelines
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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,
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion NEWS
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
9 changes: 9 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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])

Expand Down
2 changes: 2 additions & 0 deletions python/ola/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
24 changes: 16 additions & 8 deletions python/ola/PidStore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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))
Expand Down
38 changes: 38 additions & 0 deletions python/ola/PidStoreTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")])
Expand Down Expand Up @@ -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 = [""]
Expand Down
45 changes: 45 additions & 0 deletions python/ola/StringUtils.py
Original file line number Diff line number Diff line change
@@ -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))
57 changes: 57 additions & 0 deletions python/ola/StringUtilsTest.py
Original file line number Diff line number Diff line change
@@ -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()
1 change: 1 addition & 0 deletions tools/rdm/ExpectedResultsTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 18 additions & 1 deletion tools/rdm/Makefile.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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__/*

Expand Down
Loading

0 comments on commit bb3fb8b

Please sign in to comment.