-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
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
Reenable Smarty integration #124148
Reenable Smarty integration #124148
Conversation
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.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
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.
Hey there @z0mbieprocess, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
This reverts commit 639fef3.
Do you have a link to the new library? |
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.
https://github.com/martinssipenko/pysmarty2/blob/master/requirements.txt
Looks like we are pinning modbus, let's not do that and try to set a lower boundary so we can update modbus in HA without having to get the lib bumped at the same time
@joostlek sorry, this was my first day in python :) I've made the changes you requested in the library and released them in |
@joostlek could you please take a look at this? |
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.
It would also be awesome if you could pick up codeowner ship and maybe we can even migrate this integration to the config flow
Please don't merge dev into your branch, now I have to run the CI again and wait longer |
@joostlek I’m okay with codeownership, it it okay if I change that is seperate PR? Regarding config flow - I’ll check out what I can do about it. |
Looks like this integration has some potential to be reworked using the config flow :) But yes, we can do the codeownership in a separate PR |
* Reenable Smarty integration * Updated codeowners to myself * Revert "Updated codeowners to myself" This reverts commit 639fef3. * Upgraded pysmarty2 to version 0.10.1 which is not pinned to specific pymodbus version * Update requirements_all.txt
@martinssipenko can you maybe send me a message on discord? :) |
Breaking change
Proposed change
Type of change
Additional information
This PR addresses/is related to:
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: