|
52 | 52 | NSString * const MTRPreviousDataKey = @"previousData"; |
53 | 53 | NSString * const MTRDataVersionKey = @"dataVersion"; |
54 | 54 |
|
55 | | -#define kTimeToWaitBeforeMarkingUnreachableAfterSettingUpSubscription 10 |
| 55 | +#define kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription 10 |
56 | 56 |
|
57 | 57 | // Consider moving utility classes to their own file |
58 | 58 | #pragma mark - Utility Classes |
@@ -129,11 +129,31 @@ - (id)strongObject |
129 | 129 |
|
130 | 130 | #pragma mark - MTRDevice |
131 | 131 | typedef NS_ENUM(NSUInteger, MTRInternalDeviceState) { |
| 132 | + // Unsubscribed means we do not have a subscription and are not trying to set one up. |
132 | 133 | MTRInternalDeviceStateUnsubscribed = 0, |
| 134 | + // Subscribing means we are actively trying to establish our initial subscription (e.g. doing |
| 135 | + // DNS-SD discovery, trying to establish CASE to the peer, getting priming reports, etc). |
133 | 136 | MTRInternalDeviceStateSubscribing = 1, |
134 | | - MTRInternalDeviceStateSubscribed = 2 |
| 137 | + // InitialSubscriptionEstablished means we have at some point finished setting up a |
| 138 | + // subscription. That subscription may have dropped since then, but if so it's the ReadClient's |
| 139 | + // responsibility to re-establish it. |
| 140 | + MTRInternalDeviceStateInitalSubscriptionEstablished = 2, |
135 | 141 | }; |
136 | 142 |
|
| 143 | +// Utility methods for working with MTRInternalDeviceState, located near the |
| 144 | +// enum so it's easier to notice that they need to stay in sync. |
| 145 | +namespace { |
| 146 | +bool HadSubscriptionEstablishedOnce(MTRInternalDeviceState state) |
| 147 | +{ |
| 148 | + return state >= MTRInternalDeviceStateInitalSubscriptionEstablished; |
| 149 | +} |
| 150 | + |
| 151 | +bool NeedToStartSubscriptionSetup(MTRInternalDeviceState state) |
| 152 | +{ |
| 153 | + return state <= MTRInternalDeviceStateUnsubscribed; |
| 154 | +} |
| 155 | +} // anonymous namespace |
| 156 | + |
137 | 157 | typedef NS_ENUM(NSUInteger, MTRDeviceExpectedValueFieldIndex) { |
138 | 158 | MTRDeviceExpectedValueFieldExpirationTimeIndex = 0, |
139 | 159 | MTRDeviceExpectedValueFieldValueIndex = 1, |
@@ -283,6 +303,7 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle |
283 | 303 | _expectedValueCache = [NSMutableDictionary dictionary]; |
284 | 304 | _asyncWorkQueue = [[MTRAsyncWorkQueue alloc] initWithContext:self]; |
285 | 305 | _state = MTRDeviceStateUnknown; |
| 306 | + _internalDeviceState = MTRInternalDeviceStateUnsubscribed; |
286 | 307 | _clusterData = [NSMutableDictionary dictionary]; |
287 | 308 | MTR_LOG_INFO("%@ init with hex nodeID 0x%016llX", self, _nodeID.unsignedLongLongValue); |
288 | 309 | } |
@@ -576,6 +597,10 @@ - (void)invalidate |
576 | 597 | // attempt, since we now have no delegate. |
577 | 598 | _reattemptingSubscription = NO; |
578 | 599 |
|
| 600 | + // We do not change _internalDeviceState here, because we might still have a |
| 601 | + // subscription. In that case, _internalDeviceState will update when the |
| 602 | + // subscription is actually terminated. |
| 603 | + |
579 | 604 | os_unfair_lock_unlock(&self->_lock); |
580 | 605 | } |
581 | 606 |
|
@@ -678,7 +703,7 @@ - (void)_handleSubscriptionEstablished |
678 | 703 |
|
679 | 704 | // reset subscription attempt wait time when subscription succeeds |
680 | 705 | _lastSubscriptionAttemptWait = 0; |
681 | | - _internalDeviceState = MTRInternalDeviceStateSubscribed; |
| 706 | + _internalDeviceState = MTRInternalDeviceStateInitalSubscriptionEstablished; |
682 | 707 |
|
683 | 708 | // As subscription is established, check if the delegate needs to be informed |
684 | 709 | if (!_delegateDeviceCachePrimedCalled) { |
@@ -786,12 +811,13 @@ - (void)_handleUnsolicitedMessageFromPublisher |
786 | 811 | [self _reattemptSubscriptionNowIfNeeded]; |
787 | 812 | } |
788 | 813 |
|
789 | | -- (void)_markDeviceAsUnreachableIfNotSusbcribed |
| 814 | +- (void)_markDeviceAsUnreachableIfNeverSubscribed |
790 | 815 | { |
791 | 816 | os_unfair_lock_assert_owner(&self->_lock); |
792 | 817 |
|
793 | | - if (_internalDeviceState >= MTRInternalDeviceStateSubscribed) |
| 818 | + if (HadSubscriptionEstablishedOnce(_internalDeviceState)) { |
794 | 819 | return; |
| 820 | + } |
795 | 821 |
|
796 | 822 | MTR_LOG_DEFAULT("%@ still not subscribed, marking the device as unreachable", self); |
797 | 823 | [self _changeState:MTRDeviceStateUnreachable]; |
@@ -1025,20 +1051,19 @@ - (void)_setupSubscription |
1025 | 1051 | #endif |
1026 | 1052 |
|
1027 | 1053 | // for now just subscribe once |
1028 | | - if (_internalDeviceState > MTRInternalDeviceStateUnsubscribed) { |
| 1054 | + if (!NeedToStartSubscriptionSetup(_internalDeviceState)) { |
1029 | 1055 | return; |
1030 | 1056 | } |
1031 | 1057 |
|
1032 | 1058 | _internalDeviceState = MTRInternalDeviceStateSubscribing; |
1033 | 1059 |
|
1034 | 1060 | // Set up a timer to mark as not reachable if it takes too long to set up a subscription |
1035 | 1061 | MTRWeakReference<MTRDevice *> * weakSelf = [MTRWeakReference weakReferenceWithObject:self]; |
1036 | | - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t) (kTimeToWaitBeforeMarkingUnreachableAfterSettingUpSubscription * NSEC_PER_SEC)), self.queue, ^{ |
| 1062 | + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast<int64_t>(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast<int64_t>(NSEC_PER_SEC)), self.queue, ^{ |
1037 | 1063 | MTRDevice * strongSelf = weakSelf.strongObject; |
1038 | 1064 | if (strongSelf != nil) { |
1039 | | - os_unfair_lock_lock(&strongSelf->_lock); |
1040 | | - [strongSelf _markDeviceAsUnreachableIfNotSusbcribed]; |
1041 | | - os_unfair_lock_unlock(&strongSelf->_lock); |
| 1065 | + std::lock_guard lock(strongSelf->_lock); |
| 1066 | + [strongSelf _markDeviceAsUnreachableIfNeverSubscribed]; |
1042 | 1067 | } |
1043 | 1068 | }); |
1044 | 1069 |
|
|
0 commit comments