fix: preserve water-slide codes in cleaning-mode commands and status - #961
Open
xiaoyaner0201 wants to merge 1 commit into
Open
xiaoyaner0201 wants to merge 1 commit into
xiaoyaner0201 wants to merge 1 commit into
Conversation
Collaborator
|
Hi thanks for the fix! Can we take it a step further? Right now, this will still fail for say 223. Can we resolve the value via the range? When the user selects a value - we can set it to an exact value. But if the user changes it in the app to one of the inbetween slider values, we could still display the closest category. e.g. Add a small feature-gated range resolver Preserve the raw value 223. Resolve its range (221–223) to "slight". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs #931 and #930. Thanks to @liborplisek for identifying the enum-alias root cause and @BlackRockCity for the command/status captures.
Bring the combined cleaning-mode paths into line with the existing feature-aware water-mode mapping:
vac_and_mopandmopemit235instead of the aliased member's legacy202.WATER_SLIDE_MODE_MAPPINGwhen classifying cleaning mode. Keep the existing legacy-code fallback and direct enum-input behavior.PURE_WATER_FLOW_MIDDLE.codeon both sides: those tests were passing with the same incorrect202. Expected wire values now use explicit protocol integers.StatusTraitread/write regressions for all currently mapped slide intensities, vacuum/mop selection, optional mop-route fields, legacy codes, enum inputs, and incomplete/unknown status.Compatibility and scope
This deliberately keeps the public enum values and user-facing option strings unchanged, so existing consumers using
mode.valuedo not need a coordinated migration. It fixes the demonstrated combined-mode command/classification paths, not the standalone enum-alias representation:WaterModes.PURE_WATER_FLOW_MIDDLE.codeand genericWaterModes.from_code_optional(235)are still affected. A broader unique-value/display-name migration remains separate, so this PR references rather than closes #931.The water-flow range/label changes discussed in #801, other protocol families, Home Assistant automation policies, and login fixes are outside this PR.
Reproduction and hardware evidence
On a G30S Ultra with Home Assistant 2026.9.1 and python-roborock 7.2.3, a real status payload contained
fan_power=102, water_box_mode=235, mop_mode=303. Individual settings decoded as balanced/medium/deep_plus, while the combined mode wasNone(HAunknown). An automation waiting for confirmedvac_and_mopstopped before sending its start command.After applying the same feature-mapping read/write fix locally, the combined mode reported
vac_and_mop, the outgoing combined-mode payload used235, and the automation passed its setting check and started real cleaning. Direct device readback showed cleaning active, no error, and the expected motor settings; cleaning counters increased. The upstream change is tested independently against current main, not just the deployed library version.Validation
Base:
eac001c9d0a5eb45cd20e49033c37ff00054617f.202 != 235and modeNoneversusmop/vac_and_mop.uv run pytest tests/devices/traits/v1/test_status.py -q— 79 passed.uv run pre-commit run --all-files— passed, including Ruff, Mypy and Codespell.uv build— source distribution and wheel built successfully.tests/conformance/test_model_conformance.py::test_data_model_subclasses_roborock_base[roborock.data.b01_q10.b01_q10_containers.Q10RoborockPoint]. This pre-existing Q10 conformance failure is not changed, suppressed, or marked xfail by this PR.No device/account credentials, raw diagnostic dumps, or deployment scripts are included.