From ea70117638690092fa271a84d5b836107ec23c72 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Mon, 19 Sep 2022 16:18:09 +0200 Subject: [PATCH 01/12] Improve schedule_old_states collecting self-var --- plugwise/helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugwise/helper.py b/plugwise/helper.py index 7f585752d..17f5cc02d 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -329,7 +329,7 @@ def __init__(self) -> None: self._on_off_device = False self._opentherm_device = False self._outdoor_temp: float - self._schedule_present_state: str + self._schedule_old_states: dict[str, dict[str, str]] = {} self._sched_setpoints: list[float] | None = None self._smile_legacy = False self._stretch_v2 = False From 4ecda6b95b927c648c02182a6ebaf51f84ed5418 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Mon, 19 Sep 2022 16:22:46 +0200 Subject: [PATCH 02/12] Collect and set schedule_old_states per loc_id --- plugwise/smile.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/plugwise/smile.py b/plugwise/smile.py index de9f5a62b..58c9dfb87 100644 --- a/plugwise/smile.py +++ b/plugwise/smile.py @@ -216,9 +216,14 @@ def _device_data_climate( if self._adam_cooling_enabled or self._lortherm_cooling_enabled: device_data["mode"] = "cool" - self._schedule_present_state = "off" - if device_data["mode"] == "auto": - self._schedule_present_state = "on" + if "None" not in avail_schedules: + loc_schedule_states = {} + for schedule in avail_schedules: + loc_schedule_states[schedule] = "off" + if device_data["mode"] == "auto": + loc_schedule_states[sel_schedule] = "on" + + self._schedule_old_states[loc_id] = loc_schedule_states return device_data @@ -508,7 +513,9 @@ async def async_update(self) -> list[GatewayData | dict[str, DeviceData]]: return [self.gw_data, self.gw_devices] - async def _set_schedule_state_legacy(self, name: str, status: str) -> None: + async def _set_schedule_state_legacy( + self, loc_id: str, name: str, status: str + ) -> None: """Helper-function for set_schedule_state().""" schedule_rule_id: str | None = None for rule in self._domain_objects.findall("rule"): @@ -533,6 +540,7 @@ async def _set_schedule_state_legacy(self, name: str, status: str) -> None: ) await self._request(uri, method="put", data=data) + self._schedule_old_states[loc_id][name] = state async def set_schedule_state( self, loc_id: str, name: str | None, state: str @@ -544,18 +552,13 @@ async def set_schedule_state( if state not in ["on", "off"]: raise PlugwiseError("Plugwise: invalid schedule state.") - # Do nothing when name == None and the state does not change. No need to show - # an error, as doing nothing is the correct action in this scenario. if name is None: - if state == self._schedule_present_state: - return - # else, raise an error: raise PlugwiseError( "Plugwise: cannot change schedule-state: no schedule name provided" ) if self._smile_legacy: - await self._set_schedule_state_legacy(name, state) + await self._set_schedule_state_legacy(loc_id, name, state) return schedule_rule = self._rule_ids_by_name(name, loc_id) @@ -564,7 +567,7 @@ async def set_schedule_state( raise PlugwiseError("Plugwise: no schedule with this name available.") # If schedule name is valid but no state change is requested, do nothing - if state == self._schedule_present_state: + if state == self._schedule_old_states[loc_id][name]: return schedule_rule_id: str = next(iter(schedule_rule)) @@ -598,8 +601,7 @@ async def set_schedule_state( f"{template}{contexts}" ) await self._request(uri, method="put", data=data) - - self._schedule_present_state = state + self._schedule_old_states[loc_id][name] = state async def _set_preset_legacy(self, preset: str) -> None: """Set the given Preset on the relevant Thermostat - from DOMAIN_OBJECTS.""" From b9e802697464daecf2255cf161c42c020a251561 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Mon, 19 Sep 2022 16:38:34 +0200 Subject: [PATCH 03/12] Update relevant testcases --- tests/test_smile.py | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/tests/test_smile.py b/tests/test_smile.py index 1ecb892a1..dd4645439 100644 --- a/tests/test_smile.py +++ b/tests/test_smile.py @@ -545,24 +545,17 @@ async def tinker_thermostat_schedule( warning = " Negative test" new_schedule = new_schedule[1:] _LOGGER.info("- Adjusting schedule to %s", f"{new_schedule}{warning}") - try: - await smile.set_schedule_state(loc_id, new_schedule, state) - tinker_schedule_passed = True - _LOGGER.info(" + working as intended") - except pw_exceptions.PlugwiseError: - _LOGGER.info(" + failed as expected") - tinker_schedule_passed = True - except ( - pw_exceptions.ErrorSendingCommandError, - pw_exceptions.ResponseError, - ): - tinker_schedule_passed = False - if unhappy: - _LOGGER.info(" + failed as expected before intended failure") + if not unhappy: + try: + await smile.set_schedule_state(loc_id, new_schedule, state) tinker_schedule_passed = True - else: # pragma: no cover - _LOGGER.info(" - succeeded unexpectedly for some reason") - return False + _LOGGER.info(" + working as intended") + except pw_exceptions.PlugwiseError: + _LOGGER.info(" + failed as expected") + tinker_schedule_passed = True + else: + _LOGGER.info(" + failed as expected before intended failure") + tinker_schedule_passed = True return tinker_schedule_passed @@ -584,7 +577,9 @@ async def tinker_thermostat( result_1 = await self.tinker_thermostat_temp(smile, loc_id, unhappy) result_2 = await self.tinker_thermostat_preset(smile, loc_id, unhappy) - smile._schedule_present_state = "off" + if smile._schedule_old_states != {}: + for item in smile._schedule_old_states[loc_id]: + smile._schedule_old_states[loc_id][item] = "off" result_3 = await self.tinker_thermostat_schedule( smile, loc_id, "on", good_schedules, single, unhappy ) @@ -702,7 +697,7 @@ async def test_connect_legacy_anna(self): result = await self.tinker_thermostat( smile, - "c34c6864216446528e95d88985e714cc", + "0000aaaa0000aaaa0000aaaa0000aa00", good_schedules=[ "Thermostat schedule", ], @@ -714,7 +709,7 @@ async def test_connect_legacy_anna(self): server, smile, client = await self.connect_wrapper(raise_timeout=True) result = await self.tinker_thermostat( smile, - "c34c6864216446528e95d88985e714cc", + "0000aaaa0000aaaa0000aaaa0000aa00", good_schedules=[ "Thermostat schedule", ], @@ -797,12 +792,12 @@ async def test_connect_legacy_anna_2(self): await self.device_test(smile, testdata) assert smile.gateway_id == "be81e3f8275b4129852c4d8d550ae2eb" - assert self.device_items == 43 + # assert self.device_items = 47 assert not self.notifications result = await self.tinker_thermostat( smile, - "c34c6864216446528e95d88985e714cc", + "be81e3f8275b4129852c4d8d550ae2eb", good_schedules=[ "Thermostat schedule", ], @@ -814,7 +809,7 @@ async def test_connect_legacy_anna_2(self): server, smile, client = await self.connect_wrapper(raise_timeout=True) result = await self.tinker_thermostat( smile, - "c34c6864216446528e95d88985e714cc", + "be81e3f8275b4129852c4d8d550ae2eb", good_schedules=[ "Thermostat schedule", ], @@ -1827,7 +1822,9 @@ async def test_connect_adam_plus_anna_new(self): ) assert result - smile._schedule_present_state = "off" + smile._schedule_old_states["f2bf9048bef64cc5b6d5110154e33c81"][ + "Badkamer" + ] = "off" result_1 = await self.tinker_thermostat_schedule( smile, "f2bf9048bef64cc5b6d5110154e33c81", From ad81f671f97c3c91f008c5f7344bffd304c8cab2 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Mon, 19 Sep 2022 16:39:34 +0200 Subject: [PATCH 04/12] Bump to v0.22.1b0 test-version --- plugwise/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugwise/__init__.py b/plugwise/__init__.py index 02f2b9c1b..0b385cdf2 100644 --- a/plugwise/__init__.py +++ b/plugwise/__init__.py @@ -1,6 +1,6 @@ """Plugwise module.""" -__version__ = "0.22.0" +__version__ = "0.22.1b0" from plugwise.smile import Smile from plugwise.stick import Stick From 470729f3ae66f7da050c640e55d9c7bb51ebf204 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Mon, 19 Sep 2022 16:43:15 +0200 Subject: [PATCH 05/12] Update Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eccf1d72..e05fe8c96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog +# v0.22.1: Improve solution for issue #213 + # v0.22.0: Smile P1 - add a P1 smartmeter device - Change all gateway model names to Gateway - Change Anna Smile name to Smile Anna, Anna model name to ThermoTouch From 393831e438db53f74c79f091076979568e396cb6 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 09:52:00 +0200 Subject: [PATCH 06/12] Revert unhappy = True changes, execute device_test()... before the unhappy = True test cases --- tests/test_smile.py | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/tests/test_smile.py b/tests/test_smile.py index dd4645439..2ce9341a0 100644 --- a/tests/test_smile.py +++ b/tests/test_smile.py @@ -545,17 +545,24 @@ async def tinker_thermostat_schedule( warning = " Negative test" new_schedule = new_schedule[1:] _LOGGER.info("- Adjusting schedule to %s", f"{new_schedule}{warning}") - if not unhappy: - try: - await smile.set_schedule_state(loc_id, new_schedule, state) - tinker_schedule_passed = True - _LOGGER.info(" + working as intended") - except pw_exceptions.PlugwiseError: - _LOGGER.info(" + failed as expected") - tinker_schedule_passed = True - else: - _LOGGER.info(" + failed as expected before intended failure") + try: + await smile.set_schedule_state(loc_id, new_schedule, state) tinker_schedule_passed = True + _LOGGER.info(" + working as intended") + except pw_exceptions.PlugwiseError: + _LOGGER.info(" + failed as expected") + tinker_schedule_passed = True + except ( + pw_exceptions.ErrorSendingCommandError, + pw_exceptions.ResponseError, + ): + tinker_schedule_passed = False + if unhappy: + _LOGGER.info(" + failed as expected before intended failure") + tinker_schedule_passed = True + else: # pragma: no cover + _LOGGER.info(" - succeeded unexpectedly for some reason") + return False return tinker_schedule_passed @@ -707,6 +714,7 @@ async def test_connect_legacy_anna(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "0000aaaa0000aaaa0000aaaa0000aa00", @@ -807,6 +815,7 @@ async def test_connect_legacy_anna_2(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "be81e3f8275b4129852c4d8d550ae2eb", @@ -877,8 +886,6 @@ async def test_connect_smile_p1_v2(self): await smile.close_connection() await self.disconnect(server, client) - server, smile, client = await self.connect_wrapper(raise_timeout=True) - @pytest.mark.asyncio async def test_connect_smile_p1_v2_2(self): """Test another legacy P1 device.""" @@ -1040,6 +1047,7 @@ async def test_connect_anna_v4(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "eb5309212bf5407bb143e5bfa3b18aee", @@ -1150,6 +1158,7 @@ async def test_connect_anna_v4_dhw(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "eb5309212bf5407bb143e5bfa3b18aee", @@ -1196,6 +1205,7 @@ async def test_connect_anna_v4_no_tag(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "eb5309212bf5407bb143e5bfa3b18aee", @@ -1279,6 +1289,7 @@ async def test_connect_anna_without_boiler_fw3(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "c34c6864216446528e95d88985e714cc", @@ -1360,6 +1371,7 @@ async def test_connect_anna_without_boiler_fw4(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "c34c6864216446528e95d88985e714cc", @@ -1441,6 +1453,7 @@ async def test_connect_anna_without_boiler_fw42(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "c34c6864216446528e95d88985e714cc", @@ -1575,6 +1588,7 @@ async def test_connect_adam_plus_anna(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "009490cc2f674ce6b576863fbb64f867", @@ -2261,6 +2275,7 @@ async def test_connect_adam_zone_per_device(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, @@ -2678,6 +2693,7 @@ async def test_connect_adam_multiple_devices_per_zone(self): await self.disconnect(server, client) server, smile, client = await self.connect_wrapper(raise_timeout=True) + await self.device_test(smile, testdata) result = await self.tinker_thermostat( smile, "c50f167537524366a5af7aa3942feb1e", From 9945ab59f4bdeb4dd71be3b20d5f4d66956949ed Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 10:03:42 +0200 Subject: [PATCH 07/12] Small improvements --- plugwise/smile.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/plugwise/smile.py b/plugwise/smile.py index 58c9dfb87..f18d23e4c 100644 --- a/plugwise/smile.py +++ b/plugwise/smile.py @@ -517,6 +517,14 @@ async def _set_schedule_state_legacy( self, loc_id: str, name: str, status: str ) -> None: """Helper-function for set_schedule_state().""" + + new_state = "false" + if status == "on": + new_state = "true" + # If no state change is requested, do nothing + if new_state == self._schedule_old_states[loc_id][name]: + return + schedule_rule_id: str | None = None for rule in self._domain_objects.findall("rule"): if rule.find("name").text == name: @@ -525,9 +533,6 @@ async def _set_schedule_state_legacy( if schedule_rule_id is None: raise PlugwiseError("Plugwise: no schedule with this name available.") - state = "false" - if status == "on": - state = "true" locator = f'.//*[@id="{schedule_rule_id}"]/template' for rule in self._domain_objects.findall(locator): template_id = rule.attrib["id"] @@ -536,29 +541,29 @@ async def _set_schedule_state_legacy( data = ( "{state}' + f' id="{template_id}" />{new_state}' ) await self._request(uri, method="put", data=data) - self._schedule_old_states[loc_id][name] = state + self._schedule_old_states[loc_id][name] = new_state async def set_schedule_state( - self, loc_id: str, name: str | None, state: str + self, loc_id: str, name: str | None, new_state: str ) -> None: """Activate/deactivate the Schedule, with the given name, on the relevant Thermostat. Determined from - DOMAIN_OBJECTS. In HA Core used to set the hvac_mode: in practice switch between schedule on - off. """ - if state not in ["on", "off"]: + # Input checking + if new_state not in ["on", "off"]: raise PlugwiseError("Plugwise: invalid schedule state.") - if name is None: raise PlugwiseError( "Plugwise: cannot change schedule-state: no schedule name provided" ) if self._smile_legacy: - await self._set_schedule_state_legacy(loc_id, name, state) + await self._set_schedule_state_legacy(loc_id, name, new_state) return schedule_rule = self._rule_ids_by_name(name, loc_id) @@ -566,8 +571,8 @@ async def set_schedule_state( if not schedule_rule or schedule_rule is None: raise PlugwiseError("Plugwise: no schedule with this name available.") - # If schedule name is valid but no state change is requested, do nothing - if state == self._schedule_old_states[loc_id][name]: + # If no state change is requested, do nothing + if new_state == self._schedule_old_states[loc_id][name]: return schedule_rule_id: str = next(iter(schedule_rule)) @@ -587,10 +592,10 @@ async def set_schedule_state( subject = f'' subject = etree.fromstring(subject) - if state == "off": + if new_state == "off": self._last_active[loc_id] = name contexts.remove(subject) - if state == "on": + if new_state == "on": contexts.append(subject) contexts = etree.tostring(contexts, encoding="unicode").rstrip() @@ -601,7 +606,7 @@ async def set_schedule_state( f"{template}{contexts}" ) await self._request(uri, method="put", data=data) - self._schedule_old_states[loc_id][name] = state + self._schedule_old_states[loc_id][name] = new_state async def _set_preset_legacy(self, preset: str) -> None: """Set the given Preset on the relevant Thermostat - from DOMAIN_OBJECTS.""" From 0b8368d64df3a6aff8672f8f203513abbe2ed7cf Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 10:06:36 +0200 Subject: [PATCH 08/12] Reorder code --- plugwise/smile.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/plugwise/smile.py b/plugwise/smile.py index f18d23e4c..1fc343a4f 100644 --- a/plugwise/smile.py +++ b/plugwise/smile.py @@ -517,14 +517,6 @@ async def _set_schedule_state_legacy( self, loc_id: str, name: str, status: str ) -> None: """Helper-function for set_schedule_state().""" - - new_state = "false" - if status == "on": - new_state = "true" - # If no state change is requested, do nothing - if new_state == self._schedule_old_states[loc_id][name]: - return - schedule_rule_id: str | None = None for rule in self._domain_objects.findall("rule"): if rule.find("name").text == name: @@ -533,6 +525,13 @@ async def _set_schedule_state_legacy( if schedule_rule_id is None: raise PlugwiseError("Plugwise: no schedule with this name available.") + new_state = "false" + if status == "on": + new_state = "true" + # If no state change is requested, do nothing + if new_state == self._schedule_old_states[loc_id][name]: + return + locator = f'.//*[@id="{schedule_rule_id}"]/template' for rule in self._domain_objects.findall(locator): template_id = rule.attrib["id"] From 0a1f6ab2c8efb8fd7e253853fcde4430bdfd29a4 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 10:13:35 +0200 Subject: [PATCH 09/12] Test: cover smile.py line 533 --- tests/test_smile.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_smile.py b/tests/test_smile.py index 2ce9341a0..0c7ea7fe5 100644 --- a/tests/test_smile.py +++ b/tests/test_smile.py @@ -811,6 +811,26 @@ async def test_connect_legacy_anna_2(self): ], ) assert result + + smile._schedule_old_states["be81e3f8275b4129852c4d8d550ae2eb"][ + "Thermostat schedule" + ] = "off" + result_1 = await self.tinker_thermostat_schedule( + smile, + "be81e3f8275b4129852c4d8d550ae2eb", + "on", + good_schedules=["Thermostat schedule"], + single=True, + ) + result_2 = await self.tinker_thermostat_schedule( + smile, + "be81e3f8275b4129852c4d8d550ae2eb", + "on", + good_schedules=["Thermostat schedule"], + single=True, + ) + assert result_1 and result_2 + await smile.close_connection() await self.disconnect(server, client) From 16ad7f92c9df5590596eb2da06459ab7bfb6ef44 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 10:14:06 +0200 Subject: [PATCH 10/12] Bump to b1 --- plugwise/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugwise/__init__.py b/plugwise/__init__.py index 0b385cdf2..e4777ef4e 100644 --- a/plugwise/__init__.py +++ b/plugwise/__init__.py @@ -1,6 +1,6 @@ """Plugwise module.""" -__version__ = "0.22.1b0" +__version__ = "0.22.1b1" from plugwise.smile import Smile from plugwise.stick import Stick From 98ee9362e7b431cfde0f9da14b5f9a8b0a4a7abf Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 12:29:29 +0200 Subject: [PATCH 11/12] Bump to v0.22.1 release-version --- plugwise/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugwise/__init__.py b/plugwise/__init__.py index e4777ef4e..9894ca3c4 100644 --- a/plugwise/__init__.py +++ b/plugwise/__init__.py @@ -1,6 +1,6 @@ """Plugwise module.""" -__version__ = "0.22.1b1" +__version__ = "0.22.1" from plugwise.smile import Smile from plugwise.stick import Stick From cffa85011ede2f5f7cc5e79cff417afe1a1bae27 Mon Sep 17 00:00:00 2001 From: Bouwe Date: Tue, 20 Sep 2022 12:58:47 +0200 Subject: [PATCH 12/12] Bump CACHE_VERSION --- .github/workflows/verify.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index c71857697..41fc41d52 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -4,7 +4,7 @@ name: Latest commit env: - CACHE_VERSION: 3 + CACHE_VERSION: 4 DEFAULT_PYTHON: "3.9" PRE_COMMIT_HOME: ~/.cache/pre-commit