Add light.adjust service/action#1
Conversation
|
@codex review этого кода, учитывая что в HA эта тема щепетильная и меня будут терроризировать на точность, аккуратность, лишность и тд. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a82bddfe5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review new changes and all other code until you find everything. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbbc2b13bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not call.data["params"]: | ||
| return | ||
|
|
||
| params = process_turn_on_params(hass, light, call.data["params"]) |
There was a problem hiding this comment.
When light.adjust is called on a Home Assistant light group with brightness_step or brightness_step_pct, this line resolves the step against the group entity before LightGroup.async_adjust fans the request out to the currently-on members. Because the group brightness is an aggregate, members at different brightness levels all receive the same absolute brightness instead of being adjusted relative to their own current values (for example, bulbs at 50 and 200 with step 10 both become the group average + 10), unlike targeting those lights directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
well, yes, but this problem exists in light.turn_on also< in current behaviour. So solving it is for another pr
| if params.get(ATTR_BRIGHTNESS) == 0 or params.get(ATTR_WHITE) == 0: | ||
| return |
There was a problem hiding this comment.
When a caller uses light.adjust on an already-on light with brightness: 0, brightness_pct: 0, or a negative brightness step that resolves to 0, process_turn_on_params normalizes the request to ATTR_BRIGHTNESS: 0 and this return exits without calling async_adjust. Since the new service schema exposes 0 as a valid brightness value, slider/action callers can try to dim active lights all the way down and get a silent no-op instead of either turning those active lights off or rejecting the value.
Useful? React with 👍 / 👎.
| entity_ids = [ | ||
| entity_id | ||
| for entity_id in self._entity_ids | ||
| if self.hass.states.is_state(entity_id, STATE_ON) | ||
| ] |
There was a problem hiding this comment.
When a light group contains another HA light group configured with all: true, that child group reports off until every member is on; this filter skips it, so light.adjust on the parent is a no-op even when the child group has currently-on members. Nested light groups are already supported, and the new direct all-group adjust path is intended to reach active members, so parent groups need to forward into child light groups instead of filtering only STATE_ON children.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f013d82210
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| params = process_turn_on_params(hass, light, raw_params) | ||
|
|
||
| if params.get(ATTR_BRIGHTNESS) == 0 or params.get(ATTR_WHITE) == 0: |
There was a problem hiding this comment.
When light.adjust is called with a negative brightness_step/brightness_step_pct and the target set includes any currently-off light, process_turn_on_params() resolves that off light from brightness 0 to brightness 0 and this handler raises before LightEntity.async_adjust() can no-op the off entity. In that mixed-target scenario the whole service call can fail instead of adjusting the on lights and ignoring the off ones, which contradicts the new service contract that off lights are no-ops.
Useful? React with 👍 / 👎.
| color_name: *color_name | ||
| hs_color: *hs_color | ||
| xy_color: *xy_color | ||
| brightness: *brightness |
There was a problem hiding this comment.
The service metadata reuses the turn_on brightness selector, which allows selecting 0, but the new LIGHT_ADJUST_SCHEMA and handler reject zero brightness/white values. Users configuring light.adjust from the actions UI can therefore select a value the backend refuses at runtime; the adjust metadata should use selectors whose minimums match the adjust validation instead of the turn_on aliases.
Useful? React with 👍 / 👎.
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bededb8ee8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not light.is_on and ( | ||
| ATTR_BRIGHTNESS_STEP in raw_params | ||
| or ATTR_BRIGHTNESS_STEP_PCT in raw_params | ||
| ): | ||
| return |
There was a problem hiding this comment.
When light.adjust is called with brightness_step or brightness_step_pct on a light group configured with all: true, the group reports is_on == false whenever only some members are on, so this early return drops the service call before LightGroup.async_adjust can expand the target to the active members. In that partially-on all-mode group scenario, absolute brightness/color adjusts reach the on children, but step-based dim/brighten actions become a no-op for the same active lights.
Useful? React with 👍 / 👎.
| brightness_pct_adjust: &brightness_pct_adjust | ||
| filter: *brightness_support | ||
| selector: | ||
| number: | ||
| min: 1 |
There was a problem hiding this comment.
In the light.turn_on service metadata, this creates a selectable field named brightness_pct_adjust (and brightness_adjust is added similarly below), but the registered LIGHT_TURN_ON_SCHEMA only accepts brightness_pct/brightness. When the action editor exposes one of these new fields for light.turn_on, saving or running that action sends an extra key that the backend rejects instead of turning the light on.
Useful? React with 👍 / 👎.
| ): | ||
| return | ||
|
|
||
| params = process_turn_on_params(hass, light, raw_params) |
There was a problem hiding this comment.
For a light.adjust call that targets a light group with multiple active members at different brightness levels, process_turn_on_params runs against the group entity before LightGroup.async_adjust expands to the children, so brightness_step/brightness_step_pct is converted into one absolute brightness based on the group's averaged brightness. A dim-by-10 action on children at 100 and 50 therefore forwards the same value (about 65) to both instead of adjusting them independently to 90 and 40, unlike targeting those lights directly.
Useful? React with 👍 / 👎.
|
@codex GOD I WISH THIS WILL BE THE LAST TIME!!! |
|
Summary
Testing
|
|
@codex re-check please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 326d0d34bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| data = { | ||
| key: value for key, value in kwargs.items() if key in FORWARDED_ATTRIBUTES |
There was a problem hiding this comment.
When light.adjust is called on a Home Assistant light group with both color_name and brightness_step/brightness_step_pct, the service handler deliberately skips process_turn_on_params so each member can resolve its own step, then this filter drops color_name because it is not in FORWARDED_ATTRIBUTES. The nested light.adjust call therefore receives only the brightness step and silently ignores the requested color, even though the service schema accepts that combination.
Useful? React with 👍 / 👎.
| VALID_BRIGHTNESS_STEP = vol.All(vol.Coerce(int), vol.Clamp(min=-255, max=255)) | ||
| VALID_BRIGHTNESS_STEP_PCT = vol.All(vol.Coerce(float), vol.Clamp(min=-100, max=100)) | ||
| VALID_BRIGHTNESS_ADJUST = vol.All(vol.Coerce(int), vol.Range(min=1, max=255)) | ||
| VALID_BRIGHTNESS_PCT_ADJUST = vol.All(vol.Coerce(float), vol.Range(min=0.001, max=100)) |
There was a problem hiding this comment.
This validator accepts very small positive percentages such as brightness_pct: 0.1, but preprocess_turn_on_alternatives converts percentages with round(255 * pct / 100), so any value below roughly 0.196 becomes brightness: 0 and is later rejected by light.adjust with a service error. Users can submit values allowed by the new schema/UI selector that still fail at runtime; raising the minimum to the first percentage that maps to brightness 1 or clamping after conversion would make the contract consistent.
Useful? React with 👍 / 👎.
| ): | ||
| return | ||
|
|
||
| params = process_turn_on_params(hass, light, raw_params.copy()) |
There was a problem hiding this comment.
When light.adjust targets an all: true light group with only some members on, the group state is off even though LightGroup.async_adjust will still forward the adjustment to the active members. Running process_turn_on_params against that off group makes Profiles.apply_default inject the group or group.all_lights default profile, so a brightness-only adjust can unexpectedly add the default color/transition to the on members.
Useful? React with 👍 / 👎.
| if params.get(ATTR_BRIGHTNESS) == 0 or params.get(ATTR_WHITE) == 0: | ||
| raise HomeAssistantError( | ||
| "light.adjust does not accept zero brightness or white values" | ||
| ) |
There was a problem hiding this comment.
For an on light that does not support brightness, a negative brightness_step/brightness_step_pct is resolved from light.brightness == None to brightness: 0, and this check raises before filter_turn_on_params can drop the unsupported brightness field. This can make light.adjust fail for mixed groups where one active member is ONOFF-only while another member exposes brightness support, instead of ignoring the unsupported step on the ONOFF member.
Useful? React with 👍 / 👎.
|
@codex допушил follow-up на 3 P2 замечания. Что сделал коротко:
Пуш в эту же ветку: Посмотри, пожалуйста, ещё раз. |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55898c6ec5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raw_params = dict(call.data["params"]) | ||
|
|
||
| if light.__class__.async_adjust is not LightEntity.async_adjust: | ||
| params = filter_turn_on_params(light, raw_params) |
There was a problem hiding this comment.
When light.adjust targets a Home Assistant light group whose active members support color but not the exact color space supplied by the service UI (for example an HS-only light and rgb_color), this custom-adjust branch filters the raw params against the group's supported modes before process_turn_on_params can convert RGB to HS/XY/color-temp. The result is that the color field is dropped and the group adjust becomes a no-op, while targeting the same light directly would convert and apply the color.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds a new
light.adjustservice/action to the light domain for attribute-only light changes without turning on lights that are currently off.This is implemented conservatively:
async_adjustonThis PR is paired with the frontend PR that calls
light.adjustfrom group light controls:This proposal shape follows the recent architecture discussion comment:
Why
Recent frontend UX improvements made HA light groups a natural room-level control surface. In that UI, changing brightness or color-related attributes on a partially-on group should adjust the active lights in the group, not wake up members that are currently off.
Using
light.turn_onfor that intent is too broad because it inherently carries a power-on semantic.light.adjustgives the frontend and automations a precise action for "change attributes, but do not turn on currently-off lights".What Changed
light.adjustservice schema and handlerLightEntity.async_adjustLightGroup.async_adjusttarget only currently-on membersstrings.jsonandicons.jsonEdge Cases Resolved
adjust-only selectors fromlight.turn_onmetadata so the action UI does not advertise unsupported fields thereasync_adjust()handlers do not observe shared mutation across group membersbrightness_stepandbrightness_step_pctsemantics when a group fans out to active members with different current brightnessall: truelight groups on adjust-only semantics without waking members that are currently offcolor_namewhenlight.adjustis combined with step-based brightness changes on groupsprocess_turn_on_params()during custom groupasync_adjust()handlingRelation To Frontend
This core change is intentionally paired with a frontend caller change. On its own, this PR provides the backend semantics and service metadata. The companion frontend PR updates group light controls to use
light.adjustwhen the entity is a Home Assistant light group with member entities, while preservinglight.turn_onfor off or fully-off groups.That frontend PR should land together with this one, or after this one.
Testing
services.yamlparse checkgit diff --checktests/components/group/test_light.pytests/components/light/test_init.pyReferences