From 8cc78f236968925b23a5325e954e8f5172751220 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Sun, 8 Dec 2024 13:52:47 +0100 Subject: [PATCH 1/9] Break out common bit of actuator locating --- plugwise/helper.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 20fda20f0..84f601bfd 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -68,6 +68,15 @@ from packaging import version +def search_actuator_functionalities(appliance: etree, actuator: str) -> etree | None: + """Helper-function for finding the relevant xml-structure.""" + locator = f"./actuator_functionalities/{actuator}" + if (search := appliance.find(locator)) is not None: + return search + + return None + + class SmileComm: """The SmileComm class.""" @@ -455,8 +464,9 @@ def _appl_gateway_info(self, appl: Munch, appliance: etree) -> Munch: def _appl_regulation_mode_info(self, appliance: etree) -> None: """Helper-function for _appliance_info_finder().""" reg_mode_list: list[str] = [] - locator = "./actuator_functionalities/regulation_mode_control_functionality" - if (search := appliance.find(locator)) is not None: + if (search := search_actuator_functionalities( + appliance, "regulation_mode_control_functionality" + )) is not None: if search.find("allowed_modes") is not None: for mode in search.find("allowed_modes"): reg_mode_list.append(mode.text) @@ -470,10 +480,9 @@ def _appl_dhw_mode_info(self, appl: Munch, appliance: etree) -> Munch: Collect dhw control operation modes - Anna + Loria. """ dhw_mode_list: list[str] = [] - locator = ( - "./actuator_functionalities/domestic_hot_water_mode_control_functionality" - ) - if (search := appliance.find(locator)) is not None: + if (search := search_actuator_functionalities( + appliance, "domestic_hot_water_mode_control_functionality" + )) is not None: if search.find("allowed_modes") is not None: for mode in search.find("allowed_modes"): dhw_mode_list.append(mode.text) @@ -711,8 +720,9 @@ def _get_regulation_mode( if not (self.smile(ADAM) and entity_id == self.gateway_id): return - locator = "./actuator_functionalities/regulation_mode_control_functionality" - if (search := appliance.find(locator)) is not None: + if (search := search_actuator_functionalities( + appliance, "regulation_mode_control_functionality" + )) is not None: data["select_regulation_mode"] = search.find("mode").text self._count += 1 self._cooling_enabled = data["select_regulation_mode"] == "cooling" @@ -727,8 +737,9 @@ def _get_gateway_mode( if not (self.smile(ADAM) and entity_id == self.gateway_id): return - locator = "./actuator_functionalities/gateway_mode_control_functionality" - if (search := appliance.find(locator)) is not None: + if (search := search_actuator_functionalities( + appliance, "gateway_mode_control_functionality" + )) is not None: data["select_gateway_mode"] = search.find("mode").text self._count += 1 From a76a36bae519e45cfaf149d0c378f6ce5222c260 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Sun, 8 Dec 2024 15:51:16 +0100 Subject: [PATCH 2/9] Avoid double search.find() --- plugwise/helper.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 84f601bfd..7986fa26f 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -69,7 +69,7 @@ def search_actuator_functionalities(appliance: etree, actuator: str) -> etree | None: - """Helper-function for finding the relevant xml-structure.""" + """Helper-function for finding the relevant actuator xml-structure.""" locator = f"./actuator_functionalities/{actuator}" if (search := appliance.find(locator)) is not None: return search @@ -461,14 +461,16 @@ def _appl_gateway_info(self, appl: Munch, appliance: etree) -> Munch: return appl + def _appl_actuator_modes() + def _appl_regulation_mode_info(self, appliance: etree) -> None: """Helper-function for _appliance_info_finder().""" reg_mode_list: list[str] = [] if (search := search_actuator_functionalities( appliance, "regulation_mode_control_functionality" )) is not None: - if search.find("allowed_modes") is not None: - for mode in search.find("allowed_modes"): + if (modes := search.find("allowed_modes")) is not None: + for mode in modes: reg_mode_list.append(mode.text) if mode.text == "cooling": self._cooling_present = True @@ -483,8 +485,8 @@ def _appl_dhw_mode_info(self, appl: Munch, appliance: etree) -> Munch: if (search := search_actuator_functionalities( appliance, "domestic_hot_water_mode_control_functionality" )) is not None: - if search.find("allowed_modes") is not None: - for mode in search.find("allowed_modes"): + if (modes := search.find("allowed_modes")) is not None: + for mode in modes: dhw_mode_list.append(mode.text) self._dhw_allowed_modes = dhw_mode_list From 359a36a133466bc2ae519b1cb18e2ae8b44d158c Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Sun, 8 Dec 2024 16:31:54 +0100 Subject: [PATCH 3/9] Breakout _get_appl_actuator_modes() function --- plugwise/helper.py | 42 +++++++++++++----------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 7986fa26f..53c421b47 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -404,10 +404,10 @@ def _appliance_info_finder(self, appl: Munch, appliance: etree) -> Munch: return self._appl_thermostat_info(appl, appliance) case "heater_central": # Collect heater_central device info - self._appl_heater_central_info( - appl, appliance, False - ) # False means non-legacy device - self._appl_dhw_mode_info(appl, appliance) + self._appl_heater_central_info(appl, appliance, False) # False means non-legacy device + self._dhw_allowed_modes = self._get_appl_actuator_modes( + appliance, "domestic_hot_water_mode_control_functionality" + ) # Skip orphaned heater_central (Core Issue #104433) if appl.entity_id != self._heater_id: return Munch() @@ -450,7 +450,9 @@ def _appl_gateway_info(self, appl: Munch, appliance: etree) -> Munch: appl.zigbee_mac = found.find("mac_address").text # Also, collect regulation_modes and check for cooling, indicating cooling-mode is present - self._appl_regulation_mode_info(appliance) + self._reg_allowed_modes = self._get_appl_actuator_modes( + appliance, "regulation_mode_control_functionality" + ) # Finally, collect the gateway_modes self._gw_allowed_modes = [] @@ -461,36 +463,18 @@ def _appl_gateway_info(self, appl: Munch, appliance: etree) -> Munch: return appl - def _appl_actuator_modes() - - def _appl_regulation_mode_info(self, appliance: etree) -> None: + def _get_appl_actuator_modes(self, appliance: etree, actuator_type: str) -> list[str]: """Helper-function for _appliance_info_finder().""" - reg_mode_list: list[str] = [] - if (search := search_actuator_functionalities( - appliance, "regulation_mode_control_functionality" - )) is not None: + mode_list: list[str] = [] + if (search := search_actuator_functionalities(appliance, actuator_type)) is not None: if (modes := search.find("allowed_modes")) is not None: for mode in modes: - reg_mode_list.append(mode.text) + mode_list.append(mode.text) + # Collect cooling_present state from the available regulation_modes if mode.text == "cooling": self._cooling_present = True - self._reg_allowed_modes = reg_mode_list - - def _appl_dhw_mode_info(self, appl: Munch, appliance: etree) -> Munch: - """Helper-function for _appliance_info_finder(). - Collect dhw control operation modes - Anna + Loria. - """ - dhw_mode_list: list[str] = [] - if (search := search_actuator_functionalities( - appliance, "domestic_hot_water_mode_control_functionality" - )) is not None: - if (modes := search.find("allowed_modes")) is not None: - for mode in modes: - dhw_mode_list.append(mode.text) - self._dhw_allowed_modes = dhw_mode_list - - return appl + return mode_list def _get_appliances_with_offset_functionality(self) -> list[str]: """Helper-function collecting all appliance that have offset_functionality.""" From d0cb08cb1a4ac0a584cc7dd84f05fb854e134ed0 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Sun, 8 Dec 2024 16:33:31 +0100 Subject: [PATCH 4/9] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 824441db6..f2a81850a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Ongoing + +- Code optimizing + ## v1.6.3 - Implement cooling-related fixes, trying to solve HA Core issue [#132479](https://github.com/home-assistant/core/issues/132479) From 86eaf12b51f1b9e632de3f7fc3c5c2f751b2a0a3 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Sun, 8 Dec 2024 16:49:21 +0100 Subject: [PATCH 5/9] Optimize further --- plugwise/helper.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 53c421b47..863444887 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -696,6 +696,19 @@ def _get_actuator_functionalities( act_item = cast(ActuatorType, item) data[act_item] = temp_dict + def _get_actuator_mode(self, appliance: etree, entity_id: str, key: str) -> str | None: + """Helper-function for _get_regulation_mode and _get_gateway_mode. + + Collect the requested gateway mode. + """ + if not (self.smile(ADAM) and entity_id == self.gateway_id): + return None + + if (search := search_actuator_functionalities(appliance, key)) is not None: + return str(search.find("mode").text) + + return None + def _get_regulation_mode( self, appliance: etree, entity_id: str, data: GwEntityData ) -> None: @@ -703,15 +716,10 @@ def _get_regulation_mode( Adam: collect the gateway regulation_mode. """ - if not (self.smile(ADAM) and entity_id == self.gateway_id): - return - - if (search := search_actuator_functionalities( - appliance, "regulation_mode_control_functionality" - )) is not None: - data["select_regulation_mode"] = search.find("mode").text + if (mode := self._get_actuator_mode(appliance, entity_id, "regulation_mode_control_functionality")) is not None: + data["select_regulation_mode"] = mode self._count += 1 - self._cooling_enabled = data["select_regulation_mode"] == "cooling" + self._cooling_enabled = mode == "cooling" def _get_gateway_mode( self, appliance: etree, entity_id: str, data: GwEntityData @@ -720,13 +728,8 @@ def _get_gateway_mode( Adam: collect the gateway mode. """ - if not (self.smile(ADAM) and entity_id == self.gateway_id): - return - - if (search := search_actuator_functionalities( - appliance, "gateway_mode_control_functionality" - )) is not None: - data["select_gateway_mode"] = search.find("mode").text + if (mode := self._get_actuator_mode(appliance, entity_id, "gateway_mode_control_functionality")) is not None: + data["select_gateway_mode"] = mode self._count += 1 def _get_gateway_outdoor_temp(self, entity_id: str, data: GwEntityData) -> None: From a648c73eedaef1516e0bd9770833f74f4a3a640a Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 9 Dec 2024 08:34:01 +0100 Subject: [PATCH 6/9] Ruff formatting --- plugwise/helper.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 863444887..233e525f2 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -404,7 +404,9 @@ def _appliance_info_finder(self, appl: Munch, appliance: etree) -> Munch: return self._appl_thermostat_info(appl, appliance) case "heater_central": # Collect heater_central device info - self._appl_heater_central_info(appl, appliance, False) # False means non-legacy device + self._appl_heater_central_info( + appl, appliance, False + ) # False means non-legacy device self._dhw_allowed_modes = self._get_appl_actuator_modes( appliance, "domestic_hot_water_mode_control_functionality" ) @@ -463,10 +465,14 @@ def _appl_gateway_info(self, appl: Munch, appliance: etree) -> Munch: return appl - def _get_appl_actuator_modes(self, appliance: etree, actuator_type: str) -> list[str]: + def _get_appl_actuator_modes( + self, appliance: etree, actuator_type: str + ) -> list[str]: """Helper-function for _appliance_info_finder().""" mode_list: list[str] = [] - if (search := search_actuator_functionalities(appliance, actuator_type)) is not None: + if ( + search := search_actuator_functionalities(appliance, actuator_type) + ) is not None: if (modes := search.find("allowed_modes")) is not None: for mode in modes: mode_list.append(mode.text) @@ -696,7 +702,9 @@ def _get_actuator_functionalities( act_item = cast(ActuatorType, item) data[act_item] = temp_dict - def _get_actuator_mode(self, appliance: etree, entity_id: str, key: str) -> str | None: + def _get_actuator_mode( + self, appliance: etree, entity_id: str, key: str + ) -> str | None: """Helper-function for _get_regulation_mode and _get_gateway_mode. Collect the requested gateway mode. @@ -716,7 +724,11 @@ def _get_regulation_mode( Adam: collect the gateway regulation_mode. """ - if (mode := self._get_actuator_mode(appliance, entity_id, "regulation_mode_control_functionality")) is not None: + if ( + mode := self._get_actuator_mode( + appliance, entity_id, "regulation_mode_control_functionality" + ) + ) is not None: data["select_regulation_mode"] = mode self._count += 1 self._cooling_enabled = mode == "cooling" @@ -728,7 +740,11 @@ def _get_gateway_mode( Adam: collect the gateway mode. """ - if (mode := self._get_actuator_mode(appliance, entity_id, "gateway_mode_control_functionality")) is not None: + if ( + mode := self._get_actuator_mode( + appliance, entity_id, "gateway_mode_control_functionality" + ) + ) is not None: data["select_gateway_mode"] = mode self._count += 1 From f8d94bd5351bb1dc794bb090b58b6bde3cda2ce8 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 9 Dec 2024 08:47:19 +0100 Subject: [PATCH 7/9] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a81850a..fdf9682c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Ongoing -- Code optimizing +- Continuous improvements ## v1.6.3 From fc714e163e3b64ace2142bb6d078d135150c97e4 Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 9 Dec 2024 09:43:20 +0100 Subject: [PATCH 8/9] Fix issue reported by SonarCloud --- plugwise/helper.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 233e525f2..51257e552 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -472,13 +472,12 @@ def _get_appl_actuator_modes( mode_list: list[str] = [] if ( search := search_actuator_functionalities(appliance, actuator_type) - ) is not None: - if (modes := search.find("allowed_modes")) is not None: - for mode in modes: - mode_list.append(mode.text) - # Collect cooling_present state from the available regulation_modes - if mode.text == "cooling": - self._cooling_present = True + ) is not None and (modes := search.find("allowed_modes")) is not None: + for mode in modes: + mode_list.append(mode.text) + # Collect cooling_present state from the available regulation_modes + if mode.text == "cooling": + self._cooling_present = True return mode_list From d64b02e825481fb123a1256e563ebb15cf38ab3b Mon Sep 17 00:00:00 2001 From: Bouwe Westerdijk Date: Mon, 9 Dec 2024 09:58:37 +0100 Subject: [PATCH 9/9] Improve doc-string, implement suggested breakout function --- plugwise/helper.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 51257e552..cea2f130f 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -468,19 +468,22 @@ def _appl_gateway_info(self, appl: Munch, appliance: etree) -> Munch: def _get_appl_actuator_modes( self, appliance: etree, actuator_type: str ) -> list[str]: - """Helper-function for _appliance_info_finder().""" + """Get allowed modes for the given actuator type.""" mode_list: list[str] = [] if ( search := search_actuator_functionalities(appliance, actuator_type) ) is not None and (modes := search.find("allowed_modes")) is not None: for mode in modes: mode_list.append(mode.text) - # Collect cooling_present state from the available regulation_modes - if mode.text == "cooling": - self._cooling_present = True + self._check_cooling_mode(mode.text) return mode_list + def _check_cooling_mode(self, mode: str) -> None: + """Check if cooling mode is present and update state.""" + if mode == "cooling": + self._cooling_present = True + def _get_appliances_with_offset_functionality(self) -> list[str]: """Helper-function collecting all appliance that have offset_functionality.""" therm_list: list[str] = [] @@ -645,7 +648,10 @@ def _get_plugwise_notifications(self) -> None: def _get_actuator_functionalities( self, xml: etree, entity: GwEntityData, data: GwEntityData ) -> None: - """Helper-function for _get_measurement_data().""" + """Get and process the actuator_functionalities details for an entity. + + Add the resulting dict(s) to the entity's data. + """ for item in ACTIVE_ACTUATORS: # Skip max_dhw_temperature, not initially valid, # skip thermostat for all but zones with thermostats