Device support for the xiaomi air purifier added. #31

syssi · 2017-07-23T21:16:10Z

No description provided.

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+        s += "limit_hum=%s%%, trans_level=%s%% " % (self.limit_hum, self.trans_level)
+        s += "bright=%s%%, favorite_level=%s%% " % (self.bright, self.favorite_level)
+        s += "filter_life_remaining=%s%%, act_det=%s%% " % (self.filter_life_remaining, self.act_det)
+        s += "filter_hours_used=%s%%, use_time=%s%% " % (self.filter_hours_used, self.use_time)


line too long (95 > 79 characters)

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+        s += "buzzer=%s%%, child_lock=%s%% " % (self.buzzer, self.child_lock)
+        s += "limit_hum=%s%%, trans_level=%s%% " % (self.limit_hum, self.trans_level)
+        s += "bright=%s%%, favorite_level=%s%% " % (self.bright, self.favorite_level)
+        s += "filter_life_remaining=%s%%, act_det=%s%% " % (self.filter_life_remaining, self.act_det)


line too long (101 > 79 characters)

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+        s += "mode=%s%%, led=%s%% , led_b=%s%% " % (self.mode, self.led, self.led_b)
+        s += "buzzer=%s%%, child_lock=%s%% " % (self.buzzer, self.child_lock)
+        s += "limit_hum=%s%%, trans_level=%s%% " % (self.limit_hum, self.trans_level)
+        s += "bright=%s%%, favorite_level=%s%% " % (self.bright, self.favorite_level)


line too long (85 > 79 characters)

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+        s += "temperature=%s%%, humidity=%s%% " % (self.temperature, self.humidity)
+        s += "mode=%s%%, led=%s%% , led_b=%s%% " % (self.mode, self.led, self.led_b)
+        s += "buzzer=%s%%, child_lock=%s%% " % (self.buzzer, self.child_lock)
+        s += "limit_hum=%s%%, trans_level=%s%% " % (self.limit_hum, self.trans_level)


line too long (85 > 79 characters)

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+    def __str__(self) -> str:
+        s = "<AirPurifierStatus power=%s, aqi=%s " % (self.power, self.aqi)
+        s += "temperature=%s%%, humidity=%s%% " % (self.temperature, self.humidity)
+        s += "mode=%s%%, led=%s%% , led_b=%s%% " % (self.mode, self.led, self.led_b)


line too long (84 > 79 characters)

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+
+    def __str__(self) -> str:
+        s = "<AirPurifierStatus power=%s, aqi=%s " % (self.power, self.aqi)
+        s += "temperature=%s%%, humidity=%s%% " % (self.temperature, self.humidity)


line too long (83 > 79 characters)

houndci-bot · 2017-07-23T21:16:21Z

mirobo/containers.py

+
+    @property
+    def mode(self) -> str:
+        return self.data["mode"]  # auto, silent, favorite, medium, high, strong, idle


line too long (86 > 79 characters)

rytilahti · 2017-07-23T21:45:22Z

Hi Sebastian! Patches for new devices are very welcome and I think people with those devices will truly appreciate that :-) I think it is time to start considering how to deal with the issue of the separate binaries soon enough, but before that I think it makes sense not to populate the containers file with everything, but just have those in the same file as the rest of the (device-specific) implementation. What do you think?

syssi · 2017-07-23T21:52:43Z

This is fine for me. I will just move it.

houndci-bot · 2017-07-23T21:56:50Z

mirobo/airpurifier.py

+
+class AirPurifierStatus:
+    """Container for status reports from the air purifier."""
+    def __init__(self, data: Dict[str, Any]) -> None:


undefined name 'Dict'
undefined name 'Any'

rytilahti

Looking good in general, I left some small notes behind.

rytilahti · 2017-07-23T21:49:36Z

mirobo/containers.py

+
+    @property
+    def power(self) -> str:
+        return self.data["power"]


Do you have an example what type of response is expected here? A numerical or a string?

rytilahti · 2017-07-23T21:50:28Z

mirobo/containers.py

+
+    @property
+    def aqi(self) -> int:
+        return self.data["aqi"]


aqi = air quality index?

rytilahti · 2017-07-23T21:50:59Z

mirobo/containers.py

+
+    @property
+    def child_lock(self) -> str:
+        return self.data["child_lock"]


Is this a boolean or a string?

rytilahti · 2017-07-23T21:52:49Z

mirobo/airpurifier.py

+        values = self.send(
+            "get_prop",
+            properties
+        )


Considering that the get_prop is pretty generic (it's also used for the strip it seems, as well as for the yeelights), maybe it makes sense to have a list (or a mapping?) of properties and their corresponding types listed and move the status() implementation to the parent class at some point? See also the comments below regarding to property names.

Yes, the properties are somehow generic but they cannot be applied to all devices. For example there are three versions of the air purifier out there all of them with different supported OperationModes. Some devices does respond with "on" and "off"... others with "true" and "false". I suggest to refactor the code if the support is stable and the knowledge about different responses stronger.

Ok, sounds sane! 👍

To add, do you happen to have an example also for this case? It would be nice to have example answers for all calls supported by devices.

I must confess I don't own the device. I just want to see it supported. I will ask around. :-)

Thanks for helping to get it supported! :-) Potential tester available here: https://community.home-assistant.io/t/xiaomi-mi-air-purifier-support/14228/12 but for that we need to have a simple test script.

rytilahti · 2017-07-23T21:53:33Z

mirobo/airpurifier.py

+        """Set mode."""
+
+        # auto, silent, favorite, medium, high, strong, idle
+        return self.send("set_mode", [mode])


Enumize setting and getting of the mode?

rytilahti · 2017-07-23T21:54:46Z

mirobo/containers.py

+        #  'limit_hum', 'trans_level', 'bright',
+        #  'favorite_level', 'filter1_life', 'act_det',
+        #  'f1_hour_used', 'use_time', 'motor1_speed']
+        self.data = data


Do you have an example of a response? It would be nice to have included also.

rytilahti · 2017-07-23T21:55:26Z

mirobo/containers.py

+    @property
+    def favorite_level(self) -> str:
+        # Favorite level used when the mode is `favorite`. Between 0 and 16.
+        return self.data["favorite_level"]


Is this an integer or str?

rytilahti · 2017-07-23T21:56:28Z

mirobo/containers.py

+
+    @property
+    def act_det(self) -> bool:
+        return self.data["act_det"] == "on"


Any description for this , limit_hum and trans_level? The names are not really descriptive.

houndci-bot · 2017-07-25T19:59:59Z

mirobo/airpurifier.py

+        return self.data["temp_dec"] / 10.0
+
+    @property
+    def mode(self) -> OperationMode:


undefined name 'OperationMode'

houndci-bot · 2017-07-25T19:59:59Z

mirobo/airpurifier.py

+        )
+        return AirPurifierStatus(dict(zip(properties, values)))
+
+    def set_mode(self, mode: OperationMode):


undefined name 'OperationMode'

It's better to move the class on top of the file, another possibility would be to use a forward declaration ('OperationMode'), which does not look so nice IMO and should be avoided where not really necessary (like when using it inside its own defining class).

rytilahti · 2017-07-25T20:14:48Z

mirobo/airpurifier.py

+
+    def set_led(self, led: bool):
+        """Turn led on/off."""
+


No need for extra white-spaces after the docstring, odd that hound did not yell about that...

rytilahti · 2017-07-25T20:25:26Z

mirobo/airpurifier.py

+
+    @property
+    def limit_hum(self) -> str:
+        return self.data["limit_hum"]


I think my comment on these got lost somewhere, can we have better, more descriptive names for this, trans_level, act_det and led_b? led_brightness probably for led_b, but I have no idea what those two others could even be..

Done. I don't know the meaning of trans_level and act_det, too. ;-)

Whitespaces removed.

rytilahti · 2017-07-25T20:41:01Z

mirobo/airpurifier.py

+
+    @property
+    def child_lock(self) -> str:
+        return self.data["child_lock"]


Is this a boolean or "on" / "off"?

rytilahti · 2017-07-25T20:41:14Z

mirobo/airpurifier.py

+
+    @property
+    def humidity_limit(self) -> str:
+        return self.data["limit_hum"]


str or float/int?

rytilahti · 2017-07-25T20:41:34Z

mirobo/airpurifier.py

+    @property
+    def favorite_level(self) -> str:
+        # Favorite level used when the mode is `favorite`. Between 0 and 16.
+        return self.data["favorite_level"]


Considering the setter is using int, this is probably integer too, right?

rytilahti · 2017-07-25T20:42:03Z

mirobo/airpurifier.py

+
+    @property
+    def trans_level(self) -> str:
+        return self.data["trans_level"]


Level indicates that it's probably an integer, but this is ok and can be tuned when the payload is known..

syssi · 2017-07-25T21:11:00Z

I will fix/clarify the open questions if some payload of a response is known. It's fine for me if the PR won't get merged awhile.

houndci-bot · 2017-07-27T07:02:49Z

mirobo/airpurifier.py

+    def __init__(self, data: Dict[str, Any]) -> None:
+        # Response of a Air Purifier Pro:
+        # ['power': 'on, 'aqi': 41, 'humidity': 62, 'temp_dec': 293,
+        #  'mode': 'auto', 'led': 'on', 'led_b': null, 'buzzer': null, 'child_lock': 'off',


line too long (91 > 79 characters)

…max at once.

rytilahti · 2017-07-27T16:09:49Z

mirobo/airpurifier.py

+        #  'mode': 'auto', 'led': 'on', 'led_b': null, 'buzzer': null,
+        #  'child_lock': 'off', 'limit_hum': null, 'trans_level': null,
+        #  'bright': 71, 'favorite_level': 17, 'filter1_life': 77,
+        #  'act_det': null, 'f1_hour_used': 771, 'use_time': 2776200,


Favorite level is 17 here, but in the comment of it says: # Favorite level used when the mode is favorite. Between 0 and 16. Those which return null should probably be marked in typing with eg. Optional[bool] (buzzer for example). Hopefully we can get some more test examples to see if those are always null..

Oh, to add. Those are probably encoded as Nones, trying to construct an enum out of them will cause ValueError, so trying to access led_brightness would cause a crash at least. I think

if self.data["led_b"] is not None: return LedBrightness(self.data["led_b"])

is probably the best solution here.

…lid. Mode medium, high and strong removed.

… into feature/air-purifier

rytilahti · 2017-08-01T20:36:07Z

We are also going to need mirobo -d info output from this device to add support to the (upcoming) new cli tool.

syssi · 2017-08-04T06:47:41Z

Known models: 'zhimi.airpurifier.m1', 'zhimi.airpurifier.v1', 'zhimi.airpurifier.v2', 'zhimi.airpurifier.v3', 'zhimi.airpurifier.v6', 'zhimi.humidifier.v1'.

rytilahti · 2017-08-14T06:13:05Z

Ok, thanks! Let's get this merged, the client tool can be added / updated later on, maybe having this in encourages someone to do that :-)

Device support for the xiaomi air purifier added.

ef60009

houndci-bot reviewed Jul 23, 2017

View reviewed changes

Code reformatted.

f1c03c3

Status class moved.

fa9b0e7

houndci-bot reviewed Jul 23, 2017

View reviewed changes

rytilahti reviewed Jul 23, 2017

View reviewed changes

syssi added 2 commits July 23, 2017 23:57

Missing import added.

bad603f

OperationMode class introduced.

d8f5920

houndci-bot reviewed Jul 25, 2017

View reviewed changes

rytilahti reviewed Jul 25, 2017

View reviewed changes

Position of the OperationMode class moved.

6191b80

rytilahti reviewed Jul 25, 2017

View reviewed changes

syssi added 2 commits July 25, 2017 22:28

LedBrightness class introduced.

19fc11e

Whitespaces removed.

More meaningful names used.

7b990a2

rytilahti reviewed Jul 25, 2017

View reviewed changes

syssi and others added 2 commits July 25, 2017 23:21

Merge branch 'master' into feature/air-purifier

7933690

Example response of an air purifier pro added. Thanks to @af950833.

1b3a934

houndci-bot reviewed Jul 27, 2017

View reviewed changes

syssi added 2 commits July 27, 2017 09:21

Code reformatted.

7ae0247

Example response updated. It looks like we can request 16 properties …

72a2086

…max at once.

rytilahti reviewed Jul 27, 2017

View reviewed changes

syssi added 3 commits July 27, 2017 19:25

Don't interpret unavailable property.

5409359

A few operation modes are no longer exposed by the air purifier as va…

8085496

…lid. Mode medium, high and strong removed.

Merge branch 'feature/air-purifier' of github.com:syssi/python-mirobo…

61b5189

… into feature/air-purifier

rytilahti merged commit 42a9d9b into rytilahti:master Aug 14, 2017

rytilahti mentioned this pull request Sep 3, 2017

Add support for other devices using the same protocol #17

Closed

Device support for the xiaomi air purifier added. #31

Device support for the xiaomi air purifier added. #31

Conversation

syssi commented Jul 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rytilahti commented Jul 23, 2017

syssi commented Jul 23, 2017

Choose a reason for hiding this comment

rytilahti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syssi commented Jul 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rytilahti Jul 27, 2017 • edited

Choose a reason for hiding this comment

rytilahti commented Aug 1, 2017

syssi commented Aug 4, 2017

rytilahti commented Aug 14, 2017

syssi commented Jul 25, 2017 •

edited

rytilahti Jul 27, 2017 •

edited