[im] Land IM WriteRequest message

src/app/InteractionModelEngine.cpp

@@ -137,7 +137,7 @@ CHIP_ERROR InteractionModelEngine::NewCommandSender(CommandSender ** const apCom
     return CHIP_ERROR_NO_MEMORY;
 }
 
-CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClient, intptr_t aAppIdentifier)
+CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClient, uint64_t aAppIdentifier)
 {
     CHIP_ERROR err = CHIP_ERROR_NO_MEMORY;
 
@@ -158,9 +158,9 @@ CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClien
     return err;
 }
 
-CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClient ** const apWriteClient)
+CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClientHandle & apWriteClient, uint64_t aApplicationIdentifier)
 {
-    *apWriteClient = nullptr;
+    apWriteClient.SetWriteClient(nullptr);
 
     for (auto & writeClient : mWriteClients)
     {
@@ -169,8 +169,8 @@ CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClient ** const apWriteCl
             continue;
         }
 
-        ReturnErrorOnFailure(writeClient.Init(mpExchangeMgr, mpDelegate));
-        *apWriteClient = &writeClient;
+        ReturnLogErrorOnFailure(writeClient.Init(mpExchangeMgr, mpDelegate, aApplicationIdentifier));
+        apWriteClient.SetWriteClient(&writeClient);
         return CHIP_NO_ERROR;
     }
 
@@ -321,7 +321,7 @@ CHIP_ERROR InteractionModelEngine::SendReadRequest(NodeId aNodeId, FabricIndex a
                                                    EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize,
                                                    AttributePathParams * apAttributePathParamsList,
                                                    size_t aAttributePathParamsListSize, EventNumber aEventNumber,
-                                                   intptr_t aAppIdentifier)
+                                                   uint64_t aAppIdentifier)
 {
     ReadClient * client = nullptr;
     CHIP_ERROR err      = CHIP_NO_ERROR;
@@ -335,19 +335,6 @@ CHIP_ERROR InteractionModelEngine::SendReadRequest(NodeId aNodeId, FabricIndex a
     return err;
 }
 
-CHIP_ERROR __attribute__((weak))
-WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & aReader, WriteHandler * apWriteHandler)
-{
-    ChipLogDetail(DataManagement,
-                  "Received Cluster Attribute: Cluster=" ChipLogFormatMEI " NodeId=0x" ChipLogFormatX64 " Endpoint=%" PRIx16
-                  " FieldId=%" PRIx32 " ListIndex=%" PRIx16,
-                  ChipLogValueMEI(aClusterInfo.mClusterId), ChipLogValueX64(aClusterInfo.mNodeId), aClusterInfo.mEndpointId,
-                  aClusterInfo.mFieldId, aClusterInfo.mListIndex);
-    ChipLogError(DataManagement,
-                 "Default WriteSingleClusterData is called, this should be replaced by actual dispatched for cluster");
-    return CHIP_NO_ERROR;
-}
-
 uint16_t InteractionModelEngine::GetReadClientArrayIndex(const ReadClient * const apReadClient) const
 {
     return static_cast<uint16_t>(apReadClient - mReadClients);

src/app/InteractionModelEngine.h

@@ -115,18 +115,22 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate
     CHIP_ERROR SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex, SecureSessionHandle * apSecureSession,
                                EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize,
                                AttributePathParams * apAttributePathParamsList, size_t aAttributePathParamsListSize,
-                               EventNumber aEventNumber, intptr_t aAppIdentifier = 0);
+                               EventNumber aEventNumber, uint64_t aAppIdentifier = 0);
 
     /**
      *  Retrieve a WriteClient that the SDK consumer can use to send a write.  If the call succeeds,
      *  see WriteClient documentation for lifetime handling.
      *
+     *  The Write interaction is more like Invoke interaction (cluster specific commands) since it will include cluster specific
+     * payload, and may have the need to encode non-scalar values (like structs and arrays). Thus we use WriteClientHandle to
+     * prevent user's code from leaking WriteClients.
+     *
      *  @param[out]    apWriteClient    A pointer to the WriteClient object.
      *
      *  @retval #CHIP_ERROR_NO_MEMORY If there is no WriteClient available
      *  @retval #CHIP_NO_ERROR On success.
      */
-    CHIP_ERROR NewWriteClient(WriteClient ** const apWriteClient);
+    CHIP_ERROR NewWriteClient(WriteClientHandle & apWriteClient, uint64_t aApplicationIdentifier = 0);
 
     /**
      *  Get read client index in mReadClients
@@ -177,7 +181,7 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate
      *  @retval #CHIP_ERROR_INCORRECT_STATE If there is no ReadClient available
      *  @retval #CHIP_NO_ERROR On success.
      */
-    CHIP_ERROR NewReadClient(ReadClient ** const apReadClient, intptr_t aAppIdentifier);
+    CHIP_ERROR NewReadClient(ReadClient ** const apReadClient, uint64_t aAppIdentifier);
 
     Messaging::ExchangeManager * mpExchangeMgr = nullptr;
     InteractionModelDelegate * mpDelegate      = nullptr;

src/app/ReadClient.cpp

@@ -30,7 +30,7 @@ namespace chip {
 namespace app {
 
 CHIP_ERROR ReadClient::Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate,
-                            intptr_t aAppIdentifier)
+                            uint64_t aAppIdentifier)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     // Error if already initialized.

src/app/ReadClient.h

@@ -77,7 +77,7 @@ class ReadClient : public Messaging::ExchangeDelegate
                                AttributePathParams * apAttributePathParamsList, size_t aAttributePathParamsListSize,
                                EventNumber aEventNumber);
 
-    intptr_t GetAppIdentifier() const { return mAppIdentifier; }
+    uint64_t GetAppIdentifier() const { return mAppIdentifier; }
     Messaging::ExchangeContext * GetExchangeContext() const { return mpExchangeCtx; }
 
 private:
@@ -105,7 +105,7 @@ class ReadClient : public Messaging::ExchangeDelegate
      *  @retval #CHIP_NO_ERROR On success.
      *
      */
-    CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate, intptr_t aAppIdentifier);
+    CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate, uint64_t aAppIdentifier);
 
     virtual ~ReadClient() = default;
 
@@ -140,7 +140,7 @@ class ReadClient : public Messaging::ExchangeDelegate
     Messaging::ExchangeContext * mpExchangeCtx = nullptr;
     InteractionModelDelegate * mpDelegate      = nullptr;
     ClientState mState                         = ClientState::Uninitialized;
-    intptr_t mAppIdentifier                    = 0;
+    uint64_t mAppIdentifier                    = 0;
 };
 
 }; // namespace app

src/app/WriteClient.cpp

@@ -29,7 +29,8 @@
 namespace chip {
 namespace app {
 
-CHIP_ERROR WriteClient::Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate)
+CHIP_ERROR WriteClient::Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate,
+                             uint64_t aApplicationIdentifier)
 {
     VerifyOrReturnError(apExchangeMgr != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(mpExchangeMgr == nullptr, CHIP_ERROR_INCORRECT_STATE);
@@ -50,6 +51,7 @@ CHIP_ERROR WriteClient::Init(Messaging::ExchangeManager * apExchangeMgr, Interac
     mpExchangeMgr         = apExchangeMgr;
     mpDelegate            = apDelegate;
     mAttributeStatusIndex = 0;
+    mAppIdentifier        = aApplicationIdentifier;
     MoveToState(State::Initialized);
 
     return CHIP_NO_ERROR;
@@ -396,5 +398,21 @@ CHIP_ERROR WriteClient::ProcessAttributeStatusElement(AttributeStatusElement::Pa
     return err;
 }
 
+CHIP_ERROR WriteClientHandle::SendWriteRequest(NodeId aNodeId, FabricIndex aFabricIndex, SecureSessionHandle * apSecureSession)
+{
+    CHIP_ERROR err = mpWriteClient->SendWriteRequest(aNodeId, aFabricIndex, apSecureSession);
+
+    if (err == CHIP_NO_ERROR)
+    {
+        // On success, the write client will handle lifetime up until success/falure callbacks are invoked
+        mpWriteClient = nullptr;
+    }
+    else
+    {
+        SetWriteClient(nullptr);
+    }
+    return err;
+}
+
 } // namespace app
 } // namespace chip

src/app/WriteClient.h

@@ -36,6 +36,10 @@
 
 namespace chip {
 namespace app {
+
+class WriteClientHandle;
+class InteractionModelEngine;
+
 /**
  *  @brief The read client represents the initiator side of a Write Interaction, and is responsible
  *  for generating one Write Request for a particular set of attributes, and handling the Write response.
@@ -52,22 +56,21 @@ class WriteClient : public Messaging::ExchangeDelegate
      */
     void Shutdown();
 
-    /**
-     *  Once SendWriteRequest returns successfully, the WriteClient will
-     *  handle calling Shutdown on itself once it decides it's done with waiting
-     *  for a response (i.e. times out or gets a response).
-     *  If SendWriteRequest is never called, or the call fails, the API
-     *  consumer is responsible for calling Shutdown on the WriteClient.
-     */
-    CHIP_ERROR SendWriteRequest(NodeId aNodeId, FabricIndex aFabricIndex, SecureSessionHandle * apSecureSession);
-
     CHIP_ERROR PrepareAttribute(const AttributePathParams & attributePathParams);
     CHIP_ERROR FinishAttribute();
     TLV::TLVWriter * GetAttributeDataElementTLVWriter();
 
+    uint64_t GetAppIdentifier() const { return mAppIdentifier; }
+    void SetAppIdentifier(uint64_t aAppIdentifier) { mAppIdentifier = aAppIdentifier; }
+    NodeId GetSourceNodeId() const
+    {
+        return mpExchangeCtx != nullptr ? mpExchangeCtx->GetSecureSession().GetPeerNodeId() : kUndefinedNodeId;
+    }
+
 private:
     friend class TestWriteInteraction;
     friend class InteractionModelEngine;
+    friend class WriteClientHandle;
 
     enum class State
     {
@@ -77,6 +80,20 @@ class WriteClient : public Messaging::ExchangeDelegate
         AwaitingResponse,  // The client has sent out the write request message
     };
 
+    /**
+     * Finalize Write Request Message TLV Builder and retrieve final data from tlv builder for later sending
+     */
+    CHIP_ERROR FinalizeMessage(System::PacketBufferHandle & aPacket);
+
+    /**
+     *  Once SendWriteRequest returns successfully, the WriteClient will
+     *  handle calling Shutdown on itself once it decides it's done with waiting
+     *  for a response (i.e. times out or gets a response).
+     *  If SendWriteRequest is never called, or the call fails, the API
+     *  consumer is responsible for calling Shutdown on the WriteClient.
+     */
+    CHIP_ERROR SendWriteRequest(NodeId aNodeId, FabricIndex aFabricIndex, SecureSessionHandle * apSecureSession);
+
     /**
      *  Initialize the client object. Within the lifetime
      *  of this instance, this method is invoked once after object
@@ -88,7 +105,8 @@ class WriteClient : public Messaging::ExchangeDelegate
      *  @retval #CHIP_ERROR_INCORRECT_STATE incorrect state if it is already initialized
      *  @retval #CHIP_NO_ERROR On success.
      */
-    CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate);
+    CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate,
+                    uint64_t aApplicationIdentifier);
 
     virtual ~WriteClient() = default;
 
@@ -101,11 +119,6 @@ class WriteClient : public Messaging::ExchangeDelegate
      */
     bool IsFree() const { return mState == State::Uninitialized; };
 
-    /**
-     * Finalize Write Request Message TLV Builder and retrieve final data from tlv builder for later sending
-     */
-    CHIP_ERROR FinalizeMessage(System::PacketBufferHandle & aPacket);
-
     void MoveToState(const State aTargetState);
     CHIP_ERROR ProcessWriteResponseMessage(System::PacketBufferHandle && payload);
     CHIP_ERROR ProcessAttributeStatusElement(AttributeStatusElement::Parser & aAttributeStatusElement);
@@ -128,7 +141,75 @@ class WriteClient : public Messaging::ExchangeDelegate
     System::PacketBufferTLVWriter mMessageWriter;
     WriteRequest::Builder mWriteRequestBuilder;
     uint8_t mAttributeStatusIndex = 0;
-    intptr_t mAppIdentifier       = 0;
+    uint64_t mAppIdentifier       = 0;
+};
+
+class WriteClientHandle
+{
+public:
+    /**
+     * Construct an empty WriteClientHandle.
+     */
+    WriteClientHandle() : mpWriteClient(nullptr) {}
+    WriteClientHandle(decltype(nullptr)) : mpWriteClient(nullptr) {}
+
+    /**
+     * Construct a WriteClientHandle that takes ownership of a WriteClient from another.
+     */
+    WriteClientHandle(WriteClientHandle && aOther)
+    {
+        mpWriteClient        = aOther.mpWriteClient;
+        aOther.mpWriteClient = nullptr;
+    }
+
+    ~WriteClientHandle() { SetWriteClient(nullptr); }
+
+    /**
+     * Access a WriteClientHandle's public methods.
+     */
+    WriteClient * operator->() const { return mpWriteClient; }
+
+    /**
+     *  Finalize the message and send it to the desired node. The underlying write object will always be released, and the user
+     * should not use this object after calling this function.
+     */
+    CHIP_ERROR SendWriteRequest(NodeId aNodeId, FabricIndex aFabricIndex, SecureSessionHandle * apSecureSession);
+
+    /**
+     *  Encode an attribute value that can be directly encoded using TLVWriter::Put
+     */
+    template <class T>
+    CHIP_ERROR EncodeScalarAttributeWritePayload(const chip::app::AttributePathParams & attributePath, T value)
+    {
+        chip::TLV::TLVWriter * writer = nullptr;
+
+        VerifyOrReturnError(mpWriteClient != nullptr, CHIP_ERROR_INCORRECT_STATE);
+        ReturnErrorOnFailure(app::InteractionModelEngine::GetInstance()->NewWriteClient(handle));
+        ReturnErrorOnFailure(mpWriteClient->PrepareAttribute(attributePath));
+        VerifyOrReturnError((writer = mpWriteClient->GetAttributeDataElementTLVWriter()) != nullptr, CHIP_ERROR_INCORRECT_STATE);
+        ReturnErrorOnFailure(mpWriteClient->Put(chip::TLV::ContextTag(chip::app::AttributeDataElement::kCsTag_Data), value));
+        ReturnErrorOnFailure(mpWriteClient->FinishAttribute());
+
+        return CHIP_NO_ERROR;
+    }
+
+    void SetWriteClient(WriteClient * apWriteClient)
+    {
+        if (mpWriteClient != nullptr)
+        {
+            mpWriteClient->Shutdown();
+        }
+        mpWriteClient = apWriteClient;
+    }
+
+private:
+    friend class TestWriteInteraction;
+
+    WriteClientHandle(const WriteClientHandle &) = delete;
+    WriteClientHandle & operator=(const WriteClientHandle &) = delete;
+    WriteClientHandle & operator=(const WriteClientHandle &&) = delete;
+
+    WriteClient * mpWriteClient = nullptr;
 };
 
 } // namespace app

src/app/WriteHandler.cpp

@@ -27,17 +27,17 @@ namespace chip {
 namespace app {
 CHIP_ERROR WriteHandler::Init(InteractionModelDelegate * apDelegate)
 {
-    VerifyOrReturnError(apDelegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
+    IgnoreUnusedVariable(apDelegate);
+    VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
 
     System::PacketBufferHandle packet = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
-    VerifyOrReturnError(!packet.IsNull(), CHIP_ERROR_NO_MEMORY);
+    VerifyOrReturnLogError(!packet.IsNull(), CHIP_ERROR_NO_MEMORY);
 
     mMessageWriter.Init(std::move(packet));
-    ReturnErrorOnFailure(mWriteResponseBuilder.Init(&mMessageWriter));
+    ReturnLogErrorOnFailure(mWriteResponseBuilder.Init(&mMessageWriter));
 
     AttributeStatusList::Builder attributeStatusListBuilder = mWriteResponseBuilder.CreateAttributeStatusListBuilder();
-    ReturnErrorOnFailure(attributeStatusListBuilder.GetError());
+    ReturnLogErrorOnFailure(attributeStatusListBuilder.GetError());
 
     MoveToState(State::Initialized);