Make default dim level configurable in Lutron (#122805)

[cameronr]

Proposed change

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

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] New feature (which adds functionality to an existing integration)
• [ ] Deprecation (breaking change to happen in the future)
• [ ] Breaking change (fix/feature causing existing functionality to break)
• [ ] Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #122805
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] I have followed the perfect PR recommendations
• [x] The code has been formatted using Ruff (ruff format homeassistant tests)
• [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• [ ] Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• [ ] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [ ] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[home-assistant[bot]]

Hey there @cdheiser, @wilburcforce, mind taking a look at this pull request as it has been labeled with an integration (lutron) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of lutron can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign lutron Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[cameronr]

This is what the option looks like

[joostlek]

Why do we need this and why can't we do

Just change any automations to go to a specific dim level instead

[cameronr]

The underlying library has a limitation where it has to have a level to turn on a light. Normally, the integration keeps track of the last level and uses that when turning on a light. The problem is what level to use when turning on a light for the first time after HA starts. The code before this PR picks a hard coded value (255/2, ~50%), this PR makes that value user configurable. For me personally, I usually keep my lights at or near 100% so having them set to 50% whenever I've restarted HA is quite jarring

The level can't just be set in an automation because you usually want to use the last value the light had (i.e. you don't always want to be overriding it). This PR is just to handle the case where HA has restarted and we need to pick a reasonable default in that edge case.

[joostlek]

How does it level a light?

[joostlek]

I checked and we already have this functionality built into home assistant

https://www.home-assistant.io/integrations/light/#default-turn-on-values

The benefit of this method is that you can choose per light how you want it to turn on, instead on on integration level

[cameronr]

Interesting, I didn't know about that feature. I tested it out and, unfortunately, it doesn't do was requested in the original ticket (and what I want). Setting group.all_lights.default will set the brightness every time the light is turned on (both from HA and from a wall switch) not just the first time it's turned on from HA. That means it won't remember any changes to the brightness in subsequent switches on as it will always override with the value from the profile.

Just to be clear, the way the integration works today is:

1. If a light switch is turned on via the wall switch, HA will see the brightness level and remember it
2. If a switch is turned on via HA and it has a remembered brightness level, it uses that brightness level when turning on the light
3. If that switch doesn't have a remembered brightness level (i.e. this is the first time HA is seeing that light being turned on), it has to pick a brightness level to use because the underlying library requires a brightness level when turning on a light. The integration currently picks 255 / 2 as a hard coded value.

This PR is setting a configurable value for the integration to use only in that last case where the integration has never seen that light switched turned on so it doesn't have a brightness level for it. That means the value will only be used the first time HA turns on the light (assuming it wasn't turned on previously by the wall switch). That value isn't used every subsequent time the light is turned on.

[cdheiser]

Just wanted to point out a few things:

1. It's not the library that requires a brightness level, it's Lutron's protocol (which the library integrates with)

2. It's theoretically possible to get the behavior your want, but it would be a bit clunky with the device model that's currently used in HA + Lutron. You can tell lutron to press the button on a dimmer and thus you'd observe whatever behavior would normally happen. But we don't really model the devices in the integration, we model the outputs. Lutron requires you pass a brightness level when you're controlling an output. It's possible to have outputs that aren't directly controlled by a device (e.g. they're part of a scene or other lutron automation), which means if we switched to a device-centric model, users would lose access to outputs.

I believe the current model makes the most sense. You have outputs, and 2 different sets of devices that control those outputs: Physical switches, keypads, etc... from Lutron; and Home Assistant.

What are your thoughts Joost? This bit of code is effectively trying to mimic some behavior of physical Lutron controls in a way that works-around limitations in the integration protocol.

On Fri, Sep 20, 2024 at 2:02 PM Cameron Ring @.***> wrote:

Interesting, I didn't know about that feature. I tested it out and, unfortunately, it doesn't do was requested in the original ticket (and what I want). Setting group.all_lights.default will set the brightness every time the light is turned on (both from HA and from a wall switch) not just the first time it's turned on from HA. That means it won't remember any changes to the brightness in subsequent switches on as it will always override with the value from the profile.

Just to be clear, the way the integration works today is:

1. If a light switch is turned on via the wall switch, HA will see the brightness level and remember it
2. If a switch is turned on via HA and it has a remembered brightness level, it uses that brightness level when turning on the light
3. If that switch doesn't have a remembered brightness level (i.e. this is the first time HA is seeing that light being turned on), it has to pick a brightness level to use because the underlying library requires a brightness level when turning on a light. The integration currently picks 255 / 2 as a hard coded value.

This PR is setting a configurable value for the integration to use only in that last case where the integration has never seen that light switched turned on so it doesn't have a brightness level for it. That means the value will only be used the first time HA turns on the light (assuming it wasn't turned on previously by the wall switch). That value isn't used every subsequent time the light is turned on.

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/pull/126160#issuecomment-2364612736, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQARWUSHRKPOXBG3HAABBTZXSEOJAVCNFSM6AAAAABOMPDWFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRUGYYTENZTGY . You are receiving this because you were mentioned.Message ID: @.***>

[cameronr]

That's helpful. I'm not trying to change any model of how the switches/HA work. I just find it really jarring when the lights are set to 50% if i use HA to turn on a switch for the first time following a HA restart. I would be perfectly happy if the hard coded value of 255 / 2 was changed to 255 but I figured that making it configurable was better since at least two people had different thoughts on what the hard coded value should be.

[joostlek]

If this is a workaround for a deficit in the protocol, I think this could be fine. Another option maybe worth exploring is a way to persist the values that are needed to turn on an output, so this only is a problem for first time integration users.

(But I think the main problem is that I don't exactly know how the lutron protocol works and how leveling works and what that exactly means)

For the other questions about device centric or output centric I think I don't have enough info on the lutron protocol to answer what is best here

[joostlek]

The main reason I am being difficult is because ideally we want as little configuration options as possible, so if we can automate this some way, the better. (I also want to avoid adding something for a problem only a few people have, because everyone has different goals (maybe they want light 1 to turn on 100% when unknown and light 2 to turn on 50% when unknown), so hence I am exploring all options

[cdheiser]

@joostlek what's the preferred mechanism for saving the last known state?

On the devices vs. outputs, let me try to clarify.

Since lutron is mostly switches, dimmers, and keypads, there are 2 ways of addressing entities and taking action:

1. Device centric: Press that dimmer button. Flip that swtich. Press that keypad button. This is the way you interact with the system without home assistant. You're basically limited to how the devices have been connected and programmed. A keypad button may turn on 1 light or 50 lights. It's the keypads where you start running into issues where there isn't a 1:1 mapping between devices (switches, keypad buttons, etc...) to loads like lights, fans, or shades. There are some lutron devices that can only be controlled "remotely"

2. Output/Load centric: You're basically bypassing the dimmers, switches, etc... and addressing the loads connected to them directly. So I can dim a light connected to a hybrid keypad even if there's no button on the keypad that lets me control just that light. We cheat a little and pull in keypad buttons in as scenes to give users access to the programmed mechanics of the more advanced devices, but that's about it.

I could get MOST of the way @cameronr wants to go by implementing some fun logic like:

• If you're turning an output on, and we don't have a previous brighness value for it
• Then see if we have a device associated with that output
• If we do have a device associated, then press the device instead of setting the output

That would work for anything connected to a swtich or dimmer. Anything connected to a keypad would be out-of-luck.

Now the really fun part is... where to implement this. If I put it in pylutron (the most obvious place), we'd tweak the API so you can omit the level and do this little dance internally. But what do we do with the other outputs? Do we move the 50% hardcoded default down into pylutron? Do we keep it where it is and add another check that would let HA know if it should pass a default or not?

[joostlek]

If you're turning an output on, and we don't have a previous brighness value for it

So if I understand correctly, an output has a single brightness value. Would it make sense to persist that and retrieve it at boot to restore those values? Would that make the experience more consistent? (Also, where is this brightness saved? pylutron?)

Then see if we have a device associated with that output If we do have a device associated, then press the device instead of setting the output

It sounds difficult and it sounds like a lot of added complexity. I think going this route is a big workaround for this (unless you want to convince me otherwise). So in that case I would rather go the direction of this PR than the whole output device matching direction

[cameronr]

I appreciate how thoughtful you all are being on the best path forward, especially since I had a pretty limited understanding of the larger potential issues at play. Any thoughts on the best path forward now?

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!

[cameronr]

I'd like to keep this open (and my changes are synced to the tip of dev)

[cameronr]

@joostlek I'm ready to commit my changes to work with the updates to OptionFlow but theno-commit-to-branch pre-commit check is failing (I'm assuming since I didn't create a separate branch and it's just called dev). I don't think i can change the source branch for a PR so what's the best way to proceed? I can create a branch and open a new PR but wanted to check that that was the right thing to do first.

Thanks for all your help!