diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 9a73f8ed..032c4286 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -1343,12 +1343,13 @@ def _run_characteristic_writes( and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS] ) # A new write replaces a former response of this connection. The - # request of a readable write response characteristic is not - # shared, whatever the setter does, also not with ev subscribers. + # request of a readable write response or read back characteristic + # is not shared, whatever the setter does, also not with ev + # subscribers. if value is not None: self._write_responses.get(client_addr, {}).pop(char, None) private_write = ( - write_response + (write_response or char.read_back_response) and value is not None and HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS] ) @@ -1366,8 +1367,22 @@ def _run_characteristic_writes( # the writer reads the answer back, see _store_write_response. # client_update_value stored the request as the value, so put # back the former one unless the setter set a value of its own. - if private_write and char.value == _stored_value(char, value): - char.value = previous_value + # Whatever the setter of a read back characteristic leaves in the + # value is an answer for the writer only, e.g. the Setup Endpoints + # answer of a subclass that calls super() and returns nothing. + # Neither the former value of a refused write nor the request the + # setter left alone is an answer. + if private_write: + own_value = char.value + if char.read_back_response: + if set_result_value is None and own_value not in ( + previous_value, + _stored_value(char, value), + ): + set_result_value = own_value + char.value = previous_value + elif own_value == _stored_value(char, value): + char.value = previous_value result: dict[str, Any] = {HAP_REPR_STATUS: set_result} if inspect.iscoroutine(set_result_value): @@ -1381,9 +1396,9 @@ def _run_characteristic_writes( not private_write, ) ) - elif set_result_value is not None and write_response: - result[HAP_REPR_VALUE] = self._store_write_response( - char, set_result_value, client_addr + elif set_result_value is not None: + self._keep_response( + result, char, set_result_value, write_response, client_addr ) results[aid][iid] = result @@ -1464,11 +1479,11 @@ async def _async_finish_set_characteristics( write.char.client_update_finished( write.previous_value, client_addr, notify=write.notify ) - if outcome is not None and write.write_response: + if outcome is not None: # A response queued when the connection closes is dropped by # the deferred cleanup in connection_lost. - write.result[HAP_REPR_VALUE] = self._store_write_response( - write.char, outcome, client_addr + self._keep_response( + write.result, write.char, outcome, write.write_response, client_addr ) return _set_characteristics_response(results) @@ -1491,6 +1506,25 @@ def _finish_done_write( write.previous_value, client_addr, notify=write.notify ) + def _keep_response( + self, + result: dict[str, Any], + char: Characteristic, + response: HAPValue, + write_response: bool, + client_addr: tuple[str, int] | None, + ) -> None: + """Keep what a setter returned as a write response or read back value.""" + if write_response: + result[HAP_REPR_VALUE] = self._store_write_response( + char, response, client_addr + ) + elif ( + char.read_back_response + and HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS] + ): + self._store_write_response(char, response, client_addr) + def _take_prepared_write(self, pid: Any, client_addr: tuple[str, int]) -> bool: """Consume the prepared write ``pid`` and return whether it is still valid.""" if not _is_hap_int(pid): @@ -1513,8 +1547,9 @@ def _store_write_response( response back, until it closes, and it wins over the getter and a later ``set_value``. Other connections read the characteristic's own value, which the write leaves as it was. A write without ``r`` stays readable - for everyone, and a characteristic that also has ``ev`` notifies the - written value as any write does. + for everyone and notifies ``ev`` subscribers as any write does, unless + the characteristic has ``read_back_response``, which keeps the response + the same way without a write response. """ readable = HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS] try: diff --git a/pyhap/camera.py b/pyhap/camera.py index e58fe377..8de494d0 100644 --- a/pyhap/camera.py +++ b/pyhap/camera.py @@ -7,8 +7,8 @@ [0. Read the current streaming status] 1. Sets the SetupEndpoints characteristic to notify the camera about its IP address, selected security parameters, etc. -2. The camera responds to the above by setting the SetupEndpoints with its IP address, -etc. +2. The camera answers the write with its IP address, etc., and the client reads the +answer back from SetupEndpoints. 3. The client sets the SelectedRTPStreamConfiguration characteristic to notify the camera of its prefered audio and video configuration and to initiate the start of the streaming. @@ -349,7 +349,7 @@ def _decode_setup_endpoints( ) -def _setup_endpoints_status(session_id: UUID, status: bytes) -> Any: +def _setup_endpoints_status(session_id: UUID, status: bytes) -> str: """Encode a Setup Endpoints answer that has only a status.""" return tlv.encode( SETUP_TYPES["SESSION_ID"], @@ -745,12 +745,14 @@ def _create_stream_management( "SelectedRTPStreamConfiguration", setter_callback=self.set_selected_stream_configuration, ) - management.configure_char( + setup_endpoints = management.configure_char( "SetupEndpoints", setter_callback=lambda value: self.set_endpoints( value, stream_idx=stream_idx ), ) + # The answer has the session's SRTP keys, only its controller reads it. + setup_endpoints.read_back_response = True return management def _default_recording_configs( @@ -1748,15 +1750,22 @@ def set_streaming_available(self, stream_idx: int) -> None: self._streaming_status[stream_idx] = STREAMING_STATUS["AVAILABLE"] self._management[stream_idx].get_characteristic("StreamingStatus").notify() - def set_endpoints(self, value: str, stream_idx: int | None = None) -> None: + def set_endpoints(self, value: str, stream_idx: int | None = None) -> str: """Configure streaming endpoints. - Called when iOS sets the SetupEndpoints ``Characteristic``. The endpoint - information for the camera should be set as the current value of SetupEndpoints. - A refused write sets the error answer. + Called when iOS sets the SetupEndpoints ``Characteristic``. Sets the + endpoint information of the camera, or a refusal, as the value of + SetupEndpoints and returns it. A subclass that calls ``super()`` and + returns nothing still answers. For a controller's write the driver + keeps the answer for the connection that wrote and puts the former + value back. An async setter must return the answer and must not set + the value itself, a value it sets is shared with every controller. A + direct ``client_update_value`` leaves the answer as the value. :param value: The base64-encoded stream session details in TLV format. :param value: ``str`` + :return: The base64-encoded answer in TLV format. + :rtype: ``str`` """ if stream_idx is None: stream_idx = 0 @@ -1774,23 +1783,16 @@ def set_endpoints(self, value: str, stream_idx: int | None = None) -> None: ) = _decode_setup_endpoints(session_id, objs) except (IndexError, KeyError, ValueError, struct.error) as ex: logger.warning("[%s] Refusing the endpoints: %s", session_id, ex) - self._management[stream_idx].get_characteristic("SetupEndpoints").set_value( - _setup_endpoints_status(session_id, SETUP_STATUS["ERROR"]) + # Like an answer, the refusal is only for the writer. + return self._answer_endpoints( + stream_idx, _setup_endpoints_status(session_id, SETUP_STATUS["ERROR"]) ) - return if not self.streaming_allowed: logger.info("[%s] Streaming is turned off, refusing it", session_id) - self._management[stream_idx].get_characteristic("SetupEndpoints").set_value( - tlv.encode( - SETUP_TYPES["SESSION_ID"], - session_id.bytes, - SETUP_TYPES["STATUS"], - SETUP_STATUS["ERROR"], - to_base64=True, - ) + return self._answer_endpoints( + stream_idx, _setup_endpoints_status(session_id, SETUP_STATUS["ERROR"]) ) - return # Configure the SetupEndpoints response @@ -1888,9 +1890,14 @@ def set_endpoints(self, value: str, stream_idx: int | None = None) -> None: "audio_backchannel": audio_backchannel, } + return self._answer_endpoints(stream_idx, response_tlv) + + def _answer_endpoints(self, stream_idx: int, answer: str) -> str: + """Set the Setup Endpoints answer as the value and return it.""" self._management[stream_idx].get_characteristic("SetupEndpoints").set_value( - response_tlv + answer, should_notify=False ) + return answer async def run(self) -> None: """Start the HDS listener when recording transport is available.""" diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index 19a2730e..e5739cd1 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -148,6 +148,13 @@ class Characteristic: the current temperature. Characteristics are contained in services. Each characteristic has a unique type UUID and a set of properties, like format, min and max values, valid values and others. + + Set ``read_back_response`` to True for a readable control point that a + controller writes and then reads, such as Setup Endpoints: what the + setter returns or leaves as the value is read back only by the connection + that wrote, and the shared value stays as it was. An async setter must + return the answer and must not set the value itself, a value it sets is + shared with every controller. """ __slots__ = ( @@ -168,6 +175,7 @@ class Characteristic: "_always_null", "_setter_addr_cache", "_setter_addr_cache_for", + "read_back_response", ) def __init__( @@ -224,6 +232,10 @@ def __init__( self._loader_display_name: str | None = None self._to_hap_cache_with_value: dict[str, Any] | None = None self._to_hap_cache: dict[str, Any] | None = None + # For a control point a controller writes and then reads, such as + # Setup Endpoints: the setter's return value is what the writing + # connection reads back, like a write response. + self.read_back_response = False @property def display_name(self) -> str | None: diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index d8e9a856..13251a16 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -2928,6 +2928,113 @@ def test_prepare_bounds_prepared_writes(driver: AccessoryDriver) -> None: assert MAX_PREPARED_WRITES_PER_CLIENT + 4 in pending +def _add_read_back_char(driver: AccessoryDriver) -> tuple[Accessory, Characteristic]: + """Add a pr and pw char that answers a write with 7 to read back.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw"], False) + char.getter_callback = None + char.setter_callback = MagicMock(return_value=7) + char.read_back_response = True + return acc, char + + +@pytest.mark.parametrize( + ("reader", "expected"), + [ + pytest.param(WRITER, 7, id="writer"), + pytest.param(OTHER_CONTROLLER, 0, id="other-controller"), + ], +) +def test_read_back_response_only_for_writer( + driver: AccessoryDriver, reader: Any, expected: int +) -> None: + """Only the writing controller reads back what the setter answered.""" + acc, char = _add_read_back_char(driver) + + assert _write(driver, acc, char, **{HAP_REPR_VALUE: 5}) is None + + assert _read(driver, f"{acc.aid}.{acc.iid_manager.get_iid(char)}", reader) == ( + expected + ) + acc_rep = driver.get_accessories()[HAP_REPR_ACCS][-1] + assert acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS][0][HAP_REPR_VALUE] == 0 + + +def test_read_back_prefers_the_returned_answer(driver: AccessoryDriver) -> None: + """The answer a setter returns wins over a value it sets itself.""" + acc, char = _add_read_back_char(driver) + + def _setter(value: int) -> int: + char.set_value(42) + return 7 + + char.setter_callback = _setter + + assert _write(driver, acc, char, **{HAP_REPR_VALUE: 5}) is None + + char_id = f"{acc.aid}.{acc.iid_manager.get_iid(char)}" + assert _read(driver, char_id, WRITER) == 7 + assert _read(driver, char_id, OTHER_CONTROLLER) == 0 + + +@pytest.mark.parametrize( + ("reader", "expected"), + [ + pytest.param(WRITER, 42, id="writer"), + pytest.param(OTHER_CONTROLLER, 0, id="other-controller"), + ], +) +def test_read_back_value_set_by_setter_only_for_writer( + driver: AccessoryDriver, reader: Any, expected: int +) -> None: + """A value the setter sets itself on a read back char stays with the writer.""" + acc, char = _add_read_back_char(driver) + + def _setter(value: int) -> None: + char.set_value(42) + + char.setter_callback = _setter + + assert _write(driver, acc, char, **{HAP_REPR_VALUE: 5}) is None + + assert _read(driver, f"{acc.aid}.{acc.iid_manager.get_iid(char)}", reader) == ( + expected + ) + acc_rep = driver.get_accessories()[HAP_REPR_ACCS][-1] + assert acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS][0][HAP_REPR_VALUE] == 0 + + +async def test_async_read_back_response(driver: AccessoryDriver) -> None: + """A coroutine setter's answer is read back, not sent as a write response.""" + acc, char = _add_read_back_char(driver) + char.setter_callback = _async_return_one + + assert await _controller_write(driver, acc, char, **{HAP_REPR_VALUE: 5}) is None + + char_id = f"{acc.aid}.{acc.iid_manager.get_iid(char)}" + assert _read(driver, char_id, WRITER) == 1 + assert _read(driver, char_id, OTHER_CONTROLLER) == 0 + + +@pytest.mark.parametrize( + ("value", "setter"), + [ + pytest.param("notanint", MagicMock(return_value=None), id="invalid-value"), + pytest.param(5, MagicMock(return_value=None), id="setter-does-nothing"), + ], +) +def test_read_back_without_answer_keeps_nothing_for_writer( + driver: AccessoryDriver, value: Any, setter: MagicMock +) -> None: + """Neither the former value nor the request is kept as the writer's answer.""" + acc, char = _add_read_back_char(driver) + char.setter_callback = setter + + _write(driver, acc, char, **{HAP_REPR_VALUE: value}) + char.set_value(3) + + assert _read(driver, f"{acc.aid}.{acc.iid_manager.get_iid(char)}", WRITER) == 3 + + CLIENT = ("1.2.3.4", 5) diff --git a/tests/test_camera.py b/tests/test_camera.py index a9350f39..62bf32b8 100644 --- a/tests/test_camera.py +++ b/tests/test_camera.py @@ -7,13 +7,20 @@ import socket import struct from typing import Any, cast -from unittest.mock import AsyncMock, Mock, patch +from unittest.mock import AsyncMock, Mock, PropertyMock, patch from uuid import UUID import pytest from pyhap import camera, tlv +# A Setup Endpoints request of a controller at 192.168.1.114. +_SETUP_ENDPOINTS_REQUEST = ( + "ARCszGzBBWNFFY2pdLRQkAaRAxoBAQACDTE5Mi4xNjguMS4xMTQDAjPFBAKs1gQ" + "lAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QEBAAUlAhAKRPSRVa" + "qGeNmESTIojxNiAw78WkjTLtGv0waWnLo9gQEBAA==" +) + _OPTIONS = { "stream_count": 4, "video": { @@ -75,12 +82,6 @@ def test_init(mock_driver: Any) -> None: def test_setup_endpoints(mock_driver: Any) -> None: """Test that the SetupEndpoint response is computed correctly""" - set_endpoint_req = ( - "ARCszGzBBWNFFY2pdLRQkAaRAxoBAQACDTE5Mi4xNjguMS4xMTQDAjPFBAKs1gQ" - "lAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QEBAAUlAhAKRPSRVa" - "qGeNmESTIojxNiAw78WkjTLtGv0waWnLo9gQEBAA==" - ) - set_endpoint_res = ( "ARCszGzBBWNFFY2pdLRQkAaRAgEAAxoBAQACDTE5Mi4xNjguMS4yMjYDAjPFBAK" "s1gQlAQEAAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QUlAQEAAh" @@ -91,33 +92,29 @@ def test_setup_endpoints(mock_driver: Any) -> None: management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") - setup_endpoints.client_update_value(set_endpoint_req) + response = setup_endpoints.client_update_value(_SETUP_ENDPOINTS_REQUEST) - assert setup_endpoints.get_value()[:171] == set_endpoint_res[:171] + assert response[:171] == set_endpoint_res[:171] + # Without the driver nothing puts the former value back. + assert setup_endpoints.get_value() == response def test_setup_endpoints_does_not_log_srtp_keys( mock_driver: Any, caplog: pytest.LogCaptureFixture ) -> None: """Verify the negotiated SRTP key material is not written to the logs.""" - set_endpoint_req = ( - "ARCszGzBBWNFFY2pdLRQkAaRAxoBAQACDTE5Mi4xNjguMS4xMTQDAjPFBAKs1gQ" - "lAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QEBAAUlAhAKRPSRVa" - "qGeNmESTIojxNiAw78WkjTLtGv0waWnLo9gQEBAA==" - ) - acc = camera.Camera(_OPTIONS, mock_driver, "Camera") management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") with caplog.at_level(logging.DEBUG, logger="pyhap.camera"): - setup_endpoints.client_update_value(set_endpoint_req) + setup_endpoints.client_update_value(_SETUP_ENDPOINTS_REQUEST) # The endpoint configuration was logged at DEBUG level, but without any # of the SRTP master key/salt material in it. assert "Received endpoint configuration:" in caplog.text - key_and_salt = _audio_srtp_key_and_salt(set_endpoint_req) + key_and_salt = _audio_srtp_key_and_salt(_SETUP_ENDPOINTS_REQUEST) assert base64.b64encode(key_and_salt).decode() not in caplog.text assert key_and_salt.hex() not in caplog.text @@ -186,19 +183,13 @@ def test_no_talkback_by_default(mock_driver: Any) -> None: SetupEndpoints keeps echoing the controller's port back (unchanged, pre-existing behavior for accessories that don't opt in). """ - set_endpoint_req = ( - "ARCszGzBBWNFFY2pdLRQkAaRAxoBAQACDTE5Mi4xNjguMS4xMTQDAjPFBAKs1gQ" - "lAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QEBAAUlAhAKRPSRVa" - "qGeNmESTIojxNiAw78WkjTLtGv0waWnLo9gQEBAA==" - ) - acc = camera.Camera(_OPTIONS, mock_driver, "Camera") assert acc.get_service("Speaker") is None management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") - setup_endpoints.client_update_value(set_endpoint_req) + setup_endpoints.client_update_value(_SETUP_ENDPOINTS_REQUEST) session_id = list(acc.sessions)[0] assert acc.sessions[session_id]["audio_backchannel"] is None @@ -260,26 +251,20 @@ def test_setup_endpoints_reports_real_port_when_talkback_enabled( this module fixes: HAP clients had nowhere valid to send the talkback audio to, because the accessory never actually listened anywhere). """ - set_endpoint_req = ( - "ARCszGzBBWNFFY2pdLRQkAaRAxoBAQACDTE5Mi4xNjguMS4xMTQDAjPFBAKs1gQ" - "lAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QEBAAUlAhAKRPSRVa" - "qGeNmESTIojxNiAw78WkjTLtGv0waWnLo9gQEBAA==" - ) - options = dict(_OPTIONS, talkback=True) acc = camera.Camera(options, mock_driver, "Camera") management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") - setup_endpoints.client_update_value(set_endpoint_req) + response = setup_endpoints.client_update_value(_SETUP_ENDPOINTS_REQUEST) session_id = list(acc.sessions)[0] receiver = acc.sessions[session_id]["audio_backchannel"] assert receiver is not None - reported_port = _accessory_audio_port(setup_endpoints.get_value()) + reported_port = _accessory_audio_port(response) assert reported_port == receiver.local_port - assert reported_port != _controller_audio_port(set_endpoint_req) + assert reported_port != _controller_audio_port(_SETUP_ENDPOINTS_REQUEST) receiver.stop() @@ -300,18 +285,12 @@ class TalkbackCamera(camera.Camera): def talkback_audio_received(self, session_id: Any, payload: Any) -> None: received.append((session_id, payload)) - set_endpoint_req = ( - "ARCszGzBBWNFFY2pdLRQkAaRAxoBAQACDTE5Mi4xNjguMS4xMTQDAjPFBAKs1gQ" - "lAhDYlmCkyTBZQfxqFS3OnxVOAw4bQZm5NuoQjyanlqWA0QEBAAUlAhAKRPSRVa" - "qGeNmESTIojxNiAw78WkjTLtGv0waWnLo9gQEBAA==" - ) - options = dict(_OPTIONS, talkback=True) acc = TalkbackCamera(options, mock_driver, "Camera") management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") - setup_endpoints.client_update_value(set_endpoint_req) + setup_endpoints.client_update_value(_SETUP_ENDPOINTS_REQUEST) session_id = list(acc.sessions)[0] receiver = acc.sessions[session_id]["audio_backchannel"] @@ -320,7 +299,7 @@ def talkback_audio_received(self, session_id: Any, payload: Any) -> None: # controller is local so the loopback send passes the source filter. receiver._controller_address = "127.0.0.1" - key_and_salt = _audio_srtp_key_and_salt(set_endpoint_req) + key_and_salt = _audio_srtp_key_and_salt(_SETUP_ENDPOINTS_REQUEST) rtp_payload = b"opus-frame-payload" rtp_packet = struct.pack("!BBHII", 0x80, 110, 1, 0, 0x1234) + rtp_payload @@ -535,9 +514,9 @@ def test_setup_endpoints_refuses_addresses( management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") - setup_endpoints.client_update_value(_setup_request(address, is_ipv6)) + answer = setup_endpoints.client_update_value(_setup_request(address, is_ipv6)) assert acc.sessions == {} - assert tlv.decode(setup_endpoints.get_value(), from_base64=True) == { + assert tlv.decode(answer, from_base64=True) == { camera.SETUP_TYPES["SESSION_ID"]: _SESSION_ID.bytes, camera.SETUP_TYPES["STATUS"]: camera.SETUP_STATUS["ERROR"], } @@ -580,9 +559,9 @@ def test_setup_endpoints_refuses_malformed( management = acc.get_service("CameraRTPStreamManagement") assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") - setup_endpoints.client_update_value(request_value) + answer = setup_endpoints.client_update_value(request_value) assert acc.sessions == {} - assert tlv.decode(setup_endpoints.get_value(), from_base64=True) == { + assert tlv.decode(answer, from_base64=True) == { camera.SETUP_TYPES["SESSION_ID"]: _SESSION_ID.bytes, camera.SETUP_TYPES["STATUS"]: camera.SETUP_STATUS["ERROR"], } @@ -612,15 +591,133 @@ def test_setup_endpoints_refuses_address_before_streaming_off( assert management is not None setup_endpoints = management.get_characteristic("SetupEndpoints") with caplog.at_level(logging.INFO): - setup_endpoints.client_update_value(_setup_request(address, False)) + answer = setup_endpoints.client_update_value(_setup_request(address, False)) assert message in caplog.text assert acc.sessions == {} - assert tlv.decode(setup_endpoints.get_value(), from_base64=True) == { + assert tlv.decode(answer, from_base64=True) == { camera.SETUP_TYPES["SESSION_ID"]: _SESSION_ID.bytes, camera.SETUP_TYPES["STATUS"]: camera.SETUP_STATUS["ERROR"], } +_WRITER = ("192.168.1.114", 50000) +_OTHER_CONTROLLER = ("192.168.1.115", 50000) + + +class _WrappingCamera(camera.Camera): + """A subclass that wraps set_endpoints and returns nothing.""" + + def set_endpoints( # ty: ignore[incompatible-method-override] + self, value: str, stream_idx: int | None = None + ) -> None: + super().set_endpoints(value, stream_idx) + + +class _ReturningCamera(camera.Camera): + """A subclass that wraps set_endpoints and returns the answer.""" + + def set_endpoints(self, value: str, stream_idx: int | None = None) -> str: + return super().set_endpoints(value, stream_idx) + + +class _SettingCamera(camera.Camera): + """A subclass that sets the answer itself, as before, and returns nothing.""" + + def set_endpoints( # ty: ignore[incompatible-method-override] + self, value: str, stream_idx: int | None = None + ) -> None: + answer = super().set_endpoints(value, stream_idx) + _setup_endpoints_char(self).set_value(answer) + + +def _setup_endpoints_char(acc: camera.Camera) -> Any: + """Return the Setup Endpoints characteristic of the first stream.""" + management = acc.get_service("CameraRTPStreamManagement") + assert management is not None + return management.get_characteristic("SetupEndpoints") + + +def _setup_endpoints_query( + acc: camera.Camera, request: str = _SETUP_ENDPOINTS_REQUEST +) -> dict[str, Any]: + """Return a controller's write of a Setup Endpoints request.""" + iid = acc.iid_manager.get_iid(_setup_endpoints_char(acc)) + return {"characteristics": [{"aid": acc.aid, "iid": iid, "value": request}]} + + +def _read_setup_endpoints(driver: Any, acc: camera.Camera, client_addr: Any) -> Any: + """Return the Setup Endpoints value ``client_addr`` reads.""" + iid = acc.iid_manager.get_iid(_setup_endpoints_char(acc)) + (rep,) = driver.get_characteristics([f"{acc.aid}.{iid}"], client_addr)[ + "characteristics" + ] + return rep["value"] + + +@pytest.mark.parametrize( + ("request_value", "streaming_allowed", "status"), + [ + pytest.param( + _SETUP_ENDPOINTS_REQUEST, + True, + camera.SETUP_STATUS["SUCCESS"], + id="answer", + ), + pytest.param( + _setup_request(b"192.168.1.114 x", False), + True, + camera.SETUP_STATUS["ERROR"], + id="malformed", + ), + pytest.param( + _SETUP_ENDPOINTS_REQUEST, + False, + camera.SETUP_STATUS["ERROR"], + id="streaming-off", + ), + ], +) +@pytest.mark.parametrize( + "camera_cls", + [camera.Camera, _WrappingCamera, _ReturningCamera, _SettingCamera], + ids=["camera", "wrapping-subclass", "returning-subclass", "setting-subclass"], +) +def test_setup_endpoints_read_back_by_writer_only( + driver: Any, + camera_cls: type[camera.Camera], + request_value: str, + streaming_allowed: bool, + status: bytes, +) -> None: + """Test only the controller that set up a session reads its answer.""" + acc = camera_cls(_OPTIONS, driver, "Camera") + driver.add_accessory(acc) + char = _setup_endpoints_char(acc) + + with patch.object( + camera.Camera, + "streaming_allowed", + new_callable=PropertyMock, + return_value=streaming_allowed, + ): + assert ( + driver.set_characteristics( + _setup_endpoints_query(acc, request_value), _WRITER + ) + is None + ) + + # The writer reads the answer, which carries a status, not its request. + answer = tlv.decode(_read_setup_endpoints(driver, acc, _WRITER), from_base64=True) + session_id = camera.SETUP_TYPES["SESSION_ID"] + assert answer[session_id] == tlv.decode(request_value, from_base64=True)[session_id] + assert answer[camera.SETUP_TYPES["STATUS"]] == status + assert _read_setup_endpoints(driver, acc, _OTHER_CONTROLLER) == "" + assert char.value == "" + # /accessories shows the shared value. + assert char.to_HAP()["value"] == "" + + @pytest.mark.asyncio async def test_stop_stream_process_already_exited(mock_driver: Any) -> None: """Verify stopping a stream whose ffmpeg already exited does not raise.""" diff --git a/tests/test_camera_operating_mode.py b/tests/test_camera_operating_mode.py index 5d015b70..fd3f3488 100644 --- a/tests/test_camera_operating_mode.py +++ b/tests/test_camera_operating_mode.py @@ -217,9 +217,9 @@ async def test_setup_endpoints_needs_streaming_allowed( camera = _camera(driver, video=VIDEO, audio=AUDIO, **options) _char(camera, service, "HomeKitCameraActive").client_update_value(enabled) setup = _char(camera, "CameraRTPStreamManagement", "SetupEndpoints") - setup.client_update_value(SETUP_REQUEST) + answer = setup.client_update_value(SETUP_REQUEST) - response = tlv.decode(setup.get_value(), from_base64=True) + response = tlv.decode(answer, from_base64=True) assert response[SETUP_TYPES["SESSION_ID"]] == SESSION_ID.bytes assert response[SETUP_TYPES["STATUS"]] == status assert list(camera.sessions) == sessions