Add configure_reporting() method to zha component #16487
Conversation
ghost
added
the
| try: | ||
| v = await cluster.bind() | ||
| _LOGGER.debug( | ||
| "{}: bound '{}' cluster: {}".format(entity_id, cluster_name, v) |
Adminiuga
force-pushed
the
zha-configure-reporting
branch
from
6f50e97
to
fc82e23
Compare
Adminiuga
force-pushed
the
zha-configure-reporting
branch
from
fc82e23
to
b04a198
Compare
|
|
||
| try: | ||
| res = await cluster.configure_reporting(attr, min_report, | ||
| max_report, reportable_change) |
There was a problem hiding this comment.
continuation line under-indented for visual indent
Adminiuga
force-pushed
the
zha-configure-reporting
branch
from
b04a198
to
7b4d654
Compare
Adminiuga
force-pushed
the
zha-configure-reporting
branch
from
9605573
to
ffb466f
Compare
Adminiuga
force-pushed
the
zha-configure-reporting
branch
from
ffb466f
to
5e84edf
Compare
Binds a cluster and configures reporting for the specified attribute.
Adminiuga
force-pushed
the
zha-configure-reporting
branch
from
5e84edf
to
f00bbe2
Compare
| 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.
You dropped the min and max report parameters.
Reverting to the default means the max will change from 600 to 900. Is this important?
There was a problem hiding this comment.
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.
* use configure_reporting for zha switch * lint fixes
| _LOGGER.debug( | ||
| "%s: bound '%s' cluster: %s", entity_id, cluster_name, res[0] | ||
| ) | ||
| except DeliveryError as ex: |
There was a problem hiding this comment.
Should the method stop execution if this fails?
There was a problem hiding this comment.
It is the same idea as behind zha.safe_read() Better to have an entity which may not push external updates (eg manually toggled switch), but still works otherwise and could be controlled, rather than no entity at all.
Some vendors (eg Xiaomi) will throw, but still would send attribute reports.
There was a problem hiding this comment.
In the long run I'm thinking about having it as a "service call" on a device, like in a "repair_device" service call
| async_add_entities([sensor], update_before_add=True) | ||
|
|
||
|
|
||
| async def make_sensor(discovery_info): |
There was a problem hiding this comment.
Will change to switch
| await coro | ||
| except zigpy.exceptions.DeliveryError as exc: | ||
| _LOGGER.warning("Ignoring error during setup: %s", exc) | ||
| sensor = Switch(**discovery_info) |
There was a problem hiding this comment.
Oops. I should code less late nights :) i'll fix it
Implement zha.configure_reporting() method, which is very similar to zha.safe_read().
most Zigbee devices configure attribute reporting to push state updates to Hass. Currently there's similar code which binds&configure_reporting for a cluster/attribute. This PR moves that code to zha component and handles possible exceptions.
Checklist:
tox. Your PR cannot be merged unless tests pass