Remove redundant "number" translation key and description
#444
Conversation
Also removes `hasattr` check
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #444 +/- ##
==========================================
- Coverage 96.92% 96.91% -0.01%
==========================================
Files 63 63
Lines 10362 10353 -9
==========================================
- Hits 10043 10034 -9
Misses 319 319 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I totally agree with the |
|
Want me to (re-)move it in a subsequent PR or here? |
|
I'll put it in here. It's mostly just code removal. |
|
Ok, slightly bigger change but IMO for the better: We've previously had a "number" translation key when no fallback name or other translation key was specified.
It gets a bit weird with setting the fallback name and unsetting the translation key (or the other way around, depending on if (Although I did just realize this would require HA changes to work as well, as we persist the fallback name to For the sensor platform, we simply don't provide a translation key by default. So, if there's no translation key value in HA and also no fallback name, it defaults to the
Basically, the number platform is a bit more like the sensor platform now. There should be no changes on the HA side due to us removing the default So, the fallback name is now only set to the description attribute in Also updated the PR description. Let me know what you think. |
"number" translation key and description
Proposed change
This removes redundant calls to set the fallback name to the ZCL description attribute in
on_addof the sensor and number platforms, as this is already done inrecompute_capabilities.Calling this in
recompute_capabilitiesalso makes sure that the device is initialized (i.e. the attributes read once).Furthermore, the default
"number"translation key is also removed for the number platform to align with the sensor platform. Home Assistant will still default to the "Number" (US-EN) translation, same as before, if neither translation key and nor fallback name are provided by ZHA.This allows for a cleaner implementation of setting the fallback name to the description name, without needing to unset the translation key or potentially needing to re-set it to
numberif thedescriptionattribute no longer has a value.(Assuming
recompute_capabilitiescan dynamically be called later at runtime, e.g. after a firmware update.)Additional information
Addresses comments in #197 (review)
Old PR description of initial version (no longer relevant)
Proposed change
This removes the redundant
hasattr(ch, "description")check forAnalogOutputNumberandAnalogInputSensor.For the latter, getting the ZCL
descriptionattribute is also moved into theon_addmethod ofAnalogInputSensor, as it's not needed in the baseSensorclass.The typing for
_attr_translation_keyinAnalogOutputNumberis also fixed to include the option of it beingNoneif thedescriptionattribute has a value.I can also remove the walrus operator again if that's preferred. It's often used in HA code though.
Important
Also, we set the fallback name to the description in
recompute_capabilitiesfor all three platforms supporting thedescriptionattribute (binary sensor, sensor, and number), as well as initially setting it inon_add.For the number and analog sensor platforms, we also unset the translation key in
on_addonly.However,
on_addis called before the cluster handlers are initialized (so attributes read once).If a device is freshly paired, even if a
descriptionattribute is present, the translation key would take priority over the fallback name, sinceon_addis executed before the attribute is read.After a restart of HA, this behavior will change, as the attribute is already cached then.
It also seems a bit redundant to have this logic both in
on_addand partly also inrecompute_capabilities.So, can we just remove it from
on_addand include unsetting the translation key ifdescriptionis present inrecompute_capabilities?