Meraki AP Device tracker #10749

masarliev · 2017-11-22T13:40:58Z

Description:

Track devices with Meraki AP. More info here

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

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

device_tracker:
  - platform: meraki
    secret: secret
    validator: validator

Checklist:

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:

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.

houndci-bot · 2017-11-22T13:41:06Z

homeassistant/components/device_tracker/meraki.py

+                gps_location = (lat, lng)
+                attrs = {}
+                if ((not self.track_new and clientMac.upper() not in self.devs_to_track) and
+                    clientMac.upper() not in self.devs_to_track):


visually indented line with same indent as next logical line

houndci-bot · 2017-11-22T13:41:06Z

homeassistant/components/device_tracker/meraki.py

+                _LOGGER.debug("clientMac: %s", clientMac)
+                gps_location = (lat, lng)
+                attrs = {}
+                if ((not self.track_new and clientMac.upper() not in self.devs_to_track) and


line too long (92 > 79 characters)

houndci-bot · 2017-11-22T13:41:06Z

homeassistant/components/device_tracker/meraki.py

+            if data['secret'] != self.secret:
+                _LOGGER.error("Invalid Secret received from Meraki")
+            elif data['version'] != VERSION:
+                _LOGGER.error("Invalid API version received: %s", data['version'])


line too long (82 > 79 characters)

houndci-bot · 2017-11-22T13:41:06Z

homeassistant/components/device_tracker/meraki.py

+from homeassistant.const import (HTTP_BAD_REQUEST, HTTP_UNPROCESSABLE_ENTITY)
+from homeassistant.components.http import HomeAssistantView
+from homeassistant.components.device_tracker import (
+     PLATFORM_SCHEMA, SOURCE_TYPE_ROUTER, CONF_TRACK_NEW, YAML_DEVICES, load_config, DEFAULT_TRACK_NEW)


line too long (103 > 79 characters)

houndci-bot · 2017-11-22T13:47:17Z

homeassistant/components/device_tracker/meraki.py

+                gps_location = (lat, lng)
+                attrs = {}
+                if ((not self.tn and mac.upper() not in self.devices) and
+                            mac.upper() not in self.devices):


continuation line over-indented for visual indent

fabaff · 2017-11-26T09:04:43Z

homeassistant/components/device_tracker/meraki.py

+    name = 'api:meraki'
+
+    def __init__(self, config, see, devs_to_track, track_new):
+        """Initialize Locative URL endpoints."""


Stale docstring.

fabaff · 2017-11-26T09:04:47Z

homeassistant/components/device_tracker/meraki.py

+
+
+def setup_scanner(hass, config, see, discovery_info=None):
+    """Set up an endpoint for the Locative application."""


Stale docstring.

fabaff · 2017-11-26T09:05:27Z

homeassistant/components/device_tracker/meraki.py

+    @asyncio.coroutine
+    def get(self, request):
+        """Meraki message received as GET."""
+        _LOGGER.info("Merakicmx message received as a GET")


That very verbose.

fabaff · 2017-11-26T09:11:43Z

homeassistant/components/device_tracker/meraki.py

+    def _handle(self, hass, data):
+        if len(data["data"]["observations"]) == 0:
+            _LOGGER.debug("No observations found")
+        else:


Change it to a guard clause and return. This will allow you to get rid of the else and the indent of the whole code block.

fabaff · 2017-11-26T09:23:45Z

homeassistant/components/device_tracker/meraki.py

+                        mac.upper() not in self.devices):
+                    _LOGGER.debug("Skipping: %s", mac)
+                    continue
+                if 'os' in i:


A dictionary lookup could help here to make it shorter.

pvizeli · 2017-11-28T09:17:44Z

homeassistant/components/device_tracker/meraki.py

+PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
+    vol.Required(CONF_VALIDATOR): cv.string,
+    vol.Required(CONF_SECRET): cv.string,
+    vol.Optional(CONF_TRACK_NEW): cv.boolean


Remove this, that is a core function

pvizeli · 2017-11-28T09:18:39Z

homeassistant/components/device_tracker/meraki.py

+})
+
+
+def setup_scanner(hass, config, see, discovery_info=None):


use the async interface for that function

pvizeli · 2017-11-28T09:19:31Z

homeassistant/components/device_tracker/meraki.py

+            if i.get('ssid', False):
+                attrs['ssid'] = i['ssid']
+            yield from hass.async_add_job(
+                partial(self.see,


use async_see from async interface. Remove the partial and don't yield on it

Shouldn't it be:

yield from async_see(...)

?

houndci-bot · 2017-11-28T13:52:00Z

tests/components/device_tracker/test_meraki.py

+    assert req.status == 200
+    state_name = hass.states.get('{}.{}'.format('device_tracker',
+                                                '0026abb8a9a4')).state
+    assert 'home' == state_name


no newline at end of file

houndci-bot · 2017-11-28T13:52:00Z

tests/components/device_tracker/test_meraki.py

+        "secret": "secret",
+        "type": "DevicesSeen",
+        "data": {
+            "observations":[


missing whitespace after ':'

houndci-bot · 2017-11-28T13:52:00Z

tests/components/device_tracker/test_meraki.py

+import json
+from unittest.mock import patch
+import pytest
+from homeassistant.components.device_tracker.meraki import CONF_VALIDATOR, CONF_SECRET


line too long (86 > 79 characters)

houndci-bot · 2017-11-28T13:52:00Z

homeassistant/components/device_tracker/meraki.py

+    vol.Required(CONF_SECRET): cv.string
+})
+
+@asyncio.coroutine


expected 2 blank lines, found 1

houndci-bot · 2017-11-28T13:53:57Z

tests/components/device_tracker/test_meraki.py

+    state_name = hass.states.get('{}.{}'.format('device_tracker',
+                                                '0026abb8a9a4')).state
+    assert 'home' == state_name
+


blank line at end of file
blank line contains whitespace

houndci-bot · 2017-11-28T13:53:57Z

tests/components/device_tracker/test_meraki.py

+    state_name = hass.states.get('{}.{}'.format('device_tracker',
+                                                '0026abb8a9a4')).state
+    assert 'home' == state_name
+


blank line at end of file
blank line contains whitespace

MartinHjelmare · 2017-11-28T14:52:23Z

homeassistant/components/device_tracker/meraki.py

+@asyncio.coroutine
+def async_setup_scanner(hass, config, async_see, discovery_info=None):
+    """Set up an endpoint for the Meraki tracker."""
+    track_new = config.get(CONF_TRACK_NEW, DEFAULT_TRACK_NEW)


You don't need to handle this at all. It's done in the device tracker base component.

I checked https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/device_tracker/__init__.py#L273 and as I see it doesn't handle.

It handles it here for existing devices:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/device_tracker/__init__.py#L268

And here for new devices:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/device_tracker/__init__.py#L286

You are right. I used this option to check if device should be added to known. Since meraki floods known devices I will use other option

MartinHjelmare · 2017-11-28T14:55:30Z

homeassistant/components/device_tracker/meraki.py

+            _LOGGER.debug("clientMac: %s", mac)
+            gps_location = (lat, lng)
+            attrs = {}
+            if ((not self.track and mac.upper() not in self.devices) and


You can remove this whole check.

MartinHjelmare · 2017-11-28T14:59:10Z

homeassistant/components/device_tracker/meraki.py

+    track_new = config.get(CONF_TRACK_NEW, DEFAULT_TRACK_NEW)
+    yaml_path = hass.config.path(YAML_DEVICES)
+    devs_to_track = []
+    for device in load_config(yaml_path, hass, 0):


So then you don't need this either.

MartinHjelmare · 2017-11-28T15:00:02Z

homeassistant/components/device_tracker/meraki.py

+    url = URL
+    name = 'api:meraki'
+
+    def __init__(self, config, async_see, devs_to_track, track_new):


And you don't need to pass devs_to_track and track_new.

pvizeli · 2017-11-29T15:51:02Z

homeassistant/components/device_tracker/meraki.py

+                attrs['seenTime'] = i['seenTime']
+            if i.get('ssid', False):
+                attrs['ssid'] = i['ssid']
+            yield from self.async_see(


make simple:
hass.async_add_job(self.async_see(...))

So it will not block on process and is faster and make that _handle will be a normal function they can add a decorator @callback from core.py.

pvizeli · 2017-11-29T15:51:58Z

homeassistant/components/device_tracker/meraki.py

+        if len(data["data"]["observations"]) == 0:
+            _LOGGER.debug("No observations found")
+            return
+        res = yield from self._handle(request.app['hass'], data)


I see no return value from self._handle ?

addebc · 2017-12-01T16:00:47Z

Hey Great component however,
Maybe cleanup error messages before PR?

2017-12-01 16:55:59 ERROR (MainThread) [aiohttp.server] Error handling request
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/aiohttp/web_protocol.py", line 416, in start
    resp = yield from self._request_handler(request)
  File "/usr/lib/python3.6/site-packages/aiohttp/web.py", line 323, in _handle
    resp = yield from handler(request)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/http/ban.py", line 58, in ban_middleware
    return (yield from handler(request))
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 213, in coro
    res = yield from res
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 213, in coro
    res = yield from res
  File "/usr/lib/python3.6/site-packages/homeassistant/components/http/__init__.py", line 430, in handle
    result = yield from result
  File "/config/custom_components/device_tracker/meraki.py", line 93, in post
    yield from self._handle(request.app['hass'], data)
  File "/config/custom_components/device_tracker/meraki.py", line 101, in _handle
    accuracy = int(float(i["location"]["unc"]))
ValueError: cannot convert float NaN to integer

masarliev · 2017-12-01T16:09:54Z

Can you give me request body. I use it for more than 2 months and haven't received this error

addebc · 2017-12-01T18:36:17Z

After i set logging to debug error never appered but then i remembered i cleared all my known hosts. And probably thats why its not appearing... i will keep my eye open. Sorry bout that.

Setting track_new_devices: false does nothing for me... I am guessing having that should only scan and update the hosts already in known_hosts?

Btw, are you on discord?

masarliev · 2017-12-01T18:43:52Z

I haven't updated the documentation. Sorry about that, little busy these days. I misunderstood the option track new devices, so it's removed. May be should be added as feature for all platforms.
No. I don't use discord

addebc · 2017-12-02T03:10:07Z

Yes, without such feature home assistant gets cluttered with so many entities that are irrelevant (passersby) 5 hours have me 200 entities when really I just want to keep track of my own.

addebc · 2017-12-02T03:11:34Z

If possible add me on gmail-chat.

MartinHjelmare

As @pvizeli requested, see specific changes needed below.

MartinHjelmare · 2017-12-02T10:47:04Z

homeassistant/components/device_tracker/meraki.py

+            return
+        yield from self._handle(request.app['hass'], data)
+
+    @asyncio.coroutine


Change the annotation to @callback. Import the annotation from homeassistant/core.py.

MartinHjelmare · 2017-12-02T10:47:54Z

homeassistant/components/device_tracker/meraki.py

+                attrs['seenTime'] = i['seenTime']
+            if i.get('ssid', False):
+                attrs['ssid'] = i['ssid']
+            yield from hass.async_add_job(self.async_see(


Change to:

hass.async_add_job(self.async_see(...))

async_add_job is a callback and not a coroutine, so you don't yield from it.

MartinHjelmare · 2017-12-02T10:49:54Z

homeassistant/components/device_tracker/meraki.py

+        if len(data["data"]["observations"]) == 0:
+            _LOGGER.debug("No observations found")
+            return
+        yield from self._handle(request.app['hass'], data)


When you change _handle to a callback function instead of a coroutine (see comment below), change here to:

self._handle(request.app['hass'], data)

addelovein · 2017-12-03T16:52:38Z

2017-12-03 17:44:51 DEBUG (MainThread) [homeassistant.components.websocket_api] WS 1930745424: Sending {'id': 2, 'type': 'event', 'event': {'event_type': 'state_changed', 'data': {'entity_id': 'device_tracker.0055da5137b3', 'old_state': <state device_tracker.0055da5137b3=home; source_type=router, latitude=x.xxxx, longitude=x.xxxx, gps_accuracy=26, manufacturer=IEEE Registration..., ipv4=/192.168.0.218, seenTime=2017-12-03T16:44:25Z, ssid=Lovein - Home, os=Generic Linux, friendly_name=0055da5137b3, custom_ui_state_card=state-card-custom-ui @ 2017-12-03T05:55:03.151612+01:00>, 'new_state': <state device_tracker.0055da5137b3=home; source_type=router, latitude=59.1404482, longitude=x.xxxx, gps_accuracy=0, manufacturer=IEEE Registration..., ipv4=/192.168.0.218, seenTime=2017-12-03T16:44:25Z, ssid=Lovein - Home, os=Generic Linux, friendly_name=0055da5137b3, custom_ui_state_card=state-card-custom-ui @ 2017-12-03T05:55:03.151612+01:00>}, 'origin': 'LOCAL', 'time_fired': datetime.datetime(2017, 12, 3, 16, 44, 51, 247281, tzinfo=<UTC>)}}
2017-12-03 17:44:51 INFO (MainThread) [homeassistant.components.http] Serving /api/meraki to ******* (auth: True)
2017-12-03 17:44:51 ERROR (MainThread) [aiohttp.server] Error handling request
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/aiohttp/web_protocol.py", line 416, in start
    resp = yield from self._request_handler(request)
  File "/usr/lib/python3.6/site-packages/aiohttp/web.py", line 323, in _handle
    resp = yield from handler(request)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/http/ban.py", line 58, in ban_middleware
    return (yield from handler(request))
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 213, in coro
    res = yield from res
  File "/usr/lib/python3.6/asyncio/coroutines.py", line 213, in coro
    res = yield from res
  File "/usr/lib/python3.6/site-packages/homeassistant/components/http/__init__.py", line 430, in handle
    result = yield from result
  File "/config/custom_components/device_tracker/meraki.py", line 93, in post
    yield from self._handle(request.app['hass'], data)
  File "/config/custom_components/device_tracker/meraki.py", line 101, in _handle
    accuracy = int(float(i["location"]["unc"]))
ValueError: cannot convert float NaN to integer
2017-12-03 17:44:53 INFO (SyncWorker_8) [homeassistant.components.device_tracker.nmap_tracker] Scanning...

MartinHjelmare · 2017-12-04T21:15:09Z

How much coverage percentage is there with these tests? If it's above ~ 95 % you should remove the module from .coveragerc.

MartinHjelmare · 2017-12-05T08:38:31Z

Please also add tests for the cases at lines 87 and 91:
https://coveralls.io/builds/14503965/source?filename=homeassistant%2Fcomponents%2Fdevice_tracker%2Fmeraki.py

MartinHjelmare · 2017-12-05T10:27:15Z

Great!

MartinHjelmare · 2017-12-05T10:30:03Z

Wait, there's an unrelated commit in this PR history that should not be there:
1d3b7c3

Please rebase the changes that are relevant in this branch on top of latest dev.
Then we can merge.

masarliev · 2017-12-05T10:59:46Z

I merged remote dev in this branch and that's why this commit is in it.

MartinHjelmare · 2017-12-05T11:18:14Z

That commit doesn't exist in upstream dev. Your commit in your PR to update hbmqtt was squashed and merged as:
bd5a16d

You should start from a new branch based on newly synced dev when you start your work for a PR.

Can you rebase from 1a264da to HEAD, onto fresh synced dev? Something like this:

git checkout -b meraki-tracker dev
git branch -D dev
git fetch upstream dev
git checkout dev
git checkout meraki-tracker
git rebase --onto dev 69874075cbb55ab6b952b82c18e18ec6a31d3770 meraki-tracker

6987407 is the merge commit before your relevant work in this PR.

Edit:
Then you can open a new PR and we can merge that one. Switching branches will confuse github, so I don't think you can continue in this PR.

masarliev · 2017-12-05T13:02:25Z

Closing this PR. New one is here #10971

masarliev added 3 commits November 16, 2017 13:23

update hbmqtt to 0.9.1

1d3b7c3

Merge remote-tracking branch 'upstream/dev' into dev

6987407

Device tracker for meraki AP

1a264da

homeassistant added new-platform platform: device_tracker.meraki cla-signed labels Nov 22, 2017

houndci-bot reviewed Nov 22, 2017

View reviewed changes

styles fix

8357e8b

balloobbot added the platform label Nov 22, 2017

houndci-bot reviewed Nov 22, 2017

View reviewed changes

fix again

082b5f1

homeassistant added the cla-signed label Nov 22, 2017

masarliev added 3 commits November 22, 2017 16:28

again

f09a45d

and again :)

f226148

fix hide if away

9591269

masarliev mentioned this pull request Nov 26, 2017

Meraki device tracker documentation home-assistant/home-assistant.io#4044

Merged

2 tasks

fabaff reviewed Nov 26, 2017

View reviewed changes

docs and optimization

af224b4

pvizeli requested changes Nov 28, 2017

View reviewed changes

tests and fixes

8f5a0f4

houndci-bot reviewed Nov 28, 2017

View reviewed changes

styles

1cc818f

houndci-bot reviewed Nov 28, 2017

View reviewed changes

masarliev added 4 commits November 28, 2017 15:54

styles

508b42a

styles

9ab7fe1

styles

5075b41

styles fix. Hope last

4aea015

MartinHjelmare requested changes Nov 28, 2017

View reviewed changes

clear track new

d2e1b1e

pvizeli requested changes Nov 29, 2017

View reviewed changes

changes

58ed45e

MartinHjelmare requested changes Dec 2, 2017

View reviewed changes

fix accuracy error and requested changes

fac16d9

MartinHjelmare approved these changes Dec 4, 2017

View reviewed changes

remove meraki from .coveragerc

8f3fd7f

tests and minor changes

6dd86ac

masarliev closed this Dec 5, 2017

home-assistant locked and limited conversation to collaborators Mar 30, 2018

ghost added integration: meraki and removed platform: device_tracker.meraki labels Mar 21, 2019



		def setup_scanner(hass, config, see, discovery_info=None):
		"""Set up an endpoint for the Locative application."""

Meraki AP Device tracker #10749

Meraki AP Device tracker #10749

Conversation

masarliev commented Nov 22, 2017 • edited by fabaff Loading

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvizeli Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addebc commented Dec 1, 2017 • edited by MartinHjelmare Loading

masarliev commented Dec 1, 2017

addebc commented Dec 1, 2017 • edited Loading

masarliev commented Dec 1, 2017

addebc commented Dec 2, 2017

addebc commented Dec 2, 2017

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare Dec 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addelovein commented Dec 3, 2017 • edited by MartinHjelmare Loading

MartinHjelmare commented Dec 4, 2017

MartinHjelmare commented Dec 5, 2017

MartinHjelmare commented Dec 5, 2017

MartinHjelmare commented Dec 5, 2017

masarliev commented Dec 5, 2017

MartinHjelmare commented Dec 5, 2017 • edited Loading

masarliev commented Dec 5, 2017

masarliev commented Nov 22, 2017 •

edited by fabaff

Loading

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

pvizeli Nov 28, 2017 •

edited

Loading

addebc commented Dec 1, 2017 •

edited by MartinHjelmare

Loading

addebc commented Dec 1, 2017 •

edited

Loading

MartinHjelmare Dec 2, 2017 •

edited

Loading

addelovein commented Dec 3, 2017 •

edited by MartinHjelmare

Loading

MartinHjelmare commented Dec 5, 2017 •

edited

Loading