From b4f2d0932297b47b6af2742a67071e57f72d56c4 Mon Sep 17 00:00:00 2001 From: Luke Lashley Date: Sun, 1 Nov 2026 18:29:05 -0500 Subject: [PATCH] fix(mower): document error semantics and log unknown status codes --- roborock/data/mower/mower_code_mappings.py | 4 ++ roborock/data/mower/mower_containers.py | 1 + tests/data/mower/test_mower_code_mappings.py | 49 ++++++++++++++++++++ tests/data/mower/test_mower_containers.py | 29 ++++++++++++ 4 files changed, 83 insertions(+) diff --git a/roborock/data/mower/mower_code_mappings.py b/roborock/data/mower/mower_code_mappings.py index 3dc907b6..26adbe9f 100644 --- a/roborock/data/mower/mower_code_mappings.py +++ b/roborock/data/mower/mower_code_mappings.py @@ -1,9 +1,12 @@ """Code mappings for Roborock mower devices.""" +import logging from typing import Self, cast from roborock.data.code_mappings import RoborockModeEnum +_LOGGER = logging.getLogger(__name__) + __all__ = [ "MowerAfsStatus", "MowerChargeState", @@ -38,6 +41,7 @@ def from_code(cls, code: int | str) -> Self: """Resolve a wire value by name, string value, or integer code.""" if (member := cls.from_any_optional(code)) is not None: return member + _LOGGER.debug("Unknown %s code %r; defaulting to UNKNOWN", cls.__name__, code) return cast(Self, cls.UNKNOWN) # type: ignore[attr-defined] diff --git a/roborock/data/mower/mower_containers.py b/roborock/data/mower/mower_containers.py index 3413bb39..461d7d41 100644 --- a/roborock/data/mower/mower_containers.py +++ b/roborock/data/mower/mower_containers.py @@ -48,6 +48,7 @@ class MowerStatus(RoborockBase): """Core mower status backed by mower DPS updates (data points 120-145).""" error_code: MowerErrorCode | None = field(default=None, metadata={"dps": RoborockMowerDataProtocol.ERROR_CODE}) + """None means no error code was supplied; UNKNOWN means an unknown error.""" battery: int | None = field(default=None, metadata={"dps": RoborockMowerDataProtocol.BATTERY}) mow_type: MowerMowType | None = field(default=None, metadata={"dps": RoborockMowerDataProtocol.MOW_TYPE}) mow_state: MowerDetailState | None = field(default=None, metadata={"dps": RoborockMowerDataProtocol.MOW_STATE}) diff --git a/tests/data/mower/test_mower_code_mappings.py b/tests/data/mower/test_mower_code_mappings.py index 4f36bfe2..f32a1c5e 100644 --- a/tests/data/mower/test_mower_code_mappings.py +++ b/tests/data/mower/test_mower_code_mappings.py @@ -1,5 +1,7 @@ """Tests for mower code mappings.""" +import logging + import pytest from roborock.data import mower @@ -127,3 +129,50 @@ def test_from_code_accepts_wire_forms_and_falls_back( enum_cls: type[MowerEnum], raw: int | str, expected: MowerEnum ) -> None: assert enum_cls.from_code(raw) is expected + + +@pytest.mark.parametrize("enum_cls", [MowerMowType, MowerErrorCode]) +@pytest.mark.parametrize("raw", [9999, "9999", "FUTURE_VALUE"]) +def test_unrecognized_mower_value_logs_debug( + enum_cls: type[MowerEnum], raw: int | str, caplog: pytest.LogCaptureFixture +) -> None: + with caplog.at_level(logging.DEBUG): + member = enum_cls.from_code(raw) + + assert member.value == "unknown" + assert len(caplog.records) == 1 + record = caplog.records[0] + assert record.levelno == logging.DEBUG + assert enum_cls.__name__ in record.getMessage() + assert repr(raw) in record.getMessage() + + +@pytest.mark.parametrize( + ("enum_cls", "raw"), + [ + (MowerMowType, 1), + (MowerMowType, "GLOBAL"), + (MowerMowType, "global"), + (MowerMowType, "1"), + (MowerMowType, -1), + (MowerErrorCode, 0), + (MowerErrorCode, "0"), + (MowerErrorCode, "UNKNOWN"), + (MowerErrorCode, "unknown"), + ], +) +def test_recognized_mower_value_does_not_log( + enum_cls: type[MowerEnum], raw: int | str, caplog: pytest.LogCaptureFixture +) -> None: + with caplog.at_level(logging.DEBUG): + enum_cls.from_code(raw) + + assert not caplog.records + + +def test_optional_mower_enum_lookups_remain_silent(caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level(logging.DEBUG): + assert MowerErrorCode.from_code_optional(9999) is None + assert MowerErrorCode.from_any_optional("FUTURE_VALUE") is None + + assert not caplog.records diff --git a/tests/data/mower/test_mower_containers.py b/tests/data/mower/test_mower_containers.py index 5cbbb943..1212d469 100644 --- a/tests/data/mower/test_mower_containers.py +++ b/tests/data/mower/test_mower_containers.py @@ -113,3 +113,32 @@ def test_mower_status_from_empty_dps(dps: dict[int | str, object] | None) -> Non def test_mower_status_from_dps_unknown_state() -> None: assert MowerStatus.from_dps({"123": 9999}).mow_state is MowerDetailState.UNKNOWN + + +@pytest.mark.parametrize("key", ["error_code", "errorCode", 120, "120"]) +@pytest.mark.parametrize( + ("raw", "expected"), + [ + (None, None), + (0, MowerErrorCode.UNKNOWN), + ("0", MowerErrorCode.UNKNOWN), + ("UNKNOWN", MowerErrorCode.UNKNOWN), + (18, MowerErrorCode.LIFT), + ("LIFT", MowerErrorCode.LIFT), + (9999, MowerErrorCode.UNKNOWN), + ], +) +def test_mower_status_error_semantics(key: int | str, raw: int | str | None, expected: MowerErrorCode | None) -> None: + """A missing code and an unknown error remain distinct through serialization.""" + if key in ("error_code", "errorCode"): + status = MowerStatus.from_dict({str(key): raw}) + else: + status = MowerStatus.from_dps({key: raw}) + + assert status.error_code is expected + serialized = status.as_dict() + if expected is None: + assert "errorCode" not in serialized + else: + assert serialized["errorCode"] == expected.value + assert MowerStatus.from_dict(serialized).error_code is expected