From 372677e14001446698f417a3ab75700f45a44581 Mon Sep 17 00:00:00 2001 From: Swastik Tawate Date: Sun, 18 Oct 2026 14:23:45 +0530 Subject: [PATCH] Refactor WaterModes to use display_name instead of value aliases --- roborock/data/v1/v1_clean_modes.py | 44 ++++++++++------------ tests/conformance/test_enum_conformance.py | 34 +++++++++++++++++ tests/devices/traits/v1/test_status.py | 4 +- 3 files changed, 55 insertions(+), 27 deletions(-) diff --git a/roborock/data/v1/v1_clean_modes.py b/roborock/data/v1/v1_clean_modes.py index 1799e96c9..fe12b37c9 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): @@ -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: @@ -220,15 +220,8 @@ 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. - """ - 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)} + """Get water mode mapping by supported feature set.""" + return {mode.code: mode.display_name for mode in get_water_modes(features)} def get_cleaning_mode_options(features: DeviceFeatures) -> list[CleaningMode]: @@ -309,14 +302,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 +328,16 @@ 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, accounting for water-slide range codes.""" + if value is None or isinstance(value, WaterModes): + return value + mode = WaterModes.from_code_optional(value) + if features.is_water_slide_mode_supported: + mode = _resolve_water_slide_mode(value) or mode + return mode + + def get_current_cleaning_mode( clean_mode: int | VacuumModes | None, water_mode: int | WaterModes | None, @@ -352,9 +348,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..3767af15c 100644 --- a/tests/conformance/test_enum_conformance.py +++ b/tests/conformance/test_enum_conformance.py @@ -161,3 +161,37 @@ def test_known_missing_unknown_baseline_inventory() -> None: f"Enums in KNOWN_ENUMS_MISSING_UNKNOWN are no longer missing 'unknown': {stale_in_baseline}. " "Please remove them from KNOWN_ENUMS_MISSING_UNKNOWN." ) + + +# Baseline inventory of RoborockModeEnum subclasses that intentionally define +# multiple names sharing one value string (e.g. fault descriptions shared +# across several firmware codes). These enums accept the silent aliasing +# behavior; every other mode enum must give each member a distinct value +# (using display_name for the shared user-facing name) so no code is lost. +KNOWN_MODE_ENUMS_WITH_VALUE_ALIASES = { + "roborock.data.b01_q7.b01_q7_code_mappings.B01Fault", +} + + +@pytest.mark.parametrize("mode_enum_cls", to_pytest_params(_ALL_MODE_ENUMS)) +def test_roborock_mode_enum_no_silent_value_aliases(mode_enum_cls: type[RoborockModeEnum]) -> None: + """RoborockModeEnum subclasses must not define silent value aliases. + + StrEnum collapses members sharing a value string into one, silently + discarding the other member's code. Give each member a distinct value and + use display_name for the shared user-facing name instead. + """ + fqn = f"{mode_enum_cls.__module__}.{mode_enum_cls.__name__}" + if fqn in KNOWN_MODE_ENUMS_WITH_VALUE_ALIASES: + return + seen: set[int] = set() + aliases = [] + for name, member in mode_enum_cls.__members__.items(): + if id(member) in seen: + aliases.append(name) + seen.add(id(member)) + assert not aliases, ( + f"{fqn} defines silent value aliases: {aliases}. " + "Members sharing a value string collapse into one and lose their codes; " + "use display_name for the shared user-facing name instead." + ) diff --git a/tests/devices/traits/v1/test_status.py b/tests/devices/traits/v1/test_status.py index 0f08a0b08..bf5b936e2 100644 --- a/tests/devices/traits/v1/test_status.py +++ b/tests/devices/traits/v1/test_status.py @@ -697,7 +697,7 @@ def test_current_cleaning_mode_water_code_fallback( @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.""" + """Enum inputs resolve to distinct members, each keeping its own wire code.""" status_trait = _create_cleaning_mode_status_trait(is_water_slide_mode_supported=True) assert ( get_current_cleaning_mode(102, water_mode, 303, status_trait._device_features_trait) == CleaningMode.VAC_AND_MOP @@ -778,7 +778,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",