-
-
Notifications
You must be signed in to change notification settings - Fork 566
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
Device support for the xiaomi smart wifi socket added #29
Conversation
mirobo/plug.py
Outdated
def status(self): | ||
"""Retrieve properties.""" | ||
status = self.send("get_prop", [PROPERTY_POWER, PROPERTY_TEMPERATURE, PROPERTY_CURRENT]) | ||
return {PROPERTY_POWER: status[0], PROPERTY_TEMPERATURE: status[1], PROPERTY_CURRENT: status[2]} |
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.
line too long (104 > 79 characters)
mirobo/plug.py
Outdated
|
||
def status(self): | ||
"""Retrieve properties.""" | ||
status = self.send("get_prop", [PROPERTY_POWER, PROPERTY_TEMPERATURE, PROPERTY_CURRENT]) |
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.
multiple spaces after operator
line too long (97 > 79 characters)
mirobo/plug.py
Outdated
class Plug(Device): | ||
"""Main class representing the smart wifi socket / plug.""" | ||
|
||
def __init__(self, ip: str, token: str, start_id: int = 0, debug: int = 0) -> None: |
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.
line too long (87 > 79 characters)
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 don't want to fix this. ;-)
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.
Fair enough, I think hound should complain first after 100 characters..
mirobo/plug.py
Outdated
PROPERTY_TEMPERATURE = 'temperature' | ||
PROPERTY_CURRENT = 'current' | ||
|
||
class Plug(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/plug.py
Outdated
@@ -0,0 +1,29 @@ | |||
from .device import Device, DeviceException |
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.
'.device.DeviceException' imported but unused
mirobo/plug.py
Outdated
"""Retrieve properties.""" | ||
status = self.send( | ||
"get_prop", | ||
[ PROPERTY_POWER, PROPERTY_TEMPERATURE, PROPERTY_CURRENT ] |
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.
whitespace after '['
whitespace before ']'
mirobo/plug.py
Outdated
|
||
def status(self): | ||
"""Retrieve properties.""" | ||
status = self.send( |
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.
multiple spaces after operator
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.
Looks good! Have you thought how this could look from the side of the console client?
mirobo/plug.py
Outdated
class Plug(Device): | ||
"""Main class representing the smart wifi socket / plug.""" | ||
|
||
def __init__(self, ip: str, token: str, start_id: int = 0, debug: int = 0) -> None: |
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.
Fair enough, I think hound should complain first after 100 characters..
mirobo/plug.py
Outdated
|
||
def __init__(self, ip: str, token: str, start_id: int = 0, debug: int = 0) -> None: | ||
super().__init__(ip, token, start_id, debug) | ||
self.manual_seqnum = -1 |
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.
Is the __init__
here unnecessary though, it just sets the manual_seqnum
it never uses :-)
mirobo/plug.py
Outdated
PROPERTY_POWER: status[0], | ||
PROPERTY_TEMPERATURE: status[1], | ||
PROPERTY_CURRENT: status[2] | ||
} |
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.
What type of data is returned by these? Would it make sense to wrap it inside a class like PlugStatus?
mirobo/plug.py
Outdated
|
||
def raw_command(self, cmd, params): | ||
"""Send a raw command to the device.""" | ||
return self.send(cmd, params) |
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.
Is raw_command
necessary when send()
could be used directly?
mirobo/plug.py
Outdated
from .device import Device | ||
from .containers import PlugStatus | ||
|
||
class Plug(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/containers.py
Outdated
return self.data["current"] | ||
|
||
def __str__(self) -> str: | ||
s = "<PlugStatus power=%s, temperature=%s, current=%s>" % (self.state, self.temperature, self.current) |
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.
line too long (110 > 79 characters)
mirobo/containers.py
Outdated
return self.data["current"] | ||
|
||
def __str__(self) -> str: | ||
s = "<PlugStatus power=%s, temperature=%s, " % (self.state, self.temperature) |
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.
line too long (85 > 79 characters)
I have no idea honestly. Just provide a second CLI called "plug"? |
mirobo/containers.py
Outdated
s = "<PlugStatus power=%s, " % (self.state) | ||
s += "temperature=%s, " % (self.temperature) | ||
s += "current=%s>" % (self.current) | ||
return s |
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 think:
s = "<PlugStatus power=%s, temperature=%s, current=%s>" % (self.state,
self.temperature,
self.current)
Properly intended would look nicer, but this is fine too.
That, or using the same client but providing "methods" depending on the discovered device type, considering that there are quite a bit of devices which could be easily supported. I think this can be merged already now. |
mirobo/containers.py
Outdated
def __str__(self) -> str: | ||
s = "<PlugStatus power=%s, temperature=%s, current=%s>" % (self.state, | ||
self.temperature, | ||
self.current) |
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.
line too long (80 > 79 characters)
mirobo/containers.py
Outdated
|
||
def __str__(self) -> str: | ||
s = "<PlugStatus power=%s, temperature=%s, current=%s>" % (self.state, | ||
self.temperature, |
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.
line too long (84 > 79 characters)
mirobo/plug_cli.py
Outdated
import sys | ||
import json | ||
import ipaddress | ||
from typing import Any |
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.
'typing.Any' imported but unused
mirobo/plug_cli.py
Outdated
import pretty_cron | ||
import ast | ||
import sys | ||
import json |
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.
'json' imported but unused
mirobo/plug_cli.py
Outdated
# -*- coding: UTF-8 -*- | ||
import logging | ||
import click | ||
import pretty_cron |
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.
'pretty_cron' imported but unused
Thanks, maybe in the future it makes sense to just extend one tool to cover all the devices, but the cli solution shall work for now :-) |
|
No description provided.