Eddystone Beacon Temperature Sensor by citruz · Pull Request #6789 · home-assistant/core

citruz · 2017-03-25T23:57:31Z

Description:

Read temperature information from Eddystone beacons.

Eddystone beacons can be configured to broadcast telemetry data which also includes the temperature measured by the device (assuming that the device has a temperature sensor). This platform will scan for Bluetooth LE advertisements using pybluez and extract the relevant information.

Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#2334

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

sensor:
  - platform: eddystone_temperature
    bt_device_id: 0  # optional: id of the bluetooth device (hciX) 
    beacons:
      living_room:
        namespace: "12345678901234678901"
        instance: "000000000001"
        name: "Living Room"
      kitchen:
        namespace: "12345678901234678901"
        instance: "000000000002"
        name: "Kitchen"

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.

mention-bot · 2017-03-25T23:57:33Z

@citruz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

homeassistant · 2017-03-25T23:57:33Z

Hi @citruz,

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!

pvizeli

Please export the monitor class into external pipy packages. This have do device specific code.

citruz · 2017-03-26T20:38:59Z

@pvizeli Are you sure that's necessary for 120 lines of code (without the comments)? To me, this seems like too little functionality to move it to its own pipy package. Also, I haven't created a pipy package before so that would be quite an effort for me.

pvizeli · 2017-03-27T10:28:30Z

Yes it is -> https://home-assistant.io/developers/add_new_platform/#interfacing-with-devices

rytilahti · 2017-03-27T23:18:28Z

As far as I know there exists no eddystone tlm implementation publicly available (at least not in python, for urls there's this https://github.com/google/eddystone/tree/master/eddystone-url/implementations/PyBeacon), so having a python package implementing the eddystone parsing would help others too!

As for monitoring itself, I think there needs to be a discussion at some point on how the bluetooth devices should be used inside homeassistant in general, as different bluetooth devices are starting to pile up and them trying to access the bluetooth adapter at the same time will cause race conditions.. As the eddystone beacons are just some extra data contained in the advertisement messages, this component and bluetooth le tracker should in principle be using the data returned by a single scan round.

edit: to add, packaging modules is quite straight-forward, see https://packaging.python.org/distributing/ .

edit2: it'd also be nice to extract other information available in the TLM frames, esp. battery information can be useful https://github.com/google/eddystone/blob/master/eddystone-tlm/tlm-plain.md .

citruz · 2017-03-31T08:25:27Z

Alright, I will work on separating the functionality then. Having a dedicated beacon advertisement parser in fact seems like a good idea. Thanks for the suggestions.

houndci-bot · 2017-04-03T01:38:32Z

+
+    if len(devices) > 0:
+        mon = Monitor(hass, devices, bt_device_id)
+        def monitor_stop(_service_or_event):


expected 1 blank line before a nested definition, found 0

houndci-bot · 2017-04-03T01:38:32Z

+    devices = []
+
+
+    for dev_name, properties in beacons.items():


too many blank lines (2)

houndci-bot · 2017-04-03T01:38:32Z

+https://github.com/anpetrov/skybeacon
+"""
+import logging
+import threading


'threading' imported but unused

houndci-bot · 2017-04-03T02:01:34Z

+})
+
+# pylint: disable=unused-argument
+def setup_platform(hass, config, add_devices, discovery_info=None):


expected 2 blank lines, found 1

citruz · 2017-04-03T02:19:37Z

@pvizeli I have created a PyPI package and updated my PR. Please review the changes and merge the PR if you're happy with them :)

rytilahti · 2017-04-03T18:46:53Z

Looks good (as does the lib, hello fellow construct user!), just wondering how the scanning part is implemented in the backend, will the scanning block all other bluetooth activities?

There are quite a few problems related to handling of BT/BLE devices in general, originating (most likely) from blocking the device access / trying to issue too many concurrent connects on BLE devices (#4442, #3885). Would enabling this worsen the situation? If not, maybe the same scanning backend could also be used for bt_le_tracker too?

Maybe that would also fix #4186 ?

citruz · 2017-04-03T19:43:38Z

@rytilahti I don't know much about the internals of bluez, but I was able to run two scanners in parallel using the library. So it looks like the other activities are not blocked by the scanning.

Beacons require (and allow) no "connection", the scanner only reads the data from the ble advertisement. Therefore, too many concurrent connections won't be problem.
Scanning in general can be quite CPU consuming if there are lots of devices around and the scanner tries to parse every incoming packet. To prevent this, the library will only attempt to parse Eddystone packets, all others are dropped. In addition to that I configured my beacons to use a low transmission interval. With this I get a CPU load of 3-5% on a raspi zero.

Regarding bt_le_tracker: The library only scans for beacons, not BLE devices in general. Hence it cannot be used as a replacement.

pvizeli

Some small minor changes:

Move constant into your platform from const.py
Add property for polling into you entity and set it to false
After you change some value on entity, call schedule_update_ha_state()
Start thread also on EVENT_HOMEASSISTANT_START

rytilahti · 2017-04-04T14:09:10Z

@citruz ok, nice to hear about it working with two parallel scans. I think this all depends on the chipset. I know that the data is there in advertisements, was just curious what will happen when other scanners start working at the same time. At least in my case I can't use hcitool's scanle when there are open connections (caused by other ble connected devices).

About bt_le_tracker, I just mentioned it here because it shares similar needs (requiring access to adv data) while being just more lightweight with no need for parsing. The problem with the current (bt_le_tracker) solution seems to be that it keeps blocking for 10s scanning periods, a problem which could be solved by using your scanner backend for that too.

citruz · 2017-04-04T17:49:05Z

@pvizeli Done.
@rytilahti I could definitely add an option which will disable any parsing and just pass the bluetooth addr in the callback. With some additional performance tuning (no logic in the callback handler) I think it should be possible to fix the problems of the bt le tracker.

rytilahti · 2017-04-04T18:30:47Z

@citruz that would be a nice and a very welcomed addition I suppose! Not relevant for the task at hand, so I'll stop discussing it from here on.

pvizeli · 2017-04-04T21:57:29Z

Good work ⚡️

citruz added 5 commits March 26, 2017 00:35

Added eddystone_temperature platform.

96c1756

Fixed style issues.

4b24608

Fixed style issues home-assistant#2.

ec7c80f

Fixed style issues home-assistant#3.

472c370

Added new platform to .coveragerc

aac5d88

homeassistant added cla-needed new-platform platform: sensor.eddystone_temperature labels Mar 25, 2017

balloobbot added core platform requirement labels Mar 25, 2017

homeassistant added cla-signed and removed cla-needed labels Mar 25, 2017

citruz mentioned this pull request Mar 26, 2017

Added documentation and logo for eddystone_temperature platform. home-assistant/home-assistant.io#2334

Merged

pvizeli requested changes Mar 26, 2017

View reviewed changes

Refactored platform to use the beacontools package.

581ad6f

homeassistant added the cla-signed label Apr 3, 2017

houndci-bot reviewed Apr 3, 2017

View reviewed changes

citruz added 2 commits April 3, 2017 03:48

Fixed style issues and added beacontools to excluded requirements.

cd1b001

Removed obsolete constants and added pylint exception.

c77ebf9

houndci-bot reviewed Apr 3, 2017

View reviewed changes

Added blank line

968d4d8

Updated beacontools to version 1.0.0

b457b0a

homeassistant added the cla-signed label Apr 3, 2017

Updated beacontools to version 1.0.1

b8aea72

homeassistant added the cla-signed label Apr 3, 2017

Forgot to regenerate requirements_all

f485e23

pvizeli requested changes Apr 4, 2017

View reviewed changes

Minor changes

a36bb24

homeassistant added the cla-signed label Apr 4, 2017

balloobbot removed the core label Apr 4, 2017

pvizeli approved these changes Apr 4, 2017

View reviewed changes

pvizeli merged commit e4e7141 into home-assistant:dev Apr 4, 2017

fabaff mentioned this pull request Apr 6, 2017

0.42 #6956

Merged

home-assistant locked and limited conversation to collaborators Jul 17, 2017

ghost added integration: eddystone_temperature and removed platform: sensor.eddystone_temperature labels Mar 21, 2019

Uh oh!

Conversation

citruz commented Mar 25, 2017 • 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!

mention-bot commented Mar 25, 2017

Uh oh!

homeassistant commented Mar 25, 2017

Uh oh!

pvizeli left a comment

Choose a reason for hiding this comment

Uh oh!

citruz commented Mar 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pvizeli commented Mar 27, 2017

Uh oh!

rytilahti commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

citruz commented Mar 31, 2017

Uh oh!

houndci-bot Apr 3, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Apr 3, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Apr 3, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Apr 3, 2017

Choose a reason for hiding this comment

Uh oh!

citruz commented Apr 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rytilahti commented Apr 3, 2017

Uh oh!

citruz commented Apr 3, 2017

Uh oh!

pvizeli left a comment

Choose a reason for hiding this comment

Uh oh!

rytilahti commented Apr 4, 2017

Uh oh!

citruz commented Apr 4, 2017

Uh oh!

rytilahti commented Apr 4, 2017

Uh oh!

pvizeli commented Apr 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

citruz commented Mar 25, 2017 •

edited

Loading

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

citruz commented Mar 26, 2017 •

edited

Loading

rytilahti commented Mar 27, 2017 •

edited

Loading

citruz commented Apr 3, 2017 •

edited

Loading