Skip to content

Commit

Permalink
[ICD] Add OnPeerTypeChange for dynamic ICD (#31340)
Browse files Browse the repository at this point in the history 
* [icd] add OnPeerTypeChange for dynamic ICD

* Update

* add ReadICDOperationModeFromAttributeDataIB

* Update ReadClient.cpp

Fix typo nolonger

* Add tests

* Update src/app/InteractionModelEngine.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/app/InteractionModelEngine.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/app/InteractionModelEngine.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/app/ReadClient.cpp

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/app/ReadClient.h

Co-authored-by: Boris Zbarsky <[email protected]>

---------

Co-authored-by: yunhanw-google <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@erjiaqing @yunhanw-google
@bzbarsky-apple @pull
3 people authored and pull[bot] committed Mar 25, 2024
1 parent 99afe80 commit 2497617
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 1 deletion.
16 changes: 16 additions & 0 deletions src/app/InteractionModelEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,22 @@ void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer)
}
}

void InteractionModelEngine::OnPeerTypeChange(ScopedNodeId aPeer, ReadClient::PeerType aType)
{
// TODO: Follow up to use a iterator function to avoid copy/paste here.
for (ReadClient * pListItem = mpActiveReadClientList; pListItem != nullptr;)
{
// It is possible that pListItem is destroyed by the app in OnPeerTypeChange.
// Get the next item before invoking `OnPeerTypeChange`.
auto pNextItem = pListItem->GetNextClient();
if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer)
{
pListItem->OnPeerTypeChange(aType);
}
pListItem = pNextItem;
}
}

void InteractionModelEngine::AddReadClient(ReadClient * apReadClient)
{
apReadClient->SetNextClient(mpActiveReadClientList);
Expand Down
8 changes: 8 additions & 0 deletions src/app/InteractionModelEngine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,14 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
*/
void OnActiveModeNotification(ScopedNodeId aPeer);

/**
* Used to notify when a peer becomes LIT ICD or vice versa.
*
* ReadClient will call this function when it finds any updates of the OperatingMode attribute from ICD management
* cluster. The application doesn't need to call this function, usually.
*/
void OnPeerTypeChange(ScopedNodeId aPeer, ReadClient::PeerType aType);

/**
* 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.
Expand Down
64 changes: 63 additions & 1 deletion src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
#include <messaging/ReliableMessageProtocolConfig.h>
#include <platform/LockTracker.h>

#include <app-common/zap-generated/cluster-objects.h>
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>

namespace chip {
namespace app {

Expand Down Expand Up @@ -451,12 +455,27 @@ void ReadClient::OnActiveModeNotification()
TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT);
}

void ReadClient::OnPeerTypeChange(PeerType aType)
{
VerifyOrDie(mpImEngine->InActiveReadClientList(this));

mIsPeerLIT = (aType == PeerType::kLITICD);

ChipLogProgress(DataManagement, "Peer is now %s LIT ICD.", mIsPeerLIT ? "a" : "not a");

// If the peer is no longer LIT, try to wake up the subscription and do resubscribe when necessary.
if (!mIsPeerLIT)
{
OnActiveModeNotification();
}
}

CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Status status = Status::InvalidAction;
VerifyOrExit(!IsIdle(), err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(!IsIdle() && !IsInactiveICDSubscription(), err = CHIP_ERROR_INCORRECT_STATE);

if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReportData))
{
Expand Down Expand Up @@ -663,6 +682,29 @@ void ReadClient::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContex
Close(CHIP_ERROR_TIMEOUT);
}

CHIP_ERROR ReadClient::ReadICDOperatingModeFromAttributeDataIB(TLV::TLVReader && aReader, PeerType & aType)
{
Clusters::IcdManagement::Attributes::OperatingMode::TypeInfo::DecodableType operatingMode;

CHIP_ERROR err = DataModel::Decode(aReader, operatingMode);
ReturnErrorOnFailure(err);

switch (operatingMode)
{
case Clusters::IcdManagement::OperatingModeEnum::kSit:
aType = PeerType::kNormal;
break;
case Clusters::IcdManagement::OperatingModeEnum::kLit:
aType = PeerType::kLITICD;
break;
default:
err = CHIP_ERROR_INVALID_ARGUMENT;
break;
}

return err;
}

CHIP_ERROR ReadClient::ProcessAttributePath(AttributePathIB::Parser & aAttributePathParser,
ConcreteDataAttributePath & aAttributePath)
{
Expand Down Expand Up @@ -757,6 +799,26 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo
attributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
}

if (attributePath ==
ConcreteDataAttributePath(kRootEndpointId, Clusters::IcdManagement::Id,
Clusters::IcdManagement::Attributes::OperatingMode::Id))
{
PeerType peerType;
TLV::TLVReader operatingModeTlvReader;
operatingModeTlvReader.Init(dataReader);
if (CHIP_NO_ERROR == ReadICDOperatingModeFromAttributeDataIB(std::move(operatingModeTlvReader), peerType))
{
// It is safe to call `OnPeerTypeChange` since we are in the middle of parsing the attribute data, And
// the subscription should be active so `OnActiveModeNotification` is a no-op in this case.
InteractionModelEngine::GetInstance()->OnPeerTypeChange(mPeer, peerType);
}
else
{
ChipLogError(DataManagement, "Failed to get ICD state from attribute data with error'%" CHIP_ERROR_FORMAT "'",
err.Format());
}
}

NoteReportingData();
mpCallback.OnAttributeData(attributePath, &dataReader, statusIB);
}
Expand Down
16 changes: 16 additions & 0 deletions src/app/ReadClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,12 @@ class ReadClient : public Messaging::ExchangeDelegate
Subscribe,
};

enum class PeerType : uint8_t
{
kNormal,
kLITICD,
};

/**
*
* Constructor.
Expand Down Expand Up @@ -524,6 +530,15 @@ class ReadClient : public Messaging::ExchangeDelegate
System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;

/**
* Updates the type (LIT ICD or not) of the peer.
*
* When the subscription is active, this function will just set the flag. When the subscription is an InactiveICDSubscription,
* setting the peer type to SIT or normal devices will also trigger a resubscription attempt.
*
*/
void OnPeerTypeChange(PeerType aType);

/**
* Check if current read client is being used
*
Expand All @@ -544,6 +559,7 @@ class ReadClient : public Messaging::ExchangeDelegate
CHIP_ERROR BuildDataVersionFilterList(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder,
const Span<AttributePathParams> & aAttributePaths,
const Span<DataVersionFilter> & aDataVersionFilters, bool & aEncodedDataVersionList);
CHIP_ERROR ReadICDOperatingModeFromAttributeDataIB(TLV::TLVReader && aReader, PeerType & aType);
CHIP_ERROR ProcessAttributeReportIBs(TLV::TLVReader & aAttributeDataIBsReader);
CHIP_ERROR ProcessEventReportIBs(TLV::TLVReader & aEventReportIBsReader);

Expand Down
132 changes: 132 additions & 0 deletions src/controller/tests/data_model/TestRead.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ ResponseDirective responseDirective;
// Every read will increment this count by 1 and return the new value.
uint16_t totalReadCount = 0;

bool isLitIcd = false;

} // namespace

namespace chip {
Expand Down Expand Up @@ -169,6 +171,17 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
return err;
}
}
if (aPath.mClusterId == app::Clusters::IcdManagement::Id &&
aPath.mAttributeId == app::Clusters::IcdManagement::Attributes::OperatingMode::Id)
{
AttributeValueEncoder::AttributeEncodeState state =
(apEncoderState == nullptr ? AttributeValueEncoder::AttributeEncodeState() : *apEncoderState);
AttributeValueEncoder valueEncoder(aAttributeReports, aSubjectDescriptor.fabricIndex, aPath,
kDataVersion /* data version */, aIsFabricFiltered, state);

return valueEncoder.Encode(isLitIcd ? Clusters::IcdManagement::OperatingModeEnum::kLit
: Clusters::IcdManagement::OperatingModeEnum::kSit);
}

AttributeReportIB::Builder & attributeReport = aAttributeReports.CreateAttributeReport();
ReturnErrorOnFailure(aAttributeReports.GetError());
Expand Down Expand Up @@ -298,6 +311,7 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback
static void TestReadHandler_KeepSubscriptionTest(nlTestSuite * apSuite, void * apContext);
static void TestSubscribe_OnActiveModeNotification(nlTestSuite * apSuite, void * apContext);
static void TestSubscribe_ImmediatelyResubscriptionForLIT(nlTestSuite * apSuite, void * apContext);
static void TestSubscribe_DynamicLITSubscription(nlTestSuite * apSuite, void * apContext);

private:
static uint16_t mMaxInterval;
Expand Down Expand Up @@ -2722,6 +2736,123 @@ void TestReadInteraction::TestSubscribe_OnActiveModeNotification(nlTestSuite * a
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
}

/**
* When the liveness timeout of a subscription to ICD is reached, the subscription will enter "InactiveICDSubscription" state, the
* client should call "OnActiveModeNotification" to re-activate it again when the check-in message is received from the ICD.
*/
void TestReadInteraction::TestSubscribe_DynamicLITSubscription(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();

ctx.SetMRPMode(chip::Test::MessagingContext::MRPMode::kResponsive);

{
TestResubscriptionCallback callback;
app::ReadClient readClient(app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), callback,
app::ReadClient::InteractionType::Subscribe);

responseDirective = kSendDataResponse;
callback.mScheduleLITResubscribeImmediately = false;
callback.SetReadClient(&readClient);
isLitIcd = false;

app::ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());

// Read full wildcard paths, repeat twice to ensure chunking.
app::AttributePathParams attributePathParams[1];
readPrepareParams.mpAttributePathParamsList = attributePathParams;
readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams);
attributePathParams[0].mEndpointId = kRootEndpointId;
attributePathParams[0].mClusterId = app::Clusters::IcdManagement::Id;
attributePathParams[0].mAttributeId = app::Clusters::IcdManagement::Attributes::OperatingMode::Id;

constexpr uint16_t maxIntervalCeilingSeconds = 1;

readPrepareParams.mMaxIntervalCeilingSeconds = maxIntervalCeilingSeconds;
readPrepareParams.mIsPeerLIT = true;

auto err = readClient.SendAutoResubscribeRequest(std::move(readPrepareParams));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

//
// Drive servicing IO till we have established a subscription.
//
ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2000),
[&]() { return callback.mOnSubscriptionEstablishedCount >= 1; });
NL_TEST_ASSERT(apSuite, callback.mOnSubscriptionEstablishedCount == 1);
NL_TEST_ASSERT(apSuite, callback.mOnError == 0);
NL_TEST_ASSERT(apSuite, callback.mOnResubscriptionsAttempted == 0);
chip::app::ReadHandler * readHandler = app::InteractionModelEngine::GetInstance()->ActiveHandlerAt(0);

uint16_t minInterval;
uint16_t maxInterval;
readHandler->GetReportingIntervals(minInterval, maxInterval);

// Part 1. LIT -> SIT

//
// Disable packet transmission, and drive IO till timeout.
// We won't actually request resubscription, since the device is not active, the resubscription will be deferred until
// WakeUp() is called.
//
// Even if we set the peer type to LIT before, the report indicates that the peer is a SIT now, it will just bahve as
// normal, non-LIT subscriptions.
ctx.GetLoopback().mNumMessagesToDrop = chip::Test::LoopbackTransport::kUnlimitedMessageCount;
ctx.GetIOContext().DriveIOUntil(ComputeSubscriptionTimeout(System::Clock::Seconds16(maxInterval)),
[&]() { return callback.mOnResubscriptionsAttempted != 0; });
NL_TEST_ASSERT(apSuite, callback.mOnResubscriptionsAttempted == 1);
NL_TEST_ASSERT(apSuite, callback.mLastError == CHIP_ERROR_TIMEOUT);

ctx.GetLoopback().mNumMessagesToDrop = 0;
callback.ClearCounters();

//
// Drive servicing IO till we have established a subscription.
//
ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2000),
[&]() { return callback.mOnSubscriptionEstablishedCount == 1; });
NL_TEST_ASSERT(apSuite, callback.mOnSubscriptionEstablishedCount == 1);

//
// With re-sub enabled, we shouldn't have encountered any errors
//
NL_TEST_ASSERT(apSuite, callback.mOnError == 0);
NL_TEST_ASSERT(apSuite, callback.mOnDone == 0);

// Part 2. SIT -> LIT

isLitIcd = true;
{
app::AttributePathParams path;
path.mEndpointId = kRootEndpointId;
path.mClusterId = Clusters::IcdManagement::Id;
path.mAttributeId = Clusters::IcdManagement::Attributes::OperatingMode::Id;
app::InteractionModelEngine::GetInstance()->GetReportingEngine().SetDirty(path);
}
callback.ClearCounters();
ctx.GetIOContext().DriveIOUntil(System::Clock::Seconds16(60), [&]() {
return app::InteractionModelEngine::GetInstance()->GetNumDirtySubscriptions() == 0;
});

// When we received the update that OperatingMode becomes LIT, we automatically set the inner peer type to LIT ICD.
ctx.GetLoopback().mNumMessagesToDrop = chip::Test::LoopbackTransport::kUnlimitedMessageCount;
ctx.GetIOContext().DriveIOUntil(ComputeSubscriptionTimeout(System::Clock::Seconds16(maxInterval)), [&]() { return false; });
NL_TEST_ASSERT(apSuite, callback.mOnResubscriptionsAttempted == 1);
NL_TEST_ASSERT(apSuite, callback.mLastError == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT);

ctx.GetLoopback().mNumMessagesToDrop = 0;
callback.ClearCounters();
}

ctx.SetMRPMode(chip::Test::MessagingContext::MRPMode::kDefault);

app::InteractionModelEngine::GetInstance()->ShutdownActiveReads();
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);

isLitIcd = false;
}

/**
* When the liveness timeout of a subscription to ICD is reached, the app can issue resubscription immediately
* if they know the peer is active.
Expand Down Expand Up @@ -4920,6 +5051,7 @@ const nlTest sTests[] =
NL_TEST_DEF("TestReadHandler_KeepSubscriptionTest", TestReadInteraction::TestReadHandler_KeepSubscriptionTest),
NL_TEST_DEF("TestSubscribe_OnActiveModeNotification", TestReadInteraction::TestSubscribe_OnActiveModeNotification),
NL_TEST_DEF("TestSubscribe_ImmediatelyResubscriptionForLIT", TestReadInteraction::TestSubscribe_ImmediatelyResubscriptionForLIT),
NL_TEST_DEF("TestSubscribe_DynamicLITSubscription", TestReadInteraction::TestSubscribe_DynamicLITSubscription),
NL_TEST_SENTINEL()
};
// clang-format on
Expand Down

0 comments on commit 2497617

Please sign in to comment.