From b6a8af5d125d5bc58073b502016411b0d4a40567 Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Fri, 29 May 2020 12:36:47 +0100 Subject: [PATCH 1/2] Standardise on one approach to handling numeric values Previously there was a mix of define-based polyfill and explicit version checking. This change canonicalizes upon the latter, *always* using PyInt under Python 2.x and PyLong under 3.x. As such this change will remove implicit compatibility with Long datatypes from the Python 2.x version of this library. Since this (possibly unintended) compatibility witgh Python 2.x long types led to unexpected behaviour (see #34) it seems sensible to remove it and favour a TypeError. Additionally, SPI doesn't support the transmission of any one value that might be represented by a Long in Python 2.x so this support was redundant. --- spidev_module.c | 173 +++++++++++++++--------------------------------- 1 file changed, 53 insertions(+), 120 deletions(-) diff --git a/spidev_module.c b/spidev_module.c index 162b7cb..a6675aa 100644 --- a/spidev_module.c +++ b/spidev_module.c @@ -47,17 +47,13 @@ #define XFER3_MAX_BLOCK_SIZE 65535 -#if PY_MAJOR_VERSION < 3 -#define PyLong_AS_LONG(val) PyInt_AS_LONG(val) -#define PyLong_AsLong(val) PyInt_AsLong(val) -#endif - // Macros needed for Python 3 #ifndef PyInt_Check -#define PyInt_Check PyLong_Check +#define PyInt_Check PyLong_Check #define PyInt_FromLong PyLong_FromLong -#define PyInt_AsLong PyLong_AsLong -#define PyInt_Type PyLong_Type +#define PyInt_AsLong PyLong_AsLong +#define PyInt_Type PyLong_Type +#define PyInt_AS_LONG PyLong_AS_LONG #endif // Maximum block size for xfer3 @@ -197,19 +193,12 @@ SpiDev_writebytes(SpiDevObject *self, PyObject *args) for (ii = 0; ii < len; ii++) { PyObject *val = PySequence_Fast_GET_ITEM(seq, ii); -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { buf[ii] = (__u8)PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - buf[ii] = (__u8)PyLong_AS_LONG(val); - } else { - snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); - PyErr_SetString(PyExc_TypeError, wrmsg_text); - return NULL; - } + } else { + snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); + PyErr_SetString(PyExc_TypeError, wrmsg_text); + return NULL; } } @@ -323,19 +312,12 @@ SpiDev_writebytes2_seq_internal(SpiDevObject *self, PyObject *seq, Py_ssize_t le for (ii = 0; ii < block_size; ii++, jj++) { PyObject *val = PySequence_Fast_GET_ITEM(seq, jj); -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { buf[ii] = (__u8)PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - buf[ii] = (__u8)PyLong_AS_LONG(val); - } else { - snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); - PyErr_SetString(PyExc_TypeError, wrmsg_text); - return NULL; - } + } else { + snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); + PyErr_SetString(PyExc_TypeError, wrmsg_text); + return NULL; } } @@ -498,22 +480,15 @@ SpiDev_xfer(SpiDevObject *self, PyObject *args) for (ii = 0; ii < len; ii++) { PyObject *val = PySequence_Fast_GET_ITEM(seq, ii); -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { txbuf[ii] = (__u8)PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - txbuf[ii] = (__u8)PyLong_AS_LONG(val); - } else { - snprintf(wrmsg_text, sizeof(wrmsg_text) - 1, wrmsg_val, val); - PyErr_SetString(PyExc_TypeError, wrmsg_text); - free(xferptr); - free(txbuf); - free(rxbuf); - return NULL; - } + } else { + snprintf(wrmsg_text, sizeof(wrmsg_text) - 1, wrmsg_val, val); + PyErr_SetString(PyExc_TypeError, wrmsg_text); + free(xferptr); + free(txbuf); + free(rxbuf); + return NULL; } xferptr[ii].tx_buf = (unsigned long)&txbuf[ii]; xferptr[ii].rx_buf = (unsigned long)&rxbuf[ii]; @@ -540,21 +515,14 @@ SpiDev_xfer(SpiDevObject *self, PyObject *args) #else for (ii = 0; ii < len; ii++) { PyObject *val = PySequence_Fast_GET_ITEM(seq, ii); -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { txbuf[ii] = (__u8)PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - txbuf[ii] = (__u8)PyLong_AS_LONG(val); - } else { - snprintf(wrmsg_text, sizeof(wrmsg_text) - 1, wrmsg_val, val); - PyErr_SetString(PyExc_TypeError, wrmsg_text); - free(txbuf); - free(rxbuf); - return NULL; - } + } else { + snprintf(wrmsg_text, sizeof(wrmsg_text) - 1, wrmsg_val, val); + PyErr_SetString(PyExc_TypeError, wrmsg_text); + free(txbuf); + free(rxbuf); + return NULL; } } @@ -656,21 +624,14 @@ SpiDev_xfer2(SpiDevObject *self, PyObject *args) for (ii = 0; ii < len; ii++) { PyObject *val = PySequence_Fast_GET_ITEM(seq, ii); -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { txbuf[ii] = (__u8)PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - txbuf[ii] = (__u8)PyLong_AS_LONG(val); - } else { - snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); - PyErr_SetString(PyExc_TypeError, wrmsg_text); - free(txbuf); - free(rxbuf); - return NULL; - } + } else { + snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); + PyErr_SetString(PyExc_TypeError, wrmsg_text); + free(txbuf); + free(rxbuf); + return NULL; } } @@ -804,23 +765,16 @@ SpiDev_xfer3(SpiDevObject *self, PyObject *args) for (ii = 0, jj = block_start; jj < len && ii < bufsize; ii++, jj++) { PyObject *val = PySequence_Fast_GET_ITEM(seq, jj); -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { txbuf[ii] = (__u8)PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - txbuf[ii] = (__u8)PyLong_AS_LONG(val); - } else { - snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); - PyErr_SetString(PyExc_TypeError, wrmsg_text); - free(txbuf); - free(rxbuf); - Py_DECREF(rx_tuple); - Py_DECREF(seq); - return NULL; - } + } else { + snprintf(wrmsg_text, sizeof (wrmsg_text) - 1, wrmsg_val, val); + PyErr_SetString(PyExc_TypeError, wrmsg_text); + free(txbuf); + free(rxbuf); + Py_DECREF(rx_tuple); + Py_DECREF(seq); + return NULL; } } @@ -846,7 +800,7 @@ SpiDev_xfer3(SpiDevObject *self, PyObject *args) return NULL; } for (ii = 0, jj = block_start; ii < block_size; ii++, jj++) { - PyObject *val = PyLong_FromLong((long)rxbuf[ii]); + PyObject *val = PyInt_FromLong((unsigned long)rxbuf[ii]); PyTuple_SetItem(rx_tuple, jj, val); // Steals reference, no need to Py_DECREF(val) } @@ -989,19 +943,12 @@ SpiDev_set_mode(SpiDevObject *self, PyObject *val, void *closure) "Cannot delete attribute"); return -1; } -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { mode = PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - mode = PyLong_AS_LONG(val); - } else { - PyErr_SetString(PyExc_TypeError, - "The mode attribute must be an integer"); - return -1; - } + } else { + PyErr_SetString(PyExc_TypeError, + "The mode attribute must be an integer"); + return -1; } @@ -1186,22 +1133,15 @@ SpiDev_set_bits_per_word(SpiDevObject *self, PyObject *val, void *closure) "Cannot delete attribute"); return -1; } -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { bits = PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - bits = PyLong_AS_LONG(val); - } else { - PyErr_SetString(PyExc_TypeError, - "The bits_per_word attribute must be an integer"); - return -1; - } + } else { + PyErr_SetString(PyExc_TypeError, + "The bits_per_word attribute must be an integer"); + return -1; } - if (bits < 8 || bits > 32) { + if (bits < 8 || bits > 32) { PyErr_SetString(PyExc_TypeError, "invalid bits_per_word (8 to 32)"); return -1; @@ -1235,19 +1175,12 @@ SpiDev_set_max_speed_hz(SpiDevObject *self, PyObject *val, void *closure) "Cannot delete attribute"); return -1; } -#if PY_MAJOR_VERSION < 3 if (PyInt_Check(val)) { max_speed_hz = PyInt_AS_LONG(val); - } else -#endif - { - if (PyLong_Check(val)) { - max_speed_hz = PyLong_AS_LONG(val); - } else { - PyErr_SetString(PyExc_TypeError, - "The max_speed_hz attribute must be an integer"); - return -1; - } + } else { + PyErr_SetString(PyExc_TypeError, + "The max_speed_hz attribute must be an integer"); + return -1; } if (self->max_speed_hz != max_speed_hz) { From 22835dc36bc6a83ded74c324a825e5f5dd6008e5 Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Fri, 29 May 2020 12:40:23 +0100 Subject: [PATCH 2/2] Basic tests to validate spidev functions --- tests/README.md | 15 +++++++++++++++ tests/conftest.py | 12 ++++++++++++ tests/test_spidev.py | 28 ++++++++++++++++++++++++++++ tests/test_xfer.py | 27 +++++++++++++++++++++++++++ tests/test_xfer2.py | 27 +++++++++++++++++++++++++++ tests/test_xfer3.py | 24 ++++++++++++++++++++++++ 6 files changed, 133 insertions(+) create mode 100644 tests/README.md create mode 100644 tests/conftest.py create mode 100644 tests/test_spidev.py create mode 100644 tests/test_xfer.py create mode 100644 tests/test_xfer2.py create mode 100644 tests/test_xfer3.py diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..efbf1c0 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,15 @@ +Python Spidev Tests +=================== + +These tests are intended to validate the behaviour of Spidev's functions and properties. + +They have been run in Python 2.7 and 3 on a Raspberry Pi 4. + +A short jumper wire is required between pins BCM9 (MISO) and BCM10 (MOSI) in order for loopback tests to pass, you must also enable SPI. + +Tests depend upon pytest and can be run against spidev in a virtual environment with: + +``` +python setup.py develop +python -m py.test -v -r wsx +``` diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..3d0c371 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,12 @@ +import sys +import pytest + +@pytest.fixture +def spidev(): + import spidev + dev = spidev.SpiDev(0, 0) + # Slow down SPI bus to ensure a stable result for loopback testing + dev.max_speed_hz = 100000 + yield dev + del sys.modules["spidev"] + diff --git a/tests/test_spidev.py b/tests/test_spidev.py new file mode 100644 index 0000000..063c64b --- /dev/null +++ b/tests/test_spidev.py @@ -0,0 +1,28 @@ +import pytest + +def test_spidev_set_mode(spidev): + spidev.mode = 0 + assert spidev.mode == 0 + spidev.mode = 1 + assert spidev.mode == 1 + +def test_spidev_bits_per_word_raises_typeerror(spidev): + with pytest.raises(TypeError): + spidev.bits_per_word = "1" + try: + bits = long(8) + with pytest.raises(TypeError): + spidev.bits_per_word = bits + except NameError: + pass + +def test_spidev_mode_raises_typeerror(spidev): + with pytest.raises(TypeError): + spidev.mode = "1" + try: + mode = long(1) + with pytest.raises(TypeError): + spidev.mode = mode + except NameError: + pass + diff --git a/tests/test_xfer.py b/tests/test_xfer.py new file mode 100644 index 0000000..1efb385 --- /dev/null +++ b/tests/test_xfer.py @@ -0,0 +1,27 @@ +import pytest + +def test_xfer_loopback(spidev): + result = spidev.xfer([n for n in range(255)]) + assert result == [n for n in range(255)] + +def test_xfer_empty_list_raises_type_error(spidev): + data = [] + with pytest.raises(TypeError): + spidev.xfer(data) + +def test_xfer_oversized_list_raises_overflow_error(spidev): + data = [0 for _ in range(4097)] + with pytest.raises(OverflowError): + spidev.xfer(data) + +def test_xfer_invalid_list_entry_raises_type_error(spidev): + data = ["1"] + with pytest.raises(TypeError): + spidev.xfer(data) + try: + data = [long(1)] + with pytest.raises(TypeError): + spidev.xfer(data) + except NameError: + pass + diff --git a/tests/test_xfer2.py b/tests/test_xfer2.py new file mode 100644 index 0000000..f20f7b5 --- /dev/null +++ b/tests/test_xfer2.py @@ -0,0 +1,27 @@ +import pytest + +def test_xfer2_loopback(spidev): + result = spidev.xfer2([n for n in range(255)]) + assert result == [n for n in range(255)] + +def test_xfer2_empty_list_raises_type_error(spidev): + data = [] + with pytest.raises(TypeError): + spidev.xfer2(data) + +def test_xfer2_oversized_list_raises_overflow_error(spidev): + data = [0 for _ in range(4097)] + with pytest.raises(OverflowError): + spidev.xfer2(data) + +def test_xfer2_invalid_list_entry_raises_type_error(spidev): + data = ["1"] + with pytest.raises(TypeError): + spidev.xfer2(data) + try: + data = [long(1)] + with pytest.raises(TypeError): + spidev.xfer2(data) + except NameError: + pass + diff --git a/tests/test_xfer3.py b/tests/test_xfer3.py new file mode 100644 index 0000000..99cd07c --- /dev/null +++ b/tests/test_xfer3.py @@ -0,0 +1,24 @@ +import pytest + +def test_xfer3_loopback(spidev): + data = [n for n in range(255)] + result = spidev.xfer3(data) + assert len(result) == len(data) + assert list(result) == data + +def test_xfer3_empty_list_raises_type_error(spidev): + data = [] + with pytest.raises(TypeError): + spidev.xfer3(data) + +def test_xfer3_invalid_list_entry_raises_type_error(spidev): + data = ["1"] + with pytest.raises(TypeError): + spidev.xfer3(data) + try: + data = [long(1)] + with pytest.raises(TypeError): + spidev.xfer3(data) + except NameError: + pass +