Move SPI bit writes to the right clock phase.

[kbsriram]

• Updated all SPI methods that write bits, so they clock the data on the correct clock transition.
• Added tests with a small logic net simulation to verify bits are read and written correctly.

Technically I think the underlying issue could be resolved without this fix, but I also noticed some issues with the current implementation noted in #20 (comment)

The summary is that all the SPI methods that write bits clock it out on the incorrect clock transition. These examples use the following snippet:

 with adafruit_bitbangio.SPI(clock=self.clock_pin, MOSI=self.copi_pin) as spi:
        spi.try_lock()
        spi.configure(baudrate=100, polarity=polarity, phase=phase, bits=8)
        self._enable_net(1)
        spi.write(b"\x55")
        self._enable_net(0)

Currently this produces this waveform for polarity=0 phase=0 :

The controller clocks out the bit on the rising transition when it's also being read. However the code likely also works most of the time as the sequence in which the pins are updated in the code are just enough to change the data line right before the clock transitions.

With this PR, the waveform now clocks out the bits at the right transition, and hopefully is more reliable.

Unfortunately the same situation exists in all modes, and all the write/read-write methods. This is an example when polarity=0 and phase=1 for instance, and data 0x55 and 0xaa:

With this PR, the waveform now shows this for polarity=0 and phase=1 for data 0x55 and 0xaa:

(This PR is large-ish though mostly to do with a test setup that seemed useful to sanity check the fiddly state changes, also in the hope it may be handy to check the I2C code later.)

Resolves #20

[tannewt]

Very cool! One naming request. Mind checking the C implementation of bitbangio too? https://github.com/adafruit/circuitpython/blob/main/shared-module/bitbangio/SPI.c I think this code was based on it.

[kbsriram]

Thanks for reviewing @tannewt !

Re: Pin -> DigitalInOut - just wanted to double-check, as the current library code is expecting a Pin in the constructor and internally creates another DigitalInOut from it. So I ended up faking the Pin class implementation so I could pass it into the constructor, and letting it use the real DigitalInOut (which in tests/CPython would come from blinka of course...) Let me know if you had a different idea in mind? Maybe an alternative is when the API changes to passing in a DigitalInOut instead of a Pin, we could update the test fake then?

Re: comparing bitbangio/SPI.c (just for context you're probably aware the native and python implementations started to diverge in terms of how the bits are clocked out in #30)

That said, I think there are some corner cases that appear to need updates in the native code as well, as far as I could tell. For example, with this test function and monitoring the pins:

import board
import bitbangio
import digitalio
import time


def doit():
    time.sleep(3)
    with digitalio.DigitalInOut(board.GP13) as en:
        en.switch_to_output(value=True)
        time.sleep(1)
        with bitbangio.SPI(clock=board.GP15, MOSI=board.GP14) as spi:
            spi.try_lock()
            spi.configure(baudrate=2, polarity=1, phase=0, bits=8)
            en.switch_to_output(value=False)
            spi.write(b"\xaa")
            en.switch_to_output(value=True)

I see this output:

I think the code implicitly assumes the clock is always in the idle state when the bytes start getting clocked out but for polarity=1 it's not typically the case - and in this situation it ends up essentially missing a clock transition I think, and could cause issues for the first bit. E.g. sigrok correctly identifies the transaction as starting on the enable, as the clock is active at that point. But it doesn't decode the bits.

(The fixed code in this PR for a similar test gives this)

[tannewt]

Re: Pin -> DigitalInOut - just wanted to double-check, as the current library code is expecting a Pin in the constructor and internally creates another DigitalInOut from it. So I ended up faking the Pin class implementation so I could pass it into the constructor, and letting it use the real DigitalInOut (which in tests/CPython would come from blinka of course...) Let me know if you had a different idea in mind? Maybe an alternative is when the API changes to passing in a DigitalInOut instead of a Pin, we could update the test fake then?

Ah, right. Let's leave as-is for now then. We'll want to move both over when we do.

[tannewt]

Thank you!