8000 Refactor set_characteristics (#462) · ikalchev/HAP-python@f95cd15 · GitHub
[go: up one dir, main page]
Skip to content

Commit f95cd15

Browse files
kormaxkormax
authored
Refactor set_characteristics (#462)
1 parent e281b36 commit f95cd15

File tree

2 files changed

+95
-114
lines changed

2 files changed

+95
-114
lines changed

‎pyhap/accessory_driver.py

Lines changed: 93 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"""
1818
import asyncio
1919
import base64
20+
from collections import defaultdict
2021
from concurrent.futures import ThreadPoolExecutor
2122
import hashlib
2223
import logging
@@ -26,14 +27,14 @@
2627
import tempfile
2728
import threading
2829
import time
29-
from typing import Optional
30+
from typing import Any, Dict, List, Optional, Tuple
3031

3132
from zeroconf import ServiceInfo
3233
from zeroconf.asyncio import AsyncZeroconf
3334

3435
from pyhap import util
3536
from pyhap.accessory import Accessory, get_topic
36-
from pyhap.characteristic import CharacteristicError
37+
from pyhap.characteristic import Characteristic, CharacteristicError
3738
from pyhap.const import (
3839
HAP_PERMISSION_NOTIFY,
3940
HAP_PROTOCOL_SHORT_VERSION,
@@ -53,6 +54,7 @@
5354
from pyhap.hsrp import Server as SrpServer
5455
from pyhap.loader import Loader
5556
from pyhap.params import get_srp_context
57+
from pyhap.service import Service
5658
from pyhap.state import State
5759

5860
from .const import HAP_SERVER_STATUS
@@ -67,12 +69,13 @@
6769
VALID_MDNS_REGEX = re.compile(r"[^A-Za-z0-9\-]+")
6870
LEADING_TRAILING_SPACE_DASH = re.compile(r"^[ -]+|[ -]+$")
6971
DASH_REGEX = re.compile(r"[-]+")
72+
KEYS_TO_EXCLUDE = {HAP_REPR_IID, HAP_REPR_AID}
7073

7174

7275
def _wrap_char_setter(char, value, client_addr):
7376
"""Process an characteristic setter callback trapping and logging all exceptions."""
7477
try:
75-
result = char.client_update_value(value, client_addr)
78+
response = char.client_update_value(value, client_addr)
7679
except Exception: # pylint: disable=broad-except
7780
logger.exception(
7881
"%s: Error while setting characteristic %s to %s",
@@ -81,7 +84,7 @@ def _wrap_char_setter(char, value, client_addr):
8184
value,
8285
)
8386
return HAP_SERVER_STATUS.SERVICE_COMMUNICATION_FAILURE, None
84-
return HAP_SERVER_STATUS.SUCCESS, result
87+
return HAP_SERVER_STATUS.SUCCESS, response
8588

8689

8790
def _wrap_acc_setter(acc, updates_by_service, client_addr):
@@ -859,118 +862,98 @@ def set_characteristics(self, chars_query, client_addr):
859862
:type chars_query: dict
860863
"""
861864
# TODO: Add support for chars that do no support notifications.
862-
updates = {}
863-
setter_results = {}
864-
setter_responses = {}
865-
had_error = False
866-
had_write_response = False
867-
expired = False
868865

866+
queries: List[Dict[str, Any]] = chars_query[HAP_REPR_CHARS]
867+
868+
self._notify(queries, client_addr)
869+
870+
updates_by_accessories_services: Dict[
871+
Accessory, Dict[Service, Dict[Characteristic, Any]]
872+
] = defaultdict(lambda: defaultdict(dict))
873+
results: Dict[int, Dict[int, Dict[str, Any]]] = defaultdict(
874+
lambda: defaultdict(dict)
875+
)
876+
char_to_iid: Dict[Characteristic, int] = {}
877+
878+
expired = False
869879
if HAP_REPR_PID in chars_query:
870880
pid = chars_query[HAP_REPR_PID]
871881
expire_time = self.prepared_writes.get(client_addr, {}).pop(pid, None)
872-
if expire_time is None or time.time() > expire_time:
873-
expired = True
874-
875-
for cq in chars_query[HAP_REPR_CHARS]:
876-
aid, iid = cq[HAP_REPR_AID], cq[HAP_REPR_IID]
877-
setter_results.setdefault(aid, {})
882+
expired = expire_time is None or time.time() > expire_time
878883

879-
if HAP_REPR_WRITE_RESPONSE in cq:
880-
setter_responses.setdefault(aid, {})
881-
had_write_response = True
884+
primary_accessory = self.accessory
885+
primary_aid = primary_accessory.aid
882886

883-
if expired:
884-
setter_results[aid][iid] = HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST
885-
had_error = True
887+
for query in queries:
888+
if HAP_REPR_VALUE not in query and not expired:
886889
continue
887890

888-
if HAP_PERMISSION_NOTIFY in cq:
889-
char_topic = get_topic(aid, iid)
890-
action = "Subscribed" if cq[HAP_PERMISSION_NOTIFY] else "Unsubscribed"
891-
logger.debug(
892-
"%s client %s to topic %s", action, client_addr, char_topic
893-
)
894-
self.async_subscribe_client_topic(
895-
client_addr, char_topic, cq[HAP_PERMISSION_NOTIFY]
896-
)
891+
aid = query[HAP_REPR_AID]
892+
iid = query[HAP_REPR_IID]
893+
value = query.get(HAP_REPR_VALUE)
894+
write_response_requested = query.get(HAP_REPR_WRITE_RESPONSE, False)
897895

898-
if HAP_REPR_VALUE not in cq:
899-
continue
896+
if aid == primary_aid:
897+
acc = primary_accessory
898+
else:
899+
acc = self.accessory.accessories.get(aid)
900+
char = acc.get_characteristic(aid, iid)
901+
902+
set_result = HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST
903+
set_result_value = None
900904

901-
updates.setdefault(aid, {})[iid] = cq[HAP_REPR_VALUE]
905+
if value is not None:
906+
set_result, set_result_value = _wrap_char_setter(
907+
char, value, client_addr
908+
)
902909

903-
for aid, new_iid_values in updates.items():
904-
if self.accessory.aid == aid:
905-
acc = self.accessory
910+
if set_result_value is not None and write_response_requested:
911+
result = {HAP_REPR_STATUS: set_result, HAP_REPR_VALUE: set_result_value}
906912
else:
907-
acc = self.accessory.accessories.get(aid)
913+
result = {HAP_REPR_STATUS: set_result}
908914

909-
updates_by_service = {}
910-
char_to_iid = {}
911-
for iid, value in new_iid_values.items():
912-
# Characteristic level setter callbacks
913-
char = acc.get_characteristic(aid, iid)
914-
915-
set_result, set_result_value = _wrap_char_setter(char, value, client_addr)
916-
if set_result != HAP_SERVER_STATUS.SUCCESS:
917-
had_error = True
918-
919-
setter_results[aid][iid] = set_result
920-
921-
if set_result_value is not None:
922-
if setter_responses.get(aid, None) is None:
923-
logger.warning(
924-
"Returning write response '%s' when it wasn't requested for %s %s",
925-
set_result_value, aid, iid
926-
)
927-
had_write_response = True
928-
setter_responses.setdefault(aid, {})[iid] = set_result_value
929-
930-
if not char.service or (
931-
not acc.setter_callback and not char.service.setter_callback
932-
):
933-
continue
934-
char_to_iid[char] = iid
935-
updates_by_service.setdefault(char.service, {}).update({char: value})
915+
results[aid][iid] = result
916+
char_to_iid[char] = iid
917+
service = char.service
918+
updates_by_accessories_services[acc][service][char] = value
919+
920+
# Proccess accessory and service level setter callbacks
921+
for acc, updates_by_service in updates_by_accessories_services.items():
922+
aid = acc.aid
923+
aid_results = results[aid]
936924

937925
# Accessory level setter callbacks
926+
acc_set_result = None
938927
if acc.setter_callback:
939-
set_result = _wrap_acc_setter(acc, updates_by_service, client_addr)
940-
if set_result != HAP_SERVER_STATUS.SUCCESS:
941-
had_error = True
942-
for iid in updates[aid]:
943-
setter_results[aid][iid] = set_result
928+
acc_set_result = _wrap_acc_setter(acc, updates_by_service, client_addr)
944929

945930
# Service level setter callbacks
946931
for service, chars in updates_by_service.items():
947-
if not service.setter_callback:
932+
char_set_result = None
933+
if service.setter_callback:
934+
char_set_result = _wrap_service_setter(service, chars, client_addr)
935+
set_result = char_set_result or acc_set_result
936+
937+
if not set_result:
948938
continue
949-
set_result = _wrap_service_setter(service, chars, client_addr)
950-
if set_result != HAP_SERVER_STATUS.SUCCESS:
951-
had_error = True
952-
for char in chars:
953-
setter_results[aid][char_to_iid[char]] = set_result
954939

955-
if not had_error and not had_write_response:
956-
return None
940+
for char in chars:
941+
aid_results[char_to_iid[char]][HAP_REPR_STATUS] = set_result
942+
943+
characteristics = []
944+
nonempty_results_exist = False
945+
for aid, iid_results in results.items():
946+
for iid, result in iid_results.items():
947+
result[HAP_REPR_AID] = aid
948+
result[HAP_REPR_IID] = iid
949+
characteristics.append(result)
950+
if (
951+
result[HAP_REPR_STATUS] != HAP_SERVER_STATUS.SUCCESS
952+
or HAP_REPR_VALUE in result
953+
):
954+
nonempty_results_exist = True
957955

958-
return {
959-
HAP_REPR_CHARS: [
960-
{
961-
HAP_REPR_AID: aid,
962-
HAP_REPR_IID: iid,
963-
HAP_REPR_STATUS: status,
964-
**(
965-
{HAP_REPR_VALUE: setter_responses[aid][iid]}
966-
if setter_responses.get(aid, {}).get(iid, None) is not None
967-
else {}
968-
)
969-
}
970-
for aid, iid_status in setter_results.items()
971-
for iid, status in iid_status.items()
972-
]
973-
}
956+
return {HAP_REPR_CHARS: characteristics} if nonempty_results_exist else None
974957

975958
def prepare(self, prepare_query, client_addr):
976959
"""Called from ``HAPServerHandler`` when iOS wants to prepare a write.
@@ -1013,3 +996,19 @@ def signal_handler(self, _signal, _frame):
1013996
except Exception as e:
1014997
logger.error("Could not stop AccessoryDriver because of error: %s", e)
1015998
raise
999+
1000+
def _notify(
1001+
self, queries: List[Dict[str, Any]], client_addr: Tuple[str, int]
1002+
) -> None:
1003+
"""Notify the driver that the client has subscribed or unsubscribed."""
1004+
for query in queries:
1005+
if HAP_PERMISSION_NOTIFY not in query:
1006+
continue
1007+
aid = query[HAP_REPR_AID]
1008+
iid = query[HAP_REPR_IID]
1009+
ev = query[HAP_PERMISSION_NOTIFY]
1010+
1011+
char_topic = get_topic(aid, iid)
1012+
action = "Subscribed" if ev else "Unsubscribed"
1013+
logger.debug("%s client %s to topic %s", action, client_addr, char_topic)
1014+
self.async_subscribe_client_topic(client_addr, char_topic, ev)

‎tests/test_accessory_driver.py

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,7 @@ def setter_with_write_response(value=0):
177177
},
178178
"mock_addr",
179179
)
180-
assert response == {
181-
HAP_REPR_CHARS: [
182-
{
183-
HAP_REPR_AID: acc.aid,
184-
HAP_REPR_IID: char_nfc_access_control_point_iid,
185-
HAP_REPR_STATUS: 0,
186-
HAP_REPR_VALUE: 1
187-
},
188-
]
189-
}
180+
assert response is None
190181

191182
response = driver.set_characteristics(
192183
{
@@ -200,16 +191,7 @@ def setter_with_write_response(value=0):
200191
},
201192
"mock_addr",
202193
)
203-
assert response == {
204-
HAP_REPR_CHARS: [
205-
{
206-
HAP_REPR_AID: acc.aid,
207-
HAP_REPR_IID: char_nfc_access_control_point_iid,
208-
HAP_REPR_STATUS: 0,
209-
HAP_REPR_VALUE: 1
210-
},
211-
]
212-
}
194+
assert response is None
213195

214196

215197
def test_write_response_returned_when_requested(driver: AccessoryDriver):

0 commit comments

Comments
 (0)
0