Force single for some smartcam requests (#1374)
Some checks are pending
CI / Perform linting checks (3.13) (push) Waiting to run
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, macos-latest, 3.11) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, macos-latest, 3.12) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, macos-latest, 3.13) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, ubuntu-latest, 3.11) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, ubuntu-latest, 3.12) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, ubuntu-latest, 3.13) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, windows-latest, 3.11) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, windows-latest, 3.12) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (false, windows-latest, 3.13) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (true, ubuntu-latest, 3.11) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (true, ubuntu-latest, 3.12) (push) Blocked by required conditions
CI / Python ${{ matrix.python-version}} on ${{ matrix.os }}${{ fromJSON('[" (extras)", ""]')[matrix.extras == ''] }} (true, ubuntu-latest, 3.13) (push) Blocked by required conditions
CodeQL checks / Analyze (python) (push) Waiting to run

`onboarding` requests do not return the method key and need to be sent as single requests.
This commit is contained in:
Steven B. 2024-12-15 16:03:12 +00:00 committed by GitHub
parent c439530f93
commit f8503e4df6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 117 additions and 8 deletions

View File

@ -69,6 +69,13 @@ REDACTORS: dict[str, Callable[[Any], Any] | None] = {
"map_data": lambda x: "#SCRUBBED_MAPDATA#" if x else "",
}
# Queries that are known not to work properly when sent as a
# multiRequest. They will not return the `method` key.
FORCE_SINGLE_REQUEST = {
"getConnectStatus",
"scanApList",
}
class SmartProtocol(BaseProtocol):
"""Class for the new TPLink SMART protocol."""
@ -89,6 +96,7 @@ class SmartProtocol(BaseProtocol):
self._transport._config.batch_size or self.DEFAULT_MULTI_REQUEST_BATCH_SIZE
)
self._redact_data = True
self._method_missing_logged = False
def get_smart_request(self, method: str, params: dict | None = None) -> str:
"""Get a request message as a string."""
@ -178,6 +186,7 @@ class SmartProtocol(BaseProtocol):
multi_requests = [
{"method": method, "params": params} if params else {"method": method}
for method, params in requests.items()
if method not in FORCE_SINGLE_REQUEST
]
end = len(multi_requests)
@ -246,7 +255,20 @@ class SmartProtocol(BaseProtocol):
responses = response_step["result"]["responses"]
for response in responses:
method = response["method"]
# some smartcam devices calls do not populate the method key
# these should be defined in DO_NOT_SEND_AS_MULTI_REQUEST.
if not (method := response.get("method")):
if not self._method_missing_logged:
# Avoid spamming the logs
self._method_missing_logged = True
_LOGGER.error(
"No method key in response for %s, skipping: %s",
self._host,
response_step,
)
# These will end up being queried individually
continue
self._handle_response_error_code(
response, method, raise_on_error=raise_on_error
)
@ -255,7 +277,9 @@ class SmartProtocol(BaseProtocol):
result, method, retry_count=retry_count
)
multi_result[method] = result
# Multi requests don't continue after errors so requery any missing
# Multi requests don't continue after errors so requery any missing.
# Will also query individually any DO_NOT_SEND_AS_MULTI_REQUEST.
for method, params in requests.items():
if method not in multi_result:
resp = await self._transport.send(

View File

@ -34,6 +34,7 @@ class FakeSmartCamTransport(BaseTransport):
list_return_size=10,
is_child=False,
verbatim=False,
components_not_included=False,
):
super().__init__(
config=DeviceConfig(
@ -59,12 +60,16 @@ class FakeSmartCamTransport(BaseTransport):
# self.child_protocols = self._get_child_protocols()
self.list_return_size = list_return_size
self.components = {
comp["name"]: comp["version"]
for comp in self.info["getAppComponentList"]["app_component"][
"app_component_list"
]
}
# Setting this flag allows tests to create dummy transports without
# full fixture info for testing specific cases like list handling etc
self.components_not_included = (components_not_included,)
if not components_not_included:
self.components = {
comp["name"]: comp["version"]
for comp in self.info["getAppComponentList"]["app_component"][
"app_component_list"
]
}
@property
def default_port(self):

View File

@ -2,6 +2,7 @@ import logging
import pytest
import pytest_mock
from pytest_mock import MockerFixture
from kasa.exceptions import (
SMART_RETRYABLE_ERRORS,
@ -14,6 +15,7 @@ from kasa.smart import SmartDevice
from ..conftest import device_smart
from ..fakeprotocol_smart import FakeSmartTransport
from ..fakeprotocol_smartcam import FakeSmartCamTransport
DUMMY_QUERY = {"foobar": {"foo": "bar", "bar": "foo"}}
DUMMY_MULTIPLE_QUERY = {
@ -448,3 +450,81 @@ async def test_smart_queries_redaction(
await dev.update()
assert device_id not in caplog.text
assert "REDACTED_" + device_id[9::] in caplog.text
async def test_no_method_returned_multiple(
mocker: MockerFixture, caplog: pytest.LogCaptureFixture
):
"""Test protocol handles multiple requests that don't return the method."""
req = {
"getDeviceInfo": {"device_info": {"name": ["basic_info", "info"]}},
"getAppComponentList": {"app_component": {"name": "app_component_list"}},
}
res = {
"result": {
"responses": [
{
"method": "getDeviceInfo",
"result": {
"device_info": {
"basic_info": {
"device_model": "C210",
},
}
},
"error_code": 0,
},
{
"result": {"app_component": {"app_component_list": []}},
"error_code": 0,
},
]
},
"error_code": 0,
}
transport = FakeSmartCamTransport(
{},
"dummy-name",
components_not_included=True,
)
protocol = SmartProtocol(transport=transport)
mocker.patch.object(protocol._transport, "send", return_value=res)
await protocol.query(req)
assert "No method key in response" in caplog.text
caplog.clear()
await protocol.query(req)
assert "No method key in response" not in caplog.text
async def test_no_multiple_methods(
mocker: MockerFixture, caplog: pytest.LogCaptureFixture
):
"""Test protocol sends NO_MULTI methods as single call."""
req = {
"getDeviceInfo": {"device_info": {"name": ["basic_info", "info"]}},
"getConnectStatus": {"onboarding": {"get_connect_status": {}}},
}
info = {
"getDeviceInfo": {
"device_info": {
"basic_info": {
"avatar": "Home",
}
}
},
"getConnectStatus": {
"onboarding": {
"get_connect_status": {"current_ssid": "", "err_code": 0, "status": 0}
}
},
}
transport = FakeSmartCamTransport(
info,
"dummy-name",
components_not_included=True,
)
protocol = SmartProtocol(transport=transport)
send_spy = mocker.spy(protocol._transport, "send")
await protocol.query(req)
assert send_spy.call_count == 2