You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While reviewing implementation of Boolean State Configuration cluster it was found that there are several parts that could be improved to make the cluster more easily reusable/testable.
Issues:
Initial value of sensitivity is not valid until a first write, and it always starts at "N-1", irrespective of whether this is what the product wants. This requires products calling CHIP_ERROR SetCurrentSensitivityLevel(EndpointId ep, uint8_t level); at least one time, even though it's not possible to tell easily if this was done before.
When sensitivity changes via a write, the only path to see the change is via MatterPostAttributeChangeCallback (global). The delegate exists but cannot see the value being set.
Reads of current sensitivity always hit a flash read and are not cached.
There is no document indicating that the entire public interface of the Boolean State Configuration cluster needs to be called from Matter context (i.e. not safe to call directly from application if running in different context).
The public APIs or cluster interface behaviors are not documented. This makes it harder to understand that only MatterPostAttributeChangeCallback is what will be called for all state mutations.
Number of SupportedSensitivityLevels is always read from attribute store using SupportedSensitivityLevels::Get() which forces the default value to always be included in ZAP config. This makes it harder to test (not unit testable), and also prevents a delegate-based interface to have code-backed configuration.
When any Alarm mutator (e.g. SetAlarmsActive) is called, an event is always generated, even if nothing changed in the set of alarms, which does not match the intent of the cluster (can cause a large number of events generated if SetAlarmsActive(same_value) is called many times
The text was updated successfully, but these errors were encountered:
While reviewing implementation of Boolean State Configuration cluster it was found that there are several parts that could be improved to make the cluster more easily reusable/testable.
Issues:
CHIP_ERROR SetCurrentSensitivityLevel(EndpointId ep, uint8_t level);
at least one time, even though it's not possible to tell easily if this was done before.SupportedSensitivityLevels::Get()
which forces the default value to always be included in ZAP config. This makes it harder to test (not unit testable), and also prevents a delegate-based interface to have code-backed configuration.SetAlarmsActive
) is called, an event is always generated, even if nothing changed in the set of alarms, which does not match the intent of the cluster (can cause a large number of events generated if SetAlarmsActive(same_value) is called many timesThe text was updated successfully, but these errors were encountered: