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.

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

Fix historical event detection in MTRDevice. #33462

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 37 additions & 8 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -140,21 +140,34 @@ typedef NS_ENUM(NSUInteger, MTRInternalDeviceState) {
// InitialSubscriptionEstablished means we have at some point finished setting up a
// subscription. That subscription may have dropped since then, but if so it's the ReadClient's
// responsibility to re-establish it.
MTRInternalDeviceStateInitalSubscriptionEstablished = 2,
MTRInternalDeviceStateInitialSubscriptionEstablished = 2,
// Resubscribing means we had established a subscription, but then
// detected a subscription drop due to not receiving a report on time. This
// covers all the actions that happen when re-subscribing (discovery, CASE,
// getting priming reports, etc).
MTRInternalDeviceStateResubscribing = 3,
// LaterSubscriptionEstablished meant that we had a subscription drop and
// then re-created a subscription.
MTRInternalDeviceStateLaterSubscriptionEstablished = 4,
};

// Utility methods for working with MTRInternalDeviceState, located near the
// enum so it's easier to notice that they need to stay in sync.
namespace {
bool HadSubscriptionEstablishedOnce(MTRInternalDeviceState state)
{
return state >= MTRInternalDeviceStateInitalSubscriptionEstablished;
return state >= MTRInternalDeviceStateInitialSubscriptionEstablished;
}

bool NeedToStartSubscriptionSetup(MTRInternalDeviceState state)
{
return state <= MTRInternalDeviceStateUnsubscribed;
}

bool HaveSubscriptionEstablishedRightNow(MTRInternalDeviceState state)
{
return state == MTRInternalDeviceStateInitialSubscriptionEstablished || state == MTRInternalDeviceStateLaterSubscriptionEstablished;
}
} // anonymous namespace

typedef NS_ENUM(NSUInteger, MTRDeviceExpectedValueFieldIndex) {
Expand Down Expand Up @@ -878,6 +891,16 @@ - (void)_changeState:(MTRDeviceState)state
}
}

- (void)_changeInternalState:(MTRInternalDeviceState)state
{
os_unfair_lock_assert_owner(&self->_lock);
MTRInternalDeviceState lastState = _internalDeviceState;
_internalDeviceState = state;
if (lastState != state) {
MTR_LOG_DEFAULT("%@ internal state change %lu => %lu", self, static_cast<unsigned long>(lastState), static_cast<unsigned long>(state));
}
}

// First Time Sync happens 2 minutes after reachability (this can be changed in the future)
#define MTR_DEVICE_TIME_UPDATE_INITIAL_WAIT_TIME_SEC (60 * 2)
- (void)_handleSubscriptionEstablished
Expand All @@ -886,7 +909,11 @@ - (void)_handleSubscriptionEstablished

// reset subscription attempt wait time when subscription succeeds
_lastSubscriptionAttemptWait = 0;
_internalDeviceState = MTRInternalDeviceStateInitalSubscriptionEstablished;
if (HadSubscriptionEstablishedOnce(_internalDeviceState)) {
[self _changeInternalState:MTRInternalDeviceStateLaterSubscriptionEstablished];
} else {
[self _changeInternalState:MTRInternalDeviceStateInitialSubscriptionEstablished];
}

// As subscription is established, check if the delegate needs to be informed
if (!_delegateDeviceCachePrimedCalled) {
Expand All @@ -913,7 +940,7 @@ - (void)_handleSubscriptionError:(NSError *)error
{
std::lock_guard lock(_lock);

_internalDeviceState = MTRInternalDeviceStateUnsubscribed;
[self _changeInternalState:MTRInternalDeviceStateUnsubscribed];
_unreportedEvents = nil;

[self _changeState:MTRDeviceStateUnreachable];
Expand All @@ -924,6 +951,7 @@ - (void)_handleResubscriptionNeeded
std::lock_guard lock(_lock);

[self _changeState:MTRDeviceStateUnknown];
[self _changeInternalState:MTRInternalDeviceStateResubscribing];

// If we are here, then the ReadClient either just detected a subscription
// drop or just tried again and failed. Either way, count it as "tried and
Expand Down Expand Up @@ -1044,11 +1072,12 @@ - (void)_handleReportBegin

_receivingReport = YES;
if (_state != MTRDeviceStateReachable) {
_receivingPrimingReport = YES;
[self _changeState:MTRDeviceStateReachable];
} else {
_receivingPrimingReport = NO;
}

// If we currently don't have an established subscription, this must be a
// priming report.
_receivingPrimingReport = !HaveSubscriptionEstablishedRightNow(_internalDeviceState);
}

- (NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)_clusterDataToPersistSnapshot
Expand Down Expand Up @@ -1461,7 +1490,7 @@ - (void)_setupSubscription
return;
}

_internalDeviceState = MTRInternalDeviceStateSubscribing;
[self _changeInternalState:MTRInternalDeviceStateSubscribing];

// Set up a timer to mark as not reachable if it takes too long to set up a subscription
MTRWeakReference<MTRDevice *> * weakSelf = [MTRWeakReference weakReferenceWithObject:self];
Expand Down
Loading
Loading