Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store ZHA light brightness when fading off to turn on at the correct brightness #26680

Merged
merged 6 commits into from Sep 23, 2019

Conversation

@abmantis
Copy link
Contributor

commented Sep 16, 2019

Description:

Previously, if the light is turned off with a time transition, the brightness level stored in the light will be 1. The next time the light is turned on with no explicit brightness, it will be at 1.

This can be solved by storing the current brightness before turning off, and then using that when turning on.

Related issue (if applicable): fixes #26374

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
@abmantis abmantis requested review from Adminiuga and dmulcahey as code owners Sep 16, 2019
@project-bot project-bot bot added this to Needs review in Dev Sep 16, 2019
@Adminiuga

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2019

but the light it self still has the brightness 0/1%. I'm talking about wall switches like 45857GE. If we do transition to Off, then next time you turn on the light from the switch itself, it still would come on at 1%

Also, lights will send level reports every 15 min and it would send level of 1% which would also override the level.

Perhaps for turning off the light with transition, we should set the transition time in OffTransitionTime attribute and just issue Off command. That would take care of properly restore the level. Although i don't know how may times you can write to OffTransitionTime as it is going to wear NVRAM off. My recommendation is to manually set OffTransitiontime attribute and then just issue Off command to the light.

@Adminiuga

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2019

And of course 45857GE does not support off transition time attribute :E But OSRAM works fine with that setup.

@Adminiuga

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2019

What I'm trying to say, the problem is not

@Adminiuga Adminiuga closed this Sep 17, 2019
Dev automation moved this from Needs review to Cancelled Sep 17, 2019
@Adminiuga Adminiuga reopened this Sep 17, 2019
Dev automation moved this from Cancelled to Needs review Sep 17, 2019
@abmantis abmantis changed the title Use light's on_level in ZHA to turn on at the correct brightness Store ZHA light brightness when fading off, to turn on at the correct brightness Sep 18, 2019
@abmantis abmantis changed the title Store ZHA light brightness when fading off, to turn on at the correct brightness Store ZHA light brightness when fading off to turn on at the correct brightness Sep 18, 2019
@abmantis

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2019

Just refactored this to store the brightness on HA.
Turning the light on from a zigbee switch that is directly binded to the light will still show the wrong behavior, unless the user manually sets the on_level on the light. But from HA everything should work as expected.

homeassistant/components/zha/light.py Outdated Show resolved Hide resolved
homeassistant/components/zha/light.py Show resolved Hide resolved
homeassistant/components/zha/light.py Outdated Show resolved Hide resolved
Dev automation moved this from Needs review to Review in progress Sep 19, 2019
abmantis added 5 commits Sep 16, 2019
Previously, if the light is turned off with a time transition, the
brightness level stored in the light will be 1. The next time the light
is turned on with no explicit brightness, it will be at 1.

This is solved by storing the current brightness in on_level before
turning off, and then using that when turning on (by calling the
onOff cluster 'on' command).
@abmantis abmantis force-pushed the abmantis:zha_turn_on_level branch from 611ff57 to adb57de Sep 20, 2019
@abmantis abmantis requested a review from Adminiuga Sep 21, 2019
@dmulcahey

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2019

@abmantis FYI the zha light tests are broken with the last set of changes

Copy link
Contributor

left a comment

Good to be merged.

Dev automation moved this from Review in progress to Reviewer approved Sep 22, 2019
@dmulcahey dmulcahey merged commit 61634d0 into home-assistant:dev Sep 23, 2019
11 checks passed
11 checks passed
CI Build #20190922.62 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing 5cf9ba5...867186f
Details
codecov/project 94.07% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Sep 23, 2019
@lock lock bot locked and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
4 participants
You can’t perform that action at this time.