Skip to content

Commit

Permalink
[Darwin] MTRDevice should reset subscription retry backoff on trigger…
Browse files Browse the repository at this point in the history
…ing condition (#30885)
  • Loading branch information
jtung-apple authored and pull[bot] committed Jan 11, 2024
1 parent 793b089 commit 1136992
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
7 changes: 7 additions & 0 deletions src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
mClusterStateCache = std::move(aClusterStateCache);
}

// Used to reset Resubscription backoff on events that indicate likely availability of device to come back online
void ResetResubscriptionBackoff() { mResubscriptionNumRetries = 0; }

protected:
// Report an error, which may be due to issues in our own internal state or
// due to the OnError callback happening.
Expand Down Expand Up @@ -178,6 +181,10 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
bool mHaveQueuedDeletion = false;
OnDoneHandler _Nullable mOnDoneHandler = nil;
dispatch_block_t mInterimReportBlock = nil;

// Copied from ReadClient and customized for
uint32_t ComputeTimeTillNextSubscription();
uint32_t mResubscriptionNumRetries = 0;
};

NS_ASSUME_NONNULL_END
38 changes: 36 additions & 2 deletions src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#import "MTRError_Internal.h"
#import "MTRLogging_Internal.h"

#include <lib/support/FibonacciUtils.h>
#include <platform/PlatformManager.h>

using namespace chip;
Expand Down Expand Up @@ -124,16 +125,49 @@

void MTRBaseSubscriptionCallback::OnSubscriptionEstablished(SubscriptionId aSubscriptionId)
{
// ReadClient resets it at ProcessSubscribeResponse after calling OnSubscriptionEstablished, so this is equivalent
mResubscriptionNumRetries = 0;
if (mSubscriptionEstablishedHandler) {
auto subscriptionEstablishedHandler = mSubscriptionEstablishedHandler;
subscriptionEstablishedHandler();
}
}

uint32_t MTRBaseSubscriptionCallback::ComputeTimeTillNextSubscription()
{
uint32_t maxWaitTimeInMsec = 0;
uint32_t waitTimeInMsec = 0;
uint32_t minWaitTimeInMsec = 0;

if (mResubscriptionNumRetries <= CHIP_RESUBSCRIBE_MAX_FIBONACCI_STEP_INDEX) {
maxWaitTimeInMsec = GetFibonacciForIndex(mResubscriptionNumRetries) * CHIP_RESUBSCRIBE_WAIT_TIME_MULTIPLIER_MS;
} else {
maxWaitTimeInMsec = CHIP_RESUBSCRIBE_MAX_RETRY_WAIT_INTERVAL_MS;
}

if (maxWaitTimeInMsec != 0) {
minWaitTimeInMsec = (CHIP_RESUBSCRIBE_MIN_WAIT_TIME_INTERVAL_PERCENT_PER_STEP * maxWaitTimeInMsec) / 100;
waitTimeInMsec = minWaitTimeInMsec + (Crypto::GetRandU32() % (maxWaitTimeInMsec - minWaitTimeInMsec));
}

return waitTimeInMsec;
}

CHIP_ERROR MTRBaseSubscriptionCallback::OnResubscriptionNeeded(ReadClient * apReadClient, CHIP_ERROR aTerminationCause)
{
CHIP_ERROR err = ClusterStateCache::Callback::OnResubscriptionNeeded(apReadClient, aTerminationCause);
ReturnErrorOnFailure(err);
// No need to check ReadClient internal state is Idle because ReadClient only calls OnResubscriptionNeeded after calling ClearActiveSubscriptionState(), which sets the state to Idle.

// This part is copied from ReadClient's DefaultResubscribePolicy:
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,
apReadClient->GetFabricIndex(), ChipLogValueX64(apReadClient->GetPeerNodeId()), mResubscriptionNumRetries, timeTillNextResubscription,
aTerminationCause.Format());
ReturnErrorOnFailure(apReadClient->ScheduleResubscription(timeTillNextResubscription, NullOptional, aTerminationCause == CHIP_ERROR_TIMEOUT));

// Not as good a place to increment as when resubscription timer fires, but as is, this should be as good, because OnResubscriptionNeeded is only called from ReadClient's Close() while Idle, and nothing should cause this to happen
mResubscriptionNumRetries++;

if (mResubscriptionCallback != nil) {
auto callback = mResubscriptionCallback;
Expand Down
6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ @interface MTRDevice ()
* an OnDone for that ReadClient.
*/
@property (nonatomic) ReadClient * currentReadClient;
@property (nonatomic) SubscriptionCallback * currentSubscriptionCallback; // valid when and only when currentReadClient is valid

@end

Expand Down Expand Up @@ -299,6 +300,7 @@ - (void)nodeMayBeAdvertisingOperational
// nodeMayBeAdvertisingOperational is called we are running on the Matter
// queue, and the ReadClient can't get destroyed while we are on that queue.
ReadClient * readClientToResubscribe = nullptr;
SubscriptionCallback * subscriptionCallback = nullptr;

os_unfair_lock_lock(&self->_lock);

Expand All @@ -310,10 +312,12 @@ - (void)nodeMayBeAdvertisingOperational
[self _reattemptSubscriptionNowIfNeeded];
} else {
readClientToResubscribe = self->_currentReadClient;
subscriptionCallback = self->_currentSubscriptionCallback;
}
os_unfair_lock_unlock(&self->_lock);

if (readClientToResubscribe) {
subscriptionCallback->ResetResubscriptionBackoff();
readClientToResubscribe->TriggerResubscribeIfScheduled("operational advertisement seen");
}
}
Expand Down Expand Up @@ -741,6 +745,7 @@ - (void)_setupSubscription
// holding a dangling pointer.
os_unfair_lock_lock(&self->_lock);
self->_currentReadClient = nullptr;
self->_currentSubscriptionCallback = nullptr;
os_unfair_lock_unlock(&self->_lock);
dispatch_async(self.queue, ^{
// OnDone
Expand Down Expand Up @@ -793,6 +798,7 @@ - (void)_setupSubscription
// when OnDone is called.
os_unfair_lock_lock(&self->_lock);
self->_currentReadClient = readClient.get();
self->_currentSubscriptionCallback = callback.get();
os_unfair_lock_unlock(&self->_lock);
callback->AdoptReadClient(std::move(readClient));
callback->AdoptClusterStateCache(std::move(clusterStateCache));
Expand Down

0 comments on commit 1136992

Please sign in to comment.