Record Plugwise Quality Scale #131888

CoMPaTech · 2024-11-29T07:49:34Z

Proposed change

Add initial quality indication as per new rules

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
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

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Plugwise Add update and removal information home-assistant.io#36055 and forthcoming Reorder and apply config suggestions home-assistant.io#36087

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -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.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2024-11-29T07:49:41Z

Hey there @bouwew, @frenck, mind taking a look at this pull request as it has been labeled with an integration (plugwise) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of plugwise can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign plugwise Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

CoMPaTech · 2024-11-29T10:04:19Z

Most dones have been completed for a long time, added the exempts accordingly to my best knowledge

homeassistant/components/plugwise/quality_scale.yaml

joostlek · 2024-12-02T09:15:49Z

homeassistant/components/plugwise/quality_scale.yaml

+  reconfiguration-flow:
+    status: exempt
+    comment: This integration does not have any reconfiguration steps
+  dynamic-devices: done


binary_sensor.py has this logic, but button.py not?

Sorry what do you mean here?
Reboot vs Restart? Remove the _attr_translation_key?

No, this is the unload part, it's in all others but button

Oh, I didn't even spot that one. I think we had that discussion before. Ideally we use the device class name for consistency.

But what I meant here was, binary sensors are added on the fly, but button are not, why is that?

I kept the code in button.py simple because there is only one button defined at the moment, and I don't foresee any more buttons being added in the future.

Also this button is connected to the gateway-device, it will never change.
When this device changes a new Plugwise integration must be added because the ID = pw changes.

Now that I write this, this could be a reason for adding reconfiguring!?

homeassistant/components/plugwise/quality_scale.yaml

joostlek · 2024-12-02T09:20:41Z

homeassistant/components/plugwise/quality_scale.yaml

+  reconfiguration-flow:
+    status: exempt
+    comment: This integration does not have any reconfiguration steps


And why would that make it exempt?

homeassistant/components/plugwise/quality_scale.yaml

joostlek · 2024-12-02T09:39:02Z

homeassistant/components/plugwise/quality_scale.yaml

+  parallel-updates:
+    status: done
+    comment: Using coordinator
+  test-coverage: done


It would be nice to also have tests that check failing calls in the number tests

joostlek · 2024-12-02T09:40:20Z

homeassistant/components/plugwise/quality_scale.yaml

+  parallel-updates:
+    status: done
+    comment: Using coordinator
+  test-coverage: done


test_sensor.py line 161:

Instead of enabling entities on the fly

entity_registry.async_update_entity(entity_id=entity_id, disabled_by=None) await hass.async_block_till_done() await hass.config_entries.async_reload(init_integration.entry_id) await hass.async_block_till_done()

use a fixture:

@pytest.mark.usefixtures("entity_registry_enabled_by_default")

Yes, thanks, there are also some other similar test-improvements on its way...

homeassistant/components/plugwise/quality_scale.yaml

joostlek · 2024-12-02T09:42:00Z

homeassistant/components/plugwise/quality_scale.yaml

+  parallel-updates:
+    status: done
+    comment: Using coordinator


But you have mutable platforms, and they also have a need for parallel-updates

home-assistant · 2024-12-02T09:42:50Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

CoMPaTech · 2024-12-02T10:30:27Z

Updated the quality scale as discussed for benchmark, with follow-up incrementals PRs 'done'-ing the todo's. As already shared in chat, thanks for you thorough review @joostlek, highly appreciated!

joostlek · 2024-12-02T11:04:15Z

homeassistant/components/plugwise/quality_scale.yaml

+    comment: Using coordinator, but required due to mutable platform
+  test-coverage:
+    status: todo
+    comment: Consider using snapshots + consistency in setup calls + add numerical tests + use fixtures


Note that the snapshots was an opinionated comment. Snapshots are a great way to fixate data structures and to see what fixture creates what and what the effect of changes on them are

I interpreted it that way, hence noting as a consideration :)

homeassistant/components/plugwise/quality_scale.yaml

joostlek · 2024-12-02T13:00:02Z

homeassistant/components/plugwise/quality_scale.yaml

+  common-modules:
+    status: todo
+    comment: Verify entity for async_added_to_hass usage (discard?)
+  docs-high-level-description: done


I must say, when I read the docs, I am missing what Plugwise is. I think we should make the docs more accessible for users who are looking to buy a new device for example. We never really explain what plugwise is or what a smile is for example

Will create a new docs PR for this (already spotted it myself as well) - set to todo

joostlek · 2024-12-02T13:03:32Z

homeassistant/components/plugwise/quality_scale.yaml

+    status: todo
+    comment: Verify entity for async_added_to_hass usage (discard?)
+  docs-high-level-description: done
+  docs-installation-instructions: done


I am missing the default config flow setup snippet and I personally think the text is a bit too literal. I think there is room for improvement here.

Linked to PR-in-progress / set to todo

joostlek · 2024-12-02T13:03:38Z

homeassistant/components/plugwise/quality_scale.yaml

+    comment: Verify entity for async_added_to_hass usage (discard?)
+  docs-high-level-description: done
+  docs-installation-instructions: done
+  docs-removal-instructions: done


I'm missing this one

Linked to PR-in-progress / set to todo

joostlek · 2024-12-02T13:04:31Z

homeassistant/components/plugwise/quality_scale.yaml

+    comment: Consider using snapshots + consistency in setup calls + add numerical tests + use fixtures
+  integration-owner: done
+  docs-installation-parameters: done
+  docs-configuration-parameters: done


No options flow, so this would be exempt

joostlek · 2024-12-02T13:04:57Z

homeassistant/components/plugwise/quality_scale.yaml

+    status: todo
+    comment: Consider using snapshots + consistency in setup calls + add numerical tests + use fixtures
+  integration-owner: done
+  docs-installation-parameters: done


It does describe it but it could use the configuration block for it maybe

Linked to PR-in-progress / set to todo

joostlek · 2024-12-02T13:08:10Z

homeassistant/components/plugwise/quality_scale.yaml

+  docs-use-cases: done
+  docs-supported-devices: done
+  docs-supported-functions: done
+  docs-data-update: done
+  docs-known-limitations: done
+  docs-troubleshooting: done
+  docs-examples: done


I am missing a few of these in the docs

All set to todo just for re-checking one by one

Co-authored-by: Joost Lekkerkerker <[email protected]>

joostlek · 2024-12-02T13:55:16Z

homeassistant/components/plugwise/quality_scale.yaml

+  common-modules:
+    status: todo
+    comment: Verify entity for async_added_to_hass usage (discard?)
+  docs-high-level-description: done


Suggested change

docs-high-level-description: done

docs-high-level-description: todo

Updated as future todo

joostlek · 2024-12-02T13:55:22Z

homeassistant/components/plugwise/quality_scale.yaml

+    status: todo
+    comment: Verify entity for async_added_to_hass usage (discard?)
+  docs-high-level-description: done
+  docs-installation-instructions: done


Suggested change

docs-installation-instructions: done

docs-installation-instructions: todo

Docs PR was just closed, but let's leave it as todo to make a process out of this. E.g. set to todo, create small update PR to indicate (with reference PR to docs) that it's done.

home-assistant bot added cla-signed code-quality integration: plugwise labels Nov 29, 2024

home-assistant bot assigned frenck Nov 29, 2024

home-assistant bot added by-code-owner Quality Scale: No score labels Nov 29, 2024

CoMPaTech marked this pull request as ready for review November 29, 2024 10:04

CoMPaTech requested review from frenck and a team as code owners November 29, 2024 10:04

CoMPaTech mentioned this pull request Dec 1, 2024

Reorder and apply config suggestions home-assistant/home-assistant.io#36087

Open

8 tasks

joostlek requested changes Dec 2, 2024

View reviewed changes

home-assistant bot marked this pull request as draft December 2, 2024 09:42

CoMPaTech force-pushed the plugwise_quality branch from 6e6cabe to 624c12a Compare December 2, 2024 11:02

joostlek reviewed Dec 2, 2024

View reviewed changes

homeassistant/components/plugwise/quality_scale.yaml Outdated Show resolved Hide resolved

CoMPaTech marked this pull request as ready for review December 2, 2024 12:50

home-assistant bot requested a review from joostlek December 2, 2024 12:50

joostlek requested changes Dec 2, 2024

View reviewed changes

home-assistant bot marked this pull request as draft December 2, 2024 13:08

joostlek added the quality-scale label Dec 2, 2024

CoMPaTech and others added 5 commits December 2, 2024 14:35

Initial quality scale

6c8b3c1

Mark done through docs PR 36055

3dbe22a

Update accordingly

306620b

Update homeassistant/components/plugwise/quality_scale.yaml

0b55bac

Co-authored-by: Joost Lekkerkerker <[email protected]>

Set docs for re-check, link existing PRs

0bfba76

CoMPaTech force-pushed the plugwise_quality branch from f192f82 to 0bfba76 Compare December 2, 2024 13:36

joostlek reviewed Dec 2, 2024

View reviewed changes

Update status and correct hld+install

64c3ffa

joostlek approved these changes Dec 2, 2024

View reviewed changes

joostlek changed the title ~~Plugwise quality~~ Record Plugwise Quality Scale Dec 2, 2024

joostlek marked this pull request as ready for review December 2, 2024 14:20

home-assistant bot requested a review from joostlek December 2, 2024 14:20

joostlek merged commit 13e9f19 into home-assistant:dev Dec 2, 2024
34 checks passed

CoMPaTech deleted the plugwise_quality branch December 2, 2024 14:28

This was referenced Dec 3, 2024

Plugwise fixes from quality review #132158

Merged

Plugwise Quality improvements #132175

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Record Plugwise Quality Scale #131888

Record Plugwise Quality Scale #131888

CoMPaTech commented Nov 29, 2024 •

edited

Loading

home-assistant bot commented Nov 29, 2024

CoMPaTech commented Nov 29, 2024

joostlek Dec 2, 2024

bouwew Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

bouwew Dec 2, 2024

joostlek Dec 2, 2024

joostlek Dec 2, 2024

joostlek Dec 2, 2024

bouwew Dec 2, 2024

joostlek Dec 2, 2024

home-assistant bot commented Dec 2, 2024

CoMPaTech commented Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024

joostlek Dec 2, 2024

CoMPaTech Dec 2, 2024 •

edited

Loading

	docs-high-level-description: done
	docs-high-level-description: todo

	docs-installation-instructions: done
	docs-installation-instructions: todo

Record Plugwise Quality Scale #131888

Record Plugwise Quality Scale #131888

Conversation

CoMPaTech commented Nov 29, 2024 • edited Loading

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Nov 29, 2024

CoMPaTech commented Nov 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

home-assistant bot commented Dec 2, 2024

CoMPaTech commented Dec 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CoMPaTech Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

CoMPaTech commented Nov 29, 2024 •

edited

Loading

CoMPaTech Dec 2, 2024 •

edited

Loading