Make the attribute access Read method a bit easier to implement. (#9976)

Consumers should not have to worry about what the right TLV tag is for
the attribute value, or exactly how to put their data into the TLV
writer.  We can use a partially applied version of the Encode generic
for this.

Fixes: #9938

Also fixes an issue around the handling of null TLVWriter and the
apDataExists outparam in ReadSingleClusterData.

src/app/AttributeAccessInterface.h

@@ -19,6 +19,8 @@
 #pragma once
 
 #include <app/ClusterInfo.h>
+#include <app/MessageDef/AttributeDataElement.h>
+#include <app/data-model/Encode.h>
 #include <app/util/basic-types.h>
 #include <lib/core/CHIPTLV.h>
 #include <lib/core/Optional.h>
@@ -36,6 +38,29 @@
 namespace chip {
 namespace app {
 
+class AttributeValueEncoder
+{
+public:
+    AttributeValueEncoder(TLV::TLVWriter * aWriter) : mWriter(aWriter) {}
+
+    template <typename... Ts>
+    CHIP_ERROR Encode(Ts... aArgs) const
+    {
+        if (mWriter == nullptr)
+        {
+            return CHIP_NO_ERROR;
+        }
+        return DataModel::Encode(*mWriter, TLV::ContextTag(AttributeDataElement::kCsTag_Data), std::forward<Ts>(aArgs)...);
+    }
+
+    // For consumers that can't just do a single Encode call for some reason
+    // (e.g. they're encoding a list a bit at a time).
+    TLV::TLVWriter * GetWriter() const { return mWriter; }
+
+private:
+    TLV::TLVWriter * mWriter;
+};
+
 class AttributeAccessInterface
 {
 public:
@@ -63,7 +88,7 @@ class AttributeAccessInterface
      *                        may involve reading from the attribute store or
      *                        external attribute callbacks.
      */
-    virtual CHIP_ERROR Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead) = 0;
+    virtual CHIP_ERROR Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead) = 0;
 
     /**
      * Mechanism for keeping track of a chain of AttributeAccessInterfaces.

src/app/clusters/ethernet_network_diagnostics_server/ethernet_network_diagnostics_server.cpp

@@ -40,15 +40,15 @@ class EthernetDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the EthernetNetworkDiagnostics cluster on all endpoints.
     EthernetDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), EthernetNetworkDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead) override;
+    CHIP_ERROR Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead) override;
 
 private:
-    CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConnectivityManager::*getter)(uint64_t &), TLV::TLVWriter * aWriter);
+    CHIP_ERROR ReadIfSupported(CHIP_ERROR (ConnectivityManager::*getter)(uint64_t &), const AttributeValueEncoder & aEncoder);
 };
 
 EthernetDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR EthernetDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead)
+CHIP_ERROR EthernetDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead)
 {
     if (aClusterInfo.mClusterId != EthernetNetworkDiagnostics::Id)
     {
@@ -60,19 +60,19 @@ CHIP_ERROR EthernetDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::T
     switch (aClusterInfo.mFieldId)
     {
     case Ids::PacketRxCount: {
-        return ReadIfSupported(&ConnectivityManager::GetEthPacketRxCount, aWriter);
+        return ReadIfSupported(&ConnectivityManager::GetEthPacketRxCount, aEncoder);
     }
     case Ids::PacketTxCount: {
-        return ReadIfSupported(&ConnectivityManager::GetEthPacketTxCount, aWriter);
+        return ReadIfSupported(&ConnectivityManager::GetEthPacketTxCount, aEncoder);
     }
     case Ids::TxErrCount: {
-        return ReadIfSupported(&ConnectivityManager::GetEthTxErrCount, aWriter);
+        return ReadIfSupported(&ConnectivityManager::GetEthTxErrCount, aEncoder);
     }
     case Ids::CollisionCount: {
-        return ReadIfSupported(&ConnectivityManager::GetEthCollisionCount, aWriter);
+        return ReadIfSupported(&ConnectivityManager::GetEthCollisionCount, aEncoder);
     }
     case Ids::OverrunCount: {
-        return ReadIfSupported(&ConnectivityManager::GetEthOverrunCount, aWriter);
+        return ReadIfSupported(&ConnectivityManager::GetEthOverrunCount, aEncoder);
     }
     default: {
         *aDataRead = false;
@@ -83,7 +83,7 @@ CHIP_ERROR EthernetDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::T
 }
 
 CHIP_ERROR EthernetDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (ConnectivityManager::*getter)(uint64_t &),
-                                                         TLV::TLVWriter * aWriter)
+                                                         const AttributeValueEncoder & aEncoder)
 {
     uint64_t data;
     CHIP_ERROR err = (DeviceLayer::ConnectivityMgr().*getter)(data);
@@ -96,7 +96,7 @@ CHIP_ERROR EthernetDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (Connectivit
         return err;
     }
 
-    return aWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_Data), data);
+    return aEncoder.Encode(data);
 }
 } // anonymous namespace
 

src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp

@@ -36,15 +36,16 @@ class GeneralDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the GeneralDiagnostics cluster on all endpoints.
     GeneralDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), GeneralDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead) override;
+    CHIP_ERROR Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead) override;
 
 private:
     template <typename T>
-    CHIP_ERROR ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(T &), TLV::TLVWriter * aWriter);
+    CHIP_ERROR ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(T &), const AttributeValueEncoder & aEncoder);
 };
 
 template <typename T>
-CHIP_ERROR GeneralDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(T &), TLV::TLVWriter * aWriter)
+CHIP_ERROR GeneralDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(T &),
+                                                        const AttributeValueEncoder & aEncoder)
 {
     T data;
     CHIP_ERROR err = (DeviceLayer::PlatformMgr().*getter)(data);
@@ -57,12 +58,12 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (PlatformMana
         return err;
     }
 
-    return aWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_Data), data);
+    return aEncoder.Encode(data);
 }
 
 GeneralDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR GeneralDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead)
+CHIP_ERROR GeneralDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead)
 {
     if (aClusterInfo.mClusterId != GeneralDiagnostics::Id)
     {
@@ -74,16 +75,16 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::TL
     switch (aClusterInfo.mFieldId)
     {
     case Ids::RebootCount: {
-        return ReadIfSupported(&PlatformManager::GetRebootCount, aWriter);
+        return ReadIfSupported(&PlatformManager::GetRebootCount, aEncoder);
     }
     case Ids::UpTime: {
-        return ReadIfSupported(&PlatformManager::GetUpTime, aWriter);
+        return ReadIfSupported(&PlatformManager::GetUpTime, aEncoder);
     }
     case Ids::TotalOperationalHours: {
-        return ReadIfSupported(&PlatformManager::GetTotalOperationalHours, aWriter);
+        return ReadIfSupported(&PlatformManager::GetTotalOperationalHours, aEncoder);
     }
     case Ids::BootReasons: {
-        return ReadIfSupported(&PlatformManager::GetBootReasons, aWriter);
+        return ReadIfSupported(&PlatformManager::GetBootReasons, aEncoder);
     }
     default: {
         *aDataRead = false;

src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp

@@ -40,15 +40,15 @@ class SoftwareDiagosticsAttrAccess : public AttributeAccessInterface
     // Register for the SoftwareDiagnostics cluster on all endpoints.
     SoftwareDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), SoftwareDiagnostics::Id) {}
 
-    CHIP_ERROR Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead) override;
+    CHIP_ERROR Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead) override;
 
 private:
-    CHIP_ERROR ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(uint64_t &), TLV::TLVWriter * aWriter);
+    CHIP_ERROR ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(uint64_t &), const AttributeValueEncoder & aEncoder);
 };
 
 SoftwareDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead)
+CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead)
 {
     if (aClusterInfo.mClusterId != SoftwareDiagnostics::Id)
     {
@@ -60,13 +60,13 @@ CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::T
     switch (aClusterInfo.mFieldId)
     {
     case Ids::CurrentHeapFree: {
-        return ReadIfSupported(&PlatformManager::GetCurrentHeapFree, aWriter);
+        return ReadIfSupported(&PlatformManager::GetCurrentHeapFree, aEncoder);
     }
     case Ids::CurrentHeapUsed: {
-        return ReadIfSupported(&PlatformManager::GetCurrentHeapUsed, aWriter);
+        return ReadIfSupported(&PlatformManager::GetCurrentHeapUsed, aEncoder);
     }
     case Ids::CurrentHeapHighWatermark: {
-        return ReadIfSupported(&PlatformManager::GetCurrentHeapHighWatermark, aWriter);
+        return ReadIfSupported(&PlatformManager::GetCurrentHeapHighWatermark, aEncoder);
     }
     default: {
         *aDataRead = false;
@@ -77,7 +77,7 @@ CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::T
 }
 
 CHIP_ERROR SoftwareDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(uint64_t &),
-                                                         TLV::TLVWriter * aWriter)
+                                                         const AttributeValueEncoder & aEncoder)
 {
     uint64_t data;
     CHIP_ERROR err = (DeviceLayer::PlatformMgr().*getter)(data);
@@ -90,7 +90,7 @@ CHIP_ERROR SoftwareDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (PlatformMan
         return err;
     }
 
-    return aWriter->Put(TLV::ContextTag(AttributeDataElement::kCsTag_Data), data);
+    return aEncoder.Encode(data);
 }
 } // anonymous namespace
 

src/app/clusters/thread_network_diagnostics_server/thread_network_diagnostics_server.cpp

@@ -44,20 +44,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, TLV::TLVWriter * aWriter, bool * aDataRead) override;
+    CHIP_ERROR Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead) override;
 };
 
 ThreadDiagosticsAttrAccess gAttrAccess;
 
-CHIP_ERROR ThreadDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, TLV::TLVWriter * aWriter, bool * aDataRead)
+CHIP_ERROR ThreadDiagosticsAttrAccess::Read(ClusterInfo & aClusterInfo, const AttributeValueEncoder & aEncoder, bool * aDataRead)
 {
     if (aClusterInfo.mClusterId != ThreadNetworkDiagnostics::Id)
     {
         // We shouldn't have been called at all.
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    CHIP_ERROR err = ConnectivityMgr().WriteThreadNetworkDiagnosticAttributeToTlv(aClusterInfo.mFieldId, aWriter);
+    CHIP_ERROR err = ConnectivityMgr().WriteThreadNetworkDiagnosticAttributeToTlv(aClusterInfo.mFieldId, aEncoder);
 
     *aDataRead = true;
 

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

@@ -208,13 +208,20 @@ CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * ap
         bool dataRead;
         // TODO: We should probably clone the writer and convert failures here
         // into status responses, unless our caller already does that.
-        ReturnErrorOnFailure(attrOverride->Read(aClusterInfo, apWriter, &dataRead));
+        ReturnErrorOnFailure(attrOverride->Read(aClusterInfo, AttributeValueEncoder(apWriter), &dataRead));
 
         if (dataRead)
         {
-            // TODO: Add DataVersion support
-            ReturnErrorOnFailure(
-                apWriter->Put(chip::TLV::ContextTag(AttributeDataElement::kCsTag_DataVersion), kTemporaryDataVersion));
+            if (apDataExists != nullptr)
+            {
+                *apDataExists = true;
+            }
+            if (apWriter != nullptr)
+            {
+                // TODO: Add DataVersion support
+                ReturnErrorOnFailure(
+                    apWriter->Put(chip::TLV::ContextTag(AttributeDataElement::kCsTag_DataVersion), kTemporaryDataVersion));
+            }
             return CHIP_NO_ERROR;
         }
     }

src/include/platform/ConnectivityManager.h

@@ -24,7 +24,7 @@
 #pragma once
 #include <memory>
 
-#include <lib/core/CHIPTLV.h>
+#include <app/AttributeAccessInterface.h>
 #include <lib/support/CodeUtils.h>
 #include <platform/CHIPDeviceBuildConfig.h>
 #include <platform/CHIPDeviceEvent.h>
@@ -165,7 +165,7 @@ class ConnectivityManager
     bool IsThreadProvisioned();
     void ErasePersistentInfo();
 
-    CHIP_ERROR WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, TLV::TLVWriter * aWriter);
+    CHIP_ERROR WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, const app::AttributeValueEncoder & encoder);
 
     // Ethernet network diagnostics methods
     CHIP_ERROR GetEthPacketRxCount(uint64_t & packetRxCount);
@@ -452,20 +452,21 @@ inline void ConnectivityManager::ErasePersistentInfo()
 
 /*
  * @brief Get runtime value from the thread network based on the given attribute ID.
- *        The info is written in the TLVWriter for the zcl read command reply.
+ *        The info is encoded via the AttributeValueEncoder.
  *
- * @param  attributeId: Id of the attribute for the requested info.
- *         * aWriter: Pointer to a TLVWriter were to write the obtained info.
+ * @param attributeId Id of the attribute for the requested info.
+ * @param aEncoder Encoder to encode the attribute value.
  *
  * @return CHIP_NO_ERROR = Succes.
  *         CHIP_ERROR_NOT_IMPLEMENTED = Runtime value for this attribute to yet available to send as reply
  *                                      Use standard read.
  *         CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE = Is not a Runtime readable attribute. Use standard read
  *         All other errors should be treated as a read error and reported as such.
  */
-inline CHIP_ERROR ConnectivityManager::WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, TLV::TLVWriter * aWriter)
+inline CHIP_ERROR ConnectivityManager::WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId,
+                                                                                  const app::AttributeValueEncoder & encoder)
 {
-    return static_cast<ImplClass *>(this)->_WriteThreadNetworkDiagnosticAttributeToTlv(attributeId, aWriter);
+    return static_cast<ImplClass *>(this)->_WriteThreadNetworkDiagnosticAttributeToTlv(attributeId, encoder);
 }
 
 inline Ble::BleLayer * ConnectivityManager::GetBleLayer()

src/include/platform/ThreadStackManager.h

@@ -23,8 +23,8 @@
 
 #pragma once
 
+#include <app/AttributeAccessInterface.h>
 #include <app/util/basic-types.h>
-#include <lib/core/CHIPTLV.h>
 #include <lib/support/Span.h>
 
 namespace chip {
@@ -108,7 +108,7 @@ class ThreadStackManager
 #endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
 #endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
 
-    CHIP_ERROR WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, TLV::TLVWriter * aWriter);
+    CHIP_ERROR WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, const app::AttributeValueEncoder & encoder);
 
 private:
     // ===== Members for internal use by the following friends.
@@ -375,20 +375,21 @@ inline CHIP_ERROR ThreadStackManager::JoinerStart()
 
 /*
  * @brief Get runtime value from the thread network based on the given attribute ID.
- *        The info is written in the TLVWriter for the zcl read command reply.
+ *        The info is encoded via the AttributeValueEncoder.
  *
- * @param  attributeId: Id of the attribute for the requested info.
- *         * aWriter: Pointer to a TLVWriter were to write the obtained info.
+ * @param attributeId Id of the attribute for the requested info.
+ * @param aEncoder Encoder to encode the attribute value.
  *
  * @return CHIP_NO_ERROR = Succes.
  *         CHIP_ERROR_NOT_IMPLEMENTED = Runtime value for this attribute to yet available to send as reply
  *                                      Use standard read.
  *         CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE = Is not a Runtime readable attribute. Use standard read
  *         All other errors should be treated as a read error and reported as such.
  */
-inline CHIP_ERROR ThreadStackManager::WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, TLV::TLVWriter * aWriter)
+inline CHIP_ERROR ThreadStackManager::WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId,
+                                                                                 const app::AttributeValueEncoder & encoder)
 {
-    return static_cast<ImplClass *>(this)->_WriteThreadNetworkDiagnosticAttributeToTlv(attributeId, aWriter);
+    return static_cast<ImplClass *>(this)->_WriteThreadNetworkDiagnosticAttributeToTlv(attributeId, encoder);
 }
 
 } // namespace DeviceLayer

src/include/platform/internal/GenericConnectivityManagerImpl_NoThread.h

@@ -24,7 +24,7 @@
 
 #pragma once
 
-#include <lib/core/CHIPTLV.h>
+#include <app/AttributeAccessInterface.h>
 
 namespace chip {
 namespace DeviceLayer {
@@ -55,7 +55,7 @@ class GenericConnectivityManagerImpl_NoThread
     bool _IsThreadAttached(void);
     bool _IsThreadProvisioned(void);
     void _ErasePersistentInfo(void);
-    CHIP_ERROR _WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, TLV::TLVWriter * aWriter);
+    CHIP_ERROR _WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId, const app::AttributeValueEncoder & encoder);
 
     ImplClass * Impl() { return static_cast<ImplClass *>(this); }
 };
@@ -128,9 +128,8 @@ inline CHIP_ERROR GenericConnectivityManagerImpl_NoThread<ImplClass>::_SetThread
 }
 
 template <class ImplClass>
-inline CHIP_ERROR
-GenericConnectivityManagerImpl_NoThread<ImplClass>::_WriteThreadNetworkDiagnosticAttributeToTlv(AttributeId attributeId,
-                                                                                                TLV::TLVWriter * aWriter)
+inline CHIP_ERROR GenericConnectivityManagerImpl_NoThread<ImplClass>::_WriteThreadNetworkDiagnosticAttributeToTlv(
+    AttributeId attributeId, const app::AttributeValueEncoder & encoder)
 {
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
 }