Have start and end kwargs respect element size

[tannewt]

The comment says it is buffer[start:end] but it assumed elements were a single byte long. Now it correctly does multibyte elements from array.array.

Fixes #4988

I only compiled it. I'm hoping @dmopalmer and @dmcomm will give it a try.

[dmopalmer]

I am unfamiliar with the workflow here. Is the compiled version (for this unmerged change) in the main downloads directory at
https://adafruit-circuit-python.s3.amazonaws.com/index.html?prefix=bin/vcc_gnd_yd_rp2040/en_US/
and specifically the filename ending 20230111-75241c4.uf2

Or do pull-request generated artifacts go somewhere else?

[Neradoc]

Build artifacts are: go to "checks" (at the top here) > "Build CI" (on the left) and scroll to the list of artifacts.
That should be a direct link to that list: https://github.com/adafruit/circuitpython/actions/runs/3897703093

[dmopalmer]

Summary: Reading into arrays of 16,32 bit values with start=, end= arguments works. Reading into slices of arrays doesn't.

There is also a regression in that the rp2pio.StateMachine() pin arguments: first_{out,in,set,sideside}_pin and jmp_pin are no longer accepted, and must be specified as a Pin. (Of type Pin, rejecting int pin numbers as well).

TypeError: first_set_pin must be of type Pin, not NoneType

Details:
For the test for reading 3 FIFO entries into an array.array of 10 uint32 values: dest array.array('L', [99]*10), starting at istart=2

✓ The previous workaround sm.readinto(dest, start=4*istart, end=4*(istart+n)) which is requesting to read into [8:16] now reads 2 values into [8] and [9] and leaves the last value in the FIFO because it would run off the end of the array.

✓ The correct call: sm.readinto(dest, start=istart, end=istart+n) successfully reads the values into elements[2],[3],[4].

❌ Reading into a slice of the array sm.readinto(dest[istart:istart+n]) pulls three values off the FIFO, but does not put them into the array. Worst case scenario: they are written to other memory locations corrupting the system.

❌ Even if the slice starts at zero sm.readinto(dest[:n]) the data is pulled from the FIFO but not written to the array.

❌ If the slice is shorter than the FIFO sm.readinto(dest[:n-1]) the correct number of data are pulled from the FIFO leaving one behind. (But still not written to the array).

❌ If the full-array slice is used, but the start and end arguments are used, sm.readinto(dest[:], start=istart, end=istart+n) it still doesn't write into the array.

✓ If the array is uint16 dest = array.array('H', [99]*10) and the state machine is set with in_shift_right=False, then it works correctly with sm.readinto(dest, start=istart, end=istart+n)

[tannewt]

Thank you for the thorough testing!

Summary: Reading into arrays of 16,32 bit values with start=, end= arguments works. Reading into slices of arrays doesn't.

Slices of arrays are copies. In order to pre-slice it, you'll need to wrap the array in a memoryview(), which will ignore the type iirc.

There is also a regression in that the rp2pio.StateMachine() pin arguments: first_{out,in,set,sideside}_pin and jmp_pin are no longer accepted, and must be specified as a Pin. (Of type Pin, rejecting int pin numbers as well).

TypeError: first_set_pin must be of type Pin, not NoneType

Thanks for finding this! I've fixed it now. It is a regression from another recent PR #7437

[dhalbert]

Thanks for fixing the None regression! Sorry - oops!

[tannewt]

Here is my test script for SPI.readinto():

import array
import busio
import board
import bitbangio
import time

# Wait for usb
time.sleep(10)

for sck in dir(board):
    if not sck.startswith("G"):
        continue
    sck = getattr(board, sck)
    for miso in dir(board):
        if not miso.startswith("G"):
            continue
        miso = getattr(board, miso)

        try:
            spi = busio.SPI(clock=sck, MISO=miso)
            break
        except ValueError:
            pass

spi.try_lock()
for size in ("b", "B", "h", "H", "i", "I", "l", "L", "q", "Q"):
    a = array.array(size, 10 * [99])
    spi.readinto(a, start=1)
    print(a)
    a = array.array(size, 10 * [99])
    spi.readinto(a, start=2, end=100)
    print(a)
    a = array.array(size, 10 * [99])
    spi.readinto(a, start=-2)
    print(a)
    a = array.array(size, 10 * [99])
    spi.readinto(a, end=-2)
    print(a)

I got lucky finding a bug in readinto. The others would be harder to test.

[dmopalmer]

I have verified on the current version that it works on signed and unsigned 8, 16, and 32 bit destination arrays.
(And agree that it should not 'work' on array.array slices. I was mislead by experience with numpy semantics where a simple slice is a memoryview.)

I also verify that the current version has fixed the regression on the pin arguments.

A feature request #7452 for non-polling read is not part of this merge request, but if enacted would probably be best addressed while this part of the code is still fresh in @tannewt or @dhalbert 's mind.

[dhalbert]

@dmopalmer Thanks for the testing!