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

Create a binary_sensor using the gpiozero library #12105

Closed
wants to merge 1 commit into from

Conversation

maihde
Copy link

@maihde maihde commented Feb 1, 2018

Description:

The gpiozero library provides a unified interface for both local GPIO
pins and remote GPIO pins (via pigpiod). This allows it to replace
the standard rpi_gpio component while also adding new capabilities to
easily interact with GPIO pins on networked Raspberry Pis.

I chose to use pigpiod over other alternatives because it is the de-facto standard that is installed via raspi-config. Until this issue is resolved in pigpio there is a possibility for the component to enter a deadlock state if network connectivity between Home Assistant and pigpiod is lost. I have provided a pull request to address this issue; as a workaround my fork of pigpio can be installed instead.

I chose to use gpiozero instead of directly using the pigpio library so that one unified component could interact with both local and remote ports, reducing duplicate code.

**Documentation has been started (here)[https://github.com/maihde/hass-rpi-gpiozero] and will be migrated to home-assistant.io when this component is accepted.

Example entry for configuration.yaml (if applicable):

binary_sensor:
  - platform: rpi_gpiozero
    ports:
      12: Local Input
  - platform: rpi_gpiozero
    host: 192.168.3.76
    ports:
      12: Remote Input 1
      13: Remote Input 2

Checklist:

  • The code change is tested and works locally.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • 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.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@homeassistant
Copy link
Contributor

Hi @maihde,

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!

btn = Button(
port,
pull_up=(pull_mode.upper() == 'UP'),
bounce_time=float(bouncetime) / 1e3,

Choose a reason for hiding this comment

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

trailing whitespace

raise ValueError("invalid bouncetime %s", bouncetime)

pin_factory = get_pinfactory(hostport)
if pin_factory == None:

Choose a reason for hiding this comment

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

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


if pull_mode.upper() not in ('UP', 'DOWN'):
raise ValueError("invalid pull_mode %s", pull_mode)

Choose a reason for hiding this comment

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

blank line contains whitespace

:param hostport: the remote host/port, None for local.
"""
from gpiozero import Button

Choose a reason for hiding this comment

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

blank line contains whitespace

pin_factory = _LOCAL_FACTORY
return pin_factory

def setup_button(port, pull_mode, bouncetime, hostport):

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

hass.bus.listen_once(EVENT_HOMEASSISTANT_START, prepare_gpiozero)
return True

def close_remote_pinfactory(hostport):

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

def cleanup_gpiozero(event):
"""Stuff to do before stopping."""
for dev in _DEVICES:
try:

Choose a reason for hiding this comment

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

trailing whitespace

# Make the default pin factory 'mock' so that
# it other pin factories can be loaded after import
os.environ['GPIOZERO_PIN_FACTORY'] = 'mock'
import gpiozero

Choose a reason for hiding this comment

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

'gpiozero' imported but unused

_LOCAL_FACTORY = None

# pylint: disable=no-member
def setup(hass, config):

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

self._state = False # if would be preferable to use None here

_LOGGER.info("%s has been updated to state %s",
self._name, self._state)

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

@homeassistant
Copy link
Contributor

Hi @maihde,

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!

_LOGGER.exception("%s has failed to update", self._name)
self._reset()
else:
self._state = False # if would be preferable to use None here

Choose a reason for hiding this comment

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

at least two spaces before inline comment


@property
def available(self):
return self.btn != None

Choose a reason for hiding this comment

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

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

"""Read state from GPIO."""
self._state = device.is_pressed
_LOGGER.info("%s has changed to %s",
self._name, self._state)

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


if self._btn is None:
_LOGGER.error("failed to create button %s on port %s",
self._name, self._port)

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

if self._btn is None and self._hostport:

_LOGGER.debug("creating button %s on port %s",
self._name, self._port)

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

"""Represent a binary sensor that uses Raspberry Pi GPIO via gpiozero"""

def __init__(self, name, port, pull_mode, bouncetime, invert_logic,
hostport):

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

_LOGGER.exception("%s has failed to update", self._name)
self._reset()
else:
self._state = False # if would be preferable to use None here

Choose a reason for hiding this comment

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

at least two spaces before inline comment


@property
def available(self):
return self.btn != None

Choose a reason for hiding this comment

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

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

"""Read state from GPIO."""
self._state = device.is_pressed
_LOGGER.info("%s has changed to %s",
self._name, self._state)

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


if self._btn is None:
_LOGGER.error("failed to create button %s on port %s",
self._name, self._port)

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

if self._btn is None and self._hostport:

_LOGGER.debug("creating button %s on port %s",
self._name, self._port)

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

"""Represent a binary sensor that uses Raspberry Pi GPIO via gpiozero"""

def __init__(self, name, port, pull_mode, bouncetime, invert_logic,
hostport):

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

@homeassistant
Copy link
Contributor

Hi @maihde,

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!

The `gpiozero` library provides a unified interface for both local GPIO
pins and remote GPIO pins (via `pigpiod`).  This allows it to replace
the standard `rpi_gpio` component while also adding new capabilities to
easily interact with GPIO pins on networked Raspberry Pis.
@gieljnssns

This comment has been minimized.

@balloob
Copy link
Member

balloob commented May 2, 2018

Component names are based on what they control, not on the name of the lib. So if this is a replacement for rpi_gpio, you should update that component.

@balloob
Copy link
Member

balloob commented May 2, 2018

However, I'm a bit confused. rpi_gpio seems to talk to the pins, this PR just talks to a daemon ?

@maihde
Copy link
Author

maihde commented May 2, 2018 via email

@balloob
Copy link
Member

balloob commented May 2, 2018

Hmm. I wonder if we would want this at all or that we should just wait for #13876 to land so that someone can just connect Home Assistant on the remote Pi too.

I wouldn't want to replace the current implementation because it would mean extra dependencies and usage of resources for no extra gain.

@fabaff
Copy link
Member

fabaff commented Aug 7, 2018

This PR seems to gone stale. Please re-open it when you want to proceed.

@fabaff fabaff closed this Aug 7, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants