Merge pull request #317 from ikalchev/v3.3.1

v3.3.1

CHANGELOG.md

@@ -16,6 +16,11 @@ Sections
 ### Developers
 -->
 
+## [3.3.1] - 2021-02-28
+
+### Changed
+- Implement partial success response for `set_characteristics` (was `BAD_REQUEST` on error). [#316](https://github.com/ikalchev/HAP-python/pull/316) 
+
 ## [3.3.0] - 2021-02-13
 
 ### Fixed
@@ -24,6 +29,7 @@ Sections
 - Speed up event subscription for new connections. [#308](https://github.com/ikalchev/HAP-python/pull/308)
 - Properly handle camera snapshots. [#311](https://github.com/ikalchev/HAP-python/pull/311)
 - Properly handle pairing attempt when already paired. [#314](https://github.com/ikalchev/HAP-python/pull/314)
+
 ### Changed
 - Use github actions for codecov. [#312](https://github.com/ikalchev/HAP-python/pull/312), [#313](https://github.com/ikalchev/HAP-python/pull/313)
 

pyhap/accessory_driver.py

@@ -56,8 +56,9 @@
 
 CHAR_STAT_OK = 0
 SERVICE_COMMUNICATION_FAILURE = -70402
-SERVICE_CALLBACK = 0
-SERVICE_CALLBACK_DATA = 1
+SERVICE_CALLBACK = "callback"
+SERVICE_CHARS = "chars"
+SERVICE_IIDS = "iids"
 HAP_SERVICE_TYPE = "_hap._tcp.local."
 VALID_MDNS_REGEX = re.compile(r"[^A-Za-z0-9\-]+")
 
@@ -707,9 +708,13 @@ def set_characteristics(self, chars_query, client_addr):
         :type chars_query: dict
         """
         # TODO: Add support for chars that do no support notifications.
-        service_callbacks = {}
+        accessory_callbacks = {}
+        setter_results = {}
+        had_error = False
+
         for cq in chars_query[HAP_REPR_CHARS]:
             aid, iid = cq[HAP_REPR_AID], cq[HAP_REPR_IID]
+            setter_results.setdefault(aid, {})
             char = self.accessory.get_characteristic(aid, iid)
 
             if HAP_PERMISSION_NOTIFY in cq:
@@ -721,31 +726,79 @@ def set_characteristics(self, chars_query, client_addr):
                     client_addr, char_topic, cq[HAP_PERMISSION_NOTIFY]
                 )
 
-            if HAP_REPR_VALUE in cq:
-                # TODO: status needs to be based on success of set_value
-                char.client_update_value(cq[HAP_REPR_VALUE], client_addr)
-                # For some services we want to send all the char value
-                # changes at once.  This resolves an issue where we send
-                # ON and then BRIGHTNESS and the light would go to 100%
-                # and then dim to the brightness because each callback
-                # would only send one char at a time.
-                service = char.service
-
-                if service and service.setter_callback:
-                    service_name = service.display_name
-                    service_callbacks.setdefault(aid, {})
-                    service_callbacks[aid].setdefault(
-                        service_name, [service.setter_callback, {}]
-                    )
-                    service_callbacks[aid][service_name][SERVICE_CALLBACK_DATA][
-                        char.display_name
-                    ] = cq[HAP_REPR_VALUE]
-
-        for aid in service_callbacks:
-            for service_name in service_callbacks[aid]:
-                service_callbacks[aid][service_name][SERVICE_CALLBACK](
-                    service_callbacks[aid][service_name][SERVICE_CALLBACK_DATA]
+            if HAP_REPR_VALUE not in cq:
+                continue
+
+            value = cq[HAP_REPR_VALUE]
+
+            try:
+                char.client_update_value(value, client_addr)
+            except Exception:  # pylint: disable=broad-except
+                logger.exception(
+                    "%s: Error while setting characteristic %s to %s",
+                    client_addr,
+                    char.display_name,
+                    value,
                 )
+                setter_results[aid][iid] = SERVICE_COMMUNICATION_FAILURE
+                had_error = True
+            else:
+                setter_results[aid][iid] = CHAR_STAT_OK
+
+            # For some services we want to send all the char value
+            # changes at once.  This resolves an issue where we send
+            # ON and then BRIGHTNESS and the light would go to 100%
+            # and then dim to the brightness because each callback
+            # would only send one char at a time.
+            if not char.service or not char.service.setter_callback:
+                continue
+
+            services = accessory_callbacks.setdefault(aid, {})
+
+            if char.service.display_name not in services:
+                services[char.service.display_name] = {
+                    SERVICE_CALLBACK: char.service.setter_callback,
+                    SERVICE_CHARS: {},
+                    SERVICE_IIDS: [],
+                }
+
+            service_data = services[char.service.display_name]
+            service_data[SERVICE_CHARS][char.display_name] = value
+            service_data[SERVICE_IIDS].append(iid)
+
+        for aid, services in accessory_callbacks.items():
+            for service_name, service_data in services.items():
+                try:
+                    service_data[SERVICE_CALLBACK](service_data[SERVICE_CHARS])
+                except Exception:  # pylint: disable=broad-except
+                    logger.exception(
+                        "%s: Error while setting characteristics to %s for the %s service",
+                        service_data[SERVICE_CHARS],
+                        client_addr,
+                        service_name,
+                    )
+                    set_result = SERVICE_COMMUNICATION_FAILURE
+                    had_error = True
+                else:
+                    set_result = CHAR_STAT_OK
+
+                for iid in service_data[SERVICE_IIDS]:
+                    setter_results[aid][iid] = set_result
+
+        if not had_error:
+            return None
+
+        return {
+            HAP_REPR_CHARS: [
+                {
+                    HAP_REPR_AID: aid,
+                    HAP_REPR_IID: iid,
+                    HAP_REPR_STATUS: status,
+                }
+                for aid, iid_status in setter_results.items()
+                for iid, status in iid_status.items()
+            ]
+        }
 
     def signal_handler(self, _signal, _frame):
         """Stops the AccessoryDriver for a given signal.

pyhap/const.py

@@ -1,7 +1,7 @@
 """This module contains constants used by other modules."""
 MAJOR_VERSION = 3
 MINOR_VERSION = 3
-PATCH_VERSION = 0
+PATCH_VERSION = 1
 __short_version__ = '{}.{}'.format(MAJOR_VERSION, MINOR_VERSION)
 __version__ = '{}.{}'.format(__short_version__, PATCH_VERSION)
 REQUIRED_PYTHON_VER = (3, 5)

pyhap/hap_handler.py

@@ -599,18 +599,16 @@ def handle_set_characteristics(self):
             "%s: Set characteristics content: %s", self.client_address, requested_chars
         )
 
-        # TODO: Outline how chars return errors on set_chars.
-        try:
-            self.accessory_handler.set_characteristics(
-                requested_chars, self.client_address
-            )
-        except Exception as ex:  # pylint: disable=broad-except
-            logger.exception(
-                "%s: Exception in set_characteristics: %s", self.client_address, ex
-            )
-            self.send_response(HTTPStatus.BAD_REQUEST)
-        else:
+        response = self.accessory_handler.set_characteristics(
+            requested_chars, self.client_address
+        )
+        if response is None:
             self.send_response(HTTPStatus.NO_CONTENT)
+            return
+
+        self.send_response(207)
+        self.send_header("Content-Type", self.JSON_RESPONSE_TYPE)
+        self.end_response(json.dumps(response).encode("utf-8"))
 
     def handle_pairings(self):
         """Handles a client request to update or remove a pairing."""