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

Take CASE BUSY delay into account when scheduling retries in Matter.framework #32920

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Take CASE BUSY delay into account when scheduling retries in Matter.framework #32920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue

[self.deviceController getSessionForNode:self.nodeID
completion:^(ExchangeManager * _Nullable exchangeManager, const Optional<SessionHandle> & session,
NSError * _Nullable error) {
NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
if (error != nil) {
dispatch_async(queue, ^{
errorHandler(error);
Expand Down Expand Up @@ -1603,7 +1603,7 @@ - (void)subscribeToAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullabl
[self.deviceController
getSessionForNode:self.nodeID
completion:^(ExchangeManager * _Nullable exchangeManager, const Optional<SessionHandle> & session,
NSError * _Nullable error) {
NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
if (error != nil) {
dispatch_async(queue, ^{
reportHandler(nil, error);
Expand Down
6 changes: 3 additions & 3 deletions src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class MTRCallbackBridgeBase {

[device.deviceController getSessionForCommissioneeDevice:device.nodeID
completion:^(chip::Messaging::ExchangeManager * exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * error) {
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
MaybeDoAction(exchangeManager, session, error);
}];
}
Expand All @@ -93,8 +93,8 @@ class MTRCallbackBridgeBase {
LogRequestStart();

[controller getSessionForNode:nodeID
completion:^(chip::Messaging::ExchangeManager * exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * error) {
completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
MaybeDoAction(exchangeManager, session, error);
}];
}
Expand Down
27 changes: 20 additions & 7 deletions src/darwin/Framework/CHIP/MTRDevice.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ - (void)_handleResubscriptionNeeded
[self _changeState:MTRDeviceStateUnknown];
}

- (void)_handleSubscriptionReset
- (void)_handleSubscriptionReset:(NSNumber * _Nullable)retryDelay
{
std::lock_guard lock(_lock);
// if there is no delegate then also do not retry
Expand All @@ -790,17 +790,29 @@ - (void)_handleSubscriptionReset

self.reattemptingSubscription = YES;

NSTimeInterval secondsToWait;
if (_lastSubscriptionAttemptWait < MTRDEVICE_SUBSCRIPTION_ATTEMPT_MIN_WAIT_SECONDS) {
_lastSubscriptionAttemptWait = MTRDEVICE_SUBSCRIPTION_ATTEMPT_MIN_WAIT_SECONDS;
secondsToWait = _lastSubscriptionAttemptWait;
} else if (retryDelay != nil) {
// The device responded but is currently busy. Reset our backoff
// counter, so that we don't end up waiting for a long time if the next
// attempt fails for some reason, and retry after whatever time period
// the device told us to use.
_lastSubscriptionAttemptWait = 0;
secondsToWait = retryDelay.doubleValue;
MTR_LOG_INFO("%@ resetting resubscribe attempt counter, and delaying by the server-provided delay: %f",
self, secondsToWait);
} else {
_lastSubscriptionAttemptWait *= 2;
if (_lastSubscriptionAttemptWait > MTRDEVICE_SUBSCRIPTION_ATTEMPT_MAX_WAIT_SECONDS) {
_lastSubscriptionAttemptWait = MTRDEVICE_SUBSCRIPTION_ATTEMPT_MAX_WAIT_SECONDS;
}
secondsToWait = _lastSubscriptionAttemptWait;
}

MTR_LOG_DEFAULT("%@ scheduling to reattempt subscription in %u seconds", self, _lastSubscriptionAttemptWait);
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (_lastSubscriptionAttemptWait * NSEC_PER_SEC)), self.queue, ^{
MTR_LOG_DEFAULT("%@ scheduling to reattempt subscription in %f seconds", self, secondsToWait);
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (secondsToWait * NSEC_PER_SEC)), self.queue, ^{
os_unfair_lock_lock(&self->_lock);
[self _reattemptSubscriptionNowIfNeeded];
os_unfair_lock_unlock(&self->_lock);
Expand Down Expand Up @@ -1133,12 +1145,13 @@ - (void)_setupSubscription
[_deviceController
getSessionForNode:_nodeID.unsignedLongLongValue
completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error) {
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error,
NSNumber * _Nullable retryDelay) {
if (error != nil) {
MTR_LOG_ERROR("%@ getSessionForNode error %@", self, error);
dispatch_async(self.queue, ^{
[self _handleSubscriptionError:error];
[self _handleSubscriptionReset];
[self _handleSubscriptionReset:retryDelay];
});
return;
}
Expand Down Expand Up @@ -1193,7 +1206,7 @@ - (void)_setupSubscription

dispatch_async(self.queue, ^{
// OnDone
[self _handleSubscriptionReset];
[self _handleSubscriptionReset:nil];
});
os_unfair_lock_unlock(&self->_lock);
},
Expand Down Expand Up @@ -1300,7 +1313,7 @@ - (void)_setupSubscription
MTR_LOG_ERROR("%@ SendAutoResubscribeRequest error %@", self, error);
dispatch_async(self.queue, ^{
[self _handleSubscriptionError:error];
[self _handleSubscriptionReset];
[self _handleSubscriptionReset:nil];
});

return;
Expand Down
12 changes: 8 additions & 4 deletions src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#import <Foundation/Foundation.h>

#include <app/OperationalSessionSetup.h>
#include <controller/CHIPDeviceController.h>
#include <lib/core/ReferenceCounted.h>
#include <messaging/ExchangeMgr.h>
Expand All @@ -27,9 +28,12 @@
NS_ASSUME_NONNULL_BEGIN

// Either exchangeManager will be non-nil and session will have a value, or
// error will be non-nil.
// error will be non-nil. If error is non-nil, retryDelay might be non-nil. In
// that case it will contain an NSTimeInterval indicating how long consumers
// should delay before trying again (e.g. based on the information communicated
// in a BUSY response).
typedef void (^MTRInternalDeviceConnectionCallback)(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error);
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay);

/**
* Helper to establish or look up a CASE session and hand its session
Expand Down Expand Up @@ -62,11 +66,11 @@ class MTRDeviceConnectionBridge : public chip::ReferenceCounted<MTRDeviceConnect
private:
MTRInternalDeviceConnectionCallback mCompletionHandler;
chip::Callback::Callback<chip::OnDeviceConnected> mOnConnected;
chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnConnectFailed;
chip::Callback::Callback<chip::OperationalSessionSetup::OnSetupFailure> mOnConnectFailed;

static void OnConnected(
void * context, chip::Messaging::ExchangeManager & exchangeMgr, const chip::SessionHandle & sessionHandle);
static void OnConnectionFailure(void * context, const chip::ScopedNodeId & peerId, CHIP_ERROR error);
static void OnConnectionFailure(void * context, const chip::OperationalSessionSetup::ConnnectionFailureInfo & failureInfo);
};

NS_ASSUME_NONNULL_END
10 changes: 7 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceConnectionBridge.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@
void * context, chip::Messaging::ExchangeManager & exchangeMgr, const chip::SessionHandle & sessionHandle)
{
auto * object = static_cast<MTRDeviceConnectionBridge *>(context);
object->mCompletionHandler(&exchangeMgr, chip::MakeOptional<chip::SessionHandle>(*sessionHandle->AsSecureSession()), nil);
object->mCompletionHandler(&exchangeMgr, chip::MakeOptional<chip::SessionHandle>(*sessionHandle->AsSecureSession()), nil, nil);
object->Release();
}

void MTRDeviceConnectionBridge::OnConnectionFailure(void * context, const chip::ScopedNodeId & peerId, CHIP_ERROR error)
void MTRDeviceConnectionBridge::OnConnectionFailure(void * context, const chip::OperationalSessionSetup::ConnnectionFailureInfo & failureInfo)
{
NSNumber * retryDelay;
if (failureInfo.requestedBusyDelay.HasValue()) {
retryDelay = @(static_cast<double>(failureInfo.requestedBusyDelay.Value().count()) / MSEC_PER_SEC);
}
auto * object = static_cast<MTRDeviceConnectionBridge *>(context);
object->mCompletionHandler(nil, chip::NullOptional, [MTRError errorForCHIPErrorCode:error]);
object->mCompletionHandler(nil, chip::NullOptional, [MTRError errorForCHIPErrorCode:failureInfo.error], retryDelay);
object->Release();
}
12 changes: 6 additions & 6 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ - (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConn
connectionBridge->connect(commissioner, nodeID);
}
errorHandler:^(NSError * error) {
completion(nullptr, chip::NullOptional, error);
completion(nullptr, chip::NullOptional, error, nil);
}];
}

Expand All @@ -1226,20 +1226,20 @@ - (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRIn
chip::CommissioneeDeviceProxy * deviceProxy;
CHIP_ERROR err = commissioner->GetDeviceBeingCommissioned(deviceID, &deviceProxy);
if (err != CHIP_NO_ERROR) {
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:err]);
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:err], nil);
return;
}

chip::Optional<chip::SessionHandle> session = deviceProxy->GetSecureSession();
if (!session.HasValue() || !session.Value()->AsSecureSession()->IsPASESession()) {
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
return;
}

completion(deviceProxy->GetExchangeManager(), session, nil);
completion(deviceProxy->GetExchangeManager(), session, nil, nil);
}
errorHandler:^(NSError * error) {
completion(nullptr, chip::NullOptional, error);
completion(nullptr, chip::NullOptional, error, nil);
}];
}

Expand Down Expand Up @@ -1619,7 +1619,7 @@ - (BOOL)getBaseDevice:(uint64_t)deviceID queue:(dispatch_queue_t)queue completio
// that we are running.
[self getSessionForNode:deviceID
completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error) {
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
// Create an MTRBaseDevice for the node id involved, now that our
// CASE session is primed. We don't actually care about the session
// information here.
Expand Down
Loading