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

WIP: Yi Home Cameras #9743

Closed
wants to merge 50 commits into from
Closed

WIP: Yi Home Cameras #9743

wants to merge 50 commits into from

Conversation

bachya
Copy link
Contributor

@bachya bachya commented Oct 8, 2017

Description:

Adds support for YI Home Cameras running the alternative firmware for Xiaomi Cameras based on Hi3518e Chipset. Awaiting some underlying library changes before pressing forward.

Related issue (if applicable): N/A

Pull request in home-assistant.github.io with documentation (if applicable): TBD

Example entry for configuration.yaml (if applicable):

camera:
  - platform: yi
    name: Kitchen Camera
    host: '192.168.1.100'
    username: root
    password: my_password_123
    ffmpeg_arguments: '-vf scale=800:450'

Checklist:

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.

@@ -0,0 +1,144 @@
"""
This component provides basic support for Xiaomi Cameras (HiSilicon Hi3518e V200).

Choose a reason for hiding this comment

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

line too long (82 > 79 characters)

@@ -0,0 +1,144 @@
"""
This component provides basic support for Xiaomi Cameras (HiSilicon Hi3518e V200).

Choose a reason for hiding this comment

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

line too long (82 > 79 characters)

"""Retrieve the latest video file from the customized Yi FTP server."""
from aioftp import ClientSession

async with ClientSession(self.host, 21, self.user,
Copy link
Member

Choose a reason for hiding this comment

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

We are still supporting Python 3.4 till the beginning of 2018. This means that we do not support async and await keywords. Use yield from / @asyncio.coroutine

Copy link
Member

Choose a reason for hiding this comment

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

That also means that aioftp cannot be used as it requires a minimum of Python 3.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. After considering it, I’m okay backing down to ftplib; will implement soon.

self.passwd) as client:
dirs = [
p
for p, i in sorted(
Copy link
Member

Choose a reason for hiding this comment

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

This is unreadable, please just write it as a normal for loop

balloob and others added 6 commits October 8, 2017 09:26
* Make Arlo battery_level icon dynamic

* makes lint happy
…i changes (home-assistant#9454)

* bump the pyhs100 version and fix api changes

* switch.tplink: avoid I/O during __init__

* initialize _name to None in __init__

* update requirements_all.txt for the new version
* Add Helpers bind_hass functionality

* Update other helpers
@bachya
Copy link
Contributor Author

bachya commented Oct 8, 2017

@balloob: I've rewritten this using ftplib. One area of concern I have has to do with image retrieval. Since I'm a relative newbie to async programming, I'd appreciate your input.

I'm using @pvizeli's ha-ffmpeg library to process the MP4; since his method to retrieve an image from a stream is a coroutine, I imagine I need the async_camera_image method (instead of camera_image). Unless I'm mistaken, doesn't line 122 represent IO going on inside the event loop?

I'm using this as a custom component in my HASS install and everything appears fine (camera is updating, no timer desynchronization, etc.), but would love to be sure.

EDIT: Just saw my first timer desynchronization on a fresh install with nothing but this platform running.

EDIT 2: I think I figured out the async_add_job method; giving that a shot.

bachya and others added 6 commits October 8, 2017 10:54
* New Skybell platform with components

* Added skybell components to omit.

* Preemptively fixing lint issues (hopefully).

* Removed unused variable.

* Requested changes.

* Additional CRs

* Hopefully the last of the CR's!
zabuldon and others added 5 commits October 9, 2017 14:38
* Tesla bug fixes.

* Added myself to CODEOWNERS for tesla.
There is an off by one error that causes period exceptions. Fix this.
* Added is_closed

* whitespaces --

* removed whitespace
* Support for Wink Dome siren/chimes
This fixes some bugs with interfacing with yamaha receivers, including
closing bug home-assistant#5209.
@bachya bachya requested a review from a team as a code owner October 9, 2017 18:23
@bachya
Copy link
Contributor Author

bachya commented Oct 9, 2017

Whoa – not sure what happened with that rebase. Going to close this out and submit a new PR (with the latest code).

@bachya bachya closed this Oct 9, 2017
@bachya bachya deleted the yi-camera branch October 9, 2017 19:57
@bachya bachya mentioned this pull request Oct 9, 2017
6 tasks
@home-assistant home-assistant locked and limited conversation to collaborators Mar 3, 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.