Add support for MCP23017 and MCP23S17 #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ess pin support for MCP23S17 since there's currently no way to enable address pin support, we shouldn't offer setting address pins on the SPI variant
Rahix
approved these changes
Mar 30, 2024
There was a problem hiding this comment.
Very nice work, thanks a lot for the contribution :)
I got quite scared when I read that you attempted a driver that supports both SPI and I2C at the same time, but I have to say - you're implementation looks really really good. I'm surprised the design of port-expander didn't get in the way of this. When I originally envisioned it, I only considered I2C support...
Anyway, LGTM 👍
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is based on the work from #2. I have updated the driver to reflect changes made to the codebase since that PR was created, and extended it to work with both the MCP23017 (I2C version) and MCP23S17 (SPI version).
To support both I2C and SPI, I have created an "intermediate" trait and newtypes for the SPI and I2C busses. I didn't implement something like
SpiBusExt(analog toI2cBusExt) since the register reading/writing on this chip is a bit weird and probably shouldn't be generalized. Any feedback on this kind of multi-interface support for a single driver is welcome, as I'm not sure whether this is the best way to do it.The driver has not been tested on real hardware yet, but implements tests and realworld testing should happen in the coming week or two.