-
Notifications
You must be signed in to change notification settings - Fork 116
Feature: On demand device polling #48
base: master
Are you sure you want to change the base?
Conversation
return | ||
poll_device_supported = 'poll_device' in worker_obj.status_update.__code__.co_varnames | ||
topic_prefixes = [] | ||
topic_prefixes.append(userdata.get('global_topic_prefix', '')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use some topic helpers, to be sure it wont break after some refactoring.
Line 81 in 54ae518
def _format_topic(self, topic): |
bt-mqtt-gateway/workers/base.py
Line 10 in 54ae518
def format_topic(self, *args): |
return | ||
elif not poll_device_supported: | ||
poll_args = [] | ||
elif device_name not in getattr(worker_obj, 'devices', []): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some getter for devices and use it, instead of accessing variable directly.
@@ -23,12 +23,14 @@ def searchmac(self, devices, mac): | |||
|
|||
return None | |||
|
|||
def status_update(self): | |||
def status_update(self, poll_device=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should be generalized to something like: device_filter=None , which will act as filter for self.devices, then that if and continue wont be needed inside each loop. When self.devices will be a getter method ( see other comment ) it could take array to filter devices and return only filtered subset.
Hi thanks for great PR. Added some thoughts. Also maybe we should also do something like /poll_all , and run update_all ? |
def _poll_wrapper(self, worker_obj, client, userdata, c): | ||
if c.payload.decode('utf-8') != '': | ||
return | ||
poll_device_supported = 'poll_device' in worker_obj.status_update.__code__.co_varnames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could also benefit from some devices getter. When there is None / not defined / only one device ( ? ) return from worker_obj.devices then it does not support
Thanks heaps @zewelor for the feedback! I was a little hesitant to add heaps for the PR but now I know you like the idea I'll implement the changes you have suggested 😄 👍 I'm thinking maybe changing the Seems this is a py3 project would you be okay for me to implement the |
Changing poll to some update sounds good, maybe something like flower/force_update or flower/update_stat(e|us). Only flower/update seems a bit non informative, but maybe its just me. I've looked at @Property and sound great. I'm not a python programmer, just learning, that's also why this project, so any python specific improvements or others are welcome :D Project targets python 3.5 so everything supported since 3.5 suits. |
Description
This adds on-demand polling to the devices which support it.
It is used by publishing an empty MQTT message to the
/poll
topic of a device or device-type.Usage examples:
Motivation for this change:
It may be desirable to set a long
update_interval
to conserve battery, but situations can arise which require quicker updates.An example of this would be using MiFlora devices, you may have them set to update once per 30mins-1hr, however when watering you may want to check their status on demand in order to prevent over-watering.
Or when using BLE tracking, you may want to dynamically poll based on attributes such as GPS location, movement or RSSI values.
Type of change