Skip to content

Commit 294f29e

Browse files
Stop assuming all unknown attributes have the C quality in MTRDevice. (#33009)
* Stop assuming all unknown attributes have the C quality in MTRDevice. 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. * Address review comments and fix tests to work in the new setup.
1 parent b4a6213 commit 294f29e

8 files changed

+199
-87
lines changed

src/darwin/Framework/CHIP/MTRCluster.h

+11
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
132132
*/
133133
@property (nonatomic, copy, nullable) NSNumber * minEventNumber MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
134134

135+
/**
136+
* Controls whether attributes without known schema (e.g. vendor-specific
137+
* attributes) should be assumed to be reportable normally via subscriptions.
138+
* The default is YES.
139+
*
140+
* This setting is only relevant to some consumers of MTRReadParams. One of
141+
* those consumers is readAttributeWithEndpointID:clusterID:attributeID:params:
142+
* on MTRDevice.
143+
*/
144+
@property (nonatomic, assign, getter=shouldAssumeUnknownAttributesReportable) BOOL assumeUnknownAttributesReportable MTR_NEWLY_AVAILABLE;
145+
135146
@end
136147

137148
/**

src/darwin/Framework/CHIP/MTRCluster.mm

+3
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ - (instancetype)init
8484
{
8585
if (self = [super init]) {
8686
_filterByFabric = YES;
87+
_assumeUnknownAttributesReportable = YES;
8788
}
8889
return self;
8990
}
@@ -93,6 +94,7 @@ - (id)copyWithZone:(NSZone * _Nullable)zone
9394
auto other = [[MTRReadParams alloc] init];
9495
other.filterByFabric = self.filterByFabric;
9596
other.minEventNumber = self.minEventNumber;
97+
other.assumeUnknownAttributesReportable = self.assumeUnknownAttributesReportable;
9698
return other;
9799
}
98100

@@ -124,6 +126,7 @@ - (id)copyWithZone:(NSZone * _Nullable)zone
124126
auto other = [[MTRSubscribeParams alloc] initWithMinInterval:self.minInterval maxInterval:self.maxInterval];
125127
other.filterByFabric = self.filterByFabric;
126128
other.minEventNumber = self.minEventNumber;
129+
other.assumeUnknownAttributesReportable = self.assumeUnknownAttributesReportable;
127130
other.replaceExistingSubscriptions = self.replaceExistingSubscriptions;
128131
other.reportEventsUrgently = self.reportEventsUrgently;
129132
other.resubscribeAutomatically = self.resubscribeAutomatically;

src/darwin/Framework/CHIP/MTRDevice.mm

+30-8
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ - (void)unitTestReportEndForDevice:(MTRDevice *)device;
336336
- (BOOL)unitTestShouldSetUpSubscriptionForDevice:(MTRDevice *)device;
337337
- (BOOL)unitTestShouldSkipExpectedValuesForWrite:(MTRDevice *)device;
338338
- (NSNumber *)unitTestMaxIntervalOverrideForSubscription:(MTRDevice *)device;
339+
- (BOOL)unitTestForceAttributeReportsIfMatchingCache:(MTRDevice *)device;
339340
@end
340341
#endif
341342

@@ -745,7 +746,7 @@ - (BOOL)_subscriptionAbleToReport
745746

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

@@ -1603,24 +1604,33 @@ static BOOL AttributeHasChangesOmittedQuality(MTRAttributePath * attributePath)
16031604
- (NSDictionary<NSString *, id> * _Nullable)readAttributeWithEndpointID:(NSNumber *)endpointID
16041605
clusterID:(NSNumber *)clusterID
16051606
attributeID:(NSNumber *)attributeID
1606-
params:(MTRReadParams *)params
1607+
params:(MTRReadParams * _Nullable)params
16071608
{
16081609
MTRAttributePath * attributePath = [MTRAttributePath attributePathWithEndpointID:endpointID
16091610
clusterID:clusterID
16101611
attributeID:attributeID];
16111612

16121613
BOOL attributeIsSpecified = MTRAttributeIsSpecified(clusterID.unsignedIntValue, attributeID.unsignedIntValue);
1613-
BOOL hasChangesOmittedQuality = AttributeHasChangesOmittedQuality(attributePath);
1614+
BOOL hasChangesOmittedQuality;
1615+
if (attributeIsSpecified) {
1616+
hasChangesOmittedQuality = AttributeHasChangesOmittedQuality(attributePath);
1617+
} else {
1618+
if (params == nil) {
1619+
hasChangesOmittedQuality = NO;
1620+
} else {
1621+
hasChangesOmittedQuality = !params.assumeUnknownAttributesReportable;
1622+
}
1623+
}
16141624

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

16181628
// Send read request to device if any of the following are true:
1619-
// 1. The attribute is not in the specification (so we don't know whether hasChangesOmittedQuality can be trusted).
1620-
// 2. Subscription not in a state we can expect reports
1621-
// 3. There is subscription but attribute has Changes Omitted quality
1622-
// TODO: add option for BaseSubscriptionCallback to report during priming, to reduce when case 4 is hit
1623-
if (!attributeIsSpecified || ![self _subscriptionAbleToReport] || hasChangesOmittedQuality) {
1629+
// 1. Subscription not in a state we can expect reports
1630+
// 2. The attribute has the Changes Omitted quality, so we won't get reports for it.
1631+
// 3. The attribute is not in the spec, and the read params asks to assume
1632+
// an unknown attribute has the Changes Omitted quality.
1633+
if (![self _subscriptionAbleToReport] || hasChangesOmittedQuality) {
16241634
// Read requests container will be a mutable array of items, each being an array containing:
16251635
// [attribute request path, params]
16261636
// Batching handler should only coalesce when params are equal.
@@ -2344,6 +2354,18 @@ - (NSArray *)_getAttributesToReportWithReportedValues:(NSArray<NSDictionary<NSSt
23442354
}
23452355
NSArray * expectedValue = _expectedValueCache[attributePath];
23462356

2357+
// Unit test only code.
2358+
#ifdef DEBUG
2359+
if (!readCacheValueChanged) {
2360+
id delegate = _weakDelegate.strongObject;
2361+
if (delegate) {
2362+
if ([delegate respondsToSelector:@selector(unitTestForceAttributeReportsIfMatchingCache:)]) {
2363+
readCacheValueChanged = [delegate unitTestForceAttributeReportsIfMatchingCache:self];
2364+
}
2365+
}
2366+
}
2367+
#endif // DEBUG
2368+
23472369
// Report the attribute if a read would get a changed value. This happens
23482370
// when our cached value changes and no expected value exists.
23492371
if (readCacheValueChanged && !expectedValue) {

src/darwin/Framework/CHIPTests/MTRDeviceTests.m

+6-1
Original file line numberDiff line numberDiff line change
@@ -1518,6 +1518,10 @@ - (void)test017_TestMTRDeviceBasics
15181518
[self waitForExpectations:@[ onTimeWriteSuccess, onTimePreviousValue ] timeout:10];
15191519

15201520
// Test if errors are properly received
1521+
// TODO: We might stop reporting these altogether from MTRDevice, and then
1522+
// this test will need updating.
1523+
__auto_type * readThroughForUnknownAttributesParams = [[MTRReadParams alloc] init];
1524+
readThroughForUnknownAttributesParams.assumeUnknownAttributesReportable = NO;
15211525
XCTestExpectation * attributeReportErrorExpectation = [self expectationWithDescription:@"Attribute read error"];
15221526
delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * data) {
15231527
for (NSDictionary<NSString *, id> * attributeReponseValue in data) {
@@ -1526,8 +1530,9 @@ - (void)test017_TestMTRDeviceBasics
15261530
}
15271531
}
15281532
};
1533+
15291534
// use the nonexistent attribute and expect read error
1530-
[device readAttributeWithEndpointID:testEndpointID clusterID:testClusterID attributeID:testAttributeID params:nil];
1535+
[device readAttributeWithEndpointID:testEndpointID clusterID:testClusterID attributeID:testAttributeID params:readThroughForUnknownAttributesParams];
15311536
[self waitForExpectations:@[ attributeReportErrorExpectation ] timeout:10];
15321537

15331538
// Resubscription test setup

0 commit comments

Comments
 (0)