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 Loxone plattform #9706
Added Loxone plattform #9706
Conversation
homeassistant/components/loxone.py
Outdated
fields[1] + "-" + \ | ||
fields[2] + "-" + \ | ||
fields[3] + \ | ||
fields[4] |
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 over-indented for visual indent
homeassistant/components/loxone.py
Outdated
uuidstr = fields[0] + "-" + \ | ||
fields[1] + "-" + \ | ||
fields[2] + "-" + \ | ||
fields[3] + \ |
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 over-indented for visual indent
homeassistant/components/loxone.py
Outdated
fields = event_uuid.urn.replace("urn:uuid:", "").split("-") | ||
uuidstr = fields[0] + "-" + \ | ||
fields[1] + "-" + \ | ||
fields[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.
continuation line over-indented for visual indent
homeassistant/components/loxone.py
Outdated
event_uuid = uuid.UUID(bytes_le=packet[0:16]) | ||
fields = event_uuid.urn.replace("urn:uuid:", "").split("-") | ||
uuidstr = fields[0] + "-" + \ | ||
fields[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.
continuation line over-indented for visual indent
homeassistant/components/loxone.py
Outdated
_LOGGER.debug(parsed_data) | ||
self._hass.bus.async_fire(EVENT, parsed_data) | ||
|
||
@asyncio.coroutine |
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
homeassistant/components/loxone.py
Outdated
try: | ||
if not self._ws: | ||
self._ws = yield from wslib.connect( | ||
"ws://{}:{}/ws/rfc6455".format(self._host, self._port), timeout=5) |
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 (86 > 79 characters)
homeassistant/components/loxone.py
Outdated
:param device_uuid: Device id of a sensor or input | ||
:param value: Value to be sent | ||
""" | ||
yield from self._ws.send("jdev/sps/io/{}/{}".format(device_uuid, value)) |
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)
homeassistant/components/loxone.py
Outdated
device_uuid = call.data.get(ATTR_UUID, DEFAULT) | ||
yield from api.send_websocket_command(device_uuid, value) | ||
|
||
hass.services.async_register(DOMAIN, 'event_websocket_command', handle_websocket_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.
line too long (93 > 79 characters)
homeassistant/components/loxone.py
Outdated
data = {device_uuid: value} | ||
hass.bus.async_fire(EVENT, data) | ||
|
||
hass.services.async_register(DOMAIN, 'event_bus_command', handle_event_bus_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.
line too long (87 > 79 characters)
homeassistant/components/loxone.py
Outdated
|
||
import voluptuous as vol | ||
import homeassistant.helpers.config_validation as cv | ||
from homeassistant.const import (CONF_HOST, CONF_PORT, CONF_USERNAME, CONF_PASSWORD) |
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)
homeassistant/components/loxone.py
Outdated
event_uuid = uuid.UUID(bytes_le=packet[0:16]) | ||
fields = event_uuid.urn.replace("urn:uuid:", "").split("-") | ||
uuidstr = fields[0] + "-" + fields[1] + "-" + fields[2] + "-" + \ | ||
fields[3] + fields[4] |
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 over-indented for visual indent
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.
String formatting -> "".format()
Hello everybody, can somebody help me to fix this errors? I think most of them are code styling problems. Thanks for your help. Jojo |
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.
Just a couple of quick comments.
|
||
def __init__(self, name, uuid, sensor_typ, unit_of_measurement): | ||
"""Initialize the sensor.""" | ||
self._state = 0.0 |
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.
Set it to None
and let Home Assistant handle the rest.
EVENT = 'loxone_received' | ||
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
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.
Configuration validation is missing -> https://home-assistant.io/developers/code_review_platform/#3-configuration
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.
Done
Loxone simple sensor | ||
|
||
For more details about this platform, please refer to the documentation at | ||
https://home-assistant.io/components/loxone/ |
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.
Needs to point to the sensor documentation.
homeassistant/components/loxone.py
Outdated
event_uuid = uuid.UUID(bytes_le=packet[0:16]) | ||
fields = event_uuid.urn.replace("urn:uuid:", "").split("-") | ||
uuidstr = fields[0] + "-" + fields[1] + "-" + fields[2] + "-" + \ | ||
fields[3] + fields[4] |
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.
String formatting -> "".format()
homeassistant/components/loxone.py
Outdated
""" | ||
key_dict = json.loads(key) | ||
key_value = key_dict['LL']['value'] | ||
data = username + ":" + password |
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.
String formatting.
homeassistant/components/loxone.py
Outdated
:param user: Username to communicate with the Miniserver | ||
:param password: Password of the user | ||
:param host: Address where the Loxone Miniserver runs | ||
:param port: Port of the Loxone Miniserver on the host |
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.
Please replace the parameter with a simple docstring.
homeassistant/components/loxone.py
Outdated
Main class for the communication with the miniserver | ||
""" | ||
|
||
# pylint: disable=too-many-arguments, too-many-instance-attributes |
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.
Those are already disabled globally.
homeassistant/components/loxone.py
Outdated
""" | ||
Loxone Gateway | ||
Main class for the communication with the miniserver | ||
""" |
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.
Please go for an one-line docstring.
homeassistant/components/loxone.py
Outdated
_LOGGER = logging.getLogger(__name__) | ||
|
||
DOMAIN = 'loxone' | ||
DEFAULT_HOST = '127.0.0.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.
This doesn't make much sense. Or isn't the Miniserver an independent hardware unit only?
homeassistant/components/loxone.py
Outdated
CONF_PASSWORD) | ||
from homeassistant.helpers.event import async_track_time_interval | ||
|
||
REQUIREMENTS = ['websockets==3.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.
If needed then you could go with websockets 3.4. I guess that we haven't update the requirement because it's not heavily used.
https://home-assistant.io/developers/website/ Please check the list in the PR template. |
homeassistant/components/loxone.py
Outdated
ATTR_UUID = 'uuid' | ||
ATTR_VALUE = 'value' | ||
|
||
@asyncio.coroutine |
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
Hello fabaff, can you help with the documentation and the errors for the "The Travis CI build"? Thanks in advance. Jojo |
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.
You need to pull the latest changes dev
into your local branch and the run gen_requirements_all.py
Again, the checklist in the PR template is there to help.
CONF_SENSOR_TYP = 'sensortyp' | ||
CONF_UUID = 'uuid' | ||
|
||
CONFIG_SCHEMA = vol.Schema({ |
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.
You have to extend the sensor validation. Get the details from the link I provided in the previous comment about the configuration validation.
homeassistant/components/loxone.py
Outdated
CONF_PASSWORD) | ||
from homeassistant.helpers.event import async_track_time_interval | ||
|
||
REQUIREMENTS = ['websockets==3.4'] |
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.
You need to update the requirement of the other platform that uses websockets
as well.
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 my biggest problem is that I do not know how to pull the latest changes from dev.
I tried it without a gui for github but I did not get it to work. Now I tried GitKraken. I fixed all of
the pylint issues. Maybe it is better you delete my pull request and I delete my fork. And I first try to get used with github and git.
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.
And also I have big problems with the documentation page. But this is also a problem because I do not understand the github way to do it. I work personally with subversion and this is much easier for me. I believe that git is better but also more complicated.
homeassistant/components/loxone.py
Outdated
password = config[DOMAIN][CONF_PASSWORD] | ||
host = config[DOMAIN][CONF_HOST] | ||
port = config[DOMAIN][CONF_PORT] | ||
api = LoxoneGateway(hass, user, password, host, port) |
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.
Will there be no exception if something is worn with the connection?
homeassistant/components/loxone.py
Outdated
|
||
|
||
class LoxoneGateway: | ||
""" Main class for the communication with the miniserver """ |
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.
homeassistant/components/loxone.py
Outdated
event_uuid = uuid.UUID(bytes_le=packet[0:16]) | ||
fields = event_uuid.urn.replace("urn:uuid:", "").split("-") | ||
uuidstr = "{}-{}-{}-{}{}".format(fields[0], fields[1], fields[2], | ||
fields[3], fields[4]) |
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 over-indented for visual indent
homeassistant/components/loxone.py
Outdated
event_uuid = uuid.UUID(bytes_le=packet[0:16]) | ||
fields = event_uuid.urn.replace("urn:uuid:", "").split("-") | ||
uuidstr = "{}-{}-{}-{}{}".format(fields[0], fields[1], fields[2], | ||
fields[3], fields[4]) |
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 over-indented for visual indent
homeassistant/components/loxone.py
Outdated
For more details about this component, please refer to the documentation at | ||
https://home-assistant.io/components/loxone/ | ||
""" | ||
# pylint: disable=unused-import, too-many-lines |
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.
Do not disable pylint
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.
Please add yourself as owner in https://github.com/home-assistant/home-assistant/blob/dev/CODEOWNERS for the new files.
The requirements files seem out of sync.
Please exclude |
Hello fabaff, Thanks. |
I guess that you can close the PR by yourself. Please consider to move forward with this PR as you are close to the finish line. |
@JoDehli You sound frustrated but just to say I really do hope you persevere and successfully commit this into Home Assistant. I have a couple of Loxone servers and am really interested in this capability. |
Sorry but I deleted my repository. I switch to Node-Red. There you can find a good binding for the Loxone. |
Description:
First real integration of the Loxone Miniserver.
Example entry for
configuration.yaml
(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).script/gen_requirements_all.py
.