Move emAfLoadAttributeDefaults to private inside attribute-storage.cpp (

#31427)

* Move emAfLoadAttributeDefaults to private.

Reduce the API surface for ember attribute storage:
- remove unused ability to "load defaults without loading persistence"
- make the full loadDefautls + cluster private for now as it is not used

Looking to reduce the API surface of ember attribute storage to consider
making it pluggable/modular (to allow for better unit testing support).

* Add extra static to method

* Replace a usage of emAfLoadAttributeDefaults

* Update test cluster docs

* Fix verifyordie logic

* Remove one more unused  (not even implemented) function

* Fix macro usage

* Move 2 more methods outside public API

* Restyle

* More strict const in readorwriteattribute

examples/all-clusters-app/all-clusters-common/all-clusters-app.matter

@@ -6742,7 +6742,8 @@ internal cluster UnitTesting = 4294048773 {
     int8u fillCharacter = 2;
   }
 
-  /** Simple command without any parameters and without a specific response */
+  /** Simple command without any parameters and without a specific response.
+        To aid in unit testing, this command will re-initialize attribute storage to defaults. */
   command Test(): DefaultSuccess = 0;
   /** Simple command without any parameters and without a specific response not handled by the server */
   command TestNotHandled(): DefaultSuccess = 1;

examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter

@@ -5812,7 +5812,8 @@ internal cluster UnitTesting = 4294048773 {
     int8u fillCharacter = 2;
   }
 
-  /** Simple command without any parameters and without a specific response */
+  /** Simple command without any parameters and without a specific response.
+        To aid in unit testing, this command will re-initialize attribute storage to defaults. */
   command Test(): DefaultSuccess = 0;
   /** Simple command without any parameters and without a specific response not handled by the server */
   command TestNotHandled(): DefaultSuccess = 1;

src/app/clusters/test-cluster-server/test-cluster-server.cpp

@@ -720,7 +720,7 @@ bool emberAfUnitTestingClusterTestCallback(app::CommandHandler * commandObj, con
                                            const Clusters::UnitTesting::Commands::Test::DecodableType & commandData)
 {
     // Setup the test variables
-    emAfLoadAttributeDefaults(commandPath.mEndpointId, true, MakeOptional(commandPath.mClusterId));
+    emberAfInitializeAttributes(commandPath.mEndpointId);
     for (int i = 0; i < kAttributeListLength; ++i)
     {
         gListUint8Data[i] = 0;

src/app/util/attribute-storage.cpp

@@ -56,6 +56,15 @@ EmberAfDefinedEndpoint emAfEndpoints[MAX_ENDPOINT_COUNT];
 
 uint8_t attributeData[ACTUAL_ATTRIBUTE_SIZE];
 
+// ----- internal-only methods, not part of the external API -----
+
+// Loads the attributes from built-in default and storage.
+static void emAfLoadAttributeDefaults(chip::EndpointId endpoint, chip::Optional<chip::ClusterId> = chip::NullOptional);
+
+static bool emAfMatchCluster(const EmberAfCluster * cluster, const EmberAfAttributeSearchRecord * attRecord);
+static bool emAfMatchAttribute(const EmberAfCluster * cluster, const EmberAfAttributeMetadata * am,
+                               const EmberAfAttributeSearchRecord * attRecord);
+
 namespace {
 
 #if (!defined(ATTRIBUTE_SINGLETONS_SIZE)) || (ATTRIBUTE_SINGLETONS_SIZE == 0)
@@ -536,7 +545,7 @@ static EmberAfStatus typeSensitiveMemCopy(ClusterId clusterId, uint8_t * dest, u
  *   1. Cluster ids match AND
  *   2. Cluster is a server cluster (because there are no client attributes).
  */
-bool emAfMatchCluster(const EmberAfCluster * cluster, EmberAfAttributeSearchRecord * attRecord)
+bool emAfMatchCluster(const EmberAfCluster * cluster, const EmberAfAttributeSearchRecord * attRecord)
 {
     return (cluster->clusterId == attRecord->clusterId && (cluster->mask & CLUSTER_MASK_SERVER));
 }
@@ -549,7 +558,7 @@ bool emAfMatchCluster(const EmberAfCluster * cluster, EmberAfAttributeSearchReco
  * Attributes match if attr ids match.
  */
 bool emAfMatchAttribute(const EmberAfCluster * cluster, const EmberAfAttributeMetadata * am,
-                        EmberAfAttributeSearchRecord * attRecord)
+                        const EmberAfAttributeSearchRecord * attRecord)
 {
     return (am->attributeId == attRecord->attributeId);
 }
@@ -567,7 +576,7 @@ bool emAfMatchAttribute(const EmberAfCluster * cluster, const EmberAfAttributeMe
 // type.  For strings, the function will copy as many bytes as will fit in the
 // attribute.  This means the resulting string may be truncated.  The length
 // byte(s) in the resulting string will reflect any truncated.
-EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord, const EmberAfAttributeMetadata ** metadata,
+EmberAfStatus emAfReadOrWriteAttribute(const EmberAfAttributeSearchRecord * attRecord, const EmberAfAttributeMetadata ** metadata,
                                        uint8_t * buffer, uint16_t readLength, bool write)
 {
     assertChipStackLockedByCurrentThread();
@@ -1187,23 +1196,18 @@ uint8_t emberAfGetClustersFromEndpoint(EndpointId endpoint, ClusterId * clusterL
 
 void emberAfInitializeAttributes(EndpointId endpoint)
 {
-    emAfLoadAttributeDefaults(endpoint, false);
+    emAfLoadAttributeDefaults(endpoint);
 }
 
-void emberAfResetAttributes(EndpointId endpoint)
-{
-    emAfLoadAttributeDefaults(endpoint, true);
-}
-
-void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional<ClusterId> clusterId)
+void emAfLoadAttributeDefaults(EndpointId endpoint, Optional<ClusterId> clusterId)
 {
     uint16_t ep;
     uint8_t clusterI;
     uint16_t attr;
     uint8_t * ptr;
     uint16_t epCount = emberAfEndpointCount();
     uint8_t attrData[ATTRIBUTE_LARGEST];
-    auto * attrStorage = ignoreStorage ? nullptr : app::GetAttributePersistenceProvider();
+    auto * attrStorage = app::GetAttributePersistenceProvider();
     // Don't check whether we actually have an attrStorage here, because it's OK
     // to have one if none of our attributes have NVM storage.
 
@@ -1245,9 +1249,9 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
                 ptr                                 = nullptr; // Will get set to the value to write, as needed.
 
                 // First check for a persisted value.
-                if (!ignoreStorage && am->IsAutomaticallyPersisted())
+                if (am->IsAutomaticallyPersisted())
                 {
-                    VerifyOrDie(attrStorage && "Attribute persistence needs a persistence provider");
+                    VerifyOrDieWithMsg(attrStorage != nullptr, Zcl, "Attribute persistence needs a persistence provider");
                     MutableByteSpan bytes(attrData);
                     CHIP_ERROR err = attrStorage->ReadValue(
                         app::ConcreteAttributePath(de->endpoint, cluster->clusterId, am->attributeId), am, bytes);
@@ -1328,10 +1332,6 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
                                              ptr,
                                              0,     // buffer size - unused
                                              true); // write?
-                    if (ignoreStorage)
-                    {
-                        emAfSaveAttributeToStorageIfNeeded(ptr, de->endpoint, record.clusterId, am);
-                    }
                 }
             }
         }

src/app/util/attribute-storage.h

@@ -95,13 +95,9 @@ void emAfCallInits(void);
 // Initial configuration
 void emberAfEndpointConfigure(void);
 
-EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord, const EmberAfAttributeMetadata ** metadata,
+EmberAfStatus emAfReadOrWriteAttribute(const EmberAfAttributeSearchRecord * attRecord, const EmberAfAttributeMetadata ** metadata,
                                        uint8_t * buffer, uint16_t readLength, bool write);
 
-bool emAfMatchCluster(const EmberAfCluster * cluster, EmberAfAttributeSearchRecord * attRecord);
-bool emAfMatchAttribute(const EmberAfCluster * cluster, const EmberAfAttributeMetadata * am,
-                        EmberAfAttributeSearchRecord * attRecord);
-
 // Check if a cluster is implemented or not. If yes, the cluster is returned.
 //
 // mask = 0 -> find either client or server
@@ -156,14 +152,8 @@ const EmberAfCluster * emberAfFindClusterIncludingDisabledEndpoints(chip::Endpoi
 // cast it.
 EmberAfGenericClusterFunction emberAfFindClusterFunction(const EmberAfCluster * cluster, EmberAfClusterMask functionMask);
 
-// Public APIs for loading attributes
+// Loads attribute defaults and any non-volatile attributes stored
 void emberAfInitializeAttributes(chip::EndpointId endpoint);
-void emberAfResetAttributes(chip::EndpointId endpoint);
-
-// Loads the attributes from built-in default and / or storage.  If
-// ignoreStorage is true, only defaults will be read, and the storage for
-// non-volatile attributes will be overwritten with those defaults.
-void emAfLoadAttributeDefaults(chip::EndpointId endpoint, bool ignoreStorage, chip::Optional<chip::ClusterId> = chip::NullOptional);
 
 // After the RAM value has changed, code should call this function. If this
 // attribute has been tagged as non-volatile, its value will be stored.
@@ -177,10 +167,6 @@ void emAfClusterAttributeChangedCallback(const chip::app::ConcreteAttributePath
 EmberAfStatus emAfClusterPreAttributeChangedCallback(const chip::app::ConcreteAttributePath & attributePath,
                                                      EmberAfAttributeType attributeType, uint16_t size, uint8_t * value);
 
-// Checks a cluster mask byte against ticks passed bitmask
-// returns true if the mask matches a passed interval
-bool emberAfCheckTick(EmberAfClusterMask mask, uint8_t passedMask);
-
 // Check whether there is an endpoint defined with the given endpoint id that is
 // enabled.
 bool emberAfEndpointIsEnabled(chip::EndpointId endpoint);

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml

@@ -240,7 +240,8 @@ limitations under the License.
     <!-- Test Commands -->
     <command source="client" code="0x00" name="Test" optional="false">
       <description>
-        Simple command without any parameters and without a specific response
+        Simple command without any parameters and without a specific response.
+        To aid in unit testing, this command will re-initialize attribute storage to defaults.
       </description>
     </command>
 

src/controller/data_model/controller-clusters.matter

@@ -8929,7 +8929,8 @@ internal cluster UnitTesting = 4294048773 {
     int8u fillCharacter = 2;
   }
 
-  /** Simple command without any parameters and without a specific response */
+  /** Simple command without any parameters and without a specific response.
+        To aid in unit testing, this command will re-initialize attribute storage to defaults. */
   command Test(): DefaultSuccess = 0;
   /** Simple command without any parameters and without a specific response not handled by the server */
   command TestNotHandled(): DefaultSuccess = 1;