Review feedback

src/app/InteractionModelDelegate.h

@@ -99,7 +99,7 @@ class InteractionModelDelegate
      *                            fail to process report data.
      * @retval # CHIP_ERROR_NOT_IMPLEMENTED if not implemented
      */
-    virtual CHIP_ERROR ReadError(const ReadClient * apReadClient, CHIP_ERROR aError) { return CHIP_ERROR_NOT_IMPLEMENTED; }
+    virtual CHIP_ERROR ReadError(ReadClient * apReadClient, CHIP_ERROR aError) { return CHIP_ERROR_NOT_IMPLEMENTED; }
 
     /**
      * Notification that a WriteClient has received a Write Response containing a status code.

src/app/InteractionModelEngine.cpp

@@ -123,7 +123,6 @@ void InteractionModelEngine::Shutdown()
 CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClient, ReadClient::InteractionType aInteractionType,
                                                  uint64_t aAppIdentifier, InteractionModelDelegate * apDelegateOverride)
 {
-    CHIP_ERROR err = CHIP_ERROR_NO_MEMORY;
 
     if (apDelegateOverride == nullptr)
     {
@@ -136,6 +135,8 @@ CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClien
     {
         if (readClient.IsFree())
         {
+            CHIP_ERROR err;
+
             *apReadClient = &readClient;
             err           = readClient.Init(mpExchangeMgr, apDelegateOverride, aInteractionType, aAppIdentifier);
             if (CHIP_NO_ERROR != err)
@@ -146,10 +147,10 @@ CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClien
         }
     }
 
-    return err;
+    return CHIP_ERROR_NO_MEMORY;
 }
 
-uint32_t InteractionModelEngine::GetNumActiveReadClients()
+uint32_t InteractionModelEngine::GetNumActiveReadClients() const
 {
     uint32_t numActive = 0;
 
@@ -164,7 +165,7 @@ uint32_t InteractionModelEngine::GetNumActiveReadClients()
     return numActive;
 }
 
-uint32_t InteractionModelEngine::GetNumActiveReadHandlers()
+uint32_t InteractionModelEngine::GetNumActiveReadHandlers() const
 {
     uint32_t numActive = 0;
 

src/app/InteractionModelEngine.h

@@ -140,21 +140,21 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
      *
      *  @param[inout] 	apReadClient	    A double pointer to a ReadClient that is updated to point to a valid ReadClient
      *                                      on successful completion of this function. On failure, it will be updated to point to
-     * nullptr.
+     *                                      nullptr.
      *  @param[in]      aInteractionType    Type of interaction (read or subscription) that the requested ReadClient should execute.
      *  @param[in]      aAppIdentifier      A unique token that can be attached to the returned ReadClient object that will be
-     * passed through some of the methods in the registered InteractionModelDelegate.
+     *                                      passed through some of the methods in the registered InteractionModelDelegate.
      *  @param[in]      apDelegateOverride  If not-null, permits overriding the default delegate registered with the
-     * InteractionModelEngine that will be used by the ReadClient.
+     *                                      InteractionModelEngine that will be used by the ReadClient.
      *
      *  @retval #CHIP_ERROR_INCORRECT_STATE If there is no ReadClient available
      *  @retval #CHIP_NO_ERROR On success.
      */
     CHIP_ERROR NewReadClient(ReadClient ** const apReadClient, ReadClient::InteractionType aInteractionType,
                              uint64_t aAppIdentifier, InteractionModelDelegate * apDelegateOverride = nullptr);
 
-    uint32_t GetNumActiveReadHandlers();
-    uint32_t GetNumActiveReadClients();
+    uint32_t GetNumActiveReadHandlers() const;
+    uint32_t GetNumActiveReadClients() const;
 
     /**
      *  Get read client index in mReadClients

src/app/tests/TestReadInteraction.cpp

@@ -196,7 +196,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate
         return CHIP_NO_ERROR;
     }
 
-    CHIP_ERROR ReadError(const chip::app::ReadClient * apReadClient, CHIP_ERROR aError) override
+    CHIP_ERROR ReadError(chip::app::ReadClient * apReadClient, CHIP_ERROR aError) override
     {
         mReadError = true;
         return CHIP_NO_ERROR;

src/app/tests/TestWriteInteraction.cpp

@@ -267,9 +267,7 @@ CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & a
     writer.Init(attributeDataTLV);
     writer.CopyElement(TLV::AnonymousTag, aReader);
     attributeDataTLVLen = writer.GetLengthWritten();
-    return aWriteHandler->AddStatus(AttributePathParams(aClusterInfo.mNodeId, aClusterInfo.mEndpointId, aClusterInfo.mClusterId,
-                                                        aClusterInfo.mFieldId),
-
+    return aWriteHandler->AddStatus(AttributePathParams(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mFieldId),
                                     Protocols::InteractionModel::Status::Success);
 }
 

src/app/tests/integration/chip_im_initiator.cpp

@@ -161,7 +161,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate, publ
         return CHIP_NO_ERROR;
     }
 
-    CHIP_ERROR ReadError(const chip::app::ReadClient * apReadClient, CHIP_ERROR aError) override
+    CHIP_ERROR ReadError(chip::app::ReadClient * apReadClient, CHIP_ERROR aError) override
     {
         printf("ReadError with err %" CHIP_ERROR_FORMAT, aError.Format());
         return CHIP_NO_ERROR;

src/controller/CHIPDeviceController.cpp

@@ -1709,7 +1709,7 @@ void DeviceControllerInteractionModelDelegate::OnReportData(const app::ReadClien
     IMReadReportAttributesResponseCallback(apReadClient, aPath, apData, status);
 }
 
-CHIP_ERROR DeviceControllerInteractionModelDelegate::ReadError(const app::ReadClient * apReadClient, CHIP_ERROR aError)
+CHIP_ERROR DeviceControllerInteractionModelDelegate::ReadError(app::ReadClient * apReadClient, CHIP_ERROR aError)
 {
     app::ClusterInfo path;
     path.mNodeId = apReadClient->GetPeerNodeId();

src/controller/DeviceControllerInteractionModelDelegate.h

@@ -25,7 +25,7 @@ class DeviceControllerInteractionModelDelegate : public chip::app::InteractionMo
 
     void OnReportData(const app::ReadClient * apReadClient, const app::ClusterInfo & aPath, TLV::TLVReader * apData,
                       Protocols::InteractionModel::Status status) override;
-    CHIP_ERROR ReadError(const app::ReadClient * apReadClient, CHIP_ERROR aError) override;
+    CHIP_ERROR ReadError(app::ReadClient * apReadClient, CHIP_ERROR aError) override;
 
     CHIP_ERROR WriteResponseStatus(const app::WriteClient * apWriteClient, const app::StatusIB & aStatusIB,
                                    app::AttributePathParams & aAttributePathParams, uint8_t aAttributeIndex) override;

src/controller/TypedReadCallback.h

@@ -82,9 +82,10 @@ class TypedReadCallback final : public app::InteractionModelDelegate
         }
     }
 
-    CHIP_ERROR ReadError(const app::ReadClient * apReadClient, CHIP_ERROR aError) override
+    CHIP_ERROR ReadError(app::ReadClient * apReadClient, CHIP_ERROR aError) override
     {
         mOnError(nullptr, Protocols::InteractionModel::Status::Failure, aError);
+        mOnDone(apReadClient, this);
         return CHIP_NO_ERROR;
     }
 

src/controller/tests/data_model/TestRead.cpp

@@ -16,12 +16,6 @@
  *    limitations under the License.
  */
 
-/**
- *    @file
- *      This file implements unit tests for CHIP Interaction Model Command Interaction
- *
- */
-
 #include <app-common/zap-generated/cluster-objects.h>
 #include <app/InteractionModelEngine.h>
 #include <controller/ReadInteraction.h>
@@ -69,7 +63,7 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath)
     return (aCommandPath.mEndpointId == kTestEndpointId && aCommandPath.mClusterId == TestCluster::Id);
 }
 
-CHIP_ERROR ReadSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVWriter * apWriter, bool * apDataExists)
+CHIP_ERROR ReadSingleClusterData(const ConcreteAttributePath & aPath, TLV::TLVWriter * apWriter, bool * apDataExists)
 {
     if (responseDirective == kSendDataResponse)
     {