Skip to content

Commit

Permalink
Apply API review fixes for MTRReadParams and MTRSubscribeParams. (#22592
Browse files Browse the repository at this point in the history
)

* Move minInterval and maxInterval into MTRSubscribeParams.

* Make the booleans in MTRSubsribeParams and MTRReadParams just BOOL (inited to
  the default) instead of "NSNumber with nil meaning default".

Fixes #22536
Fixes #22534

Addresses part of #22420
  • Loading branch information
bzbarsky-apple authored Sep 14, 2022
1 parent bad4408 commit 72d838a
Show file tree
Hide file tree
Showing 23 changed files with 20,568 additions and 27,955 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class ReadAttribute : public ModelCommand {
{
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.command", DISPATCH_QUEUE_SERIAL);
MTRReadParams * params = [[MTRReadParams alloc] init];
params.fabricFiltered = mFabricFiltered.HasValue() ? [NSNumber numberWithBool:mFabricFiltered.Value()] : nil;
if (mFabricFiltered.HasValue()) {
params.fabricFiltered = mFabricFiltered.Value();
}
[device readAttributePathWithEndpointID:[NSNumber numberWithUnsignedShort:endpointId]
clusterID:[NSNumber numberWithUnsignedInteger:mClusterId]
attributeID:[NSNumber numberWithUnsignedInteger:mAttributeId]
Expand Down Expand Up @@ -124,16 +126,20 @@ class SubscribeAttribute : public ModelCommand {
{
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.command", DISPATCH_QUEUE_SERIAL);

MTRSubscribeParams * params = [[MTRSubscribeParams alloc] init];
params.keepPreviousSubscriptions
= mKeepSubscriptions.HasValue() ? [NSNumber numberWithBool:mKeepSubscriptions.Value()] : nil;
params.autoResubscribe = mAutoResubscribe.HasValue() ? [NSNumber numberWithBool:mAutoResubscribe.Value()] : nil;
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(mMinInterval) maxInterval:@(mMaxInterval)];
if (mFabricFiltered.HasValue()) {
params.fabricFiltered = mFabricFiltered.Value();
}
if (mKeepSubscriptions.HasValue()) {
params.keepPreviousSubscriptions = mKeepSubscriptions.Value();
}
if (mAutoResubscribe.HasValue()) {
params.autoResubscribe = mAutoResubscribe.Value();
}

[device subscribeAttributePathWithEndpointID:[NSNumber numberWithUnsignedShort:endpointId]
clusterID:[NSNumber numberWithUnsignedInteger:mClusterId]
attributeID:[NSNumber numberWithUnsignedInteger:mAttributeId]
minInterval:[NSNumber numberWithUnsignedInteger:mMinInterval]
maxInterval:[NSNumber numberWithUnsignedInteger:mMaxInterval]
params:params
queue:callbackQueue
reportHandler:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
Expand Down Expand Up @@ -192,14 +198,15 @@ class SubscribeEvent : public ModelCommand {
{
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.command", DISPATCH_QUEUE_SERIAL);

MTRSubscribeParams * params = [[MTRSubscribeParams alloc] init];
params.keepPreviousSubscriptions
= mKeepSubscriptions.HasValue() ? [NSNumber numberWithBool:mKeepSubscriptions.Value()] : nil;
params.autoResubscribe = mAutoResubscribe.HasValue() ? [NSNumber numberWithBool:mAutoResubscribe.Value()] : nil;
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(mMinInterval) maxInterval:@(mMaxInterval)];
if (mKeepSubscriptions.HasValue()) {
params.keepPreviousSubscriptions = mKeepSubscriptions.Value();
}
if (mAutoResubscribe.HasValue()) {
params.autoResubscribe = mAutoResubscribe.Value();
}

[device subscribeWithQueue:callbackQueue
minInterval:@(mMinInterval)
maxInterval:@(mMaxInterval)
params:params
attributeCacheContainer:nil
attributeReportHandler:^(NSArray * value) {
Expand Down
18 changes: 11 additions & 7 deletions examples/darwin-framework-tool/templates/commands.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ public:
MTRBaseCluster{{asUpperCamelCase parent.name}} * cluster = [[MTRBaseCluster{{asUpperCamelCase parent.name}} alloc] initWithDevice:device endpoint:@(endpointId) queue:callbackQueue];
{{#if_is_fabric_scoped_struct type}}
MTRReadParams * params = [[MTRReadParams alloc] init];
params.fabricFiltered = mFabricFiltered.HasValue() ? [NSNumber numberWithBool:mFabricFiltered.Value()] : nil;
if (mFabricFiltered.HasValue()) {
params.fabricFiltered = mFabricFiltered.Value();
}
{{/if_is_fabric_scoped_struct}}
[cluster readAttribute{{asUpperCamelCase name}}With
{{~#if_is_fabric_scoped_struct type~}}
Expand Down Expand Up @@ -216,12 +218,14 @@ public:
ChipLogProgress(chipTool, "Sending cluster ({{asHex parent.code 8}}) ReportAttribute ({{asHex code 8}}) on endpoint %u", endpointId);
dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.command", DISPATCH_QUEUE_SERIAL);
MTRBaseCluster{{asUpperCamelCase parent.name}} * cluster = [[MTRBaseCluster{{asUpperCamelCase parent.name}} alloc] initWithDevice:device endpoint:@(endpointId) queue:callbackQueue];
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] init];
params.keepPreviousSubscriptions = mKeepSubscriptions.HasValue() ? [NSNumber numberWithBool:mKeepSubscriptions.Value()] : nil;
params.fabricFiltered = mFabricFiltered.HasValue() ? [NSNumber numberWithBool:mFabricFiltered.Value()] : nil;
[cluster subscribe{{>attribute}}WithMinInterval:[NSNumber numberWithUnsignedInt:mMinInterval]
maxInterval:[NSNumber numberWithUnsignedInt:mMaxInterval]
params:params
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(mMinInterval) maxInterval:@(mMaxInterval)];
if (mKeepSubscriptions.HasValue()) {
params.keepPreviousSubscriptions = mKeepSubscriptions.Value();
}
if (mFabricFiltered.HasValue()) {
params.fabricFiltered = mFabricFiltered.Value();
}
[cluster subscribe{{>attribute}}WithParams:params
subscriptionEstablished:^(){ mSubscriptionEstablished=YES; }
reportHandler:^({{asObjectiveCClass type parent.name}} * _Nullable value, NSError * _Nullable error) {
NSLog(@"{{asUpperCamelCase parent.name}}.{{asUpperCamelCase name}} response %@", [value description]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,8 @@ class {{filename}}: public TestCommandBridge
{{#chip_tests_item_parameters}}
{{asObjectiveCBasicType type}} {{asLowerCamelCase name}}Argument = {{asTypedLiteral definedValue type}};
{{/chip_tests_item_parameters}}
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] init];
[cluster subscribeAttribute{{asUpperCamelCase attribute}}WithMinInterval:[NSNumber numberWithUnsignedInt:minIntervalArgument]
maxInterval:[NSNumber numberWithUnsignedInt:maxIntervalArgument]
params:params
MTRSubscribeParams * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(minIntervalArgument) maxInterval:@(maxIntervalArgument)];
[cluster subscribeAttribute{{asUpperCamelCase attribute}}WithParams:params
subscriptionEstablished:^{
VerifyOrReturn(testSendCluster{{parent.filename}}_{{waitForReport.index}}_{{asUpperCamelCase waitForReport.command}}_Fulfilled, SetCommandExitStatus(CHIP_ERROR_INCORRECT_STATE));
NextTest();
Expand All @@ -168,7 +166,7 @@ class {{filename}}: public TestCommandBridge
{{else if isReadAttribute}}
{{#if_is_fabric_scoped_struct attributeObject.type}}
MTRReadParams * params = [[MTRReadParams alloc] init];
params.fabricFiltered = [NSNumber numberWithBool:{{fabricFiltered}}];
params.fabricFiltered = {{fabricFiltered}};
{{/if_is_fabric_scoped_struct}}
[cluster readAttribute{{asUpperCamelCase attribute}}With
{{~#if_is_fabric_scoped_struct attributeObject.type~}}
Expand Down
2 changes: 1 addition & 1 deletion scripts/tests/chiptest/lsan-mac-suppressions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ leak:[CHIPToolKeypair signMessageECDSA_RAW:]
leak:[CHIPToolPersistentStorageDelegate storageDataForKey:]

# TODO: https://github.com/project-chip/connectedhomeip/issues/22333
leak:[MTRBaseCluster* subscribeAttribute*WithMinInterval:maxInterval:params:subscriptionEstablished:reportHandler:]
leak:[MTRBaseCluster* subscribeAttribute*WithParams:subscriptionEstablished:reportHandler:]
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ - (void)fetchFabricsList
queue:dispatch_get_main_queue()];
[self updateResult:[NSString stringWithFormat:@"readAttributeFabrics command sent."] isError:NO];
MTRReadParams * params = [[MTRReadParams alloc] init];
params.fabricFiltered = @NO;
params.fabricFiltered = NO;
[cluster
readAttributeFabricsWithParams:params
completion:^(NSArray * _Nullable fabricsList, NSError * _Nullable error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,10 @@ - (void)reportFromUserEnteredSettings
if (MTRGetConnectedDevice(^(MTRBaseDevice * _Nullable chipDevice, NSError * _Nullable error) {
if (chipDevice) {
// Use a wildcard subscription
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(minIntervalSeconds)
maxInterval:@(maxIntervalSeconds)];
[chipDevice subscribeWithQueue:dispatch_get_main_queue()
minInterval:@(minIntervalSeconds)
maxInterval:@(maxIntervalSeconds)
params:nil
params:params
attributeCacheContainer:nil
attributeReportHandler:^(NSArray * _Nullable reports) {
if (!reports)
Expand Down
8 changes: 2 additions & 6 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,7 @@ extern NSString * const MTRArrayValueType;
* attempts fail.
*/
- (void)subscribeWithQueue:(dispatch_queue_t)queue
minInterval:(NSNumber *)minInterval
maxInterval:(NSNumber *)maxInterval
params:(MTRSubscribeParams * _Nullable)params
params:(MTRSubscribeParams *)params
attributeCacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
Expand Down Expand Up @@ -254,9 +252,7 @@ extern NSString * const MTRArrayValueType;
- (void)subscribeAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
minInterval:(NSNumber *)minInterval
maxInterval:(NSNumber *)maxInterval
params:(MTRSubscribeParams * _Nullable)params
params:(MTRSubscribeParams *)params
queue:(dispatch_queue_t)queue
reportHandler:(MTRDeviceResponseHandler)reportHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished;
Expand Down
33 changes: 13 additions & 20 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,7 @@ - (void)invalidateCASESession
} // anonymous namespace

- (void)subscribeWithQueue:(dispatch_queue_t)queue
minInterval:(NSNumber *)minInterval
maxInterval:(NSNumber *)maxInterval
params:(MTRSubscribeParams * _Nullable)params
params:(MTRSubscribeParams *)params
attributeCacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
Expand Down Expand Up @@ -335,13 +333,14 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue
auto attributePath = std::make_unique<AttributePathParams>();
auto eventPath = std::make_unique<EventPathParams>();
ReadPrepareParams readParams(session.Value());
readParams.mMinIntervalFloorSeconds = [minInterval unsignedShortValue];
readParams.mMaxIntervalCeilingSeconds = [maxInterval unsignedShortValue];
readParams.mMinIntervalFloorSeconds = [params.minInterval unsignedShortValue];
readParams.mMaxIntervalCeilingSeconds = [params.maxInterval unsignedShortValue];
readParams.mpAttributePathParamsList = attributePath.get();
readParams.mAttributePathParamsListSize = 1;
readParams.mpEventPathParamsList = eventPath.get();
readParams.mEventPathParamsListSize = 1;
readParams.mKeepSubscriptions = [params.keepPreviousSubscriptions boolValue];
readParams.mIsFabricFiltered = params.fabricFiltered;
readParams.mKeepSubscriptions = params.keepPreviousSubscriptions;

std::unique_ptr<SubscriptionCallback> callback;
std::unique_ptr<ReadClient> readClient;
Expand All @@ -366,7 +365,7 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue
}

CHIP_ERROR err;
if (params != nil && params.autoResubscribe != nil && ![params.autoResubscribe boolValue]) {
if (!params.autoResubscribe) {
err = readClient->SendRequest(readParams);
} else {
// SendAutoResubscribeRequest cleans up the params, even on failure.
Expand Down Expand Up @@ -831,7 +830,7 @@ new MTRDataValueDictionaryCallbackBridge(queue, self, completion,
chip::app::ReadPrepareParams readParams(session);
readParams.mpAttributePathParamsList = &attributePath;
readParams.mAttributePathParamsListSize = 1;
readParams.mIsFabricFiltered = params == nil || params.fabricFiltered == nil || [params.fabricFiltered boolValue];
readParams.mIsFabricFiltered = params.fabricFiltered;

auto onDone = [resultArray, resultSuccess, resultFailure, context, successCb, failureCb](
BufferedReadAttributeCallback<MTRDataValueDictionaryDecodableType> * callback) {
Expand Down Expand Up @@ -1117,9 +1116,7 @@ new MTRDataValueDictionaryCallbackBridge(queue, self, completion,
- (void)subscribeAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
minInterval:(NSNumber *)minInterval
maxInterval:(NSNumber *)maxInterval
params:(MTRSubscribeParams * _Nullable)params
params:(MTRSubscribeParams *)params
queue:(dispatch_queue_t)queue
reportHandler:(MTRDeviceResponseHandler)reportHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler)subscriptionEstablished
Expand All @@ -1136,8 +1133,6 @@ - (void)subscribeAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
endpointID = (endpointID == nil) ? nil : [endpointID copy];
clusterID = (clusterID == nil) ? nil : [clusterID copy];
attributeID = (attributeID == nil) ? nil : [attributeID copy];
minInterval = (minInterval == nil) ? nil : [minInterval copy];
maxInterval = (maxInterval == nil) ? nil : [maxInterval copy];
params = (params == nil) ? nil : [params copy];

[self.deviceController
Expand Down Expand Up @@ -1211,12 +1206,10 @@ - (void)subscribeAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
chip::app::ReadPrepareParams readParams(session.Value());
readParams.mpAttributePathParamsList = container.pathParams;
readParams.mAttributePathParamsListSize = 1;
readParams.mMinIntervalFloorSeconds = static_cast<uint16_t>([minInterval unsignedShortValue]);
readParams.mMaxIntervalCeilingSeconds = static_cast<uint16_t>([maxInterval unsignedShortValue]);
readParams.mIsFabricFiltered
= (params == nil || params.fabricFiltered == nil || [params.fabricFiltered boolValue]);
readParams.mKeepSubscriptions
= (params != nil && params.keepPreviousSubscriptions != nil && [params.keepPreviousSubscriptions boolValue]);
readParams.mMinIntervalFloorSeconds = static_cast<uint16_t>([params.minInterval unsignedShortValue]);
readParams.mMaxIntervalCeilingSeconds = static_cast<uint16_t>([params.maxInterval unsignedShortValue]);
readParams.mIsFabricFiltered = params.fabricFiltered;
readParams.mKeepSubscriptions = params.keepPreviousSubscriptions;

auto onDone = [container](BufferedReadAttributeCallback<MTRDataValueDictionaryDecodableType> * callback) {
[container onDone];
Expand All @@ -1232,7 +1225,7 @@ - (void)subscribeAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
auto readClient = Platform::New<app::ReadClient>(
engine, exchangeManager, callback->GetBufferedCallback(), chip::app::ReadClient::InteractionType::Subscribe);

if (params != nil && params.autoResubscribe != nil && ![params.autoResubscribe boolValue]) {
if (!params.autoResubscribe) {
err = readClient->SendRequest(readParams);
} else {
err = readClient->SendAutoResubscribeRequest(std::move(readParams));
Expand Down
Loading

0 comments on commit 72d838a

Please sign in to comment.