fix: include superQuiet in 3-speed unit fan_speeds#73
Conversation
Units reporting numberOfFanSpeeds=3 were mapped to ["quiet", "low", "powerful"], omitting superQuiet. However, pykumo's set_fan_speed() already accepts superQuiet (it's in ALL_FAN_SPEEDS and the unit firmware honours it), so the omission is a profile under-report rather than a hardware limitation — confirmed on Mitsubishi mini-splits that respond to superQuiet commands despite advertising 3 speeds. Update the speeds==3 branch to match the speeds==5 branch in including superQuiet as the quietest option. Add unit tests covering all three speed-count branches. Co-Authored-By: Claude <noreply@anthropic.com>
Extends the speeds==3 branch to return the full ["superQuiet", "quiet", "low", "powerful", "superPowerful"] list, matching the speeds==5 branch. Hardware testing on Mitsubishi units advertising numberOfFanSpeeds=3 confirms all five speeds are accepted by the firmware. The previous fix (PR dlarrick#73 v1) added superQuiet; this update adds superPowerful at the loud end, giving the full symmetric range. Tests updated: 3-speed branch now asserts the complete 5-element list and two new tests verify both superQuiet and superPowerful are accepted by set_fan_speed. 19/19 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates PyKumo.get_fan_speeds() so that units reporting numberOfFanSpeeds == 3 advertise the full 5-speed range (including superQuiet and superPowerful), aligning behavior with the existing 5-speed branch and preventing downstream integrations from rejecting those modes.
Changes:
- Update
get_fan_speeds()to return["superQuiet", "quiet", "low", "powerful", "superPowerful"]whennumberOfFanSpeeds == 3. - Add a new unit test module covering fan speed lists for 3/4/5-speed profiles, including ordering and
autosuffix behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pykumo/py_kumo.py |
Expands the 3-speed fan speed list to include superQuiet and superPowerful. |
tests/test_fan_speeds.py |
Adds tests asserting the returned fan speed lists and ordering for several profile combinations. |
Comments suppressed due to low confidence (1)
pykumo/py_kumo.py:396
- The 3-speed and default branches now return the exact same 5-speed list, which duplicates logic and risks future drift. Consider collapsing this to a single non-4-speed branch (or reuse a shared constant) so only one place defines the 5-speed ordering.
valid_speeds = ["superQuiet", "quiet", "low", "powerful", "superPowerful"]
elif speeds == 4:
valid_speeds = ["quiet", "Low", "powerful", "superPowerful"]
else:
valid_speeds = ["superQuiet", "quiet", "low", "powerful", "superPowerful"]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_set_fan_speed_accepts_superquiet_on_3speed(self): | ||
| """set_fan_speed must accept superQuiet for 3-speed units.""" | ||
| with patch.object(PyKumo, "__init__", lambda self, *a, **kw: None): | ||
| unit = PyKumo.__new__(PyKumo) | ||
| unit._profile = {"numberOfFanSpeeds": 3} | ||
| unit._status = {} | ||
| valid = unit.get_fan_speeds() | ||
| self.assertIn("superQuiet", valid) | ||
|
|
||
| def test_set_fan_speed_accepts_superpowerful_on_3speed(self): | ||
| """set_fan_speed must accept superPowerful for 3-speed units.""" | ||
| with patch.object(PyKumo, "__init__", lambda self, *a, **kw: None): | ||
| unit = PyKumo.__new__(PyKumo) | ||
| unit._profile = {"numberOfFanSpeeds": 3} | ||
| unit._status = {} | ||
| valid = unit.get_fan_speeds() | ||
| self.assertIn("superPowerful", valid) |
There was a problem hiding this comment.
Copilot has a point here but I think the solution is to rename these tests
|
If you address these comments I will merge and make a new release |
- Add inline comment at the speeds == 3 branch explaining that 3-speed-reporting units under-report their capability and intentionally receive the full 5-speed list. - Rename test_set_fan_speed_accepts_superquiet_on_3speed → test_get_fan_speeds_includes_superquiet_on_3speed and test_set_fan_speed_accepts_superpowerful_on_3speed → test_get_fan_speeds_includes_superpowerful_on_3speed. These tests assert get_fan_speeds() membership, not set_fan_speed() behaviour; rename + docstring fix makes that accurate. Co-Authored-By: Claude <noreply@anthropic.com>
|
Added the intentional-5-speeds comment at the |
Problem
`get_fan_speeds()` maps `numberOfFanSpeeds` to a hardcoded list. The `speeds == 3` branch returned only `["quiet", "low", "powerful"]`, omitting both ends of the speed range:
```python
if speeds == 3:
valid_speeds = ["quiet", "low", "powerful"] # before
```
This caused two downstream problems:
Root cause
The profile field `numberOfFanSpeeds` under-reports capability on some hardware. Firmware on units advertising 3 speeds honours all five speed commands — both `superQuiet` and `superPowerful` are already in `ALL_FAN_SPEEDS` and the 5-speed branch already exposes the full set.
Fix
Align the `speeds == 3` branch with the `speeds == 5` branch — return the full 5-speed list:
```python
if speeds == 3:
valid_speeds = ["superQuiet", "quiet", "low", "powerful", "superPowerful"] # after
```
Order: quietest → loudest (superQuiet < quiet < low < powerful < superPowerful), auto appended last when `hasFanSpeedAuto` is set.
The 4-speed branch is unchanged (it uses a different speed set).
Hardware verification
Tested on Mitsubishi units at a real installation where `numberOfFanSpeeds=3`:
Tests
`tests/test_fan_speeds.py` updated:
19/19 tests pass.
Caveat
Some hardware may genuinely only support the middle 3 speeds and not honour superQuiet or superPowerful. If a unit ignores or errors on those commands, the result is a no-op at the device level — not a driver crash. The fix takes the pragmatic position that the full range should be offered where the firmware accepts it.