Merge pull request #755 from plugwise/relay-sw-improve

Set_switch: don't raise eror when locked

Author: bouwew

CHANGELOG.md

@@ -1,8 +1,9 @@
 # Changelog
 
-## Ongoing
+## v1.7.6
 
-- Maintenance chores (mostly reworking Github CI Actions) backporting from efforts on Python Plugwise [USB: 264](https://github.com/plugwise/python-plugwise-usb/pull/264) after porting our progress using [USB: 263](https://github.com/plugwise/python-plugwise-usb/pull/263)
+- Maintenance chores (mostly reworking Github CI Actions) backporting from efforts on Python Plugwise [USB: #264](https://github.com/plugwise/python-plugwise-usb/pull/264) after porting our progress using [USB: #263](https://github.com/plugwise/python-plugwise-usb/pull/263)
+- Don't raise an error when a locked switch is being toggled, and other switch-related improvements via [#755](https://github.com/plugwise/python-plugwise/pull/755)
 
 ## v1.7.5
 

fixtures/adam_multiple_devices_per_zone/data.json

@@ -540,6 +540,19 @@
     "vendor": "Plugwise",
     "zigbee_mac_address": "ABCD012345670A11"
   },
+  "e8ef2a01ed3b4139a53bf749204fe6b4": {
+    "dev_class": "switching",
+    "members": [
+      "02cf28bfec924855854c544690a609ef",
+      "4a810418d5394b3f82727340b91ba740"
+    ],
+    "model": "Switchgroup",
+    "name": "Test",
+    "switches": {
+      "relay": true
+    },
+    "vendor": "Plugwise"
+  },
   "f1fee6043d3642a9b0a65297455f008e": {
     "available": true,
     "binary_sensors": {

fixtures/m_adam_multiple_devices_per_zone/data.json

@@ -531,6 +531,19 @@
     "vendor": "Plugwise",
     "zigbee_mac_address": "ABCD012345670A11"
   },
+  "e8ef2a01ed3b4139a53bf749204fe6b4": {
+    "dev_class": "switching",
+    "members": [
+      "02cf28bfec924855854c544690a609ef",
+      "4a810418d5394b3f82727340b91ba740"
+    ],
+    "model": "Switchgroup",
+    "name": "Test",
+    "switches": {
+      "relay": true
+    },
+    "vendor": "Plugwise"
+  },
   "f1fee6043d3642a9b0a65297455f008e": {
     "available": true,
     "binary_sensors": {

plugwise/__init__.py

@@ -15,6 +15,8 @@
     MODULES,
     NONE,
     SMILES,
+    STATE_OFF,
+    STATE_ON,
     STATUS,
     SYSTEM,
     GwEntityData,
@@ -398,10 +400,21 @@ async def set_temperature_offset(self, dev_id: str, offset: float) -> None:
 
     async def set_switch_state(
         self, appl_id: str, members: list[str] | None, model: str, state: str
-    ) -> None:
-        """Set the given State of the relevant Switch."""
+    ) -> bool:
+        """Set the given State of the relevant Switch.
+
+        Return the result:
+          - True when switched to state on,
+          - False when switched to state off,
+          - the unchanged state when the switch is for instance locked.
+        """
+        if state not in (STATE_OFF, STATE_ON):
+            raise PlugwiseError("Invalid state supplied to set_switch_state")
+
         try:
-            await self._smile_api.set_switch_state(appl_id, members, model, state)
+            return await self._smile_api.set_switch_state(
+                appl_id, members, model, state
+            )
         except ConnectionFailedError as exc:
             raise ConnectionFailedError(
                 f"Failed to set switch state: {str(exc)}"

plugwise/constants.py

@@ -23,6 +23,8 @@
 PRESET_AWAY: Final = "away"
 PRESSURE_BAR: Final = "bar"
 SIGNAL_STRENGTH_DECIBELS_MILLIWATT: Final = "dBm"
+STATE_OFF: Final = "off"
+STATE_ON: Final = "on"
 TEMP_CELSIUS: Final = "°C"
 TEMP_KELVIN: Final = "°K"
 TIME_MILLISECONDS: Final = "ms"

plugwise/legacy/smile.py

@@ -18,6 +18,8 @@
     OFF,
     REQUIRE_APPLIANCES,
     RULES,
+    STATE_OFF,
+    STATE_ON,
     GwEntityData,
     ThermoLoc,
 )
@@ -195,7 +197,7 @@ async def set_schedule_state(
         Determined from - DOMAIN_OBJECTS.
         Used in HA Core to set the hvac_mode: in practice switch between schedule on - off.
         """
-        if state not in ("on", "off"):
+        if state not in (STATE_OFF, STATE_ON):
             raise PlugwiseError("Plugwise: invalid schedule state.")
 
         # Handle no schedule-name / Off-schedule provided
@@ -214,7 +216,7 @@ async def set_schedule_state(
             )  # pragma: no cover
 
         new_state = "false"
-        if state == "on":
+        if state == STATE_ON:
             new_state = "true"
 
         locator = f'.//*[@id="{schedule_rule_id}"]/template'
@@ -234,13 +236,16 @@ async def set_schedule_state(
 
     async def set_switch_state(
         self, appl_id: str, members: list[str] | None, model: str, state: str
-    ) -> None:
+    ) -> bool:
         """Set the given state of the relevant switch.
 
         For individual switches, sets the state directly.
         For group switches, sets the state for each member in the group separately.
         For switch-locks, sets the lock state using a different data format.
+        Return the requested state when succesful, the current state otherwise.
         """
+        current_state = self.gw_entities[appl_id]["switches"]["relay"]
+        requested_state = state == STATE_ON
         switch = Munch()
         switch.actuator = "actuator_functionalities"
         switch.func_type = "relay_functionality"
@@ -250,7 +255,7 @@ async def set_switch_state(
 
         # Handle switch-lock
         if model == "lock":
-            state = "false" if state == "off" else "true"
+            state = "true" if state == STATE_ON else "false"
             appliance = self._appliances.find(f'appliance[@id="{appl_id}"]')
             appl_name = appliance.find("name").text
             appl_type = appliance.find("type").text
@@ -269,37 +274,45 @@ async def set_switch_state(
                 "</appliances>"
             )
             await self.call_request(APPLIANCES, method="post", data=data)
-            return
+            return requested_state
 
         # Handle group of switches
         data = f"<{switch.func_type}><state>{state}</state></{switch.func_type}>"
         if members is not None:
             return await self._set_groupswitch_member_state(
-                data, members, state, switch
+                appl_id, data, members, state, switch
             )
 
         # Handle individual relay switches
         uri = f"{APPLIANCES};id={appl_id}/relay"
-        if model == "relay":
-            locator = (
-                f'appliance[@id="{appl_id}"]/{switch.actuator}/{switch.func_type}/lock'
-            )
+        if model == "relay" and self.gw_entities[appl_id]["switches"]["lock"]:
             # Don't bother switching a relay when the corresponding lock-state is true
-            if self._appliances.find(locator).text == "true":
-                raise PlugwiseError("Plugwise: the locked Relay was not switched.")
+            return current_state
 
         await self.call_request(uri, method="put", data=data)
+        return requested_state
 
     async def _set_groupswitch_member_state(
-        self, data: str, members: list[str], state: str, switch: Munch
-    ) -> None:
+        self, appl_id: str, data: str, members: list[str], state: str, switch: Munch
+    ) -> bool:
         """Helper-function for set_switch_state().
 
-        Set the given State of the relevant Switch (relay) within a group of members.
+        Set the requested state of the relevant switch within a group of switches.
+        Return the current group-state when none of the switches has changed its state, the requested state otherwise.
         """
+        current_state = self.gw_entities[appl_id]["switches"]["relay"]
+        requested_state = state == STATE_ON
+        switched = 0
         for member in members:
-            uri = f"{APPLIANCES};id={member}/relay"
-            await self.call_request(uri, method="put", data=data)
+            if not self.gw_entities[member]["switches"]["lock"]:
+                uri = f"{APPLIANCES};id={member}/relay"
+                await self.call_request(uri, method="put", data=data)
+                switched += 1
+
+        if switched > 0:
+            return requested_state
+
+        return current_state  # pragma: no cover
 
     async def set_temperature(self, _: str, items: dict[str, float]) -> None:
         """Set the given Temperature on the relevant Thermostat."""
@@ -310,7 +323,7 @@ async def set_temperature(self, _: str, items: dict[str, float]) -> None:
         if setpoint is None:
             raise PlugwiseError(
                 "Plugwise: failed setting temperature: no valid input provided"
-            )  # pragma: no cover"
+            )  # pragma: no cover
 
         temperature = str(setpoint)
         data = (

plugwise/smile.py

@@ -7,7 +7,7 @@
 
 from collections.abc import Awaitable, Callable
 import datetime as dt
-from typing import Any
+from typing import Any, cast
 
 from plugwise.constants import (
     ADAM,
@@ -22,7 +22,10 @@
     NOTIFICATIONS,
     OFF,
     RULES,
+    STATE_OFF,
+    STATE_ON,
     GwEntityData,
+    SwitchType,
     ThermoLoc,
 )
 from plugwise.data import SmileData
@@ -309,12 +312,12 @@ async def set_schedule_state(
         Used in HA Core to set the hvac_mode: in practice switch between schedule on - off.
         """
         # Input checking
-        if new_state not in ("on", "off"):
+        if new_state not in (STATE_OFF, STATE_ON):
             raise PlugwiseError("Plugwise: invalid schedule state.")
 
         # Translate selection of Off-schedule-option to disabling the active schedule
         if name == OFF:
-            new_state = "off"
+            new_state = STATE_OFF
 
         # Handle no schedule-name / Off-schedule provided
         if name is None or name == OFF:
@@ -367,18 +370,27 @@ def determine_contexts(
             subject = f'<context><zone><location id="{loc_id}" /></zone></context>'
             subject = etree.fromstring(subject)
 
-        if state == "off":
+        if state == STATE_OFF:
             self._last_active[loc_id] = name
             contexts.remove(subject)
-        if state == "on":
+        if state == STATE_ON:
             contexts.append(subject)
 
         return str(etree.tostring(contexts, encoding="unicode").rstrip())
 
     async def set_switch_state(
         self, appl_id: str, members: list[str] | None, model: str, state: str
-    ) -> None:
-        """Set the given State of the relevant Switch."""
+    ) -> bool:
+        """Set the given state of the relevant Switch.
+
+        For individual switches, sets the state directly.
+        For group switches, sets the state for each member in the group separately.
+        For switch-locks, sets the lock state using a different data format.
+        Return the requested state when succesful, the current state otherwise.
+        """
+        model_type = cast(SwitchType, model)
+        current_state = self.gw_entities[appl_id]["switches"][model_type]
+        requested_state = state == STATE_ON
         switch = Munch()
         switch.actuator = "actuator_functionalities"
         switch.device = "relay"
@@ -396,10 +408,18 @@ async def set_switch_state(
 
         if model == "lock":
             switch.func = "lock"
-            state = "false" if state == "off" else "true"
+            state = "true" if state == STATE_ON else "false"
+
+        data = (
+            f"<{switch.func_type}>"
+            f"<{switch.func}>{state}</{switch.func}>"
+            f"</{switch.func_type}>"
+        )
 
         if members is not None:
-            return await self._set_groupswitch_member_state(members, state, switch)
+            return await self._set_groupswitch_member_state(
+                appl_id, data, members, state, switch
+            )
 
         locator = f'appliance[@id="{appl_id}"]/{switch.actuator}/{switch.func_type}'
         found = self._domain_objects.findall(locator)
@@ -412,39 +432,42 @@ async def set_switch_state(
             else:  # actuators with a single item like relay_functionality
                 switch_id = item.attrib["id"]
 
-        data = (
-            f"<{switch.func_type}>"
-            f"<{switch.func}>{state}</{switch.func}>"
-            f"</{switch.func_type}>"
-        )
         uri = f"{APPLIANCES};id={appl_id}/{switch.device};id={switch_id}"
         if model == "relay":
-            locator = (
-                f'appliance[@id="{appl_id}"]/{switch.actuator}/{switch.func_type}/lock'
-            )
-            # Don't bother switching a relay when the corresponding lock-state is true
-            if self._domain_objects.find(locator).text == "true":
-                raise PlugwiseError("Plugwise: the locked Relay was not switched.")
+            lock_blocked = self.gw_entities[appl_id]["switches"].get("lock")
+            if lock_blocked or lock_blocked is None:
+                # Don't switch a relay when its corresponding lock-state is true or no
+                # lock is present. That means the relay can't be controlled by the user.
+                return current_state
 
         await self.call_request(uri, method="put", data=data)
+        return requested_state
 
     async def _set_groupswitch_member_state(
-        self, members: list[str], state: str, switch: Munch
-    ) -> None:
+        self, appl_id: str, data: str, members: list[str], state: str, switch: Munch
+    ) -> bool:
         """Helper-function for set_switch_state().
 
-        Set the given State of the relevant Switch within a group of members.
+        Set the requested state of the relevant switch within a group of switches.
+        Return the current group-state when none of the switches has changed its state, the requested state otherwise.
         """
+        current_state = self.gw_entities[appl_id]["switches"]["relay"]
+        requested_state = state == STATE_ON
+        switched = 0
         for member in members:
             locator = f'appliance[@id="{member}"]/{switch.actuator}/{switch.func_type}'
             switch_id = self._domain_objects.find(locator).attrib["id"]
             uri = f"{APPLIANCES};id={member}/{switch.device};id={switch_id}"
-            data = (
-                f"<{switch.func_type}>"
-                f"<{switch.func}>{state}</{switch.func}>"
-                f"</{switch.func_type}>"
-            )
-            await self.call_request(uri, method="put", data=data)
+            lock_blocked = self.gw_entities[member]["switches"].get("lock")
+            # Assume Plugs under Plugwise control are not part of a group
+            if lock_blocked is not None and not lock_blocked:
+                await self.call_request(uri, method="put", data=data)
+                switched += 1
+
+        if switched > 0:
+            return requested_state
+
+        return current_state
 
     async def set_temperature(self, loc_id: str, items: dict[str, float]) -> None:
         """Set the given Temperature on the relevant Thermostat."""

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name            = "plugwise"
-version         = "1.7.5"
+version         = "1.7.6"
 license         = "MIT"
 description     = "Plugwise Smile (Adam/Anna/P1) and Stretch module for Python 3."
 readme          = "README.md"

tests/data/adam/adam_multiple_devices_per_zone.json

@@ -567,6 +567,19 @@
       "vendor": "Plugwise",
       "zigbee_mac_address": "ABCD012345670A08"
     },
+    "e8ef2a01ed3b4139a53bf749204fe6b4": {
+      "dev_class": "switching",
+      "members": [
+        "02cf28bfec924855854c544690a609ef",
+        "4a810418d5394b3f82727340b91ba740"
+      ],
+      "model": "Switchgroup",
+      "name": "Test",
+      "switches": {
+        "relay": true
+      },
+      "vendor": "Plugwise"
+    },
     "fe799307f1624099878210aa0b9f1475": {
       "binary_sensors": {
         "plugwise_notification": true