-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
Explicit support for old style (legacy) groups #116664
Conversation
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
self.state_group_mapping: dict[str, SingleStateType] = {} | ||
|
||
@callback | ||
def exclude_domain(self, domain: str) -> None: |
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.
Should we keep this method to not make it a breaking change, and just make it a no op and log a warning that it's deprecated?
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.
There was already a breaking change as we changed the signature for the registry
method (#116317). I do not expect this to break anything. The removal is mentioned in the blog post though.
@@ -16,6 +16,23 @@ | |||
|
|||
from .const import DOMAIN, REG_KEY | |||
|
|||
EXPLICT_SUPPORTED_ON_OFF_DOMAINS = { |
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.
Some of these domains are not documented as supported by the old style groups in the user docs. We should probably add them to the docs list.
https://www.home-assistant.io/integrations/group/#old-style-groups
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.
Why are some of the domains in the user docs list not included in this set, eg alarm_control_panel? Are those the single state types?
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.
These domains explicit register their states and their domain will be added to supported_domains
.
The documentation was updated to the new list already with:
#116318 and home-assistant/home-assistant.io#32574.
The single state types of the combinations of on
/off
, open
/closed
, home
/not_home
and locked
/unlocked
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.
The list only contains the explicit list of domains with the on
/off
single state.
@@ -292,7 +292,7 @@ def _set_tracked(self, entity_ids: Collection[str] | None) -> None: | |||
return | |||
|
|||
registry: GroupIntegrationRegistry = self.hass.data[REG_KEY] | |||
excluded_domains = registry.exclude_domains | |||
supported_domains = registry.supported_domains |
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.
Should we rename the local variable to supported_on_off_domains
to make it more clear what it should contain?
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.
supported_domains
will be extended with the domains the register alternate state (other then on
/off
. This is also the route for custom integrations to make sure they do not break. As far as we know there is one HACS integration that supports legacy groups and the maintainer was notified.
Co-authored-by: Martin Hjelmare <[email protected]>
Closing as there are no plans to explictly exclude (custom)integrations. |
Breaking change
Legacy groups explicit supported means that we do no longer implicit support legacy groups form (custom)domains that and
on
off
state.Only the domains in the documentation are supported.
Proposed change
Instead of allowing any entity in a mixed group, entity platforms must be supported explictly. Custom integrations can stall implement
async_describe_on_off_states
in thegroup.py
module and registeron_off_states
, but we no longer allow new domains to implement support for legacy groups.Or in other words, without this PR mixed groups support:
The PR changes this to:
The open blog post PR home-assistant/developers.home-assistant#2157 was updated to include these changes.
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: