Make default dim level configurable in Lutron #137127
Conversation
|
Hey there @cdheiser, @wilburCforce, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
@joostlek Here's the updated Lutron default light level PR after changing the source branch name to comply with the git commit checks. Thanks! |
| "step": { | ||
| "init": { | ||
| "data": { | ||
| "default_dimmer_level": "Default light level when first turning on a light from HomeAssistant" |
There was a problem hiding this comment.
"HomeAssistant" -> "Home Assistant"
| @@ -72,7 +73,9 @@ def turn_on(self, **kwargs: Any) -> None: | |||
| if ATTR_BRIGHTNESS in kwargs and self._lutron_device.is_dimmable: | |||
| brightness = kwargs[ATTR_BRIGHTNESS] | |||
| elif self._prev_brightness == 0: | |||
| brightness = 255 / 2 | |||
| brightness = self.platform.config_entry.options.get( | |||
There was a problem hiding this comment.
(oh lol I missed this one).
We shouldn't get the config entry via self.platform, please just pass it as a parameter (or just pass the default dimmer level as parameter)
| default=self.config_entry.options.get( | ||
| CONF_DEFAULT_DIMMER_LEVEL, DEFAULT_DIMMER_LEVEL | ||
| ), | ||
| ): vol.All(vol.Coerce(int), vol.Range(min=1, max=255)), |
There was a problem hiding this comment.
Also, I think you can use a NumberSelector here (not sure how this currently shows in the flow)
There was a problem hiding this comment.
Now looks like this:
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
dc9e211 to
fc6470e
Compare
| config_entry: ConfigEntry, | ||
| ) -> OptionsFlowHandler: | ||
| """Get the options flow for this handler.""" | ||
| return OptionsFlowHandler() |
There was a problem hiding this comment.
Tests are missing. The CI checks for required coverage failed.
There was a problem hiding this comment.
I added a test for the OptionsFlowHandler. Should I open another PR to submit it?
Proposed change
Re-submitting #126160 after changing branch name to pass commit checks.
Make the default dim setting user configurable as requested in issue #122805
On docs, I'll submit a doc PR assuming this change looks ok.
On tests, the existing tests pass but I'm not sure how to update the tests to test options. I was mostly following what ESPHome does and I didn't see any examples of them testing options. I'm happy to add if I can get a pointer on how to test them.
thanks!
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: