Add Rointe integration
Proposed change
This integration provides support for electrical radiators manufactured by Rointe. This is an initial implementation that supports some of Rointe's products:
- D-Series Radiator
- Towel Rails
- Thermostat.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [X] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
The integration uses cloud polling via a REST API owned by Rointe. Config flow requires the user to login to the API and then select which particular installation they want to add to HA.
It's currently available via HACS on https://github.com/tggm/rointe-hacs/
- This PR fixes or closes issue: fixes #
- This PR is related to issue:
- Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/25118
Checklist
- [X] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [X] There is no commented out code in this PR.
- [X] I have followed the development checklist
- [X] The code has been formatted using Black (
black --fast homeassistant tests) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [X] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [X] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [x] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [x] Untested files have been added to
.coveragerc.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Thank you for the first look at this @bachya. It was indeed in a very rough state. I cleaned some of the issue and will ping you when I manage to clear the rest.
This latest commit contains a reauth flow I've been working. It also contains a bunch of debug statements which I added to help another user setup this integration (it's currently available via HACS so I have a couple of people beta testing this).
This latest commit contains a reauth flow I've been working. It also contains a bunch of debug statements which I added to help another user setup this integration (it's currently available via HACS so I have a couple of people beta testing this).
Per my previous comment, I would recommend that you do a reauth flow in a separate PR—this PR already has plenty in it.
Per my previous comment, I would recommend that you do a reauth flow in a separate PR—this PR already has plenty in it.
Will do.
@bachya I cleared up the issues. I changed async_unload_entry a bit and during testing I noticed that when removing the integration and then adding it again the entities do not show up (shown as unavailable). They do however appear after rebooting HASS.
No errors, everything appears OK in logs, both removing and adding again. Maybe async_unload_entry is still not correct
@bachya I fixed the problem with unloading the integration. It was a stupid error where I was using a class variable to store my entities.
I also updated the code to use async_forward_entry_setups() according to this post
Finally I removed some useless code from async_unload_entry that I'd copied from another integration.
Hi @bachya,
Are there any other tasks you want me to work to help with the review?
What are the test coverage targets for files other than config_flow.py ?
What are the test coverage targets for files other than
config_flow.py?
Depends on your objective. Config flow tests are required; (almost) everything else is discretionary.
More info: https://developers.home-assistant.io/docs/integration_quality_scale_index?_highlight=coverage#gold-
What are the test coverage targets for files other than
config_flow.py?Depends on your objective. Config flow tests are required; (almost) everything else is discretionary.
More info: https://developers.home-assistant.io/docs/integration_quality_scale_index?_highlight=coverage#gold-
Thanks. Right now the objective is getting the integration merged (no-score) and then aim for Silver and up.
I'll keep adding tests in the meantime (assuming that adding new test files is OK during the review process).
I'll remove entries from .coveragerc as soon as individual files reach 100%
@bachya Added a couple more tests and fixed minor issues found during tests
Hi @bachya !
Do you have any next steps for me?
I've reviewed what I can and think this is shaping up nicely, but given its size, I will ask for additional review. A reminder that with new integrations, you'll generally have a faster review if the initial contribution contains only one platform (e.g., sensor).
I've reviewed what I can and think this is shaping up nicely, but given its size, I will ask for additional review. A reminder that with new integrations, you'll generally have a faster review if the initial contribution contains only one platform (e.g., sensor).
Thank you for the review @bachya.
I understand that it will take time.
Hi @bachya,
How long do you think it will take for the second opinion review? It has been 2 months :cry:
I don't know, unfortunately. I'm currently busy at a new job, so I'm not as focused in HASS for the time being.
I recommend you go to Discord, go to the #devs_core channel, and see if anyone would be willing to help you push this across the finish line.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
Please strip it down to an implementation of a single platform https://developers.home-assistant.io/docs/review-process Also it has conflicts that needs to be addressed.
I'm sorry, after all the effort and the time this took back and forth in reviews with other people you're are seriously asking me to strip the code down?!
Yes, please remove all but one platform. Downsizing a PR is the fastest way to get it merged. And now you even have a reviewer that has requested changes, which normally means they will merge the PR when all the requested changes are complete.
Thanks for your understanding!
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.
Yes, please remove all but one platform. Downsizing a PR is the fastest way to get it merged. And now you even have a reviewer that has requested changes, which normally means they will merge the PR when all the requested changes are complete.
Thanks for your understanding!
Removed the other platforms (update and sensor).
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!