Repository: XKNX/xknx
Pull request #967 (2022-06-05 22:22:39)

Add support for separate fan speed and on/off GAs
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description

Added two new optional variables to provide GAs for fan switch. This closes the gap for fans where the steps/percentage have no influence on the on/off state. If those new GAs are not provided, then the existing behavior of controlling on/off via the fan speed remains.

The only breaking change part is that now if you turn on a fan without a switch GA, a default speed of 50% will automatically be set (unless the caller provides an explicit value of course). This default was previously set by the HA integration. See comment below reg. the reasoning behind it: https://github.com/XKNX/xknx/pull/967#discussion_r891028912

Corresponding HA integration PR: https://github.com/home-assistant/core/pull/73131

## Type of change
<!--
Please tick the applicable options.
NOTE: Ticking multiple options most likely indicates
that your change is to big and it is suggested to split it into several smaller PRs.
-->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)
- [X] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## Checklist:

- [X] The documentation has been adjusted accordingly
- [X] Tests have been added that prove the fix is effective or that the feature works
- [X] The changes are documented in the changelog (docs/changelog.md)

codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/967?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#967](https://codecov.io/gh/XKNX/xknx/pull/967?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (255174a) into [main](https://codecov.io/gh/XKNX/xknx/commit/5addac61088c2785f64f6f9a3f82ebc5c5c0dfac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (5addac6) will **decrease** coverage by `0.10%`.
> The diff coverage is `60.86%`.

> :exclamation: Current head 255174a differs from pull request most recent head 9e33049. Consider uploading reports for the commit 9e33049 to get more accurate results

[![Impacted file tree graph](https://codecov.io/gh/XKNX/xknx/pull/967/graphs/tree.svg?width=650&height=150&src=pr&token=irWbIygS84&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)](https://codecov.io/gh/XKNX/xknx/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
- Coverage   95.99%   95.89%   -0.11%     
==========================================
  Files         131      131              
  Lines        7924     7946      +22     
==========================================
+ Hits         7607     7620      +13     
- Misses        317      326       +9     
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/devices/fan.py](https://codecov.io/gh/XKNX/xknx/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2Zhbi5weQ==) | `85.71% <60.86%> (-14.29%)` | :arrow_down: |


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 87205e58..42ae05da 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -13,6 +13,7 @@ nav_order: 2
 - Lock sending telegrams via a Tunnel until a confirmation is received
 - Use device subclass for `device_updated_cb` callback argument type hint
 - Fix CEMI Frame Ack-request flag set wrongly
+- Fan: Add support for dedicated on/off switch GA
 
 ## 0.21.3 Cover updates 2022-05-17
 
diff --git a/docs/fan.md b/docs/fan.md
index c1c2faaa..909c5453 100644
--- a/docs/fan.md
+++ b/docs/fan.md
@@ -9,16 +9,18 @@ nav_order: 4
 
 ## [](#header-2)Overview
 
-Fans are simple representations of KNX controlled fans. They support setting the speed and the oscillation.
+Fans are simple representations of KNX controlled fans. They support switching on/off, setting the speed and the oscillation.
 
 ## [](#header-2)Interface
 
 - `xknx` XKNX object.
 - `name` name of the device.
-- `group_address` is the KNX group address of the fan speed. Used for sending. *DPT 5.001 / 5.010*
-- `group_address_state` is the KNX group address of the fan speed state. Used for updating and reading state. *DPT 5.001 / 5.010*
+- `group_address_speed` is the KNX group address of the fan speed. Used for sending. If no `group_address_switch` is provided, it will implicitly control switching the fan on/off as well. *DPT 5.001 / 5.010*
+- `group_address_speed_state` is the KNX group address of the fan speed state. Used for updating and reading state. *DPT 5.001 / 5.010*
 - `group_address_oscillation` is the KNX group address of the oscillation. Used for sending. *DPT 1.001*
 - `group_address_oscillation_state` is the KNX group address of the fan oscillation state. Used for updating and reading state. *DPT 1.001*
+- `group_address_switch` is the KNX group address of the fan on/off state. If not used, on/off will implicitly be controlled via `group_address_speed` instead. Used for sending. *DPT 1.001*
+- `group_address_switch_state` is the KNX group address of the fan on/off state. Used for updating and reading state. *DPT 1.001*
 - `sync_state` defines if and how often the value should be actively read from the bus. If `False` no GroupValueRead telegrams will be sent to its group address. Defaults to `True`
 - `device_updated_cb` awaitable callback for each update.
 - `max_step` Maximum step amount for fans which are controlled with steps and not percentage. If this attribute is set, the fan is controlled by sending the step value in the range `0` and `max_step`. In that case, the group address DPT changes from *DPT 5.001* to *DPT 5.010*. Default: None
@@ -26,12 +28,14 @@ Fans are simple representations of KNX controlled fans. They support setting the
 ## [](#header-2)Example
 
 ```python
-fan = Fan(xknx,
-              'TestFan',
-              group_address='1/2/1',
-              group_address_state='1/2/2',
-              group_address_oscillation='1/2/3',
-              group_address_oscillation_state='1/2/4')
+fan = Fan(
+    xknx,
+    'TestFan',
+    group_address_speed='1/2/1',
+    group_address_speed_state='1/2/2',
+    group_address_oscillation='1/2/3',
+    group_address_oscillation_state='1/2/4'
+)
 
 # Set the fan speed
 await fan.set_speed(50)
@@ -42,9 +46,12 @@ print(fan.current_speed)
 # Set the oscillation
 await fan.set_oscillation(True)
 
-# Accessing speed
+# Accessing oscillation
 print(fan.current_oscillation)
 
+# Accessing on/off state
+print(fan.is_on)
+
 # Requesting state via KNX GroupValueRead
 await fan.sync()
 ```
diff --git a/xknx/devices/fan.py b/xknx/devices/fan.py
index 4147dc6e..8f477f88 100644
--- a/xknx/devices/fan.py
+++ b/xknx/devices/fan.py
@@ -26,6 +26,8 @@ if TYPE_CHECKING:
     from xknx.telegram import Telegram
     from xknx.xknx import XKNX
 
+DEFAULT_TURN_ON_SPEED = 50
+
 logger = logging.getLogger("xknx.log")
 
 
@@ -47,6 +49,8 @@ class Fan(Device):
         group_address_speed_state: GroupAddressesType | None = None,
         group_address_oscillation: GroupAddressesType | None = None,
         group_address_oscillation_state: GroupAddressesType | None = None,
+        group_address_switch: GroupAddressesType | None = None,
+        group_address_switch_state: GroupAddressesType | None = None,
         sync_state: bool | int | float | str = True,
         device_updated_cb: DeviceCallbackType[Fan] | None = None,
         max_step: int | None = None,
@@ -58,6 +62,19 @@ class Fan(Device):
         self.mode = FanSpeedMode.STEP if max_step else FanSpeedMode.PERCENT
         self.max_step = max_step
 
+        # If there is a dedicated switch GA, it controls the on/off behavior of the fan.
+        # Otherwise the speed GA of the fan implicitly controls the on/off behavior instead.
+        # `self.switch.initialized`` can be used to check which setup is used.
+        self.switch = RemoteValueSwitch(
+            xknx,
+            group_address_switch,
+            group_address_switch_state,
+            sync_state=sync_state,
+            device_name=self.name,
+            feature_name="Switch",
+            after_update_cb=self.after_update,
+        )
+
         if self.mode == FanSpeedMode.STEP:
             self.speed = RemoteValueDptValue1Ucount(
                 xknx,
@@ -93,15 +110,40 @@ class Fan(Device):
 
     def _iter_remote_values(self) -> Iterator[RemoteValue[Any, Any]]:
         """Iterate the devices RemoteValue classes."""
-        yield from (self.speed, self.oscillation)
+        yield from (self.switch, self.speed, self.oscillation)
 
     @property
     def supports_oscillation(self) -> bool:
         """Return if fan supports oscillation."""
         return self.oscillation.initialized
 
+    @property
+    def is_on(self) -> bool:
+        """Return the current fan state of the device."""
+        if self.switch.initialized:
+            return bool(self.switch.value)
+        return bool(self.current_speed)
+
+    async def turn_on(self, speed: int | None = None) -> None:
+        """Turn on fan."""
+        if self.switch.initialized:
+            await self.switch.on()
+            # For a switch GA fan, we only use an explicitly provided speed, but not
+            # arbitrarily set a default speed here, compared to the speed GA based fans below.
+            if speed is not None:
+                await self.set_speed(speed)
+        else:
+            await self.set_speed(speed or DEFAULT_TURN_ON_SPEED)
+
+    async def turn_off(self) -> None:
+        """Turn off fan."""
+        if self.switch.initialized:
+            await self.switch.off()
+        else:
+            await self.set_speed(0)
+
     async def set_speed(self, speed: int) -> None:
-        """Set the fan to a desginated speed."""
+        """Set the fan to a designated speed."""
         await self.speed.set(speed)
 
     async def set_oscillation(self, oscillation: bool) -> None:
@@ -110,6 +152,7 @@ class Fan(Device):
 
     async def process_group_write(self, telegram: "Telegram") -> None:
         """Process incoming and outgoing GROUP WRITE telegram."""
+        await self.switch.process(telegram)
         await self.speed.process(telegram)
         await self.oscillation.process(telegram)
 


Test patch:
diff --git a/requirements/testing.txt b/requirements/testing.txt
index e1fb7034..4c7f9c2b 100644
--- a/requirements/testing.txt
+++ b/requirements/testing.txt
@@ -10,4 +10,4 @@ pytest-asyncio==0.18.3
 pytest-cov==3.0.0
 setuptools==62.3.2
 tox==3.25.0
-mypy==0.960
+mypy==0.961
diff --git a/test/devices_tests/fan_test.py b/test/devices_tests/fan_test.py
index 7ae82a09..cc4c2a60 100644
--- a/test/devices_tests/fan_test.py
+++ b/test/devices_tests/fan_test.py
@@ -60,6 +60,83 @@ class TestFan:
         )
 
     #
+    # TEST SWITCH ON/OFF
+    #
+    async def test_switch_on_off(self):
+        """Test switching on/off of a Fan."""
+        xknx = XKNX()
+        fan = Fan(xknx, name="TestFan", group_address_speed="1/2/3")
+
+        # Turn the fan on via speed GA. First try without providing a speed,
+        # which will set it to the default 50% percentage.
+        await fan.turn_on()
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        # 128 is 50% as byte (0...255)
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(128)),
+        )
+
+        # Try again, but this time with a speed provided
+        await fan.turn_on(55)
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        # 140 is 55% as byte (0...255)
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(140)),
+        )
+
+        # Turn the fan off via the speed GA
+        await fan.turn_off()
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(0)),
+        )
+
+        fan_with_switch = Fan(
+            xknx,
+            name="TestFanSwitch",
+            group_address_speed="1/2/3",
+            group_address_switch="4/5/6",
+        )
+
+        # Turn the fan on via the switch GA, which should not adjust the speed
+        await fan_with_switch.turn_on()
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("4/5/6"),
+            payload=GroupValueWrite(DPTBinary(1)),
+        )
+
+        # Turn the fan off via the switch GA
+        await fan_with_switch.turn_off()
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("4/5/6"),
+            payload=GroupValueWrite(DPTBinary(0)),
+        )
+
+        # Turn the fan on again this time with a provided speed, which for a switch GA fan
+        # should result in separate telegrams to switch on the fan and then set the speed.
+        await fan_with_switch.turn_on(55)
+        assert xknx.telegrams.qsize() == 2
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("4/5/6"),
+            payload=GroupValueWrite(DPTBinary(1)),
+        )
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(140)),
+        )
+
     #
     # TEST SET SPEED
     #
@@ -75,8 +152,51 @@ class TestFan:
             destination_address=GroupAddress("1/2/3"),
             payload=GroupValueWrite(DPTArray(140)),
         )
+        await fan.process(telegram)
+        assert fan.is_on == True
+
+        # A speed of 0 will turn off the fan implicitly if there is no
+        # dedicated switch GA
+        await fan.set_speed(0)
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        # 140 is 55% as byte (0...255)
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(0)),
+        )
+        await fan.process(telegram)
+        assert fan.is_on == False
+
+        fan_with_switch = Fan(
+            xknx,
+            name="TestFan",
+            group_address_speed="1/2/3",
+            group_address_switch="4/5/6",
+        )
+        await fan_with_switch.turn_on()
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("4/5/6"),
+            payload=GroupValueWrite(DPTBinary(1)),
+        )
+        await fan_with_switch.process(telegram)
+        assert fan_with_switch.is_on == True
+
+        # A speed of 0 will not turn off the fan implicitly if there is a
+        # dedicated switch GA defined. So we only expect a speed change telegram,
+        # but no state switch one.
+        await fan_with_switch.set_speed(0)
+        assert xknx.telegrams.qsize() == 1
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(0)),
+        )
+        await fan_with_switch.process(telegram)
+        assert fan_with_switch.is_on == True
 
-    #
     #
     # TEST SET SPEED STEP
     #
@@ -98,7 +218,6 @@ class TestFan:
             payload=GroupValueWrite(DPTArray(2)),
         )
 
-    #
     #
     # TEST SET OSCILLATION
     #
@@ -120,12 +239,13 @@ class TestFan:
         )
 
     #
-    # TEST PROCESS
+    # TEST PROCESS SPEED
     #
     async def test_process_speed(self):
         """Test process / reading telegrams from telegram queue. Test if speed is processed."""
         xknx = XKNX()
         fan = Fan(xknx, name="TestFan", group_address_speed="1/2/3")
+        assert fan.is_on is False
         assert fan.current_speed is None
 
         # 140 is 55% as byte (0...255)
@@ -134,8 +254,20 @@ class TestFan:
             payload=GroupValueWrite(DPTArray(140)),
         )
         await fan.process(telegram)
+        # Setting a speed for a fan that has no dedicated switch GA,
+        # should turn on the fan.
+        assert fan.is_on is True
         assert fan.current_speed == 55
 
+        # Now set a speed of zero which should turn off the fan.
+        telegram = Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(0)),
+        )
+        await fan.process(telegram)
+        assert fan.is_on is False
+        assert fan.current_speed == 0
+
     async def test_process_speed_wrong_payload(self):
         """Test process wrong telegrams. (wrong payload type)."""
         xknx = XKNX()
@@ -152,6 +284,69 @@ class TestFan:
             log_mock.assert_called_once()
             cb_mock.assert_not_called()
 
+    #
+    # TEST PROCESS SWITCH
+    #
+    async def test_process_switch(self):
+        """Test process / reading telegrams from telegram queue. Test if switch is handled correctly."""
+        xknx = XKNX()
+        fan = Fan(
+            xknx,
+            name="TestFan",
+            group_address_speed="1/2/3",
+            group_address_switch="4/5/6",
+        )
+        assert fan.is_on is False
+        assert fan.current_speed is None
+
+        # 140 is 55% as byte (0...255)
+        telegram = Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(140)),
+        )
+        await fan.process(telegram)
+        # Setting a speed for a fan with dedicated switch GA,
+        # should not turn on the fan
+        assert fan.is_on is False
+        assert fan.current_speed == 55
+
+        # Now turn on the fan via its switch GA
+        telegram = Telegram(
+            destination_address=GroupAddress("4/5/6"),
+            payload=GroupValueWrite(DPTBinary(1)),
+        )
+        await fan.process(telegram)
+        assert fan.is_on is True
+        assert fan.current_speed == 55
+
+        # Setting a speed of 0 should not turn off the fan
+        telegram = Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(0)),
+        )
+        await fan.process(telegram)
+        assert fan.is_on is True
+        assert fan.current_speed == 0
+
+        # Set the speed again so we can verify that switching off the fan does not
+        # modify the set speed
+        telegram = Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray(140)),
+        )
+        await fan.process(telegram)
+        assert fan.is_on is True
+        assert fan.current_speed == 55
+
+        # Now turn off the fan via the dedicated switch GA
+        telegram = Telegram(
+            destination_address=GroupAddress("4/5/6"),
+            payload=GroupValueWrite(DPTBinary(0)),
+        )
+        await fan.process(telegram)
+        assert fan.is_on is False
+        assert fan.current_speed == 55
+
     #
     # TEST PROCESS OSCILLATION
     #
@@ -221,9 +416,13 @@ class TestFan:
             group_address_speed_state="1/7/2",
             group_address_oscillation="1/6/1",
             group_address_oscillation_state="1/6/2",
+            group_address_switch="1/5/1",
+            group_address_switch_state="1/5/2",
         )
         assert fan.has_group_address(GroupAddress("1/7/1"))
         assert fan.has_group_address(GroupAddress("1/7/2"))
         assert not fan.has_group_address(GroupAddress("1/7/3"))
         assert fan.has_group_address(GroupAddress("1/6/1"))
         assert fan.has_group_address(GroupAddress("1/6/2"))
+        assert fan.has_group_address(GroupAddress("1/5/1"))
+        assert fan.has_group_address(GroupAddress("1/5/2"))