diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp index 13981a95c0e787..063d87ac200830 100644 --- a/src/app/clusters/bindings/BindingManager.cpp +++ b/src/app/clusters/bindings/BindingManager.cpp @@ -36,15 +36,9 @@ namespace chip { BindingManager BindingManager::sBindingManager; -CHIP_ERROR BindingManager::UnicastBindingCreated(uint8_t bindingEntryId) +CHIP_ERROR BindingManager::UnicastBindingCreated(const EmberBindingTableEntry & bindingEntry) { - EmberBindingTableEntry entry{}; - emberGetBinding(bindingEntryId, &entry); - if (entry.type != EMBER_UNICAST_BINDING) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - return EstablishConnection(entry.fabricIndex, entry.nodeId); + return EstablishConnection(bindingEntry.fabricIndex, bindingEntry.nodeId); } CHIP_ERROR BindingManager::UnicastBindingRemoved(uint8_t bindingEntryId) @@ -72,13 +66,12 @@ CHIP_ERROR BindingManager::EstablishConnection(FabricIndex fabric, NodeId node) // Release the least recently used entry // TODO: Some reference counting mechanism shall be added the CASESessionManager // so that other session clients don't get accidentally closed. - int16_t lruBindingEntryIndex = mPendingNotificationMap.FindLRUBindingEntryIndex(); - if (lruBindingEntryIndex >= 0) + FabricIndex fabricToRemove; + NodeId nodeToRemove; + if (mPendingNotificationMap.FindLRUConnectPeer(&fabricToRemove, &nodeToRemove) == CHIP_NO_ERROR) { - EmberBindingTableEntry entry; - emberGetBinding(static_cast(lruBindingEntryIndex), &entry); - mPendingNotificationMap.RemoveAllEntriesForNode(entry.fabricIndex, entry.nodeId); - PeerId lruPeer = PeerIdForNode(mAppServer->GetFabricTable(), entry.fabricIndex, entry.nodeId); + mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove); + PeerId lruPeer = PeerIdForNode(mAppServer->GetFabricTable(), fabricToRemove, nodeToRemove); mAppServer->GetCASESessionManager()->ReleaseSession(lruPeer); // Now retry error = mAppServer->GetCASESessionManager()->FindOrEstablishSession(peer, &mOnConnectedCallback, @@ -96,6 +89,9 @@ void BindingManager::HandleDeviceConnected(void * context, OperationalDeviceProx void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device) { + FabricIndex fabricToRemove = kUndefinedFabricIndex; + NodeId nodeToRemove = kUndefinedNodeId; + for (const PendingNotificationEntry & pendingNotification : mPendingNotificationMap) { EmberBindingTableEntry entry; @@ -104,9 +100,12 @@ void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device) PeerId peer = PeerIdForNode(mAppServer->GetFabricTable(), entry.fabricIndex, entry.nodeId); if (device->GetPeerId() == peer) { + fabricToRemove = entry.fabricIndex; + nodeToRemove = entry.nodeId; mBoundDeviceChangedHandler(&entry, device, pendingNotification.mContext); } } + mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove); } void BindingManager::HandleDeviceConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error) diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h index ef2b0bd36dbb32..4b3397d132f87d 100644 --- a/src/app/clusters/bindings/BindingManager.h +++ b/src/app/clusters/bindings/BindingManager.h @@ -67,10 +67,10 @@ class BindingManager * Notifies the BindingManager that a new unicast binding is created. * */ - CHIP_ERROR UnicastBindingCreated(uint8_t bindingEntryId); + CHIP_ERROR UnicastBindingCreated(const EmberBindingTableEntry & bindingEntry); /* - * Notifies the BindingManager that a new unicast binding is removed. + * Notifies the BindingManager that a unicast binding is about to be removed from the given index. * */ CHIP_ERROR UnicastBindingRemoved(uint8_t bindingEntryId); diff --git a/src/app/clusters/bindings/PendingNotificationMap.cpp b/src/app/clusters/bindings/PendingNotificationMap.cpp index b76677dd7ffb98..7d3cf7a10bd90b 100644 --- a/src/app/clusters/bindings/PendingNotificationMap.cpp +++ b/src/app/clusters/bindings/PendingNotificationMap.cpp @@ -22,7 +22,7 @@ namespace chip { -int16_t PendingNotificationMap::FindLRUBindingEntryIndex() +CHIP_ERROR PendingNotificationMap::FindLRUConnectPeer(FabricIndex * fabric, NodeId * node) { uint8_t bindingWithSamePeer[EMBER_BINDING_TABLE_SIZE]; @@ -64,8 +64,8 @@ int16_t PendingNotificationMap::FindLRUBindingEntryIndex() lastAppear[bindingWithSamePeer[pendingNotification.mBindingEntryId]] = appearIndex; appearIndex++; } - int16_t lruBindingEntryIndex = -1; - uint16_t minApperValue = UINT16_MAX; + uint8_t lruBindingEntryIndex; + uint16_t minApperValue = UINT16_MAX; for (uint8_t i = 0; i < EMBER_BINDING_TABLE_SIZE; i++) { if (lastAppear[i] < minApperValue) @@ -74,65 +74,56 @@ int16_t PendingNotificationMap::FindLRUBindingEntryIndex() minApperValue = lastAppear[i]; } } - return lruBindingEntryIndex; + if (minApperValue < UINT16_MAX) + { + EmberBindingTableEntry entry; + emberGetBinding(lruBindingEntryIndex, &entry); + *fabric = entry.fabricIndex; + *node = entry.nodeId; + return CHIP_NO_ERROR; + } + return CHIP_ERROR_NOT_FOUND; } void PendingNotificationMap::AddPendingNotification(uint8_t bindingEntryId, void * context) { RemoveEntry(bindingEntryId); - uint8_t addIndex = mNumEntries < kMaxPendingNotifications ? mNumEntries : mNextToOverride; - mPendingBindingEntries[addIndex] = bindingEntryId; - mPendingContexts[addIndex] = context; - if (mNumEntries < kMaxPendingNotifications) - { - mNumEntries++; - } - else - { - mNextToOverride++; - } + mPendingBindingEntries[mNumEntries] = bindingEntryId; + mPendingContexts[mNumEntries] = context; + mNumEntries++; } void PendingNotificationMap::RemoveEntry(uint8_t bindingEntryId) { - uint8_t newBindingEntries[kMaxPendingNotifications]; - void * newContexts[kMaxPendingNotifications]; uint8_t newEntryCount = 0; - for (const PendingNotificationEntry & pendingNotification : *this) + for (int i = 0; i < mNumEntries; i++) { - if (pendingNotification.mBindingEntryId != bindingEntryId) + if (mPendingBindingEntries[i] != bindingEntryId) { - newBindingEntries[newEntryCount] = pendingNotification.mBindingEntryId; - newContexts[newEntryCount] = pendingNotification.mContext; + mPendingBindingEntries[newEntryCount] = mPendingBindingEntries[i]; + mPendingContexts[newEntryCount] = mPendingContexts[i]; newEntryCount++; } } - memcpy(mPendingBindingEntries, newBindingEntries, sizeof(newBindingEntries)); - memcpy(mPendingContexts, newContexts, sizeof(newContexts)); - mNextToOverride = 0; - mNumEntries = newEntryCount; + mNumEntries = newEntryCount; } void PendingNotificationMap::RemoveAllEntriesForNode(FabricIndex fabric, NodeId node) { - uint8_t newBindingEntries[kMaxPendingNotifications]; - void * newContexts[kMaxPendingNotifications]; uint8_t newEntryCount = 0; - for (const PendingNotificationEntry & pendingNotification : *this) + for (int i = 0; i < mNumEntries; i++) { EmberBindingTableEntry entry; - emberGetBinding(pendingNotification.mBindingEntryId, &entry); + emberGetBinding(mPendingBindingEntries[i], &entry); + if (entry.fabricIndex != fabric || entry.nodeId != node) { - newBindingEntries[newEntryCount] = pendingNotification.mBindingEntryId; - newContexts[newEntryCount] = pendingNotification.mContext; + mPendingBindingEntries[newEntryCount] = mPendingBindingEntries[i]; + mPendingContexts[newEntryCount] = mPendingContexts[i]; newEntryCount++; } } - memcpy(mPendingBindingEntries, newBindingEntries, sizeof(newBindingEntries)); - memcpy(mPendingContexts, newContexts, sizeof(newContexts)); - mNextToOverride = 0; - mNumEntries = newEntryCount; + mNumEntries = newEntryCount; } } // namespace chip diff --git a/src/app/clusters/bindings/PendingNotificationMap.h b/src/app/clusters/bindings/PendingNotificationMap.h index 9c33d756075edb..417606a5c142bb 100644 --- a/src/app/clusters/bindings/PendingNotificationMap.h +++ b/src/app/clusters/bindings/PendingNotificationMap.h @@ -48,12 +48,6 @@ class PendingNotificationMap Iterator operator++() { mIndex++; - if (mIndex == mMap->mNumEntries) - { - // end of iteration - mIndex = -1; - } - mIndex = static_cast(mIndex % kMaxPendingNotifications); return *this; } @@ -64,11 +58,11 @@ class PendingNotificationMap int16_t mIndex; }; - Iterator begin() { return Iterator(this, mNumEntries > 0 ? mNextToOverride : -1); } + Iterator begin() { return Iterator(this, 0); } - Iterator end() { return Iterator(this, -1); } + Iterator end() { return Iterator(this, mNumEntries); } - int16_t FindLRUBindingEntryIndex(); + CHIP_ERROR FindLRUConnectPeer(FabricIndex * fabric, NodeId * node); void AddPendingNotification(uint8_t bindingEntryId, void * context); @@ -80,8 +74,7 @@ class PendingNotificationMap uint8_t mPendingBindingEntries[kMaxPendingNotifications]; void * mPendingContexts[kMaxPendingNotifications]; - uint8_t mNumEntries = 0; - uint8_t mNextToOverride = 0; + uint8_t mNumEntries = 0; }; } // namespace chip diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 8a3b0cccef673f..cfe7ea5f81e773 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -111,7 +111,7 @@ bool emberAfBindingClusterBindCallback(app::CommandHandler * commandObj, const a emberSetBinding(bindingIndex, &bindingEntry); if (nodeId) { - CHIP_ERROR err = BindingManager::GetInstance().UnicastBindingCreated(bindingIndex); + CHIP_ERROR err = BindingManager::GetInstance().UnicastBindingCreated(bindingEntry); if (err != CHIP_NO_ERROR) { ChipLogProgress(