Merge pull request #670 from plugwise/try-something

Improve P1 fault-handling

Author: bouwew

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
 

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"

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.
         """

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(

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,

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 <error>
         self.smile_setup = "error/adam_plus_anna_new"

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 <error>
+        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)
 

userdata/error/p1v4_442_single/core.domain_objects.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<domain_objects>
+    <error>
+    </error>
+</domain_objects>

userdata/reboot/p1v4_442_single/core.domain_objects.xml

@@ -0,0 +1,3 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<domain_objects>
+</domain_objects>