Added support for Xiaomi Philips LED Ceiling Lamp by kuduka · Pull Request #35 · rytilahti/python-miio

kuduka · 2017-07-30T18:15:18Z

Usage: miceil [OPTIONS] COMMAND [ARGS]...

A tool to command Xiaomi Philips LED Ceiling Lamp.

Options:
--ip TEXT
--token TEXT
-d, --debug
--help Show this message and exit.

Commands:
acct_off Auto CCT on.
acct_on Auto CCT on.
delay_off Set delay off in seconds.
discover Search for plugs in the network.
off Power off.
on Power on.
set_bright Set brightness level.
set_cct Set CCT level.
set_scene Set scene number.
sml_off Smart Midnight Light off.
sml_on Smart Midnight Light on.
status Returns the state information.

Not supported:

Auto On/Off Not Supported
Adjust Scens with Wall Switch Not Supported

houndci-bot · 2017-07-30T18:15:26Z

mirobo/ceil_cli.py

+    """Auto CCT on."""
+    click.echo("Auto CCT On: %s" % dev.ac_on())
+
+@cli.command()


expected 2 blank lines, found 1

houndci-bot · 2017-07-30T18:15:26Z

mirobo/ceil_cli.py

+    """Set delay off in seconds."""
+    click.echo("Delay off: %s" % dev.delay_off(seconds))
+
+@cli.command()


expected 2 blank lines, found 1

houndci-bot · 2017-07-30T18:15:26Z

mirobo/ceil_cli.py

+    click.echo("Auto On/Off When Mi Band is nearby: %s" % res.mb)
+    click.echo("Auto CCT: %s" % res.ac)
+
+@cli.command()


expected 2 blank lines, found 1

houndci-bot · 2017-07-30T18:15:26Z

mirobo/ceil_cli.py

+    """Search for plugs in the network."""
+    mirobo.Ceil.discover()
+
+@cli.command()


expected 2 blank lines, found 1

houndci-bot · 2017-07-30T18:15:26Z

mirobo/ceil_cli.py

+    if ctx.invoked_subcommand is None:
+        ctx.invoke(status)
+
+@cli.command()


expected 2 blank lines, found 1

houndci-bot · 2017-07-30T18:15:26Z

mirobo/ceil.py

+    def __str__(self) -> str:
+        s = "<CeilStatus power=%s, bright=%s, snm=%s, dv=%s, cctsw=%s " \
+            "bl=%s, mb=%s, ac=%s, >" % \
+            (self.power, self.bright, self.snm, self.dv, self.cctsw, 


trailing whitespace

houndci-bot · 2017-07-30T18:15:27Z

mirobo/ceil.py

+        )
+        return CeilStatus(dict(zip(properties, values)))
+
+class CeilStatus:


expected 2 blank lines, found 1

houndci-bot · 2017-07-30T18:15:27Z

mirobo/ceil.py

+    def status(self):
+        """Retrieve properties."""
+        properties = ['power', 'bright', 'snm', 'dv', 'cct'
+            'sw', 'bl', 'mb', 'ac', 'ms', ]


continuation line under-indented for visual indent

houndci-bot · 2017-07-30T18:15:27Z

mirobo/ceil.py

+    def set_cct(self, level: int):
+        """Set Correlated Color Temperature."""
+        return self.send("set_cct", [level])
+


blank line contains whitespace

houndci-bot · 2017-07-30T18:15:27Z

mirobo/ceil.py

+class Ceil(Device):
+    """Main class representing Xiaomi Philips LED Ceiling Lamp."""
+
+    #TODO: - Auto On/Off Not Supported


block comment should start with '# '

houndci-bot · 2017-07-30T18:26:51Z

mirobo/ceil.py

+from .device import Device
+from typing import Any, Dict
+
+class Ceil(Device):


expected 2 blank lines, found 1

rytilahti · 2017-08-01T20:34:33Z

Hi @kuduka and thanks for the PR! It is looking code-wise good but I'm currently hesitant to add more console tools to be installed, so I think I'll have to make some time and make it easier to add new devices without having to copy & paste the cli tool code around.

Could you please do mirobo -d info with a recent device to gather the model information for this and the another eyecare lamp? I think the new tool should use the model information, and provide the device-specific commands to be defined in the device classes (e.g. Ceil in this case).

kuduka · 2017-08-01T21:00:25Z

Sure, I'm no proficient with Python or even programming but I bet there's a better way to do it as we're replicating the same code over and over.

(venv) [kuduk@xps13 python-mirobo]$ venv/bin/mirobo --ip 192.168.1.71 --token 3d5f7ae53b51aa312e464b150b37453b -d info
INFO:mirobo.vacuum_cli:Debug mode active
ERROR:mirobo.vacuum_cli:Unable to read the stored msgid: [Errno 2] No such file or directory: '/tmp/python-mirobo.seq'
DEBUG:mirobo.vacuum_cli:Connecting to 192.168.1.71 with token 3d5f7ae53b51aa312e464b150b37453b
DEBUG:mirobo.protocol:Unable to decrypt, returning raw bytes.
DEBUG:mirobo.device:Discovered 884 55764 with ts: 1970-01-01 00:01:45
DEBUG:mirobo.device:192.168.1.71:54321 >>: {'id': 1, 'method': 'miIO.info', 'params': []}
DEBUG:mirobo.device:192.168.1.71:54321 (ts: 1970-01-01 00:01:45, id: 1) << {'result': {'life': 105, 'token': '3d5f7ae53b51aa312e464b150b37453b', 'mac': '34:CE:00:92:E6:21', 'fw_ver': '1.5.0', 'hw_ver': 'MTK7697', 'uid': '1647617138', 'model': 'philips.light.ceiling', 'mcu_fw_ver': '0027', 'wifi_fw_ver': '4.0.0', 'ap': {'rssi': -61, 'ssid': 'XarxaCasa', 'bssid': '5C:35:3B:EA:97:DE'}, 'netif': {'localIp': '192.168.1.71', 'mask': '255.255.255.0', 'gw': '192.168.1.1', 'gw_mac': '5C:35:3B:EA:97:DC'}, 'mmfree': 106488}, 'id': 1}
philips.light.ceiling v1.5.0 (34:CE:00:92:E6:21) @ 192.168.1.71 - token: 3d5f7ae53b51aa312e464b150b37453b
DEBUG:mirobo.vacuum_cli:Full response: {'ap': {'bssid': '5C:35:3B:EA:97:DE', 'rssi': -61, 'ssid': 'XarxaCasa'},
 'fw_ver': '1.5.0',
 'hw_ver': 'MTK7697',
 'life': 105,
 'mac': '34:CE:00:92:E6:21',
 'mcu_fw_ver': '0027',
 'mmfree': 106488,
 'model': 'philips.light.ceiling',
 'netif': {'gw': '192.168.1.1',
           'gw_mac': '5C:35:3B:EA:97:DC',
           'localIp': '192.168.1.71',
           'mask': '255.255.255.0'},
 'token': '3d5f7ae53b51aa312e464b150b37453b',
 'uid': '1647617138',
 'wifi_fw_ver': '4.0.0'}
DEBUG:mirobo.vacuum_cli:Writing {'seq': 1, 'manual_seq': 0} to /tmp/python-mirobo.seq

syssi · 2017-08-06T06:47:09Z

The xiaomi philips led ball (9290012800, 450lm, 3000K-5700K, 6.5W, E27) supports the same feature set:

$ ./mirobo --ip 192.168.130.58 --token 6f3cxxxxxxxxxxxxxxxxxxxxxxxx3790 -d info 
INFO:mirobo.vacuum_cli:Debug mode active
DEBUG:mirobo.vacuum_cli:Read stored sequence ids: {'manual_seq': 0, 'seq': 1}
DEBUG:mirobo.vacuum_cli:Connecting to 192.168.130.58 with token 6f3xxxxxxxxxxxxxxxxxxxxxxxx13790
DEBUG:mirobo.protocol:Unable to decrypt, returning raw bytes.
DEBUG:mirobo.device:Discovered 926 2690 with ts: 1970-01-01 00:06:52
DEBUG:mirobo.device:192.168.130.58:54321 >>: {'id': 2, 'method': 'miIO.info', 'params': []}
DEBUG:mirobo.device:192.168.130.58:54321 (ts: 1970-01-01 00:06:52, id: 2) << {'id': 2, 'result': {'fw_ver': '1.3.0_0032', 'token': '6f3cxxxxxxxxxxxxxxxxxxxxxxxx3790', 'hw_ver': 'ESP8266', 'model': 'philips.light.bulb', 'life': 412, 'ap': {'rssi': -61, 'bssid': 'C0:25:06:A8:9A:5E', 'ssid': 'Greendale'}, 'mac': '34:CE:FF:FF:FF:FF', 'wifi_fw_ver': '1.5.0-dev(7f7a714)', 'mmfree': 5744, 'netif': {'gw': '192.168.130.1', 'mask': '255.255.255.0', 'localIp': '192.168.130.58'}}}
philips.light.bulb v1.3.0_0032 (34:CE:FF:FF:FF:FF) @ 192.168.130.58 - token: 6f3cxxxxxxxxxxxxxxxxxxxxxxxx3790
DEBUG:mirobo.vacuum_cli:Full response: {'ap': {'bssid': 'C0:25:06:A8:9A:5E', 'rssi': -61, 'ssid': 'Greendale'},
 'fw_ver': '1.3.0_0032',
 'hw_ver': 'ESP8266',
 'life': 412,
 'mac': '34:CE:FF:FF:FF:FF',
 'mmfree': 5744,
 'model': 'philips.light.bulb',
 'netif': {'gw': '192.168.130.1',
           'localIp': '192.168.130.58',
           'mask': '255.255.255.0'},
 'token': '6f3cxxxxxxxxxxxxxxxxxxxxxxxx3790',
 'wifi_fw_ver': '1.5.0-dev(7f7a714)'}

syssi · 2017-08-14T05:31:22Z

I would be happy if this PR will be merged. Thanks in advance! :-)

rytilahti · 2017-08-14T06:08:22Z

@syssi did you test it out? If everything is working I think we can just merge all the pending PRs as they are, and refactor the client tool(s) afterwards. The information necessary is in the PRs anyway, so if you say they're good to go.

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

mirobo/ceil.py

+        """Auto CCT Off."""
+        return self.send("enable_ac", [0])
+
+    def status(self):


This could be set with typing to return CeilStatus, or is that deliberately left out to avoid moving it on top of this class?

@kuduka Could to change this line to def status(self): -> CeilStatus and move the class CeilStatus to the top of the file?

I've fixed but I think that I did a merge that wasn't needed on this PR, bare with me as I'm not a developer.

If I did something wrong, please let me know so I could improve it :)

syssi · 2017-08-14T06:27:51Z

I was lazy and waiting for the merge. I will test it tonight.

syssi · 2017-08-14T08:35:37Z

Just tested. Works fine! 👍

houndci-bot · 2017-08-14T10:41:35Z

mirobo/ceil.py

+            properties
+        )
+        return CeilStatus(dict(zip(properties, values)))
+


blank line at end of file

houndci-bot · 2017-08-14T10:41:36Z

mirobo/ceil.py

+             self.bl, self.mb, self.ac)
+        return s
+
+class Ceil(Device):


expected 2 blank lines, found 1

houndci-bot · 2017-08-14T10:41:36Z

mirobo/ceil.py

+            properties
+        )
+        return CeilStatus(dict(zip(properties, values)))
+


blank line at end of file

houndci-bot · 2017-08-14T10:41:36Z

mirobo/ceil.py

+             self.bl, self.mb, self.ac)
+        return s
+
+class Ceil(Device):


expected 2 blank lines, found 1

syssi · 2017-08-14T12:22:23Z

Did you something mix up with the color temperature? At "get_prop" you request "cct" and "sw". At the CeilStatus you are working with cctsw. My philips led ball just provides a value for "cct".

kuduka · 2017-08-14T12:40:15Z

cctsw is the property of "Adjust Scenes with Wall Switch" functionality that's not implemented yet on this code.

syssi · 2017-08-14T12:50:53Z

Got it! Does your lamp provide a value for "cct" (color_temp)?

mirobo --ip addr --token token raw_command get_prop "['cct']"

It should be a value between 1 and 100. Which can be set by set_cct. Please provide this information at CeilStatus if available for your device.

kuduka · 2017-08-14T14:04:11Z

Yes, it does, so I've added it:

DEBUG:mirobo.device:192.168.1.227:54321 >>: {'id': 2, 'method': 'set_cct', 'params': [25]}
DEBUG:mirobo.device:192.168.1.227:54321 (ts: 1970-01-01 00:20:18, id: 2) << {'result': ['ok'], 'id': 2}

Sending cmd get_prop with params ['cct']
[25]

Also I've removed unused properties as I've just discovered that there's some kind of issue getting all properties at once, for example:

cctsw param if queried with sw it gets different results

# mirobo raw_command get_prop "'power', 'bl', 'cctsw'"
Sending cmd get_prop with params ('power', 'bl', 'cctsw')
['on', 1, [[0, 3], [0, 2], [0, 1]]]

# mirobo raw_command get_prop "'power', 'bl', 'sw'"
Sending cmd get_prop with params ('power', 'bl', 'sw')
['on', 1, [[0, 3], [0, 2], [0, 1]]]

# mirobo raw_command get_prop "'power', 'bl', 'cctsw', 'sw'"
Sending cmd get_prop with params ('power', 'bl', 'sw', 'cctsw')
['on', 1, [[0, 3], [0, 2], [0, 1]], 1]

If I query more than 8 params at once, last one gets overwrited or it disappears:

Sending cmd get_prop with params ('power', 'bl', 'ac', 'mb', 'ms', 'snm', 'bright', 'cct')
['on', 1, 0, 1, 1, 0, 100, 100]

Sending cmd get_prop with params ('power', 'bl', 'ac', 'mb', 'ms', 'snm', 'bright', 'cct', 'sw')
['on', 1, 0, 1, 1, 0, 100, [[0, 3], [0, 2], [0, 1]]]

Sending cmd get_prop with params ('power', 'bl', 'ac', 'mb', 'ms', 'sw', 'bright', 'cct')
['on', 1, 0, 1, 1, [[0, 3], [0, 2], [0, 1]], 100, 100]

Sending cmd get_prop with params ('power', 'bl', 'ac', 'mb', 'ms', 'sw', 'bright', 'cct', 'snm')
['on', 1, 0, 1, 1, [[0, 3], [0, 2], [0, 1]], 100, 100]

rytilahti · 2017-08-14T20:24:51Z

Thanks a lot again @kuduka! Btw, in PRs it is better to use rebase than merge when rebasing after changes in the master:

git pull
<a merge commit gets created>
git rebase -i
<the log is without merge commits again>

or alternatively depending on your setup:

git fetch upstream/master
git rebase upstream/master

This will allow keeping the history cleaner. It's not a big deal though, I'll merge this one too.

syssi · 2017-08-14T20:32:39Z

Awesome! Please bump the version number.

rytilahti · 2017-08-14T22:03:12Z

Could you do that & update the README.md to have a mention about other supported devices? I'm hesitating bumping the version before making the README up to date, but do not have currently time to make that happen (I'd like to fix the cli tool issue, but that may need to wait).

syssi · 2017-08-15T05:16:09Z

I will prepare a PR!

Added support for Xiaomi Philips LED Ceiling Lamp

e1ad4e8

houndci-bot reviewed Jul 30, 2017

View reviewed changes

[FIX] houndci-bot errors

0c8ba3b

houndci-bot reviewed Jul 30, 2017

View reviewed changes

mirobo/ceil.py

from .device import Device

from typing import Any, Dict

class Ceil(Device):

Copy link
Copy Markdown

houndci-bot Jul 30, 2017

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

[FIX] houndci-bot errors second time

81638cf

syssi added a commit to syssi/python-miio that referenced this pull request Aug 14, 2017

Pull request rytilahti#35 merged for testing.

bf4c7dd

rytilahti approved these changes Aug 14, 2017

View reviewed changes

kuduka and others added 3 commits August 14, 2017 11:45

Merge branch 'master' into ceil

a398528

[FIX] Relocation of CeilStatus and redefine status func

08ff67e

Merge branch 'ceil' of github.com:kuduka/python-mirobo into ceil

2a86ecf

houndci-bot reviewed Aug 14, 2017

View reviewed changes

[FIX] Hound errors

e6bbc81

[FIX] Removed unused properties and added CCT one

eb2a6cb

rytilahti merged commit 96b7125 into rytilahti:master Aug 14, 2017

kuduka mentioned this pull request Aug 30, 2017

Xiaomi Ceiling Lamp: Some refactoring and fault tolerance if a philips light ball is used #45

Merged

Uh oh!

Conversation

kuduka commented Jul 30, 2017

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rytilahti commented Aug 1, 2017

Uh oh!

kuduka commented Aug 1, 2017

Uh oh!

syssi commented Aug 6, 2017

Uh oh!

syssi commented Aug 14, 2017

Uh oh!

rytilahti commented Aug 14, 2017

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 Aug 14, 2017

Uh oh!

syssi commented Aug 14, 2017

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!

syssi commented Aug 14, 2017

Uh oh!

kuduka commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

syssi commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kuduka commented Aug 14, 2017

Uh oh!

rytilahti commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

syssi commented Aug 14, 2017

Uh oh!

rytilahti commented Aug 14, 2017

Uh oh!

syssi commented Aug 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

kuduka commented Aug 14, 2017 •

edited

Loading

syssi commented Aug 14, 2017 •

edited

Loading

rytilahti commented Aug 14, 2017 •

edited

Loading