Looking to remove some redundant and unused functions from the ember attribute storage/table

examples/all-clusters-app/nxp/mw320/main.cpp

@@ -1152,12 +1152,10 @@ void task_test_main(void * param)
             PRINTF("--> update CurrentPosition [%d] \r\n", value);
             Clusters::Switch::Attributes::CurrentPosition::Set(1, value);
 #ifdef SUPPORT_MANUAL_CTRL
-#error                                                                                                                             \
-    "This code thinks it's setting the OnOff attribute, but it's actually setting the NumberOfPositions attribute!  And passing the wrong size for either case.  Figure out what it's trying to do."
-            // sync-up the Light attribute (for test event, OO.M.ManuallyControlled)
-            PRINTF("--> update [Clusters::Switch::Id]: OnOff::Id [%d] \r\n", value);
-            emAfWriteAttribute(1, Clusters::Switch::Id, Clusters::OnOff::Attributes::OnOff::Id, (uint8_t *) &value, sizeof(value),
-                               true, false);
+#error "Not implemented"
+            // TODO: previous code was trying to write a OnOff cluster attribute id to a switch attribute, generally
+            //       not working. Determine if this should maybe be
+            //       OnOff::Attributes::OnOff::Set(1, is_on) or similar
 #endif // SUPPORT_MANUAL_CTRL
 
             need2sync_sw_attr = false;

src/app/util/af.h

@@ -105,18 +105,6 @@ bool emberAfContainsClient(chip::EndpointId endpoint, chip::ClusterId clusterId)
 EmberAfStatus emberAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
                                     uint8_t * dataPtr, EmberAfAttributeType dataType);
 
-/**
- * @brief Read the attribute value, performing all the checks.
- *
- * This function will attempt to read the attribute and store it into the
- * pointer.
- *
- * dataPtr may be NULL, signifying that we don't need the value, just the status
- * (i.e. whether the attribute can be read).
- */
-EmberAfStatus emberAfReadAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
-                                   uint8_t * dataPtr, uint16_t readLength);
-
 /**
  * @brief macro that returns size of attribute in bytes.
  *

src/app/util/attribute-metadata.h

@@ -105,17 +105,6 @@ union EmberAfDefaultOrMinMaxAttributeValue
     const EmberAfAttributeMinMaxValue * ptrToMinMaxValue;
 };
 
-enum class EmberAfAttributeWritePermission
-{
-    DenyWrite            = 0,
-    AllowWriteNormal     = 1,
-    AllowWriteOfReadOnly = 2,
-    UnsupportedAttribute = 0x86, // Protocols::InteractionModel::Status::UnsupportedAttribute
-    InvalidValue         = 0x87, // Protocols::InteractionModel::Status::ConstraintError
-    ReadOnly             = 0x88, // Protocols::InteractionModel::Status::UnsupportedWrite
-    InvalidDataType      = 0x8d, // Protocols::InteractionModel::Status::InvalidDataType
-};
-
 // Attribute masks modify how attributes are used by the framework
 //
 // Attribute that has this mask is NOT read-only

src/app/util/attribute-table.cpp

@@ -46,33 +46,13 @@ using namespace chip;
 EmberAfStatus emberAfWriteAttributeExternal(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * dataPtr,
                                             EmberAfAttributeType dataType)
 {
-    EmberAfAttributeWritePermission extWritePermission =
-        emberAfAllowNetworkWriteAttributeCallback(endpoint, cluster, attributeID, dataPtr, dataType);
-    switch (extWritePermission)
-    {
-    case EmberAfAttributeWritePermission::DenyWrite:
-        return EMBER_ZCL_STATUS_FAILURE;
-    case EmberAfAttributeWritePermission::AllowWriteNormal:
-    case EmberAfAttributeWritePermission::AllowWriteOfReadOnly:
-        return emAfWriteAttribute(endpoint, cluster, attributeID, dataPtr, dataType,
-                                  (extWritePermission == EmberAfAttributeWritePermission::AllowWriteOfReadOnly), false);
-    default:
-        return (EmberAfStatus) extWritePermission;
-    }
+    return emAfWriteAttribute(endpoint, cluster, attributeID, dataPtr, dataType, false /* override read-only */);
 }
 
 EmberAfStatus emberAfWriteAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * dataPtr,
                                     EmberAfAttributeType dataType)
 {
-    return emAfWriteAttribute(endpoint, cluster, attributeID, dataPtr, dataType,
-                              true,   // override read-only?
-                              false); // just test?
-}
-
-EmberAfStatus emberAfReadAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * dataPtr,
-                                   uint16_t readLength)
-{
-    return emAfReadAttribute(endpoint, cluster, attributeID, dataPtr, readLength, nullptr);
+    return emAfWriteAttribute(endpoint, cluster, attributeID, dataPtr, dataType, true /* override read-only */);
 }
 
 //------------------------------------------------------------------------------
@@ -134,30 +114,8 @@ static bool IsNullValue(const uint8_t * data, uint16_t dataLen, bool isAttribute
     return false;
 }
 
-// writes an attribute (identified by clusterID and attrID to the given value.
-// this returns:
-// - EMBER_ZCL_STATUS_UNSUPPORTED_ENDPOINT: if endpoint isn't supported by the device.
-// - EMBER_ZCL_STATUS_UNSUPPORTED_CLUSTER: if cluster isn't supported on the endpoint.
-// - EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE: if attribute isn't supported in the cluster.
-// - EMBER_ZCL_STATUS_INVALID_DATA_TYPE: if the data type passed in doesnt match the type
-//           stored in the attribute table
-// - EMBER_ZCL_STATUS_UNSUPPORTED_WRITE: if the attribute isnt writable
-// - EMBER_ZCL_STATUS_CONSTRAINT_ERROR: if the value is set out of the allowable range for
-//           the attribute
-// - EMBER_ZCL_STATUS_SUCCESS: if the attribute was found and successfully written
-//
-// if true is passed in for overrideReadOnlyAndDataType then the data type is
-// not checked and the read-only flag is ignored. This mode is meant for
-// testing or setting the initial value of the attribute on the device.
-//
-// if true is passed for justTest, then the type is not written but all
-// checks are done to see if the type could be written
-// reads the attribute specified, returns false if the attribute is not in
-// the table or the data is too large, returns true and writes to dataPtr
-// if the attribute is supported and the readLength specified is less than
-// the length of the data.
 EmberAfStatus emAfWriteAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * data,
-                                 EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType, bool justTest)
+                                 EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType)
 {
     const EmberAfAttributeMetadata * metadata = nullptr;
     EmberAfAttributeSearchRecord record;
@@ -235,77 +193,63 @@ EmberAfStatus emAfWriteAttribute(EndpointId endpoint, ClusterId cluster, Attribu
         }
     }
 
-    // write the data unless this is only a test
-    if (!justTest)
-    {
-        const app::ConcreteAttributePath attributePath(endpoint, cluster, attributeID);
+    const app::ConcreteAttributePath attributePath(endpoint, cluster, attributeID);
 
-        // Pre write attribute callback for all attribute changes,
-        // regardless of cluster.
-        Protocols::InteractionModel::Status imStatus =
-            MatterPreAttributeChangeCallback(attributePath, dataType, emberAfAttributeSize(metadata), data);
-        if (imStatus != Protocols::InteractionModel::Status::Success)
-        {
-            return app::ToEmberAfStatus(imStatus);
-        }
+    // Pre write attribute callback for all attribute changes,
+    // regardless of cluster.
+    Protocols::InteractionModel::Status imStatus =
+        MatterPreAttributeChangeCallback(attributePath, dataType, emberAfAttributeSize(metadata), data);
+    if (imStatus != Protocols::InteractionModel::Status::Success)
+    {
+        return app::ToEmberAfStatus(imStatus);
+    }
 
-        // Pre-write attribute callback specific
-        // to the cluster that the attribute lives in.
-        status = emAfClusterPreAttributeChangedCallback(attributePath, dataType, emberAfAttributeSize(metadata), data);
+    // Pre-write attribute callback specific
+    // to the cluster that the attribute lives in.
+    status = emAfClusterPreAttributeChangedCallback(attributePath, dataType, emberAfAttributeSize(metadata), data);
 
-        // Ignore the following write operation and return success
-        if (status == EMBER_ZCL_STATUS_WRITE_IGNORED)
-        {
-            return EMBER_ZCL_STATUS_SUCCESS;
-        }
+    // Ignore the following write operation and return success
+    if (status == EMBER_ZCL_STATUS_WRITE_IGNORED)
+    {
+        return EMBER_ZCL_STATUS_SUCCESS;
+    }
 
-        if (status != EMBER_ZCL_STATUS_SUCCESS)
-        {
-            return status;
-        }
+    if (status != EMBER_ZCL_STATUS_SUCCESS)
+    {
+        return status;
+    }
 
-        // write the attribute
-        status = emAfReadOrWriteAttribute(&record,
-                                          nullptr, // metadata
-                                          data,
-                                          0,     // buffer size - unused
-                                          true); // write?
+    // write the attribute
+    status = emAfReadOrWriteAttribute(&record,
+                                      nullptr, // metadata
+                                      data,
+                                      0,     // buffer size - unused
+                                      true); // write?
 
-        if (status != EMBER_ZCL_STATUS_SUCCESS)
-        {
-            return status;
-        }
+    if (status != EMBER_ZCL_STATUS_SUCCESS)
+    {
+        return status;
+    }
 
-        // Save the attribute to persistent storage if needed
-        // The callee will weed out attributes that do not need to be stored.
-        emAfSaveAttributeToStorageIfNeeded(data, endpoint, cluster, metadata);
+    // Save the attribute to persistent storage if needed
+    // The callee will weed out attributes that do not need to be stored.
+    emAfSaveAttributeToStorageIfNeeded(data, endpoint, cluster, metadata);
 
-        MatterReportingAttributeChangeCallback(endpoint, cluster, attributeID);
+    MatterReportingAttributeChangeCallback(endpoint, cluster, attributeID);
 
-        // Post write attribute callback for all attributes changes, regardless
-        // of cluster.
-        MatterPostAttributeChangeCallback(attributePath, dataType, emberAfAttributeSize(metadata), data);
+    // Post write attribute callback for all attributes changes, regardless
+    // of cluster.
+    MatterPostAttributeChangeCallback(attributePath, dataType, emberAfAttributeSize(metadata), data);
 
-        // Post-write attribute callback specific
-        // to the cluster that the attribute lives in.
-        emAfClusterAttributeChangedCallback(attributePath);
-    }
-    else
-    {
-        // bug: 11618, we are not handling properly external attributes
-        // in this case... We need to do something. We don't really
-        // know if it will succeed.
-        ChipLogProgress(Zcl, "WRITE: no write, just a test");
-    }
+    // Post-write attribute callback specific
+    // to the cluster that the attribute lives in.
+    emAfClusterAttributeChangedCallback(attributePath);
 
     return EMBER_ZCL_STATUS_SUCCESS;
 }
 
-// If dataPtr is NULL, no data is copied to the caller.
-// readLength should be 0 in that case.
-
 EmberAfStatus emAfReadAttribute(EndpointId endpoint, ClusterId cluster, AttributeId attributeID, uint8_t * dataPtr,
-                                uint16_t readLength, EmberAfAttributeType * dataType)
+                                uint16_t readLength)
 {
     const EmberAfAttributeMetadata * metadata = nullptr;
     EmberAfAttributeSearchRecord record;
@@ -316,20 +260,10 @@ EmberAfStatus emAfReadAttribute(EndpointId endpoint, ClusterId cluster, Attribut
     status             = emAfReadOrWriteAttribute(&record, &metadata, dataPtr, readLength,
                                                   false); // write?
 
-    if (status == EMBER_ZCL_STATUS_SUCCESS)
+    // failed, print debug info
+    if (status == EMBER_ZCL_STATUS_RESOURCE_EXHAUSTED)
     {
-        // It worked!  If the user asked for the type, set it before returning.
-        if (dataType != nullptr)
-        {
-            (*dataType) = metadata->attributeType;
-        }
-    }
-    else
-    { // failed, print debug info
-        if (status == EMBER_ZCL_STATUS_RESOURCE_EXHAUSTED)
-        {
-            ChipLogProgress(Zcl, "READ: attribute size too large for caller");
-        }
+        ChipLogProgress(Zcl, "READ: attribute size too large for caller");
     }
 
     return status;

src/app/util/attribute-table.h

@@ -25,8 +25,47 @@
 EmberAfStatus emberAfWriteAttributeExternal(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
                                             uint8_t * dataPtr, EmberAfAttributeType dataType);
 
+/**
+ * @brief write an attribute, performing all the checks.
+ *
+ * This function will attempt to write the attribute value from
+ * the provided pointer. This function will only check that the
+ * attribute exists. If it does it will write the value into
+ * the attribute table for the given attribute.
+ *
+ * This function will not check to see if the attribute is
+ * writable since the read only / writable characteristic
+ * of an attribute only pertains to external devices writing
+ * over the air. Because this function is being called locally
+ * it assumes that the device knows what it is doing and has permission
+ * to perform the given operation.
+ *
+ * if true is passed in for overrideReadOnlyAndDataType then the data type is
+ * not checked and the read-only flag is ignored. This mode is meant for
+ * testing or setting the initial value of the attribute on the device.
+ *
+ * this returns:
+ * - EMBER_ZCL_STATUS_UNSUPPORTED_ENDPOINT: if endpoint isn't supported by the device.
+ * - EMBER_ZCL_STATUS_UNSUPPORTED_CLUSTER: if cluster isn't supported on the endpoint.
+ * - EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE: if attribute isn't supported in the cluster.
+ * - EMBER_ZCL_STATUS_INVALID_DATA_TYPE: if the data type passed in doesnt match the type
+ *           stored in the attribute table
+ * - EMBER_ZCL_STATUS_UNSUPPORTED_WRITE: if the attribute isnt writable
+ * - EMBER_ZCL_STATUS_CONSTRAINT_ERROR: if the value is set out of the allowable range for
+ *           the attribute
+ * - EMBER_ZCL_STATUS_SUCCESS: if the attribute was found and successfully written
+ */
 EmberAfStatus emAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID, uint8_t * data,
-                                 EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType, bool justTest);
+                                 EmberAfAttributeType dataType, bool overrideReadOnlyAndDataType);
 
+/**
+ * @brief Read the attribute value, performing all the checks.
+ *
+ * This function will attempt to read the attribute and store it into the
+ * pointer.
+ *
+ * dataPtr may be NULL, signifying that we don't need the value, just the status
+ * (i.e. whether the attribute can be read).
+ */
 EmberAfStatus emAfReadAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID,
-                                uint8_t * dataPtr, uint16_t readLength, EmberAfAttributeType * dataType);
+                                uint8_t * dataPtr, uint16_t readLength);

src/app/util/generic-callback-stubs.cpp

@@ -24,13 +24,6 @@ using namespace chip;
 
 // This file contains overridable callbacks that are not cluster specific.
 
-EmberAfAttributeWritePermission __attribute__((weak))
-emberAfAllowNetworkWriteAttributeCallback(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId, uint8_t * value,
-                                          uint8_t type)
-{
-    return EmberAfAttributeWritePermission::AllowWriteNormal; // Default
-}
-
 bool __attribute__((weak)) emberAfAttributeReadAccessCallback(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId)
 {
     return true;

src/app/util/generic-callbacks.h

@@ -39,35 +39,6 @@
  */
 void emberAfClusterInitCallback(chip::EndpointId endpoint, chip::ClusterId clusterId);
 
-/** @brief Allow Network Write Attribute
- *
- * This function is called by the application framework before it writes an
- * attribute in response to a write attribute request from an external device.
- * The value passed into this callback is the value to which the attribute is to
- * be set by the framework.
-        Example:    In mirroring simple metering data
- * on an Energy Services Interface (ESI) (formerly called Energy Service Portal
- * (ESP) in SE 1.0).), a mirrored simple meter needs to write read-only
- * attributes on its mirror. The-meter-mirror sample application, located in
- * app/framework/sample-apps, uses this callback to allow the mirrored device to
- * write simple metering attributes on the mirror regardless of the fact that
- * most simple metering attributes are defined as read-only by the ZigBee
- * specification.
-        Note:   The ZCL specification does not (as of this
- * writing) specify any permission-level security for writing writeable
- * attributes. As far as the ZCL specification is concerned, if an attribute is
- * writeable, any device that has a link key for the device should be able to
- * write that attribute. Furthermore if an attribute is read only, it should not
- * be written over the air. Thus, if you implement permissions for writing
- * attributes as a feature, you MAY be operating outside the specification. This
- * is unlikely to be a problem for writing read-only attributes, but it may be a
- * problem for attributes that are writeable according to the specification but
- * restricted by the application implementing this callback.
- */
-EmberAfAttributeWritePermission emberAfAllowNetworkWriteAttributeCallback(chip::EndpointId endpoint, chip::ClusterId clusterId,
-                                                                          chip::AttributeId attributeId, uint8_t * value,
-                                                                          uint8_t type);
-
 /** @brief Attribute Read Access
  *
  * This function is called whenever the Application Framework needs to check

src/app/zap-templates/templates/app/attributes/Accessors-src.zapt

@@ -10,8 +10,8 @@
 #include <app-common/zap-generated/attribute-type.h>
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app-common/zap-generated/ids/Clusters.h>
-#include <app/util/af.h>
 #include <app/util/attribute-storage-null-handling.h>
+#include <app/util/attribute-table.h>
 #include <app/util/odd-sized-integers.h>
 #include <lib/core/CHIPEncoding.h>
 
@@ -40,7 +40,7 @@ EmberAfStatus Get(chip::EndpointId endpoint, {{accessorGetterType this}} value)
     {{~#if (isString type)}}
     {{~#*inline "lengthType"}}uint{{#if (isShortString type)}}8{{else}}16{{/if}}_t{{/inline}}
     uint8_t zclString[{{maxLength}} + {{>sizingBytes}}];
-    EmberAfStatus status = emberAfReadAttribute(endpoint, {{>clusterId}}, Id, zclString, sizeof(zclString));
+    EmberAfStatus status = emAfReadAttribute(endpoint, {{>clusterId}}, Id, zclString, sizeof(zclString));
     VerifyOrReturnError(EMBER_ZCL_STATUS_SUCCESS == status, status);
     size_t length = emberAf{{#if (isLongString type)}}Long{{/if}}StringLength(zclString);
     if (length == NumericAttributeTraits<{{>lengthType}}>::kNullValue)
@@ -64,7 +64,7 @@ EmberAfStatus Get(chip::EndpointId endpoint, {{accessorGetterType this}} value)
     using Traits = NumericAttributeTraits<{{accessorTraitType type}}>;
     Traits::StorageType temp;
     uint8_t * readable = Traits::ToAttributeStoreRepresentation(temp);
-    EmberAfStatus status = emberAfReadAttribute(endpoint, {{>clusterId}}, Id, readable, sizeof(temp));
+    EmberAfStatus status = emAfReadAttribute(endpoint, {{>clusterId}}, Id, readable, sizeof(temp));
     VerifyOrReturnError(EMBER_ZCL_STATUS_SUCCESS == status, status);
     {{#if isNullable}}
     if (Traits::IsNullValue(temp))