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
Fix missing EntityDescription names in Overkiz #95583
Conversation
Hey there @vlebourl, @tetienne, @nyroDev, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
@@ -119,3 +119,5 @@ def __init__( | |||
# In case of sub device, use the provided label | |||
# and append the name of the type of entity | |||
self._attr_name = f"{self.device.label} {description.name}" | |||
else: | |||
self._attr_name = description.name |
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.
Casting something to string meant that both None
and UndefinedType
(if no name was set) got turned into a string. I removed that statement and now type checking is correctly complaining.
It is better to just set name=None
in the entity descriptions that need it.
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.
I will need to have a better look here... Setting name
to None
in entity description won't work, because we actually want to use the name from the entity description.
There are other cases where we want to inherit the name from the device name, for example for cover entities. Since a recent change in core, it will print an error when we implicit inherit the name from the device. Looking at this at a first glance, it will require more changes to explicit set the name.
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.
I have made a fix for now. However I should have a better look in the coming days (or shortly after the next release), to see if we can rewrite the OverkizEntity
.
Proposed change
The fix in #95238 caused an issue with all entities that use an EntityDescription, this fix should resolve this. Tested on 2 of my accounts and now all entities show up correctly.
(unfortunately we don't have tests for this part, thus I didn't catch it last time).
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: