Allow targeting non-primary entities in conditions#169291
Allow targeting non-primary entities in conditions#169291justanotherariel merged 10 commits intodevfrom
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
|
There was a problem hiding this comment.
Pull request overview
Extends purpose-specific condition evaluation to optionally include non-primary (entity_category) entities when expanding indirect targets (area/device/floor/label), aligning conditions with the earlier trigger changes.
Changes:
- Add a
primary_entities_onlyflag to entity state and numerical condition factories, and thread it into target expansion. - Configure battery conditions (YAML + runtime condition factories) to include non-primary entities in indirect target expansion.
- Update battery condition tests to use diagnostic entities to validate the new behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
homeassistant/helpers/condition.py |
Adds the primary_entities_only flag to entity condition bases and passes it into indirect target expansion. |
homeassistant/components/battery/condition.py |
Instantiates battery conditions with primary_entities_only=False so indirect targets include diagnostic battery entities. |
homeassistant/components/battery/conditions.yaml |
Updates target selector metadata to indicate non-primary entities should be included for battery conditions. |
tests/components/battery/test_condition.py |
Adjusts fixtures to create diagnostic-category entities to ensure indirect target expansion includes them. |
| targeted_entities = async_extract_referenced_entity_ids( | ||
| self._hass, self._target_selection, expand_group=False | ||
| self._hass, | ||
| self._target_selection, | ||
| expand_group=False, | ||
| primary_entities_only=self._primary_entities_only, |
There was a problem hiding this comment.
Pass the same primary_entities_only flag to the target state-change tracker so duration-based conditions track the same expanded entity set that _async_check evaluates.
| diagnostic_id, | ||
| ) = await _create_primary_and_diagnostic_entities_in_area(hass, "test") | ||
|
|
||
| # Primary starts with valid attribute, diagnostic with invalid attribute |
There was a problem hiding this comment.
I think 'valid'/'invalid' is a bit misleading here. Maybe 'matching' and 'non-matching'?
There was a problem hiding this comment.
That follows other tests like test_state_condition_attr_duration_initial_state have, but I agree that its weird. Changed it.
| # If diagnostic is included (primary_entities_only=False), behavior=all fails because | ||
| # the diagnostic entity is off. If excluded, only the primary is checked and it's on. | ||
| assert test(hass) is primary_entities_only | ||
|
|
There was a problem hiding this comment.
Use test.async_check() (or test.async_check(variables=...)) instead of calling the checker as test(hass), since the hass argument is explicitly ignored for backwards compatibility and this call pattern can mislead readers into thinking the passed hass is used.
| # If diagnostic is included (primary_entities_only=False), behavior=all fails because | ||
| # the diagnostic value is below the threshold. If excluded, only the primary is | ||
| # checked and it's above. | ||
| assert test(hass) is primary_entities_only | ||
|
|
||
| # Both above threshold — true regardless of flag | ||
| hass.states.async_set(diagnostic_id, "75") | ||
| await hass.async_block_till_done() | ||
| assert test(hass) is True |
There was a problem hiding this comment.
Use test.async_check() instead of calling the checker as test(hass), since the hass argument is ignored and the callable form is only kept for backwards compatibility.
| assert test(hass) is primary_entities_only | ||
|
|
||
| # 3 more seconds later (6s after diagnostic became matching). Now diagnostic | ||
| # has also been matching for >= 5s → True regardless of flag. | ||
| freezer.tick(timedelta(seconds=3)) | ||
| assert test(hass) is True |
There was a problem hiding this comment.
Use test.async_check() instead of calling the checker as test(hass), since the callable form ignores the provided hass and is kept only for backwards compatibility.
| assert test(hass) is primary_entities_only | |
| # 3 more seconds later (6s after diagnostic became matching). Now diagnostic | |
| # has also been matching for >= 5s → True regardless of flag. | |
| freezer.tick(timedelta(seconds=3)) | |
| assert test(hass) is True | |
| assert await test.async_check() is primary_entities_only | |
| # 3 more seconds later (6s after diagnostic became matching). Now diagnostic | |
| # has also been matching for >= 5s → True regardless of flag. | |
| freezer.tick(timedelta(seconds=3)) | |
| assert await test.async_check() is True |
Proposed change
Follow up to #168857, for conditions.
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.To help with the load of incoming pull requests: