Give fallback_name priority over device class in ZHA #152155
Conversation
epenet
requested review from
Adminiuga,
TheJulianJES,
dmulcahey and
puddly
as code owners
home-assistant
bot
added
cla-signed
code-quality
integration: zha
small-pr
|
Hey there @dmulcahey, @Adminiuga, @puddly, @TheJulianJES, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the ZHA entity naming logic to prioritize fallback names from quirks over device class names. The change ensures that when a device quirk explicitly sets a fallback_name, it takes precedence over auto-generated names from device classes.
- Updates the
nameproperty to check translation key availability before applying fallback names - Removes unused import
UNDEFINEDfrom typing module - Adds logic to only use fallback_name when the translation_key is not found in platform translations
| self._attr_name = meta.fallback_name | ||
| # The fallback name takes priority over the device class name. | ||
| if ( | ||
| (translation_key := self._name_translation_key) |
There was a problem hiding this comment.
Potential AttributeError if self.platform_data is None or doesn't have a platform_translations attribute. Consider adding a guard to ensure these attributes exist before accessing them.
| (translation_key := self._name_translation_key) | |
| (translation_key := self._name_translation_key) | |
| and self.platform_data is not None | |
| and hasattr(self.platform_data, "platform_translations") |
There was a problem hiding this comment.
I don't think this applies to ZHA
There was a problem hiding this comment.
If we want to use the name of the device class, then it is simple to remove the
fallback_namefrom the quirk
The fallback name is required to exist for all v2 quirks to identify what the entity does when just looking at the quirks v2 definition. And it may always be used by other projects that don’t have a project of device classes with names.
We also use it to generate the default US English translations when bumping the ZHA library in HA (if a translation key is provided only).
So, the fallback name should only ever directly be used for custom quirks.
The behavior we have at the moment works fine for built-in quirks and is like this:
- translation key value (if translation key is set)
- device class name
- fallback name, should never happen with built-in quirks, since we enforce setting a
translation_keyordevice_classin v2 quirks. So, this only applies to custom quirks that don’t set a device class. But that’s also where the issue (you’re trying to fix here) lies, since the device class will unexpectedly take precedence over the fallback name, even if a translation key is set (meaning a non-device class name should be used), only for custom quirks.
So, if we want a quirks v2 entity to assume the name of the device class, no translation key should be set. The fallback name will still be available internally, but should not be used (if no translation key is set).
To fix this and keep the current structure in quirks, we‘d need to just use the fallback name if a translation key is provided in the first place AND if no translation was found before. If translation key is provided, the device class should be used instead.
There’s some more explanation on the behavior here (and other stuff): zigpy/zha#527
We should ideally also get some tests in HA that verify the correct behavior for this. I should be able to do that soon.
|
I have't checked this at all and it's just quickly thrown together, but something along these lines might work: @cached_property
def name(self) -> str | UndefinedType | None:
"""Return the name of the entity."""
meta = self.entity_data.entity.info_object
if meta.primary:
self._attr_name = None
return super().name
if meta.translation_key is not None or super().name in (UNDEFINED, None): # revision: changed condition
# if we have a translation for this key, use it
if (
(translation_key := self._name_translation_key) is not None
and translation_key in self.platform_data.platform_translations
):
return super().name
# a translation won't be available for custom quirks, so use fallback_name
if meta.fallback_name is not None:
self._attr_name = meta.fallback_name
return super().name
# we should only reach this for the device class name at this point
return super().nameI should be able to get back to to this in |
Your version breaks the
|
Ah, yeah. Thanks for testing. I did not think of that case for this. The linked zigpy issue for the "forced name" separation from fallback name does have a purpose after all. 😅 When we do that, we can also introduce a bool (or enum?) attr on the ZHA lib entity side that HA can directly use to see which name type has to be used (translation/fallback name OR device class name). As a workaround, until that's all in place and done cleanly on the ZHA side, we might be able to change the condition for the translation/fallback name block to |
I think your condition is wrong, as AFAICT, that still doesn't allow us to set both a custom name AND a device class |
It was added in response to the test failure you reported (listed below) to make sure we don't get Previously, a translation key had to be present for the entity to have the
As long as a I think it should work, but I'll definitely take an actual look in the coming days or weeks at the latest. |
Proposed change
SSIA
If we want to use the name of the device class, then it is simple to remove the
fallback_namefrom the quirkNote I am aware of zigpy/zha#525 and zigpy/zha#533, but I think this change makes sense regardless.
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: