Report unavailable entities to google #28501
Conversation
probot-home-assistant
bot
added
core
integration: google_assistant
small-pr
|
Hey there @home-assistant/cloud, mind taking a look at this pull request as its been labeled with a integration ( |
elupus
changed the title
|
|
||
| def convert(mireds): | ||
| if mireds is None: | ||
| return None | ||
| return color_util.color_temperature_mired_to_kelvin(mireds) | ||
|
|
There was a problem hiding this comment.
Somewhat of a workaround. A better solution might be to let the color util return None when input is None.
There was a problem hiding this comment.
Nah, color util shouldn't catch Nones, it should blow up.
|
Hehe. Lol. Yea same pull. I've hopefully fixed the failing tests thou. The color temperature ends up None when unavailable. So we must support that case. |
|
#28228 will break tests on this pull. Needs adjustment once that goes in. |
|
The 3.6 test fail seem unrelated: |
|
Wouldn't we still be hit by home-assistant/architecture#297 ? |
|
Yes, that is why the fix for color conversion is needed. Min/max ends up none. I don't have any light with that synced to cloud, so not sure what GA will do with none values. Now that you say it. Given that it's part of sync call, that trait will likely not work untill next sync. So this will solve it for normal on/off /brightness lights, but not for color setting ones. |
|
So what about we implement the architecture issue first ? |
|
I don't mind that. But this same pull will be valid then too. So not sure why wait. It solves issue for some users now. Not sure when I'd have to time to look at the arch issue. Personally rather get the disconnect stuff in shape first. |
|
Didn't need rebase. Thought brightness was part of sync, but obviously not. I've tried re-triggering test on 3.6. Will se if starts. |
MartinHjelmare
changed the title
|
The thing is that min/max mireds will never be |
Entities should only removed when removed from HA. Removing a temporarily unavailable entity from google causes it to need to re-configured once it become available again.
elupus
force-pushed
the
unavailable_entities
branch
from
228da6d
to
6a976db
Compare
Description:
Entities should only removed when removed from HA. Removing a temporarily unavailable entity from google causes it to need to re-configured once it become available again.
Related issue (if applicable): fixes #28268
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest.requirements_all.txtby runningpython3 -m script.gen_requirements_all..coveragerc.If the code does not interact with devices: