Additional properties of the Xiaomi Air Purifier 2 introduced #132

syssi · 2017-11-26T14:11:10Z

Some comments improved.
Some code clean-up.

houndci-bot · 2017-11-26T14:11:21Z

miio/airpurifier.py

@@ -1,5 +1,6 @@
 import logging
 import enum
+import math


'math' imported but unused

coveralls · 2017-11-26T14:12:44Z

Coverage decreased (-0.009%) to 46.029% when pulling e516138 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

coveralls · 2017-11-26T14:12:44Z

Coverage decreased (-0.009%) to 46.029% when pulling e516138 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

coveralls · 2017-11-26T14:50:43Z

Coverage decreased (-0.03%) to 46.008% when pulling d21e954 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

houndci-bot · 2017-11-26T16:21:48Z

miio/tests/test_airpurifier.py

+
+        assert self.is_on() is True
+        assert self.state().temperature == \
+               self.device.start_state["temp_dec"] / 10.0


continuation line over-indented for visual indent

houndci-bot · 2017-11-26T17:06:19Z

miio/tests/test_airpurifier.py

+        assert child_lock() == True
+
+        self.device.set_child_lock(False)
+        assert child_lock() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2017-11-26T17:06:19Z

miio/tests/test_airpurifier.py

+            return self.device.status().child_lock
+
+        self.device.set_child_lock(True)
+        assert child_lock() == True


comparison to True should be 'if cond is True:' or 'if cond:'

houndci-bot · 2017-11-26T17:06:19Z

miio/tests/test_airpurifier.py

+        assert buzzer() == True
+
+        self.device.set_buzzer(False)
+        assert buzzer() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2017-11-26T17:06:19Z

miio/tests/test_airpurifier.py

+            return self.device.status().buzzer
+
+        self.device.set_buzzer(True)
+        assert buzzer() == True


comparison to True should be 'if cond is True:' or 'if cond:'

houndci-bot · 2017-11-26T17:06:19Z

miio/tests/test_airpurifier.py

+        assert led() == True
+
+        self.device.set_led(False)
+        assert led() == False


comparison to False should be 'if cond is False:' or 'if not cond:'

houndci-bot · 2017-11-26T17:06:19Z

miio/tests/test_airpurifier.py

+        assert self.state().mode == OperationMode(self.device.start_state["mode"])
+        assert self.state().favorite_level == self.device.start_state["favorite_level"]
+        assert self.state().filter_life_remaining == self.device.start_state["filter1_life"]
+        assert self.state().filter_hours_used == self.device.start_state["f1_hour_used"]


line too long (88 > 79 characters)

houndci-bot · 2017-11-26T17:06:20Z

miio/tests/test_airpurifier.py

+        assert self.state().humidity == self.device.start_state["humidity"]
+        assert self.state().mode == OperationMode(self.device.start_state["mode"])
+        assert self.state().favorite_level == self.device.start_state["favorite_level"]
+        assert self.state().filter_life_remaining == self.device.start_state["filter1_life"]


line too long (92 > 79 characters)

houndci-bot · 2017-11-26T17:06:20Z

miio/tests/test_airpurifier.py

+        assert self.state().temperature == self.device.start_state["temp_dec"] / 10.0
+        assert self.state().humidity == self.device.start_state["humidity"]
+        assert self.state().mode == OperationMode(self.device.start_state["mode"])
+        assert self.state().favorite_level == self.device.start_state["favorite_level"]


line too long (87 > 79 characters)

houndci-bot · 2017-11-26T17:06:20Z

miio/tests/test_airpurifier.py

+        assert self.state().average_aqi == self.device.start_state["average_aqi"]
+        assert self.state().temperature == self.device.start_state["temp_dec"] / 10.0
+        assert self.state().humidity == self.device.start_state["humidity"]
+        assert self.state().mode == OperationMode(self.device.start_state["mode"])


line too long (82 > 79 characters)

houndci-bot · 2017-11-26T17:06:20Z

miio/tests/test_airpurifier.py

+        assert self.is_on() is True
+        assert self.state().aqi == self.device.start_state["aqi"]
+        assert self.state().average_aqi == self.device.start_state["average_aqi"]
+        assert self.state().temperature == self.device.start_state["temp_dec"] / 10.0


line too long (85 > 79 characters)

coveralls · 2017-11-26T17:15:01Z

Coverage increased (+3.4%) to 49.448% when pulling 85b0f10 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

houndci-bot · 2017-11-26T17:39:30Z

miio/tests/test_airpurifier.py

+        self.device.state["bright"] = self.device.state["led_b"]
+        del self.device.state["led_b"]
+
+        assert self.state().led_brightness == LedBrightness(self.device.start_state["led_b"])


line too long (93 > 79 characters)

houndci-bot · 2017-11-26T17:39:30Z

miio/tests/test_airpurifier.py

+
+        # The LED brightness of a Air Purifier Pro cannot be set so far.
+        self.device.set_led(True)
+        assert led() == True


comparison to True should be 'if cond is True:' or 'if cond:'

coveralls · 2017-11-26T17:42:39Z

Coverage increased (+3.9%) to 49.924% when pulling e5c83c3 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

rytilahti

Thanks for fixing docstrings and new tests! I added a couple of comments behind, feel free to merge if you consider things are fine as they are / after changing those (and fixing those hound warnings where deemed necessary, i.e. the None checks at least) :-)

rytilahti · 2017-11-26T18:45:34Z

miio/airpurifier.py


        values = self.send(
            "get_prop",
-            properties
+            properties[0:13]


Mind leaving a comment behind to explain why using only a slice here? Same below too.

rytilahti · 2017-11-26T18:59:52Z

miio/airpurifier.py

+
+        # This is the property name of the Air Purifier Pro
+        if self.data["bright"] is not None:
+            return LedBrightness(self.data["bright"])


Does this mean that when the device is not air purifier pro, the "led_b" will be empty? If yes, then it's fine.

Yep. The air purifier provides a "bright" property XOR a "led_b" property.

rytilahti · 2017-11-26T19:03:16Z

miio/tests/test_airpurifier.py

+            return self.device.status().favorite_level
+
+        self.device.set_favorite_level(1)
+        assert favorite_level() == 1


Is it known what values are allowed for favorite level? If yes, it makes probably sense to add an exception if out-of-range values are inputed, and to add a test for that also here (https://docs.pytest.org/en/latest/assert.html#assertions-about-expected-exceptions).

coveralls · 2017-11-26T20:11:55Z

Coverage increased (+3.8%) to 49.867% when pulling 7b96492 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

houndci-bot · 2017-11-26T20:25:15Z

miio/tests/test_airpurifier.py

+        del self.device.state["led_b"]
+
+        assert self.state().led_brightness == \
+               LedBrightness(self.device.start_state["led_b"])


continuation line over-indented for visual indent

houndci-bot · 2017-11-26T20:25:15Z

miio/tests/test_airpurifier.py

+
+        assert self.is_on() is True
+        assert self.state().aqi == self.device.start_state["aqi"]
+        assert self.state().average_aqi == self.device.start_state["average_aqi"]


line too long (81 > 79 characters)

houndci-bot · 2017-11-26T20:25:16Z

miio/tests/test_airpurifier.py

+        del self.device.state["led_b"]
+
+        assert self.state().led_brightness == \
+               LedBrightness(self.device.start_state["led_b"])


continuation line over-indented for visual indent

houndci-bot · 2017-11-26T20:25:16Z

miio/tests/test_airpurifier.py

+
+        assert self.is_on() is True
+        assert self.state().aqi == self.device.start_state["aqi"]
+        assert self.state().average_aqi == self.device.start_state["average_aqi"]


line too long (81 > 79 characters)

coveralls · 2017-11-26T20:26:57Z

Coverage increased (+4.04%) to 50.075% when pulling fc596f8 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

coveralls · 2017-11-26T20:28:41Z

Coverage increased (+4.04%) to 50.075% when pulling 3911b40 on syssi:feature/update-some-comments-and-fixmes into 3bed026 on rytilahti:master.

rytilahti · 2017-11-26T20:38:42Z

miio/tests/test_airpurifier.py

-    def test_status_string(self):
-        self.device._reset_state()
-
-        assert self.state().__str__


The status string should be __repr__ instead of __str__, right? https://stackoverflow.com/questions/1436703/difference-between-str-and-repr-in-python

rytilahti · 2017-11-26T20:39:25Z

miio/airpurifier.py


-        use_time and motor1_speed is missing because a request is limitted
-        to 16 properties. We request 15 properties at the moment.
+        A request is limitted to 16 properties.


Additional properties of the Xiaomi Air Purifier 2 introduced. Some code clean-up.

Property average_aqi fixed. Missing method "set_child_lock" added.

… fixed.

coveralls · 2017-11-26T20:47:06Z

Coverage increased (+3.2%) to 57.797% when pulling cf5a544 on syssi:feature/update-some-comments-and-fixmes into 3df66fd on rytilahti:master.

coveralls · 2017-11-26T20:55:08Z

Coverage increased (+3.2%) to 57.797% when pulling 9f1e4c1 on syssi:feature/update-some-comments-and-fixmes into 3df66fd on rytilahti:master.

coveralls · 2017-11-26T21:39:48Z

Coverage increased (+3.3%) to 57.854% when pulling 779bb83 on syssi:feature/update-some-comments-and-fixmes into 3df66fd on rytilahti:master.

This reverts commit 779bb83.

coveralls · 2017-11-26T21:48:28Z

Coverage increased (+3.2%) to 57.797% when pulling c22086a on syssi:feature/update-some-comments-and-fixmes into 3df66fd on rytilahti:master.

houndci-bot reviewed Nov 26, 2017

View reviewed changes

miio/airpurifier.py Outdated

@@ -1,5 +1,6 @@

import logging

import enum

import math

Copy link

houndci-bot Nov 26, 2017

Choose a reason for hiding this comment

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

'math' imported but unused

houndci-bot reviewed Nov 26, 2017

View reviewed changes

syssi mentioned this pull request Nov 26, 2017

Add service for setting the child_lock syssi/xiaomi_airpurifier#13

Closed

houndci-bot reviewed Nov 26, 2017

View reviewed changes

rytilahti reviewed Nov 26, 2017

View reviewed changes

houndci-bot reviewed Nov 26, 2017

View reviewed changes

rytilahti reviewed Nov 26, 2017

View reviewed changes

syssi added 8 commits November 26, 2017 21:45

Some comments improved.

63113b3

Additional properties of the Xiaomi Air Purifier 2 introduced. Some code clean-up.

Unused import removed.

eeadba7

Unittests for the air purifier added.

223e106

Property average_aqi fixed. Missing method "set_child_lock" added.

Unit tests for the xiaomi air purifier 2 introduced. A few found bugs…

6824040

… fixed.

Some unit tests for edge cases added.

e193fbe

Testing of the __str__ method doesn't work.

1256959

Testing of limits introduced. Hound errors fixed.

68921db

Hound error fixed.

cf5a544

syssi force-pushed the feature/update-some-comments-and-fixmes branch from 3911b40 to cf5a544 Compare November 26, 2017 20:45

Moved from __str__ to __repr__.

9f1e4c1

Use a specific code path.

779bb83

Revert "Use a specific code path."

c22086a

This reverts commit 779bb83.

syssi merged commit 5ba379e into rytilahti:master Nov 26, 2017

Additional properties of the Xiaomi Air Purifier 2 introduced #132

Additional properties of the Xiaomi Air Purifier 2 introduced #132

Conversation

syssi commented Nov 26, 2017

Choose a reason for hiding this comment

coveralls commented Nov 26, 2017

coveralls commented Nov 26, 2017 • edited

coveralls commented Nov 26, 2017 • edited

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

coveralls commented Nov 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 26, 2017

rytilahti left a comment • edited

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

coveralls commented Nov 26, 2017 • edited

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

coveralls commented Nov 26, 2017 • edited

coveralls commented Nov 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 26, 2017 • edited

coveralls commented Nov 26, 2017 • edited

coveralls commented Nov 26, 2017 • edited

coveralls commented Nov 26, 2017 • edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

rytilahti left a comment •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited

coveralls commented Nov 26, 2017 •

edited