project-chip · isiu-apple · May 23, 2022 · May 23, 2022
diff --git a/examples/chip-tool/commands/clusters/ReportCommand.h b/examples/chip-tool/commands/clusters/ReportCommand.h
@@ -96,7 +96,7 @@ class ReportCommand : public InteractionModelReports, public ModelCommand, publi
         mError = error;
     }
 
-    void OnDone() override
+    void OnDone(chip::app::ReadClient *) override
     {
         InteractionModelReports::Shutdown();
         SetCommandExitStatus(mError);

diff --git a/examples/tv-casting-app/tv-casting-common/commands/clusters/ReportCommand.h b/examples/tv-casting-app/tv-casting-common/commands/clusters/ReportCommand.h
@@ -99,7 +99,7 @@ class ReportCommand : public ModelCommand, public chip::app::ReadClient::Callbac
         mError = error;
     }
 
-    void OnDone() override
+    void OnDone(chip::app::ReadClient *) override
     {
         mReadClient.reset();
         SetCommandExitStatus(mError);

diff --git a/src/app/BufferedReadCallback.h b/src/app/BufferedReadCallback.h
@@ -75,7 +75,7 @@ class BufferedReadCallback : public ReadClient::Callback
         return mCallback.OnEventData(aEventHeader, apData, apStatus);
     }
 
-    void OnDone() override { return mCallback.OnDone(); }
+    void OnDone(ReadClient * apReadClient) override { return mCallback.OnDone(apReadClient); }
     void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override
     {
         mCallback.OnSubscriptionEstablished(aSubscriptionId);

diff --git a/src/app/ClusterStateCache.h b/src/app/ClusterStateCache.h
@@ -563,10 +563,10 @@ class ClusterStateCache : protected ReadClient::Callback
 
     void OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus) override;
 
-    void OnDone() override
+    void OnDone(ReadClient * apReadClient) override
     {
         mRequestPathSet.clear();
-        return mCallback.OnDone();
+        return mCallback.OnDone(apReadClient);
     }
 
     void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override

diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -156,7 +156,7 @@ void ReadClient::Close(CHIP_ERROR aError)
         StopResubscription();
     }
 
-    mpCallback.OnDone();
+    mpCallback.OnDone(this);
 }
 
 const char * ReadClient::GetStateStr() const

diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h
@@ -145,17 +145,19 @@ class ReadClient : public Messaging::ExchangeDelegate
         virtual void OnError(CHIP_ERROR aError) {}
 
         /**
-         * OnDone will be called when ReadClient has finished all work and is safe to destroy and free the
-         * allocated CommandSender object.
+         * OnDone will be called when ReadClient has finished all work and it is
+         * safe to destroy and free the allocated ReadClient object and any
+         * other objects associated with the Read or Subscribe interaction.
          *
          * This function will:
          *      - Always be called exactly *once* for a given ReadClient instance.
          *      - Be called even in error circumstances.
          *      - Only be called after a successful call to SendRequest has been
          *        made, when the read completes or the subscription is shut down.
          *
+         * @param[in] apReadClient the ReadClient for the completed interaction.
          */
-        virtual void OnDone() = 0;
+        virtual void OnDone(ReadClient * apReadClient) = 0;
 
         /**
          * This function is invoked when using SendAutoResubscribeRequest, where the ReadClient was configured to auto re-subscribe

diff --git a/src/app/tests/TestBufferedReadCallback.cpp b/src/app/tests/TestBufferedReadCallback.cpp
@@ -81,7 +81,7 @@ class DataSeriesValidator : public BufferedReadCallback::Callback
     void OnReportBegin() override;
     void OnReportEnd() override;
     void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
-    void OnDone() override {}
+    void OnDone(ReadClient *) override {}
 
     std::vector<ValidationInstruction> mInstructionList;
     uint32_t mCurrentInstruction = 0;

diff --git a/src/app/tests/TestClusterStateCache.cpp b/src/app/tests/TestClusterStateCache.cpp
@@ -280,7 +280,7 @@ class CacheValidator : public ClusterStateCache::Callback
     Clusters::TestCluster::Attributes::TypeInfo::DecodableType clusterValue;
 
 private:
-    void OnDone() override {}
+    void OnDone(ReadClient *) override {}
     void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override
     {
         ChipLogProgress(DataManagement, "\t\t -- Validating OnAttributeData callback");

diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp
@@ -180,7 +180,7 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback
         mReadError = true;
     }
 
-    void OnDone() override {}
+    void OnDone(chip::app::ReadClient *) override {}
 
     void OnDeallocatePaths(chip::app::ReadPrepareParams && aReadPrepareParams) override
     {

diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp
@@ -151,8 +151,13 @@ class MockInteractionModelApp : public ::chip::app::CommandSender::Callback,
 
     void OnError(CHIP_ERROR aError) override { printf("ReadError with err %" CHIP_ERROR_FORMAT, aError.Format()); }
 
-    void OnDone() override
+    void OnDone(chip::app::ReadClient * apReadClient) override
     {
+        if (apReadClient != mReadClient.get())
+        {
+            printf("Unexpected read client.");
+        }
+
         if (!mReadClient->IsSubscriptionType())
         {
             HandleReadComplete();

diff --git a/src/app/tests/suites/commands/interaction_model/InteractionModel.cpp b/src/app/tests/suites/commands/interaction_model/InteractionModel.cpp
@@ -114,8 +114,10 @@ void InteractionModel::OnError(CHIP_ERROR error)
     OnResponse(status, nullptr);
 }
 
-void InteractionModel::OnDone()
+void InteractionModel::OnDone(ReadClient * aReadClient)
 {
+    // TODO: Keep track of multiple read/subscribe interactions, clear out the
+    // right thing here.
     mReadClient.reset();
     ContinueOnChipMainThread(CHIP_NO_ERROR);
 }

diff --git a/src/app/tests/suites/commands/interaction_model/InteractionModel.h b/src/app/tests/suites/commands/interaction_model/InteractionModel.h
@@ -327,7 +327,7 @@ class InteractionModel : public InteractionModelReports,
     void OnEventData(const chip::app::EventHeader & eventHeader, chip::TLV::TLVReader * data,
                      const chip::app::StatusIB * status) override;
     void OnError(CHIP_ERROR error) override;
-    void OnDone() override;
+    void OnDone(chip::app::ReadClient * aReadClient) override;
     void OnSubscriptionEstablished(chip::SubscriptionId subscriptionId) override;
 
     /////////// WriteClient Callback Interface /////////

diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp
@@ -1575,7 +1575,7 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer
 }
 
 // ClusterStateCache::Callback impl
-void DeviceCommissioner::OnDone()
+void DeviceCommissioner::OnDone(app::ReadClient *)
 {
     CHIP_ERROR err;
     CHIP_ERROR return_err = CHIP_NO_ERROR;

diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h
@@ -600,7 +600,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     DevicePairingDelegate * GetPairingDelegate() const { return mPairingDelegate; }
 
     // ClusterStateCache::Callback impl
-    void OnDone() override;
+    void OnDone(app::ReadClient *) override;
 
     // Commissioner will establish new device connections after PASE.
     OperationalDeviceProxy * GetDeviceSession(const PeerId & peerId) override;

diff --git a/src/controller/TypedReadCallback.h b/src/controller/TypedReadCallback.h
@@ -96,7 +96,7 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback
 
     void OnError(CHIP_ERROR aError) override { mOnError(nullptr, aError); }
 
-    void OnDone() override { mOnDone(this); }
+    void OnDone(app::ReadClient *) override { mOnDone(this); }
 
     void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override
     {
@@ -173,7 +173,7 @@ class TypedReadEventCallback final : public app::ReadClient::Callback
 
     void OnError(CHIP_ERROR aError) override { mOnError(nullptr, aError); }
 
-    void OnDone() override { mOnDone(this); }
+    void OnDone(app::ReadClient *) override { mOnDone(this); }
 
     void OnDeallocatePaths(chip::app::ReadPrepareParams && aReadPrepareParams) override
     {

diff --git a/src/controller/java/AndroidCallbacks.cpp b/src/controller/java/AndroidCallbacks.cpp
@@ -296,7 +296,7 @@ void ReportCallback::OnError(CHIP_ERROR aError)
     ReportError(nullptr, aError);
 }
 
-void ReportCallback::OnDone()
+void ReportCallback::OnDone(app::ReadClient *)
 {
     JniReferences::GetInstance().GetEnvForCurrentThread()->DeleteGlobalRef(mWrapperCallbackRef);
 }

diff --git a/src/controller/java/AndroidCallbacks.h b/src/controller/java/AndroidCallbacks.h
@@ -57,7 +57,7 @@ struct ReportCallback : public app::ReadClient::Callback
 
     void OnError(CHIP_ERROR aError) override;
 
-    void OnDone() override;
+    void OnDone(app::ReadClient *) override;
 
     void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override;
 

diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp
@@ -203,7 +203,7 @@ class ReadClientCallback : public ReadClient::Callback
 
     void OnReportEnd() override { gOnReportEndCallback(mAppContext); }
 
-    void OnDone() override
+    void OnDone(ReadClient *) override
     {
         gOnReadDoneCallback(mAppContext);
 

diff --git a/src/controller/tests/TestEventCaching.cpp b/src/controller/tests/TestEventCaching.cpp
@@ -126,7 +126,7 @@ class TestReadCallback : public app::ClusterStateCache::Callback
 {
 public:
     TestReadCallback() : mClusterCacheAdapter(*this) {}
-    void OnDone() {}
+    void OnDone(app::ReadClient *) {}
 
     app::ClusterStateCache mClusterCacheAdapter;
 };

diff --git a/src/controller/tests/TestEventChunking.cpp b/src/controller/tests/TestEventChunking.cpp
@@ -150,7 +150,7 @@ class TestReadCallback : public app::ReadClient::Callback
 
     void OnEventData(const app::EventHeader & aEventHeader, TLV::TLVReader * apData, const app::StatusIB * apStatus) override;
 
-    void OnDone() override;
+    void OnDone(app::ReadClient * apReadClient) override;
 
     void OnReportEnd() override { mOnReportEnd = true; }
 
@@ -222,7 +222,7 @@ void TestReadCallback::OnEventData(const app::EventHeader & aEventHeader, TLV::T
     mEventCount++;
 }
 
-void TestReadCallback::OnDone() {}
+void TestReadCallback::OnDone(app::ReadClient *) {}
 
 class TestAttrAccess : public app::AttributeAccessInterface
 {

diff --git a/src/controller/tests/TestReadChunking.cpp b/src/controller/tests/TestReadChunking.cpp
@@ -128,7 +128,7 @@ class TestReadCallback : public app::ReadClient::Callback
     void OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
                          const app::StatusIB & aStatus) override;
 
-    void OnDone() override;
+    void OnDone(app::ReadClient * apReadClient) override;
 
     void OnReportEnd() override { mOnReportEnd = true; }
 
@@ -198,7 +198,7 @@ void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aP
     mAttributeCount++;
 }
 
-void TestReadCallback::OnDone() {}
+void TestReadCallback::OnDone(app::ReadClient *) {}
 
 class TestMutableAttrAccess
 {
@@ -295,7 +295,7 @@ class TestMutableReadCallback : public app::ReadClient::Callback
     void OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
                          const app::StatusIB & aStatus) override;
 
-    void OnDone() override {}
+    void OnDone(app::ReadClient *) override {}
 
     void OnReportBegin() override { mAttributeCount = 0; }
 

diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp
@@ -263,7 +263,7 @@ class MockInteractionModelApp : public chip::app::ClusterStateCache::Callback
         mReadError = true;
     }
 
-    void OnDone() override {}
+    void OnDone(app::ReadClient *) override {}
 
     void OnDeallocatePaths(chip::app::ReadPrepareParams && aReadPrepareParams) override
     {
@@ -2052,7 +2052,7 @@ class TestReadCallback : public app::ReadClient::Callback
         }
     }
 
-    void OnDone() override { mOnDone++; }
+    void OnDone(app::ReadClient *) override { mOnDone++; }
 
     void OnReportEnd() override { mOnReportEnd++; }
 

@@ -293,7 +293,7 @@ - (instancetype)initWithDevice:(chip::DeviceProxy *)device
 
     void OnError(CHIP_ERROR aError) override;
 
-    void OnDone() override;
+    void OnDone(ReadClient * aReadClient) override;
 
     void OnDeallocatePaths(ReadPrepareParams && aReadPrepareParams) override;
 
@@ -789,7 +789,7 @@ void OnAttributeData(
 
     void OnError(CHIP_ERROR aError) override { mOnError(nullptr, aError); }
 
-    void OnDone() override { mOnDone(this); }
+    void OnDone(ReadClient *) override { mOnDone(this); }
 
     void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override
     {
@@ -1487,7 +1487,7 @@ - (instancetype)initWithPath:(const ConcreteEventPath &)path
 
 void SubscriptionCallback::OnError(CHIP_ERROR aError) { ReportError([CHIPError errorForCHIPErrorCode:aError]); }
 
-void SubscriptionCallback::OnDone()
+void SubscriptionCallback::OnDone(ReadClient *)
 {
     if (mOnDoneHandler) {
         mOnDoneHandler();