Skip to content

Commit

Permalink
Don't interrupt a CASE attempt triggered by _deviceMayBeReachable. (#…
Browse files Browse the repository at this point in the history
…37871)

There's a really good chance this attempt will in fact succeed, so don't touch
it.

As a followup, we will need to figure out a better state machine here.
  • Loading branch information
bzbarsky-apple authored Mar 4, 2025
1 parent ac1c96a commit 023d1e5
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@

static NSString * const sLastInitialSubscribeLatencyKey = @"lastInitialSubscribeLatency";

static NSString * const sDeviceMayBeReachableReason = @"SPI client indicated the device may now be reachable";

// Not static, because these are public API.
NSString * const MTRPreviousDataKey = @"previousData";
NSString * const MTRDataVersionKey = @"dataVersion";
Expand Down Expand Up @@ -306,6 +308,7 @@ @interface MTRDevice_Concrete ()
// Actively receiving priming report

@property (nonatomic) MTRInternalDeviceState internalDeviceState;
@property (nonatomic) BOOL doingCASEAttemptForDeviceMayBeReachable;

#define MTRDEVICE_SUBSCRIPTION_ATTEMPT_MIN_WAIT_SECONDS (1)
#define MTRDEVICE_SUBSCRIPTION_ATTEMPT_MAX_WAIT_SECONDS (3600)
Expand Down Expand Up @@ -470,6 +473,7 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
_state = MTRDeviceStateUnknown;
_internalDeviceState = MTRInternalDeviceStateUnsubscribed;
_internalDeviceStateForDescription = MTRInternalDeviceStateUnsubscribed;
_doingCASEAttemptForDeviceMayBeReachable = NO;
if (controller.controllerDataStore) {
_persistedClusterData = [[NSCache alloc] init];
} else {
Expand Down Expand Up @@ -1049,7 +1053,7 @@ - (void)_triggerResubscribeWithReason:(NSString *)reason nodeLikelyReachable:(BO
subscriptionCallback->ResetResubscriptionBackoff();
}
readClientToResubscribe->TriggerResubscribeIfScheduled(reason.UTF8String);
} else if (((_internalDeviceState == MTRInternalDeviceStateSubscribing) || shouldReattemptSubscription) && nodeLikelyReachable) {
} else if (((_internalDeviceState == MTRInternalDeviceStateSubscribing && !self.doingCASEAttemptForDeviceMayBeReachable) || shouldReattemptSubscription) && nodeLikelyReachable) {
// If we have reason to suspect that the node is now reachable and we haven't established a
// CASE session yet, let's consider it to be stalled and invalidate the pairing session.

Expand Down Expand Up @@ -2745,6 +2749,9 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
[self _changeInternalState:MTRInternalDeviceStateSubscribing];

MTR_LOG("%@ setting up subscription with reason: %@", self, reason);
if ([reason hasPrefix:sDeviceMayBeReachableReason]) {
self.doingCASEAttemptForDeviceMayBeReachable = YES;
}

__block bool markUnreachableAfterWait = true;
#ifdef DEBUG
Expand Down Expand Up @@ -2784,6 +2791,8 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
mtr_strongify(self);
VerifyOrReturn(self, MTR_LOG_DEBUG("_setupSubscriptionWithReason directlyGetSessionForNode called back with nil MTRDevice"));

self.doingCASEAttemptForDeviceMayBeReachable = NO;

if (error != nil) {
MTR_LOG_ERROR("%@ getSessionForNode error %@", self, error);
[self->_deviceController asyncDispatchToMatterQueue:^{
Expand Down Expand Up @@ -4844,7 +4853,7 @@ - (void)_deviceMayBeReachable
std::lock_guard lock(self->_lock);
// Use _ensureSubscriptionForExistingDelegates so that the subscriptions
// will go through the pool as needed, not necessarily happen immediately.
[self _ensureSubscriptionForExistingDelegates:@"SPI client indicated the device may now be reachable"];
[self _ensureSubscriptionForExistingDelegates:sDeviceMayBeReachableReason];
} errorHandler:nil];
}

Expand Down

0 comments on commit 023d1e5

Please sign in to comment.