Skip to content

Commit 55f8b88

Browse files
erjiaqingyunhanw-googlemkardous-silabs
authored
[icd] add IdleSubscription to ReadClient to support LIT ICD (project-chip#30812)
* [IM] Support put ReadClient to on hold when device is sleeping * [icd] add IdleSubscription to ReadClient to support LIT ICD * Update ReadClient.h fix typo * address comments * Update src/app/ReadClient.cpp Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/ReadClient.cpp Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/ReadClient.h Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * address comments --------- Co-authored-by: yunhanw-google <yunhanw@google.com> Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com>
1 parent 643b840 commit 55f8b88

10 files changed

+307
-20
lines changed

docs/ERROR_CODES.md

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ This file was **AUTOMATICALLY** generated by
3737
| 19 | 0x13 | `CHIP_ERROR_INTEGRITY_CHECK_FAILED` |
3838
| 20 | 0x14 | `CHIP_ERROR_INVALID_SIGNATURE` |
3939
| 21 | 0x15 | `CHIP_ERROR_INVALID_TLV_CHAR_STRING` |
40+
| 22 | 0x16 | `CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT` |
4041
| 23 | 0x17 | `CHIP_ERROR_UNSUPPORTED_SIGNATURE_TYPE` |
4142
| 24 | 0x18 | `CHIP_ERROR_INVALID_MESSAGE_LENGTH` |
4243
| 25 | 0x19 | `CHIP_ERROR_BUFFER_TOO_SMALL` |

src/app/InteractionModelEngine.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,21 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
980980
}
981981

982982
#if CHIP_CONFIG_ENABLE_READ_CLIENT
983+
void InteractionModelEngine::OnActiveModeNotification(ScopedNodeId aPeer)
984+
{
985+
for (ReadClient * pListItem = mpActiveReadClientList; pListItem != nullptr;)
986+
{
987+
auto pNextItem = pListItem->GetNextClient();
988+
// It is possible that pListItem is destroyed by the app in OnActiveModeNotification.
989+
// Get the next item before invoking `OnActiveModeNotification`.
990+
if (ScopedNodeId(pListItem->GetPeerNodeId(), pListItem->GetFabricIndex()) == aPeer)
991+
{
992+
pListItem->OnActiveModeNotification();
993+
}
994+
pListItem = pNextItem;
995+
}
996+
}
997+
983998
void InteractionModelEngine::AddReadClient(ReadClient * apReadClient)
984999
{
9851000
apReadClient->SetNextClient(mpActiveReadClientList);

src/app/InteractionModelEngine.h

+8
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,14 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
239239
const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload);
240240

241241
#if CHIP_CONFIG_ENABLE_READ_CLIENT
242+
/**
243+
* Activate the idle subscriptions.
244+
*
245+
* When subscribing to ICD and liveness timeout reached, the read client will move to `InactiveICDSubscription` state and
246+
* resubscription can be triggered via OnActiveModeNotification().
247+
*/
248+
void OnActiveModeNotification(ScopedNodeId aPeer);
249+
242250
/**
243251
* Add a read client to the internally tracked list of weak references. This list is used to
244252
* correctly dispatch unsolicited reports to the right matching handler by subscription ID.

src/app/ReadClient.cpp

+60-18
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ void ReadClient::ClearActiveSubscriptionState()
6767
mMaxInterval = 0;
6868
mSubscriptionId = 0;
6969
mIsResubscriptionScheduled = false;
70+
7071
MoveToState(ClientState::Idle);
7172
}
7273

@@ -187,11 +188,20 @@ void ReadClient::Close(CHIP_ERROR aError, bool allowResubscription)
187188
if (allowResubscription &&
188189
(mReadPrepareParams.mEventPathParamsListSize != 0 || mReadPrepareParams.mAttributePathParamsListSize != 0))
189190
{
191+
CHIP_ERROR originalReason = aError;
192+
190193
aError = mpCallback.OnResubscriptionNeeded(this, aError);
191194
if (aError == CHIP_NO_ERROR)
192195
{
193196
return;
194197
}
198+
if (aError == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT)
199+
{
200+
VerifyOrDie(originalReason == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT);
201+
ChipLogProgress(DataManagement, "ICD device is inactive mark subscription as InactiveICDSubscription");
202+
MoveToState(ClientState::InactiveICDSubscription);
203+
return;
204+
}
195205
}
196206

197207
//
@@ -223,6 +233,8 @@ const char * ReadClient::GetStateStr() const
223233
return "AwaitingSubscribeResponse";
224234
case ClientState::SubscriptionActive:
225235
return "SubscriptionActive";
236+
case ClientState::InactiveICDSubscription:
237+
return "InactiveICDSubscription";
226238
}
227239
#endif // CHIP_DETAIL_LOGGING
228240
return "N/A";
@@ -427,6 +439,18 @@ CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Build
427439
return CHIP_NO_ERROR;
428440
}
429441

442+
void ReadClient::OnActiveModeNotification()
443+
{
444+
// This function just tries to complete the deferred resubscription logic in `OnLivenessTimeoutCallback`.
445+
VerifyOrDie(mpImEngine->InActiveReadClientList(this));
446+
// If we are not in InactiveICDSubscription state, that means the liveness timeout has not been reached. Simply do nothing.
447+
VerifyOrReturn(IsInactiveICDSubscription());
448+
449+
// When we reach here, the subscription definitely exceeded the liveness timeout. Just continue the unfinished resubscription
450+
// logic in `OnLivenessTimeoutCallback`.
451+
TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT);
452+
}
453+
430454
CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
431455
System::PacketBufferHandle && aPayload)
432456
{
@@ -884,6 +908,10 @@ void ReadClient::OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void *
884908
{
885909
ReadClient * const _this = reinterpret_cast<ReadClient *>(apAppState);
886910

911+
// TODO: add a more specific error here for liveness timeout failure to distinguish between other classes of timeouts (i.e
912+
// response timeouts).
913+
CHIP_ERROR subscriptionTerminationCause = CHIP_ERROR_TIMEOUT;
914+
887915
//
888916
// Might as well try to see if this instance exists in the tracked list in the IM.
889917
// This might blow-up if either the client has since been free'ed (use-after-free), or if the engine has since
@@ -895,32 +923,39 @@ void ReadClient::OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void *
895923
"Subscription Liveness timeout with SubscriptionID = 0x%08" PRIx32 ", Peer = %02x:" ChipLogFormatX64,
896924
_this->mSubscriptionId, _this->GetFabricIndex(), ChipLogValueX64(_this->GetPeerNodeId()));
897925

926+
if (_this->mIsPeerLIT)
927+
{
928+
subscriptionTerminationCause = CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT;
929+
}
930+
931+
_this->TriggerResubscriptionForLivenessTimeout(subscriptionTerminationCause);
932+
}
933+
934+
void ReadClient::TriggerResubscriptionForLivenessTimeout(CHIP_ERROR aReason)
935+
{
898936
// We didn't get a message from the server on time; it's possible that it no
899937
// longer has a useful CASE session to us. Mark defunct all sessions that
900938
// have not seen peer activity in at least as long as our session.
901-
const auto & holder = _this->mReadPrepareParams.mSessionHolder;
939+
const auto & holder = mReadPrepareParams.mSessionHolder;
902940
if (holder)
903941
{
904942
System::Clock::Timestamp lastPeerActivity = holder->AsSecureSession()->GetLastPeerActivityTime();
905-
_this->mpImEngine->GetExchangeManager()->GetSessionManager()->ForEachMatchingSession(
906-
_this->mPeer, [&lastPeerActivity](auto * session) {
907-
if (!session->IsCASESession())
908-
{
909-
return;
910-
}
943+
mpImEngine->GetExchangeManager()->GetSessionManager()->ForEachMatchingSession(mPeer, [&lastPeerActivity](auto * session) {
944+
if (!session->IsCASESession())
945+
{
946+
return;
947+
}
911948

912-
if (session->GetLastPeerActivityTime() > lastPeerActivity)
913-
{
914-
return;
915-
}
949+
if (session->GetLastPeerActivityTime() > lastPeerActivity)
950+
{
951+
return;
952+
}
916953

917-
session->MarkAsDefunct();
918-
});
954+
session->MarkAsDefunct();
955+
});
919956
}
920957

921-
// TODO: add a more specific error here for liveness timeout failure to distinguish between other classes of timeouts (i.e
922-
// response timeouts).
923-
_this->Close(CHIP_ERROR_TIMEOUT);
958+
Close(aReason);
924959
}
925960

926961
CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aPayload)
@@ -999,6 +1034,8 @@ CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadP
9991034
mReadPrepareParams.mSessionHolder = aReadPrepareParams.mSessionHolder;
10001035
}
10011036

1037+
mIsPeerLIT = aReadPrepareParams.mIsPeerLIT;
1038+
10021039
mMinIntervalFloorSeconds = aReadPrepareParams.mMinIntervalFloorSeconds;
10031040

10041041
// Todo: Remove the below, Update span in ReadPrepareParams
@@ -1103,6 +1140,12 @@ CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadP
11031140

11041141
CHIP_ERROR ReadClient::DefaultResubscribePolicy(CHIP_ERROR aTerminationCause)
11051142
{
1143+
if (aTerminationCause == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT)
1144+
{
1145+
ChipLogProgress(DataManagement, "ICD device is inactive, skipping scheduling resubscribe within DefaultResubscribePolicy");
1146+
return CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT;
1147+
}
1148+
11061149
VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE);
11071150

11081151
auto timeTillNextResubscription = ComputeTimeTillNextSubscription();
@@ -1111,8 +1154,7 @@ CHIP_ERROR ReadClient::DefaultResubscribePolicy(CHIP_ERROR aTerminationCause)
11111154
"ms due to error %" CHIP_ERROR_FORMAT,
11121155
GetFabricIndex(), ChipLogValueX64(GetPeerNodeId()), mNumRetries, timeTillNextResubscription,
11131156
aTerminationCause.Format());
1114-
ReturnErrorOnFailure(ScheduleResubscription(timeTillNextResubscription, NullOptional, aTerminationCause == CHIP_ERROR_TIMEOUT));
1115-
return CHIP_NO_ERROR;
1157+
return ScheduleResubscription(timeTillNextResubscription, NullOptional, aTerminationCause == CHIP_ERROR_TIMEOUT);
11161158
}
11171159

11181160
void ReadClient::HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr,

src/app/ReadClient.h

+23
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ class ReadClient : public Messaging::ExchangeDelegate
165165
* ReadClient::DefaultResubscribePolicy is broken down into its constituent methods that are publicly available for
166166
* applications to call and sequence.
167167
*
168+
* If the peer is LIT ICD, and the timeout is reached, `aTerminationCause` will be
169+
* `CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT`. In this case, returning `CHIP_NO_ERROR` will still trigger a resubscribe
170+
* attempt, while returning `CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT` will put the subscription in the
171+
* `InactiveICDSubscription` state. In the latter case, OnResubscriptionNeeded will be called again when
172+
* `OnActiveModeNotification` is called.
173+
*
168174
* If the method is over-ridden, it's the application's responsibility to take the appropriate steps needed to eventually
169175
* call-back into the ReadClient object to schedule a re-subscription (by invoking ReadClient::ScheduleResubscription).
170176
*
@@ -332,6 +338,18 @@ class ReadClient : public Messaging::ExchangeDelegate
332338
*/
333339
CHIP_ERROR SendRequest(ReadPrepareParams & aReadPrepareParams);
334340

341+
/**
342+
* Re-activate an inactive subscription.
343+
*
344+
* When subscribing to LIT-ICD and liveness timeout reached and OnResubscriptionNeeded returns
345+
* CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT, the read client will move to the InactiveICDSubscription state and
346+
* resubscription can be triggered via OnActiveModeNotification().
347+
*
348+
* If the subscription is not in the `InactiveICDSubscription` state, this function will do nothing. So it is always safe to
349+
* call this function when a check-in message is received.
350+
*/
351+
void OnActiveModeNotification();
352+
335353
void OnUnsolicitedReportData(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload);
336354

337355
void OnUnsolicitedMessageFromPublisher()
@@ -486,6 +504,7 @@ class ReadClient : public Messaging::ExchangeDelegate
486504
AwaitingInitialReport, ///< The client is waiting for initial report
487505
AwaitingSubscribeResponse, ///< The client is waiting for subscribe response
488506
SubscriptionActive, ///< The client is maintaining subscription
507+
InactiveICDSubscription, ///< The client is waiting to resubscribe for LIT device
489508
};
490509

491510
enum class ReportType
@@ -510,6 +529,7 @@ class ReadClient : public Messaging::ExchangeDelegate
510529
*
511530
*/
512531
bool IsIdle() const { return mState == ClientState::Idle; }
532+
bool IsInactiveICDSubscription() const { return mState == ClientState::InactiveICDSubscription; }
513533
bool IsSubscriptionActive() const { return mState == ClientState::SubscriptionActive; }
514534
bool IsAwaitingInitialReport() const { return mState == ClientState::AwaitingInitialReport; }
515535
bool IsAwaitingSubscribeResponse() const { return mState == ClientState::AwaitingSubscribeResponse; }
@@ -533,6 +553,7 @@ class ReadClient : public Messaging::ExchangeDelegate
533553
CHIP_ERROR ComputeLivenessCheckTimerTimeout(System::Clock::Timeout * aTimeout);
534554
void CancelLivenessCheckTimer();
535555
void CancelResubscribeTimer();
556+
void TriggerResubscriptionForLivenessTimeout(CHIP_ERROR aReason);
536557
void MoveToState(const ClientState aTargetState);
537558
CHIP_ERROR ProcessAttributePath(AttributePathIB::Parser & aAttributePath, ConcreteDataAttributePath & aClusterInfo);
538559
CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload, ReportType aReportType);
@@ -621,6 +642,8 @@ class ReadClient : public Messaging::ExchangeDelegate
621642

622643
System::Clock::Timeout mLivenessTimeoutOverride = System::Clock::kZero;
623644

645+
bool mIsPeerLIT = false;
646+
624647
// End Of Container (0x18) uses one byte.
625648
static constexpr uint16_t kReservedSizeForEndOfContainer = 1;
626649
// Reserved size for the uint8_t InteractionModelRevision flag, which takes up 1 byte for the control tag and 1 byte for the

src/app/ReadPrepareParams.h

+3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ struct ReadPrepareParams
4747
uint16_t mMaxIntervalCeilingSeconds = 0;
4848
bool mKeepSubscriptions = false;
4949
bool mIsFabricFiltered = true;
50+
bool mIsPeerLIT = false;
5051

5152
ReadPrepareParams() {}
5253
ReadPrepareParams(const SessionHandle & sessionHandle) { mSessionHolder.Grab(sessionHandle); }
@@ -64,6 +65,7 @@ struct ReadPrepareParams
6465
mMaxIntervalCeilingSeconds = other.mMaxIntervalCeilingSeconds;
6566
mTimeout = other.mTimeout;
6667
mIsFabricFiltered = other.mIsFabricFiltered;
68+
mIsPeerLIT = other.mIsPeerLIT;
6769
other.mpEventPathParamsList = nullptr;
6870
other.mEventPathParamsListSize = 0;
6971
other.mpAttributePathParamsList = nullptr;
@@ -88,6 +90,7 @@ struct ReadPrepareParams
8890
mMaxIntervalCeilingSeconds = other.mMaxIntervalCeilingSeconds;
8991
mTimeout = other.mTimeout;
9092
mIsFabricFiltered = other.mIsFabricFiltered;
93+
mIsPeerLIT = other.mIsPeerLIT;
9194
other.mpEventPathParamsList = nullptr;
9295
other.mEventPathParamsListSize = 0;
9396
other.mpAttributePathParamsList = nullptr;

src/app/icd/client/DefaultCheckInDelegate.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
#include "CheckInHandler.h"
19+
#include <app/InteractionModelEngine.h>
1920
#include <app/icd/client/DefaultCheckInDelegate.h>
2021
#include <crypto/CHIPCryptoPAL.h>
2122
#include <lib/support/CodeUtils.h>
@@ -38,6 +39,9 @@ void DefaultCheckInDelegate::OnCheckInComplete(const ICDClientInfo & clientInfo)
3839
ChipLogProgress(
3940
ICD, "Check In Message processing complete: start_counter=%" PRIu32 " offset=%" PRIu32 " nodeid=" ChipLogFormatScopedNodeId,
4041
clientInfo.start_icd_counter, clientInfo.offset, ChipLogValueScopedNodeId(clientInfo.peer_node));
42+
#if CHIP_CONFIG_ENABLE_READ_CLIENT
43+
InteractionModelEngine::GetInstance()->OnActiveModeNotification(clientInfo.peer_node);
44+
#endif
4145
}
4246

4347
} // namespace app

src/controller/tests/data_model/BUILD.gn

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ chip_test_suite_using_nltest("data_model") {
2525
if (chip_device_platform != "mbed" && chip_device_platform != "efr32" &&
2626
chip_device_platform != "esp32" && chip_device_platform != "fake") {
2727
test_sources = [ "TestCommands.cpp" ]
28-
test_sources += [ "TestRead.cpp" ]
2928
test_sources += [ "TestWrite.cpp" ]
29+
test_sources += [ "TestRead.cpp" ]
3030
}
3131

3232
public_deps = [

0 commit comments

Comments
 (0)