From 7d24db2d2253bb375d432f497e5403a2471601d2 Mon Sep 17 00:00:00 2001 From: Rohit Patil Date: Mon, 19 Oct 2026 12:40:27 +0530 Subject: [PATCH] refactor: give water slide modes their own enum values Water slide members of WaterModes reused the value strings of the legacy modes ("low", "medium", "high", "extreme"), so StrEnum collapsed them into aliases and dropped their codes (225, 235, 245, 250). Give each a unique value and keep the shared user-facing name in display_name, like CleanRoutes.DEEP_PLUS_CN. This removes the workarounds that existed only to preserve those codes: the WATER_SLIDE_MODE_MAPPING side table, the reverse lookup in get_cleaning_mode_parameters, and the inline patch in get_current_cleaning_mode (now _resolve_water_mode). Also add a conformance check that fails when RoborockModeEnum members alias each other by value. B01Fault already has such aliases and is tracked in a baseline. Fixes #987 --- roborock/data/v1/v1_clean_modes.py | 63 +++++++++++----------- tests/conformance/test_enum_conformance.py | 26 +++++++++ tests/devices/traits/v1/test_status.py | 18 ++++++- 3 files changed, 73 insertions(+), 34 deletions(-) diff --git a/roborock/data/v1/v1_clean_modes.py b/roborock/data/v1/v1_clean_modes.py index 1799e96c9..7d1e1fe05 100644 --- a/roborock/data/v1/v1_clean_modes.py +++ b/roborock/data/v1/v1_clean_modes.py @@ -57,12 +57,12 @@ class WaterModes(RoborockModeEnum): SMART_MODE = ("smart_mode", 209) SMART_WATER = ("smart_water", 210) PURE_WATER_FLOW_START = ("slight", 221) - PURE_WATER_FLOW_SMALL = ("low", 225) + PURE_WATER_FLOW_SMALL = ("pure_water_flow_small", 225, "low") PURE_WATER_FLOW_GENTLE = ("gentle", 230) - PURE_WATER_FLOW_MIDDLE = ("medium", 235) + PURE_WATER_FLOW_MIDDLE = ("pure_water_flow_middle", 235, "medium") PURE_WATER_FLOW_LARGE = ("moderate", 240) - PURE_WATER_SUPER_BEGIN = ("high", 245) - PURE_WATER_FLOW_END = ("extreme", 250) + PURE_WATER_SUPER_BEGIN = ("pure_water_super_begin", 245, "high") + PURE_WATER_FLOW_END = ("pure_water_flow_end", 250, "extreme") class WashTowelModes(RoborockModeEnum): @@ -89,16 +89,16 @@ class CleaningMode(StrEnum): SMART_MODE = "smart_mode" -WATER_SLIDE_MODE_MAPPING: dict[int, WaterModes] = { - 200: WaterModes.OFF, - 221: WaterModes.PURE_WATER_FLOW_START, - 225: WaterModes.PURE_WATER_FLOW_SMALL, - 230: WaterModes.PURE_WATER_FLOW_GENTLE, - 235: WaterModes.PURE_WATER_FLOW_MIDDLE, - 240: WaterModes.PURE_WATER_FLOW_LARGE, - 245: WaterModes.PURE_WATER_SUPER_BEGIN, - 250: WaterModes.PURE_WATER_FLOW_END, -} +_WATER_SLIDE_MODES: tuple[WaterModes, ...] = ( + WaterModes.OFF, + WaterModes.PURE_WATER_FLOW_START, + WaterModes.PURE_WATER_FLOW_SMALL, + WaterModes.PURE_WATER_FLOW_GENTLE, + WaterModes.PURE_WATER_FLOW_MIDDLE, + WaterModes.PURE_WATER_FLOW_LARGE, + WaterModes.PURE_WATER_SUPER_BEGIN, + WaterModes.PURE_WATER_FLOW_END, +) # Water slide devices report the raw slider position (1-30) as 220 + position. # The app groups positions into named ranges, keyed here by their last position. @@ -177,7 +177,7 @@ def get_water_modes(features: DeviceFeatures) -> list[WaterModes]: """Get the valid water modes for the device - also known as 'water flow' or 'water level'""" # Water slide mode supports a separate set of water flow codes. if features.is_water_slide_mode_supported: - return list(WATER_SLIDE_MODE_MAPPING.values()) + return list(_WATER_SLIDE_MODES) supported_modes = [WaterModes.OFF] if features.is_mop_shake_module_supported: @@ -201,7 +201,7 @@ def get_water_modes(features: DeviceFeatures) -> list[WaterModes]: def get_water_slide_mode_name(code: int) -> str | None: """Get the app's range name for a water slide code, or None if out of range.""" mode = _resolve_water_slide_mode(code) - return mode.value if mode is not None else None + return mode.display_name if mode is not None else None def _resolve_water_slide_mode(code: int) -> WaterModes | None: @@ -222,13 +222,10 @@ def _resolve_water_slide_mode(code: int) -> WaterModes | None: def get_water_mode_mapping(features: DeviceFeatures) -> dict[int, str]: """Get water mode mapping by supported feature set. - WaterModes contains aliases for multiple codes that share the same value - string (e.g. low can be 201 or 225). For water slide mode devices we need - explicit code mapping to preserve those slide-specific codes. + Water slide modes share user-facing names with legacy modes (e.g. "low" is + 201 or 225), so the mapping uses display_name rather than the enum value. """ - if features.is_water_slide_mode_supported: - return {code: mode.value for code, mode in WATER_SLIDE_MODE_MAPPING.items()} - return {mode.code: mode.value for mode in get_water_modes(features)} + return {mode.code: mode.display_name for mode in get_water_modes(features)} def get_cleaning_mode_options(features: DeviceFeatures) -> list[CleaningMode]: @@ -309,14 +306,7 @@ def get_cleaning_mode_parameters(cleaning_mode: CleaningMode, features: DeviceFe raise RoborockUnsupportedFeature(f"Cleaning mode {cleaning_mode.value!r} is not supported") fan_power, water_box_mode, mop_mode = _get_clean_motor_mode_params(cleaning_mode, features) - water_code = water_box_mode.code - if features.is_water_slide_mode_supported: - # Aliased water enums retain legacy codes; the device mapping preserves - # slide-specific wire values without changing the user-facing names. - water_code = next( - code for code, name in get_water_mode_mapping(features).items() if name == water_box_mode.value - ) - params: dict[str, int] = {"fan_power": fan_power.code, "water_box_mode": water_code} + params: dict[str, int] = {"fan_power": fan_power.code, "water_box_mode": water_box_mode.code} if features.is_clean_route_setting_supported: params["mop_mode"] = mop_mode.code return [params] @@ -342,6 +332,15 @@ def _resolve_clean_mode(value: int | VacuumModes | None, features: DeviceFeature return VacuumModes.from_code_optional(value) +def _resolve_water_mode(value: int | WaterModes | None, features: DeviceFeatures) -> WaterModes | None: + """Resolve a water mode code, mapping raw slider positions on water slide devices.""" + if value is None or isinstance(value, WaterModes): + return value + if features.is_water_slide_mode_supported: + return _resolve_water_slide_mode(value) or WaterModes.from_code_optional(value) + return WaterModes.from_code_optional(value) + + def get_current_cleaning_mode( clean_mode: int | VacuumModes | None, water_mode: int | WaterModes | None, @@ -352,9 +351,7 @@ def get_current_cleaning_mode( if not features.is_support_water_mode: return None clean_mode_enum = _resolve_clean_mode(clean_mode, features) - water_mode_enum = _resolve_mode_code(water_mode, WaterModes) - if features.is_water_slide_mode_supported and isinstance(water_mode, int): - water_mode_enum = _resolve_water_slide_mode(water_mode) or water_mode_enum + water_mode_enum = _resolve_water_mode(water_mode, features) mop_mode_enum = _resolve_mode_code(mop_mode, CleanRoutes) if clean_mode_enum is None or water_mode_enum is None: return None diff --git a/tests/conformance/test_enum_conformance.py b/tests/conformance/test_enum_conformance.py index a22eb23b7..ea9549957 100644 --- a/tests/conformance/test_enum_conformance.py +++ b/tests/conformance/test_enum_conformance.py @@ -136,6 +136,32 @@ def test_roborock_mode_enum_handles_unknown_code(mode_enum_cls: type[RoborockMod assert mode_enum_cls.from_code_optional(sentinel) is None +# Legacy RoborockModeEnum classes whose members still share value strings. +# As these enums are remediated, remove them from this baseline. +KNOWN_MODE_ENUMS_WITH_VALUE_ALIASES = { + "roborock.data.b01_q7.b01_q7_code_mappings.B01Fault", +} + +_VALUE_ALIAS_XFAIL_MARKS = { + fqn: pytest.mark.xfail(reason=f"{fqn} has members aliased by value", strict=True) + for fqn in KNOWN_MODE_ENUMS_WITH_VALUE_ALIASES +} + + +@pytest.mark.parametrize("mode_enum_cls", to_pytest_params(_ALL_MODE_ENUMS, marks_by_fqn=_VALUE_ALIAS_XFAIL_MARKS)) +def test_roborock_mode_enum_has_no_value_aliases(mode_enum_cls: type[RoborockModeEnum]) -> None: + """Members sharing a value string collapse into one member and lose their own code. + + Modes that need the same user-facing name should use a distinct value and + pass the shared name as display_name instead. + """ + aliases = {name: member.name for name, member in mode_enum_cls.__members__.items() if name != member.name} + assert not aliases, ( + f"{mode_enum_cls.__module__}.{mode_enum_cls.__name__} has members aliased by value: {aliases}. " + "Give each member a unique value and use display_name for the shared name." + ) + + @pytest.mark.parametrize("enum_cls", to_pytest_params(_ALL_CODE_MAPPING_ENUMS)) def test_code_mapping_enums_inherit_resilient_bases(enum_cls: type[enum.Enum]) -> None: """Enums in wire code mapping modules must inherit from RoborockEnum or RoborockModeEnum.""" diff --git a/tests/devices/traits/v1/test_status.py b/tests/devices/traits/v1/test_status.py index 0f08a0b08..b30b24ad8 100644 --- a/tests/devices/traits/v1/test_status.py +++ b/tests/devices/traits/v1/test_status.py @@ -695,6 +695,22 @@ def test_current_cleaning_mode_water_code_fallback( assert status_trait.current_cleaning_mode == expected_mode +@pytest.mark.parametrize( + ("code", "expected_mode", "display_name"), + [ + (225, WaterModes.PURE_WATER_FLOW_SMALL, "low"), + (235, WaterModes.PURE_WATER_FLOW_MIDDLE, "medium"), + (245, WaterModes.PURE_WATER_SUPER_BEGIN, "high"), + (250, WaterModes.PURE_WATER_FLOW_END, "extreme"), + ], +) +def test_water_slide_modes_keep_their_own_codes(code: int, expected_mode: WaterModes, display_name: str) -> None: + """Water slide modes are distinct from the legacy modes that share their names.""" + assert WaterModes.from_code_optional(code) is expected_mode + assert expected_mode.code == code + assert expected_mode.display_name == display_name + + @pytest.mark.parametrize("water_mode", [WaterModes.MEDIUM, WaterModes.PURE_WATER_FLOW_MIDDLE]) def test_current_cleaning_mode_water_slide_accepts_enums(water_mode: WaterModes) -> None: """Enum inputs remain valid even when they alias a legacy water mode.""" @@ -778,7 +794,7 @@ def test_water_slide_mode_mapping() -> None: 245: "high", 250: "extreme", } - assert [mode.value for mode in status_trait.water_mode_options] == [ + assert [mode.display_name for mode in status_trait.water_mode_options] == [ "off", "slight", "low",