Attributes on dynamic endpoints can't be non-volatile

[wqx6]

Problem

Attributes on dynamic endpoints have a mask EXTERNAL_STORAGE. They can't be non-volatile because we check whether an attribute is non-volatile by

bool IsNonVolatile() const { return (mask & ATTRIBUTE_MASK_NONVOLATILE) && !IsExternal(); }

Can we have non-volatile attributes on dynamic endpoints? Or can we remove the external-storage check from the non-volatile check?

Proposed Solution

<suggested fix, suggested enhancement>

[bzbarsky-apple]

When configuring attributes in ZAP, there are three mutually exclusive storage modes: RAM, non-volatile, external. We use two bits to represent these three mutually exclusive states. The check represents the use of the bits we have.

The problems with introducing a fourth (external but non-volatile) state, especially for dynamic endpoint are as follows:

1. The system that handles storing (and reading from storage) non-volatile values cannot handle external attributes that are handled via AttributeAccessInterface. This might be ameliorated by not marking those as non-volatile....
2. The storage key for non-volatile attributes contains the endpoint id. For dynamic endpoints, the endpoint id might not mean the same entity as time passes.

Fundamentally, for an external attribute, storage is up to the implementation of the external attribute... If that implementation wants to use the same keys as "normal" non-volatile attribute storage, it can.

[chiragatal]

I think the problem is that some clusters have a dependency on whether a certain attribute is non-volatile. For example, in the on-off cluster, the start-up-on-off behaviour only works if the on-off and the start-up-on-off attributes are non-volatile (areStartUpOnOffServerAttributesNonVolatile()).

In cases where all the endpoints are dynamic, this check fails, since even though the attributes are non-volatile, they are in external storage.

[bzbarsky-apple]

Ah. That is a problem, thank you for the clarification.... @jmartinez-silabs We should probably either adjust those checks to exclude external attributes, or in fact allow flagging external things as non-volatile and exclude them in other places where we currently test IsNonVolatile().

[bzbarsky-apple]

In particular, IsNonVolatile clearly documents that it's about whether the attribute is automatically stored in non-volatile memory. But external attributes can be thus stored too, and the cluster impl should only be erroring out if it knows for sure the attributes are not being stored persistently....

[bzbarsky-apple]

#20144 should at least fix the issue @chiragatal describes. The actual non-volatility is still the responsibility of the implementation of the external attribute.