RX buffer overwrites the TX buffer in 'xfer2' about py-spidev

[Gadgetoid]

For better or worse, this is intentional- potentially since in-place modification of the TX/RX buffer isn't entirely uncommon in SPI communications. However I didn't write this code originally so I can only rationalize what's there in hindsight.

If you supply your input as a list it will be modified in-place. However if you supply your input as an tuple (you can trivially convert a list to a tuple by just calling rx1 = spi.xfer2(tuple(tx1))) then the tuple will remain unchanged (since they are immutable) and a new list will be returned.

You can see the relevant lines for conversion from a tuple to a new list here:

py-spidev/spidev_module.c

Lines 696 to 699 in a5d82b8

	if (PyTuple_Check(obj)) {
	Py_DECREF(seq);
	seq = PySequence_List(obj);
	}

and back again here:

py-spidev/spidev_module.c

Lines 737 to 741 in a5d82b8

	if (PyTuple_Check(obj)) {
	PyObject *old = seq;
	seq = PySequence_Tuple(seq);
	Py_DECREF(old);
	}

Outside the context of a microcontroller this behavior is weird, surprising, very unpythonic, and somewhat conservative given I suspect most systems running this code could spare some memory for separate TX/RX buffers in all cases.... but it's a widely used library and I'm not convinced it's worth fixing when you can toss a tuple() in there.

Or, in your case, just use:

tx1 = (0xCA, 0xFE, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0xff,
       0xff, 0x00, 0x00, 0xff, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00,
       0x00)

[TheMatt2]

This is a duplicate of #61

Though, having spent a good amount of time debugging due to this behavior, I disagree with the assertion this should be considered a feature and not a bug.

While the input list to xfer() and xfer2() is modified in place, the input to xfer3() is not, making this behavior inconsistent across spidev. None of these functions have anything in their documentation to indicate they modify their input, which seems to be a recipe for a code bug that hides quite well in plain sight.

If you really wanted to copy data to and from SPI without allocating the additional memory, you could use a writebytes() call followed by readbytes() to accomplish the same thing.

While converting the input to a tuple gets the desired behavior, I think this is not a particularly elegant solution in a duck-typed language, where precise typing should not cause a behavioral change. Having read the code, I'm still not sure how the code ends up correctly handling a bytes object.

I think this behavior needs to be fixed, or at least enabled by an inplace flag or similar.
I do not think this undocumented, inconsistent, un-pythonic behavior should be kept as a feature.

Edit: Just wanted to say this library has worked fine overall. Don't want to come off as too negative.