Skip to content

Fix device temperature entity divisor#843

Draft
puddly wants to merge 2 commits into
zigpy:devfrom
puddly:puddly/fix-device-temp-scaling
Draft

Fix device temperature entity divisor#843
puddly wants to merge 2 commits into
zigpy:devfrom
puddly:puddly/fix-device-temp-scaling

Conversation

@puddly

@puddly puddly commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

We incorrectly treated the device temperature (a signed 16-bit integer, -32,768 to 32,767) as representing a scaled temperature. Strangely, it is unscaled: it represents the temperature in whole degrees C.

This PR removes the 100 divisor. Quirks PR: zigpy/zha-device-handlers#5197

@puddly

puddly commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

This unfortunately presents an issue: the entity will scale incorrectly until the device reports a new value, since we load its state from the ZCL cache that has the 100x scaling applied.

@TheJulianJES TheJulianJES added the bugfix This PR fixes a bug label Jul 22, 2026
@TheJulianJES

Copy link
Copy Markdown
Contributor

This has come up a bit in quirks reviews. I've not had a good idea for how to deal with the old ZCL cache entries.
I do worry a bit about some people potentially having set up (alarm) automations for high device temperatures, which could trigger when restarting HA with this change for a bit, but I'm not sure if people actually do this... But they may still be worried when they see some sensor reporting a really high value (for some time after updating).

We don't have a nice way to (one-time) migrate ZCL state in the DB from quirks but I almost wonder if we should just add a simple check in ZHA for a few releases where we divide the state from the ZCL cache by 100 if its (absolute) value is above 150 (and maybe below 15000 to filter out other incorrectly reported values)?

We'd miss devices that are genuinely -1.5°C to 1.5°C but as this isn't an air temperature sensor, I feel like this should rarely happen? Not sure. I don't like it a lot either. 😅

But we'd at least limit it to the devices with a "feature flag" in the quirk that actually used the multiplication before. So, two device temperature entities in ZHA and only the one that's discovered by the feature flag actually divides the value in that range (for a few releases, after which we just remove it entirely).

It would be better if we could use that workaround for a one time migration though, yet there's no plumbing for that in quirks/zigpy though and we'd also need to persist a non-ZCL value/state that the migration has taken place (if we don't want a complete schema migration just for this fix). (Allowing you to store non-ZCL values in the DB in something that isn't a local/fake attribute is also something we really need to do.)

Let's see if the bot also has an opinion on this...

@puddly

puddly commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

We do keep track of a ZCL attribute's last update time and we have discussed some form of entity versioning in the past. Maybe we can combine the two or delete the ZCL attribute cache, forcing the entity to become unavailable until a new report comes in?

zigpy-review-bot

This comment was marked as resolved.

@TheJulianJES

TheJulianJES commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

We do keep track of a ZCL attribute's last update time and we have discussed some form of entity versioning in the past.

Yeah. Some form of entity versioning would be nice but we'll probably need to have a closer look at that, and I don't think you/Aeotec want to wait for that here? 😄 (And for the last attribute update time, I don't see that being useful here, unless we also save at what time ZHA started with a specific version.)

Maybe we can combine the two or delete the ZCL attribute cache, forcing the entity to become unavailable until a new report comes in?

I don't know, but I'm not a huge fan of making these entities unavailable (if we could also just migrate the data instead). They're only diagnostics entities, so if there's nothing better we can do in the end, I'm fine with it, but unchanged values may take ages to be reported again from end-devices, assuming they even do it at all (Aqara?). And I'm not sure I see how making entities unavailable just once is more easy than just doing the migration for the affected entities in the first place?

I almost wonder if we should just keep the quirks for the existing devices (that multiply the value in cache) but also add a second entity in quirks* that just divides the cached value by 100 again, until we have a better solution to migrate everything properly. That way, there'd be no visible breakage whatsoever, yet all currently unquirked devices would work according to spec then.

*: If the entity is defined in quirks/QuirkBuilder, we'd also need to explicitly disable default discovery for the ZCL entity from ZHA, which I think would be fine in this case, as one helper/builder method could do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This PR fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants