Skip to content

Fix thermostat fan modes to respect fan_mode_sequence - #708

Merged
TheJulianJES merged 30 commits into
zigpy:devfrom
vmvarga:dev
Oct 26, 2026
Merged

TheJulianJES merged 30 commits into
zigpy:devfrom
vmvarga:dev

Conversation

@vmvarga

@vmvarga vmvarga commented Mar 12, 2026 •

Copy link
Copy Markdown
Contributor

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-42).
  • Fix the fan_mode getter to read the actual fan_mode attribute from the Fan Control cluster, falling back to the thermostat's running_state heuristic only when it is not cached.
  • Fix async_set_fan_mode to map all supported fan mode strings (off, low, medium, high, on, auto) to their corresponding FanMode ZCL enum values.
  • Subscribe the entity to attribute events of the Fan Control cluster, so fan attribute reports and writes emit a state change.
  • Allow HVACMode.FAN_ONLY on thermostats that have a Fan Control cluster, as an opt-in via the THERMOSTAT_FAN_ONLY_HVAC quirk feature (zha.quirks). SEQ_OF_OPERATION (derived from controlSequenceOfOperation) never includes it per ZCL spec, and not every thermostat with a fan cluster supports SystemMode.Fan_only.

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-42), FanModeSequence 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 and FanModeSequence from zigpy
  • Add SEQ_FAN_MODES dict mapping FanModeSequence values to fan mode string lists (On_Auto is [auto, on], the same order as before)
  • Add FAN_MODE_TO_ZCL dict mapping fan mode strings (including off) to FanMode enum values
  • Add ZCL_TO_FAN_MODE reverse mapping dict (FanMode.Smart is deliberately unmapped)

zha/application/platforms/climate/__init__.py

  • on_add: also subscribe to the attribute events of the Fan Control cluster, so reports and writes of its attributes emit a state change
  • fan_modes: read fan_mode_sequence from the fan cluster, look up modes via SEQ_FAN_MODES, fall back to [auto, on] for unknown sequences. If the device currently reports a fan mode outside its advertised sequence (e.g. on with Low_Med_High_Auto), that mode is appended so fan_mode stays selectable
  • fan_mode: read the actual fan_mode attribute from the fan cluster and map it back via ZCL_TO_FAN_MODE (None for Smart or a mode not in fan_modes). Without a cached attribute it falls back to the running_state heuristic, clamped to fan_modes
  • async_set_fan_mode: use FAN_MODE_TO_ZCL instead of the hardcoded On/Auto branch; unmapped modes log a warning and write nothing
  • hvac_modes: append HVACMode.FAN_ONLY when a fan cluster is present and the device exposes THERMOSTAT_FAN_ONLY_HVAC

Backwards compatibility

  • Devices with fan_mode_sequence = 0x04 still get [auto, on] (unchanged)
  • Devices without a Fan Control cluster are unaffected
  • Unknown fan_mode_sequence values fall back to [auto, on]
  • FAN_ONLY is only added for devices that opt in through the quirk feature
  • No changes to HVAC_MODE_2_SYSTEM, SYSTEM_MODE_2_HVAC, or async_set_hvac_mode

Test plan

  • Each fan_mode_sequence (0x00–0x04, plus an unknown value) exposes the expected modes
  • Setting fan mode to "low" / "medium" / "high" writes FanMode.Low / Medium / High
  • Current fan mode reflects the fan_mode attribute reported by the Fan Control cluster, and a report emits a state change
  • A reported mode outside the sequence is added to fan_modes; Off maps to off, Smart to None
  • The running_state fallback is clamped to fan_modes
  • FAN_ONLY is exposed only with THERMOSTAT_FAN_ONLY_HVAC, and not without it
  • Selecting "Fan only" writes SystemMode.Fan_only (0x07): this relies on the existing, unchanged async_set_hvac_mode mapping and has no new test

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.24%. Comparing base (9d740ee) to head (c1245a7).
⚠️ Report is 12 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #708      +/-   ##
==========================================
+ Coverage   97.22%   97.24%   +0.01%     
==========================================
  Files          61       61              
  Lines       10676    10748      +72     
==========================================
+ Hits        10380    10452      +72     
  Misses        296      296              

☔ 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
Contributor 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
Contributor Author

Added opt-in via quirks

@vmvarga

vmvarga commented May 26, 2026

Copy link
Copy Markdown
Contributor 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
Contributor 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
@TheJulianJES
TheJulianJES requested review from zigpy-review-bot and removed request for zigpy-review-bot August 10, 2026 18:12

@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.

Request changes — everything from the last round is fixed; one item left, and it's visible in the PR's own regenerated snapshot.

Fixed since 2026-08-10 (re-verified at 76deb4a): FanModeSequence.On_Auto is back to [auto, on], so centralite-systems-3156105 and zen-within-zen-01 regenerate unchanged and CI is green on 3.12/3.13/3.14. The running_state fallback now runs through the same in self.fan_modes clamp. The test gap is closed — sequence-derived fan_modes, fan_mode read back from the Fan cluster, and low/medium/high writes are all asserted now.

Must address — the clamp nulls out a real device's fan mode

tests/data/devices/atlantic-group-adapter-zigbee-fujitsu.json caches fan_mode = 0x04 (On) together with fan_mode_sequence = 0x02 (Low_Med_High_Auto). SEQ_FAN_MODES maps that sequence to [low, medium, high, auto], so on fails the mode in self.fan_modes check at zha/application/platforms/climate/__init__.py:612 and fan_mode returns None. The PR's own snapshot records it: "fan_mode": "auto" → "fan_mode": null.

In HA that means climate.fan_mode is None while ClimateEntityFeature.FAN_MODE is set — the fan dropdown renders with nothing selected on a device whose fan is genuinely running on On. The same list rejects the way back: async_set_fan_mode("on") returns early at line 833, so the user can't select it either.

The device is out of spec — ZCL R8 §6.4.2.1.2 Table 6-42 doesn't include On in the Low/Med/High/Auto sequence — but §6.4.2.1.1 Table 6-41 makes On (0x04) a valid FanMode value regardless of the sequence, and this is a device in the test DB that a user in this thread reported testing the PR against, so it isn't a hypothetical.

Suggested shape — union the reported mode into fan_modes rather than discarding it:

@property
def fan_modes(self) -> list[str] | None:
    """Return supported FAN modes."""
    if self._fan_cluster is None:
        return None
    seq = self._fan_cluster.get(FanCluster.AttributeDefs.fan_mode_sequence.name)
    modes = list(SEQ_FAN_MODES.get(seq, [FAN_AUTO, FAN_ON]))
    current = ZCL_TO_FAN_MODE.get(
        self._fan_cluster.get(FanCluster.AttributeDefs.fan_mode.name)
    )
    if current is not None and current not in modes:
        modes.append(current)
    return modes

That keeps the invariant the clamp was added for (fan_mode is always in fan_modes), turns the Atlantic snapshot back into "fan_mode": "on" instead of null, makes on selectable again, and leaves the three spec-conforming devices byte-identical. FanMode.Smart still maps to nothing and still yields None, which is the right answer for it. The list(...) copy also stops fan_modes handing out the shared module-level list object from SEQ_FAN_MODES.

Behavior note (no action needed)

No quirk in zigpy/zha-device-handlers references THERMOSTAT_FAN_ONLY_HVAC yet, so HVACMode.FAN_ONLY ships inert until a quirks-side PR opts a device in. That's the intended opt-in design, but worth flagging for the people in this thread running their own ESP32 Zigbee firmware — a custom device with no quirk won't pick it up.

Verified (7 checks)
  • Reviewed at 76deb4a in an isolated worktree + venv. pytest tests/test_climate.py: 83 passed. mypy zha/ inside that venv (real deps, unlike the CI hook's dependency-free env): no errors. ruff check: clean. CI green on 3.12/3.13/3.14.
  • Surveyed all 54 device snapshots for a Fan cluster (0x0202). Four have a thermostat entity: atlantic-group-adapter-zigbee-fujitsu (fan_mode 0x04 / seq 0x02), enktro-acmidea (0x05 / 0x02), centralite-systems-3156105 (0x05 / 0x04), zen-within-zen-01 (0x05 / 0x04). Only the Atlantic one hits the mismatch above; the other three are correct under this PR.
  • Confirmed the On_Auto ordering fix: no centralite/zen-within snapshot churn in the diff, and test_devices_from_files passes.
  • Confirmed the running_state fallback is now clamped (lines 616-628), covered by test_fan_mode_running_state_fallback_clamped.
  • read_on_startup=False on fan_mode_sequence is not a problem for the new derivation: initialize_cluster_configs reads cache-preferring attributes with allow_cache=True (not only_cache), so a freshly-paired device still reads it off the wire once.
  • Checked the new Fan-cluster event subscription in on_add against every handle_attribute_updated override — MoesThermostat, BecaThermostat and ZONNSMARTThermostat dispatch on the attribute name operation_preset, which can't collide with fan_mode / fan_mode_sequence despite the overlapping attribute IDs, and SinopeTechnologiesThermostat.on_add calls super().on_add().
  • Second opinion from GitHub Copilot (GPT-5.5, read-only): one finding, independently the same clamp regression on the Atlantic device. Nothing else.

Comment thread zha/application/platforms/climate/__init__.py Outdated
Comment thread tests/data/devices/atlantic-group-adapter-zigbee-fujitsu.json Outdated
@TheJulianJES
TheJulianJES requested review from zigpy-review-bot and removed request for zigpy-review-bot August 11, 2026 07:40

@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.

Comment — the fan_modes union does exactly what it was meant to, and every item from the previous rounds is now closed. CI is green on 3.12/3.13/3.14, the full suite passes locally, and regenerate_diagnostics produces zero churn at 1fd4370. One new item arrived with the union commit; it's a one-line fix, so I'm flagging rather than blocking.

Fixed since 76deb4a: atlantic-group-adapter-zigbee-fujitsu regenerates as "fan_mode": "on" with on appended to fan_modes, so the state is accurate and the mode is selectable again. The list(...) copy stops fan_modes handing out the shared SEQ_FAN_MODES list object, and dropping functools.cached_property (which dev has on this property) is what lets the union actually track reports instead of freezing at first access.

Must address — off becomes selectable but can never be written

ZCL_TO_FAN_MODE maps FanMode.Off (0x00) to "off", so a device reporting that mode now gets "off" unioned into fan_modes and shown in the HA fan dropdown. FAN_MODE_TO_ZCL has no "off" entry, so async_set_fan_mode("off") clears the fan_mode not in self.fan_modes gate at line 842 and then dead-ends at line 848 — No ZCL mapping for fan mode 'off' in the log, nothing on the wire.

Verified on this head, with fan_mode_sequence = Low_Med_High_Auto and a reported FanMode.Off:

fan_modes = ['low', 'medium', 'high', 'auto', 'off']
fan_mode  = 'off'
write_attributes calls after async_set_fan_mode('off') = []

This was unreachable before the union commit — off appears in no SEQ_FAN_MODES list, so the fan_modes gate rejected it first. Adding FAN_OFF: FanMode.Off, to FAN_MODE_TO_ZCL closes it, and Off is a writable value of the fan_mode attribute per ZCL R8 §6.4.2.1.1 (Table 6-41), so there's no spec objection to sending it.

test_fan_mode_off_mapped_correctly asserts the read side (FanState.OFF in entity.fan_modes) but not the write side, which is why the suite stays green over this. Worth extending it with an async_set_fan_mode("off") assertion once the mapping is added.

Behavior note — the union is scoped to the currently-reported mode (no action requested)

The union only holds while the device is actively reporting the out-of-sequence mode. Verified: the Atlantic shape gives ['low', 'medium', 'high', 'auto', 'on'] while On, but after a Low report it drops to ['low', 'medium', 'high', 'auto'] and async_set_fan_mode("on") is rejected again — so a user who switches away from on can't get back to it. On dev that device's list is the hardcoded [auto, on], where on is always offered.

Net it's still clearly better than dev for that device (low/medium/high were previously unreachable), and this is the shape I suggested last round, so I'm not asking for a change — just making sure the trade-off is a deliberate choice rather than a surprise later.

Re: a second entity class + feature group for FAN_ONLY (raised by TheJulianJES on zha/application/platforms/climate/__init__.py:711)

I'd keep the inline check that's here. The THERMOSTAT_FAN feature group is already carrying this entity's priority ladder: the base Thermostat registers at feature_priority=(PlatformFeatureGroup.THERMOSTAT_FAN, 1) (line 242) and seven manufacturer subclasses register at priority 2. A ThermostatFanOnly(Thermostat) gated on exposed_features={THERMOSTAT_FAN_ONLY_HVAC} would need to outrank the base but lose to the manufacturer classes, and there is no slot between 1 and 2. Put it at 2 and it ties with the manufacturer match — selected_matches = matches_by_priority[highest_priority] (zha/application/discovery.py:323) returns the whole tied list, so a Zen/Moes/Beca/Centralite device that opted in would get two climate entities. Making it work means either renumbering the ladder or adding a fan-only variant of each of the seven subclasses.

The SIREN_BASIC precedent (zha/application/platforms/siren.py:335) composes cleanly because it pairs with not_exposed_features on the competing select entities and has no manufacturer subclass ladder to fight. Here the three-condition check at lines 708-714 costs five lines and works with every subclass for free, so the cost/benefit points the other way.

Verified (8 checks)
  • Reviewed at 1fd4370 in an isolated worktree + venv. Full suite: 1385 passed. tests/test_climate.py: 83 passed. mypy zha/ inside that venv (real deps, unlike the CI hook's dependency-free env): no issues in 57 source files. ruff check and ruff format --check: clean. CI green on 3.12/3.13/3.14.
  • python -m tools.regenerate_diagnostics at this head produces an empty diff — the committed snapshots match what the code actually generates.
  • Re-surveyed every device snapshot: four expose a fan_modes list — atlantic-group-adapter-zigbee-fujitsu (on / [low, medium, high, auto, on]), enktro-acmidea (auto / [low, medium, high, auto]), centralite-systems-3156105 and zen-within-zen-01 (both auto / [auto, on]). Last round's Atlantic null is gone and the other three are unchanged from dev.
  • Confirmed the off dead-end by probe (output quoted above) and confirmed it is new to the union commit, not pre-existing.
  • Confirmed the union's mode-scoping by probe (the on → low → on-unreachable sequence quoted above).
  • Confirmed fan_modes is a plain @property on this branch. dev declares it functools.cached_property; had that survived, the union would have frozen at first access and the fix would have been inert after the first state read.
  • THERMOSTAT_FAN_ONLY_HVAC still has no consumer in zigpy/zha-device-handlers (grepped dev), so HVACMode.FAN_ONLY ships inert until a quirks-side PR opts a device in. That's the intended design, but worth restating for the people in this thread running their own ESP32 Zigbee firmware — a custom device with no quirk won't pick it up.
  • Second opinion from GitHub Copilot (GPT-5.5, read-only): one finding, independently the same off dead-end with the same reasoning. Nothing else.

Comment thread zha/application/platforms/climate/const.py
Comment thread zha/application/platforms/climate/__init__.py
Comment thread zha/application/platforms/climate/__init__.py Outdated

@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.

Approve — the FAN_OFF → FanMode.Off write mapping from the last round is in, along with the test extension, and every item from the previous rounds is now closed. CI is green on 3.12/3.13/3.14, the full suite passes locally, regenerate_diagnostics produces an empty diff at 18dadc3, and mypy/ruff are clean.

Fixed since 1fd4370: FAN_MODE_TO_ZCL gains FAN_OFF: FanMode.Off, so the off entry the union puts into fan_modes is now actually writable instead of dead-ending at the No ZCL mapping for fan mode 'off' warning. test_fan_mode_off_mapped_correctly asserts the write side as well as the read side.

Verified by probe at this head, on a Low_Med_High_Auto device:

reported=Auto  -> fan_modes=['low','medium','high','auto']        fan_mode='auto'  set('off') -> no write (rejected by the fan_modes gate)
reported=Off   -> fan_modes=['low','medium','high','auto','off']  fan_mode='off'   set('off') -> write_attributes({'fan_mode': FanMode.Off})
reported=Auto  -> fan_modes=['low','medium','high','auto']        fan_mode='auto'
reported=Smart -> fan_modes=['low','medium','high','auto']        fan_mode=None

Behavior note — off is reachable only while the device reports it (no action requested)

This is the same mode-scoping caveat as the on case I flagged last round, so it isn't an ask. Because no FanModeSequence value contains Off, off enters fan_modes only through the union — that is, only while the device is already reporting FanMode.Off. So in practice the new write mapping only ever fires as a redundant re-write of the mode the device is already in; "turn the fan off" from low/auto stays unavailable.

That is not a regression (dev's hardcoded [auto, on] never offered off either), and it is defensible on spec: ZCL R8 §6.4.2.1.2 describes FanModeSequence as "the possible fan speeds that the thermostat can set", and Table 6-42 lists no sequence containing Off — so unconditionally offering off would expose a mode no device advertises. The mapping is still worth having exactly as added, because it stops HA from rendering a dropdown option that silently does nothing.

If a device ever turns up that genuinely accepts Off outside its advertised sequence, the follow-up is a one-liner (append FAN_OFF unconditionally), but it would change the dropdown for every existing fan-capable thermostat and churn the four snapshots — not something to fold into this PR.

Notes carried over, unchanged

  • THERMOSTAT_FAN_ONLY_HVAC still has no consumer in zigpy/zha-device-handlers (re-grepped origin/dev), so HVACMode.FAN_ONLY ships inert until a quirks-side PR opts a device in. That is the intended shape of an opt-in, but the people in this thread running their own ESP32/ESP32-C6 Zigbee firmware each need a quirk before they see any change.
  • The design thread on hvac_modes (TheJulianJES, climate/__init__.py:711) is still open. My position from last round is unchanged: the inline feature check is the cheaper option here, because a ThermostatFanOnly subclass has no free slot in the THERMOSTAT_FAN priority ladder between the base class (priority 1) and the seven manufacturer subclasses (priority 2).
Verified (8 checks)
  • Reviewed at 18dadc3 in an isolated worktree + venv. Full suite: 1385 passed. tests/test_climate.py: 83 passed. mypy zha/ inside that venv (real deps, unlike the CI hook's dependency-free env): no issues in 57 source files. ruff check and ruff format --check: clean. CI green on 3.12/3.13/3.14.
  • python -m tools.regenerate_diagnostics at this head produces an empty diff — the committed snapshots match what the code actually generates.
  • The delta since 1fd4370 is exactly one source line (FAN_OFF: FanMode.Off in FAN_MODE_TO_ZCL) plus the test extension and its docstring. No other behavior change to re-verify.
  • Re-surveyed every device snapshot: four expose a fan_modes list — atlantic-group-adapter-zigbee-fujitsu (on / [low, medium, high, auto, on]), enktro-acmidea (auto / [low, medium, high, auto]), centralite-systems-3156105 and zen-within-zen-01 (both auto / [auto, on]). Unchanged from last round; the latter two still match dev.
  • Probe above covers the read and write of off, its disappearance once the device moves to another mode, and FanMode.Smart still yielding None without being unioned in.
  • ZCL R8 §6.4.2.1.1 Table 6-41 (FanMode values, access RW) and §6.4.2.1.2 Table 6-42 (FanModeSequence) re-read for the Off question above.
  • Confirmed functools is still used elsewhere in climate/__init__.py after the cached_property removal, and that Device.exposes_features is the right accessor for the hvac_modes opt-in check.
  • Second opinion from GitHub Copilot (GPT-5.5, read-only): one finding — the same observation that off is absent from every sequence-derived fan_modes, raised as must-address. I verified the cited code and agree with the mechanism, but not with the severity: the behavior predates this PR and no FanModeSequence advertises Off, so I've recorded it as the behavior note above rather than an ask.

@vmvarga

vmvarga commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@puddly @TheJulianJES Bot seems happy maybe this can be merged?

@zigpy-review-bot zigpy-review-bot added the bugfix This PR fixes a bug label Oct 10, 2026
@vmvarga

vmvarga commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@puddly @TheJulianJES Any comments?

@TheJulianJES TheJulianJES added the smash This PR is close to be merged soon label Oct 23, 2026
Comment thread zha/application/platforms/climate/__init__.py Outdated
@TheJulianJES TheJulianJES changed the title Fix thermostat fan modes to respect fanModeSequence Fix thermostat fan modes to respect fan_mode_sequence Oct 26, 2026
@TheJulianJES
TheJulianJES requested a balanced review from Copilot October 26, 2026 21:33

Copilot AI 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.

🟢 Approval recommended

Static review identified no blocking issues in these focused changes, with regression coverage added; automated tests were not run.

No open issues 🎉

What changed in this PR

Updates thermostat fan controls to use the Fan Control cluster’s advertised speeds and reported mode, replacing hardcoded choices and inferred state.

Changes:

  • Adds sequence-based fan modes, ZCL value mappings, and fan-cluster event subscriptions.
  • Enables fan-only HVAC mode when a device explicitly opts in through a quirk feature.
  • Adds regression tests and updates device fixtures for the corrected fan behavior.
File Description
zha/​quirks.py Defines the fan-only HVAC opt-in feature.
zha/​application/​platforms/​climate/​const.py Adds fan sequence and mode mappings.
zha/​application/​platforms/​climate/​__init__.py Updates fan state, supported modes, writes, and event handling.
tests/​test_climate.py Tests fan mappings, reports, fallbacks, and fan-only opt-in.
tests/​data/​devices/​enktro-acmidea.json Updates expected supported fan speeds.
tests/​data/​devices/​atlantic-group-adapter-zigbee-fujitsu.json Updates expected fan state and available speeds.

Review effort: Balanced

@TheJulianJES
TheJulianJES merged commit 60e79cc into zigpy:dev Oct 26, 2026
8 checks passed
@TheJulianJES

This comment was marked as resolved.

@TheJulianJES

Copy link
Copy Markdown
Contributor

Thanks for the PR!

@zigpy-review-bot

This comment was marked as resolved.

@TheJulianJES

Copy link
Copy Markdown
Contributor

@zigpy-review-bot Also, open issue in quirks repo for which devices need the exposed feature added. Possibly also mention if/when/how we can move this entirely to quirks possibly (and whether we even should). Note puddly's open PRs for direct entity creation. Do review rounds for everything.

@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Filed zigpy/zha-device-handlers#5551 with the candidate devices and the quirks question.

  • Only the ATLANTIC GROUP Adapter Zigbee FUJITSU has both kinds of evidence (a ZHA diagnostics snapshot with a fan cluster, and Zigbee2MQTT exposing fan_only). It needs a new minimal quirk. The enktro ACmidea has no evidence either way.
  • The Tuya _TZE204_mpbki2zm has no quirk and uses the Tuya cluster, so the feature does not apply to it. CentraLite 3156105 and Zen-01 are not candidates, and Sinope HP6000ZB maps to a different system mode value. Several other Z2M-listed AC thermostats are in the issue as unconfirmed.
  • The feature constant is not in a zha release yet (latest tag 2.3.1 predates this PR), so a quirk using it needs the next release and a minimum-version bump in the quirks repo first.
  • Moving it entirely into quirks: not yet, and probably not for a single capability flag. A shared quirk-side Thermostat subclass via replaces_entity would work in principle, but that API is still a provisional draft in Add APIs to directly create ZHA entities zha-device-handlers#5410 and quirks would import ZHA climate internals. The issue also proposes a lighter follow-up in zigpy/zha: pass the existing exposes_feature config through, which zha currently drops.

The issue text went through two rounds with a Fable reviewer and one Copilot pass; their verified findings are folded in. This PR itself was not re-reviewed, since it is already merged.

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 smash This PR is close to be merged soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants