Skip to content

Commit

Permalink
Remove un-named callbacks and raw integers from Darwin framework API. (
Browse files Browse the repository at this point in the history
…#23364)

Switches to typedefs for all the callbacks and using NSNumber for numbers,
except in MTRDeviceController (and the XPC version), which will need some more
complicated changes.

This is a re-landing of PR #22551 but modified to preserve the old APIs, plus
PR #22843 (the parts that had not relanded yet).

The header changes not accompanied by backwards-compat shims are OK for the
following reasons:

* MTRAttributeCacheContainer+XPC.h is not public API.
* The change to MTRAttributeCacheContainer.h is both source and binary
  compatible.
* MTRAttributeCacheContainer_Internal.h is not public API.
* The change to MTRBaseDevice's subscribeAttributeWithEndpointId is both source
  and binary compatible.
* The change to MTRBaseDevice's deregisterReportHandlersWithClientQueue is both
  source and binary compatible.
* The change to sharedControllerWithId in MTRDeviceController+XPC.h is both
  source and binary compatible.
* The change to getAnyDeviceControllerWithCompletion in
  MTRDeviceController+XPC.h is both source and binary compatible.
* MTRDeviceControllerOverXPC.h is not public API.
* MTRDeviceControllerXPCConnection.h is not public API.
* MTRDeviceController_Internal.h is not public API.
* MTRDevice_Internal.h is not public API.
* MTRDeviceOverXPC.h is not public API.
* The changes to MTROTAProviderDelegate.h are both source and binary compatible.

* Address review comments.
  • Loading branch information
bzbarsky-apple authored Oct 28, 2022
1 parent 8d7669d commit 09acc29
Show file tree
Hide file tree
Showing 76 changed files with 9,990 additions and 6,710 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ class SubscribeEvent : public ModelCommand {
params.autoResubscribe = mAutoResubscribe.HasValue() ? [NSNumber numberWithBool:mAutoResubscribe.Value()] : nil;

[device subscribeWithQueue:callbackQueue
minInterval:mMinInterval
maxInterval:mMaxInterval
minInterval:@(mMinInterval)
maxInterval:@(mMaxInterval)
params:params
cacheContainer:nil
attributeCacheContainer:nil
attributeReportHandler:^(NSArray * value) {
SetCommandExitStatus(CHIP_NO_ERROR);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
constexpr const char * identities[] = { kIdentityAlpha, kIdentityBeta, kIdentityGamma };
for (size_t i = 0; i < ArraySize(identities); ++i) {
auto controllerParams = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:gNocSigner
fabricId:(i + 1)
fabricID:@(i + 1)
ipk:ipk];

// We're not sure whether we're creating a new fabric or using an
Expand Down Expand Up @@ -201,7 +201,7 @@
constexpr const char * identities[] = { kIdentityAlpha, kIdentityBeta, kIdentityGamma };
for (size_t i = 0; i < ArraySize(identities); ++i) {
auto controllerParams = [[MTRDeviceControllerStartupParams alloc] initWithSigningKeypair:gNocSigner
fabricId:(i + 1)
fabricID:@(i + 1)
ipk:ipk];

auto controller = [factory startControllerOnExistingFabric:controllerParams];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

auto * self = this;
if (mCommissioningWindowOption == 0) {
auto * cluster = [[MTRClusterAdministratorCommissioning alloc] initWithDevice:device endpoint:0 queue:mWorkQueue];
auto * cluster = [[MTRClusterAdministratorCommissioning alloc] initWithDevice:device endpointID:@(0) queue:mWorkQueue];
auto * params = [[MTRAdministratorCommissioningClusterOpenBasicCommissioningWindowParams alloc] init];
params.commissioningTimeout = @(mCommissioningWindowTimeoutMs);
params.timedInvokeTimeoutMs = @(10000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
} else {
ChipLogProgress(chipTool, "Attempting to unpair device %llu", mNodeId);
MTRBaseClusterOperationalCredentials * opCredsCluster =
[[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpoint:0 queue:callbackQueue];
[[MTRBaseClusterOperationalCredentials alloc] initWithDevice:device endpointID:@(0) queue:callbackQueue];
[opCredsCluster readAttributeCurrentFabricIndexWithCompletionHandler:^(
NSNumber * _Nullable value, NSError * _Nullable readError) {
if (readError) {
Expand Down
8 changes: 4 additions & 4 deletions examples/darwin-framework-tool/templates/commands.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public:
ChipLogProgress(chipTool, "Sending cluster ({{asHex parent.code 8}}) command ({{asHex code 8}}) on endpoint %u", endpointId);

dispatch_queue_t callbackQueue = dispatch_queue_create("com.chip.command", DISPATCH_QUEUE_SERIAL);
MTRBaseCluster{{asUpperCamelCase clusterName}} * cluster = [[MTRBaseCluster{{asUpperCamelCase clusterName}} alloc] initWithDevice:device endpoint:endpointId queue:callbackQueue];
MTRBaseCluster{{asUpperCamelCase clusterName}} * cluster = [[MTRBaseCluster{{asUpperCamelCase clusterName}} alloc] initWithDevice:device endpointID:@(endpointId) queue:callbackQueue];
__auto_type * params = [[MTR{{asUpperCamelCase clusterName}}Cluster{{asUpperCamelCase name}}Params alloc] init];
params.timedInvokeTimeoutMs = mTimedInteractionTimeoutMs.HasValue() ? [NSNumber numberWithUnsignedShort:mTimedInteractionTimeoutMs.Value()] : nil;
{{#chip_cluster_command_arguments}}
Expand Down Expand Up @@ -111,7 +111,7 @@ public:
ChipLogProgress(chipTool, "Sending cluster ({{asHex parent.code 8}}) ReadAttribute ({{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];
MTRBaseCluster{{asUpperCamelCase parent.name}} * cluster = [[MTRBaseCluster{{asUpperCamelCase parent.name}} alloc] initWithDevice:device endpointID:@(endpointId) queue:callbackQueue];
{{#if_is_fabric_scoped_struct type}}
MTRReadParams * params = [[MTRReadParams alloc] init];
params.fabricFiltered = mFabricFiltered.HasValue() ? [NSNumber numberWithBool:mFabricFiltered.Value()] : nil;
Expand Down Expand Up @@ -161,7 +161,7 @@ public:
{
ChipLogProgress(chipTool, "Sending cluster ({{asHex parent.code 8}}) WriteAttribute ({{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];
MTRBaseCluster{{asUpperCamelCase parent.name}} * cluster = [[MTRBaseCluster{{asUpperCamelCase parent.name}} alloc] initWithDevice:device endpointID:@(endpointId) queue:callbackQueue];
MTRWriteParams * params = [[MTRWriteParams alloc] init];
params.timedWriteTimeout = mTimedInteractionTimeoutMs.HasValue() ? [NSNumber numberWithUnsignedShort:mTimedInteractionTimeoutMs.Value()] : nil;
params.dataVersion = mDataVersion.HasValue() ? [NSNumber numberWithUnsignedInt:mDataVersion.Value()] : nil;
Expand Down Expand Up @@ -215,7 +215,7 @@ 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];
MTRBaseCluster{{asUpperCamelCase parent.name}} * cluster = [[MTRBaseCluster{{asUpperCamelCase parent.name}} alloc] initWithDevice:device endpointID:@(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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class {{filename}}: public TestCommandBridge
return {{command}}("{{identity}}", value);
{{else}}
MTRBaseDevice * device = GetDevice("{{identity}}");
MTRBaseCluster{{asUpperCamelCase cluster}} * cluster = [[MTRBaseCluster{{asUpperCamelCase cluster}} alloc] initWithDevice:device endpoint:{{endpoint}} queue:mCallbackQueue];
MTRBaseCluster{{asUpperCamelCase cluster}} * cluster = [[MTRBaseCluster{{asUpperCamelCase cluster}} alloc] initWithDevice:device endpointID:@({{endpoint}}) queue:mCallbackQueue];
VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE);

{{#if isCommand}}
Expand Down
2 changes: 2 additions & 0 deletions src/app/zap-templates/partials/header.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@
* limitations under the License.
*/

{{#unless excludeZapComment}}
// THIS FILE IS GENERATED BY ZAP
{{/unless}}
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRAttributeCacheContainer+XPC.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface MTRAttributeCacheContainer (XPC)
- (void)setXPCConnection:(MTRDeviceControllerXPCConnection *)xpcConnection
controllerId:(id<NSCopying>)controllerId
deviceId:(uint64_t)deviceId;
deviceId:(NSNumber *)deviceId;
@end

NS_ASSUME_NONNULL_END
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTRAttributeCacheContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import <Foundation/Foundation.h>

#import <Matter/MTRBaseDevice.h>
#import <Matter/MTRDeviceController.h>

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -40,8 +41,7 @@ NS_ASSUME_NONNULL_BEGIN
clusterId:(NSNumber * _Nullable)clusterId
attributeId:(NSNumber * _Nullable)attributeId
clientQueue:(dispatch_queue_t)clientQueue
completion:(void (^)(NSArray<NSDictionary<NSString *, id> *> * _Nullable values,
NSError * _Nullable error))completion;
completion:(MTRDeviceResponseHandler)completion;

@end

Expand Down
9 changes: 4 additions & 5 deletions src/darwin/Framework/CHIP/MTRAttributeCacheContainer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ - (instancetype)init

- (void)setXPCConnection:(MTRDeviceControllerXPCConnection *)xpcConnection
controllerId:(id<NSCopying>)controllerId
deviceId:(uint64_t)deviceId
deviceId:(NSNumber *)deviceId
{
self.xpcConnection = xpcConnection;
self.xpcControllerId = controllerId;
Expand Down Expand Up @@ -82,8 +82,7 @@ - (void)readAttributeWithEndpointId:(NSNumber * _Nullable)endpointId
clusterId:(NSNumber * _Nullable)clusterId
attributeId:(NSNumber * _Nullable)attributeId
clientQueue:(dispatch_queue_t)clientQueue
completion:(void (^)(NSArray<NSDictionary<NSString *, id> *> * _Nullable values,
NSError * _Nullable error))completion
completion:(MTRDeviceResponseHandler)completion
{
__auto_type completionHandler = ^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
dispatch_async(clientQueue, ^{
Expand All @@ -99,12 +98,12 @@ - (void)readAttributeWithEndpointId:(NSNumber * _Nullable)endpointId
return;
}
__auto_type controllerId = self.xpcControllerId;
uint64_t nodeId = self.deviceId;
NSNumber * nodeId = self.deviceId;
[xpcConnection
getProxyHandleWithCompletion:^(dispatch_queue_t _Nonnull queue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
if (handle) {
[handle.proxy readAttributeCacheWithController:controllerId
nodeId:nodeId
nodeId:nodeId.unsignedLongLongValue
endpointId:endpointId
clusterId:clusterId
attributeId:attributeId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface MTRAttributeCacheContainer ()

@property (atomic, readwrite, nullable) chip::app::ClusterStateCache * cppAttributeCache;
@property (nonatomic, readwrite) uint64_t deviceId;
@property (nonatomic, readwrite, copy) NSNumber * deviceId;
@property (nonatomic, readwrite, weak, nullable) MTRDeviceControllerXPCConnection * xpcConnection;
@property (nonatomic, readwrite, strong, nullable) id<NSCopying> xpcControllerId;
@property (atomic, readwrite) BOOL shouldUseXPC;
Expand Down
33 changes: 27 additions & 6 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,15 @@ extern NSString * const MTRArrayValueType;
* attempts fail.
*/
- (void)subscribeWithQueue:(dispatch_queue_t)queue
minInterval:(uint16_t)minInterval
maxInterval:(uint16_t)maxInterval
minInterval:(NSNumber *)minInterval
maxInterval:(NSNumber *)maxInterval
params:(MTRSubscribeParams * _Nullable)params
cacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeCacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(MTRDeviceErrorHandler)errorHandler
subscriptionEstablished:(dispatch_block_t _Nullable)subscriptionEstablishedHandler
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduledHandler;
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduledHandler MTR_NEWLY_AVAILABLE;

/**
* Read attribute in a designated attribute path
Expand Down Expand Up @@ -235,7 +235,7 @@ extern NSString * const MTRArrayValueType;
params:(MTRSubscribeParams * _Nullable)params
clientQueue:(dispatch_queue_t)clientQueue
reportHandler:(MTRDeviceResponseHandler)reportHandler
subscriptionEstablished:(nullable void (^)(void))subscriptionEstablishedHandler;
subscriptionEstablished:(dispatch_block_t _Nullable)subscriptionEstablishedHandler;

/**
* Deregister all local report handlers for a remote device
Expand All @@ -244,7 +244,7 @@ extern NSString * const MTRArrayValueType;
* There could be multiple clients accessing a node through a remote controller object and hence it is not appropriate
* for one of those clients to shut down the entire stack to stop receiving reports.
*/
- (void)deregisterReportHandlersWithClientQueue:(dispatch_queue_t)clientQueue completion:(void (^)(void))completion;
- (void)deregisterReportHandlersWithClientQueue:(dispatch_queue_t)clientQueue completion:(dispatch_block_t)completion;

/**
* Open a commissioning window on the device.
Expand All @@ -269,6 +269,27 @@ extern NSString * const MTRArrayValueType;

@end

@interface MTRBaseDevice (Deprecated)

/**
* Deprecated MTRBaseDevice APIs.
*/
- (void)subscribeWithQueue:(dispatch_queue_t)queue
minInterval:(uint16_t)minInterval
maxInterval:(uint16_t)maxInterval
params:(MTRSubscribeParams * _Nullable)params
cacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(MTRDeviceErrorHandler)errorHandler
subscriptionEstablished:(dispatch_block_t _Nullable)subscriptionEstablishedHandler
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduledHandler
MTR_NEWLY_DEPRECATED("Please use "
"subscribeWithQueue:params:attributeCacheContainer:attributeReportHandler:eventReportHandler:errorHandler:"
"subscriptionEstablished:resubscriptionScheduled:");

@end

@interface MTRAttributePath : NSObject <NSCopying>
@property (nonatomic, readonly, strong, nonnull) NSNumber * endpoint;
@property (nonatomic, readonly, strong, nonnull) NSNumber * cluster;
Expand Down
49 changes: 38 additions & 11 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,11 @@ - (instancetype)initWithPASEDevice:(chip::DeviceProxy *)device controller:(MTRDe
return self;
}

- (instancetype)initWithNodeID:(chip::NodeId)nodeID controller:(MTRDeviceController *)controller
- (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller
{
if (self = [super init]) {
_isPASEDevice = NO;
_nodeID = nodeID;
_nodeID = nodeID.unsignedLongLongValue;
_deviceController = controller;
}
return self;
Expand Down Expand Up @@ -260,14 +260,14 @@ - (void)invalidateCASESession
} // anonymous namespace

- (void)subscribeWithQueue:(dispatch_queue_t)queue
minInterval:(uint16_t)minInterval
maxInterval:(uint16_t)maxInterval
minInterval:(NSNumber *)minInterval
maxInterval:(NSNumber *)maxInterval
params:(nullable MTRSubscribeParams *)params
cacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(nullable void (^)(NSArray * value))attributeReportHandler
eventReportHandler:(nullable void (^)(NSArray * value))eventReportHandler
attributeCacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(void (^)(NSError * error))errorHandler
subscriptionEstablished:(nullable void (^)(void))subscriptionEstablishedHandler
subscriptionEstablished:(dispatch_block_t _Nullable)subscriptionEstablishedHandler
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduledHandler
{
if (self.isPASEDevice) {
Expand Down Expand Up @@ -297,8 +297,8 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue
// We want to get event reports at the minInterval, not the maxInterval.
eventPath->mIsUrgentEvent = true;
ReadPrepareParams readParams(session.Value());
readParams.mMinIntervalFloorSeconds = minInterval;
readParams.mMaxIntervalCeilingSeconds = maxInterval;
readParams.mMinIntervalFloorSeconds = [minInterval unsignedShortValue];
readParams.mMaxIntervalCeilingSeconds = [maxInterval unsignedShortValue];
readParams.mpAttributePathParamsList = attributePath.get();
readParams.mAttributePathParamsListSize = 1;
readParams.mpEventPathParamsList = eventPath.get();
Expand Down Expand Up @@ -1256,7 +1256,7 @@ - (void)subscribeAttributeWithEndpointId:(NSNumber * _Nullable)endpointId
}];
}

- (void)deregisterReportHandlersWithClientQueue:(dispatch_queue_t)clientQueue completion:(void (^)(void))completion
- (void)deregisterReportHandlersWithClientQueue:(dispatch_queue_t)clientQueue completion:(dispatch_block_t)completion
{
// This method must only be used for MTRDeviceOverXPC. However, for unit testing purpose, the method purges all read clients.
MTR_LOG_DEBUG("Unexpected call to deregister report handlers");
Expand Down Expand Up @@ -1452,6 +1452,33 @@ + (id)CHIPEncodeAndDecodeNSObject:(id)object

@end

@implementation MTRBaseDevice (Deprecated)

- (void)subscribeWithQueue:(dispatch_queue_t)queue
minInterval:(uint16_t)minInterval
maxInterval:(uint16_t)maxInterval
params:(MTRSubscribeParams * _Nullable)params
cacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(MTRDeviceErrorHandler)errorHandler
subscriptionEstablished:(dispatch_block_t _Nullable)subscriptionEstablishedHandler
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduledHandler
{
[self subscribeWithQueue:queue
minInterval:@(minInterval)
maxInterval:@(maxInterval)
params:params
attributeCacheContainer:attributeCacheContainer
attributeReportHandler:attributeReportHandler
eventReportHandler:eventReportHandler
errorHandler:errorHandler
subscriptionEstablished:subscriptionEstablishedHandler
resubscriptionScheduled:resubscriptionScheduledHandler];
}

@end

@implementation MTRAttributePath
- (instancetype)initWithPath:(const ConcreteDataAttributePath &)path
{
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ NS_ASSUME_NONNULL_BEGIN
* the controller's fabric, but attempts to actually use the MTRBaseDevice will
* fail (asynchronously) in that case.
*/
- (instancetype)initWithNodeID:(chip::NodeId)nodeID controller:(MTRDeviceController *)controller;
- (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller;

@end

Expand Down
Loading

0 comments on commit 09acc29

Please sign in to comment.