mutable polarity and phase on SPIDevice

[FoamyGuy]

allow phase and polarity to be changed after the SPIDevice has been created.

Something like this is required for the latest approach taken in: adafruit/Adafruit_CircuitPython_SPA06_003#1

If this causes builds to overflow, the added code could be shrunk a little by making a single configure() function that accepts both phase and polarity instead of two separate properties.

[dhalbert]

The current C implementation of SPIDevice doesn't provide these properties, and the current Python implementation makes them public properties (.phase and .polarity) but setting them does nothing after instantiation, since they're just attributes.

Since you can call spi_device.spi.configure() yourself already, is that good enough?

[FoamyGuy]

Since you can call spi_device.spi.configure() yourself already, is that good enough?

This doesn't seem to be possible currently:

Traceback (most recent call last):
  File "code.py", line 65, in <module>
  File "/lib/adafruit_spa06_003.py", line 393, in __init__
  File "/lib/adafruit_spa06_003.py", line 453, in _configure_spi_device
AttributeError: 'SPIDevice' object has no attribute 'spi'

But making spi_device.spi property available would solve my problem as well since I could then call configure() on it.

The crux of this is in the new approach for the driver Scott suggested to have it accept the SPIDevice as init argument. But with the current implementation it means that user code must set the phase and polarity correctly. Making these mutable from python (no matter how) will allow the driver to automatically configure the correct phase and polarity so that the user code doesn't have to worry about that.

In the old approach for drivers the SPI bus was passed in to the driver init and the SPIDevice was created internally within the driver so it was able to set the phase and polarity when it created SPIDevice.

[dhalbert]

But making spi_device.spi property available would solve my problem as well since I could then call configure() on it.

That property is available in the Python version, so I mistakenly thought it was already available in the C version too.

Maybe the phase and polarity should not be available as non-underscore names in the Python library.

I am inclined to say that is the easiest choice. Otherwise we are just reflecting all the SPI properties in SPIDevice.

[tannewt]

So we need native SPIDevice to allow access to .spi?

[FoamyGuy]

@tannewt yep, that is my understanding. Will update this PR with that.

[dhalbert]

I guess a significant issue is that it violates locking, since now anyone holding the SPIDevice can change it while someone else is using it, like a display. But that was true with the properties too, I think?

[tannewt]

Not true with the properties because when entering the context spi.configure() is called. Exposing spi doesn't really help you because spidevice will call configure() again. We could change make the accessor do it but that doesn't make much sense to me either because other non-registered devices lose the auto-configure.

I think the properties are the right way to go because spidevice can still manage the configure.

[dhalbert]

Exposing spi doesn't really help you because spidevice will call configure() again.

I didn't realize it called it again.

Oh, well, never mind, I am wrong. Though the C code is wordy, I think the additional compiled code is actually quite small, because everything is just fetches, and assignments, and the QSTR's exist already.

[FoamyGuy]

closing this based on further discussion in discord.