Add sighthound integration by robmarkcole · Pull Request #28824 · home-assistant/core

robmarkcole · 2019-11-17T04:56:52Z

Description:

Adds integration for person detection with Sighthound cloud. The Sighthound Developer tier (free for non-commercial use) allows 5000 requests per month. If you need more requests per month you will need to sign up for a production account (i.e. Basic or Pro account).

This integration adds an image processing entity where the state of the entity is the number of people detected in an image. For each person detected, an image_processing.person_detected event is fired. The event data includes the entity_id of the image processing entity firing the event, and the bounding box around the detected person. Note that in order to prevent accidentally using up your requets to Sighthound, by default the component will not automatically scan images, but requires you to call the image_processing.scan service e.g. using an automation triggered by motion. Alternativley, periodic scanning can be enabled by configuring a scan_interval.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11203

Example entry for `configuration.yaml` (if applicable):

image_processing:
  - platform: sighthound
    api_key: some_key
    source:
      - entity_id: camera.my_cam

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist

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

Documentation added/updated in home-assistant.io

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

The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

robmarkcole · 2019-11-17T05:44:01Z

OK CI is failing because simplehound has python_requires=">=3.7", do I need to make this dependency backwards compatible to py 36?

frenck · 2019-11-17T13:38:28Z

We currently still support 3.6, so that requires an integration to work with Python 3.6 as well. However, we are in the process of moving away from 3.6, now Python 3.8 has become stable.

IMHO, there are 2 options. Either make it Python 3.6 compatible or hold off a bit until we've officially dropped Python 3.6.

robmarkcole · 2019-11-18T18:38:10Z

Made python 3.6 compatible

robmarkcole · 2020-01-19T08:57:06Z

The commit history is a bit of a mess now, does this matter?

MartinHjelmare · 2020-01-19T10:44:33Z

History looks ok. 👍

MartinHjelmare · 2020-01-19T10:46:12Z

We should prefix the event with the integration name. Replace image_processing.

MartinHjelmare · 2020-01-19T10:50:14Z

Set should_poll property to False instead in the entity.

MartinHjelmare · 2020-01-19T10:50:52Z

Please rename add_devices to add_entities.

MartinHjelmare · 2020-01-19T10:51:27Z

Please use dict[key] for required config keys and keys with default config schema values.

MartinHjelmare · 2020-01-19T10:52:45Z

We can remove call to parent. It doesn't have an init method.

MartinHjelmare · 2020-01-19T10:54:22Z

Does this have side effects? We don't want side effects in init method.

We should check that the api key is ok before creating the entity.

Probably create the api instance before creating the entity and pass the api to the entity.

Don't we want to create a single api instance and pass the same instance to all entities?

Unfortunately the API doesn't have an endpoint to validate credentials, so the integration will raise an exception if invalid credentials only when processing is performed. One option is to perform processing of a test image to see if credentials are valid? Any other suggestions? In the meantime I will raise this with the API provider

Processing a mock image would be ok, I think.

MartinHjelmare · 2020-01-19T10:57:54Z

Camera name doesn't have to be an instance attribute since it's only used in this method. Just make it a local variable.

MartinHjelmare · 2020-01-20T09:19:40Z

If we add a test for the api test failure case, coverage should be enough.

https://codecov.io/gh/home-assistant/home-assistant/compare/9993333bf91d6a9f093d6cc52fc0229b381b86cc...0720c35dfc76d1e4bf790a6638f676cbdf34fcb6/diff#D7-44

MartinHjelmare

Looks good!

robmarkcole · 2020-01-23T05:43:20Z

Are PR merged automatically?

MartinHjelmare · 2020-01-23T08:29:41Z

No, it's done manually. Thanks for the ping. 👍

homeassistant added the cla-signed label Nov 17, 2019

probot-home-assistant Bot added has-tests new-integration integration: sighthound labels Nov 17, 2019

robmarkcole mentioned this pull request Nov 17, 2019

Add sighthound docs home-assistant/home-assistant.io#11203

Merged

2 tasks

robmarkcole requested a review from MartinHjelmare November 17, 2019 05:31

MartinHjelmare changed the title ~~Adds sighthound~~ Add sighthound Nov 17, 2019

springstan changed the title ~~Add sighthound~~ Add sighthound integration Jan 15, 2020

MartinHjelmare requested changes Jan 19, 2020

View reviewed changes

robmarkcole added 11 commits January 20, 2020 07:41

Add component files

5e0313c

Add test state

4eef45d

Adds person detected event

4ffe77e

Update CODEOWNERS

8c46cce

Updates requirements

55936c6

remove unused datetime

0a5bc0f

Bump sighthound version

ec196fd

Update CODEOWNERS

15e2fb7

Update CODEOWNERS

beff46c

Create requirements_test_all.txt

7d1957f

Address reviewer comments

0720c35

Add test for bad_api_key

6bab851

MartinHjelmare approved these changes Jan 22, 2020

View reviewed changes

MartinHjelmare merged commit c71ae09 into home-assistant:dev Jan 23, 2020

lock Bot locked and limited conversation to collaborators Jan 24, 2020

Uh oh!

Conversation

robmarkcole commented Nov 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

robmarkcole commented Nov 17, 2019

Uh oh!

frenck commented Nov 17, 2019

Uh oh!

robmarkcole commented Nov 18, 2019

Uh oh!

robmarkcole commented Jan 19, 2020

Uh oh!

MartinHjelmare commented Jan 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare commented Jan 20, 2020

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

robmarkcole commented Jan 23, 2020

Uh oh!

MartinHjelmare commented Jan 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

robmarkcole commented Nov 17, 2019 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):