Skip to content

Commit

Permalink
Apply API review fixes for MTRReadParams and MTRSubscribeParams.
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 committed Sep 13, 2022
1 parent 9c131da commit 967dc7d
Show file tree
Hide file tree
Showing 22 changed files with 20,567 additions and 27,954 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
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
37 changes: 28 additions & 9 deletions src/darwin/Framework/CHIP/MTRCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,15 @@ NS_ASSUME_NONNULL_BEGIN
@interface MTRReadParams : NSObject <NSCopying>

/**
* Whether the read/subscribe is fabric-filtered. nil (the default value) is
* treated as YES.
* Whether the read/subscribe is fabric-filtered. The default is YES.
*
* If YES, the read/subscribe is fabric-filtered and will only see things
* associated with the fabric of the reader/subscriber.
*
* If NO, the read/subscribe is not fabric-filtered and will see all
* non-fabric-sensitive data for the given attribute path.
*/
@property (nonatomic, copy, nullable) NSNumber * fabricFiltered;
@property (nonatomic, assign) BOOL fabricFiltered;

@end

Expand All @@ -100,18 +99,18 @@ NS_ASSUME_NONNULL_BEGIN

/**
* Whether the subscribe should allow previous subscriptions to stay in
* place. nil (the default value) is treated as NO.
* place. The default value is NO.
*
* If NO, the subscribe will cancel any existing subscriptions to the target
* node when it sets up the new one.
*
* If YES, the subscribe will allow any previous subscriptions to remain.
*/
@property (nonatomic, copy, nullable) NSNumber * keepPreviousSubscriptions;
@property (nonatomic, assign) BOOL keepPreviousSubscriptions;

/**
* Whether the subscription should automatically try to re-establish if it
* drops. nil (the default value) is treated as YES.
* drops. The default value is YES.
*
* If NO, loss of subscription will simply lead to an error report. Some
* subscription APIs do not support this value.
Expand All @@ -121,11 +120,31 @@ NS_ASSUME_NONNULL_BEGIN
* called again.
*
*/
@property (nonatomic, copy, nullable) NSNumber * autoResubscribe;
@property (nonatomic, assign) BOOL autoResubscribe;

- (instancetype)init;
- (id)copyWithZone:(NSZone * _Nullable)zone;
/**
* The minimum time, in seconds, between consecutive reports a server will send
* for this subscription. This can be used to rate-limit the subscription
* traffic. Any non-negative value is allowed, including 0.
*/
@property (nonatomic, copy) NSNumber * minInterval;

/**
* The suggested maximum time, in seconds, during which the server is allowed to
* send no reports at all for this subscription. Must be at least as large as
* minInterval. The server is allowed to use a larger time than this as the
* maxInterval it selects if it needs to (e.g. to meet its power budget).
*/
@property (nonatomic, copy) NSNumber * maxInterval;

/**
* Initialize an MTRSubscribeParams. Must provide a minInterval and
* maxInterval; there are no default values for those.
*/
- (instancetype)initWithMinInterval:(NSNumber *)minInterval maxInterval:(NSNumber *)maxInterval;

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
@end

NS_ASSUME_NONNULL_END
Loading

0 comments on commit 967dc7d

Please sign in to comment.