-
-
Notifications
You must be signed in to change notification settings - Fork 38.3k
Add configure_reporting() method to zha component #16487
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
Changes from 4 commits
143499c
d4f854b
f00bbe2
5fb2061
665e5b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,9 +57,9 @@ def make_sensor(discovery_info): | |
|
|
||
| if discovery_info['new_join']: | ||
| cluster = list(in_clusters.values())[0] | ||
| yield from cluster.bind() | ||
| yield from cluster.configure_reporting( | ||
| sensor.value_attribute, 300, 600, sensor.min_reportable_change, | ||
| yield from zha.configure_reporting( | ||
| sensor.entity_id, cluster, sensor.value_attribute, | ||
| reportable_change=sensor.min_reportable_change | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You dropped the min and max report parameters. Reverting to the default means the max will change from 600 to 900. Is this important?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The max_report interval is more of a "keep_alive" update, which is sent even if the attribute hasn't changed. IMO doesn't really matter if it 10min or 15min, but it is more graceful on battery operated devices. The only downside, it is going to be 5 min longer for the status to synchronize if the device was offline for whatever reason. Once/If this PR gets merged, I'm planning to submit another one where min/max_report interval will be part of base Sensor class, allowing child classes to override those to match sensor specifics. |
||
| ) | ||
|
|
||
| return sensor | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,13 +21,25 @@ async def async_setup_platform(hass, config, async_add_entities, | |
| if discovery_info is None: | ||
| return | ||
|
|
||
| sensor = await make_sensor(discovery_info) | ||
| async_add_entities([sensor], update_before_add=True) | ||
|
|
||
|
|
||
| async def make_sensor(discovery_info): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this called
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will change to switch |
||
| """Create ZHA sensors factory.""" | ||
| from zigpy.zcl.clusters.general import OnOff | ||
| in_clusters = discovery_info['in_clusters'] | ||
| cluster = in_clusters[OnOff.cluster_id] | ||
| await cluster.bind() | ||
| await cluster.configure_reporting(0, 0, 600, 1,) | ||
|
|
||
| async_add_entities([Switch(**discovery_info)], update_before_add=True) | ||
| sensor = Switch(**discovery_info) | ||
|
|
||
| if discovery_info['new_join']: | ||
| in_clusters = discovery_info['in_clusters'] | ||
| cluster = in_clusters[OnOff.cluster_id] | ||
| await zha.configure_reporting( | ||
| sensor.entity_id, cluster, sensor.value_attribute, | ||
| min_report=0, max_report=600, reportable_change=1 | ||
| ) | ||
|
|
||
| return sensor | ||
|
|
||
|
|
||
| class Switch(zha.Entity, SwitchDevice): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -435,3 +435,42 @@ async def safe_read(cluster, attributes, allow_cache=True): | |
| return result | ||
| except Exception: # pylint: disable=broad-except | ||
| return {} | ||
|
|
||
|
|
||
| async def configure_reporting(entity_id, cluster, attr, skip_bind=False, | ||
| min_report=300, max_report=900, | ||
| reportable_change=1): | ||
| """Configure attribute reporting for a cluster. | ||
|
|
||
| while swallowing the DeliverError exceptions in case of unreachable | ||
| devices. | ||
| """ | ||
| from zigpy.exceptions import DeliveryError | ||
|
|
||
| attr_name = cluster.attributes.get(attr, [attr])[0] | ||
| cluster_name = cluster.ep_attribute | ||
| if not skip_bind: | ||
| try: | ||
| res = await cluster.bind() | ||
| _LOGGER.debug( | ||
| "%s: bound '%s' cluster: %s", entity_id, cluster_name, res[0] | ||
| ) | ||
| except DeliveryError as ex: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the method stop execution if this fails?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the same idea as behind
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the long run I'm thinking about having it as a "service call" on a device, like in a "repair_device" service call |
||
| _LOGGER.debug( | ||
| "%s: Failed to bind '%s' cluster: %s", | ||
| entity_id, cluster_name, str(ex) | ||
| ) | ||
|
|
||
| try: | ||
| res = await cluster.configure_reporting(attr, min_report, | ||
| max_report, reportable_change) | ||
| _LOGGER.debug( | ||
| "%s: reporting '%s' attr on '%s' cluster: %d/%d/%d: Status: %s", | ||
| entity_id, attr_name, cluster_name, min_report, max_report, | ||
| reportable_change, res[0][0].status | ||
| ) | ||
| except DeliveryError as ex: | ||
| _LOGGER.debug( | ||
| "%s: failed to set reporting for '%s' attr on '%s' cluster: %s", | ||
| entity_id, attr_name, cluster_name, str(ex) | ||
| ) | ||
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.
sensor = switch ?
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.
Oops. I should code less late nights :) i'll fix it