Skip to content

Fix thermostat fan modes to respect fanModeSequence - #708

Open
vmvarga wants to merge 22 commits into
zigpy:devfrom
vmvarga:dev
Open

Fix thermostat fan modes to respect fanModeSequence#708
vmvarga wants to merge 22 commits into
zigpy:devfrom
vmvarga:dev

Conversation

@vmvarga

@vmvarga vmvarga commented Mar 12, 2026

Copy link
Copy Markdown

Summary

  • Fix the Thermostat entity to read the fan_mode_sequence attribute from the ZCL Fan Control cluster (0x0202) instead of hardcoding fan modes to [auto, on]. Devices now correctly expose Low, Medium, High, and Auto fan speeds based on the ZCL spec (Table 6-20).
  • Fix the fan_mode getter to read the actual fan_mode attribute from the Fan Control cluster handler instead of guessing from the thermostat's running_state bitmap.
  • Fix async_set_fan_mode to map all supported fan mode strings (low, medium, high, on, auto) to their corresponding FanMode ZCL enum values.
  • Expose HVACMode.FAN_ONLY as an available HVAC mode when a Fan Control cluster is present on the endpoint, since SEQ_OF_OPERATION (derived from controlSequenceOfOperation) never includes it per ZCL spec.

Problem

The Thermostat entity hardcodes fan modes to [auto, on] and completely ignores the fan_mode_sequence attribute (attribute 0x0001) from the Fan Control cluster. Per ZCL 8 (Table 6-20), FanModeSequenceType defines which fan modes a device supports:

Value Available Modes
0x00 Low, Medium, High
0x01 Low, High
0x02 Low, Medium, High, Auto
0x03 Low, High, Auto
0x04 On, Auto

Devices reporting e.g. fan_mode_sequence = 0x02 (Low/Medium/High/Auto) were stuck with only Auto/On in the UI.

Additionally, the thermostat never exposes HVACMode.FAN_ONLY even when the device supports SystemMode.Fan_only (0x07). The mappings HVAC_MODE_2_SYSTEM and SYSTEM_MODE_2_HVAC already handle FAN_ONLY <-> Fan_only, but hvac_modes derives its list solely from SEQ_OF_OPERATION which never includes it, since controlSequenceOfOperation only covers cooling/heating per the ZCL spec.

Changes

zha/application/platforms/climate/const.py

  • Import FanMode from zigpy
  • Add SEQ_FAN_MODES dict mapping fan_mode_sequence values (0x00–0x04) to fan mode string lists
  • Add FAN_MODE_TO_ZCL dict mapping fan mode strings to FanMode enum values
  • Add ZCL_TO_FAN_MODE reverse mapping dict

zha/application/platforms/climate/__init__.py

  • fan_modes: Read fan_mode_sequence from the fan cluster handler, look up modes via SEQ_FAN_MODES, fall back to [on, auto] for unknown sequences
  • fan_mode: Read actual fan_mode attribute from the fan cluster handler, map back via ZCL_TO_FAN_MODE, fall back to running_state heuristic when unavailable
  • async_set_fan_mode: Use FAN_MODE_TO_ZCL mapping instead of hardcoded On/Auto branch
  • hvac_modes: Append HVACMode.FAN_ONLY when a fan cluster handler is present

Backwards compatibility

  • Devices with fan_mode_sequence = 0x04 still get [on, auto] (unchanged)
  • Devices without a Fan Control cluster are completely unaffected
  • Unknown fan_mode_sequence values fall back to [on, auto]
  • No changes to HVAC_MODE_2_SYSTEM, SYSTEM_MODE_2_HVAC, or async_set_hvac_mode

Test plan

  • Device with fan_mode_sequence = 0x02 exposes: Low, Medium, High, Auto
  • Device with fan_mode_sequence = 0x04 (or unknown) exposes: On, Auto (backwards compatible)
  • Setting fan mode to "low" sends FanMode.Low (0x01) to the device
  • Setting fan mode to "high" sends FanMode.High (0x03) to the device
  • Current fan mode reflects the actual fan_mode attribute from the Fan Control cluster
  • Thermostat with a Fan Control cluster exposes "Fan only" as an HVAC mode
  • Selecting "Fan only" writes SystemMode.Fan_only (0x07) to the thermostat
  • Thermostat without a Fan Control cluster is unaffected

Affected devices

Any thermostat with a Fan Control cluster (0x0202) that reports fan_mode_sequence != 0x04, such as Tuya HVAC thermostats with cooling/heating/fan modes (e.g. _TZE204_mpbki2zm).

Comment thread zha/application/platforms/climate/const.py Outdated
@puddly

puddly commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

To regenerate device diagnostics files (to see what this change would affect in real devices), run the following:

python -m tools.regenerate_diagnostics

It looks like four devices (in the testing DB) change their exposed fan modes.

@codecov

codecov Bot commented Mar 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.27%. Comparing base (ccc3651) to head (cb51c85).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #708   +/-   ##
=======================================
  Coverage   97.26%   97.27%           
=======================================
  Files          55       55           
  Lines       10906    10922   +16     
=======================================
+ Hits        10608    10624   +16     
  Misses        298      298           

☔ View full report in Codecov by Harness.
📢 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.

@vmvarga

vmvarga commented Mar 13, 2026

Copy link
Copy Markdown
Author

Should resolve #226

@vmvarga
vmvarga requested a review from puddly March 14, 2026 14:49
@puddly

puddly commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

I noticed that one of the test devices was my thermostat. I can confirm this shows up properly:

image

But the fan mode doesn't really do anything for my specific thermostat even when switching the fan mode from "auto" to "on", it doesn't actually support fan mode independent from heat and cool. This is probably just a device-specific bug.

@dmulcahey @TheJulianJES Do you have a device to test this change with?

@TheJulianJES

TheJulianJES commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

I'll have a closer look later, but I think the issue is still present in Matter: https://github.com/home-assistant/core/blob/9ddefaaacd1df9da4f5a4f673eea2348b524d0ac/homeassistant/components/matter/climate.py#L121-L159 (assuming it's even the same issue here, which it might not be...)

@Arol450

Arol450 commented Apr 15, 2026

Copy link
Copy Markdown

Hi, I wanted to add a real-world data point in support of this PR.

I've been running a custom ESP32-C6 Zigbee bridge that exposes a Midea split AC unit to Home Assistant via ZHA using standard clusters (Thermostat 0x0201, Fan Control 0x0202, Temperature Measurement 0x0402). The device reports FanModeSequence.Low_Med_High_Auto (0x02) and supports SystemMode.Fan_only (0x07) and SystemMode.Dry (0x08).

With the current ZHA code:

  • Fan modes are hardcoded to [auto, on] — the actual sequence reported by the device is ignored
  • fan_only is not available as an HVAC mode despite the Fan Control cluster being present

Both of these issues are exactly what this PR addresses. I'd be happy to test a build with this change applied against my device if that would help move things forward.

Thanks for working on this.

@puddly

puddly commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

The specific concern I brought above is that "fan only" support isn't reliably signaled by devices. My thermostat (and I imagine many of the others in our testing database) show up as supporting "fan only" but do not actually support it. There is, as far as I can tell, no way to tell from the ZCL alone that a device supports "fan only". We may need an opt-in mechanism via quirks, unfortunately.

@vmvarga

vmvarga commented Apr 22, 2026

Copy link
Copy Markdown
Author

Added opt-in via quirks

@vmvarga

vmvarga commented May 26, 2026

Copy link
Copy Markdown
Author

@puddly Can you take a look if latest commit is in line with what you suggested?

@grizeldi

grizeldi commented Jun 8, 2026

Copy link
Copy Markdown

Is there anything I can help with to get this merged? Or is it purely waiting for someone to merge it?

I'm also developing a esp32h2 based air conditioning controller and am hitting the exact same situation Arol described above when running the stable ZHA.

@vmvarga

vmvarga commented Jun 11, 2026

Copy link
Copy Markdown
Author

Fixed merge conflicts

@puddly

puddly commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

I think as an opt-in setting via quirk feature ID this is totally fine, since it's disabled by default.

@TheJulianJES thoughts?

@TheJulianJES TheJulianJES left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think using an "exposed feature" via quirks is indeed the best we can do here for the FAN_ONLY mode.

modes = SEQ_OF_OPERATION.get(self._ctrl_sequence_of_oper, [HVACMode.OFF])
if (
self._fan_cluster is not None
and THERMOSTAT_FAN_ONLY_HVAC in self._device.exposes_features

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not saying we should, but if we wanted to, we could also have another entity class that overrides the fist one (so likely using a feature group) and then matches on the exposed feature. There, we'd only add FAN_ONLY to the list of possible modes. It would keep the base ZCL class clean of the exposed feature, but I think either is fine.

Comment thread zha/application/platforms/climate/const.py Outdated
@vmvarga

vmvarga commented Jun 15, 2026

Copy link
Copy Markdown
Author

THERMOSTAT_FAN_ONLY_HVAC was moved to zhaquirks

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	tests/test_climate.py
@vmvarga

vmvarga commented Jun 25, 2026

Copy link
Copy Markdown
Author

Updated according to #762

@vmvarga

vmvarga commented Jul 8, 2026

Copy link
Copy Markdown
Author

@puddly @TheJulianJES Is there anything else that stops us from merging?

@vmvarga
vmvarga requested a review from TheJulianJES July 17, 2026 07:25

@zigpy-review-bot zigpy-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right fix for a real bug — reading fan_mode_sequence instead of hardcoding [auto, on] is clearly correct per ZCL R8 §6.4.2.1.2 (Table 6-42), and the quirk-gated opt-in for FAN_ONLY matches what was asked for in the discussion and follows the existing exposes_feature prior art (SIREN_BASIC in zhaquirks/develco/*.py). One blocker and a few must-address items before this can merge.

Blocker

Fan Control cluster changes never reach Home Assistant. Thermostat.on_add() (zha/application/platforms/climate/__init__.py:549-563) registers the attribute-event callbacks on self._cluster (Thermostat, 0x0201) only. Both new getters read self._fan_cluster (Fan, 0x0202), which has no subscription — so nothing calls maybe_emit_state_changed_event() when the fan mode changes.

This isn't theoretical: ZHA already binds the Fan cluster and configures reporting on fan_mode (climate/__init__.py:353-366, min 5 s / max 900 s / reportable_change 1), so those reports do arrive and update the zigpy cache — they're just dropped on the floor. I verified it against this head: a fan_mode report on the Fan cluster flips entity.fan_mode from auto to high with 0 STATE_CHANGED emissions, and async_set_fan_mode() writes the attribute and returns without emitting either. On dev the same report also emits 0, but there fan_mode is derived from running_state — a Thermostat-cluster attribute that is subscribed — so the value only ever changed at a moment a state event was already firing. This PR moves the value onto an event stream nobody listens to.

User-visible effect: the test-plan item "Current fan mode reflects the actual fan_mode attribute" holds when you read the property directly, but HA keeps showing the old fan mode until some unrelated thermostat attribute happens to report — up to the reporting interval, or indefinitely on a quiet device.

The fix is small — extend the on_add() loop to the fan cluster:

def on_add(self) -> None:
    """Run when entity is added."""
    super().on_add()
    clusters = [self._cluster]
    if self._fan_cluster is not None:
        clusters.append(self._fan_cluster)
    for cluster in clusters:
        for event_type in (
            AttributeReadEvent,
            AttributeReportedEvent,
            AttributeUpdatedEvent,
            AttributeWrittenEvent,
        ):
            self._on_remove_callbacks.append(
                cluster.on_event(event_type.event_type, self.handle_attribute_updated)
            )

(_handle_attribute_updated is keyed on ATTR_OCCP_* names only before it emits, so fan attributes fall straight through to maybe_emit_state_changed_event() — no further changes needed. Worth a test asserting the emission, since nothing currently would catch this.)

Must-address

  1. fan_mode can return a value that isn't in fan_modes — the PR's own regenerated snapshot demonstrates it. tests/data/devices/atlantic-group-adapter-zigbee-fujitsu.json (fan_mode_sequence = 0x02, fan_mode = 0x04) now reports fan_modes: [low, medium, high, auto] alongside fan_mode: "on". HA rejects a set_fan_mode for anything outside fan_modes (_valid_mode_or_raise, homeassistant/components/climate/__init__.py:584) and the frontend renders the selector with no matching option, so the user is left looking at a fan mode they cannot re-select. Please decide the reconciliation deliberately — e.g. union the reported fan_mode into fan_modes, or clamp the reported mode to the sequence.

  2. FanMode.Off and FanMode.Smart are silently reported as auto. ZCL_TO_FAN_MODE is built by inverting FAN_MODE_TO_ZCL, which covers only 5 of the 7 non-reserved values in ZCL R8 §6.4.2.1.1 (Table 6-41); Off (0x00) and Smart (0x06) fall through to the FAN_AUTO default. Verified against this head: a device reporting FanMode.Off shows up in HA as auto, which is the opposite of what it means. It compounds with item 1 — with fan_mode_sequence = 0x00 (Low/Med/High), fan_modes contains no auto at all, so the fallback is out of range by construction. FAN_OFF already exists in const.py:35; please map FanMode.Off to it, and return None rather than FAN_AUTO for anything still unmapped.

  3. The tests don't cover the headline behavior. The three added tests exercise the FAN_ONLY opt-in and the defensive zcl_mode is None branch. Nothing asserts fan_modes derived from a reported fan_mode_sequence, fan_mode read back from the Fan cluster, or that low/medium/high write FanMode.Low/Medium/High — i.e. the first, third, fourth and fifth test-plan checkboxes. The green Codecov patch comes from the four regenerated device snapshots exercising those lines incidentally, which is coverage without assertions. Please add explicit tests (send_attributes_report on the fan cluster in tests/test_climate.py is all it takes).

Optional

  1. The unknown-sequence fallback flipped from [FAN_AUTO, FAN_ON] to [FAN_ON, FAN_AUTO], which is visible in the centralite-systems-3156105 and zen-within-zen-01 snapshots. The PR body says fan_mode_sequence = 0x04 devices are "unchanged" — they're reordered, which changes the dropdown order for every existing user of those devices. Harmless, but if it wasn't intentional it's free to restore.

Re: the hvac_modes design thread (TheJulianJES, inline on line 688)

Asked whether to keep the feature check in the base entity or split it into a feature-matched subclass — my read is keep it inline as written, with one caveat worth stating out loud.

The caveat: climate/__init__.py:688 is the first place in the codebase where a platform entity reads device.exposes_features directly. Until now that set was consumed exclusively by discovery.py:229-239 (match.exposed_features / not_exposed_features), so the subclass route isn't hypothetical — the matching machinery for it already exists and would keep the ZCL base class free of quirk knowledge.

But the cost doesn't scale with the benefit here. A Thermostat subclass that exists only to append one enum member to one property buys a registry entry and a second entity class, and swapping which class a device discovers as brings its own unique_id/continuity questions for anyone who later adds the quirk. The three-line guard is self-documenting and stays local to the property it affects. I'd revisit it if a second or third feature check lands in this entity — at that point the subclass earns its keep.

Separately: nothing in zigpy/zha-device-handlers references THERMOSTAT_FAN_ONLY_HVAC yet, so FAN_ONLY is inert on merge. That's the intended shape of an opt-in, but worth flagging that the DIY ESP32 bridges reported in this thread would each need a quirk (or a custom one) before they see any change — a companion quirks PR would make the feature reachable.

Verified (8 checks)
  • State emission, base vs head. Ran an identical scratch test against ccc36515 and cb51c852: a Fan-cluster fan_mode report emits 0 STATE_CHANGED on both, but only on this head does the reported value change as a result (autoon). A Thermostat-cluster running_state report emits 1 on both.
  • async_set_fan_mode emission. 0 STATE_CHANGED after the write completes.
  • Out-of-range fan_mode. Reproduced from the snapshot values: fan_mode_sequence = 2 + fan_mode = 4fan_modes = ['low','medium','high','auto'], fan_mode = 'on'.
  • Off / Smart fallback. Devices reporting FanMode.Off (0) and FanMode.Smart (6) both surface as 'auto'; with fan_mode_sequence = 0 the list is ['low','medium','high'], so 'auto' isn't in it.
  • fan_mode_sequence availability. Initially suspected read_on_startup=False meant the attribute was never fetched — it isn't; cluster_config.py:245-270 reads those with allow_cache=True, so a freshly paired device does read it (falling back to [on, auto] only when the read fails). Not an issue.
  • ZCL. R8 §6.4.2.1.1 Table 6-41 (FanMode values) and §6.4.2.1.2 Table 6-42 (FanModeSequence) — SEQ_FAN_MODES matches Table 6-42 exactly.
  • Local suite + tooling. tests/test_climate.py 65 passed; mypy zha/ clean in the worktree venv (real deps, not the dependency-less pre-commit env); ruff check clean. CI green across 3.12/3.13/3.14.
  • Second opinion (Copilot, GPT-5.5). Independently raised the state-staleness and the Off/Smart fallback; both excerpts verified against the cited lines. No other findings.

Comment thread zha/application/platforms/climate/__init__.py
Comment thread zha/application/platforms/climate/__init__.py Outdated
Comment thread zha/application/platforms/climate/const.py Outdated
Comment thread zha/application/platforms/climate/__init__.py Outdated
Comment thread tests/test_climate.py
@vmvarga
vmvarga requested a review from zigpy-review-bot July 29, 2026 07:45
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.

6 participants