Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@yunhanw-google
yunhanw-google committed Feb 24, 2025
1 parent 577651c commit 71eb65f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 49 deletions.
76 changes: 36 additions & 40 deletions src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ void ReadClient::Close(CHIP_ERROR aError, bool allowResubscription)
aError = mpCallback.OnResubscriptionNeeded(this, aError);
if (aError == CHIP_NO_ERROR)
{
if (IsPeerLIT() && (originalReason == CHIP_ERROR_TIMEOUT))
{
ChipLogProgress(DataManagement,
"ICD device is unreachable, mark subscription as FailedICDSubscription, retrying has been scheduled");
MoveToState(ClientState::FailedICDSubscription);
}
return;
}
if (aError == CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT)
Expand All @@ -217,13 +223,6 @@ void ReadClient::Close(CHIP_ERROR aError, bool allowResubscription)
MoveToState(ClientState::InactiveICDSubscription);
return;
}
if (aError == CHIP_ERROR_LIT_CASE_SUBSCRIBE_PRIMING_TIMEOUT)
{
ChipLogProgress(DataManagement,
"ICD device is unreachable, mark subscription as FailedICDSubscription, retrying is scheduled");
MoveToState(ClientState::FailedICDSubscription);
return;
}
}

//
Expand Down Expand Up @@ -494,15 +493,25 @@ CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Build
void ReadClient::OnActiveModeNotification()
{
VerifyOrDie(mpImEngine->InActiveReadClientList(this));
// If the current state is InactiveICDSubscription (LIT ICD unreachable) or FailedICDSubscription (session/subscription creation
// failed), the previous close call should have executed ClearActiveSubscriptionState. Upon receiving a check-in, cancel pending
// resubscription if any and initiate a new one immediately. Case sessions need to be invalidated.
VerifyOrReturn(IsInactiveICDSubscription() && IsFailedICDSubscription());
// If the current state is InactiveICDSubscription (the subscription definitely exceeded the liveness timeout for LIT ICD), it means subscription retry is on-hold, once OnActiveModeNotification
// is triggered, and the deferred resubscription logic in `OnLivenessTimeoutCallback` would be executed immediately.
if (IsInactiveICDSubscription())
{.
TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT);
return;
}

CloseSession();
CancelResubscribeTimer();
MoveToState(ClientState::Idle);
OnResubscribeTimerCallback(nullptr, this);
// If the current state is FailedICDSubscription (various timeouts happens), the subscription retrying continues to run,
// once OnActiveModeNotification is triggered, scheduled resubscription would be cancelled, the subscription would be
// rescheduled immediately.
if (IsFailedICDSubscription())
{
mNumRetries = 0;
CancelResubscribeTimer();
TriggerResubscriptionForLivenessTimeout(CHIP_ERROR_TIMEOUT);
return;
}
return;
}

void ReadClient::OnPeerOperatingModeChange(PeerOperatingMode aMode)
Expand Down Expand Up @@ -728,17 +737,7 @@ void ReadClient::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContex
{
ChipLogError(DataManagement, "Time out! failed to receive report data from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(apExchangeContext));

if (IsPeerLIT() && (IsIdle() || IsAwaitingInitialReport() || IsAwaitingSubscribeResponse()))
{
ChipLogError(DataManagement, "LIT ICD device is unreachable during Subscription establishment procedure with state %s",
GetStateStr());
Close(CHIP_ERROR_LIT_CASE_SUBSCRIBE_PRIMING_TIMEOUT);
}
else
{
Close(CHIP_ERROR_TIMEOUT);
}
Close(CHIP_ERROR_TIMEOUT);
}

CHIP_ERROR ReadClient::ReadICDOperatingModeFromAttributeDataIB(TLV::TLVReader && aReader, PeerOperatingMode & aMode)
Expand Down Expand Up @@ -1054,11 +1053,10 @@ void ReadClient::OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void *
subscriptionTerminationCause = CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT;
}

_this->CloseSession();
_this->Close(subscriptionTerminationCause);
_this->TriggerResubscriptionForLivenessTimeout(subscriptionTerminationCause);
}

void ReadClient::CloseSession()
void ReadClient::TriggerResubscriptionForLivenessTimeout(CHIP_ERROR aReason)
{
// We didn't get a message from the server on time; it's possible that it no
// longer has a useful CASE session to us. Mark defunct all sessions that
Expand All @@ -1081,6 +1079,8 @@ void ReadClient::CloseSession()
session->MarkAsDefunct();
});
}

Close(aReason);
}

CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aPayload)
Expand Down Expand Up @@ -1289,10 +1289,10 @@ CHIP_ERROR ReadClient::DefaultResubscribePolicy(CHIP_ERROR aTerminationCause)
ChipLogProgress(DataManagement, "ICD device is inactive, skipping scheduling resubscribe within DefaultResubscribePolicy");
return CHIP_ERROR_LIT_SUBSCRIBE_INACTIVE_TIMEOUT;
}

VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE);

auto timeTillNextResubscription = ComputeTimeTillNextSubscription();

ChipLogProgress(DataManagement,
"Will try to resubscribe to %02x:" ChipLogFormatX64 " at retry index %" PRIu32 " after %" PRIu32
"ms due to error %" CHIP_ERROR_FORMAT,
Expand Down Expand Up @@ -1324,8 +1324,9 @@ void ReadClient::HandleDeviceConnectionFailure(void * context, const Operational
{
ReadClient * const _this = static_cast<ReadClient *>(context);
VerifyOrDie(_this != nullptr);
CHIP_ERROR err = failureInfo.error;
ChipLogError(DataManagement, "Failed to establish CASE for re-subscription with error '%" CHIP_ERROR_FORMAT "'", err.Format());

ChipLogError(DataManagement, "Failed to establish CASE for re-subscription with error '%" CHIP_ERROR_FORMAT "'",
failureInfo.error.Format());

#if CHIP_CONFIG_ENABLE_BUSY_HANDLING_FOR_OPERATIONAL_SESSION_SETUP
#if CHIP_DETAIL_LOGGING
Expand All @@ -1339,14 +1340,8 @@ void ReadClient::HandleDeviceConnectionFailure(void * context, const Operational
#else
_this->mMinimalResubscribeDelay = System::Clock::kZero;
#endif // CHIP_CONFIG_ENABLE_BUSY_HANDLING_FOR_OPERATIONAL_SESSION_SETUP

if (_this->IsPeerLIT() && err == CHIP_ERROR_TIMEOUT && _this->IsIdle())
{
ChipLogError(DataManagement, "LIT ICD device is unreachable during CASE establishment procedure");
err = CHIP_ERROR_LIT_CASE_SUBSCRIBE_PRIMING_TIMEOUT;
}

_this->Close(err);

_this->Close(failureInfo.error);
}

void ReadClient::OnResubscribeTimerCallback(System::Layer * /* If this starts being used, fix callers that pass nullptr */,
Expand All @@ -1358,6 +1353,7 @@ void ReadClient::OnResubscribeTimerCallback(System::Layer * /* If this starts be
_this->mIsResubscriptionScheduled = false;

CHIP_ERROR err;

ChipLogProgress(DataManagement, "OnResubscribeTimerCallback: ForceCASE = %d", _this->mForceCaseOnNextResub);
_this->mNumRetries++;

Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ class ReadClient : public Messaging::ExchangeDelegate
CHIP_ERROR ComputeLivenessCheckTimerTimeout(System::Clock::Timeout * aTimeout);
void CancelLivenessCheckTimer();
void CancelResubscribeTimer();
void CloseSession();
void TriggerResubscriptionForLivenessTimeout(CHIP_ERROR aReason);
void MoveToState(const ClientState aTargetState);
CHIP_ERROR ProcessAttributePath(AttributePathIB::Parser & aAttributePath, ConcreteDataAttributePath & aClusterInfo);
CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload, ReportType aReportType);
Expand Down
9 changes: 1 addition & 8 deletions src/lib/core/CHIPError.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -837,14 +837,7 @@ using CHIP_ERROR = ::chip::ChipError;
*/
#define CHIP_ERROR_TLV_CONTAINER_OPEN CHIP_CORE_ERROR(0x27)

/**
* @def CHIP_ERROR_LIT_CASE_SUBSCRIBE_PRIMING_TIMEOUT
*
* @brief
* CASE fails to be established or subscription fails to be established
*
*/
#define CHIP_ERROR_LIT_CASE_SUBSCRIBE_PRIMING_TIMEOUT CHIP_CORE_ERROR(0x28)
// AVAILABLE: 0x28

// AVAILABLE: 0x29

Expand Down

0 comments on commit 71eb65f

Please sign in to comment.