Skip to content
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

Pilight binary sensor components #6774

Merged
merged 18 commits into from May 3, 2017

Conversation

Projects
None yet
5 participants
@zeltom
Copy link
Contributor

commented Mar 24, 2017

Pilight binary sensor components.

Description:

This component implement the pilight hub binary sensor functionality.
Two type of pilight binary sensor configuration available. A normal sensor which send the on and off state cyclicaly and a trigger sensor which send only a trigger when an event happend (for example lots of cheap PIR motion detector) (see example configuration below).

Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#2325

Example entry for configuration.yaml:

binary_sensor:
  - platform: pilight
    name: 'Motion'
    variable: 'state'
    payload:
      unitcode: 371399
    payload_on: 'closed'
    disarm_after_trigger: True  <-- use this if you have trigger type sensor

Checklist:

  • Documentation added/updated in home-assistant.github.io

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

  • New dependencies have been added to the REQUIREMENTS variable (example).

  • New dependencies are only imported inside functions that use them (example).

  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

  • New files were added to .coveragerc.

Add files via upload
Pilight binary sensor components.
@homeassistant

This comment has been minimized.

Copy link

commented Mar 24, 2017

Hi @zeltom,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

except KeyError:
_LOGGER.error(
'No variable %s in received code data %s',
str(self._variable), str(call.data))

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

trailing whitespace

def _reset_state(self, call):
self._state = False
self.schedule_update_ha_state()

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

blank line contains whitespace

self._reset_delay_sec = rst_dly_sec
self._delay_after = None
self._hass = hass

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

blank line contains whitespace

class PilightTriggerSensor(BinarySensorDevice):
"""Representation of a binary sensor that can be updated using Pilight."""

def __init__(self, hass, name, variable, payload, on_value, off_value, rst_dly_sec=30):

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

line too long (91 > 79 characters)

'No variable %s in received code data %s',
str(self._variable), str(call.data))

class PilightTriggerSensor(BinarySensorDevice):

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

expected 2 blank lines, found 1

add_devices([PilightBinarySensor(
hass=hass,
name=config.get(CONF_NAME),
variable=config.get(CONF_VARIABLE),

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

indentation contains tabs
indentation contains mixed spaces and tabs

else:
add_devices([PilightBinarySensor(
hass=hass,
name=config.get(CONF_NAME),

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

indentation contains tabs
indentation contains mixed spaces and tabs

)])
else:
add_devices([PilightBinarySensor(
hass=hass,

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

indentation contains tabs
indentation contains mixed spaces and tabs

off_value=config.get(CONF_PAYLOAD_OFF),
)])
else:
add_devices([PilightBinarySensor(

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

indentation contains tabs
indentation contains mixed spaces and tabs

on_value=config.get(CONF_PAYLOAD_ON),
off_value=config.get(CONF_PAYLOAD_OFF),
)])
else:

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

trailing whitespace

@zeltom

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2017

Starting to fix asap.

from homeassistant.components.binary_sensor import PLATFORM_SCHEMA
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.event import track_point_in_time
from homeassistant.components.binary_sensor import (

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

'homeassistant.components.binary_sensor.DOMAIN' imported but unused
'homeassistant.components.binary_sensor.DEVICE_CLASSES' imported but unused

STATE_ON,
STATE_OFF)
from homeassistant.components.binary_sensor import PLATFORM_SCHEMA
from homeassistant.helpers.entity import Entity

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

'homeassistant.helpers.entity.Entity' imported but unused


import voluptuous as vol

from homeassistant.const import (

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 24, 2017

'homeassistant.const.STATE_UNKNOWN' imported but unused

zeltom added some commits Mar 24, 2017

@balloobbot balloobbot added the platform label Mar 24, 2017

zeltom added some commits Mar 25, 2017

try:
value = call.data[self._variable]
self._state = (value == self._on_value)
if self._delay_after == None:

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 26, 2017

comparison to None should be 'if cond is None:'

zeltom added some commits Mar 26, 2017

@balloob
Copy link
Member

left a comment

First my apologies for not getting to code review this PR earlier. I will be sure to be swift in responding to followups on this PR.

Found some things that need updating, see comments.

.coveragerc Outdated
@@ -159,6 +159,7 @@ omit =
homeassistant/components/binary_sensor/hikvision.py
homeassistant/components/binary_sensor/iss.py
homeassistant/components/binary_sensor/rest.py
homeassistant/components/binary_sensor/pilight.py

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

Please insert alphabetically.

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

Done.

vol.Required(CONF_VARIABLE): cv.string,
vol.Required(CONF_PAYLOAD): vol.Schema(dict),
vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
vol.Optional(CONF_PAYLOAD_ON, default='on'): cv.string,

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

These three need to be cv.boolean.

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

The 'CONF_DISARM_AFTER_TRIGGER' is really a cv.boolean, but 'CONF_PAYLOAD_ON' can be any string (e.g. "opened", "high", etc.) so I think it's work only with string, because at state generation the 'self._state = (value == self._on_value)' make a string comparison.

return self._state

@property
def state(self):

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

You don't have to overwrite state, that's handled by the binary sensor base class.

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

Ok. (The hell of Ctrl+C Ctrl+V :)

# Check if received code matches defined playoad
# True if payload is contained in received code dict, not
# all items have to match
if self._payload.items() <= call.data.items():

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

What does this do ? this is not doing a proper comparison. Please specify the keys that are compared.

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

Ok changed. It's working for me but true it's a dirty code.

value = call.data[self._variable]
self._state = (value == self._on_value)
self.schedule_update_ha_state()
except KeyError:

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

You can also do a simple check: if self._variable not in call.data

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

Implemented.

except KeyError:
_LOGGER.error(
'No variable %s in received code data %s',
str(self._variable), str(call.data))

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

No need to stringify, will be done if message would be printed.

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

The try, except completly removed, because some sensor can send different variable (for example phisical state in one msg and battery state in the next msg), and logging this as error is a bad behavior.

return self._state

@property
def state(self):

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

Please remove.

This comment has been minimized.

Copy link
@zeltom

zeltom May 1, 2017

Author Contributor

Done.

# Check if received code matches defined playoad
# True if payload is contained in received code dict, not
# all items have to match
if self._payload.items() <= call.data.items():

This comment has been minimized.

Copy link
@balloob

balloob Apr 30, 2017

Member

Please update this construct too as per above comments.

@balloob

This comment has been minimized.

Copy link
Member

commented May 2, 2017

You commented to some of my comments but I don't see it fixed. Add a comment when they are addressed and I'll get a notification 👍

# Read out variable if payload ok
if payload_ok:
if self._variable not in call.data:
return

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 2, 2017

trailing whitespace

if key not in call.data:
payload_ok = False
continue
if payload[key] != call.data[key]:

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 2, 2017

undefined name 'payload'

if key not in call.data:
payload_ok = False
continue
if payload[key] != call.data[key]:

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 2, 2017

undefined name 'payload'

zeltom added some commits May 2, 2017

@zeltom

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2017

Sorry the push cannot finished sucessfully. But now...I hope yes.

@balloob

balloob approved these changes May 3, 2017

Copy link
Member

left a comment

Nice! 🐬

@balloob balloob merged commit 517bd39 into home-assistant:dev May 3, 2017

4 checks passed

cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 93.461%
Details
hound No violations found. Woof!

@balloob balloob referenced this pull request May 5, 2017

Merged

0.44 #7444

@home-assistant home-assistant locked and limited conversation to collaborators Aug 12, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.