Skip to content

Fix race condition that causes pairing and unpairing failures #246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ikalchev merged 1 commit into from
May 9, 2020
Merged

Fix race condition that causes pairing and unpairing failures #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions pyhap/accessory_driver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,7 @@ def load(self):
def pair(self, client_uuid, client_public):
"""Called when a client has paired with the accessory.

Updates the accessory with the paired client and updates the mDNS service. Also,
persist the new state.
Persist the new accessory state.

:param client_uuid: The client uuid.
:type client_uuid: uuid.UUID
Expand All @@ -527,24 +526,37 @@ def pair(self, client_uuid, client_public):
logger.info("Paired with %s.", client_uuid)
self.state.add_paired_client(client_uuid, client_public)
self.persist()
# Safe mode added to avoid error during pairing, see
# https://github.com/home-assistant/home-assistant/issues/14567
if not self.safe_mode:
self.update_advertisement()
return True

def unpair(self, client_uuid):
"""Removes the paired client from the accessory.

Updates the accessory and updates the mDNS service. Persist the new accessory
state.
Persist the new accessory state.

:param client_uuid: The client uuid.
:type client_uuid: uuid.UUID
"""
logger.info("Unpairing client %s.", client_uuid)
self.state.remove_paired_client(client_uuid)
self.persist()

def finish_pair(self):
"""Finishing pairing or unpairing.

Updates the accessory and updates the mDNS service.

The mDNS announcement must not be updated until AFTER
the final pairing response is sent or homekit will
see that the accessory is already paired and assume
it should stop pairing.
"""
# Safe mode added to avoid error during pairing, see
# https://github.com/home-assistant/home-assistant/issues/14567
#
# This may no longer be needed now that we defer
# updating the advertisement until after the final
# pairing response is sent.
#
if not self.safe_mode:
self.update_advertisement()

Expand Down
11 changes: 11 additions & 0 deletions pyhap/hap_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,13 @@ def _handle_add_pairing(self, tlv_objects):
self.send_header("Content-Type", self.PAIRING_RESPONSE_TYPE)
self.end_response(data)

# Avoid updating the announcement until
# after the response is sent as homekit will
# drop the connection and fail to pair if it
# sees the accessory is now paired as it doesn't
# know that it was the one doing the pairing.
self.accessory_handler.finish_pair()

def _handle_remove_pairing(self, tlv_objects):
"""Remove pairing with the client."""
logger.debug("Removing client pairing.")
Expand All @@ -610,6 +617,10 @@ def _handle_remove_pairing(self, tlv_objects):
self.send_header("Content-Type", self.PAIRING_RESPONSE_TYPE)
self.end_response(data)

# Avoid updating the announcement until
# after the response is sent.
self.accessory_handler.finish_pair()

def handle_resource(self):
"""Get a snapshot from the camera."""
if not hasattr(self.accessory_handler.accessory, 'get_snapshot'):
Expand Down