Support of the unified command line interface for all devices by syssi · Pull Request #289 · rytilahti/python-miio

syssi · 2018-03-31T20:15:21Z

No description provided.

houndci-bot · 2018-03-31T20:15:33Z

miio/powerstrip.py

        return self.send("set_power", ["on"])

+    @command(
+        default_output = format_output("Powering off"),


unexpected spaces around keyword / parameter equals

houndci-bot · 2018-03-31T20:15:33Z

miio/powerstrip.py

            defaultdict(lambda: None, zip(properties, values)))

+    @command(
+        default_output = format_output("Powering on"),


unexpected spaces around keyword / parameter equals

houndci-bot · 2018-03-31T20:15:33Z

miio/philips_bulb.py

+    @command(
+        click.argument("brightness", type=int),
+        click.argument("cct", type=int),
+        default_output=format_output("Setting brightness to {brightness} and color temperature to {cct}")


line too long (105 > 100 characters)

houndci-bot · 2018-03-31T20:15:33Z

miio/fan.py

        return self.send("set_angle_enable", ["off"])

+    @command(
+        click.argument("brightness", type=EnumType(LedBrightness, False)),


undefined name 'EnumType'

Import added.

houndci-bot · 2018-03-31T20:15:33Z

miio/fan.py

        return self.send("set_speed_level", [level])  # 0...100

+    @command(
+        click.argument("direction", type=EnumType(MoveDirection, False)),


undefined name 'EnumType'

Import added.

houndci-bot · 2018-03-31T20:15:34Z

miio/ceil.py

+    @command(
+        click.argument("brightness", type=int),
+        click.argument("cct", type=int),
+        default_output=format_output("Setting brightness to {brightness} and color temperature to {cct}")


line too long (105 > 100 characters)

coveralls · 2018-03-31T20:29:53Z

Coverage increased (+0.4%) to 70.095% when pulling bb9302e on syssi:feature/unified-cli-support into 91b05a1 on rytilahti:master.

houndci-bot · 2018-04-01T07:15:08Z

miio/tests/test_vacuum.py

-from miio import Vacuum, VacuumStatus
 from .dummies import DummyDevice
+import datetime
+from miio import Vacuum, VacuumStatus, VacuumException


'miio.VacuumException' imported but unused

houndci-bot · 2018-04-01T07:15:08Z

miio/tests/test_vacuum.py

-from miio import Vacuum, VacuumStatus
 from .dummies import DummyDevice
+import datetime
+from miio import Vacuum, VacuumStatus, VacuumException


'miio.VacuumException' imported but unused

houndci-bot · 2018-04-01T08:05:46Z

miio/wifispeaker.py


+import click
+
+from .click_common import command, format_output, EnumType


'.click_common.EnumType' imported but unused

This reverts commit 91dc4d7.

syssi · 2018-04-04T06:55:40Z

I would be happy to merge this soon. Rebasing the PR takes a lot of time.

rytilahti

There is quite a bit to review, but I think we can simply merge this and do clean-ups later on. I just added a couple of comments for now.

rytilahti · 2018-04-02T14:06:01Z

miio/__init__.py

 from miio.wifirepeater import WifiRepeater
 from miio.wifispeaker import WifiSpeaker
 from miio.yeelight import Yeelight
+


Remove the newline.

I made this newline because miio.discovery cannot be moved to line 10. The position of this import is important. The newline shall help here. ;-)

Hmm, I'm not following? Why it cannot be moved/why it is important?

If I move the discovery import to the top the tests are failing:

_______________________________________________________________________________________ ERROR collecting miio/tests/test_airconditioningcompanion.py _________________________________________________________________________________________ ImportError while importing test module '/home/sebastian/src/python-miio/miio/tests/test_airconditioningcompanion.py'. Hint: make sure your test modules/packages have valid Python names. Traceback: miio/__init__.py:2: in <module> from miio.discovery import Discovery miio/discovery.py:10: in <module> from . import (Device, Vacuum, ChuangmiPlug, PowerStrip, AirPurifier, Ceil, E ImportError: cannot import name 'Device' ______________________________________________________________________________________________ ERROR collecting miio/tests/test_airhumidifier.py ______________________________________________________________________________________________ ImportError while importing test module '/home/sebastian/src/python-miio/miio/tests/test_airhumidifier.py'. Hint: make sure your test modules/packages have valid Python names. Traceback: miio/__init__.py:2: in <module> from miio.discovery import Discovery miio/discovery.py:10: in <module> from . import (Device, Vacuum, ChuangmiPlug, PowerStrip, AirPurifier, Ceil, E ImportError: cannot import name 'Device'

Ohh, I see. I just didn't understand why the new line was necessary (as it should be just about the order, right?), but this is fine for me.

Correct! :-)

rytilahti · 2018-04-04T18:48:50Z

miio/airconditioningcompanion.py


+    @command(
+        default_output=format_output("Powering the air condition on"),
+    )


When it all fits into one line (and there are no arguments), I think it'd be nice to have it all on a single line.

Are you sure? It feels hard to read because of the braces and the little spaces.

Well, I think it would be nicer for grepping the code, but it's not so important and can be changed later if needed.

rytilahti

Looks good to me, I think we can merge and adjust as needed. What also needs to be added is an updated documentation for the new style cli before releasing a new version.

rytilahti · 2018-04-07T21:57:31Z

miio/airconditioningcompanion.py


+    @command(
+        default_output=format_output(
+            "",


Maybe it would make sense to use a keyword argument here, otherwise it remains unclear what does "" do.

houndci-bot reviewed Mar 31, 2018

View reviewed changes

syssi assigned syssi and unassigned syssi Mar 31, 2018

syssi requested a review from rytilahti March 31, 2018 21:18

syssi force-pushed the feature/unified-cli-support branch from 17ac625 to f5547d7 Compare April 1, 2018 07:14

houndci-bot reviewed Apr 1, 2018

View reviewed changes

syssi changed the title ~~Support of the unified commad line interface added to many devices~~ Support of the unified command line interface for all devices Apr 3, 2018

syssi added 17 commits April 4, 2018 08:08

Add unified cli support to Air Humidifier

dc1e015

Add unified cli support to Air Quality Monitor

01d2182

Add unified cli support to Ceiling Lamp

239c412

Add unified cli support to Chuangmi IR controller

c7ae443

Add unified cli support to Fan

70a2521

Optimize imports

5cde0c8

Add unified cli support to Philips Bulb

1c49375

Add unified cli support to Philips Eyecare Lamp

2f24058

Add unified cli support to Power Strip

beed73a

Add unified cli support to Water Purifier

5f4974a

Add unified cli support to WiFi Repeater

9968bd8

Make hound happy

17e505e

Don't access Optional enums

2474d88

Revert "Optimize imports"

8c310bd

This reverts commit 91dc4d7.

Use optimized imports

9ab36b2

Add unified cli support to Air Conditioner Companion

9e247a4

Add unified cli support to WiFi Speaker

c7939cd

syssi force-pushed the feature/unified-cli-support branch from fc92fde to c7939cd Compare April 4, 2018 06:24

Typo fixed

bb9302e

rytilahti reviewed Apr 4, 2018

View reviewed changes

rytilahti approved these changes Apr 7, 2018

View reviewed changes

syssi merged commit 71195ce into rytilahti:master Apr 8, 2018


		import click

		from .click_common import command, format_output, EnumType

Uh oh!

Conversation

syssi commented Mar 31, 2018

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!

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!

coveralls commented Mar 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

syssi commented Apr 4, 2018

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Mar 31, 2018 •

edited

Loading