diff --git a/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp b/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp index 03c96e8c66be09..476ce1b75ceee8 100644 --- a/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp @@ -103,12 +103,18 @@ static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, ch } } +static void BoundDeviceContextReleaseHandler(void * context) +{ + (void) context; +} + static void InitBindingHandlerInternal(intptr_t arg) { auto & server = chip::Server::GetInstance(); chip::BindingManager::GetInstance().Init( { &server.GetFabricTable(), server.GetCASESessionManager(), &server.GetPersistentStorage() }); chip::BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(BoundDeviceChangedHandler); + chip::BindingManager::GetInstance().RegisterBoundDeviceContextReleaseHandler(BoundDeviceContextReleaseHandler); } CHIP_ERROR InitBindingHandlers() diff --git a/examples/light-switch-app/efr32/src/binding-handler.cpp b/examples/light-switch-app/efr32/src/binding-handler.cpp index 6ce872e811ae47..6a7f5bd8b238ae 100644 --- a/examples/light-switch-app/efr32/src/binding-handler.cpp +++ b/examples/light-switch-app/efr32/src/binding-handler.cpp @@ -132,6 +132,13 @@ void LightSwitchChangedHandler(const EmberBindingTableEntry & binding, DevicePro } } +void LightSwitchContextReleaseHandler(void * context) +{ + VerifyOrReturn(context != nullptr, ChipLogError(NotSpecified, "LightSwitchContextReleaseHandler: context is null")); + + Platform::Delete(static_cast(context)); +} + #ifdef ENABLE_CHIP_SHELL /******************************************************** @@ -384,6 +391,7 @@ void InitBindingHandlerInternal(intptr_t arg) chip::BindingManager::GetInstance().Init( { &server.GetFabricTable(), server.GetCASESessionManager(), &server.GetPersistentStorage() }); chip::BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(LightSwitchChangedHandler); + chip::BindingManager::GetInstance().RegisterBoundDeviceContextReleaseHandler(LightSwitchContextReleaseHandler); } } // namespace @@ -398,8 +406,6 @@ void SwitchWorkerFunction(intptr_t context) BindingCommandData * data = reinterpret_cast(context); BindingManager::GetInstance().NotifyBoundClusterChanged(data->localEndpointId, data->clusterId, static_cast(data)); - - Platform::Delete(data); } void BindingWorkerFunction(intptr_t context) diff --git a/examples/light-switch-app/nrfconnect/main/BindingHandler.cpp b/examples/light-switch-app/nrfconnect/main/BindingHandler.cpp index 54cce9924887fd..9cf98455d66ede 100644 --- a/examples/light-switch-app/nrfconnect/main/BindingHandler.cpp +++ b/examples/light-switch-app/nrfconnect/main/BindingHandler.cpp @@ -57,9 +57,13 @@ void BindingHandler::OnInvokeCommandFailure(DeviceProxy * aDevice, BindingData & // Set flag to not try recover session multiple times. BindingHandler::GetInstance().mCaseSessionRecovered = true; + // Allocate new object to make sure its life time will be appropriate. + BindingHandler::BindingData * data = Platform::New(); + *data = aBindingData; + // Establish new CASE session and retrasmit command that was not applied. error = BindingManager::GetInstance().NotifyBoundClusterChanged(aBindingData.EndpointId, aBindingData.ClusterId, - static_cast(&aBindingData)); + static_cast(data)); } else { @@ -222,6 +226,13 @@ void BindingHandler::LightSwitchChangedHandler(const EmberBindingTableEntry & bi } } +void BindingHandler::LightSwitchContextReleaseHandler(void * context) +{ + VerifyOrReturn(context != nullptr, LOG_ERR("Invalid context for Light switch context release handler");); + + Platform::Delete(static_cast(context)); +} + void BindingHandler::InitInternal(intptr_t aArg) { LOG_INF("Initialize binding Handler"); @@ -234,6 +245,7 @@ void BindingHandler::InitInternal(intptr_t aArg) } BindingManager::GetInstance().RegisterBoundDeviceChangedHandler(LightSwitchChangedHandler); + BindingManager::GetInstance().RegisterBoundDeviceContextReleaseHandler(LightSwitchContextReleaseHandler); BindingHandler::GetInstance().PrintBindingTable(); } @@ -298,6 +310,4 @@ void BindingHandler::SwitchWorkerHandler(intptr_t aContext) BindingData * data = reinterpret_cast(aContext); LOG_INF("Notify Bounded Cluster | endpoint: %d cluster: %d", data->EndpointId, data->ClusterId); BindingManager::GetInstance().NotifyBoundClusterChanged(data->EndpointId, data->ClusterId, static_cast(data)); - - Platform::Delete(data); } diff --git a/examples/light-switch-app/nrfconnect/main/LightSwitch.cpp b/examples/light-switch-app/nrfconnect/main/LightSwitch.cpp index 7420d8f5254feb..2b54264eb45b14 100644 --- a/examples/light-switch-app/nrfconnect/main/LightSwitch.cpp +++ b/examples/light-switch-app/nrfconnect/main/LightSwitch.cpp @@ -57,7 +57,6 @@ void LightSwitch::InitiateActionSwitch(Action mAction) } data->IsGroup = BindingHandler::GetInstance().IsGroupBound(); DeviceLayer::PlatformMgr().ScheduleWork(BindingHandler::SwitchWorkerHandler, reinterpret_cast(data)); - Platform::Delete(data); } } @@ -79,6 +78,5 @@ void LightSwitch::DimmerChangeBrightness() data->Value = (uint8_t) sBrightness; data->IsGroup = BindingHandler::GetInstance().IsGroupBound(); DeviceLayer::PlatformMgr().ScheduleWork(BindingHandler::SwitchWorkerHandler, reinterpret_cast(data)); - Platform::Delete(data); } } diff --git a/examples/light-switch-app/nrfconnect/main/include/BindingHandler.h b/examples/light-switch-app/nrfconnect/main/include/BindingHandler.h index fcd2d4fa8288b3..c3820e1ec33815 100644 --- a/examples/light-switch-app/nrfconnect/main/include/BindingHandler.h +++ b/examples/light-switch-app/nrfconnect/main/include/BindingHandler.h @@ -54,6 +54,7 @@ class BindingHandler static void OnOffProcessCommand(chip::CommandId, const EmberBindingTableEntry &, chip::DeviceProxy *, void *); static void LevelControlProcessCommand(chip::CommandId, const EmberBindingTableEntry &, chip::DeviceProxy *, void *); static void LightSwitchChangedHandler(const EmberBindingTableEntry &, chip::DeviceProxy *, void *); + static void LightSwitchContextReleaseHandler(void * context); static void InitInternal(intptr_t); bool mCaseSessionRecovered = false; diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index a87fe777c7533b..6f8b301b90f18f 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -19,6 +19,7 @@ #include #include #include +#include namespace { @@ -160,7 +161,7 @@ void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device) { fabricToRemove = entry.fabricIndex; nodeToRemove = entry.nodeId; - mBoundDeviceChangedHandler(entry, device, pendingNotification.mContext); + mBoundDeviceChangedHandler(entry, device, pendingNotification.mContext->GetContext()); } } mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove); @@ -190,6 +191,12 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR); + CHIP_ERROR error = CHIP_NO_ERROR; + auto * bindingContext = mPendingNotificationMap.NewPendingNotificationContext(context); + VerifyOrReturnError(bindingContext != nullptr, CHIP_ERROR_NO_MEMORY); + + bindingContext->IncrementConsumersNumber(); + for (auto iter = BindingTable::GetInstance().begin(); iter != BindingTable::GetInstance().end(); ++iter) { if (iter->local == endpoint && (!iter->clusterId.HasValue() || iter->clusterId.Value() == cluster)) @@ -203,21 +210,26 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste if (peerDevice != nullptr && peerDevice->IsConnected()) { // We already have an active connection - mBoundDeviceChangedHandler(*iter, peerDevice, context); + mBoundDeviceChangedHandler(*iter, peerDevice, bindingContext->GetContext()); } else { - mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), context); - ReturnErrorOnFailure(EstablishConnection(iter->fabricIndex, iter->nodeId)); + mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), bindingContext); + error = EstablishConnection(iter->fabricIndex, iter->nodeId); + SuccessOrExit(error == CHIP_NO_ERROR); } } else if (iter->type == EMBER_MULTICAST_BINDING) { - mBoundDeviceChangedHandler(*iter, nullptr, context); + mBoundDeviceChangedHandler(*iter, nullptr, bindingContext->GetContext()); } } } - return CHIP_NO_ERROR; + +exit: + bindingContext->DecrementConsumersNumber(); + + return error; } } // namespace chip diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index 88bedac4df357b..28fc11959e62ec 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -40,6 +40,12 @@ namespace chip { */ using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry & binding, DeviceProxy * peer_device, void * context); +/** + * Application callback function when a context used in NotifyBoundClusterChanged will not be needed and should be + * released. + */ +using BoundDeviceContextReleaseHandler = PendingNotificationContextReleaseHandler; + struct BindingManagerInitParams { FabricTable * mFabricTable = nullptr; @@ -70,6 +76,16 @@ class BindingManager void RegisterBoundDeviceChangedHandler(BoundDeviceChangedHandler handler) { mBoundDeviceChangedHandler = handler; } + /* + * Registers handler that will be called when context used in NotifyBoundClusterChanged will not be needed and could be + * released. + * + */ + void RegisterBoundDeviceContextReleaseHandler(BoundDeviceContextReleaseHandler handler) + { + mPendingNotificationMap.RegisterPendingNotificationContextReleaseHandler(handler); + } + CHIP_ERROR Init(const BindingManagerInitParams & params); /* diff --git a/src/app/clusters/bindings/PendingNotificationMap.cpp b/src/app/clusters/bindings/PendingNotificationMap.cpp index 7c992677587532..6676a82b779d4d 100644 --- a/src/app/clusters/bindings/PendingNotificationMap.cpp +++ b/src/app/clusters/bindings/PendingNotificationMap.cpp @@ -79,11 +79,15 @@ CHIP_ERROR PendingNotificationMap::FindLRUConnectPeer(FabricIndex * fabric, Node return CHIP_ERROR_NOT_FOUND; } -void PendingNotificationMap::AddPendingNotification(uint8_t bindingEntryId, void * context) +void PendingNotificationMap::AddPendingNotification(uint8_t bindingEntryId, PendingNotificationContext * context) { RemoveEntry(bindingEntryId); mPendingBindingEntries[mNumEntries] = bindingEntryId; mPendingContexts[mNumEntries] = context; + if (context) + { + context->IncrementConsumersNumber(); + } mNumEntries++; } @@ -98,6 +102,10 @@ void PendingNotificationMap::RemoveEntry(uint8_t bindingEntryId) mPendingContexts[newEntryCount] = mPendingContexts[i]; newEntryCount++; } + else if (mPendingContexts[i] != nullptr) + { + mPendingContexts[i]->DecrementConsumersNumber(); + } } mNumEntries = newEntryCount; } @@ -114,6 +122,10 @@ void PendingNotificationMap::RemoveAllEntriesForNode(FabricIndex fabric, NodeId mPendingContexts[newEntryCount] = mPendingContexts[i]; newEntryCount++; } + else if (mPendingContexts[i] != nullptr) + { + mPendingContexts[i]->DecrementConsumersNumber(); + } } mNumEntries = newEntryCount; } @@ -130,6 +142,10 @@ void PendingNotificationMap::RemoveAllEntriesForFabric(FabricIndex fabric) mPendingContexts[newEntryCount] = mPendingContexts[i]; newEntryCount++; } + else if (mPendingContexts[i] != nullptr) + { + mPendingContexts[i]->DecrementConsumersNumber(); + } } mNumEntries = newEntryCount; } diff --git a/src/app/clusters/bindings/PendingNotificationMap.h b/src/app/clusters/bindings/PendingNotificationMap.h index 2b66fa7efa97fc..5ef12ee560cbad 100644 --- a/src/app/clusters/bindings/PendingNotificationMap.h +++ b/src/app/clusters/bindings/PendingNotificationMap.h @@ -21,11 +21,46 @@ namespace chip { +/** + * Application callback function when a context used in PendingNotificationEntry will not be needed and should be + * released. + */ +using PendingNotificationContextReleaseHandler = void (*)(void * context); + +class PendingNotificationContext +{ +public: + PendingNotificationContext(void * context, PendingNotificationContextReleaseHandler contextReleaseHandler) : + mContext(context), mPendingNotificationContextReleaseHandler(contextReleaseHandler) + {} + void * GetContext() { return mContext; }; + uint32_t GetConsumersNumber() { return mConsumersNumber; } + void IncrementConsumersNumber() { mConsumersNumber++; } + void DecrementConsumersNumber() + { + VerifyOrDie(mConsumersNumber > 0); + if (--mConsumersNumber == 0) + { + // Release the context only if there is no pending notification pointing to us. + if (mPendingNotificationContextReleaseHandler != nullptr) + { + mPendingNotificationContextReleaseHandler(mContext); + } + Platform::Delete(this); + } + } + +private: + void * mContext; + uint32_t mConsumersNumber = 0; + PendingNotificationContextReleaseHandler mPendingNotificationContextReleaseHandler; +}; + struct PendingNotificationEntry { public: uint8_t mBindingEntryId; - void * mContext; + PendingNotificationContext * mContext; }; // The pool for all the pending comands. @@ -67,7 +102,7 @@ class PendingNotificationMap CHIP_ERROR FindLRUConnectPeer(FabricIndex * fabric, NodeId * node); - void AddPendingNotification(uint8_t bindingEntryId, void * context); + void AddPendingNotification(uint8_t bindingEntryId, PendingNotificationContext * context); void RemoveEntry(uint8_t bindingEntryId); @@ -77,9 +112,20 @@ class PendingNotificationMap void RemoveAllEntriesForFabric(FabricIndex fabric); + void RegisterPendingNotificationContextReleaseHandler(PendingNotificationContextReleaseHandler handler) + { + mPendingNotificationContextReleaseHandler = handler; + } + + PendingNotificationContext * NewPendingNotificationContext(void * context) + { + return Platform::New(context, mPendingNotificationContextReleaseHandler); + }; + private: uint8_t mPendingBindingEntries[kMaxPendingNotifications]; - void * mPendingContexts[kMaxPendingNotifications]; + PendingNotificationContext * mPendingContexts[kMaxPendingNotifications]; + PendingNotificationContextReleaseHandler mPendingNotificationContextReleaseHandler; uint8_t mNumEntries = 0; };