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

Add device_tracker.bluetooth_update service #15252

Conversation

kariudo
Copy link
Contributor

@kariudo kariudo commented Jul 1, 2018

Will immediately scan for Bluetooth devices outside of the interval timer. Allows for less frequent scanning, with scanning on demand via automation.

Will immediately scan for Bluetooth devices outside of the interval timer. Allows for less frequent scanning, with scanning on demand via automation.

update_bluetooth(dt_util.utcnow())

Choose a reason for hiding this comment

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

blank line contains whitespace

@@ -79,7 +80,7 @@ def discover_devices():

request_rssi = config.get(CONF_REQUEST_RSSI, False)

def update_bluetooth(now):
def update_bluetooth(now, once = False):

Choose a reason for hiding this comment

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

unexpected spaces around keyword / parameter equals

@@ -99,9 +100,17 @@ def update_bluetooth(now):
see_device(mac, result, rssi)
except bluetooth.BluetoothError:
_LOGGER.exception("Error looking up Bluetooth device")
track_point_in_utc_time(
hass, update_bluetooth, dt_util.utcnow() + interval)
if not once:
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest extract line 85-102 to a new function. And call that function from update_bluetooth and handle_update_bluetooth. The you can avoid the once parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, addressed with 5b4bfa4


update_bluetooth(dt_util.utcnow())

hass.services.register(DOMAIN, "bluetooth_update", handle_update_bluetooth)
Copy link
Member

Choose a reason for hiding this comment

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

Prefix the service name with the name of the platform, eg like this: 'bluetooth_tracker_update'. I intentionally stripped the name of the second bluetooth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense to me, I updated it to 'bluetooth_tracker_update' in a6d6f76


update_bluetooth(dt_util.utcnow())

hass.services.register(DOMAIN, "bluetooth_tracker_update", handle_update_bluetooth)

Choose a reason for hiding this comment

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

line too long (87 > 79 characters)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fun story: PyCharm for some weird totally non PEP 8 reason has a default gutter of 120, rather than 80. They fooled me!


update_bluetooth(dt_util.utcnow())
def handle_update_bluetooth(call):
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this function?
Why not call update_bluetooth_once directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certianly could, I just left it using the handle function per a previous comment you said:

I would suggest extract line 85-102 to a new function. And call that function from update_bluetooth and handle_update_bluetooth. The you can avoid the once parameter

I assumed you wanted it left using the handle_update_bluetooth function, to absorb the unused call parameter.

@@ -79,7 +80,13 @@ def discover_devices():

request_rssi = config.get(CONF_REQUEST_RSSI, False)

def update_bluetooth(now):
def update_bluetooth():
Copy link
Member

Choose a reason for hiding this comment

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

I thought the now when calling the function from track_point_in_utc_time ?

Copy link
Contributor Author

@kariudo kariudo Jul 18, 2018

Choose a reason for hiding this comment

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

Look at the changeset, track_point_in_utc_time is called with dt_util.utcnow() not with now, I assumed that the unused now was not being used due to the delay introduced by it being called at the end of the function which means that time would be in the past when it was called, which could cause it to just continuously run again after the interval + time it starts, rather than interval+ time it completed.

Copy link
Contributor

@awarecan awarecan Sep 13, 2018

Choose a reason for hiding this comment

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

This broke the code, argument even not used you still need have it

2018-09-12 16:28:11 ERROR (MainThread) [homeassistant.core] Error doing job: Future exception was never retrieved
 Traceback (most recent call last):
   File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
     result = self.fn(*self.args, **self.kwargs)
 TypeError: update_bluetooth() takes 0 positional arguments but 1 was given

@MartinHjelmare
Copy link
Member

@Danielhiversen any further comments?

@Danielhiversen Danielhiversen merged commit dec2d8d into home-assistant:dev Aug 27, 2018
@ghost ghost removed the in progress label Aug 27, 2018
mbennett pushed a commit to mbennett/home-assistant that referenced this pull request Aug 27, 2018
* Add device_tracker.bluetooth_update service

Will immediately scan for Bluetooth devices outside of the interval timer. Allows for less frequent scanning, with scanning on demand via automation.

* remove excess whitespace per bot comments

* Refactored update_bluetooth to call new function update_bluetooth_once

* Change service name to bluetooth_tracker_update to reflect platform name

* Reformat for line length

* Linting fix, pydoc, first line should end with a period

* Fixed a method call, and removed some more unsused parameters
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* Add device_tracker.bluetooth_update service

Will immediately scan for Bluetooth devices outside of the interval timer. Allows for less frequent scanning, with scanning on demand via automation.

* remove excess whitespace per bot comments

* Refactored update_bluetooth to call new function update_bluetooth_once

* Change service name to bluetooth_tracker_update to reflect platform name

* Reformat for line length

* Linting fix, pydoc, first line should end with a period

* Fixed a method call, and removed some more unsused parameters
@awarecan awarecan mentioned this pull request Sep 13, 2018
2 tasks
@balloob balloob mentioned this pull request Sep 17, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
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.

None yet

6 participants