Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for tca95xx i/o extenders. #51

Merged
merged 2 commits into from
Oct 1, 2022
Merged

Conversation

schmidtw
Copy link
Contributor

  1. 👍
  2. n/a
  3. 👍

Things of note

  • I am able to test the TCA9534 and TCA9535 chips using a RPI4. I included chips that were nearly identical as to reduce the likelihood of introducing a bug.
  • I added a conn.Conn interface and implemented that for the extender to allow access to the entire port as a byte.
  • This code is based on the mcp23xxx code pretty heavily.

@maruel
Copy link
Member

maruel commented Sep 29, 2022

Hi! Thanks for the contribution! It's really appreciated.
Would you mind fixing the staticcheck warning? It helps code readability overall.

@schmidtw
Copy link
Contributor Author

Can I add https://github.com/stretchr/testify to help me clean up a bunch of cruft in the unit tests?

@maruel
Copy link
Member

maruel commented Sep 29, 2022

While I understand the value for those who are familiar with this kind of library, a package like https://pkg.go.dev/github.com/stretchr/testify/assert is not what I call a great design. It's so large that it cannot fit in my head, which means that I have to look up every single function when I use it, or try to guess what it does based on the name. Feel free to create a one-off local function in the test file to reduce copy pasting.

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2022

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 70.33898% with 70 lines in your changes missing coverage. Please review.

Project coverage is 64.7%. Comparing base (600c8cc) to head (4c598f2).
Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
tca95xx/pins.go 60.4% 36 Missing and 4 partials ⚠️
tca95xx/tca95xx.go 78.2% 8 Missing and 4 partials ⚠️
tca95xx/variants.go 63.6% 11 Missing and 1 partial ⚠️
tca95xx/registers.go 87.2% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main     #51     +/-   ##
=======================================
+ Coverage   64.4%   64.7%   +0.3%     
=======================================
  Files         56      60      +4     
  Lines       6824    7060    +236     
=======================================
+ Hits        4397    4570    +173     
- Misses      2283    2337     +54     
- Partials     144     153      +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@schmidtw
Copy link
Contributor Author

Is there anything else I need to do before you can merge?

@maruel
Copy link
Member

maruel commented Oct 1, 2022

Thanks a lot for the PR. Apologies for the delay, work was busy this week.

@maruel maruel merged commit a6bfa87 into periph:main Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants