Merge pull request #388 from ikalchev/v4.3.0

V4.3.0

CHANGELOG.md

@@ -16,7 +16,13 @@ Sections
 ### Developers
 -->
 
-## [4.2.1] - 2021-09-6
+## [4.3.0] - 2021-10-07
+
+### Fixed
+- Only send the latest state in case of multiple events for the same characteristic. [#385](https://github.com/ikalchev/HAP-python/pull/385)
+- Handle invalid formats from clients. [#387](https://github.com/ikalchev/HAP- python/pull/387)
+
+## [4.2.1] - 2021-09-06
 
 ### Fixed
 - Fix floating point values with minStep. [#382](https://github.com/ikalchev/HAP-python/pull/382)

pyhap/accessory.py

@@ -61,9 +61,7 @@ def __init__(self, driver, display_name, aid=None):
     def __repr__(self):
         """Return the representation of the accessory."""
         services = [s.display_name for s in self.services]
-        return "<accessory display_name='{}' services={}>".format(
-            self.display_name, services
-        )
+        return f"<accessory display_name='{self.display_name}' services={services}>"
 
     @property
     def available(self):
@@ -236,14 +234,13 @@ def setup_message(self):
         pincode = self.driver.state.pincode.decode()
         if SUPPORT_QR_CODE:
             xhm_uri = self.xhm_uri()
-            print("Setup payload: {}".format(xhm_uri), flush=True)
+            print(f"Setup payload: {xhm_uri}", flush=True)
             print(
                 "Scan this code with your HomeKit app on your iOS device:", flush=True
             )
             print(QRCode(xhm_uri).terminal(quiet_zone=2), flush=True)
             print(
-                "Or enter this code in your HomeKit app on your iOS device: "
-                "{}".format(pincode),
+                f"Or enter this code in your HomeKit app on your iOS device: {pincode}",
                 flush=True,
             )
         else:
@@ -252,9 +249,7 @@ def setup_message(self):
                 flush=True,
             )
             print(
-                "Enter this code in your HomeKit app on your iOS device: {}".format(
-                    pincode
-                ),
+                f"Enter this code in your HomeKit app on your iOS device: {pincode}",
                 flush=True,
             )
 

pyhap/accessory_driver.py

@@ -125,17 +125,12 @@ def __init__(self, accessory, state, zeroconf_server=None):
         self.state = state
 
         adv_data = self._get_advert_data()
+        valid_name = self._valid_name()
+        short_mac = self.state.mac[-8:].replace(":", "")
         # Append part of MAC address to prevent name conflicts
-        name = "{} {}.{}".format(
-            self._valid_name(),
-            self.state.mac[-8:].replace(":", ""),
-            HAP_SERVICE_TYPE,
-        )
-        server = zeroconf_server or "{}-{}.{}".format(
-            self._valid_host_name(),
-            self.state.mac[-8:].replace(":", ""),
-            "local.",
-        )
+        name = f"{valid_name} {short_mac}.{HAP_SERVICE_TYPE}"
+        valid_host_name = self._valid_host_name()
+        server = zeroconf_server or f"{valid_host_name}-{short_mac}.local."
         super().__init__(
             HAP_SERVICE_TYPE,
             name=name,

pyhap/characteristic.py

@@ -162,9 +162,7 @@ def __init__(self, display_name, type_id, properties):
 
     def __repr__(self):
         """Return the representation of the characteristic."""
-        return "<characteristic display_name={} value={} properties={}>".format(
-            self.display_name, self.value, self.properties
-        )
+        return f"<characteristic display_name={self.display_name} value={self.value} properties={self.properties}>"
 
     def _get_default_value(self):
         """Return default value for format."""
@@ -191,9 +189,7 @@ def to_valid_value(self, value):
         """Perform validation and conversion to valid value."""
         if self.properties.get(PROP_VALID_VALUES):
             if value not in self.properties[PROP_VALID_VALUES].values():
-                error_msg = "{}: value={} is an invalid value.".format(
-                    self.display_name, value
-                )
+                error_msg = f"{self.display_name}: value={value} is an invalid value."
                 logger.error(error_msg)
                 raise ValueError(error_msg)
         elif self.properties[PROP_FORMAT] == HAP_FORMAT_STRING:
@@ -204,8 +200,8 @@ def to_valid_value(self, value):
             value = bool(value)
         elif self.properties[PROP_FORMAT] in HAP_FORMAT_NUMERICS:
             if not isinstance(value, (int, float)):
-                error_msg = "{}: value={} is not a numeric value.".format(
-                    self.display_name, value
+                error_msg = (
+                    f"{self.display_name}: value={value} is not a numeric value."
                 )
                 logger.error(error_msg)
                 raise ValueError(error_msg)
@@ -281,10 +277,14 @@ def client_update_value(self, value, sender_client_addr=None):
 
         Change self.value to value and call callback.
         """
+        original_value = value
+        if self.type_id not in ALWAYS_NULL or original_value is not None:
+            value = self.to_valid_value(value)
         logger.debug(
-            "client_update_value: %s to %s from client: %s",
+            "client_update_value: %s to %s (original: %s) from client: %s",
             self.display_name,
             value,
+            original_value,
             sender_client_addr,
         )
         changed = self.value != value

pyhap/const.py

@@ -1,9 +1,9 @@
 """This module contains constants used by other modules."""
 MAJOR_VERSION = 4
-MINOR_VERSION = 2
-PATCH_VERSION = 1
-__short_version__ = "{}.{}".format(MAJOR_VERSION, MINOR_VERSION)
-__version__ = "{}.{}".format(__short_version__, PATCH_VERSION)
+MINOR_VERSION = 3
+PATCH_VERSION = 0
+__short_version__ = f"{MAJOR_VERSION}.{MINOR_VERSION}"
+__version__ = f"{__short_version__}.{PATCH_VERSION}"
 REQUIRED_PYTHON_VER = (3, 6)
 
 BASE_UUID = "-0000-1000-8000-0026BB765291"

pyhap/hap_handler.py

@@ -50,8 +50,8 @@ def __init__(self):
 
     def __repr__(self):
         """Return a human readable view of the response."""
-        return "<HAPResponse {} {} {} {}>".format(
-            self.status_code, self.reason, self.headers, self.body
+        return (
+            f"<HAPResponse {self.status_code} {self.reason} {self.headers} {self.body}>"
         )
 
 
@@ -453,7 +453,7 @@ def handle_pair_verify(self):
             self._pair_verify_two(tlv_objects)
         else:
             raise ValueError(
-                "Unknown pairing sequence of %s during pair verify" % (sequence)
+                f"Unknown pairing sequence of {sequence} during pair verify"
             )
 
     def _pair_verify_one(self, tlv_objects):
@@ -662,7 +662,7 @@ def handle_pairings(self):
             self._handle_list_pairings()
         else:
             raise ValueError(
-                "Unknown pairing request type of %s during pair verify" % (request_type)
+                f"Unknown pairing request type of {request_type} during pair verify"
             )
 
     def _handle_add_pairing(self, tlv_objects):
@@ -747,9 +747,7 @@ def handle_resource(self):
         if self.accessory_handler.accessory.category == CATEGORY_BRIDGE:
             accessory = self.accessory_handler.accessory.accessories.get(data["aid"])
             if not accessory:
-                raise ValueError(
-                    "Accessory with aid == {} not found".format(data["aid"])
-                )
+                raise ValueError(f"Accessory with aid == {data['aid']} not found")
         else:
             accessory = self.accessory_handler.accessory
 

pyhap/hap_protocol.py

@@ -48,7 +48,7 @@ def __init__(self, loop, connections, accessory_driver) -> None:
         self.last_activity = None
         self.hap_crypto = None
         self._event_timer = None
-        self._event_queue = []
+        self._event_queue = {}
 
         self.start_time = None
 
@@ -112,7 +112,7 @@ def close(self) -> None:
 
     def queue_event(self, data: dict, immediate: bool) -> None:
         """Queue an event for sending."""
-        self._event_queue.append(data)
+        self._event_queue[(data[HAP_REPR_AID], data[HAP_REPR_IID])] = data
         if immediate:
             self.loop.call_soon(self._send_events)
         elif not self._event_timer:
@@ -212,14 +212,14 @@ def _send_events(self):
         subscribed_events = self._event_queue_with_active_subscriptions()
         if subscribed_events:
             self.write(create_hap_event(subscribed_events))
-        self._event_queue = []
+        self._event_queue.clear()
 
     def _event_queue_with_active_subscriptions(self):
         """Remove any topics that have been unsubscribed after the event was generated."""
         topics = self.accessory_driver.topics
         return [
             event
-            for event in self._event_queue
+            for event in self._event_queue.values()
             if self.peername
             in topics.get(get_topic(event[HAP_REPR_AID], event[HAP_REPR_IID]), [])
         ]
@@ -253,7 +253,7 @@ def _process_one_event(self) -> bool:
             self.request_body = None
             return True
 
-        return self._handle_invalid_conn_state("Unexpected event: {}".format(event))
+        return self._handle_invalid_conn_state(f"Unexpected event: {event}")
 
     def _process_response(self, response) -> None:
         """Process a response from the handler."""

pyhap/loader.py

@@ -44,14 +44,14 @@ def get_char(self, name):
             or "Permissions" not in char_dict
             or "UUID" not in char_dict
         ):
-            raise KeyError("Could not load char {}!".format(name))
+            raise KeyError(f"Could not load char {name}!")
         return Characteristic.from_dict(name, char_dict, from_loader=True)
 
     def get_service(self, name):
         """Return new service object."""
         service_dict = self.serv_types[name].copy()
         if "RequiredCharacteristics" not in service_dict or "UUID" not in service_dict:
-            raise KeyError("Could not load service {}!".format(name))
+            raise KeyError(f"Could not load service {name}!")
         return Service.from_dict(name, service_dict, self)
 
     @classmethod

pyhap/service.py

@@ -42,9 +42,8 @@ def __init__(self, type_id, display_name=None):
 
     def __repr__(self):
         """Return the representation of the service."""
-        return "<service display_name={} chars={}>".format(
-            self.display_name, {c.display_name: c.value for c in self.characteristics}
-        )
+        chars_dict = {c.display_name: c.value for c in self.characteristics}
+        return f"<service display_name={self.display_name} chars={chars_dict}>"
 
     def add_linked_service(self, service):
         """Add the given service as "linked" to this Service."""

pyhap/tlv.py

@@ -17,8 +17,9 @@ def encode(*args, to_base64=False):
     :return: The args in TLV format
     :rtype: ``bytes`` if ``toBase64`` is False and ``str`` otherwise.
     """
-    if len(args) % 2 != 0:
-        raise ValueError("Even number of args expected (%d given)" % len(args))
+    arg_len = len(args)
+    if arg_len % 2 != 0:
+        raise ValueError(f"Even number of args expected ({arg_len} given)")
 
     pieces = []
     for x in range(0, len(args), 2):

pyhap/util.py

@@ -80,7 +80,7 @@ def generate_mac():
     :return: MAC address in format XX:XX:XX:XX:XX:XX
     :rtype: str
     """
-    return "{}{}:{}{}:{}{}:{}{}:{}{}:{}{}".format(
+    return "{}{}:{}{}:{}{}:{}{}:{}{}:{}{}".format(  # pylint: disable=consider-using-f-string
         *(rand.choice(HEX_DIGITS) for _ in range(12))
     )
 
@@ -104,9 +104,9 @@ def generate_pincode():
     :return: pincode in format ``xxx-xx-xxx``
     :rtype: bytearray
     """
-    return "{}{}{}-{}{}-{}{}{}".format(*(rand.randint(0, 9) for i in range(8))).encode(
-        "ascii"
-    )
+    return "{}{}{}-{}{}-{}{}{}".format(  # pylint: disable=consider-using-f-string
+        *(rand.randint(0, 9) for i in range(8))
+    ).encode("ascii")
 
 
 def to_base64_str(bytes_input) -> str:

tests/test_accessory.py

@@ -101,7 +101,7 @@ def test_bridge_add_accessory(mock_driver):
     bridge.add_accessory(acc)
     acc2 = Accessory(mock_driver, "Test Accessory 2")
     bridge.add_accessory(acc2)
-    assert acc2.aid != STANDALONE_AID and acc2.aid != acc.aid
+    assert acc2.aid not in (STANDALONE_AID, acc.aid)
 
 
 def test_bridge_n_add_accessory_bridge_aid(mock_driver):
@@ -550,3 +550,13 @@ def test_acc_with_(mock_driver):
     assert char_doorbell_detected_switch.to_HAP()[HAP_REPR_VALUE] is None
     char_doorbell_detected_switch.client_update_value(None)
     assert char_doorbell_detected_switch.to_HAP()[HAP_REPR_VALUE] is None
+
+
+def test_client_sends_invalid_value(mock_driver):
+    """Test cleaning up invalid client value."""
+    acc = Accessory(mock_driver, "Test Accessory")
+    serv_switch = acc.add_preload_service("Switch")
+    char_on = serv_switch.configure_char("On", value=False)
+    # Client sends 1, but it should be True
+    char_on.client_update_value(1)
+    assert char_on.to_HAP()[HAP_REPR_VALUE] is True

tests/test_accessory_driver.py

@@ -199,7 +199,7 @@ def test_service_callbacks(driver):
     mock_callback.assert_called_with({"On": True, "Brightness": 88})
 
     get_chars = driver.get_characteristics(
-        ["{}.{}".format(acc.aid, char_on_iid), "{}.{}".format(acc2.aid, char_on2_iid)]
+        [f"{acc.aid}.{char_on_iid}", f"{acc2.aid}.{char_on2_iid}"]
     )
     assert get_chars == {
         "characteristics": [
@@ -214,10 +214,10 @@ def _fail_func():
     char_brightness.getter_callback = _fail_func
     get_chars = driver.get_characteristics(
         [
-            "{}.{}".format(acc.aid, char_on_iid),
-            "{}.{}".format(acc2.aid, char_on2_iid),
-            "{}.{}".format(acc2.aid, char_brightness_iid),
-            "{}.{}".format(acc.aid, char_brightness2_iid),
+            f"{acc.aid}.{char_on_iid}",
+            f"{acc2.aid}.{char_on2_iid}",
+            f"{acc2.aid}.{char_brightness_iid}",
+            f"{acc.aid}.{char_brightness2_iid}",
         ]
     )
     assert get_chars == {

tests/test_hap_server.py

@@ -149,3 +149,50 @@ def _save_event(hap_event):
         b"EVENT/1.0 200 OK\r\nContent-Type: application/hap+json\r\nContent-Length: 87\r\n\r\n"
         b'{"characteristics":[{"aid":2,"iid":33,"value":false},{"aid":3,"iid":33,"value":false}]}'
     ]
+
+
+@pytest.mark.asyncio
+async def test_push_event_overwrites_old_pending_events(driver):
+    """Test push event overwrites old events in the event queue.
+
+    iOS 15 had a breaking change where events are no longer processed
+    in order. We want to make sure when we send an event message we
+    only send the latest state and overwrite all the previous states
+    for the same AID/IID that are in the queue when the state changes
+    before the event is sent.
+    """
+    addr_info = ("1.2.3.4", 1234)
+    server = hap_server.HAPServer(("127.0.01", 5555), driver)
+    server.loop = asyncio.get_event_loop()
+    hap_events = []
+
+    def _save_event(hap_event):
+        hap_events.append(hap_event)
+
+    hap_server_protocol = HAPServerProtocol(
+        server.loop, server.connections, server.accessory_handler
+    )
+    hap_server_protocol.write = _save_event
+    hap_server_protocol.peername = addr_info
+    server.accessory_handler.topics["1.33"] = {addr_info}
+    server.accessory_handler.topics["2.33"] = {addr_info}
+    server.connections[addr_info] = hap_server_protocol
+
+    assert (
+        server.push_event({"aid": 1, "iid": 33, "value": False}, addr_info, True)
+        is True
+    )
+    assert (
+        server.push_event({"aid": 1, "iid": 33, "value": True}, addr_info, True) is True
+    )
+    assert (
+        server.push_event({"aid": 2, "iid": 33, "value": False}, addr_info, True)
+        is True
+    )
+
+    await asyncio.sleep(0)
+    assert hap_events == [
+        b"EVENT/1.0 200 OK\r\nContent-Type: application/hap+json\r\nContent-Length: 86\r\n\r\n"
+        b'{"characteristics":[{"aid":1,"iid":33,"value":true},'
+        b'{"aid":2,"iid":33,"value":false}]}'
+    ]