-
-
Notifications
You must be signed in to change notification settings - Fork 37.8k
Bring nobo_hub to Bronze quality scale #168638
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
Changes from 12 commits
0216a31
1726e57
4710737
42be8c3
b396320
e9163b0
dd5e24c
e1b104c
5a5de89
19f5056
b4e890f
dbd22a8
7dab8a8
dd5ae3b
a7babcd
7fc920d
dad6336
7faa784
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| rules: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be cool if we can look into more typing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the works - waiting for final review on the upstream library: echoromeo/pynobo#43
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the HVAC mode linked hard to the preset mode?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's explained in the documentation: https://www.home-assistant.io/integrations/nobo_hub/ The zones (normally rooms) operate either on a weekly schedule (auto) or in override mode. 3 different overrides are supported: normal heating, eco heating, away (non-freeze). The overrides are implemented as presets.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we occasionaly update
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The hub does not push preset changes due to weekly schedule, so the climate entity has to be polled once per minute to pick up when the preset changes due to time passing. All other data are pushed from the hub on a TCP socket.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes but what does that do? The docstring mentions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarified docstring. Hope it's OK to fix in this PR :)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the overwrite part for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have some context for whats overwritten?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Global override, I am not sure what that does from the docs
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It overrides current mode for all zones. We could add a link to the mobile app manual if it helps:
joostlek marked this conversation as resolved.
joostlek marked this conversation as resolved.
|
||
| # Bronze | ||
| action-setup: | ||
| status: exempt | ||
| comment: Integration does not register custom actions. | ||
| appropriate-polling: done | ||
| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make sure all tests end in CREATE_ENTRY or ABORT
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in PR #170141 (also updated new tests in other open PRs) |
||
| config-flow: done | ||
| dependency-transparency: done | ||
| docs-actions: | ||
| status: exempt | ||
| comment: Integration does not register custom actions. | ||
| docs-high-level-description: done | ||
| docs-installation-instructions: done | ||
| docs-removal-instructions: done | ||
| entity-event-setup: done | ||
| entity-unique-id: done | ||
| has-entity-name: done | ||
| runtime-data: done | ||
| test-before-configure: done | ||
| test-before-setup: done | ||
| unique-config-entry: done | ||
|
|
||
| # Silver | ||
| action-exceptions: | ||
| status: exempt | ||
| comment: Integration does not register custom actions. | ||
|
oyvindwe marked this conversation as resolved.
Outdated
oyvindwe marked this conversation as resolved.
Outdated
|
||
| config-entry-unloading: done | ||
| docs-configuration-parameters: done | ||
| docs-installation-parameters: done | ||
| entity-unavailable: todo | ||
| integration-owner: done | ||
| log-when-unavailable: todo | ||
| parallel-updates: done | ||
| reauthentication-flow: | ||
| status: exempt | ||
| comment: The hub does not require authentication. | ||
| test-coverage: done | ||
|
oyvindwe marked this conversation as resolved.
Outdated
|
||
|
|
||
| # Gold | ||
| devices: done | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is nobo ecohub the model, nobo sounds like the manufacturer
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manufacturer is Glen Dimplex Nordic AS. Nobø is just a brand. The device is sometimes called "Nobø Hub", and other times "Nobø ECO hub", I guess in case there would be another hub in the Nobø brand (no other have been made though). When connecting, by default it identifies itself with the name "My Eco Hub". Official product page uses "Nobø Hub" - I'm happy to rename! |
||
| diagnostics: todo | ||
| discovery: todo | ||
| discovery-update-info: | ||
| status: exempt | ||
| comment: IP address changes are recovered via UDP rediscovery on connection failure. | ||
| docs-data-update: todo | ||
| docs-examples: todo | ||
| docs-known-limitations: todo | ||
| docs-supported-devices: todo | ||
| docs-supported-functions: todo | ||
| docs-troubleshooting: todo | ||
| docs-use-cases: todo | ||
| dynamic-devices: todo | ||
| entity-category: todo | ||
| entity-device-class: todo | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't use custom device classes, the select has a custom one
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in PR #170135 |
||
| entity-disabled-by-default: todo | ||
| entity-translations: todo | ||
| exception-translations: todo | ||
| icon-translations: todo | ||
| reconfiguration-flow: todo | ||
| repair-issues: | ||
| status: exempt | ||
| comment: Integration has no repair scenarios. | ||
| stale-devices: todo | ||
|
|
||
| # Platinum | ||
| async-dependency: todo | ||
| inject-websession: | ||
| status: exempt | ||
| comment: Integration uses a local TCP socket (via pynobo); no HTTP client is used. | ||
| strict-typing: todo | ||
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.
_attr_target_temperature_step = 1could useSTEP_WHOLE, just a nitThere 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.
Did you mean
PRECISION_WHOLEor as a local constant?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.
yes, precision
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.
Done in this PR