-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
Bump Intellifire to 4.1.9 #121091
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
Bump Intellifire to 4.1.9 #121091
Conversation
79c66cd
to
b30ad06
Compare
213599f
to
818e6a6
Compare
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
c9f22d6
to
84f5bce
Compare
Hi, my name is Garrett Kranz and I work for Hearth & Home Technologies. We manufacture the fireplaces and control systems that this integration leverages. We had requested these changes from @jeeftor awhile back to reduce stress on our devices and servers. Is there any way you can help expedite the request? @joostlek |
Co-authored-by: Erik Montnemery <erik@montnemery.com>
Co-authored-by: Erik Montnemery <erik@montnemery.com>
Co-authored-by: Erik Montnemery <erik@montnemery.com>
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>
…to intellifire_bump_migrate
28837c2
to
ab1427d
Compare
except LoginException: | ||
errors["base"] = "api_error" | ||
LOGGER.error("Invalid credentials for iftapi.net") | ||
Local control of IntelliFire devices requires that the user download the correct API KEY which is only available on the cloud. Cloud control of the devices requires the user has at least once authenticated against the cloud and a set of cookie variables have been stored locally. |
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.
Please follow PEP8 for docstrings. Lines should be shorter than 72 characters.
Proposed change
I had a previous PR #107479 that was closed due to inactivity. The hold up with that PR was trying to figure out if I should do a migration / minor-migration or no migration. Based on reading through the docs I'm going to use a proper migration - as the new configuration is no longer going to be backwards compatible.
As for the contents of the PR
Reason
In preparation for ultimately adding Cloud support to this integration I need to bump the backing library. I've been in touch with the manufacture and they've given me updated guidance on how to fix the backing library. As such this backing library comes with some additional code changes as well.
Update backing lib
The new backing library uses a
UnifiedFireplace
which has access to both Cloud and Local functions. This PR is only using the local functions but in order to support the new backing-lib we there was some code refactoring that needed to occur.Update to config-flow
Based on discussion with HTT (Home and Hearth technologies) - previous backing library was handling the "login flow" a little wonky. By changing how thats handled - it also gave me the opportunity to simply the config flow.
Sensor updates
Some of the sensors also had to be updated to support the backing-lib changes...
jeeftor/intellifire4py@2.2.2...v4.1.9
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, 10000 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
..coveragerc
.To help with the load of incoming pull requests: