From 1258416a15623c90059cd9f71995c2ff7a949cf3 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 23 May 2022 18:15:17 -0400 Subject: [PATCH] Pass the ReadClient to ReadClient::Callback::OnDone. (#18740) This allows a single callback to be used across multiple read clients as needed, and is symmetric with how WriteClient and CommandSender work. --- examples/chip-tool/commands/clusters/ReportCommand.h | 2 +- .../tv-casting-common/commands/clusters/ReportCommand.h | 2 +- src/app/BufferedReadCallback.h | 2 +- src/app/ClusterStateCache.h | 4 ++-- src/app/ReadClient.cpp | 2 +- src/app/ReadClient.h | 8 +++++--- src/app/tests/TestBufferedReadCallback.cpp | 2 +- src/app/tests/TestClusterStateCache.cpp | 2 +- src/app/tests/TestReadInteraction.cpp | 2 +- src/app/tests/integration/chip_im_initiator.cpp | 7 ++++++- .../commands/interaction_model/InteractionModel.cpp | 4 +++- .../suites/commands/interaction_model/InteractionModel.h | 2 +- src/controller/CHIPDeviceController.cpp | 2 +- src/controller/CHIPDeviceController.h | 2 +- src/controller/TypedReadCallback.h | 4 ++-- src/controller/java/AndroidCallbacks.cpp | 2 +- src/controller/java/AndroidCallbacks.h | 2 +- src/controller/python/chip/clusters/attribute.cpp | 2 +- src/controller/tests/TestEventCaching.cpp | 2 +- src/controller/tests/TestEventChunking.cpp | 4 ++-- src/controller/tests/TestReadChunking.cpp | 6 +++--- src/controller/tests/data_model/TestRead.cpp | 4 ++-- src/darwin/Framework/CHIP/CHIPDevice.mm | 6 +++--- 23 files changed, 42 insertions(+), 33 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ReportCommand.h b/examples/chip-tool/commands/clusters/ReportCommand.h index eef50364807a4e..a6e5d12d5b8dff 100644 --- 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 index 92bdae2f04fa75..f99c5528892162 100644 --- 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 index c72516ce85b9f3..77159d7aece042 100644 --- 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 index ee9f9c465130f5..be6308aee1dadd 100644 --- 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 index 19ebc43e32a1c5..6ff7801934d0dd 100644 --- 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 index 7cb89fe46be003..68b2f63b1ab815 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -145,8 +145,9 @@ 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. @@ -154,8 +155,9 @@ class ReadClient : public Messaging::ExchangeDelegate * - 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 index 82f45c6ca7bad9..a4b57152259acb 100644 --- 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 mInstructionList; uint32_t mCurrentInstruction = 0; diff --git a/src/app/tests/TestClusterStateCache.cpp b/src/app/tests/TestClusterStateCache.cpp index 2aa7d48b1034a7..c8abf32cf4dfaa 100644 --- 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 index 92cba9456f6cbb..ee10fb4e68e4c4 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -181,7 +181,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 index 25086bceba395f..46008c216e7b93 100644 --- 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 index 48f675096663ee..61ead1cb835a7a 100644 --- 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 index d1b7cd7bfddd6f..af3aa2684b7191 100644 --- 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 index 1b01c192a50db4..e9dc953fc91e4b 100644 --- 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 index ce8b48e0d27550..aaf5393fb3b8c5 100644 --- 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 index 9283a9d375ebeb..13b75567a33b37 100644 --- 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 index 0d3e63805a5547..424fac43ee1605 100644 --- 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 index 35297d24e77072..420b826a33eea8 100644 --- 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 index 16cee29f58740d..5c33245450107f 100644 --- 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 index db71284ead893b..924e5a9482ac07 100644 --- 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 index 19b99ac0d434f8..c0d4da80ec2d9b 100644 --- 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 index d8b4d89913db41..e6365659d588ab 100644 --- 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 index 5d93aad5e7a9f0..66dcadc5f29087 100644 --- 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++; } diff --git a/src/darwin/Framework/CHIP/CHIPDevice.mm b/src/darwin/Framework/CHIP/CHIPDevice.mm index 82865ae5b421c8..ed9499f4664fde 100644 --- a/src/darwin/Framework/CHIP/CHIPDevice.mm +++ b/src/darwin/Framework/CHIP/CHIPDevice.mm @@ -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();