Skip to content

Only log one warning per unknown clean error code and status #1462

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
sdb9696 merged 3 commits into from
Jan 22, 2025
Merged

Only log one warning per unknown clean error code and status #1462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcbf239
Add error 6: trapped
rytilahti Jan 16, 2025
09dcbf6
Merge branch 'master' into janitor/vacuum_errors
sdb9696 Jan 22, 2025
a81fb5a
Only log one warning per unknown clean error code and status
sdb9696 Jan 22, 2025
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
27 changes: 21 additions & 6 deletions kasa/smart/modules/clean.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class ErrorCode(IntEnum):
SideBrushStuck = 2
MainBrushStuck = 3
WheelBlocked = 4
Trapped = 6
DustBinRemoved = 14
UnableToMove = 15
LidarBlocked = 16
Expand Down Expand Up @@ -79,6 +80,8 @@ class Clean(SmartModule):

REQUIRED_COMPONENT = "clean"
_error_code = ErrorCode.Ok
_logged_error_code_warnings: set | None = None
_logged_status_code_warnings: set

def _initialize_features(self) -> None:
"""Initialize features."""
Expand Down Expand Up @@ -229,12 +232,17 @@ def _initialize_features(self) -> None:

async def _post_update_hook(self) -> None:
"""Set error code after update."""
if self._logged_error_code_warnings is None:
self._logged_error_code_warnings = set()
self._logged_status_code_warnings = set()

errors = self._vac_status.get("err_status")
if errors is None or not errors:
self._error_code = ErrorCode.Ok
return

if len(errors) > 1:
if len(errors) > 1 and "multiple" not in self._logged_error_code_warnings:
self._logged_error_code_warnings.add("multiple")
_LOGGER.warning(
"Multiple error codes, using the first one only: %s", errors
)
Expand All @@ -243,10 +251,13 @@ async def _post_update_hook(self) -> None:
try:
self._error_code = ErrorCode(error)
except ValueError:
_LOGGER.warning(
"Unknown error code, please create an issue describing the error: %s",
error,
)
if error not in self._logged_error_code_warnings:
self._logged_error_code_warnings.add(error)
_LOGGER.warning(
"Unknown error code, please create an issue "
"describing the error: %s",
error,
)
self._error_code = ErrorCode.UnknownInternal

def query(self) -> dict:
Expand Down Expand Up @@ -360,7 +371,11 @@ def status(self) -> Status:
try:
return Status(status_code)
except ValueError:
_LOGGER.warning("Got unknown status code: %s (%s)", status_code, self.data)
if status_code not in self._logged_status_code_warnings:
self._logged_status_code_warnings.add(status_code)
_LOGGER.warning(
"Got unknown status code: %s (%s)", status_code, self.data
)
return Status.UnknownInternal

@property
Expand Down
56 changes: 49 additions & 7 deletions tests/smart/modules/test_clean.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,39 @@ async def test_actions(


@pytest.mark.parametrize(
("err_status", "error"),
("err_status", "error", "warning_msg"),
[
pytest.param([], ErrorCode.Ok, id="empty error"),
pytest.param([0], ErrorCode.Ok, id="no error"),
pytest.param([3], ErrorCode.MainBrushStuck, id="known error"),
pytest.param([123], ErrorCode.UnknownInternal, id="unknown error"),
pytest.param([3, 4], ErrorCode.MainBrushStuck, id="multi-error"),
pytest.param([], ErrorCode.Ok, None, id="empty error"),
pytest.param([0], ErrorCode.Ok, None, id="no error"),
pytest.param([3], ErrorCode.MainBrushStuck, None, id="known error"),
pytest.param(
[123],
ErrorCode.UnknownInternal,
"Unknown error code, please create an issue describing the error: 123",
id="unknown error",
),
pytest.param(
[3, 4],
ErrorCode.MainBrushStuck,
"Multiple error codes, using the first one only: [3, 4]",
id="multi-error",
),
],
)
@clean
async def test_post_update_hook(dev: SmartDevice, err_status: list, error: ErrorCode):
async def test_post_update_hook(
dev: SmartDevice,
err_status: list,
error: ErrorCode,
warning_msg: str | None,
caplog: pytest.LogCaptureFixture,
):
"""Test that post update hook sets error states correctly."""
clean = next(get_parent_and_child_modules(dev, Module.Clean))
assert clean

caplog.set_level(logging.DEBUG)

# _post_update_hook will pop an item off the status list so create a copy.
err_status = [e for e in err_status]
clean.data["getVacStatus"]["err_status"] = err_status
Expand All @@ -130,6 +148,16 @@ async def test_post_update_hook(dev: SmartDevice, err_status: list, error: Error
if error is not ErrorCode.Ok:
assert clean.status is Status.Error

if warning_msg:
assert warning_msg in caplog.text

# Check doesn't log twice
caplog.clear()
await clean._post_update_hook()

if warning_msg:
assert warning_msg not in caplog.text


@clean
async def test_resume(dev: SmartDevice, mocker: MockerFixture):
Expand Down Expand Up @@ -164,6 +192,20 @@ async def test_unknown_status(
assert clean.status is Status.UnknownInternal
assert "Got unknown status code: 123" in caplog.text

# Check only logs once
caplog.clear()

assert clean.status is Status.UnknownInternal
assert "Got unknown status code: 123" not in caplog.text

# Check logs again for other errors

caplog.clear()
clean.data["getVacStatus"]["status"] = 123456

assert clean.status is Status.UnknownInternal
assert "Got unknown status code: 123456" in caplog.text


@clean
@pytest.mark.parametrize(
Expand Down
Loading