Add support for Mi AirPurifier 3 by foxel · Pull Request #31729 · home-assistant/core

foxel · 2020-02-11T16:08:50Z

Proposed change

Adding support for Mi AirPurifier 3

Type of change

Adding support for Mi AirPurifier 3 using miot. For now this PR uses a fork of xiaomi-miio which implements miot devices support

Should help with #26957

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

fan:
  - platform: xiaomi_miio
    name: mi_purifier_1
    host: !secret purifier3_ip
    token: !secret purifier3_token
    model: zhimi.airpurifier.ma4

Additional information

need to be continued after rytilahti/python-miio#634 (or rytilahti/python-miio#585) is merged. I've tested with this change ant that code of miio - works fine for now

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Added Xiaomi Air Purifier 3/3H (2019) home-assistant.io#12653

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

probot-home-assistant · 2020-02-11T16:08:59Z

Hey there @rytilahti, @syssi, mind taking a look at this pull request as its been labeled with a integration (xiaomi_miio) you are listed as a codeowner for? Thanks!

Kane610 · 2020-02-13T06:52:02Z

-  "requirements": ["construct==2.9.45", "python-miio==0.4.8"],
+  "requirements": [
+    "construct==2.9.45",
+    "https://github.com/foxel/python-miio/archive/air-purifier-3h-support.zip#python-miio==0.5.0"


This is not allowed. The package must be published to pypi.

Sorry didn't see it was a draft.

pkrolkgp

Please also add support for 3H model

foxel · 2020-02-19T02:20:11Z

@pkrolkgp thanks for suggestions. I don't have 3H but believe it's the same as 3...

PawelKwiecinski · 2020-02-19T07:00:45Z

@pkrolkgp thanks for suggestions. I don't have 3H but believe it's the same as 3...

I do own a 3H and I'm able to test for you if needed. AFAIK the only difference is the filter type that ships with the unit. All behavior and API should be identical.

Enabled setting fan level for AirPurifier 3/3H

homeassistant · 2020-02-20T02:35:44Z

Hi @pkrolkgp,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Flagun · 2020-03-04T17:15:09Z

hi guys can you please add model "zhimi.airpurifier.mb3" as well?

pkrolkgp · 2020-03-04T19:20:47Z

hi guys can you please add model "zhimi.airpurifier.mb3" as well?

it is added in second commit

rytilahti · 2020-03-05T12:38:16Z


 # Air Humidifier CA
-ATTR_MOTOR_SPEED = "motor_speed"
+# ATTR_MOTOR_SPEED = "motor_speed"


Why change this?

Duplicates ATTR_MOTOR_SPEED on line 139. Didn't drop this entirely just to keep the notion of this attribute in this "Air Humidifier CA" section

rytilahti · 2020-03-05T12:40:27Z

        if self._state:
-            return AirpurifierOperationMode(self._state_attrs[ATTR_MODE]).name
+            if isinstance(self._device, AirPurifierMiot):
+                return AirpurifierMiotOperationMode(self._state_attrs[ATTR_MODE]).name


Would it make more sense to save a reference to the mode as given by the library and simply use _speed.name? Usage of isinstance is generally speaking not a good thing.

I've changed this to get rid of runtime instance checking

rytilahti · 2020-03-05T12:41:03Z

            "Setting operation mode of the miio device failed.",
            self._device.set_mode,
-            AirpurifierOperationMode[speed.title()],
+            AirpurifierMiotOperationMode[speed.title()]


We need to find a better solution for this, too.

pawelkw · 2020-03-13T17:22:07Z

Do you need help with this @foxel ? Would be great to finally have it supported. I have the 3H btw.

foxel · 2020-03-14T00:46:03Z

@pawelkw this PR is locked with work on rytilahti/python-miio#634. One that merged, this PR is ready to merge

bbiegun · 2020-03-15T21:06:38Z

@foxel rytilahti/python-miio#634 is merged! :)

rytilahti · 2020-03-15T22:01:29Z

I'm waiting to get some feedback on rytilahti/python-miio#643 – if all goes well, I'll prepare a new release soon and we can finally get this merged 🎉

CLARENNE-Q · 2020-03-26T21:45:20Z

Any news about this beautiful integration ?

rytilahti · 2020-03-28T21:54:05Z

rytilahti/python-miio#658 release PR is now there, so I hope we'll have python-miio 0.5.0 released very soon!

bbiegun · 2020-03-29T13:58:52Z

@rytilahti , you've just released 050! :)
drums...

rytilahti · 2020-03-29T14:00:55Z

Indeed! 🎉 I adjusted #32850 to bump the requirement, @foxel, this PR can also proceed now :-)

nhok3296 · 2020-03-31T04:47:39Z

Already able to use this integration yet? Thank you

foxel · 2020-03-31T07:21:03Z

@nhok3296 I'm using this in my installation now. But it's not merged yet.

nhok3296 · 2020-03-31T10:14:26Z

@foxel I tried xiaomi_miio but it didn't work

rytilahti · 2020-03-31T13:29:27Z

-  "requirements": ["construct==2.9.45", "python-miio==0.4.8"],
+  "requirements": [
+    "construct==2.9.45",
+    "python-miio==0.5.0"


Please bump this to 0.5.0.1, as that bump will be needed soon enough due to a problem with packaging of 0.5.0.

Otherwise this seems to be good to go from my side.

rytilahti

The CI checks need to be fixed before this can be merged, check out also my single comment wrt. naming.

rytilahti · 2020-04-02T22:49:20Z

            self._device_features = FEATURE_FLAGS_AIRPURIFIER_2S
            self._available_attributes = AVAILABLE_ATTRIBUTES_AIRPURIFIER_2S
            self._speed_list = OPERATION_MODES_AIRPURIFIER_2S
+        elif self._model == MODEL_AIRPURIFIER_3 or self._model == MODEL_AIRPURIFIER_3H:


I was rechecking this PR and saw these and wondered why the one below is not merged here, before I realized that one is _3 and one _V3. Do you think it would make sense to add MIOT somewhere in there to make it easier to differentiate?

I didn't ever see V3 version but it's something older and different from 3/3h from 2019...
I don't think adding MIOT would be the best solution. I think in not so distant future we'll need to have 2/2s/Pro support with MiOT protocol...

Or simply use some part of the real model number? It's mildly confusing to differentiate between those constants. Anyway, this is not a blocking issue.

One last thing to do before merging, please update the documentation (for the model listing & the new service): https://github.com/home-assistant/home-assistant.io/blob/current/source/_integrations/fan.xiaomi_miio.markdown

Thanks, let's get this merged, the docs will follow as soon as someone reviews it! Thanks for your hard work and patience on this! 🥇

MartinHjelmare

This platform should be refactored to follow our design guidelines before any further changes are done.

MartinHjelmare · 2020-04-03T18:29:41Z

 OPERATION_MODES_AIRPURIFIER_PRO = ["Auto", "Silent", "Favorite"]
 OPERATION_MODES_AIRPURIFIER_PRO_V7 = OPERATION_MODES_AIRPURIFIER_PRO
 OPERATION_MODES_AIRPURIFIER_2S = ["Auto", "Silent", "Favorite"]
+OPERATION_MODES_AIRPURIFIER_3 = ["Auto", "Silent", "Favorite", "Fan"]


The only allowed speed modes for fans are off, low, medium and high.

core/homeassistant/components/fan/__init__.py

Lines 35 to 38 in dc7127a

SPEED_OFF = "off"

SPEED_LOW = "low"

SPEED_MEDIUM = "medium"

SPEED_HIGH = "high"

We should not continue to extend this platform with architecture design breaking changes.

Sorry, I didn't notice that... How can we make the situation better? Should the tests be improved to capture such misuses?

I'm not sure how we can test this generally. Some platforms are already breaking the rules which might also complicate any general tests.

@MartinHjelmare what if a device does not fall into such a specification?

The platform can try to translate between home assistant mode and device mode. The platform can also register custom services.

The current design does not cater well to all fans but until we have a better design we have to follow it. We have an open architecture issue on fan speeds. You're welcome to contribute to that and try to bring it forward. We need to figure out a design that can cater for a majority of fans but is still compatible with our frontend and voice assistants.

MartinHjelmare · 2020-04-03T18:34:17Z

+    ATTR_HUMIDITY: "humidity",
+    ATTR_AIR_QUALITY_INDEX: "aqi",
+    ATTR_MODE: "mode",
+    ATTR_FILTER_HOURS_USED: "filter_hours_used",


Relative times are not allowed in the state machine. We only allow absolute utc timestamps.

Times should preferably be represented as sensors with device class timestamp.

https://developers.home-assistant.io/docs/entity_sensor#available-device-classes

So, should this be completely removed then? The problem with this value is, I presume, that there is no specific timestamp as it depends on the usage. ping @foxel - do you have an idea on this?

P.S. I think we need to have a checklist for reviewers, much like what we have for PR creators, to list things to check. It is sometimes hard to spot all mistakes, and I'm very sorry for not catching this one..

We should update this dev docs page with the allowed fan speeds:
https://developers.home-assistant.io/docs/entity_fan

I'll put it on my todo.

In general we need to make sure that we follow our architecture docs and ADRs. But those are not complete yet, so that's a problem.

It's interesting that relative timestamps are not allowed in state attributes. Didn't know that. Where can I find more docs about it?
I have MQTT devices, Esphome which are breaking this rule. This cannot be found in HA code as HA just puts received json in state_attrs.
my vacuum state attributes:

cleanTime: 202.8 cleanArea: 12530.2 cleanCount: 1349 last_run_stats: startTime: 1585934330000 endTime: 1585935335000 duration: 1005

@rytilahti solution for it is to move attributes to independent sensors.

CLARENNE-Q · 2020-04-09T00:06:36Z

Nothing about in https://www.home-assistant.io/blog/2020/04/08/release-108/ ?

Invalid config for [fan.xiaomi_miio]: value is not allowed for dictionary value @ data['model']. Got 'zhimi.airpurifier.ma4'. (See ?, line ?).

On homeassistant/qemux86-64-homeassistant with version 0.108.0b5

frenck · 2020-04-09T06:34:39Z

@CLARENNE-Q This PR is merged after the beta was cut. It is not in 0.108.

dj4shots · 2020-04-13T18:01:12Z

is this available now in 0.108.3 ?

frenck · 2020-04-14T06:53:12Z

@dj4shots No, new features are only shipped in major releases. This will be shipped with 0.109

Adding support for Mi AirPurifier 3

9cd4464

homeassistant added the cla-signed label Feb 11, 2020

probot-home-assistant Bot added the integration: xiaomi_miio label Feb 11, 2020

probot-home-assistant Bot assigned rytilahti and syssi Feb 11, 2020

MartinHjelmare changed the title ~~Adding support for Mi AirPurifier 3 (DRAFT)~~ Add support for Mi AirPurifier 3 (DRAFT) Feb 13, 2020

Kane610 reviewed Feb 13, 2020

View reviewed changes

petrkotek mentioned this pull request Feb 15, 2020

Add basic support for Xiaomi Mi Air Purifier 3/3H rytilahti/python-miio#585

Closed

2 tasks

mouth4war reviewed Feb 17, 2020

View reviewed changes

Comment thread homeassistant/components/xiaomi_miio/fan.py

mouth4war mentioned this pull request Feb 18, 2020

Support for Xiaomi Air Purifier 3 (zhimi.airpurifier.ma4) rytilahti/python-miio#577

Closed

pkrolkgp reviewed Feb 18, 2020

View reviewed changes

Comment thread homeassistant/components/xiaomi_miio/fan.py

Comment thread homeassistant/components/xiaomi_miio/fan.py Outdated

Comment thread homeassistant/components/xiaomi_miio/fan.py

pkrolkgp reviewed Feb 19, 2020

View reviewed changes

Comment thread homeassistant/components/xiaomi_miio/fan.py

Adding support for Mi AirPurifier 3H

65c2f15

foxel force-pushed the mi_purifier_3 branch from e2c950b to 65c2f15 Compare February 19, 2020 02:24

dj4shots approved these changes Feb 19, 2020

View reviewed changes

pkrolkgp and others added 2 commits February 19, 2020 20:31

Enabled setting fan level for AirPurifier 3/3H

7a6fd59

Merge pull request #1 from pkrolkgp/mi_purifier_3

7d6857e

Enabled setting fan level for AirPurifier 3/3H

homeassistant added the cla-needed label Feb 20, 2020

foxel mentioned this pull request Feb 20, 2020

Enabled setting fan level for AirPurifier 3/3H foxel/home-assistant-core#1

Merged

9 tasks

homeassistant removed the cla-needed label Feb 21, 2020

rytilahti reviewed Mar 5, 2020

View reviewed changes

Get rid of isinstance for MIOT AirPurifier

d220841

rytilahti mentioned this pull request Mar 19, 2020

viomi.vacuum.v7 and zhimi.airpurifier.mb3 support homeassistain yet? rytilahti/python-miio#645

Closed

foxel added 3 commits March 29, 2020 21:38

Merge remote-tracking branch 'hass/dev' into mi_purifier_3

d700d22

Bumping xiaomi-miio dependency

1ec5cac

Fixed lint error

9ee5178

foxel force-pushed the mi_purifier_3 branch from b00629a to 9ee5178 Compare March 29, 2020 17:37

foxel marked this pull request as ready for review March 29, 2020 17:39

foxel changed the title ~~Add support for Mi AirPurifier 3 (DRAFT)~~ Add support for Mi AirPurifier 3 Mar 31, 2020

rytilahti reviewed Mar 31, 2020

View reviewed changes

Update manifest.json

e230c72

rytilahti reviewed Apr 2, 2020

View reviewed changes

foxel added 2 commits April 3, 2020 18:50

Fixed requirements files for 0.5.0.1

93d5aa4

Merge branch 'dev' into mi_purifier_3

3639b1a

foxel mentioned this pull request Apr 3, 2020

Added Xiaomi Air Purifier 3/3H (2019) home-assistant/home-assistant.io#12653

Merged

8 tasks

rytilahti merged commit 912eb32 into home-assistant:dev Apr 3, 2020

MartinHjelmare reviewed Apr 3, 2020

View reviewed changes

iromeo mentioned this pull request Apr 6, 2020

Add support for Xiaomi Zero Fog Free Humidifier #33736

Closed

20 tasks

lock Bot locked and limited conversation to collaborators Apr 15, 2020

	SPEED_OFF = "off"
	SPEED_LOW = "low"
	SPEED_MEDIUM = "medium"
	SPEED_HIGH = "high"

Uh oh!

Conversation

foxel commented Feb 11, 2020 • edited by rytilahti Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

Uh oh!

probot-home-assistant Bot commented Feb 11, 2020

Uh oh!

Kane610 Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pkrolkgp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

foxel commented Feb 19, 2020

Uh oh!

PawelKwiecinski commented Feb 19, 2020

Uh oh!

homeassistant commented Feb 20, 2020

Uh oh!

Flagun commented Mar 4, 2020

Uh oh!

pkrolkgp commented Mar 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pawelkw commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foxel commented Mar 14, 2020

Uh oh!

bbiegun commented Mar 15, 2020

Uh oh!

rytilahti commented Mar 15, 2020

Uh oh!

CLARENNE-Q commented Mar 26, 2020

Uh oh!

rytilahti commented Mar 28, 2020

Uh oh!

bbiegun commented Mar 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rytilahti commented Mar 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nhok3296 commented Mar 31, 2020

Uh oh!

foxel commented Mar 31, 2020

Uh oh!

nhok3296 commented Mar 31, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rytilahti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

foxel commented Feb 11, 2020 •

edited by rytilahti

Loading

Example entry for `configuration.yaml`:

Kane610 Feb 13, 2020 •

edited

Loading

pawelkw commented Mar 13, 2020 •

edited

Loading

bbiegun commented Mar 29, 2020 •

edited

Loading

rytilahti commented Mar 29, 2020 •

edited

Loading

rytilahti Apr 3, 2020 •

edited

Loading

CLARENNE-Q commented Apr 9, 2020 •

edited

Loading