adafruit / Adafruit_CircuitPython_BusDevice

Two helper classes that handle transaction related state for I2C and SPI including locks.
MIT License
108 stars 76 forks source link

Make SPI chip select logic optional or remove it #1

Closed tdicola closed 7 years ago

tdicola commented 7 years ago

Right now the SPI device assumes there's a chip select line and is coded that it's active low. I'd suggest we remove the concept of chip select from this library and leave it up to the library writer or user to toggle CS high/low appropriately (like Arduino's SPI API). There are a couple issues with the current behavior:

I'd say we at least make CS optional and if set to None it's ignored entirely. If we want to keep it around then perhaps add a flag to choose if it should be active high or low. However for a lot of SPI devices you have other digital lines like DC (data/command) that you need to manage so it's messy if the SPI device code owns CS while the library code owns DC and other lines--I'd say the SPI device code here keeps track of SPI bus state (speed, polarity, phase) and the library code explicitly manages all its device control lines as digital IO.

tdicola commented 7 years ago

Closing this one, we talked about it and we'll keep CS active low logic for now. The intent is that this SPI device class represents a device that owns communication of the bus using a context manager. With SPI this implies a CS line of some sort since the clock and data lines are shared. For devices that don't have a CS line they shouldn't use this class and instead lock the SPI bus, etc. themselves.

For active high and other odd devices we can later add support to store the desired CS assertion state (low or high, default low). For now keep it as is since most devices are active low.