Use ConcreteAttributePath for reads below ReadSingleClusterData. (#10397

)

The ReadSingleClusterData in TestReportingEngine.cpp was not being
used, because it had the wrong signature.  And making it be used
by fixing the signature apparently makes the test fail...

src/app/AttributeAccessInterface.h

@@ -18,7 +18,7 @@
 
 #pragma once
 
-#include <app/ClusterInfo.h>
+#include <app/ConcreteAttributePath.h>
 #include <app/MessageDef/AttributeDataElement.h>
 #include <app/data-model/Encode.h>
 #include <app/data-model/List.h> // So we can encode lists
@@ -108,7 +108,7 @@ class AttributeAccessInterface
     /**
      * Callback for reading attributes.
      *
-     * @param [in] aClusterInfo indicates which exact data is being read.
+     * @param [in] aPath indicates which exact data is being read.
      * @param [in] aEncoder the AttributeValueEncoder to use for encoding the
      *             data.  If this function returns scucess and no attempt is
      *             made to encode data using aEncoder, the
@@ -117,7 +117,7 @@ class AttributeAccessInterface
      *             This may involve reading from the attribute store or external
      *             attribute callbacks.
      */
-    virtual CHIP_ERROR Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder) = 0;
+    virtual CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) = 0;
 
     /**
      * Mechanism for keeping track of a chain of AttributeAccessInterfaces.

src/app/InteractionModelEngine.h

@@ -42,6 +42,7 @@
 #include <app/Command.h>
 #include <app/CommandHandler.h>
 #include <app/CommandSender.h>
+#include <app/ConcreteAttributePath.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/InteractionModelDelegate.h>
 #include <app/ReadClient.h>
@@ -239,15 +240,19 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath);
  *  This function is implemented by CHIP as a part of cluster data storage & management.
  * The apWriter and apDataExists can be nullptr.
  *
- *  @param[in]    aClusterInfo      The cluster info object, for the path of cluster data.
+ *  @param[in]    aPath             The concrete path of the data being read.
  *  @param[in]    apWriter          The TLVWriter for holding cluster data. Can be a nullptr if the caller does not care
  *                                  the exact value of the attribute.
  *  @param[out]   apDataExists      Tell whether the cluster data exist on server. Can be a nullptr if the caller does not care
  *                                  whether the data exists.
  *
  *  @retval  CHIP_NO_ERROR on success
  */
-CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * apWriter, bool * apDataExists);
+CHIP_ERROR ReadSingleClusterData(const ConcreteAttributePath & aPath, TLV::TLVWriter * apWriter, bool * apDataExists);
+
+/**
+ * TODO: Document.
+ */
 CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & aReader, WriteHandler * apWriteHandler);
 } // namespace app
 } // namespace chip

src/app/clusters/ethernet_network_diagnostics_server/ethernet_network_diagnostics_server.cpp

@@ -42,7 +42,7 @@ class EthernetDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the EthernetNetworkDiagnostics cluster on all endpoints.
     EthernetDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), EthernetNetworkDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder) override;
+    CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
 private:
     template <typename T>
@@ -69,15 +69,15 @@ CHIP_ERROR EthernetDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (Connectivit
 
 EthernetDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR EthernetDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder)
+CHIP_ERROR EthernetDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
-    if (aClusterInfo.mClusterId != EthernetNetworkDiagnostics::Id)
+    if (aPath.mClusterId != EthernetNetworkDiagnostics::Id)
     {
         // We shouldn't have been called at all.
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    switch (aClusterInfo.mFieldId)
+    switch (aPath.mAttributeId)
     {
     case PHYRate::Id: {
         return ReadIfSupported(&ConnectivityManager::GetEthPHYRate, aEncoder);

src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp

@@ -35,7 +35,7 @@ class GeneralDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the GeneralDiagnostics cluster on all endpoints.
     GeneralDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), GeneralDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder) override;
+    CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
 private:
     template <typename T>
@@ -62,15 +62,15 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (PlatformMana
 
 GeneralDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR GeneralDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder)
+CHIP_ERROR GeneralDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
-    if (aClusterInfo.mClusterId != GeneralDiagnostics::Id)
+    if (aPath.mClusterId != GeneralDiagnostics::Id)
     {
         // We shouldn't have been called at all.
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    switch (aClusterInfo.mFieldId)
+    switch (aPath.mAttributeId)
     {
     case RebootCount::Id: {
         return ReadIfSupported(&PlatformManager::GetRebootCount, aEncoder);

src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp

@@ -42,23 +42,23 @@ class SoftwareDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the SoftwareDiagnostics cluster on all endpoints.
     SoftwareDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), SoftwareDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder) override;
+    CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
 private:
     CHIP_ERROR ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(uint64_t &), AttributeValueEncoder & aEncoder);
 };
 
 SoftwareDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder)
+CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
-    if (aClusterInfo.mClusterId != SoftwareDiagnostics::Id)
+    if (aPath.mClusterId != SoftwareDiagnostics::Id)
     {
         // We shouldn't have been called at all.
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    switch (aClusterInfo.mFieldId)
+    switch (aPath.mAttributeId)
     {
     case CurrentHeapFree::Id: {
         return ReadIfSupported(&PlatformManager::GetCurrentHeapFree, aEncoder);

src/app/clusters/thread_network_diagnostics_server/thread_network_diagnostics_server.cpp

@@ -46,20 +46,20 @@ class ThreadDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the ThreadNetworkDiagnostics cluster on all endpoints.
     ThreadDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), ThreadNetworkDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder) override;
+    CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 };
 
 ThreadDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR ThreadDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder)
+CHIP_ERROR ThreadDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
-    if (aClusterInfo.mClusterId != ThreadNetworkDiagnostics::Id)
+    if (aPath.mClusterId != ThreadNetworkDiagnostics::Id)
     {
         // We shouldn't have been called at all.
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    CHIP_ERROR err = ConnectivityMgr().WriteThreadNetworkDiagnosticAttributeToTlv(aClusterInfo.mFieldId, aEncoder);
+    CHIP_ERROR err = ConnectivityMgr().WriteThreadNetworkDiagnosticAttributeToTlv(aPath.mAttributeId, aEncoder);
 
     // If it isn't a run time assigned attribute, e.g. ClusterRevision, or if
     // not implemented, clear the error so we fall back to the standard read

src/app/clusters/wifi_network_diagnostics_server/wifi_network_diagnostics_server.cpp

@@ -42,7 +42,7 @@ class WiFiDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the WiFiNetworkDiagnostics cluster on all endpoints.
     WiFiDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), WiFiNetworkDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder) override;
+    CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
 private:
     template <typename T>
@@ -69,15 +69,15 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (ConnectivityMan
 
 WiFiDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR WiFiDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
-    if (aClusterInfo.mClusterId != WiFiNetworkDiagnostics::Id)
+    if (aPath.mClusterId != WiFiNetworkDiagnostics::Id)
     {
         // We shouldn't have been called at all.
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    switch (aClusterInfo.mFieldId)
+    switch (aPath.mAttributeId)
     {
     case Attributes::SecurityType::Id: {
         return ReadIfSupported(&ConnectivityManager::GetWiFiSecurityType, aEncoder);

src/app/reporting/Engine.cpp

@@ -64,7 +64,8 @@ EventNumber Engine::CountEvents(ReadHandler * apReadHandler, EventNumber * apIni
 CHIP_ERROR
 Engine::RetrieveClusterData(AttributeDataList::Builder & aAttributeDataList, ClusterInfo & aClusterInfo)
 {
-    CHIP_ERROR err                                            = CHIP_NO_ERROR;
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    ConcreteAttributePath path(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mFieldId);
     AttributeDataElement::Builder attributeDataElementBuilder = aAttributeDataList.CreateAttributeDataElementBuilder();
     AttributePath::Builder attributePathBuilder               = attributeDataElementBuilder.CreateAttributePathBuilder();
     attributePathBuilder.NodeId(aClusterInfo.mNodeId)
@@ -78,7 +79,7 @@ Engine::RetrieveClusterData(AttributeDataList::Builder & aAttributeDataList, Clu
     ChipLogDetail(DataManagement, "<RE:Run> Cluster %" PRIx32 ", Field %" PRIx32 " is dirty", aClusterInfo.mClusterId,
                   aClusterInfo.mFieldId);
 
-    err = ReadSingleClusterData(aClusterInfo, attributeDataElementBuilder.GetWriter(), nullptr /* data exists */);
+    err = ReadSingleClusterData(path, attributeDataElementBuilder.GetWriter(), nullptr /* data exists */);
     SuccessOrExit(err);
     attributeDataElementBuilder.MoreClusterData(false);
     attributeDataElementBuilder.EndOfAttributeDataElement();

src/app/tests/TestReadInteraction.cpp

@@ -22,6 +22,7 @@
  *
  */
 
+#include <app/AttributeAccessInterface.h>
 #include <app/InteractionModelEngine.h>
 #include <app/MessageDef/EventDataElement.h>
 #include <app/util/basic-types.h>
@@ -230,29 +231,28 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate
 
 namespace chip {
 namespace app {
-CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * apWriter, bool * apDataExists)
+CHIP_ERROR ReadSingleClusterData(const ConcreteAttributePath & aPath, TLV::TLVWriter * apWriter, bool * apDataExists)
 {
     uint64_t version = 0;
-    ChipLogDetail(DataManagement, "TEST Cluster %" PRIx32 ", Field %" PRIx32 " is dirty", aClusterInfo.mClusterId,
-                  aClusterInfo.mFieldId);
+    ChipLogDetail(DataManagement, "TEST Cluster %" PRIx32 ", Field %" PRIx32 " is dirty", aPath.mClusterId, aPath.mAttributeId);
 
     if (apDataExists != nullptr)
     {
-        *apDataExists = (aClusterInfo.mClusterId == kTestClusterId && aClusterInfo.mEndpointId == kTestEndpointId);
+        *apDataExists = (aPath.mClusterId == kTestClusterId && aPath.mEndpointId == kTestEndpointId);
     }
 
     if (apWriter == nullptr)
     {
         return CHIP_NO_ERROR;
     }
 
-    if (!(aClusterInfo.mClusterId == kTestClusterId && aClusterInfo.mEndpointId == kTestEndpointId))
+    if (!(aPath.mClusterId == kTestClusterId && aPath.mEndpointId == kTestEndpointId))
     {
         return apWriter->Put(chip::TLV::ContextTag(AttributeDataElement::kCsTag_Status),
                              chip::Protocols::InteractionModel::Status::UnsupportedAttribute);
     }
 
-    ReturnErrorOnFailure(apWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_Data), kTestFieldValue1));
+    ReturnErrorOnFailure(AttributeValueEncoder(apWriter).Encode(kTestFieldValue1));
     return apWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_DataVersion), version);
 }
 

src/app/tests/TestReportingEngine.cpp

@@ -22,6 +22,7 @@
  *
  */
 
+#include <app/ConcreteAttributePath.h>
 #include <app/InteractionModelEngine.h>
 #include <app/reporting/Engine.h>
 #include <lib/core/CHIPCore.h>
@@ -54,32 +55,8 @@ constexpr ClusterId kTestClusterId        = 6;
 constexpr EndpointId kTestEndpointId      = 1;
 constexpr chip::AttributeId kTestFieldId1 = 1;
 constexpr chip::AttributeId kTestFieldId2 = 2;
-constexpr uint8_t kTestFieldValue1        = 1;
-constexpr uint8_t kTestFieldValue2        = 2;
 
 namespace app {
-CHIP_ERROR ReadSingleClusterData(AttributePathParams & aAttributePathParams, TLV::TLVWriter * apWriter, bool * apDataExists)
-{
-    CHIP_ERROR err = CHIP_NO_ERROR;
-    VerifyOrExit(aAttributePathParams.mClusterId == kTestClusterId && aAttributePathParams.mEndpointId == kTestEndpointId,
-                 err = CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrExit(apWriter != nullptr, /* no op */);
-
-    if (aAttributePathParams.mFieldId == kTestFieldId1)
-    {
-        err = apWriter->Put(TLV::ContextTag(kTestFieldId1), kTestFieldValue1);
-        SuccessOrExit(err);
-    }
-    if (aAttributePathParams.mFieldId == kTestFieldId2)
-    {
-        err = apWriter->Put(TLV::ContextTag(kTestFieldId2), kTestFieldValue2);
-        SuccessOrExit(err);
-    }
-
-exit:
-    return err;
-}
-
 namespace reporting {
 class TestReportingEngine
 {

src/app/tests/integration/chip_im_initiator.cpp

@@ -27,6 +27,7 @@
 #include "system/SystemClock.h"
 #include <app/CommandHandler.h>
 #include <app/CommandSender.h>
+#include <app/ConcreteAttributePath.h>
 #include <app/InteractionModelEngine.h>
 #include <app/tests/integration/common.h>
 #include <chrono>
@@ -635,7 +636,7 @@ void DispatchSingleClusterResponseCommand(const ConcreteCommandPath & aCommandPa
     gLastCommandResult = TestCommandResult::kSuccess;
 }
 
-CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * apWriter, bool * apDataExists)
+CHIP_ERROR ReadSingleClusterData(const ConcreteAttributePath & aPath, TLV::TLVWriter * apWriter, bool * apDataExists)
 {
     // We do not really care about the value, just return a not found status code.
     VerifyOrReturnError(apWriter != nullptr, CHIP_NO_ERROR);

src/app/tests/integration/chip_im_responder.cpp

@@ -25,6 +25,7 @@
  */
 
 #include "MockEvents.h"
+#include <app/AttributeAccessInterface.h>
 #include <app/CommandHandler.h>
 #include <app/CommandSender.h>
 #include <app/EventManagement.h>
@@ -110,15 +111,14 @@ void DispatchSingleClusterResponseCommand(const ConcreteCommandPath & aCommandPa
     // Nothing todo.
 }
 
-CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * apWriter, bool * apDataExists)
+CHIP_ERROR ReadSingleClusterData(const ConcreteAttributePath & aPath, TLV::TLVWriter * apWriter, bool * apDataExists)
 {
     CHIP_ERROR err   = CHIP_NO_ERROR;
     uint64_t version = 0;
-    VerifyOrExit(aClusterInfo.mClusterId == kTestClusterId && aClusterInfo.mEndpointId == kTestEndpointId,
-                 err = CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrExit(aPath.mClusterId == kTestClusterId && aPath.mEndpointId == kTestEndpointId, err = CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrExit(apWriter != nullptr, /* no op */);
 
-    err = apWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_Data), kTestFieldValue1);
+    err = AttributeValueEncoder(apWriter).Encode(kTestFieldValue1);
     SuccessOrExit(err);
     err = apWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_DataVersion), version);
 

src/app/util/ember-compatibility-functions.cpp

@@ -23,6 +23,7 @@
 
 #include <app/ClusterInfo.h>
 #include <app/Command.h>
+#include <app/ConcreteAttributePath.h>
 #include <app/InteractionModelEngine.h>
 #include <app/reporting/Engine.h>
 #include <app/util/af.h>
@@ -189,21 +190,19 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath)
     return emberAfContainsServer(aCommandPath.mEndpointId, aCommandPath.mClusterId);
 }
 
-CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * apWriter, bool * apDataExists)
+CHIP_ERROR ReadSingleClusterData(const ConcreteAttributePath & aPath, TLV::TLVWriter * apWriter, bool * apDataExists)
 {
     ChipLogDetail(DataManagement,
-                  "Received Cluster Command: Cluster=" ChipLogFormatMEI " NodeId=0x" ChipLogFormatX64 " Endpoint=%" PRIx16
-                  " AttributeId=%" PRIx32 " ListIndex=%" PRIx16,
-                  ChipLogValueMEI(aClusterInfo.mClusterId), ChipLogValueX64(aClusterInfo.mNodeId), aClusterInfo.mEndpointId,
-                  aClusterInfo.mFieldId, aClusterInfo.mListIndex);
+                  "Reading attribute: Cluster=" ChipLogFormatMEI " Endpoint=%" PRIx16 " AttributeId=" ChipLogFormatMEI,
+                  ChipLogValueMEI(aPath.mClusterId), aPath.mEndpointId, ChipLogValueMEI(aPath.mAttributeId));
 
-    AttributeAccessInterface * attrOverride = findAttributeAccessOverride(aClusterInfo.mEndpointId, aClusterInfo.mClusterId);
+    AttributeAccessInterface * attrOverride = findAttributeAccessOverride(aPath.mEndpointId, aPath.mClusterId);
     if (attrOverride != nullptr)
     {
         // TODO: We should probably clone the writer and convert failures here
         // into status responses, unless our caller already does that.
         AttributeValueEncoder valueEncoder(apWriter);
-        ReturnErrorOnFailure(attrOverride->Read(aClusterInfo, valueEncoder));
+        ReturnErrorOnFailure(attrOverride->Read(aPath, valueEncoder));
 
         if (valueEncoder.TriedEncode())
         {
@@ -223,8 +222,8 @@ CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * ap
 
     EmberAfAttributeType attributeType;
     EmberAfStatus status;
-    status = emberAfReadAttribute(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mFieldId, CLUSTER_MASK_SERVER,
-                                  attributeData, sizeof(attributeData), &attributeType);
+    status = emberAfReadAttribute(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, CLUSTER_MASK_SERVER, attributeData,
+                                  sizeof(attributeData), &attributeType);
 
     if (apDataExists != nullptr)
     {
@@ -349,9 +348,9 @@ CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * ap
         ReturnErrorOnFailure(
             apWriter->StartContainer(TLV::ContextTag(AttributeDataElement::kCsTag_Data), TLV::kTLVType_List, containerType));
         // TODO: Encode data in TLV, now raw buffers
-        ReturnErrorOnFailure(apWriter->PutBytes(
-            TLV::AnonymousTag, attributeData,
-            emberAfAttributeValueSize(aClusterInfo.mClusterId, aClusterInfo.mFieldId, attributeType, attributeData)));
+        ReturnErrorOnFailure(
+            apWriter->PutBytes(TLV::AnonymousTag, attributeData,
+                               emberAfAttributeValueSize(aPath.mClusterId, aPath.mAttributeId, attributeType, attributeData)));
         ReturnErrorOnFailure(apWriter->EndContainer(containerType));
         break;
     }