Add unique_id to yeelightsunflower #36311
Conversation
probot-home-assistant
bot
added
bugfix
integration: yeelightsunflower
small-pr
| def hs_color(self): | ||
| """Return the color property.""" | ||
| return self._hs_color | ||
| def rgb_color(self): |
There was a problem hiding this comment.
This isn't correct. A light entity that supports color needs to implement the hs_color property.
There was a problem hiding this comment.
Ah, I didn't know that.
Can you help me understand please. When I look at https://developers.home-assistant.io/docs/core/entity/light I don't see the requirement. (I don't see rgb_color either, but I'm not implementing many of those properties).
It works without it, but I'll make it convert the RGB to HS and implement that method.
| @@ -45,13 +44,19 @@ def __init__(self, light): | |||
| self._available = light.available | |||
| self._brightness = light.brightness | |||
| self._is_on = light.is_on | |||
| self._hs_color = light.rgb_color | |||
| self._rgb_color = light.rgb_color | |||
| self._unique_id = self.name | |||
There was a problem hiding this comment.
We don't need to include sunflower_ in the unique_id. The entity registry is aware of both the integration domain and the platform domain.
What kind of id is the zid?
There was a problem hiding this comment.
It's a 4-digit HEX like 6553 or 287B.
What would you like the unique_id to be instead?
There was a problem hiding this comment.
If the zid is unique and stable, just use that. 👍
There was a problem hiding this comment.
OK. It is unique and stable, so I will.
I had sunflower_ in there to be more descriptive. light.6553 doesn't give as much information, but I guess a user probably knows where their light entities came from :)
MartinHjelmare
changed the title
|
Please don't mix a feature addition and a bug fix in the same PR. Split these tasks into two separate PRs. |
|
Thanks for the review @MartinHjelmare . I did notice that I'd missed the platform in the title and that there were two things; obviously not small enough to combine. |
|
I suggest making a new PR for the color fix and keep this PR for adding unique id. |
lindsaymarkward
changed the title
|
How is this now for just adding the unique_id? |
Proposed change
Add unique_id so that entity_ids can be changed in the UI.
Fix error that occurred because hs_color (2 values) was set to an RGB color list (3 values); replace hs_color with rgb_color (actual lights only implement RGB).
No changes to configuration.yaml needed.
Type of change
Checklist
black --fast homeassistant tests)I have not added any new tests.
The integration reached or maintains the following Integration Quality Scale: