-
Notifications
You must be signed in to change notification settings - Fork 29
Added Ambient light #105
Added Ambient light #105
Conversation
With major support of Meow
_LOGGER.debug("Tried to change brightness for: %s", self.name) | ||
"""Convert Home Assistant brightness (0-255) to Home Connect brightness (10-100)""" | ||
""" Convert Home Assistant brightness (0-255) to Home Connect brightness (10-100)""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This space should be removed; also there is no full stop. Please run pydocstyle
and make sure it is happy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, forgot to make pydocstyle
happy :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi David,
Sorry to react so late, I totally forgot this PR.
Can we finish this PR?
After it has been merged I will merge it in the official HA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
hmm strange, I also forgot about it. So is it ready to merge and all issues have been resolved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, al request have been resolved.
I am not sure if the 5 minute delays mentioned by @anthonyangel is still there. I will try this week to execute some tests to find out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Anthonie, thanks for sharing.
Would it be possible for you to check if the core version of home connect has a different delay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK great, let me know.
Also a heads-up: we should wait with the core PR until DavidMStraub/homeconnect#3 is finally closed (I just pushed a possible fix but have to wait for others testing it.).
I just checked the core version of home connect and I noticed the same delay before entities become available after a restart as seen in the beta version.
I think we can this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge, you mean? 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep merge, auto correct is not always helping 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, I just got a mini-PR merged into HA core fixing the SSE reconnection issue by simply bumping the homeconnect
dependency. I wanted to separate this from the feature PR as it is an important fix. @Sjack-Sch if you want feel free to open a PR at core with the additional changes in this repo.
This takes 5+ minutes to startup, I tested this with appliance in standby / off state, and also turned everything on and checked and it was the same result. Potentially having the appliance which the programs can't be fetched for in the logs would help troubleshoot this further.
After a few minutes more I get:
And then eventually:
The functionality for a hood works as intended (including colour & brightness), but when I try to set the ambient light on a dishwasher I get the following error. I'd assume that this ambient light only has on/off states and doesn't support brightness
|
This looks like it's due to the change requested above. During the (async) setup process, a sync call to the API is started which fails. |
Hi @anthonyangel, I removed the sync call. Could you please test the latest commits? |
Brightness set to none for Ambient gave an error when converting from home assistant brightness to home connect brightness
I checked the latest version of the code, I still get the slow startup for this component. I also checked the dishwasher ambient light and I now get a different error:
|
Hi @anthonyangel, It seems that your dishwasher does support ambient lighting. |
added the word "ambient" to the error notification for ambient light on
When it's of off I get
When the power is on I get the same, but the API reports that the power status is on. It must be one of those settings which is available in the app, but not the API. From my point of view it's not worth worrying about as it's a pretty useless feature (I only brought it up as it was discovered as an entity so I tested it) |
OK thanks, yep it seams that the ambient setting is not available/supported. Maybe Bosch will add this API in the future. |
I've just cloned your "DavidMStraub:master' branch and mapped it into my Home Assistant. |
When Functional Light of Hood will be available in HA integration? |
Good question, "somebody" has to do a pull request. It is very low on my prio list because I don't have a hood light. Happy if someone volunteers. |
I will start a PR that adds functional and ambient lighting to the core soon. |
Thank you so much !!! 👍 |
A PR to HA core has been started: |
With major support of @GrumpyMeow