From be816c0ecf38fe848a87f05eadcf4ed9fc423f28 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Wed, 4 Dec 2024 23:10:14 +0000 Subject: [PATCH 1/4] Add quality scale and initial work --- .../components/heos/quality_scale.yaml | 78 +++++++++++++++++++ script/hassfest/quality_scale.py | 1 - 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 homeassistant/components/heos/quality_scale.yaml diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml new file mode 100644 index 0000000000000..1e3b33ce33c34 --- /dev/null +++ b/homeassistant/components/heos/quality_scale.yaml @@ -0,0 +1,78 @@ +rules: + # Bronze + action-setup: done + appropriate-polling: + status: done + comment: Integration is a local push integration + brands: done + common-modules: todo + config-flow-test-coverage: done + config-flow: done + dependency-transparency: done + docs-actions: done + docs-high-level-description: done + docs-installation-instructions: done + docs-removal-instructions: todo + entity-event-setup: done + entity-unique-id: done + has-entity-name: done + runtime-data: done + test-before-configure: todo + test-before-setup: done + unique-config-entry: + status: exempt + comment: Only a single config entry is supported. + # Silver + action-exceptions: todo + config-entry-unloading: done + docs-configuration-parameters: + status: done + comment: | + The integration doesn't provide any additional configuration parameters. + docs-installation-parameters: done + entity-unavailable: done + integration-owner: done + log-when-unavailable: + status: todo + comment: | + The integration currently spams the logs until reconnected + parallel-updates: + status: todo + comment: Needs to be set to 0. The underlying library handles parallel updates. + reauthentication-flow: + status: exempt + comment: | + This integration doesn't require re-authentication. + test-coverage: done + # Gold + devices: done + diagnostics: todo + discovery-update-info: + status: todo + comment: Explore if this is possible. + discovery: done + docs-data-update: todo + docs-examples: todo + docs-known-limitations: todo + docs-supported-devices: todo + docs-supported-functions: done + docs-troubleshooting: + status: todo + comment: Has some troublehsooting setps, but needs to be improved + docs-use-cases: done + dynamic-devices: todo + entity-category: done + entity-device-class: done + entity-disabled-by-default: done + entity-translations: done + exception-translations: todo + icon-translations: done + reconfiguration-flow: todo + repair-issues: todo + stale-devices: todo + # Platinum + async-dependency: done + inject-websession: + status: done + comment: The integration does not use websession + strict-typing: todo diff --git a/script/hassfest/quality_scale.py b/script/hassfest/quality_scale.py index e16d7d095b921..20b775c696932 100644 --- a/script/hassfest/quality_scale.py +++ b/script/hassfest/quality_scale.py @@ -482,7 +482,6 @@ class Rule: "hddtemp", "hdmi_cec", "heatmiser", - "heos", "here_travel_time", "hikvision", "hikvisioncam", From 30e93fe5798633c48ee8b5f0a06e5a1b747a8cc3 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:13:51 +0000 Subject: [PATCH 2/4] Update scale per comment --- homeassistant/components/heos/quality_scale.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml index 1e3b33ce33c34..9217a2b4ea6de 100644 --- a/homeassistant/components/heos/quality_scale.yaml +++ b/homeassistant/components/heos/quality_scale.yaml @@ -19,9 +19,7 @@ rules: runtime-data: done test-before-configure: todo test-before-setup: done - unique-config-entry: - status: exempt - comment: Only a single config entry is supported. + unique-config-entry: todo # Silver action-exceptions: todo config-entry-unloading: done From fdb209bf41cafa5502f7ae9d23f53240ba0c4a05 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:41:56 +0000 Subject: [PATCH 3/4] Add comment for why single entry supported. --- homeassistant/components/heos/quality_scale.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml index 9217a2b4ea6de..5fcd7b25c8b49 100644 --- a/homeassistant/components/heos/quality_scale.yaml +++ b/homeassistant/components/heos/quality_scale.yaml @@ -19,7 +19,13 @@ rules: runtime-data: done test-before-configure: todo test-before-setup: done - unique-config-entry: todo + unique-config-entry: + status: todo + comment: | + The HEOS integration only supports a single config entry, but needs to be migrated to use + the `single_config_entry` flag. HEOS devices interconnect to each other, so connecting to + a single node yields access to all the devices setup with HEOS on your network. The HEOS API + documentation does not recommend connecting to multiple nodes which would provide no bennefit. # Silver action-exceptions: todo config-entry-unloading: done From 13f40e4cf03b2857f76ebcd4e95dd6b5215955ef Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Thu, 12 Dec 2024 02:28:40 +0000 Subject: [PATCH 4/4] Update scale with comments from review --- .../components/heos/quality_scale.yaml | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/heos/quality_scale.yaml b/homeassistant/components/heos/quality_scale.yaml index 5fcd7b25c8b49..ed9939bf37cbd 100644 --- a/homeassistant/components/heos/quality_scale.yaml +++ b/homeassistant/components/heos/quality_scale.yaml @@ -1,19 +1,36 @@ rules: # Bronze - action-setup: done + action-setup: + status: todo + comment: Future enhancement to move custom actions for login/out into an options flow. appropriate-polling: status: done comment: Integration is a local push integration brands: done common-modules: todo - config-flow-test-coverage: done - config-flow: done + config-flow-test-coverage: + status: todo + comment: + 1. The config flow is 100% covered, however some tests need to let HA create the flow + handler instead of doing it manually in the test. + 2. We should also make sure every test ends in either CREATE_ENTRY or ABORT so we test + that the flow is able to recover from an error. + config-flow: + status: todo + comment: | + 1. YAML import to be removed after core team meeting discussion on approach. + 2. Consider enhnacement to automatically select a host when multiple are discovered. + 3. Move hass.data[heos_discovered_hosts] into hass.data[heos] dependency-transparency: done docs-actions: done docs-high-level-description: done docs-installation-instructions: done docs-removal-instructions: todo - entity-event-setup: done + entity-event-setup: + status: todo + comment: | + Simplify by using async_on_remove instead of keeping track of listeners to remove + later in async_will_remove_from_hass. entity-unique-id: done has-entity-name: done runtime-data: done @@ -27,7 +44,9 @@ rules: a single node yields access to all the devices setup with HEOS on your network. The HEOS API documentation does not recommend connecting to multiple nodes which would provide no bennefit. # Silver - action-exceptions: todo + action-exceptions: + status: todo + comment: Actions currently only log and instead should raise exceptions. config-entry-unloading: done docs-configuration-parameters: status: done @@ -47,9 +66,22 @@ rules: status: exempt comment: | This integration doesn't require re-authentication. - test-coverage: done + test-coverage: + status: todo + comment: | + 1. Integration has >95% coverage, however tests need to be updated to not patch internals. + 2. test_async_setup_entry_connect_failure and test_async_setup_entry_player_failure -> Instead of + calling async_setup_entry directly, rather use hass.config_entries.async_setup and then assert + the config_entry.state is what we expect. + 3. test_unload_entry -> We should use hass.config_entries.async_unload and assert the entry state + 4. Recommend using snapshot in test_state_attributes. + 5. Find a way to avoid using internal dispatcher in test_updates_from_connection_event. # Gold - devices: done + devices: + status: todo + comment: | + The integraiton creates devices, but needs to stringify the id for the device identifier and + also migrate the device. diagnostics: todo discovery-update-info: status: todo