Repository: XKNX/xknx
Pull request #1545 (2024-08-12 22:57:46)

ClimateMode: filter modes for possible values and account for read-only modes
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description
<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->

- Restore `Climate.suppports_operation_mode` and `Climate.supports_controller_mode` to be `True` when read-only (like pre 3.0.0)
- Filter custom operation / controller modes for available settable modes
- For binary operation modes, only list configured modes and `Standby` in `operation_modes`

Fixes # (issue)

## 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)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Code quality improvements to existing code or addition of tests

## Checklist

- [ ] 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://app.codecov.io/gh/XKNX/xknx/pull/1545?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 96.91%. Comparing base [(`5ce2345`)](https://app.codecov.io/gh/XKNX/xknx/commit/5ce2345d5d80b11a4459b997ffd7a69201f47660?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) to head [(`6a26701`)](https://app.codecov.io/gh/XKNX/xknx/commit/6a267018e06e085ab371921225451d4314e17aa1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX).

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/XKNX/xknx/pull/1545/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://app.codecov.io/gh/XKNX/xknx/pull/1545?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@            Coverage Diff             @@
##             main    #1545      +/-   ##
==========================================
+ Coverage   96.88%   96.91%   +0.02%     
==========================================
  Files         157      157              
  Lines       10556    10554       -2     
==========================================
+ Hits        10227    10228       +1     
+ Misses        329      326       -3     
```

| [Files](https://app.codecov.io/gh/XKNX/xknx/pull/1545?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/devices/climate\_mode.py](https://app.codecov.io/gh/XKNX/xknx/pull/1545?src=pr&el=tree&filepath=xknx%2Fdevices%2Fclimate_mode.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2NsaW1hdGVfbW9kZS5weQ==) | `100.00% <100.00%> (+0.91%)` | :arrow_up: |
| [xknx/remote\_value/remote\_value\_climate\_mode.py](https://app.codecov.io/gh/XKNX/xknx/pull/1545?src=pr&el=tree&filepath=xknx%2Fremote_value%2Fremote_value_climate_mode.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9yZW1vdGVfdmFsdWUvcmVtb3RlX3ZhbHVlX2NsaW1hdGVfbW9kZS5weQ==) | `95.83% <100.00%> (+1.47%)` | :arrow_up: |

</details>

Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index ed7c7a9a..16b55d8a 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -12,6 +12,12 @@ nav_order: 2
 
 - Add DPT 1 definitions (as of KNX Specification 03_07_02 version 02.02.01)
 
+### Devices
+
+- ClimateMode: Restore `Climate.suppports_operation_mode` and `Climate.supports_controller_mode` to be `True` when read-only (like pre 3.0.0)
+- ClimateMode: Filter custom operation modes for available settable modes
+- ClimateMode: For binary operation modes, only list configured modes and `Standby` in `operation_modes`
+
 ### Bugfixes
 
 - Fix log message for DPT decoding errors in `GroupAddressDPT` parsing
diff --git a/xknx/devices/climate_mode.py b/xknx/devices/climate_mode.py
index 2e106468..d6f6d625 100644
--- a/xknx/devices/climate_mode.py
+++ b/xknx/devices/climate_mode.py
@@ -139,32 +139,42 @@ class ClimateMode(Device):
         self.operation_mode = HVACOperationMode.STANDBY
         self.controller_mode = HVACControllerMode.HEAT
 
-        self._operation_modes: list[HVACOperationMode] = []
-        if operation_modes is None:
-            self._operation_modes = self.gather_operation_modes()
-        else:
+        self._operation_modes = self.gather_operation_modes(only_writable=True)
+        if operation_modes is not None:
+            custom_operation_modes = []
             for op_mode in operation_modes:
                 if isinstance(op_mode, str):
-                    self._operation_modes.append(
+                    custom_operation_modes.append(
                         HVACOperationMode[op_mode.replace(" ", "_").upper()]
                     )
                 elif isinstance(op_mode, HVACOperationMode):
-                    self._operation_modes.append(op_mode)
+                    custom_operation_modes.append(op_mode)
+            self._operation_modes = [
+                mode for mode in custom_operation_modes if mode in self._operation_modes
+            ]
 
-        self._controller_modes: list[HVACControllerMode] = []
-        if controller_modes is None:
-            self._controller_modes = self.gather_controller_modes()
-        else:
+        self._controller_modes = self.gather_controller_modes(only_writable=True)
+        if controller_modes is not None:
+            custom_controller_modes = []
             for ct_mode in controller_modes:
                 if isinstance(ct_mode, str):
-                    self._controller_modes.append(
+                    custom_controller_modes.append(
                         HVACControllerMode[ct_mode.replace(" ", "_").upper()]
                     )
                 elif isinstance(ct_mode, HVACControllerMode):
-                    self._controller_modes.append(ct_mode)
+                    custom_controller_modes.append(ct_mode)
+            self._controller_modes = [
+                mode
+                for mode in custom_controller_modes
+                if mode in self._controller_modes
+            ]
 
-        self.supports_operation_mode = bool(self._operation_modes)
-        self.supports_controller_mode = bool(self._controller_modes)
+        self.supports_operation_mode = bool(
+            self.gather_operation_modes(only_writable=False)
+        )
+        self.supports_controller_mode = bool(
+            self.gather_controller_modes(only_writable=False)
+        )
 
     def _iter_remote_values(
         self,
@@ -247,31 +257,35 @@ class ClimateMode(Device):
     @property
     def operation_modes(self) -> list[HVACOperationMode]:
         """Return all configured operation modes."""
-        if not self.supports_operation_mode:
-            return []
         return self._operation_modes
 
     @property
     def controller_modes(self) -> list[HVACControllerMode]:
         """Return all configured controller modes."""
-        if not self.supports_controller_mode:
-            return []
         return self._controller_modes
 
-    def gather_operation_modes(self) -> list[HVACOperationMode]:
+    def gather_operation_modes(
+        self, only_writable: bool = True
+    ) -> list[HVACOperationMode]:
         """Gather operation modes from RemoteValues."""
         operation_modes: list[HVACOperationMode] = []
         for rv in self._iter_remote_values():
-            if rv.writable:
+            if rv.initialized:
+                if only_writable and not rv.writable:
+                    continue
                 operation_modes.extend(rv.supported_operation_modes())
         # remove duplicates
         return list(set(operation_modes))
 
-    def gather_controller_modes(self) -> list[HVACControllerMode]:
+    def gather_controller_modes(
+        self, only_writable: bool = True
+    ) -> list[HVACControllerMode]:
         """Gather controller modes from RemoteValues."""
         controller_modes: list[HVACControllerMode] = []
         for rv in self._iter_remote_values():
-            if rv.writable:
+            if rv.initialized:
+                if only_writable and not rv.writable:
+                    continue
                 controller_modes.extend(rv.supported_controller_modes())
         # remove duplicates
         return list(set(controller_modes))
diff --git a/xknx/remote_value/remote_value_climate_mode.py b/xknx/remote_value/remote_value_climate_mode.py
index e4a9ce18..2b93ecc6 100644
--- a/xknx/remote_value/remote_value_climate_mode.py
+++ b/xknx/remote_value/remote_value_climate_mode.py
@@ -251,13 +251,6 @@ class RemoteValueBinaryOperationMode(
                 device_name=str(device_name),
                 feature_name=feature_name,
             )
-        if operation_mode not in self.supported_operation_modes():
-            raise ConversionError(
-                "Operation mode not supported for binary mode object",
-                operation_mode=str(operation_mode),
-                device_name=str(device_name),
-                feature_name=feature_name,
-            )
         self.operation_mode = operation_mode
         super().__init__(
             xknx,
@@ -271,17 +264,15 @@ class RemoteValueBinaryOperationMode(
 
     def supported_operation_modes(self) -> list[HVACOperationMode]:
         """Return a list of all supported operation modes."""
-        return [
-            HVACOperationMode.COMFORT,
-            HVACOperationMode.BUILDING_PROTECTION,
-            HVACOperationMode.ECONOMY,
-            HVACOperationMode.STANDBY,
-        ]
+        # all binary operation modes off -> Standby according to MDT
+        return (
+            [self.operation_mode, HVACOperationMode.STANDBY]
+            if self.operation_mode is not HVACOperationMode.STANDBY
+            else [HVACOperationMode.STANDBY]
+        )
 
     def set_operation_mode(self, mode: HVACOperationMode) -> None:
         """Set operation mode. Return if not supported."""
-        if mode not in self.supported_operation_modes():
-            return
         super().set(mode)
 
     def supported_controller_modes(self) -> list[HVACControllerMode]:


Test patch:
diff --git a/test/devices_tests/climate_test.py b/test/devices_tests/climate_test.py
index 24077d37..62004811 100644
--- a/test/devices_tests/climate_test.py
+++ b/test/devices_tests/climate_test.py
@@ -1308,9 +1308,9 @@ class TestClimate:
 
         assert set(climate_mode.operation_modes) == {
             HVACOperationMode.COMFORT,
-            HVACOperationMode.STANDBY,
             HVACOperationMode.ECONOMY,
             HVACOperationMode.BUILDING_PROTECTION,
+            HVACOperationMode.STANDBY,
         }
 
     def test_supported_operation_modes_only_economy(self):
@@ -1319,16 +1319,15 @@ class TestClimate:
         climate_mode = ClimateMode(
             xknx, "TestClimate", group_address_operation_mode_economy="1/2/7"
         )
-        # If one binary climate object is set, all 4 operation modes are supported.
+        # If one binary climate object is set, this mode and Standby are supported.
+        # All binary modes off -> Standby according to MDT heating actuator
         assert set(climate_mode.operation_modes) == {
-            HVACOperationMode.STANDBY,
             HVACOperationMode.ECONOMY,
-            HVACOperationMode.COMFORT,
-            HVACOperationMode.BUILDING_PROTECTION,
+            HVACOperationMode.STANDBY,
         }
 
     def test_supported_operation_modes_heat_cool(self):
-        """Test get_supported_operation_modes with heat_cool group address."""
+        """Test supported controller_modes with heat_cool address."""
         xknx = XKNX()
         climate_mode = ClimateMode(
             xknx,
@@ -1336,11 +1335,27 @@ class TestClimate:
             group_address_heat_cool="1/2/14",
             group_address_heat_cool_state="1/2/15",
         )
+        assert climate_mode.supports_controller_mode
         assert set(climate_mode.controller_modes) == {
             HVACControllerMode.HEAT,
             HVACControllerMode.COOL,
         }
 
+    def test_supported_operation_modes_heat_cool_read_only(self):
+        """Test supported controller_modes with heat_cool_state address."""
+        xknx = XKNX()
+        climate_mode = ClimateMode(
+            xknx,
+            "TestClimate",
+            group_address_heat_cool_state="1/2/15",
+        )
+        assert climate_mode.supports_controller_mode
+        assert climate_mode.controller_modes == []  # only writable modes
+        assert set(climate_mode.gather_controller_modes(only_writable=False)) == {
+            HVACControllerMode.HEAT,
+            HVACControllerMode.COOL,
+        }
+
     def test_custom_supported_operation_modes(self):
         """Test get_supported_operation_modes with custom mode."""
         modes = [HVACOperationMode.STANDBY, HVACOperationMode.ECONOMY]
@@ -1366,6 +1381,24 @@ class TestClimate:
         )
         assert climate_mode.operation_modes == modes
 
+    def test_custom_supported_operation_modes_only_valid(self):
+        """Test get_supported_operation_modes with custom mode as str list."""
+        str_modes = [
+            "Standby",
+            "Building Protection",
+            "Comfort",  # Comfort can't be set - no address set
+        ]
+        modes = [HVACOperationMode.STANDBY, HVACOperationMode.BUILDING_PROTECTION]
+        xknx = XKNX()
+        climate_mode = ClimateMode(
+            xknx,
+            "TestClimate",
+            group_address_operation_mode_standby="1/2/7",
+            group_address_operation_mode_protection="1/2/8",
+            operation_modes=str_modes,
+        )
+        assert climate_mode.operation_modes == modes
+
     def test_custom_supported_controller_modes_as_str(self):
         """Test get_supported_operation_modes with custom mode as str list."""
         str_modes = ["Heat", "Cool", HVACControllerMode.NODEM]
@@ -1386,10 +1419,7 @@ class TestClimate:
     def test_custom_supported_controller_modes_when_controller_mode_unsupported(self):
         """Test get_supported_operation_modes with custom mode as str list."""
         str_modes = ["Heat", "Cool"]
-        modes = [
-            HVACControllerMode.HEAT,
-            HVACControllerMode.COOL,
-        ]
+        modes = []
         xknx = XKNX()
         climate_mode = ClimateMode(
             xknx,