From bcbf239e3078bf5c41fecf0090b8ca7122911987 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Thu, 16 Jan 2025 19:46:12 +0100 Subject: [PATCH 1/2] Add error 6: trapped --- kasa/smart/modules/clean.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kasa/smart/modules/clean.py b/kasa/smart/modules/clean.py index a2812c329..654fbd90c 100644 --- a/kasa/smart/modules/clean.py +++ b/kasa/smart/modules/clean.py @@ -37,6 +37,7 @@ class ErrorCode(IntEnum): SideBrushStuck = 2 MainBrushStuck = 3 WheelBlocked = 4 + Trapped = 6 DustBinRemoved = 14 UnableToMove = 15 LidarBlocked = 16 From a81fb5a4eaa3d278a2bb7068399ce491f60a6e41 Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:53:51 +0000 Subject: [PATCH 2/2] Only log one warning per unknown clean error code and status --- kasa/smart/modules/clean.py | 26 ++++++++++---- tests/smart/modules/test_clean.py | 56 +++++++++++++++++++++++++++---- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/kasa/smart/modules/clean.py b/kasa/smart/modules/clean.py index 654fbd90c..2764e8a15 100644 --- a/kasa/smart/modules/clean.py +++ b/kasa/smart/modules/clean.py @@ -80,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.""" @@ -230,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 ) @@ -244,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: @@ -361,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 diff --git a/tests/smart/modules/test_clean.py b/tests/smart/modules/test_clean.py index 70cbcb158..f4c2813c4 100644 --- a/tests/smart/modules/test_clean.py +++ b/tests/smart/modules/test_clean.py @@ -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 @@ -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): @@ -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(