diff --git a/CHANGELOG.md b/CHANGELOG.md index 03609593e..778305295 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Rework tooling [#664](https://github.com/plugwise/python-plugwise/pull/664) - Archive p1v4 userdata [#666](https://github.com/plugwise/python-plugwise/pull/666) - Correct manual_fixtures script [#668](https://github.com/plugwise/python-plugwise/pull/668) +- Improve P1 fault-handling, continuous improvements [#670](https://github.com/plugwise/python-plugwise/pull/670) ## v1.6.3 diff --git a/plugwise/constants.py b/plugwise/constants.py index 63c6af3e9..232da091b 100644 --- a/plugwise/constants.py +++ b/plugwise/constants.py @@ -85,6 +85,7 @@ MODULE_LOCATOR: Final = "./logs/point_log/*[@id]" NONE: Final = "None" OFF: Final = "off" +PRIORITY_DEVICE_CLASSES = ("heater_central", "gateway") # XML data paths APPLIANCES: Final = "/core/appliances" diff --git a/plugwise/helper.py b/plugwise/helper.py index cea2f130f..58e499e0c 100644 --- a/plugwise/helper.py +++ b/plugwise/helper.py @@ -29,6 +29,7 @@ NONE, OFF, P1_MEASUREMENTS, + PRIORITY_DEVICE_CLASSES, TEMP_CELSIUS, THERMOSTAT_CLASSES, TOGGLES, @@ -278,7 +279,11 @@ def __init__(self) -> None: SmileCommon.__init__(self) def _all_appliances(self) -> None: - """Collect all appliances with relevant info.""" + """Collect all appliances with relevant info. + + Also, collect the P1 smartmeter info from a location + as this one is not available as an appliance. + """ self._count = 0 self._all_locations() @@ -330,55 +335,33 @@ def _all_appliances(self) -> None: if not (appl := self._appliance_info_finder(appl, appliance)): continue - # P1: for gateway and smartmeter switch entity_id - part 1 - # This is done to avoid breakage in HA Core - if appl.pwclass == "gateway" and self.smile_type == "power": - appl.entity_id = appl.location - self._create_gw_entities(appl) - # For P1 collect the connected SmartMeter info if self.smile_type == "power": - self._p1_smartmeter_info_finder(appl) - # P1: for gateway and smartmeter switch entity_id - part 2 - for item in self.gw_entities: - if item != self.gateway_id: - self.gateway_id = item - # Leave for-loop to avoid a 2nd device_id switch - break - - # Place the gateway and optional heater_central devices as 1st and 2nd - for dev_class in ("heater_central", "gateway"): - for entity_id, entity in dict(self.gw_entities).items(): - if entity["dev_class"] == dev_class: - tmp_entity = entity - self.gw_entities.pop(entity_id) - cleared_dict = self.gw_entities - add_to_front = {entity_id: tmp_entity} - self.gw_entities = {**add_to_front, **cleared_dict} + self._get_p1_smartmeter_info() - def _all_locations(self) -> None: - """Collect all locations.""" - loc = Munch() - locations = self._domain_objects.findall("./location") - for location in locations: - loc.name = location.find("name").text - loc.loc_id = location.attrib["id"] - if loc.name == "Home": - self._home_location = loc.loc_id + # Sort the gw_entities + self._sort_gw_entities() - self._loc_data[loc.loc_id] = {"name": loc.name} + def _get_p1_smartmeter_info(self) -> None: + """For P1 collect the connected SmartMeter info from the Home/building location. - def _p1_smartmeter_info_finder(self, appl: Munch) -> None: - """Collect P1 DSMR SmartMeter info.""" + Note: For P1, the entity_id for the gateway and smartmeter are + switched to maintain backward compatibility with existing implementations. + """ + appl = Munch() loc_id = next(iter(self._loc_data.keys())) - location = self._domain_objects.find(f'./location[@id="{loc_id}"]') + if ( + location := self._domain_objects.find(f'./location[@id="{loc_id}"]') + ) is None: + return + locator = MODULE_LOCATOR module_data = self._get_module_data(location, locator) if not module_data["contents"]: LOGGER.error("No module data found for SmartMeter") # pragma: no cover - return None # pragma: no cover - + return # pragma: no cover + appl.available = None appl.entity_id = self.gateway_id appl.firmware = module_data["firmware_version"] appl.hardware = module_data["hardware_version"] @@ -391,22 +374,49 @@ def _p1_smartmeter_info_finder(self, appl: Munch) -> None: appl.vendor_name = module_data["vendor_name"] appl.zigbee_mac = None + # Replace the entity_id of the gateway by the smartmeter location_id + self.gw_entities[loc_id] = self.gw_entities.pop(self.gateway_id) + self.gateway_id = loc_id + self._create_gw_entities(appl) + def _sort_gw_entities(self) -> None: + """Place the gateway and optional heater_central entities as 1st and 2nd.""" + for dev_class in PRIORITY_DEVICE_CLASSES: + for entity_id, entity in dict(self.gw_entities).items(): + if entity["dev_class"] == dev_class: + priority_entity = entity + self.gw_entities.pop(entity_id) + other_entities = self.gw_entities + priority_entities = {entity_id: priority_entity} + self.gw_entities = {**priority_entities, **other_entities} + + def _all_locations(self) -> None: + """Collect all locations.""" + loc = Munch() + locations = self._domain_objects.findall("./location") + for location in locations: + loc.name = location.find("name").text + loc.loc_id = location.attrib["id"] + if loc.name == "Home": + self._home_location = loc.loc_id + + self._loc_data[loc.loc_id] = {"name": loc.name} + def _appliance_info_finder(self, appl: Munch, appliance: etree) -> Munch: """Collect info for all appliances found.""" match appl.pwclass: case "gateway": - # Collect gateway device info + # Collect gateway entity info return self._appl_gateway_info(appl, appliance) case _ as dev_class if dev_class in THERMOSTAT_CLASSES: - # Collect thermostat device info + # Collect thermostat entity info return self._appl_thermostat_info(appl, appliance) case "heater_central": - # Collect heater_central device info + # Collect heater_central entity info self._appl_heater_central_info( appl, appliance, False - ) # False means non-legacy device + ) # False means non-legacy entity self._dhw_allowed_modes = self._get_appl_actuator_modes( appliance, "domestic_hot_water_mode_control_functionality" ) @@ -801,19 +811,23 @@ def _process_on_off_device_c_heating_state(self, data: GwEntityData) -> None: data["binary_sensors"]["heating_state"] = data["c_heating_state"] if self.smile(ADAM): + # First count when not present, then create and init to False. + # When present init to False if "heating_state" not in data["binary_sensors"]: self._count += 1 data["binary_sensors"]["heating_state"] = False + if "cooling_state" not in data["binary_sensors"]: self._count += 1 data["binary_sensors"]["cooling_state"] = False + if self._cooling_enabled: data["binary_sensors"]["cooling_state"] = data["c_heating_state"] else: data["binary_sensors"]["heating_state"] = data["c_heating_state"] def _update_anna_cooling(self, entity_id: str, data: GwEntityData) -> None: - """Update the Anna heater_central device for cooling. + """Update the Anna heater_central entity for cooling. Support added for Techneco Elga and Thercon Loria/Thermastage. """ diff --git a/plugwise/legacy/smile.py b/plugwise/legacy/smile.py index 5afe5690d..d5be6832e 100644 --- a/plugwise/legacy/smile.py +++ b/plugwise/legacy/smile.py @@ -25,7 +25,7 @@ PlugwiseData, ThermoLoc, ) -from plugwise.exceptions import ConnectionFailedError, PlugwiseError +from plugwise.exceptions import ConnectionFailedError, DataMissingError, PlugwiseError from plugwise.legacy.data import SmileLegacyData import aiohttp @@ -120,18 +120,30 @@ async def async_update(self) -> PlugwiseData: ) self.gw_data: GatewayData = {} self.gw_entities: dict[str, GwEntityData] = {} - await self.full_xml_update() - self.get_all_gateway_entities() + try: + await self.full_xml_update() + self.get_all_gateway_entities() + # Detect failed data-retrieval + _ = self.gw_entities[self.gateway_id]["location"] + except KeyError as err: # pragma: no cover + raise DataMissingError( + "No (full) Plugwise legacy data received" + ) from err # Otherwise perform an incremental update else: - self._domain_objects = await self.request(DOMAIN_OBJECTS) - match self._target_smile: - case "smile_v2": - self._modules = await self.request(MODULES) - case self._target_smile if self._target_smile in REQUIRE_APPLIANCES: - self._appliances = await self.request(APPLIANCES) - - self._update_gw_entities() + try: + self._domain_objects = await self.request(DOMAIN_OBJECTS) + match self._target_smile: + case "smile_v2": + self._modules = await self.request(MODULES) + case self._target_smile if self._target_smile in REQUIRE_APPLIANCES: + self._appliances = await self.request(APPLIANCES) + + self._update_gw_entities() + # Detect failed data-retrieval + _ = self.gw_entities[self.gateway_id]["location"] + except KeyError as err: # pragma: no cover + raise DataMissingError("No legacy Plugwise data received") from err self._previous_day_number = day_number return PlugwiseData( diff --git a/plugwise/smile.py b/plugwise/smile.py index 9f20bfd3d..94bfebcfb 100644 --- a/plugwise/smile.py +++ b/plugwise/smile.py @@ -131,7 +131,7 @@ async def async_update(self) -> PlugwiseData: try: await self.full_xml_update() self.get_all_gateway_entities() - # Set self._cooling_enabled -required for set_temperature, + # Set self._cooling_enabled - required for set_temperature, # also, check for a failed data-retrieval if "heater_id" in self.gw_data: heat_cooler = self.gw_entities[self.gw_data["heater_id"]] @@ -142,8 +142,10 @@ async def async_update(self) -> PlugwiseData: self._cooling_enabled = heat_cooler["binary_sensors"][ "cooling_enabled" ] + else: # cover failed data-retrieval for P1 + _ = self.gw_entities[self.gateway_id]["location"] except KeyError as err: - raise DataMissingError("No Plugwise data received") from err + raise DataMissingError("No Plugwise actual data received") from err return PlugwiseData( devices=self.gw_entities, diff --git a/tests/test_adam.py b/tests/test_adam.py index 94a9075c0..bc521104f 100644 --- a/tests/test_adam.py +++ b/tests/test_adam.py @@ -137,8 +137,10 @@ async def test_connect_adam_plus_anna_new(self): self.smile_setup = "reboot/adam_plus_anna_new" try: await self.device_test(smile, initialize=False) - except pw_exceptions.PlugwiseError: - _LOGGER.debug("Receiving no data after a reboot is properly handled") + except pw_exceptions.PlugwiseError as err: + _LOGGER.debug( + f"Receiving no data after a reboot is properly handled: {err}" + ) # Simulate receiving xml-data with self.smile_setup = "error/adam_plus_anna_new" diff --git a/tests/test_p1.py b/tests/test_p1.py index 3cd72257e..ff102ba26 100644 --- a/tests/test_p1.py +++ b/tests/test_p1.py @@ -2,7 +2,7 @@ import pytest -from .test_init import _LOGGER, TestPlugwise +from .test_init import _LOGGER, TestPlugwise, pw_exceptions SMILE_TYPE = "p1" @@ -41,6 +41,22 @@ async def test_connect_p1v4_442_single(self): smile, "2022-05-16 00:00:01", testdata_updated, initialize=False ) + # Simulate receiving no xml-data after a requesting a reboot of the gateway + self.smile_setup = "reboot/p1v4_442_single" + try: + await self.device_test(smile, initialize=False) + except pw_exceptions.PlugwiseError as err: + _LOGGER.debug( + f"Receiving no data after a reboot is properly handled: {err}" + ) + + # Simulate receiving xml-data with + self.smile_setup = "error/p1v4_442_single" + try: + await self.device_test(smile, initialize=False) + except pw_exceptions.ResponseError: + _LOGGER.debug("Receiving error-data from the Gateway") + await smile.close_connection() await self.disconnect(server, client) diff --git a/userdata/error/p1v4_442_single/core.domain_objects.xml b/userdata/error/p1v4_442_single/core.domain_objects.xml new file mode 100644 index 000000000..ff93be936 --- /dev/null +++ b/userdata/error/p1v4_442_single/core.domain_objects.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/userdata/reboot/p1v4_442_single/core.domain_objects.xml b/userdata/reboot/p1v4_442_single/core.domain_objects.xml new file mode 100644 index 000000000..5cd8e19de --- /dev/null +++ b/userdata/reboot/p1v4_442_single/core.domain_objects.xml @@ -0,0 +1,3 @@ + + +