Raise on missing color mode#162715
Conversation
|
Hey there @felipecrs, 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
|
|
@epenet is the OpenRGB integration your only concern? |
There was a problem hiding this comment.
Pull request overview
This PR rolls back part of #162276 by restoring the ability for LightEntity.color_mode / _attr_color_mode to be None, to avoid an unintended breaking change in default light behavior (as discussed in #162650).
Changes:
- Make
LightEntity._attr_color_modeoptional again and updateLightEntity.color_modereturn type accordingly. - Adjust OpenRGB light attribute update logic to use
Noneas the “unknown/unset” sentinel instead ofColorMode.UNKNOWN. - Update the pylint type-hint enforcement plugin to allow
LightEntity.color_modeto returnNone.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pylint/plugins/hass_enforce_type_hints.py |
Updates type-hint enforcement rules to allow LightEntity.color_mode to be optional. |
homeassistant/components/openrgb/light.py |
Aligns OpenRGB’s internal color mode calculation with the restored None default/semantics. |
homeassistant/components/light/__init__.py |
Restores None as the default/allowed value for _attr_color_mode and color_mode. |
I am not sure. As I have been cleaning up the deprecate features of the light platform, all other failing tests were "intended" (they raised warnings before, and now they fail). However the |
|
As discussed yesterday, I think all light.py need to be reviewed to see if there are more cases than openrgb |
emontnemery
left a comment
There was a problem hiding this comment.
I don't think this is a good change, before #162276 the light's color mode would be deduced by its attributes (brightness, color, and so on) and thus not be None. Now, the color mode will be None as I understand it?
Also, tests should be updated.
As discussed on Discord, I have adjusted this to raise an error if the color_mode is still |
emontnemery
left a comment
There was a problem hiding this comment.
LGTM, thanks @epenet 👍
Proposed change
Partial reversal of #162276
As spotted in #162650, changing the default for
_attr_color_modeis an unintended breaking change for "did I already initialise the color mode"Instead of defaulting to UNKNOWN we raise an exception if the color_mode is not set
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: