python-miio icon indicating copy to clipboard operation
python-miio copied to clipboard

Add support Mi Robot Vacuum-Mop 2 Pro (ijai.vacuum.v3)

Open k402xxxcenxxx opened this issue 3 years ago • 6 comments

Add support Mi Robot Vacuum-Mop 2 Pro (ijai.vacuum.v3)

Fixes #1388 fixes #1385

k402xxxcenxxx avatar Aug 14 '22 15:08 k402xxxcenxxx

Codecov Report

Merging #1497 (33d5905) into master (f4abd59) will increase coverage by 0.11%. The diff coverage is 89.52%.

@@            Coverage Diff             @@
##           master    #1497      +/-   ##
==========================================
+ Coverage   80.82%   80.93%   +0.11%     
==========================================
  Files         151      153       +2     
  Lines       14798    14989     +191     
  Branches     1696     3678    +1982     
==========================================
+ Hits        11960    12131     +171     
- Misses       2591     2610      +19     
- Partials      247      248       +1     
Impacted Files Coverage Δ
miio/integrations/vacuum/mijia/pro2vacuum.py 87.87% <87.87%> (ø)
miio/integrations/vacuum/mijia/__init__.py 100.00% <100.00%> (ø)
...integrations/vacuum/mijia/tests/test_pro2vacuum.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Aug 14 '22 15:08 codecov-commenter

Add support for basic actions:

  • Go Home
  • Start Cleaning
  • Stop Cleaning
  • Set fan speed
  • Get status

The code structure is based on miio\integrations\vacuum\mijia\g1vacuum.py.

Currently not sure the error code mapping, so leave the mapping empty

k402xxxcenxxx avatar Aug 26 '22 16:08 k402xxxcenxxx

Sorry, I haven't had time to review this yet, one question I had was whether the implementation differs that much from the g1 to require a separate implementation file?

It is not really a big issue, especially with the upcoming introspectable interfaces (see https://github.com/rytilahti/python-miio/issues/1495 and https://python-miio.readthedocs.io/en/latest/contributing.html#status-containers), but if the changes are minor we could avoid code duplication.

rytilahti avatar Sep 06 '22 23:09 rytilahti

Sorry, I haven't had time to review this yet, one question I had was whether the implementation differs that much from the g1 to require a separate implementation file?

It is not really a big issue, especially with the upcoming introspectable interfaces (see #1495 and https://python-miio.readthedocs.io/en/latest/contributing.html#status-containers), but if the changes are minor we could avoid code duplication.

That is a good question!

I have tried integrating ijai.vacuum.v3(pro2) into g1, but finally decided to create a new file. They have some difference:

  1. G1 has ChargingStatus but pro2 combines charging status with overall status.
  2. G1 has find action but pro2 doesn't
  3. G1 separates main/side brush to different SIID, but pro2 collects these states into sweeping state
  4. pro2 has more map controls, but G1 only has map info

These differences suggest that they may not have been designed by the same product line. So I decided to split them into two files. But this is the first time I've written this kind of script, so I'm not super sure that my choices are correct. If you still suggest me to integrate them into one file after reading my rationale, I will try to do so.

Thanks again for your easy-to-use codebase!

k402xxxcenxxx avatar Sep 08 '22 06:09 k402xxxcenxxx

I have tried integrating ijai.vacuum.v3(pro2) into g1, but finally decided to create a new file. They have some difference:

The reasoning sounds good to me, thanks for trying to integrate the implementations! No need to try to force different product lines under a single integration, especially considering the ongoing work to allow downstream users to programmatically find out about available information.

These differences suggest that they may not have been designed by the same product line. So I decided to split them into two files. But this is the first time I've written this kind of script, so I'm not super sure that my choices are correct. If you still suggest me to integrate them into one file after reading my rationale, I will try to do so.

Splitting the implementations sounds good to me. One of the reasons I have tried to "force" support for different devices has been to make the interfaces somewhat consistent. This was due to structural problems in this library that are now being worked on as mentioned in my other comment.

I'm glad that you think the code base was easy to work on, I hope it will become even easier in the future :-)

rytilahti avatar Sep 08 '22 21:09 rytilahti

@rytilahti thanks for reviewing. I will find time to improve it.

k402xxxcenxxx avatar Sep 14 '22 02:09 k402xxxcenxxx

@rytilahti Hi! When there will be a new release of python-miio to support die mop 2 pro? I'd like to implement my robot into homeassistant, but this will only works with a new release.

slecram avatar Dec 22 '23 15:12 slecram