-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
MCP23XXX Refactor #1560
MCP23XXX Refactor #1560
Conversation
Hey there @SenexCrenshaw, mind taking a look at this pull request as its been labeled with an integration ( |
@jesserockz this is looking good! I want to test it out but time keeps getting away from me. I rolled through the code, though, and I like how you've (re)arranged things. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pr has lots of changes with files with almost the same name and reviewing file changes with just formattings is quite annoying waste of time. I've seen other people already do this and asked him to rollback the format change, I'm ok with a formatter update on the codebase but that will be another PR
Yeah sorry, I just ran |
cd80964
to
c5944d3
Compare
c5944d3
to
2657f78
Compare
Hey there @SenexCrenshaw, mind taking a look at this pull request as its been labeled with an integration ( |
2657f78
to
8c5a894
Compare
* Refactor MCP23XXX classes to consolidate shared code * Update test mcp23xxx pin schemas
Description:
This refactors the
MCP23008
,MCP23017
,MCP23S08
andMCP23S17
to remove duplicate code and inherit from new base classes.It also removes the duplicate pin schemas and consolidates all 4 into a single new schema with key
mcp23xxx
for the component id.Related issue (if applicable): fixes
Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#1028
Checklist:
tests/
folder).If user exposed functionality or configuration variables are added/changed: