[BUG]: Broken Implementation of WPILib SPI

[echap1]

Problem description

When trying to use wpilib SPI to read data into a buffer, a segmentation fault occurs inside of the wpilib.SPI.read method. I believe that the bindings are written improperly for this class. The signature of this function is def read(self, initiate: bool, dataReceived: buffer) -> int, and it is supposed to read data from the bus into the dataRecieved buffer. This is how it works in C++ wpilib, except a size parameter that is mentioned in the docstring but not present for some reason. However, this approach doesn't work in python because you can't pass pointers like you do in C++, and I think this function and functions like it (wpilib.SPI.transaction and others) should instead return the buffer.

Operating System

Linux, RoboRIO

Installed Python Packages

attrs                    21.4.0
bcrypt                   3.2.0
CacheControl             0.12.10
cachy                    0.3.0
certifi                  2021.10.8
cffi                     1.15.0
charset-normalizer       2.0.12
cleo                     0.8.1
click                    8.0.4
clikit                   0.6.2
crashtest                0.3.1
cryptography             36.0.1
distlib                  0.3.4
fast_unit                0.5.0
filelock                 3.4.2
html5lib                 1.1
idna                     3.3
importlib-metadata       4.11.0
iniconfig                1.1.1
jeepney                  0.7.1
keyring                  23.5.0
lockfile                 0.12.2
maturin                  0.12.9
msgpack                  1.0.3
numpy                    1.22.2
packaging                21.3
paramiko                 2.9.2
pastel                   0.2.1
pexpect                  4.8.0
Pint                     0.18
pip                      21.3.1
pkginfo                  1.8.2
platformdirs             2.5.0
pluggy                   1.0.0
poetry                   1.1.13
poetry-core              1.0.7
ptyprocess               0.7.0
py                       1.11.0
pycparser                2.21
pydantic                 1.9.0
pyfrc                    2022.0.2
pylev                    1.4.0
PyNaCl                   1.5.0
pynetconsole             2.0.2
pynetworktables          2021.0.0
pyntcore                 2022.4.1.0
pyparsing                3.0.7
pytest                   7.0.1
requests                 2.27.1
requests-toolbelt        0.9.1
robotpy                  2022.0.56
robotpy-commands-v2      2022.4.1.0
robotpy-ctre             2022.1.0
robotpy-hal              2022.4.1.0
robotpy-halsim-gui       2022.4.1.0
robotpy-installer        2022.1.1
robotpy-rev              2022.0.1
robotpy-toolkit-7407     0.3.3
robotpy-wpilib-utilities 2022.0.2
robotpy-wpimath          2022.4.1.0
robotpy-wpiutil          2022.4.1.1
SecretStorage            3.3.1
setuptools               58.3.0
shellingham              1.4.0
six                      1.16.0
toml                     0.10.2
tomli                    2.0.1
tomlkit                  0.9.2
typing_extensions        4.1.1
urllib3                  1.26.8
virtualenv               20.13.1
webencodings             0.5.1
wheel                    0.37.0
wpilib                   2022.4.1.0
zipp                     3.7.0

Reproducible example code

# Set up SPI bus
camera = wpilib.SPI(wpilib.SPI.Port.kOnboardCS0)
camera.setClockRate(CLOCK_RATE)
camera.setClockActiveHigh()

# Create buffer (also tried empty buffer)
recv_data = bytes([0] * 100000)

# Initiate read
self.left_cam.read(True, recv_data)  # ERROR HERE

print(recv_data)

[virtuald]

bytes are readonly in python. I believe if you use a bytearray instead then it will work.

It should give a good error message when you pass in a bytes... what error message do you get when you do that? We can try to make the error better in the future.

[echap1]

I will try that today. I get no type error when invoking the function (but I do when trying to pass something like [] instead of bytes), it just gives me a segmentation fault with a traceback to the read function. Is there any way I can specify the size of the read? Will it just be the size of my bytearray?

[virtuald]

Yes, it should be the size of your bytearray.

I'll see if I can take a look tonight.

[echap1]

The function returns the size of my bytearray, so I think you're right. However, when I try to initiate a read the bytearray is unmodified and the function doesn't error even when no SPI device is connected. Is this intended behavior?

[virtuald]

The binding looks right to me. Here's what robotpy-build autogenerates to bind the function:

.def("read", [](frc::SPI * __that,bool initiate, const py::buffer& dataReceived) {
          int size = 0;
          auto __dataReceived = dataReceived.request(true);
          size = __dataReceived.size * __dataReceived.itemsize;
          auto __ret =__that->Read(initiate, (uint8_t*)__dataReceived.ptr, size );
          return __ret;
        },

Looking at SPI::Read it looks like the return value should be ... something undocumented. According to HAL, it looks like the return value should be whatever ioctl is going to return: ... which is likely going to be the length of data written?

[virtuald]

Also, my understanding is that many SPI devices require you to write something in order to read it, which is why an API like transaction exists. So maybe you're just not telling the device to send any data, so it isn't.

[virtuald]

I'm going to close this as I don't believe there's a bug here, just the error message needs improvement.