Repository: XKNX/xknx
Pull request #913 (2022-04-11 08:42:04)

Properly shutdown ClimateMode if we shutdown Climate
<!--
  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.
-->
Also shutdown ClimateMode if we call Climate.shutdown(). Those two types of devices inevitably belong together and should be treated like that as well. A Climate can exist without a ClimateMode but there is no reason a ClimateMode shall exist without a Climate.


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.
-->

- [x] 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)
- [ ] This change requires a documentation update

## Checklist:

- [x] I have performed a self-review of my own code
- [x] The documentation has been adjusted accordingly
- [x] The changes generate no new warnings
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog
- [ ] The Homeassistant plugin has been adjusted in case of new config options


codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/913?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#913](https://codecov.io/gh/XKNX/xknx/pull/913?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (1a63141) into [main](https://codecov.io/gh/XKNX/xknx/commit/44c1c79f9ba2a3465266189e7ecd4dc9acd6573e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (44c1c79) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

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

```diff
@@           Coverage Diff           @@
##             main     #913   +/-   ##
=======================================
  Coverage   95.61%   95.61%           
=======================================
  Files         131      131           
  Lines        7803     7808    +5     
=======================================
+ Hits         7461     7466    +5     
  Misses        342      342           
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/devices/climate.py](https://codecov.io/gh/XKNX/xknx/pull/913/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2NsaW1hdGUucHk=) | `100.00% <100.00%> (ø)` | |
| [xknx/devices/device.py](https://codecov.io/gh/XKNX/xknx/pull/913/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2RldmljZS5weQ==) | `100.00% <100.00%> (ø)` | |


Patch:
diff --git a/changelog.md b/changelog.md
index 10751dbb..972b46f8 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased changes
+
+### Bug fixes
+
+- Properly shutdown climate mode if climate.shutdown() is called and ClimateMode exists
+
 ## 0.20.1 Add support for DPT 16.001 and SearchRequestExtended
 
 ### Features
diff --git a/docs/climate.md b/docs/climate.md
index dc0dcc9b..1bfaab99 100644
--- a/docs/climate.md
+++ b/docs/climate.md
@@ -94,6 +94,8 @@ await climate.set_setpoint_shift(1)
 # Reading climate device
 await climate.sync(wait_for_result=True)
 print("Current temperature: ", climate.temperature)
+# Shutdown the Climate and the underlying ClimateMode!
+climate.shutdown()
 ```
 
 ## [](#header-2)Example
diff --git a/xknx/devices/climate.py b/xknx/devices/climate.py
index 8af21c7f..e6725476 100644
--- a/xknx/devices/climate.py
+++ b/xknx/devices/climate.py
@@ -173,6 +173,12 @@ class Climate(Device):
         """Set power status to off."""
         await self.on.off()
 
+    def shutdown(self) -> None:
+        """Shutdown this device and the underlying mode."""
+        super().shutdown()
+        if self.mode:
+            self.mode.shutdown()
+
     @property
     def initialized_for_setpoint_shift_calculations(self) -> bool:
         """Test if object is initialized for setpoint shift calculations."""
diff --git a/xknx/devices/device.py b/xknx/devices/device.py
index 1eef32f0..8dc192bc 100644
--- a/xknx/devices/device.py
+++ b/xknx/devices/device.py
@@ -78,7 +78,8 @@ class Device(ABC):
         self, device_updated_cb: DeviceCallbackType
     ) -> None:
         """Unregister device updated callback."""
-        self.device_updated_cbs.remove(device_updated_cb)
+        if device_updated_cb in self.device_updated_cbs:
+            self.device_updated_cbs.remove(device_updated_cb)
 
     async def after_update(self) -> None:
         """Execute callbacks after internal state has been changed."""


Test patch:
diff --git a/test/devices_tests/climate_test.py b/test/devices_tests/climate_test.py
index 93b00549..f1a9a37f 100644
--- a/test/devices_tests/climate_test.py
+++ b/test/devices_tests/climate_test.py
@@ -159,6 +159,33 @@ class TestClimate:
         after_update_callback.assert_called_with(climate)
         after_update_callback.reset_mock()
 
+    def test_remove_climate_removes_climate_mode(self):
+        """Test shutting down climate will also shut down related ClimateMode."""
+
+        xknx = XKNX()
+        climate_mode = ClimateMode(
+            xknx,
+            name=None,
+            group_address_operation_mode="1/2/4",
+            group_address_operation_mode_state="1/2/5",
+            group_address_operation_mode_protection="1/2/6",
+            group_address_operation_mode_night="1/2/7",
+            group_address_operation_mode_comfort="1/2/8",
+            group_address_operation_mode_standby="1/2/9",
+            group_address_controller_status="1/2/10",
+            group_address_controller_status_state="1/2/11",
+            group_address_controller_mode="1/2/12",
+            group_address_controller_mode_state="1/2/13",
+            group_address_heat_cool="1/2/14",
+            group_address_heat_cool_state="1/2/15",
+        )
+
+        climate = Climate(xknx, name="TestClimate", mode=climate_mode)
+
+        assert len(xknx.devices) == 2
+        climate.shutdown()
+        assert len(xknx.devices) == 0
+
     async def test_process_callback_mode(self):
         """Test if after_update_callback is called after update of Climate object was changed."""