Skip to content

Conversation

@mik-laj
Copy link
Contributor

@mik-laj mik-laj commented Oct 30, 2025

Breaking change

Proposed change

Bronze

  • action-setup - Service actions are registered in async_setup
  • appropriate-polling - If it's a polling integration, set an appropriate polling interval
    SCAN_INTERVAL = timedelta(seconds=10)
    RELEASES_SCAN_INTERVAL = timedelta(hours=3)
    • Websocket
  • brands - Has branding assets available for the integration
  • common-modules - Place common patterns in common modules
    https://github.com/home-assistant/core/blob/dev/homeassistant/components/wled/entity.py
    https://github.com/home-assistant/core/blob/dev/homeassistant/components/wled/helpers.py
  • config-flow-test-coverage - Full test coverage for the config flow
  • config-flow - Integration needs to be able to be set up via the UI
    • Uses data_description to give context to fields
    • Uses ConfigEntry.data and ConfigEntry.options correctly
  • dependency-transparency - Dependency transparency
    https://github.com/frenck/python-wled/releases/tag/v0.21.0
  • docs-actions - The documentation describes the provided service actions that can be used
    No actions
  • docs-high-level-description - The documentation includes a high-level description of the integration brand, product, or service
  • docs-installation-instructions - The documentation provides step-by-step installation instructions for the integration, including, if needed, prerequisites
  • docs-removal-instructions - The documentation provides removal instructions
    Not yet
  • entity-event-setup - Entity events are subscribed in the correct lifecycle methods
    Yes. We use coordinator for that.
  • entity-unique-id - Entities have a unique ID
  • has-entity-name - Entities use has_entity_name = True
  • runtime-data - Use ConfigEntry.runtime_data to store runtime data
    hass.data[WLED_KEY] is used for coordinator that fetches information about GitHub Releases, but it is okey, as we want to avoid fetching the same data multiple times for each device.
  • test-before-configure - Test a connection in the config flow
    try:
    device = await self._async_get_device(user_input[CONF_HOST])
    except WLEDConnectionError:
    errors["base"] = "cannot_connect"
    else:
  • test-before-setup - Check during integration initialization if we are able to set it up correctly.
    We use coordinator for that.
  • unique-config-entry - Don't allow the same device or service to be able to be set up twice
    await self.async_set_unique_id(
    device.info.mac_address, raise_on_progress=False
    )
    self._abort_if_unique_id_configured(
    updates={CONF_HOST: user_input[CONF_HOST]}
    )

Silver

  • action-exceptions - Service actions raise exceptions when encountering failures
    No actions.

  • config-entry-unloading - Support config entry unloading

    async def async_unload_entry(hass: HomeAssistant, entry: WLEDConfigEntry) -> bool:
    """Unload WLED config entry."""
    if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS):
    coordinator = entry.runtime_data
    # Ensure disconnected and cleanup stop sub
    await coordinator.wled.disconnect()
    if coordinator.unsub:
    coordinator.unsub()
    return unload_ok

    Should we also remove release coordinator after last entry removal?

  • docs-configuration-parameters - The documentation describes all integration configuration options

  • docs-installation-parameters - The documentation describes all integration installation parameters
    Only one parameter HOST, but is it undocumented explciite.

  • entity-unavailable - Mark entity unavailable if appropriate
    We use coordinator.

  • integration-owner - Has an integration owner
    Yes.

  • log-when-unavailable - If internet/device/service is unavailable, log once when unavailable and once when back connected
    We use coordinator.

  • parallel-updates - Number of parallel updates is specified

  • reauthentication-flow - Reauthentication needs to be available via the UI
    No auth.

  • test-coverage - Above 95% test coverage for all integration modules
    98%.

Gold

  • devices - The integration creates devices
    def device_info(self) -> DeviceInfo:
    """Return device information about this WLED device."""
    return DeviceInfo(
    connections={
    (CONNECTION_NETWORK_MAC, self.coordinator.data.info.mac_address)
    },
    identifiers={(DOMAIN, self.coordinator.data.info.mac_address)},
    name=self.coordinator.data.info.name,
    manufacturer=self.coordinator.data.info.brand,
    model=self.coordinator.data.info.product,
    sw_version=str(self.coordinator.data.info.version),
    hw_version=self.coordinator.data.info.architecture,
    configuration_url=f"http://{self.coordinator.wled.host}",
    )
  • diagnostics - Implements diagnostics
    https://github.com/home-assistant/core/blob/dev/homeassistant/components/wled/diagnostics.py
  • discovery-update-info - Integration uses discovery info to update network information
  • discovery - Devices can be discovered
  • docs-data-update - The documentation describes how data is updated
  • docs-examples - The documentation provides automation examples the user can use.
  • docs-known-limitations - The documentation describes known limitations of the integration (not to be confused with bugs)
  • docs-supported-devices - The documentation describes known supported / unsupported devices
  • docs-supported-functions - The documentation describes the supported functionality, including entities, and platforms
  • docs-troubleshooting - The documentation provides troubleshooting information
  • docs-use-cases - The documentation describes use cases to illustrate how this integration can be used
  • dynamic-devices - Devices added after integration setup.
    This integration has a fixed single device.
  • entity-category - Entities are assigned an appropriate EntityCategory
  • entity-device-class - Entities use device classes where possible
  • entity-disabled-by-default - Integration disables less popular (or noisy) entities
  • entity-translations - Entities have translated names
  • exception-translations - Exception messages are translatable
  • icon-translations - Entities implement icon translations
  • reconfiguration-flow - Integrations should have a reconfigure flow
  • repair-issues - Repair issues and repair flows are used when user intervention is needed
  • stale-devices - Stale devices are removed
    This integration has a fixed single device.

Platinum

  • async-dependency - Dependency is async
  • inject-websession - The integration dependency supports passing in a websession
  • strict-typing - Strict typing

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:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

Checklist

  • I understand the code I am submitting and can explain how it works.
  • 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.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

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

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:

@home-assistant
Copy link

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

Code owner commands

Code owners of wled 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 wled 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.

"init": {
"data": {
"keep_master_light": "Add 'Main' control even with single LED segment"
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For unknown reason, pytest started reporting the error:

FAILED tests/components/wled/test_config_flow.py::test_options_flow - Failed: Translation not found for wled: `options.step.init.data_description.keep_master_light`. Please add to homeassistant/components/wled/strings.json

data_description should be optional fields, but it is not always true.

Copy link
Contributor

@epenet epenet Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not optional... just because it works without doesn't make them optional and it is a check that get introduced as part of the quality scale checks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I relied on the documentation which says it is optional.
Screenshot 2025-10-31 at 23 11 45
https://developers.home-assistant.io/docs/data_entry_flow_index/#labels--descriptions

Since this is the entrypoint for users to start using an integration, we should make sure that the config flow is very user-friendly and understandable.
This means we should use the right selectors at the right place, validate the input where needed, and use data_description in the strings.json to give context about the input field.

https://github.com/home-assistant/developers.home-assistant/blob/12d3c35acfc873fb8b460ff0e9fdca3ed24d4dda/docs/core/integration-quality-scale/rules/config-flow.md?plain=1#L20-L21

I found another text that describes this, and it also doesn't say it's required explicitly. This could be interpreted as meaning that config flow should be user-friendly, and one way to do this is by using data_description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a PR: #155572

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you also very much for explaining where this error message comes from.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request implements the parallel-updates Silver tier quality scale requirement for the WLED integration. The changes add the PARALLEL_UPDATES constant to platform files that were missing it, bringing the integration into compliance with Silver tier quality scale rules.

  • Adds PARALLEL_UPDATES = 0 to button.py, sensor.py, and update.py platforms
  • Creates quality_scale.yaml tracking file documenting the integration's quality scale compliance status
  • Removes WLED from the exemption list in the quality scale validation script

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
script/hassfest/quality_scale.py Removes WLED from the parallel-updates rule exemption list
homeassistant/components/wled/quality_scale.yaml Adds comprehensive quality scale tracking file with rule status for Bronze, Silver, Gold, and Platinum tiers
homeassistant/components/wled/button.py Adds PARALLEL_UPDATES = 0 constant with explanatory comment
homeassistant/components/wled/sensor.py Adds PARALLEL_UPDATES = 0 constant with explanatory comment
homeassistant/components/wled/update.py Adds PARALLEL_UPDATES = 0 constant with explanatory comment

Comment on lines 15 to 16
# Coordinator is used to centralize the data updates
PARALLEL_UPDATES = 0
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "Coordinator is used to centralize the data updates" doesn't explain why PARALLEL_UPDATES = 0 was chosen. According to the coding guidelines, PARALLEL_UPDATES = 0 means unlimited concurrent updates and is appropriate for coordinator-based platforms. Consider clarifying this in the comment, such as: "# Coordinator-based platform, unlimited parallel updates (PARALLEL_UPDATES = 0)"

Copilot generated this review using guidance from repository custom instructions.
# Coordinator is used to centralize the data updates
PARALLEL_UPDATES = 0
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "Coordinator is used to centralize the data updates" doesn't explain why PARALLEL_UPDATES = 0 was chosen. According to the coding guidelines, PARALLEL_UPDATES = 0 means unlimited concurrent updates and is appropriate for coordinator-based platforms. Consider clarifying this in the comment, such as: "# Coordinator-based platform, unlimited parallel updates (PARALLEL_UPDATES = 0)"

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 20 to 21
# Coordinator is used to centralize the data updates
PARALLEL_UPDATES = 0
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "Coordinator is used to centralize the data updates" doesn't explain why PARALLEL_UPDATES = 0 was chosen. According to the coding guidelines, PARALLEL_UPDATES = 0 means unlimited concurrent updates and is appropriate for coordinator-based platforms. Consider clarifying this in the comment, such as: "# Coordinator-based platform, unlimited parallel updates (PARALLEL_UPDATES = 0)"

Copilot generated this review using guidance from repository custom instructions.
entity-translations: done
exception-translations: todo
icon-translations: done
reconfiguration-flow: todo
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment: |
I see two problems:
- Unsupported WLED version.
- MAC address mismatch e.g. we connect to the device, but
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment: This integration does not have custom service actions.
docs-high-level-description: done
docs-installation-instructions: done
docs-removal-instructions: todo
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged. I bumped quality scale to bronze now.

Copy link
Contributor

@epenet epenet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move all code changes to separate PRs
Keep this PR for ONLY the quality_scale.yaml + quality_scale.py changes

@home-assistant home-assistant bot marked this pull request as draft October 31, 2025 10:40
@home-assistant
Copy link

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.

@mik-laj
Copy link
Contributor Author

mik-laj commented Oct 31, 2025

Please move all code changes to separate PRs

Done.
#155573
#155572
#155571

I'll do a rebase once these changes are merged so I don't have to bother updating the quality-scale.yaml file multiple times as these changes are trivial.

@mik-laj mik-laj force-pushed the wled-quality-scale-2 branch from 87d599d to 354b6bd Compare November 5, 2025 00:18
@mik-laj mik-laj force-pushed the wled-quality-scale-2 branch from 354b6bd to ee28068 Compare November 5, 2025 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants