Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[miio] Add Roborock S7/S7MaxV mop mode #16608

Merged
merged 3 commits into from Apr 5, 2024

Conversation

rsopp
Copy link
Contributor

@rsopp rsopp commented Apr 3, 2024

This PR adds the mop mode feature found in the Roborock S7 vacuum cleaners.
Closes #16607

@rsopp rsopp requested a review from marcelrv as a code owner April 3, 2024 15:20
This adds the mop mode feature found in the Roborock S7 vacuum cleaners.

Signed-off-by: Ruediger Sopp <ruediger.sopp@gmail.com>
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to have a full list of possible mop modes? These could be added as option to the channel-type, making translation / UI-support possible.

@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Apr 3, 2024
@marcelrv
Copy link
Contributor

marcelrv commented Apr 3, 2024

Thanks for your contribution!

Indeed suggest to add the options , so in the UI you don't see the numbers like 300 but indeed a proper description.
example here: https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.miio/src/main/resources/OH-INF/thing/vacuumThing.xml#L312-L326

Likewise, I think the option is still missing in the translation file:

https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.miio/src/main/resources/OH-INF/i18n/miio.properties

Signed-off-by: Ruediger Sopp <ruediger.sopp@gmail.com>
Signed-off-by: Ruediger Sopp <ruediger.sopp@gmail.com>
@rsopp rsopp requested a review from lsiepel April 3, 2024 22:21
@rsopp
Copy link
Contributor Author

rsopp commented Apr 3, 2024

Thanks for the suggestions! I just added the options for the mop mode.

Maybe, it would be a good idea to also add options for the other modes of the vaccum (fan power, water box mode) in another PR?

@marcelrv
Copy link
Contributor

marcelrv commented Apr 4, 2024

Thanks for the suggestions! I just added the options for the mop mode.

Maybe, it would be a good idea to also add options for the other modes of the vaccum (fan power, water box mode) in another PR?

yes, it it, at least for the water box mode..
If I recall right, for the fan power I declined to do it as different models have different meaning for the numeric values.
The best way to solve that would be to create the options dynamic based on the model... but I never considered it that important to add that complexity.

@marcelrv
Copy link
Contributor

marcelrv commented Apr 4, 2024

lgtm

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@lsiepel lsiepel merged commit 23502fc into openhab:main Apr 5, 2024
3 checks passed
@lsiepel lsiepel added this to the 4.2 milestone Apr 5, 2024
@rsopp rsopp deleted the miio-roborock-mop-mode branch April 5, 2024 20:47
lo92fr pushed a commit to lo92fr/openhab-addons that referenced this pull request Apr 30, 2024
* [miio] Add Roborock S7/S7MaxV mop mode
Signed-off-by: Ruediger Sopp <ruediger.sopp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[miio] Roborock S7/S7MaxV: add support for the MopMode feature
3 participants