Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kkasperczyk-no committed Apr 26, 2022
1 parent 9923d67 commit 146cc57
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 49 deletions.
27 changes: 5 additions & 22 deletions src/app/clusters/bindings/BindingManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,7 @@ void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device)
{
fabricToRemove = entry.fabricIndex;
nodeToRemove = entry.nodeId;

BindingManagerContext * context = static_cast<BindingManagerContext *>(pendingNotification.mContext);
mBoundDeviceChangedHandler(entry, device, context->GetContext());

context->DecrementConsumersNumber();
if (context->GetConsumersNumber() == 0)
{
mBoundDeviceContextReleaseHandler(context->GetContext());
Platform::Delete(context);
}
mBoundDeviceChangedHandler(entry, device, pendingNotification.mContext->GetContext());
}
}
mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove);
Expand Down Expand Up @@ -199,10 +190,10 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste
{
VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR);
VerifyOrReturnError(mBoundDeviceContextReleaseHandler, CHIP_NO_ERROR);

CHIP_ERROR error = CHIP_NO_ERROR;
BindingManagerContext * bindingContext = Platform::New<BindingManagerContext>(static_cast<void *>(context));
CHIP_ERROR error = CHIP_NO_ERROR;
auto * bindingContext = mPendingNotificationMap.NewPendingNotificationContext(context);
VerifyOrReturnError(bindingContext != nullptr, CHIP_ERROR_NO_MEMORY);

bindingContext->IncrementConsumersNumber();

Expand All @@ -223,8 +214,7 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste
}
else
{
mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), static_cast<void *>(bindingContext));
bindingContext->IncrementConsumersNumber();
mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), bindingContext);
error = EstablishConnection(iter->fabricIndex, iter->nodeId);
SuccessOrExit(error == CHIP_NO_ERROR);
}
Expand All @@ -239,13 +229,6 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste
exit:
bindingContext->DecrementConsumersNumber();

// Call release bindingContext handler only if any pending notification is not going to use it.
if (bindingContext->GetConsumersNumber() == 0)
{
mBoundDeviceContextReleaseHandler(bindingContext->GetContext());
Platform::Delete(bindingContext);
}

return error;
}

Expand Down
25 changes: 2 additions & 23 deletions src/app/clusters/bindings/BindingManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,6 @@

namespace chip {

class BindingManagerContext
{
public:
BindingManagerContext(void * context) : mContext(context) {}
void * GetContext() { return mContext; };
uint8_t GetConsumersNumber() { return mConsumersNumber; }
void IncrementConsumersNumber() { mConsumersNumber++; }
void DecrementConsumersNumber()
{
if (mConsumersNumber > 0)
{
mConsumersNumber--;
}
}

private:
void * mContext;
uint8_t mConsumersNumber = 0;
};

/**
* Application callback function when a cluster associated with a binding changes.
*
Expand All @@ -64,7 +44,7 @@ using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry & bindin
* Application callback function when a context used in NotifyBoundClusterChanged will not be needed and should be
* released.
*/
using BoundDeviceContextReleaseHandler = void (*)(void * context);
using BoundDeviceContextReleaseHandler = PendingNotificationContextReleaseHandler;

struct BindingManagerInitParams
{
Expand Down Expand Up @@ -103,7 +83,7 @@ class BindingManager
*/
void RegisterBoundDeviceContextReleaseHandler(BoundDeviceContextReleaseHandler handler)
{
mBoundDeviceContextReleaseHandler = handler;
mPendingNotificationMap.RegisterPendingNotificationContextReleaseHandler(handler);
}

CHIP_ERROR Init(const BindingManagerInitParams & params);
Expand Down Expand Up @@ -153,7 +133,6 @@ class BindingManager

PendingNotificationMap mPendingNotificationMap;
BoundDeviceChangedHandler mBoundDeviceChangedHandler;
BoundDeviceContextReleaseHandler mBoundDeviceContextReleaseHandler;
BindingManagerInitParams mInitParams;

Callback::Callback<OnDeviceConnected> mOnConnectedCallback;
Expand Down
18 changes: 17 additions & 1 deletion src/app/clusters/bindings/PendingNotificationMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -130,6 +142,10 @@ void PendingNotificationMap::RemoveAllEntriesForFabric(FabricIndex fabric)
mPendingContexts[newEntryCount] = mPendingContexts[i];
newEntryCount++;
}
else if (mPendingContexts[i] != nullptr)
{
mPendingContexts[i]->DecrementConsumersNumber();
}
}
mNumEntries = newEntryCount;
}
Expand Down
55 changes: 52 additions & 3 deletions src/app/clusters/bindings/PendingNotificationMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,49 @@

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; };
uint8_t GetConsumersNumber() { return mConsumersNumber; }
void IncrementConsumersNumber() { mConsumersNumber++; }
void DecrementConsumersNumber()
{
if (mConsumersNumber > 0)
{
mConsumersNumber--;
}
else
{
// Release the context only if there is no pending notification pointing to it context.
if (mPendingNotificationContextReleaseHandler != nullptr)
{
mPendingNotificationContextReleaseHandler(mContext);
}
Platform::Delete(this);
}
}

private:
void * mContext;
uint8_t mConsumersNumber = 0;
PendingNotificationContextReleaseHandler mPendingNotificationContextReleaseHandler;
};

struct PendingNotificationEntry
{
public:
uint8_t mBindingEntryId;
void * mContext;
PendingNotificationContext * mContext;
};

// The pool for all the pending comands.
Expand Down Expand Up @@ -67,7 +105,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);

Expand All @@ -77,9 +115,20 @@ class PendingNotificationMap

void RemoveAllEntriesForFabric(FabricIndex fabric);

void RegisterPendingNotificationContextReleaseHandler(PendingNotificationContextReleaseHandler handler)
{
mPendingNotificationContextReleaseHandler = handler;
}

PendingNotificationContext * NewPendingNotificationContext(void * context)
{
return Platform::New<PendingNotificationContext>(context, mPendingNotificationContextReleaseHandler);
};

private:
uint8_t mPendingBindingEntries[kMaxPendingNotifications];
void * mPendingContexts[kMaxPendingNotifications];
PendingNotificationContext * mPendingContexts[kMaxPendingNotifications];
PendingNotificationContextReleaseHandler mPendingNotificationContextReleaseHandler;

uint8_t mNumEntries = 0;
};
Expand Down

0 comments on commit 146cc57

Please sign in to comment.