Skip to content

Fix quirks v2 entity non-values#758

Open
TheJulianJES wants to merge 6 commits intozigpy:devfrom
TheJulianJES:tjj/fix-quirks-v2-sensor-non-value
Open

Fix quirks v2 entity non-values#758
TheJulianJES wants to merge 6 commits intozigpy:devfrom
TheJulianJES:tjj/fix-quirks-v2-sensor-non-value

Conversation

@TheJulianJES
Copy link
Copy Markdown
Contributor

@TheJulianJES TheJulianJES commented May 4, 2026

Proposed change

Move super().__init__() in the Sensor class, so _attribute_name is set by the base class before we set _attr_def based on that in the Sensor class.


AI summary

ZCL numeric measurement attributes use a sentinel value to mean "no measurement available" (e.g. 0xFFFF for uint16, -32768 for int16). Sensor._is_non_value is supposed to map those to None so the entity reports as Unknown in HA. For specialized sensor classes (Temperature, Humidity, etc.) this works, but for quirks v2 entities — which use the generic sensor.Sensor class — the sentinel was leaking through as the raw integer state.

Reported via diagnostics on a frient AQSZB-110: the device's humidity sensor (standard RelativeHumidity cluster, specialized Humidity class) correctly reported None when the measurement was unavailable, but its VOC sensor (custom develco_voc_level cluster 0xfc03, generic Sensor class via quirks v2) reported 65535 instead of None.

Root cause

Sensor.__init__ looked up the attribute definition before calling super().__init__():

if self._attribute_name is not None:
    self._attr_def = self._cluster_handler.cluster.find_attribute(self._attribute_name)

super().__init__(cluster_handlers, endpoint, device, **kwargs)

For specialized subclasses, _attribute_name = "measured_value" is a class attribute, so this works. For quirks v2 entities, _attribute_name is set inside _init_from_quirks_metadata, which runs from super().__init__() — i.e. after the lookup. So _attr_def stayed None, and _is_non_value short-circuited to False (no attr def → can't know the type's non_value), letting the sentinel through.

Fix

Move the find_attribute lookup after super().__init__() so _attribute_name is resolved (whether from a class attribute or from quirks v2 metadata) before we use it.

Cleanup

The previous if attr_def is None: return False guard inside _is_non_value is dropped in a follow-up commit — after the init-order fix it's no longer reachable through native_value / formatter (specialized sensors set _attribute_name as a class attribute; quirks v2 entities have it set via _init_from_quirks_metadata; sensors whose attribute isn't on the cluster fail _is_supported or short-circuit at raw_state is None). The guard was the exact mechanism that masked this bug, so removing it makes any future regression surface loudly. An assert attr_def is not None is kept to narrow the type for mypy.

Tests

  • test_ignore_non_value_quirks_v2 — loads the existing tests/data/devices/frient-a-s-aqszb-110.json fixture (the device from the original report), reports 0xFFFF on the develco develco_voc_level cluster, asserts the entity state is None. Mirrors the shape of the adjacent test_ignore_non_value / test_ignore_nan_value. Confirmed to fail without the fix.

For quirks v2 entities, `Sensor._attribute_name` is set by
`_init_from_quirks_metadata`, which runs inside `super().__init__()`.
The attribute definition lookup ran before `super().__init__()`, so
`_attr_def` stayed `None` and `_is_non_value` always returned `False`,
letting ZCL non-value sentinels (e.g. 0xFFFF for uint16) through as
the entity state instead of being mapped to `None`.

Move the `find_attribute` lookup after `super().__init__()` so it
works for both class-attribute defined `_attribute_name` (specialized
sensors like Humidity/Temperature) and quirks v2 entities.
Verifies that a quirks v2 sensor (using the generic `sensor.Sensor`
class with `_attribute_name` set via metadata) maps the ZCL uint16
non-value sentinel (0xFFFF) to `None` rather than reporting it as
the raw state.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.64%. Comparing base (f443146) to head (a738947).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #758      +/-   ##
==========================================
- Coverage   97.64%   97.64%   -0.01%     
==========================================
  Files          62       62              
  Lines       10873    10872       -1     
==========================================
- Hits        10617    10616       -1     
  Misses        256      256              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread zha/application/platforms/sensor/__init__.py
Match the pattern used by `test_ignore_non_value` and
`test_ignore_nan_value` (load a real device via `zigpy_device_from_json`)
instead of building a synthetic quirks v2 entity inline. The fixture
exercises the actual `develco_voc_level` cluster (0xfc03) — the same
device and code path that triggered the original report.
After fixing `Sensor.__init__` to resolve `_attr_def` after
`super().__init__()`, `_attr_def` is always populated whenever the
entity is reachable through `native_value`/`formatter` (specialized
sensors set `_attribute_name` as a class attribute; quirks v2 entities
have it set via `_init_from_quirks_metadata`; sensors whose attribute
isn't on the cluster either fail `_is_supported` or short-circuit in
`native_value` at the `raw_state is None` check).

The remaining `if attr_def is None: return False` guard masked the
exact bug we just fixed by silently letting sentinel values through
instead of detecting them. Replace it with an `assert` so a future
regression surfaces loudly rather than as bogus state.
Comment thread zha/application/platforms/sensor/__init__.py Outdated
@TheJulianJES TheJulianJES marked this pull request as ready for review May 6, 2026 13:48
Copilot AI review requested due to automatic review settings May 6, 2026 13:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes ZHA sensor entities created via quirks v2 so that ZCL numeric “non-value” sentinels (e.g., 0xFFFF for uint16) are mapped to None rather than being exposed as raw integer states.

Changes:

  • Reorders sensor.Sensor.__init__ so quirks v2 metadata initialization runs before resolving the attribute definition used for non-value detection.
  • Adjusts _is_non_value handling and adds a regression test covering a quirks v2 sensor entity path.
  • Adds a new test using a frient AQSZB-110 diagnostics fixture to validate non-value suppression on a quirks v2-created sensor.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
zha/application/platforms/sensor/__init__.py Moves attribute-definition lookup to after super().__init__() and changes non-value detection behavior.
tests/test_sensor.py Adds a regression test ensuring quirks v2 sensor entities ignore ZCL datatype non-values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


if attr_def is None:
return False
assert attr_def is not None
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep the assert.

Comment thread tests/test_sensor.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants