Skip to content

Commit

Permalink
Stop assuming all unknown attributes have the C quality in MTRDevice.
Browse files Browse the repository at this point in the history
Assume things we don't know about don't have the C quality unless told to assume
otherwise.

Also fixes XPC detection check; non-XPC controllers also respond to the
"sharedControllerWithID:xpcConnectBlock:" selector.
  • Loading branch information
bzbarsky-apple committed Apr 16, 2024
1 parent d8245cb commit 5ed6ad0
Show file tree
Hide file tree
Showing 6 changed files with 186 additions and 85 deletions.
11 changes: 11 additions & 0 deletions src/darwin/Framework/CHIP/MTRCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,17 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
*/
@property (nonatomic, copy, nullable) NSNumber * minEventNumber MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));

/**
* Controls whether attributes without known schema (e.g. vendor-specific
* attributes) should be assumed to be reportable normally via subscriptions.
* The default is YES.
*
* This setting is only relevant to some consumers of MTRReadParams. One of
* those consumers is readAttributeWithEndpointID:clusterID:attributeID:params:
* on MTRDevice.
*/
@property (nonatomic, assign, getter=shouldAssumeUnknownAttributesReportable) BOOL assumeUnknownAttributesReportable MTR_NEWLY_AVAILABLE;

@end

/**
Expand Down
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRCluster.mm
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ - (instancetype)init
{
if (self = [super init]) {
_filterByFabric = YES;
_assumeUnknownAttributesReportable = YES;
}
return self;
}
Expand All @@ -93,6 +94,7 @@ - (id)copyWithZone:(NSZone * _Nullable)zone
auto other = [[MTRReadParams alloc] init];
other.filterByFabric = self.filterByFabric;
other.minEventNumber = self.minEventNumber;
other.assumeUnknownAttributesReportable = self.assumeUnknownAttributesReportable;
return other;
}

Expand Down Expand Up @@ -124,6 +126,7 @@ - (id)copyWithZone:(NSZone * _Nullable)zone
auto other = [[MTRSubscribeParams alloc] initWithMinInterval:self.minInterval maxInterval:self.maxInterval];
other.filterByFabric = self.filterByFabric;
other.minEventNumber = self.minEventNumber;
other.assumeUnknownAttributesReportable = self.assumeUnknownAttributesReportable;
other.replaceExistingSubscriptions = self.replaceExistingSubscriptions;
other.reportEventsUrgently = self.reportEventsUrgently;
other.resubscribeAutomatically = self.resubscribeAutomatically;
Expand Down
36 changes: 28 additions & 8 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ - (void)unitTestReportEndForDevice:(MTRDevice *)device;
- (BOOL)unitTestShouldSetUpSubscriptionForDevice:(MTRDevice *)device;
- (BOOL)unitTestShouldSkipExpectedValuesForWrite:(MTRDevice *)device;
- (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device;
- (BOOL)unitTestForceAttributeReportsIfMatchingCache:(MTRDevice *)device;
@end
#endif

Expand Down Expand Up @@ -745,7 +746,7 @@ - (BOOL)_subscriptionAbleToReport

// Unfortunately, we currently have no subscriptions over our hacked-up XPC
// setup. Try to detect that situation.
if ([_deviceController.class respondsToSelector:@selector(sharedControllerWithID:xpcConnectBlock:)]) {
if ([_deviceController isKindOfClass:MTRDeviceControllerOverXPC.class]) {
return NO;
}

Expand Down Expand Up @@ -1603,24 +1604,31 @@ static BOOL AttributeHasChangesOmittedQuality(MTRAttributePath * attributePath)
- (NSDictionary<NSString *, id> * _Nullable)readAttributeWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
attributeID:(NSNumber *)attributeID
params:(MTRReadParams *)params
params:(MTRReadParams * _Nullable)params
{
MTRAttributePath * attributePath = [MTRAttributePath attributePathWithEndpointID:endpointID
clusterID:clusterID
attributeID:attributeID];

BOOL attributeIsSpecified = MTRAttributeIsSpecified(clusterID.unsignedIntValue, attributeID.unsignedIntValue);
BOOL hasChangesOmittedQuality = AttributeHasChangesOmittedQuality(attributePath);
BOOL hasChangesOmittedQuality;
if (attributeIsSpecified) {
hasChangesOmittedQuality = AttributeHasChangesOmittedQuality(attributePath);
} else {
if (params == nil) {
hasChangesOmittedQuality = NO;
} else {
hasChangesOmittedQuality = !params.assumeUnknownAttributesReportable;
}
}

// Return current known / expected value right away
NSDictionary<NSString *, id> * attributeValueToReturn = [self _attributeValueDictionaryForAttributePath:attributePath];

// Send read request to device if any of the following are true:
// 1. The attribute is not in the specification (so we don't know whether hasChangesOmittedQuality can be trusted).
// 2. Subscription not in a state we can expect reports
// 3. There is subscription but attribute has Changes Omitted quality
// TODO: add option for BaseSubscriptionCallback to report during priming, to reduce when case 4 is hit
if (!attributeIsSpecified || ![self _subscriptionAbleToReport] || hasChangesOmittedQuality) {
// 1. Subscription not in a state we can expect reports
// 2. The attribute has (or is assumed to have) the Changes Omitted quality, so we won't get reports for it.
if (![self _subscriptionAbleToReport] || hasChangesOmittedQuality) {
// Read requests container will be a mutable array of items, each being an array containing:
// [attribute request path, params]
// Batching handler should only coalesce when params are equal.
Expand Down Expand Up @@ -2356,6 +2364,18 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
shouldReportAttribute = YES;
}

#ifdef DEBUG
// Unit test only code.
if (!shouldReportAttribute) {
id delegate = _weakDelegate.strongObject;
if (delegate) {
if ([delegate respondsToSelector:@selector(unitTestForceAttributeReportsIfMatchingCache:)]) {
shouldReportAttribute = [delegate unitTestForceAttributeReportsIfMatchingCache:self];
}
}
}
#endif // DEBUG

if (!shouldReportAttribute) {
if (expectedValue) {
MTR_LOG_INFO("%@ report %@ value filtered - same as expected values", self, attributePath);
Expand Down
Loading

0 comments on commit 5ed6ad0

Please sign in to comment.