added light brightness support #100
Conversation
Home Assistant brightness (0-255) is converted to Home Connect brightness (10-100)
|
Hi David, |
| from homeconnect.api import HomeConnectError | ||
|
|
||
| from homeassistant.components.light import LightEntity | ||
| from homeassistant.components.light import (ATTR_BRIGHTNESS, SUPPORT_BRIGHTNESS, LightEntity) |
There was a problem hiding this comment.
Can you please apply isort and then format with black? (This is required for HA Core)
| """Switch the light on / change brightness.""" | ||
| if ATTR_BRIGHTNESS in kwargs: | ||
| _LOGGER.debug("Tried to change brightness for: %s", self.name) | ||
| """ Convert Home Assistant brightness (0-255) to Home Connect brightness (10-100) |
There was a problem hiding this comment.
Can you please modify this doc string to make pydocstyle happy? (This is required for HA core)
| brightness = self.device.appliance.status.get(COOKING_LIGHTINGBRIGHTNESS, {}) | ||
| if not brightness: |
There was a problem hiding this comment.
I think this should be if brightness is None because 0 is a valid value and shouldn't lead to the state being shown as "unavailable".
There was a problem hiding this comment.
Home connect returns a value from 10 up to and including 100 with steps of 1.
0 is never returned. Better to change to if brightness is None for consistency reasons?
There was a problem hiding this comment.
Fair enough, but yes, I would suggest to do it for consistency.
There was a problem hiding this comment.
Updated and committed.
|
Looks great! Just a couple of minor comments above. |
Yeah sorry I somehow missed it. |
|
I implemented AmbientLight control: |
applied isort and formated wit black
Modified doc string
Thanks, shall we wait merging ambient until brightness is merged? |
Change 'if not brightness' into 'if brightness is None'
|
Done? |
Yep |
I loaded up the code in that PR to test, the ambient light (including on/off, colour & brightness) works as expected for a hood, but not for a dishwasher, I was getting the error: There was another issue that the startup time of the home_connect_beta component was about 6 minutes, which is long |
|
Do you have an idea what it was doing in the 6 minutes? |
|
I have 'Waiting on integrations to complete setup: home_connect_beta' entries in the logs, but not much else. I'm increasing the logging of 'homeassistant.bootstrap' to debug and will try again |
@GrumpyMeow, |
Hi @anthonyangel, Could you test #105, |
|
I’m on holiday for 2 weeks and this can’t help at this time with these changes. Sorry. |
Addition to #57.
Home Assistant brightness (0-255) will be converted to Home Connect brightness (10-100) and vice versa.