adding simple bit and bits for SPI devices

[maholli]

I quickly ported over the I2C bit and bits functionality to SPI devices.

Confirmed working on a handful of SPI devices that expect register interaction with the following format: byte1:starting register address byte2:# of registers minus 1. Note this is different than I2C where you don't need byte2 and can just clock in/out data.

A little high-level reworking of the library could likely integrate I2C and SPI together (if someone was motivated).

[FoamyGuy]

@tannewt I'm working on giving this a try. I put this modified Register library inside of the frozen directory. Then ran make BOARD=circuitplayground_express however I notice in the output that it did not print anything about freezing this library:

FREEZE ../../frozen/Adafruit_CircuitPython_BusDevice 
../../frozen/Adafruit_CircuitPython_CircuitPlayground 
../../frozen/Adafruit_CircuitPython_HID 
../../frozen/Adafruit_CircuitPython_LIS3DH 
../../frozen/Adafruit_CircuitPython_NeoPixel 
../../frozen/Adafruit_CircuitPython_Thermistor

Looking at the list here: https://github.com/adafruit/circuitpython/blob/e6f11947cb053fc4a9da844a3689c6af5881590d/ports/atmel-samd/boards/circuitplayground_express/mpconfigboard.mk#L22 it seems that Adafruit_CircuitPython_Register is not one of the modules that gets frozen into the CPX build.. Am I understanding that correctly?

[dhalbert]

Adafruit_CircuitPython_Register is not one of the modules that gets frozen into the CPX build

That is correct, and it's probably not going to fit, unfortunately.

[FoamyGuy]

Thank you @dhalbert Do you know if there is some sort of testing related to this that is still needed for this PR?

[dhalbert]

The Register library is not used by any of the current CPX frozen libraries, so it doesn't need to be frozen, in, and isn't currently. So this improvement can stand on its own.

[FoamyGuy]

This is passing pylint now. I can try testing it, but I'm not sure which kinds of devices expect this type of interaction. Does anyone know of an example of a device that would need this? @maholli are you able to share which devices you used with it?

[maholli]

@FoamyGuy when I was testing I confirmed spi_bits worked with the TI ADS124S08 24-bit ADC and Bosh BMX160 IMU.

[siddacious]

@FoamyGuy This function in the MAX31856 library reads a bunch of individual bits from a byte of interrupt statuses. It should be fairly straightforward to adapt. We have other libraries that mix Register with manually reading/writing registers, so you should be able to convert just this method to start:

https://github.com/adafruit/Adafruit_CircuitPython_MAX31856/blob/master/adafruit_max31856.py#L225

LMK if you want to take that on and I can provide pointers as need be.

Thanks for this @maholli !

[tannewt]

Ok, no concerns here. Will let @FoamyGuy approve and merge.

[FoamyGuy]

@siddacious I took a try at converting the function you linked to use some of the new classes from this change. I'm not certain if I'm understanding it fully but here is what I came up with:

@property
def fault(self):
    """A dictionary with the status of each fault type where the key is the fault type and the
    value is a bool if the fault is currently active
    ===================   =================================
    Key                   Fault type
    ===================   =================================
    "cj_range"            Cold junction range fault
    "tc_range"            Thermocouple range fault
    "cj_high"             Cold junction high threshold fault
    "cj_low"              Cold junction low threshold fault
    "tc_high"             Thermocouple high threshold fault
    "tc_low"              Thermocouple low threshold fault
    "voltage"             Over/under voltage fault
    "open_tc"             Thermocouple open circuit fault
    ===================   =================================
    """

    return {
        "cj_range":  ROBit(_MAX31856_SR_REG, 7),
        "tc_range": ROBit(_MAX31856_SR_REG, 6),
        "cj_high": ROBit(_MAX31856_SR_REG, 5),
        "cj_low": ROBit(_MAX31856_SR_REG, 4),
        "tc_high": ROBit(_MAX31856_SR_REG, 3),
        "tc_low": ROBit(_MAX31856_SR_REG, 2),
        "voltage": ROBit(_MAX31856_SR_REG, 1),
        "open_tc": ROBit(_MAX31856_SR_REG, 0),
    }

Is this on the right track?

[FoamyGuy]

Would it be possible to test these changes with a BME680 or a PN532? Both of them can use SPI but I don't think they require any special interaction.

If I am understanding correctly I could use the new classes ROBit RWBit etc... to access data within these devices even though it's not required does that sound right?

[FoamyGuy]

@siddacious I think I am in need of a point in the right direction. I was able to get a MAX31856 but I don't think I understand how to use the ROBit class correctly inside of the fault function. I tried code like my post above but now the fault property looks like this:

>>> thermocouple.fault
{'voltage': <ROBit object at 20001fe0>, 'tc_low': <ROBit object at 20001f90>, 
'tc_range': <ROBit object at 20001e90>, 'tc_high': <ROBit object at 20001f40>, 
'cj_range': <ROBit object at 20001e60>, 'cj_high': <ROBit object at 20001ea0>, 
'open_tc': <ROBit object at 20002030>, 'cj_low': <ROBit object at 20001ef0>}

I'm not sure how to properly access the value of these bits once they are created.

[siddacious]

@FoamyGuy close! The ROBits need to go in the body of the class as a peer to the methods. This will add a property-like entity to the class that you can access.

You'll want to do something like:

class MAX31856:
    _cj_range = ROBit(_MAX31856_SR_REG, 7)
    _tc_range = ROBit(_MAX31856_SR_REG, 6)
    # ...more ROBits, __init__, other methods, etc.

    @property
    def fault(self):
        return {
            "cj_range": self._cj_range,
            "tc_range": self._tc_range
            #...other entries
        }

Here's an example of a definition: https://github.com/adafruit/Adafruit_CircuitPython_HTS221/blob/master/adafruit_hts221.py#L144

and usage: https://github.com/adafruit/Adafruit_CircuitPython_HTS221/blob/master/adafruit_hts221.py#L247

Sorry for the delayed response! Feel free to @ me on discord if you have any more q's Thanks for your help :)

[FoamyGuy]

I did some testing on this using some tweaks to the MAX31856 library. Using the debug_spi.py we can see the differences in behavior of these RWBit and RWBits as compared to the unmodified library code.

Unmodified MAX library:

self._write_u8(_MAX31856_CR0_REG, _MAX31856_CR0_OCFAULT0)
# output:
# spi.write: ['0x80', '0x10', '0x0', '0x0']

MAX library modified to use RWBits:

_cr0_reg = RWBits(8, _MAX31856_CR0_REG, 0)

# ... further down inside __init__():

self._cr0_reg = 0x10
# output:
# spi.write: ['0x0', '0x0']
# spi.readinto: ['0x0', '0x3']
# spi.write: ['0x0', '0x10']

Lastly with the MAX library modified to use RWBit as a simple test:

_ocfault0 = RWBit(_MAX31856_CR0_REG, 4)

# ... further down inside __init__():
print(self._ocfault0);
self._ocfault0 = True
print(self._ocfault0);

# output:
# spi.write: ['0x0', '0x0']
# spi.readinto: ['0x0', '0x3']
# False
# spi.write: ['0x0', '0x3']
# spi.readinto: ['0x0', '0x3']
# spi.write: ['0x0', '0x13']
# spi.write: ['0x0', '0x13']
# spi.readinto: ['0x0', '0x3']
# False

[jposada202020]

@tannewt :) This was an interesting reading. What would be the future of this PR? taking all aspects in consideration. what could be done to keep this going?

My only comment:

Board with reduced flash memory: register + bus device + sensor library + user code = it is very constraint.

[tannewt]

@jposada202020 I think the best thing would be to make a new library specifically for register over SPI.

[FoamyGuy]

Closing this for now.

@maholli if you're interest in spinning this off into it's own library/repo for registor over SPI we could add it to the community bundle

[maholli]

Closing this for now.

@maholli if you're interest in spinning this off into it's own library/repo for registor over SPI we could add it to the community bundle

Sounds good. If someone starts this, please ping me.