From 844ec408ca3e50adaab0778a9c0bb6ae0cbf7230 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 6 Oct 2021 08:24:29 -1000 Subject: [PATCH 1/4] Only send latest state if there are multiple for the same AID/IID in events (#385) --- pyhap/accessory.py | 13 +++------- pyhap/accessory_driver.py | 15 ++++------- pyhap/characteristic.py | 12 +++------ pyhap/const.py | 4 +-- pyhap/hap_handler.py | 12 ++++----- pyhap/hap_protocol.py | 10 ++++---- pyhap/loader.py | 4 +-- pyhap/service.py | 5 ++-- pyhap/tlv.py | 5 ++-- pyhap/util.py | 8 +++--- tests/test_accessory.py | 2 +- tests/test_accessory_driver.py | 10 ++++---- tests/test_hap_server.py | 47 ++++++++++++++++++++++++++++++++++ 13 files changed, 89 insertions(+), 58 deletions(-) diff --git a/pyhap/accessory.py b/pyhap/accessory.py index 85d10f8b..083655e4 100644 --- a/pyhap/accessory.py +++ b/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 "".format( - self.display_name, services - ) + return f"" @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, ) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 78acd175..9264504e 100644 --- a/pyhap/accessory_driver.py +++ b/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, diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index 9e6659e8..b57cec10 100644 --- a/pyhap/characteristic.py +++ b/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 "".format( - self.display_name, self.value, self.properties - ) + return f"" 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) diff --git a/pyhap/const.py b/pyhap/const.py index 2e1e506b..f8b397d5 100644 --- a/pyhap/const.py +++ b/pyhap/const.py @@ -2,8 +2,8 @@ MAJOR_VERSION = 4 MINOR_VERSION = 2 PATCH_VERSION = 1 -__short_version__ = "{}.{}".format(MAJOR_VERSION, MINOR_VERSION) -__version__ = "{}.{}".format(__short_version__, PATCH_VERSION) +__short_version__ = f"{MAJOR_VERSION}.{MINOR_VERSION}" +__version__ = f"{__short_version__}.{PATCH_VERSION}" REQUIRED_PYTHON_VER = (3, 6) BASE_UUID = "-0000-1000-8000-0026BB765291" diff --git a/pyhap/hap_handler.py b/pyhap/hap_handler.py index d65f0c05..22cd8e16 100644 --- a/pyhap/hap_handler.py +++ b/pyhap/hap_handler.py @@ -50,8 +50,8 @@ def __init__(self): def __repr__(self): """Return a human readable view of the response.""" - return "".format( - self.status_code, self.reason, self.headers, self.body + return ( + f"" ) @@ -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 diff --git a/pyhap/hap_protocol.py b/pyhap/hap_protocol.py index 9490a2ee..33b750ab 100644 --- a/pyhap/hap_protocol.py +++ b/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.""" diff --git a/pyhap/loader.py b/pyhap/loader.py index 7a89ce94..1883e188 100644 --- a/pyhap/loader.py +++ b/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 diff --git a/pyhap/service.py b/pyhap/service.py index 098ed8c0..837c8712 100644 --- a/pyhap/service.py +++ b/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 "".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"" def add_linked_service(self, service): """Add the given service as "linked" to this Service.""" diff --git a/pyhap/tlv.py b/pyhap/tlv.py index 6847b611..d43a3276 100644 --- a/pyhap/tlv.py +++ b/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): diff --git a/pyhap/util.py b/pyhap/util.py index ff1f22e0..2f8f92f9 100644 --- a/pyhap/util.py +++ b/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: diff --git a/tests/test_accessory.py b/tests/test_accessory.py index b51285ef..8e97fd8e 100644 --- a/tests/test_accessory.py +++ b/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): diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 35234f0f..4588ac03 100644 --- a/tests/test_accessory_driver.py +++ b/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 == { diff --git a/tests/test_hap_server.py b/tests/test_hap_server.py index e13a8368..ad929f53 100644 --- a/tests/test_hap_server.py +++ b/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}]}' + ] From 4b1890c2b0defcde02e8594bd2ba3f5305151ae2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 6 Oct 2021 08:24:57 -1000 Subject: [PATCH 2/4] Fix latest pylint updates (#386) From b84df626bceca989164011d753e7320710209dcb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 6 Oct 2021 08:25:55 -1000 Subject: [PATCH 3/4] Handle invalid formats from clients (#387) --- pyhap/characteristic.py | 6 +++++- tests/test_accessory.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index b57cec10..eea50fbb 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -277,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 diff --git a/tests/test_accessory.py b/tests/test_accessory.py index 8e97fd8e..00025348 100644 --- a/tests/test_accessory.py +++ b/tests/test_accessory.py @@ -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 From 7f14eb153252ac4706b9f1c658662e7a0982fb7e Mon Sep 17 00:00:00 2001 From: Ivan Kalchev Date: Thu, 7 Oct 2021 23:58:52 +0300 Subject: [PATCH 4/4] v4.3.0 --- CHANGELOG.md | 8 +++++++- pyhap/const.py | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1417f9f2..86a2322b 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/pyhap/const.py b/pyhap/const.py index f8b397d5..25834be8 100644 --- a/pyhap/const.py +++ b/pyhap/const.py @@ -1,7 +1,7 @@ """This module contains constants used by other modules.""" MAJOR_VERSION = 4 -MINOR_VERSION = 2 -PATCH_VERSION = 1 +MINOR_VERSION = 3 +PATCH_VERSION = 0 __short_version__ = f"{MAJOR_VERSION}.{MINOR_VERSION}" __version__ = f"{__short_version__}.{PATCH_VERSION}" REQUIRED_PYTHON_VER = (3, 6)