Storage of ZCL attribute datatypes by the network data store

[hsudbrock]

Currently, implementors of ZigBeeNetworkDataStore need to store information about ZCL attributes like the datatype or whether the attribute is mandatory. When reading a node from the data store, new instances of ZclAttribute are populated with that data, and those new instances are then used instead of the attributes defined in ZclCluster.initialize{Server|Client}Attributes.

I wonder if this should really be done that way. The method ZclCluster.initialize{Server|Client}Attributes provides the values defined in the ZCL spec, and I do not see why it should be possible that implementors of ZigBeeNetworkDataStore should be able to provide different values for those attribute properties.

Why do I even care about this? If there is, at some point in time, wrong attribute information in the autocoder XML files, this wrong information will make it into the data written by a network data store. If the autocoder XML files are fixed, this fix will never make it into the ZclAttribute instances that are created from network data store information that was written before that fix.

One concrete example is #534, which fixed wrong datatypes in the thermostat cluster XML files. I have one system here which reads a thermostat node that was persisted before #534 was merged, and in consequence this node still contains wrong attribute datatypes.

Chris, do you think it is sensible to not create new instances of ZclAttribute from a ZclAttributeDao here in ZclCluster.setDao, but rather to update the attributes created by initialize{Server|Client}Attributes during object creation with the infos from the ZclAttributeDao (excluding information defined by the ZCL spec like datatype, mandatory, etc.)?

[cdjackson]

In general, I don't mind this approach and can certainly see some benefit. However, there are a couple of things that we should keep in mind.

1. Custom attributes. Unless we can be sure that any custom attributes are added to the cluster before the data store updates it, then you will loose them. Since the ZclCluster is instantiated as part of the deserialisation from the data store, I don't think we can account for an manufacturer specific attribute?
2. Attribute "attributes" would be lost. By this I mean the time of last update, and the last value, or if it is even supported (see also If device reports UNSUPPORTED_ATTRIBUTE then mark it unsupported #931).

I'm not completely sure we can overcome these two issues (at least the first) if you are only updating existing attributes.

See also #236 which is I think directly linked to what you are reporting (ie changes to the XML definition do not flow through if the device was already persisted into the store) and probably should not have been closed (I will reopen it).

[hsudbrock]

I agree, #236 is directly linked to this one.

I think that your second issue it not a problem: I think we can overcome it by still taking the attribute "attributes" from the ZclAttributeDao(like last update, last value) and only ignoring those attribute properties that come from the ZCL spec (like data type, mandatory).

I agree that we need to think about the first issue, the custom attributes. Couldn't we check via the ZclAttributeDao#manufacturerCode whether it is a custom attribute - and in this case use the info from the ZclAttributeDao instead of the info from the initialize{Server|Client}Attributes.

However, I just saw that this field (manufacturerCode) is currently only contained in ZclAttribute but not contained in the ZclAttributeDao, and I believe that this is another issue that needs to be fixed before we could tackle this issue here.

[cdjackson]

So I guess you are basically suggesting a split approach. Use the existing system and create the attribute for all manufacturer specific attributes, and for standard ZCL defined attributes you only update the last update time, supported flag and last value? That sounds fine to me and would also solve the other issue. Adding the extra field to the DAO should not be a problem - at least it should not be breaking. Although I guess that depends on how the serialisation is handled. For OH and the console app I think it should be ok (although of course you will loose any manufacturer specific attributes since it will try to update the existing attribute and there won’t be one).