Skip to content

Conversation

FoamyGuy
Copy link
Contributor

This relies on the changes from: adafruit/Adafruit_CircuitPython_Register#59 So that should be merged before this is.

Adds SPI support by using new functionality pending addition to adafruit_register

This should be a major version bump if merged because the class name changed from the bus specific SPA06_003_I2C to the now generic SPA06_003.

Tested successfully with both SPI and I2C on Adafruit CircuitPython 10.0.0-beta.3 on 2025-08-29; Adafruit Feather RP2040 with rp2040

@FoamyGuy FoamyGuy marked this pull request as ready for review October 2, 2025 17:17
@FoamyGuy FoamyGuy requested a review from a team October 2, 2025 17:17
@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Oct 2, 2025

Note: this should bump to 2.0.0 when released because the class name change from SPA06_003_I2C -> SPA06_003 will cause old code using new library to break.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about what the new constructor args should be.

You could also make a function that looks like the old class SPA06_003_I2C() and does the I2CDevice wrapping. It should probably have a deprecation warning though too.

Comment on lines 393 to 396
i2c: I2C = None,
address: int = SPA06_003_DEFAULT_ADDR,
spi: SPI = None,
cs: DigitalInOut = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should take in the device now. As-is it's this weird if i2c then do these two kwargs, otherwise do these two others. The other option would be separate class method constructors but I think providing device is probably clearest. Then internally, you can check the type and create the correct accessor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this to take the bus device object in the constructor with the latest commit. I am thinking some separate class methods would be pretty convenient though. With this current version the user code must concern itself with I2C Address, and SPI polarity/phase. Before, these had default values provided by the driver so user didn't have to provide them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have a "device" kwarg that can be an I2C, I2CDevice (for non default address) or SPIDevice. What do you think? I agree the example is a bit weird now needing to import the default address.

Polarity and phase could be handled by a configure method or with mutable properties on SPIDevice. Chip select makes sense to have the user code worry about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that approach sounds good. I started the changes without realizing the SPIDevice doesn't yet allow changing phase, polarity, or baudrate. I'll try to make those mutable in the core.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened adafruit/circuitpython#10658 making phase and polarity mutable on SPIDevice, and use those new properties in the latest commit here to configure the SPIDevice internally within the driver instead of relying on user code.

The latest commit also allows I2C bus in addition to I2CDevice as the argument which will cause it to initialize with the default I2C address.

@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Oct 8, 2025

The latest commit has over_spi(spi, cs) and over_i2c(i2c, address) functions that can be used to create the driver using the specified bus type. Examples are updated to use those new functions.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple minor things. Definitely getting close. Thanks!

time.sleep(0.01)


class SPA06_003_I2C(SPA06_003):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need a class? Or can this be a function?

Also, please use the warnings module. I believe most CP builds will have it.

Comment on lines +419 to +420
except ValueError:
raise ValueError(f"No SPI device found.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why catch this? Isn't it what gets thrown anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants