Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Application-managed ReadClient objects #13108

Merged
merged 8 commits into from
Dec 22, 2021
Merged

Application-managed ReadClient objects #13108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6624609
Heap-ify ReadClient
mrjerryjohns Dec 17, 2021
cae3828
Review feedback
mrjerryjohns Dec 20, 2021
55dd826
Merge branch 'master' into read-client
mrjerryjohns Dec 20, 2021
5894e2e
Merge branch 'master' into read-client
mrjerryjohns Dec 20, 2021
6d33e2d
Relaxed the lifetime coupling between the ReadClient the
mrjerryjohns Dec 20, 2021
af63c57
Merge branch 'master' into read-client
mrjerryjohns Dec 21, 2021
5298455
Forgot to remove a Cirque test
mrjerryjohns Dec 21, 2021
0382a5a
Add back initaitor and responder tests, as well as add a test for shu…
mrjerryjohns Dec 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 100 additions & 85 deletions src/app/InteractionModelEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,8 @@ void InteractionModelEngine::Shutdown()
mpExchangeMgr->CloseAllContextsForDelegate(obj);
return Loop::Continue;
});
mTimedHandlers.ReleaseAll();

for (auto & readClient : mReadClients)
{
if (!readClient.IsFree())
{
readClient.Shutdown();
}
}
mTimedHandlers.ReleaseAll();

for (auto & readHandler : mReadHandlers)
{
Expand All @@ -102,6 +95,28 @@ void InteractionModelEngine::Shutdown()
}
}

//
// We hold weak references to ReadClient objects. The application ultimately
// actually owns them, so it's on them to eventually shut them down and free them
// up.
//
// However, we should null out their pointers back to us at the very least so that
// at destruction time, they won't attempt to reach back here to remove themselves
// from this list.
//
for (auto * readClient = mpActiveReadClientList; readClient != nullptr;)
{
readClient->mpImEngine = nullptr;
auto * tmpClient = readClient->GetNextClient();
readClient->SetNextClient(nullptr);
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
readClient = tmpClient;
}

//
// After that, we just null out our tracker.
//
mpActiveReadClientList = nullptr;

for (auto & writeClient : mWriteClients)
{
if (!writeClient.IsFree())
Expand All @@ -127,43 +142,6 @@ void InteractionModelEngine::Shutdown()
mpExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id);
}

CHIP_ERROR InteractionModelEngine::NewReadClient(ReadClient ** const apReadClient, ReadClient::InteractionType aInteractionType,
ReadClient::Callback * aCallback)
{
*apReadClient = nullptr;
for (auto & readClient : mReadClients)
{
if (readClient.IsFree())
{
CHIP_ERROR err;

*apReadClient = &readClient;
err = readClient.Init(mpExchangeMgr, aCallback, aInteractionType);
if (CHIP_NO_ERROR != err)
{
*apReadClient = nullptr;
}
return err;
}
}
return CHIP_ERROR_NO_MEMORY;
}

uint32_t InteractionModelEngine::GetNumActiveReadClients() const
{
uint32_t numActive = 0;

for (auto & readClient : mReadClients)
{
if (!readClient.IsFree())
{
numActive++;
}
}

return numActive;
}

uint32_t InteractionModelEngine::GetNumActiveReadHandlers() const
{
uint32_t numActive = 0;
Expand Down Expand Up @@ -211,35 +189,30 @@ uint32_t InteractionModelEngine::GetNumActiveWriteHandlers() const

CHIP_ERROR InteractionModelEngine::ShutdownSubscription(uint64_t aSubscriptionId)
{
CHIP_ERROR err = CHIP_ERROR_KEY_NOT_FOUND;

for (auto & readClient : mReadClients)
for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
{
if (!readClient.IsFree() && readClient.IsSubscriptionType() && readClient.IsMatchingClient(aSubscriptionId))
if (readClient->IsSubscriptionType() && readClient->IsMatchingClient(aSubscriptionId))
{
readClient.Shutdown();
err = CHIP_NO_ERROR;
readClient->Close(CHIP_NO_ERROR);
return CHIP_NO_ERROR;
}
}

return err;
return CHIP_ERROR_KEY_NOT_FOUND;
}

CHIP_ERROR InteractionModelEngine::ShutdownSubscriptions(FabricIndex aFabricIndex, NodeId aPeerNodeId)
{
CHIP_ERROR err = CHIP_ERROR_KEY_NOT_FOUND;

for (ReadClient & readClient : mReadClients)
for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
{
if (!readClient.IsFree() && readClient.IsSubscriptionType() && readClient.GetFabricIndex() == aFabricIndex &&
readClient.GetPeerNodeId() == aPeerNodeId)
if (readClient->IsSubscriptionType() && readClient->GetFabricIndex() == aFabricIndex &&
readClient->GetPeerNodeId() == aPeerNodeId)
{
readClient.Shutdown();
err = CHIP_NO_ERROR;
readClient->Close(CHIP_NO_ERROR);
}
}

return err;
return CHIP_NO_ERROR;
}

CHIP_ERROR InteractionModelEngine::NewWriteClient(WriteClientHandle & apWriteClient, WriteClient::Callback * apCallback,
Expand Down Expand Up @@ -392,18 +365,21 @@ CHIP_ERROR InteractionModelEngine::OnUnsolicitedReportData(Messaging::ExchangeCo
uint64_t subscriptionId = 0;
ReturnLogErrorOnFailure(report.GetSubscriptionId(&subscriptionId));

for (auto & readClient : mReadClients)
for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
{
if (!readClient.IsSubscriptionIdle())
if (!readClient->IsSubscriptionIdle())
{
continue;
}
if (!readClient.IsMatchingClient(subscriptionId))

if (!readClient->IsMatchingClient(subscriptionId))
{
continue;
}
return readClient.OnUnsolicitedReportData(apExchangeContext, std::move(aPayload));

return readClient->OnUnsolicitedReportData(apExchangeContext, std::move(aPayload));
}

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -462,40 +438,79 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
ChipLogValueExchange(ec));
}

CHIP_ERROR InteractionModelEngine::SendReadRequest(ReadPrepareParams & aReadPrepareParams, ReadClient::Callback * aCallback)
uint16_t InteractionModelEngine::GetWriteClientArrayIndex(const WriteClient * const apWriteClient) const
{
ReadClient * client = nullptr;
CHIP_ERROR err = CHIP_NO_ERROR;
ReturnErrorOnFailure(NewReadClient(&client, ReadClient::InteractionType::Read, aCallback));
err = client->SendRequest(aReadPrepareParams);
if (err != CHIP_NO_ERROR)
{
client->Shutdown();
}
return err;
return static_cast<uint16_t>(apWriteClient - mWriteClients);
}

CHIP_ERROR InteractionModelEngine::SendSubscribeRequest(ReadPrepareParams & aReadPrepareParams, ReadClient::Callback * aCallback)
uint16_t InteractionModelEngine::GetReadHandlerArrayIndex(const ReadHandler * const apReadHandler) const
{
ReadClient * client = nullptr;
ReturnErrorOnFailure(NewReadClient(&client, ReadClient::InteractionType::Subscribe, aCallback));
ReturnErrorOnFailure(client->SendRequest(aReadPrepareParams));
return CHIP_NO_ERROR;
return static_cast<uint16_t>(apReadHandler - mReadHandlers);
}

uint16_t InteractionModelEngine::GetReadClientArrayIndex(const ReadClient * const apReadClient) const
void InteractionModelEngine::AddReadClient(ReadClient * apReadClient)
{
return static_cast<uint16_t>(apReadClient - mReadClients);
apReadClient->SetNextClient(mpActiveReadClientList);
mpActiveReadClientList = apReadClient;
}

uint16_t InteractionModelEngine::GetWriteClientArrayIndex(const WriteClient * const apWriteClient) const
void InteractionModelEngine::RemoveReadClient(ReadClient * apReadClient)
{
return static_cast<uint16_t>(apWriteClient - mWriteClients);
ReadClient * pPrevListItem = nullptr;
ReadClient * pCurListItem = mpActiveReadClientList;

while (pCurListItem != apReadClient)
{
pPrevListItem = pCurListItem;
pCurListItem = pCurListItem->GetNextClient();
}

//
// Item must exist in this tracker list. If not, there's a bug somewhere.
//
VerifyOrDie(pCurListItem != nullptr);
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved

if (pPrevListItem)
{
pPrevListItem->SetNextClient(apReadClient->GetNextClient());
}
else
{
mpActiveReadClientList = apReadClient->GetNextClient();
}

apReadClient->SetNextClient(nullptr);
}

uint16_t InteractionModelEngine::GetReadHandlerArrayIndex(const ReadHandler * const apReadHandler) const
size_t InteractionModelEngine::GetNumActiveReadClients()
{
return static_cast<uint16_t>(apReadHandler - mReadHandlers);
ReadClient * pListItem = mpActiveReadClientList;
size_t count = 0;

while (pListItem)
{
pListItem = pListItem->GetNextClient();
count++;
}

return count;
}

bool InteractionModelEngine::InActiveReadClientList(ReadClient * apReadClient)
{
ReadClient * pListItem = mpActiveReadClientList;

while (pListItem)
{
if (pListItem == apReadClient)
{
return true;
}

pListItem = pListItem->GetNextClient();
}

return false;
}

void InteractionModelEngine::ReleaseClusterInfoList(ClusterInfo *& aClusterInfo)
Expand Down
69 changes: 23 additions & 46 deletions src/app/InteractionModelEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,24 +95,6 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman

Messaging::ExchangeManager * GetExchangeManager(void) const { return mpExchangeMgr; };

/**
* Creates a new read client and send ReadRequest message to the node using the read client,
* shutdown if fail to send it out
*
* @retval #CHIP_ERROR_NO_MEMORY If there is no ReadClient available
* @retval #CHIP_NO_ERROR On success.
*/
CHIP_ERROR SendReadRequest(ReadPrepareParams & aReadPrepareParams, ReadClient::Callback * aCallback);

/**
* Creates a new read client and sends SubscribeRequest message to the node using the read client.
* Shuts down on transmission failure.
*
* @retval #CHIP_ERROR_NO_MEMORY If there is no ReadClient available
* @retval #CHIP_NO_ERROR On success.
*/
CHIP_ERROR SendSubscribeRequest(ReadPrepareParams & aReadPrepareParams, ReadClient::Callback * aCallback);

/**
* Tears down an active subscription.
*
Expand Down Expand Up @@ -145,38 +127,11 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
CHIP_ERROR NewWriteClient(WriteClientHandle & apWriteClient, WriteClient::Callback * callback,
const Optional<uint16_t> & aTimedWriteTimeoutMs = NullOptional);

/**
* Allocate a ReadClient that can be used to do a read interaction. If the call succeeds, the consumer
* is responsible for calling Shutdown() on the ReadClient once it's done using it.
*
* @param[in,out] apReadClient A double pointer to a ReadClient that is updated to point to a valid ReadClient
* on successful completion of this function. On failure, it will be updated to point to
* nullptr.
* @param[in] aInteractionType Type of interaction (read or subscription) that the requested ReadClient should execute.
* @param[in] aCallback If not-null, permits overriding the default delegate registered with the
* InteractionModelEngine that will be used by the ReadClient.
*
* @retval #CHIP_ERROR_INCORRECT_STATE If there is no ReadClient available
* @retval #CHIP_NO_ERROR On success.
*/
CHIP_ERROR NewReadClient(ReadClient ** const apReadClient, ReadClient::InteractionType aInteractionType,
ReadClient::Callback * aCallback);

uint32_t GetNumActiveReadHandlers() const;
uint32_t GetNumActiveReadClients() const;

uint32_t GetNumActiveWriteHandlers() const;
uint32_t GetNumActiveWriteClients() const;

/**
* Get read client index in mReadClients
*
* @param[in] apReadClient A pointer to a read client object.
*
* @retval the index in mReadClients array
*/
uint16_t GetReadClientArrayIndex(const ReadClient * const apReadClient) const;

uint16_t GetWriteClientArrayIndex(const WriteClient * const apWriteClient) const;

uint16_t GetReadHandlerArrayIndex(const ReadHandler * const apReadHandler) const;
Expand Down Expand Up @@ -220,6 +175,27 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
void OnTimedWrite(TimedHandler * apTimedHandler, Messaging::ExchangeContext * apExchangeContext,
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);

/**
* Add a read client to the internally tracked list of weak references. This list is used to
* correctly dispatch unsolicited reports to the right matching handler by subscription ID.
*/
void AddReadClient(ReadClient * apReadClient);

/**
* Remove a read client from the internally tracked list of weak references.
*/
void RemoveReadClient(ReadClient * apReadClient);

/**
* Test to see if a read client is in the actively tracked list.
*/
bool InActiveReadClientList(ReadClient * apReadClient);

/**
* Return the number of active read clients being tracked by the engine.
*/
size_t GetNumActiveReadClients();

private:
friend class reporting::Engine;
friend class TestCommandInteraction;
Expand Down Expand Up @@ -287,14 +263,15 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate, public Comman
// TODO(#8006): investgate if we can provide more flexible object management on devices with more resources.
BitMapObjectPool<CommandHandler, CHIP_IM_MAX_NUM_COMMAND_HANDLER> mCommandHandlerObjs;
BitMapObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER> mTimedHandlers;
ReadClient mReadClients[CHIP_IM_MAX_NUM_READ_CLIENT];
ReadHandler mReadHandlers[CHIP_IM_MAX_NUM_READ_HANDLER];
WriteClient mWriteClients[CHIP_IM_MAX_NUM_WRITE_CLIENT];
WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER];
reporting::Engine mReportingEngine;
ClusterInfo mClusterInfoPool[CHIP_IM_SERVER_MAX_NUM_PATH_GROUPS];
ClusterInfo * mpNextAvailableClusterInfo = nullptr;

ReadClient * mpActiveReadClientList = nullptr;

// A magic number for tracking values between stack Shutdown()-s and Init()-s.
// An ObjectHandle is valid iff. its magic equals to this one.
uint32_t mMagic = 0;
Expand Down
Loading