Skip to content

Commit

Permalink
[binding] Reduce binding manager size (#14303)
Browse files Browse the repository at this point in the history 
* [binding] Reduce binding manager size

The PR stores the pending notifications as a flat array of binding table
indecies to recude the size of the BindingManager class.

* remove test code

* fix cmake build

* fix review comments
  • Loading branch information
@gjc13
gjc13 authored Feb 7, 2022
1 parent 566402a commit 6f9fa8e
Show file tree
Hide file tree
Showing 9 changed files with 304 additions and 235 deletions.
1 change: 1 addition & 0 deletions examples/all-clusters-app/mbed/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ target_sources(${APP_TARGET} PRIVATE
${APP_CLUSTERS}/basic/basic.cpp
${APP_CLUSTERS}/bindings/BindingManager.cpp
${APP_CLUSTERS}/bindings/bindings.cpp
${APP_CLUSTERS}/bindings/PendingNotificationMap.cpp
${APP_CLUSTERS}/on-off-server/on-off-server.cpp
${APP_CLUSTERS}/access-control-server/access-control-server.cpp
${APP_CLUSTERS}/account-login-server/account-login-server.cpp
Expand Down
1 change: 1 addition & 0 deletions examples/lighting-app/telink/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ target_sources(app PRIVATE
${CHIP_ROOT}/src/app/clusters/basic/basic.cpp
${CHIP_ROOT}/src/app/clusters/bindings/BindingManager.cpp
${CHIP_ROOT}/src/app/clusters/bindings/bindings.cpp
${CHIP_ROOT}/src/app/clusters/bindings/PendingNotificationMap.cpp
${CHIP_ROOT}/src/app/clusters/descriptor/descriptor.cpp
${CHIP_ROOT}/src/app/clusters/identify-server/identify-server.cpp
${CHIP_ROOT}/src/app/clusters/diagnostic-logs-server/diagnostic-logs-server.cpp
Expand Down
2 changes: 2 additions & 0 deletions src/app/chip_data_model.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ template("chip_data_model") {
"${_app_root}/clusters/${cluster}/${cluster}.cpp",
"${_app_root}/clusters/${cluster}/BindingManager.cpp",
"${_app_root}/clusters/${cluster}/BindingManager.h",
"${_app_root}/clusters/${cluster}/PendingNotificationMap.cpp",
"${_app_root}/clusters/${cluster}/PendingNotificationMap.h",
]
} else {
sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
Expand Down
148 changes: 49 additions & 99 deletions src/app/clusters/bindings/BindingManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,37 @@ BindingFabricTableDelegate gFabricTableDelegate;

} // namespace

namespace {

chip::PeerId PeerIdForNode(chip::FabricTable & fabricTable, chip::FabricIndex fabric, chip::NodeId node)
{
chip::FabricInfo * fabricInfo = fabricTable.FindFabricWithIndex(fabric);
if (fabricInfo == nullptr)
{
return chip::PeerId();
}
return fabricInfo->GetPeerIdForNode(node);
}

} // namespace

namespace chip {

BindingManager BindingManager::sBindingManager;

CHIP_ERROR BindingManager::UnicastBindingCreated(const EmberBindingTableEntry & bindingEntry)
{
return EstablishConnection(bindingEntry.fabricIndex, bindingEntry.nodeId);
}

CHIP_ERROR BindingManager::UnicastBindingRemoved(uint8_t bindingEntryId)
{
EmberBindingTableEntry entry{};
emberGetBinding(bindingEntryId, &entry);
mPendingNotificationMap.RemoveEntry(bindingEntryId);
return CHIP_NO_ERROR;
}

void BindingManager::SetAppServer(Server * appServer)
{
mAppServer = appServer;
Expand All @@ -63,21 +90,22 @@ CHIP_ERROR BindingManager::EstablishConnection(FabricIndex fabric, NodeId node)
{
VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE);

FabricInfo * fabricInfo = mAppServer->GetFabricTable().FindFabricWithIndex(fabric);
VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_NOT_FOUND);
PeerId peer = fabricInfo->GetPeerIdForNode(node);
PeerId peer = PeerIdForNode(mAppServer->GetFabricTable(), fabric, node);
VerifyOrReturnError(peer.GetNodeId() != kUndefinedNodeId, CHIP_ERROR_NOT_FOUND);
CHIP_ERROR error =
mAppServer->GetCASESessionManager()->FindOrEstablishSession(peer, &mOnConnectedCallback, &mOnConnectionFailureCallback);
if (error == CHIP_ERROR_NO_MEMORY)
{
// 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.
PendingNotificationEntry * entry = mPendingNotificationMap.FindLRUEntry();
if (entry != nullptr)
FabricIndex fabricToRemove;
NodeId nodeToRemove;
if (mPendingNotificationMap.FindLRUConnectPeer(&fabricToRemove, &nodeToRemove) == CHIP_NO_ERROR)
{
mAppServer->GetCASESessionManager()->ReleaseSession(entry->GetPeerId());
mPendingNotificationMap.RemoveEntry(entry);
mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove);
PeerId lruPeer = PeerIdForNode(mAppServer->GetFabricTable(), fabricToRemove, nodeToRemove);
mAppServer->GetCASESessionManager()->ReleaseSession(lruPeer);
// Now retry
error = mAppServer->GetCASESessionManager()->FindOrEstablishSession(peer, &mOnConnectedCallback,
&mOnConnectionFailureCallback);
Expand All @@ -94,33 +122,23 @@ void BindingManager::HandleDeviceConnected(void * context, OperationalDeviceProx

void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device)
{
mPendingNotificationMap.ForEachActiveObject([&](PendingNotificationEntry * entry) -> Loop {
if (entry->GetPeerId() == device->GetPeerId())
{
SyncPendingNotificationsToPeer(device, entry);
}
FabricIndex fabricToRemove = kUndefinedFabricIndex;
NodeId nodeToRemove = kUndefinedNodeId;

return Loop::Continue;
});
}

void BindingManager::SyncPendingNotificationsToPeer(OperationalDeviceProxy * device, PendingNotificationEntry * pendingClusters)
{
for (const ClusterPath & path : *pendingClusters)
for (const PendingNotificationEntry & pendingNotification : mPendingNotificationMap)
{
ClusterId cluster = path.cluster;
EndpointId endpoint = path.endpoint;
for (uint8_t j = 0; j < EMBER_BINDING_TABLE_SIZE; j++)
EmberBindingTableEntry entry;
emberGetBinding(pendingNotification.mBindingEntryId, &entry);

PeerId peer = PeerIdForNode(mAppServer->GetFabricTable(), entry.fabricIndex, entry.nodeId);
if (device->GetPeerId() == peer)
{
EmberBindingTableEntry entry;
if (emberGetBinding(j, &entry) == EMBER_SUCCESS && entry.type == EMBER_UNICAST_BINDING && entry.clusterId == cluster &&
entry.local == endpoint && mBoundDeviceChangedHandler)
{
mBoundDeviceChangedHandler(&entry, device, path.context);
}
fabricToRemove = entry.fabricIndex;
nodeToRemove = entry.nodeId;
mBoundDeviceChangedHandler(&entry, device, pendingNotification.mContext);
}
}
mPendingNotificationMap.RemoveEntry(pendingClusters);
mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove);
}

void BindingManager::HandleDeviceConnectionFailure(void * context, PeerId peerId, CHIP_ERROR error)
Expand All @@ -138,34 +156,10 @@ void BindingManager::HandleDeviceConnectionFailure(PeerId peerId, CHIP_ERROR err

void BindingManager::FabricRemoved(CompressedFabricId compressedFabricId, FabricIndex fabricIndex)
{
mPendingNotificationMap.ForEachActiveObject([&](PendingNotificationEntry * entry) {
if (entry->GetFabricIndex() == fabricIndex)
{
mPendingNotificationMap.RemoveEntry(entry);
return Loop::Break;
}
return Loop::Continue;
});
mPendingNotificationMap.RemoveAllEntriesForFabric(fabricIndex);
mAppServer->GetCASESessionManager()->ReleaseSessionForFabric(compressedFabricId);
}

CHIP_ERROR BindingManager::LastUnicastBindingRemoved(FabricIndex fabricIndex, NodeId node)
{
VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE);

FabricInfo * fabricInfo = mAppServer->GetFabricTable().FindFabricWithIndex(fabricIndex);
VerifyOrReturnError(fabricInfo != nullptr, CHIP_ERROR_NOT_FOUND);
PeerId peer = fabricInfo->GetPeerIdForNode(node);
PendingNotificationEntry * entry = mPendingNotificationMap.FindEntry(fabricIndex, node);
if (entry)
{
mPendingNotificationMap.RemoveEntry(entry);
}

mAppServer->GetCASESessionManager()->ReleaseSession(peer);
return CHIP_NO_ERROR;
}

CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, void * context)
{
VerifyOrReturnError(mAppServer != nullptr, CHIP_ERROR_INCORRECT_STATE);
Expand All @@ -190,9 +184,7 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste
}
else
{
// Enqueue pending cluster and establish connection
ReturnErrorOnFailure(mPendingNotificationMap.AddPendingNotification(entry.fabricIndex, entry.nodeId, endpoint,
cluster, context));
mPendingNotificationMap.AddPendingNotification(i, context);
ReturnErrorOnFailure(EstablishConnection(entry.fabricIndex, entry.nodeId));
}
}
Expand All @@ -205,46 +197,4 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste
return CHIP_NO_ERROR;
}

BindingManager::PendingNotificationEntry * BindingManager::PendingNotificationMap::FindLRUEntry()
{
PendingNotificationEntry * lruEntry = nullptr;
mPendingNotificationMap.ForEachActiveObject([&](PendingNotificationEntry * entry) {
if (lruEntry == nullptr || lruEntry->GetLastUpdateTime() > entry->GetLastUpdateTime())
{
lruEntry = entry;
}
return Loop::Continue;
});
return lruEntry;
}

BindingManager::PendingNotificationEntry * BindingManager::PendingNotificationMap::FindEntry(FabricIndex fabricIndex, NodeId node)
{
PendingNotificationEntry * foundEntry = nullptr;
mPendingNotificationMap.ForEachActiveObject([&](PendingNotificationEntry * entry) {
if (entry->GetFabricIndex() == fabricIndex && entry->GetNodeId() == node)
{
foundEntry = entry;
return Loop::Break;
}
return Loop::Continue;
});
return foundEntry;
}

CHIP_ERROR BindingManager::PendingNotificationMap::AddPendingNotification(FabricIndex fabric, NodeId node, EndpointId endpoint,
ClusterId cluster, void * context)
{
PendingNotificationEntry * entry = FindEntry(fabric, node);

if (entry == nullptr)
{
entry = mPendingNotificationMap.CreateObject(fabric, node);
VerifyOrReturnError(entry != nullptr, CHIP_ERROR_NO_MEMORY);
}
entry->AddPendingNotification(endpoint, cluster, context);
entry->Touch();
return CHIP_NO_ERROR;
}

} // namespace chip
114 changes: 6 additions & 108 deletions src/app/clusters/bindings/BindingManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#pragma once

#include <app/CASESessionManager.h>
#include <app/clusters/bindings/PendingNotificationMap.h>
#include <app/server/Server.h>
#include <app/util/binding-table.h>

Expand Down Expand Up @@ -53,8 +54,6 @@ using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry * bindin
*/
class BindingManager
{
friend class PendingNotificationEntry;

public:
BindingManager() :
mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this)
Expand All @@ -68,19 +67,19 @@ class BindingManager
* Notifies the BindingManager that a new unicast binding is created.
*
*/
CHIP_ERROR UnicastBindingCreated(FabricIndex fabric, NodeId node) { return EstablishConnection(fabric, node); }
CHIP_ERROR UnicastBindingCreated(const EmberBindingTableEntry & bindingEntry);

/*
* Notifies the BindingManager that a fabric is removed from the device
* Notifies the BindingManager that a unicast binding is about to be removed from the given index.
*
*/
void FabricRemoved(CompressedFabricId compressedId, FabricIndex fabricIndex);
CHIP_ERROR UnicastBindingRemoved(uint8_t bindingEntryId);

/*
* Notfies the BindingManager that the **last** unicast binding to a device has been removed.
* Notifies the BindingManager that a fabric is removed from the device
*
*/
CHIP_ERROR LastUnicastBindingRemoved(FabricIndex fabricIndex, NodeId node);
void FabricRemoved(CompressedFabricId compressedId, FabricIndex fabricIndex);

/*
* Notify a cluster change to **all** bound devices associated with the (endpoint, cluster) tuple.
Expand All @@ -99,104 +98,6 @@ class BindingManager
private:
static BindingManager sBindingManager;

static constexpr uint8_t kMaxPendingNotifications = 3;

struct ClusterPath
{
void * context;
ClusterId cluster;
EndpointId endpoint;
};

// A pending notification to be sent to a binding waiting for the CASE session to be established.
class PendingNotificationEntry
{
public:
PendingNotificationEntry(FabricIndex fabricIndex, NodeId node) : mNodeId(node), mFabricIndex(fabricIndex) {}

PeerId GetPeerId()
{
PeerId peer;
if (BindingManager::GetInstance().mAppServer == nullptr)
{
return peer;
}
FabricInfo * fabric = BindingManager::GetInstance().mAppServer->GetFabricTable().FindFabricWithIndex(mFabricIndex);
if (fabric == nullptr)
{
return peer;
}
return fabric->GetPeerIdForNode(mNodeId);
}

NodeId GetNodeId() { return mNodeId; }

FabricIndex GetFabricIndex() { return mFabricIndex; }

System::Clock::Timestamp GetLastUpdateTime() { return mLastUpdateTime; }
void Touch() { mLastUpdateTime = System::SystemClock().GetMonotonicTimestamp(); }

ClusterPath * begin() { return &mPendingNotifications[0]; }
ClusterPath * end() { return &mPendingNotifications[mNumPendingNotifications]; }

void AddPendingNotification(EndpointId endpoint, ClusterId cluster, void * context)
{
for (ClusterPath & path : *this)
{
// New notifications for the same (endpoint, cluster) shall
// simply overrride the old ones
if (path.cluster == cluster && path.endpoint == endpoint)
{
path.context = context;
return;
}
}
if (mNumPendingNotifications < kMaxPendingNotifications)
{
mPendingNotifications[mNumPendingNotifications++] = { context, cluster, endpoint };
}
else
{
mPendingNotifications[mNextToOverride] = { context, cluster, endpoint };
mNextToOverride++;
mNextToOverride %= kMaxPendingNotifications;
}
}

private:
System::Clock::Timestamp mLastUpdateTime;
// TODO: Make the pending notifications list of binding table indecies and list of contexts
ClusterPath mPendingNotifications[kMaxPendingNotifications];

NodeId mNodeId;
uint8_t mNumPendingNotifications = 0;
uint8_t mNextToOverride = 0;
FabricIndex mFabricIndex;
};

// The pool for all the pending comands.
class PendingNotificationMap
{
public:
PendingNotificationEntry * FindLRUEntry();

PendingNotificationEntry * FindEntry(FabricIndex fabricIndex, NodeId node);

CHIP_ERROR AddPendingNotification(FabricIndex fabricIndex, NodeId node, EndpointId endpoint, ClusterId cluster,
void * context);

void RemoveEntry(PendingNotificationEntry * entry) { mPendingNotificationMap.ReleaseObject(entry); }

template <typename Function>
Loop ForEachActiveObject(Function && function)
{
return mPendingNotificationMap.ForEachActiveObject(std::forward<Function>(function));
}

private:
ObjectPool<PendingNotificationEntry, EMBER_BINDING_TABLE_SIZE> mPendingNotificationMap;
};

static void HandleDeviceConnected(void * context, OperationalDeviceProxy * device);
void HandleDeviceConnected(OperationalDeviceProxy * device);

Expand All @@ -205,9 +106,6 @@ class BindingManager

CHIP_ERROR EstablishConnection(FabricIndex fabric, NodeId node);

// Called when CASE session is established to a peer device. Will send all the pending commands to the peer.
void SyncPendingNotificationsToPeer(OperationalDeviceProxy * device, PendingNotificationEntry * pendingClusters);

PendingNotificationMap mPendingNotificationMap;
BoundDeviceChangedHandler mBoundDeviceChangedHandler;
Server * mAppServer = nullptr;
Expand Down
Loading

0 comments on commit 6f9fa8e

Please sign in to comment.