Adds facebox by robmarkcole · Pull Request #14307 · home-assistant/core

robmarkcole · 2018-05-06T04:57:07Z

Replaces #14279

Description:

Adds component for face detection (number of faces) and identification (recognition of taught faces) using facebox. Run facebox with:

MB_KEY="INSERT-YOUR-KEY-HERE"

docker run -p 8080:8080 -e "MB_KEY=$MB_KEY" machinebox/facebox

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<5300>

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

image_processing:
  - platform: facebox_face_detect
    ip_address: 192.168.0.1
    port: 8080
    source:
      - entity_id: camera.local_file

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

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

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

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

New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
New dependencies are only imported inside functions that use them ([example][ex-import]).
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:

Tests have been added to verify that the new code works.

robmarkcole · 2018-05-06T04:59:14Z

@pvizeli will keep this initial release a MVP.

I don't understand why the test fails with the error, is that because I am not mocking the posted data?:
No mock address: POST http://192.168.0.1:8080/facebox/check

@dgomes @MartinHjelmare

MartinHjelmare · 2018-05-06T07:52:43Z

+import base64
+import requests
+import logging
+import voluptuous as vol


Add a blank line between standard library and 3rd party imports.

MartinHjelmare · 2018-05-06T07:53:28Z

+
+PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
+    vol.Required(CONF_IP_ADDRESS): cv.string,
+    vol.Required(CONF_PORT): cv.string,


Use cv.port.

MartinHjelmare · 2018-05-06T08:05:19Z

+https://home-assistant.io/components/image_processing.facebox_face_detect
+"""
+import base64
+import requests


This is a 3rd party library and should be grouped with those ie voluptuous.

MartinHjelmare · 2018-05-06T08:11:48Z

+            response = requests.post(
+                self._url,
+                json=self.encode_image(image),
+                timeout=30


Do we need a 30 second timeout? I suggest 9 seconds, to avoid slow update warning.

MartinHjelmare · 2018-05-06T08:14:04Z

+            self.faces = response['faces']
+
+        else:
+            self.total_faces = "Request_failed"


This would change the type of total faces from integer to string. That will make it hard to use templates for this attribute.

I suggest that we don't update the total faces if there's an error. Or set it to 0.

Or None? I don’t want to have users think a classification has been successful and that the face count is zero, when in fact the classification was unsuccessful

None is good.

MartinHjelmare · 2018-05-06T08:31:05Z

+            response['success'] = False
+
+        if response['success']:
+            self.total_faces = response['facesCount']


We shouldn't update these instance attributes directly but pass the faces and faces count through the process_faces method. That will fire an event with the face data and update the instance attributes.

Face detect and face identify components work differently (confusingly). Face detect (this Pr) have a state which is the number of faces, whilst face identify components have a state which is a recognised face. I prefer the former so have avoided using process_face. If we want this to be a face identify component I will use process_faces.

My plan was to release this PR as a face detect and recommend launching the Docker with recognition turned off, to enable fast processing. I would release a subsequent component for recognition (face identify component) - a slower classification process

We can still use process_faces. It will not fire events if there's no confidence set and no confidence attribute in the face dict. Just make sure faces and total are of correct types.

MartinHjelmare · 2018-05-06T08:32:14Z

+        else:
+            self._name = "Facebox {0}".format(
+                split_entity_id(camera_entity)[1])
+        self.total_faces = 0


This is already set in the parent class.

MartinHjelmare · 2018-05-06T08:32:44Z

+            self._name = "Facebox {0}".format(
+                split_entity_id(camera_entity)[1])
+        self.total_faces = 0
+        self.faces = []


Same as above.

MartinHjelmare · 2018-05-06T08:42:18Z

+    }
+
+
+class TestFaceboxSetup(object):


You don't need to use a class for the tests. Just define test functions that have names starting with test_ Pass in hass as parameter to the functions to get a hass instance to test with. Setup and teardown is handled by pytest.

MartinHjelmare · 2018-05-06T08:48:06Z

+
+        with requests_mock.Mocker() as mock_req:
+            url = "http://{}:{}/facebox/check".format(MOCK_IP, MOCK_PORT)
+            mock_req.get(url, text=MOCK_RESPONSE)


Use the post method on the mock object to mock a post request.

https://requests-mock.readthedocs.io/en/latest/mocker.html#methods

drop face_detect from filename and component contents as this component will handle both detection and identification

MartinHjelmare

I said something inaccurate in a previous comment about process_faces and when it will fire events. Actually it's like this: it will always fire a face detected event if self.confidence is not truthy.

I still think we should use process_faces. It should give us the proper behavior for this implementation.

MartinHjelmare · 2018-05-07T06:54:12Z

+        base64_img = base64.b64encode(image).decode('ascii')
+        return {"base64": base64_img}
+
+    def get_matched_faces(self, response):


Maybe pass in faces as that's what we're using in the method?

robmarkcole · 2018-05-07T07:09:06Z

@MartinHjelmare I agree that process_faces should be used for consistency. I think it's just the test to fix now? Btw re test using classes or def, appears most legacy tests use classes, but now the best practice is to use def? Would be good to document this kind of best practice in the docs

robmarkcole · 2018-05-07T07:34:07Z

@MartinHjelmare I've basically copied the tests from the microsoft component but am getting errors which appear to relate to asyncio - any ideas what's wrong here? I didn't get this error when using the class approach

tests/components/image_processing/test_facebox.py:43:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
homeassistant/setup.py:30: in setup_component
    async_setup_component(hass, domain, config), loop=hass.loop).result()
../../../miniconda3/lib/python3.6/concurrent/futures/_base.py:427: in result
    self._condition.wait(timeout)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Condition(<unlocked _thread.RLock object owner=0 count=0 at 0x10f8536f0>, 0)>, timeout = None

Perhaps I should follow this example and use async_setup_platform in the component?

MartinHjelmare

Yeah I'm not sure exactly why it fails with sync tests, but coroutines should work.

We should probably look into this some time. It's useful to not have to use coroutines in the tests when setting up components too.

MartinHjelmare · 2018-05-07T07:46:02Z

+    }
+
+
+def test_setup_platform(hass):


Make the tests coroutines with async def.

houndci-bot · 2018-05-07T08:05:50Z

+from homeassistant.setup import async_setup_component
+import homeassistant.components.image_processing as ip
+
+from tests.common import assert_setup_component


'tests.common.assert_setup_component' imported but unused

was mock_req.get

MartinHjelmare · 2018-05-07T08:47:09Z

+    with requests_mock.Mocker() as mock_req:
+        url = "http://{}:{}/facebox/check".format(MOCK_IP, MOCK_PORT)
+        mock_req.post(url, text=MOCK_RESPONSE)
+        ip.scan(hass, entity_id=VALID_ENTITY_ID)


Call the service directly with await hass.services.async_call(...). Since we're in a coroutine here, we have to use the hass async api consistently.

MartinHjelmare · 2018-05-07T08:47:54Z

+        url = "http://{}:{}/facebox/check".format(MOCK_IP, MOCK_PORT)
+        mock_req.post(url, text=MOCK_RESPONSE)
+        ip.scan(hass, entity_id=VALID_ENTITY_ID)
+        hass.block_till_done()


await hass.async_block_till_done()

robmarkcole · 2018-05-07T11:33:49Z

+        mock_req.post(url, text=MOCK_RESPONSE)
+        await hass.services.async_call(ip.DOMAIN,
+                                       'scan',
+                                       {'entity_id': VALID_ENTITY_ID})


@MartinHjelmare this is giving confusing error:

simplejson.errors.JSONDecodeError: Expecting value: line 3 column 12 (char 30) core.py 399 INFO Bus:Handling <Event service_executed[L]: service_call_id=4368981800-1>

robmarkcole · 2018-05-07T11:39:24Z

@MartinHjelmare any ideas about the test service call which is giving the error:

entity.py                  206 ERROR    Update for image_processing.facebox_demo_camera fails
Traceback (most recent call last):
  File "/Users/robincole/Documents/Github/home-assistant/homeassistant/helpers/entity.py", line 204, in async_update_ha_state
    yield from self.async_device_update()
  File "/Users/robincole/Documents/Github/home-assistant/homeassistant/helpers/entity.py", line 325, in async_device_update
    yield from self.async_update()
  File "/Users/robincole/Documents/Github/home-assistant/homeassistant/components/image_processing/__init__.py", line 146, in async_update
    yield from self.async_process_image(image.content)
  File "/Users/robincole/miniconda3/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/Users/robincole/Documents/Github/home-assistant/homeassistant/components/image_processing/facebox.py", line 67, in process_image
    timeout=9
  File "/Users/robincole/Documents/Github/home-assistant/.tox/py36/lib/python3.6/site-packages/requests/models.py", line 892, in json
    return complexjson.loads(self.text, **kwargs)
  File "/Users/robincole/Documents/Github/home-assistant/.tox/py36/lib/python3.6/site-packages/simplejson/__init__.py", line 518, in loads
    return _default_decoder.decode(s)
  File "/Users/robincole/Documents/Github/home-assistant/.tox/py36/lib/python3.6/site-packages/simplejson/decoder.py", line 370, in decode
    obj, end = self.raw_decode(s)
  File "/Users/robincole/Documents/Github/home-assistant/.tox/py36/lib/python3.6/site-packages/simplejson/decoder.py", line 400, in raw_decode
    return self.scan_once(s, idx=_w(s, idx).end())
simplejson.errors.JSONDecodeError: Expecting value: line 3 column 12 (char 30)
core.py                    399 INFO     Bus:Handling <Event service_executed[L]: service_call_id=4357840456-1>

@MartinHjelmare

* Add sensors for electric cars * Updates based on review of @MartinHjelmare * Fix Travis error * Another fix for Travis

* Add support for shutter contact and motion detector device * Add support for power switch devices * Add support for light switch device * Cleanup binary_switch and light platform * Update comment

* add 2 devices io:RollerShutterUnoIOComponent io:ExteriorVenetianBlindIOComponent * add 2 devices * Update tahoma.py * Fix hounci-bot violation * Fixed Travis CI build failure ./homeassistant/components/cover/tahoma.py:83:13: E125 continuation line with same indent as next logical line * Fixed Travis CI build failure E125 continuation line with same indent as next logical line * Fixed Travis CI build failure E127 continuation line over-indented for visual indent * Fix indent * Change check

* Gogogate2 - bump version Uses latest version of library which ensures commands to device are idempotent * Update requirements_all * Expose sensor temperature * update version * import attribute * Set temperature * Remove temperature attribute Removed temperature attribute until it can be re-implemented as a separate sensor. * Update ordering * Fix copy-&-paste issue

* Added solt values for siteId and probability * Update snips.py * Update test_snips.py

* Add help for conversation/process service * Add logging to debug text received when service is called * Move conversation to specific folder

…14251) * When zwave node's info is parsed remove it and re-add back. * Delay value entity if not ready * If node is ready consider it parsed even if manufacturer/product are missing. * Add annotations

* Improving icloud device tracker * Adding config validations for new values * Adding config validations for new values * Moving icloud specific setup to platform schema. Setting default in platform schema.

* Starting to add attributes * All attributes added to programs * Basic zone attributes in place * Added advanced properties for zones * Working to move common logic into component + dispatcher * We shouldn't calculate the MAC with every entity * Small fixes * Small adjustments * Owner-requested changes * Restart * Restart part 2 * Added ID attribute to each switch * Collaborator-requested changes

* Waze Travel Time: optional inclusive/exclusive filters Added optional `inc_filter` and `excl_filter' params that allow to refine the reported routes: the first is not always the best/desired. A simple case-insensitive filtering (no regular expression) is used. * fix line lenght * fix spaces * Rename var * Fix typo * Fix missing var

* Ignore NaN values for influxdb * Catch TypeError

* Add zone 3 for Onkyo media player * CR Updates * Fix travis lint errors

robmarkcole · 2018-05-09T06:12:13Z

@MartinHjelmare thanks for fixing the test, I wouldn't have guessed that solution.
I think this is ready for 3rd party review now - perhaps @balloob @pvizeli @OttoWinter

Obsolete

MartinHjelmare · 2018-05-09T06:51:29Z

The branch is whack again.

robmarkcole · 2018-05-09T07:09:14Z

Oh bugger

Adds facebox-detect

fd685ba

homeassistant added cla-signed new-platform platform: image_processing.facebox_face_detect labels May 6, 2018

MartinHjelmare previously requested changes May 6, 2018

View reviewed changes

robmarkcole changed the title ~~Adds facebox-detect~~ WIP Adds facebox May 7, 2018

Drop face_detect

4e6d676

drop face_detect from filename and component contents as this component will handle both detection and identification

homeassistant added the cla-signed label May 7, 2018

robmarkcole added 3 commits May 7, 2018 06:53

Update and rename test_facebox_face_detect.py to test_facebox.py

e0608ef

Adds matched_faces attr

8286f09

Update test_facebox.py

38b9eed

MartinHjelmare reviewed May 7, 2018

View reviewed changes

Amend get_matched_faces to take faces

26128d2

MartinHjelmare reviewed May 7, 2018

View reviewed changes

Update test_facebox.py

afdab60

homeassistant added the cla-signed label May 7, 2018

Update test_facebox.py

8e89ec3

homeassistant added the cla-signed label May 7, 2018

houndci-bot reviewed May 7, 2018

View reviewed changes

robmarkcole added 2 commits May 7, 2018 09:06

Update test_facebox.py

faacce1

Make mock_req.post

2568d34

was mock_req.get

MartinHjelmare reviewed May 7, 2018

View reviewed changes

Update test_facebox.py

a3ee098

robmarkcole commented May 7, 2018

View reviewed changes

Update test_facebox.py

d9ddc6e

homeassistant added the cla-signed label May 7, 2018

gerard33 and others added 16 commits May 9, 2018 04:23

Add sensors for BMW electric cars (#14293)

1c14d22

* Add sensors for electric cars * Updates based on review of @MartinHjelmare * Fix Travis error * Another fix for Travis

Add more homematicip cloud components (#14084)

673a237

* Add support for shutter contact and motion detector device * Add support for power switch devices * Add support for light switch device * Cleanup binary_switch and light platform * Update comment

Snips: Added slot values for siteId and probability (#14315)

8145a8e

* Added solt values for siteId and probability * Update snips.py * Update test_snips.py

Fix Insteon PLM coverage

753419d

Add help for conversation/process service (#14323)

5537585

* Add help for conversation/process service * Add logging to debug text received when service is called * Move conversation to specific folder

Fix BOM weather '-' value (#14042)

107cf82

Make sure zwave nodes/entities enter the registry is proper state. (#…

3afd19b

…14251) * When zwave node's info is parsed remove it and re-add back. * Delay value entity if not ready * If node is ready consider it parsed even if manufacturer/product are missing. * Add annotations

Improving icloud device tracker (#14078)

89d7896

* Improving icloud device tracker * Adding config validations for new values * Adding config validations for new values * Moving icloud specific setup to platform schema. Setting default in platform schema.

Bump frontend to 20180509.0

146fc59

Ignore NaN values for influxdb (#14347)

c038ede

* Ignore NaN values for influxdb * Catch TypeError

Add zone 3 for Onkyo media player (#14295)

013d51e

* Add zone 3 for Onkyo media player * CR Updates * Fix travis lint errors

Passing test

50806d3

robmarkcole requested a review from a team as a code owner May 9, 2018 03:23

homeassistant added the cla-signed label May 9, 2018

robmarkcole added 3 commits May 9, 2018 04:24

Merge remote-tracking branch 'home-assistant/dev' into facebox-detect

39b7223

Fix lints

3be6ae6

Fix lints

6960ea1

robmarkcole closed this May 9, 2018

robmarkcole mentioned this pull request May 9, 2018

Adds facebox #14356

Merged

3 tasks

home-assistant locked and limited conversation to collaborators Sep 5, 2018

ghost added integration: facebox_face_detect and removed platform: image_processing.facebox_face_detect labels Mar 21, 2019

Uh oh!

Conversation

robmarkcole commented May 6, 2018 • 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 May 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

robmarkcole May 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

robmarkcole May 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robmarkcole commented May 7, 2018

Uh oh!

robmarkcole commented May 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartinHjelmare left a comment

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!

robmarkcole commented May 6, 2018 •

edited

Loading

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

robmarkcole commented May 6, 2018 •

edited

Loading

robmarkcole May 6, 2018 •

edited

Loading

robmarkcole May 6, 2018 •

edited

Loading

robmarkcole commented May 7, 2018 •

edited

Loading