-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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
Genius hub #21598
Genius hub #21598
Conversation
@technicalpickles Help please. How to I move this forward? What do I need to do? |
Please, update the documentation and open a PR for it (be sure to create a documentation PR against the 🏷 I am adding the |
Came across the documentation PR. PR's where not linked. Added links to OP's. |
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.
Thanks for the PR. Please strip it down to only include the component and the climate platform. More platforms can come later one platform and PR at a time. Smaller PRs are easier to review and will be merged faster.
See more comments below.
Use geniushub-client package instead of geniushub
Codecov Report
@@ Coverage Diff @@
## dev #21598 +/- ##
======================================
Coverage ? 93.82%
======================================
Files ? 449
Lines ? 36556
Branches ? 0
======================================
Hits ? 34299
Misses ? 2257
Partials ? 0 Continue to review full report at Codecov.
|
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 add a manifest.json file. See other integrations for examples.
…into genius-hub
…into genius-hub
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.
Almost ready.
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.
Nice! Can be merged when build passes.
So we can merge! I think it looks a lot better after the refactor. 🥇 |
Hmm, I'm reading the docs PR. Is it true that host config option doubles as option for api key besides host ip address? That's not a good way of structuring the config options. Each option should only have one type of meaning. |
I wanted the client API to be agnostic of the two sources of data. The client API has this: self._api_v1 = not (username or password)
if self._api_v1: #else is v3
... I really call it a Hub Identifier, the name I can change it if you want, although I guess it would have to be a fresh PR now? Would you accept geniushub:
hub_token: skdjflksjfsljd... ... or ... geniushub:
host: 192.168.0.2
username: my_username
password: my password But how do I do mutually exclusive keys in voluptuous? |
Yes, please open a new PR with these changes. Use We have |
OK, it's 11:30pm here - I will have it done by tomorrow. |
Description:
Added support to connect to Genius hub. This brings in climate controls, switches, TRV and room sensors.
Details about Genius hub can be found here: https://www.geniushub.co.uk/
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8815
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTS
variable ([example][ex-requir]).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices: