-
-
Notifications
You must be signed in to change notification settings - Fork 569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for Xiaomi Philips LED Ceiling Lamp #35
Conversation
mirobo/ceil_cli.py
Outdated
"""Auto CCT on.""" | ||
click.echo("Auto CCT On: %s" % dev.ac_on()) | ||
|
||
@cli.command() |
There was a problem hiding this comment.
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
mirobo/ceil_cli.py
Outdated
"""Set delay off in seconds.""" | ||
click.echo("Delay off: %s" % dev.delay_off(seconds)) | ||
|
||
@cli.command() |
There was a problem hiding this comment.
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
mirobo/ceil_cli.py
Outdated
click.echo("Auto On/Off When Mi Band is nearby: %s" % res.mb) | ||
click.echo("Auto CCT: %s" % res.ac) | ||
|
||
@cli.command() |
There was a problem hiding this comment.
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
mirobo/ceil_cli.py
Outdated
"""Search for plugs in the network.""" | ||
mirobo.Ceil.discover() | ||
|
||
@cli.command() |
There was a problem hiding this comment.
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
mirobo/ceil_cli.py
Outdated
if ctx.invoked_subcommand is None: | ||
ctx.invoke(status) | ||
|
||
@cli.command() |
There was a problem hiding this comment.
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
mirobo/ceil.py
Outdated
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trailing whitespace
mirobo/ceil.py
Outdated
) | ||
return CeilStatus(dict(zip(properties, values))) | ||
|
||
class CeilStatus: |
There was a problem hiding this comment.
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
mirobo/ceil.py
Outdated
def status(self): | ||
"""Retrieve properties.""" | ||
properties = ['power', 'bright', 'snm', 'dv', 'cct' | ||
'sw', 'bl', 'mb', 'ac', 'ms', ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation line under-indented for visual indent
mirobo/ceil.py
Outdated
def set_cct(self, level: int): | ||
"""Set Correlated Color Temperature.""" | ||
return self.send("set_cct", [level]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line contains whitespace
mirobo/ceil.py
Outdated
class Ceil(Device): | ||
"""Main class representing Xiaomi Philips LED Ceiling Lamp.""" | ||
|
||
#TODO: - Auto On/Off Not Supported |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
block comment should start with '# '
mirobo/ceil.py
Outdated
from .device import Device | ||
from typing import Any, Dict | ||
|
||
class Ceil(Device): |
There was a problem hiding this comment.
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
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 |
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.
|
The xiaomi philips led ball (9290012800, 450lm, 3000K-5700K, 6.5W, E27) supports the same feature set:
|
I would be happy if this PR will be merged. Thanks in advance! :-) |
@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. |
mirobo/ceil.py
Outdated
"""Auto CCT Off.""" | ||
return self.send("enable_ac", [0]) | ||
|
||
def status(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be set with typing to return CeilStatus
, or is that deliberately left out to avoid moving it on top of this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kuduka Could to change this line to def status(self): -> CeilStatus
and move the class CeilStatus to the top of the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :)
I was lazy and waiting for the merge. I will test it tonight. |
Just tested. Works fine! 👍 |
mirobo/ceil.py
Outdated
properties | ||
) | ||
return CeilStatus(dict(zip(properties, values))) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line at end of file
mirobo/ceil.py
Outdated
self.bl, self.mb, self.ac) | ||
return s | ||
|
||
class Ceil(Device): |
There was a problem hiding this comment.
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
mirobo/ceil.py
Outdated
properties | ||
) | ||
return CeilStatus(dict(zip(properties, values))) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line at end of file
mirobo/ceil.py
Outdated
self.bl, self.mb, self.ac) | ||
return s | ||
|
||
class Ceil(Device): |
There was a problem hiding this comment.
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
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". |
cctsw is the property of "Adjust Scenes with Wall Switch" functionality that's not implemented yet on this code. |
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. |
Yes, it does, so I've added it:
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:
|
Thanks a lot again @kuduka! Btw, in PRs it is better to use rebase than merge when rebasing after changes in the master:
or alternatively depending on your setup:
This will allow keeping the history cleaner. It's not a big deal though, I'll merge this one too. |
Awesome! Please bump the version number. |
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). |
I will prepare a PR! |
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: