Skip to content

Commit

Permalink
Use "copy" ownership for members of params structs on Darwin where po…
Browse files Browse the repository at this point in the history
…ssible. (#21392)

Changes:

1. Makes all the Matter payload/struct/event structs implement NSCopying.
2. Uses "copy" for members of various params structs that are objects and not
   instances of some protocol, and "assign" for ones that are non-ARC types.
3. Somewhat aligns properties in params structs to list nonatomic first, then
   ownership, then readonly if the property is readonly, then nullable as
   needed.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Aug 30, 2023
1 parent aa67472 commit 540f3c6
Show file tree
Hide file tree
Showing 14 changed files with 6,082 additions and 1,857 deletions.
12 changes: 6 additions & 6 deletions src/darwin/Framework/CHIP/MTRCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ NS_ASSUME_NONNULL_BEGIN
@end

/**
* CHIPWriteParams
* MTRWriteParams
* This is used to control the behavior of cluster writes.
* If not provided (i.e. nil passed for the CHIPWriteParams argument), will be
* treated as if a default-initialized object was passed in.
Expand All @@ -54,15 +54,15 @@ NS_ASSUME_NONNULL_BEGIN
* This value is specified in milliseconds
*
*/
@property (strong, nonatomic, nullable) NSNumber * timedWriteTimeout;
@property (nonatomic, copy, nullable) NSNumber * timedWriteTimeout;

/**
* Sets the data version for the Write Request for the interaction.
*
* If not nil, the write will only succeed if the current data version of
* the cluster matches the provided data version.
*/
@property (strong, nonatomic, nullable) NSNumber * dataVersion;
@property (nonatomic, copy, nullable) NSNumber * dataVersion;

- (instancetype)init;

Expand All @@ -86,7 +86,7 @@ NS_ASSUME_NONNULL_BEGIN
* If NO, the read/subscribe is not fabric-filtered and will see all
* non-fabric-sensitive data for the given attribute path.
*/
@property (strong, nonatomic, nullable) NSNumber * fabricFiltered;
@property (nonatomic, copy, nullable) NSNumber * fabricFiltered;

- (instancetype)init;

Expand All @@ -109,7 +109,7 @@ NS_ASSUME_NONNULL_BEGIN
*
* If YES, the subscribe will allow any previous subscriptions to remain.
*/
@property (strong, nonatomic, nullable) NSNumber * keepPreviousSubscriptions;
@property (nonatomic, copy, nullable) NSNumber * keepPreviousSubscriptions;

/**
* Whether the subscription should automatically try to re-establish if it
Expand All @@ -123,7 +123,7 @@ NS_ASSUME_NONNULL_BEGIN
* called again.
*
*/
@property (strong, nonatomic, nullable) NSNumber * autoResubscribe;
@property (nonatomic, copy, nullable) NSNumber * autoResubscribe;

- (instancetype)init;

Expand Down
14 changes: 7 additions & 7 deletions src/darwin/Framework/CHIP/MTRCommissioningParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,31 @@ NS_ASSUME_NONNULL_BEGIN
/**
* The CSRNonce
*/
@property (nonatomic, nullable, copy, readwrite) NSData * CSRNonce;
@property (nonatomic, copy, nullable) NSData * CSRNonce;
/**
* The AttestationNonce
*/
@property (nonatomic, nullable, copy, readwrite) NSData * attestationNonce;
@property (nonatomic, copy, nullable) NSData * attestationNonce;
/**
* The Wi-Fi SSID
*/
@property (nonatomic, nullable, copy, readwrite) NSData * wifiSSID;
@property (nonatomic, copy, nullable) NSData * wifiSSID;
/**
* The Wi-Fi Credentials
*/
@property (nonatomic, nullable, copy, readwrite) NSData * wifiCredentials;
@property (nonatomic, copy, nullable) NSData * wifiCredentials;
/**
* The Thread operational dataset
*/
@property (nonatomic, nullable, copy, readwrite) NSData * threadOperationalDataset;
@property (nonatomic, copy, nullable) NSData * threadOperationalDataset;
/**
* The Device Attestation status delegate
*/
@property (nonatomic, nullable, strong, readwrite) id<MTRDeviceAttestationDelegate> deviceAttestationDelegate;
@property (nonatomic, strong, nullable) id<MTRDeviceAttestationDelegate> deviceAttestationDelegate;
/**
* The timeout in secs to set for fail-safe when attestation fails
*/
@property (nonatomic, nullable, copy, readwrite) NSNumber * failSafeExpiryTimeoutSecs;
@property (nonatomic, copy, nullable) NSNumber * failSafeExpiryTimeoutSecs;

@end

Expand Down
10 changes: 5 additions & 5 deletions src/darwin/Framework/CHIP/MTRControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,30 @@ NS_ASSUME_NONNULL_BEGIN
* controllers. It is used to store persistent information for the fabrics the
* controllers ends up interacting with.
*/
@property (strong, nonatomic, readonly) id<MTRPersistentStorageDelegate> storageDelegate;
@property (nonatomic, strong, readonly) id<MTRPersistentStorageDelegate> storageDelegate;

/*
* OTA Provider delegate to be called when an OTA Requestor is requesting a software update.
* Defaults to nil.
*/
@property (strong, nonatomic, nullable) id<MTROTAProviderDelegate> otaProviderDelegate;
@property (nonatomic, strong, nullable) id<MTROTAProviderDelegate> otaProviderDelegate;

/*
* The Product Attestation Authority certificates that are trusted to sign
* device attestation information. Defaults to nil.
*
*/
@property (strong, nonatomic, nullable) NSArray<NSData *> * paaCerts;
@property (nonatomic, copy, nullable) NSArray<NSData *> * paaCerts;
/*
* The network port to bind to. If not specified, an ephemeral port will be
* used.
*/
@property (strong, nonatomic, nullable) NSNumber * port;
@property (nonatomic, copy, nullable) NSNumber * port;
/*
* Whether to run a server capable of accepting incoming CASE
* connections. Defaults to NO.
*/
@property (nonatomic) BOOL startServer;
@property (nonatomic, assign) BOOL startServer;

- (instancetype)init NS_UNAVAILABLE;
- (instancetype)initWithStorage:(id<MTRPersistentStorageDelegate>)storageDelegate;
Expand Down
18 changes: 9 additions & 9 deletions src/darwin/Framework/CHIP/MTRDeviceControllerStartupParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ NS_ASSUME_NONNULL_BEGIN
* must be initialized using initWithOperationalKeypair (to provide the
* operational credentials for the controller itself).
*/
@property (strong, nonatomic, readonly, nullable) id<MTRKeypair> nocSigner;
@property (nonatomic, copy, readonly, nullable) id<MTRKeypair> nocSigner;
/**
* Fabric id for the controller. Must be set to a nonzero value. This is
* scoped by the root public key, which is determined as follows:
Expand All @@ -42,13 +42,13 @@ NS_ASSUME_NONNULL_BEGIN
* key of the nocSigner keypair, since in this case we are not using an
* intermediate certificate.
*/
@property (nonatomic, readonly) uint64_t fabricId;
@property (nonatomic, assign, readonly) uint64_t fabricId;
/**
* IPK to use for the controller's fabric. Allowed to change from the last time
* a controller was started on this fabric if a new IPK has been distributed to
* all the devices the controller wants to interact with.
*/
@property (strong, nonatomic, readonly) NSData * ipk;
@property (nonatomic, copy, readonly) NSData * ipk;

/**
* Vendor ID (allocated by the Connectivity Standards Alliance) for
Expand All @@ -65,7 +65,7 @@ NS_ASSUME_NONNULL_BEGIN
* * Will override existing value if not nil. Otherwise existing value will be
* used.
*/
@property (strong, nonatomic, nullable) NSNumber * vendorId;
@property (nonatomic, copy, nullable) NSNumber * vendorId;

/**
* Node id for this controller.
Expand Down Expand Up @@ -95,7 +95,7 @@ NS_ASSUME_NONNULL_BEGIN
* generated operational key.
*
*/
@property (strong, nonatomic, nullable) NSNumber * nodeId;
@property (nonatomic, copy, nullable) NSNumber * nodeId;

// TODO: Add something here for CATs?

Expand Down Expand Up @@ -130,7 +130,7 @@ NS_ASSUME_NONNULL_BEGIN
* 2) The subject DN must match the subject DN of the existing root
* certificate.
*/
@property (strong, nonatomic, nullable) NSData * rootCertificate;
@property (nonatomic, copy, nullable) NSData * rootCertificate;

/**
* Intermediate certificate, in X.509 DER form, to use.
Expand Down Expand Up @@ -162,7 +162,7 @@ NS_ASSUME_NONNULL_BEGIN
* allows switching from using an intermediate CA to not using one.
*
*/
@property (strong, nonatomic, nullable) NSData * intermediateCertificate;
@property (nonatomic, copy, nullable) NSData * intermediateCertificate;

/**
* Operational certificate, in X.509 DER form, to use.
Expand All @@ -173,7 +173,7 @@ NS_ASSUME_NONNULL_BEGIN
* If nil, an operational certificate will be determined as described in the
* documentation for nodeId.
*/
@property (strong, nonatomic, nullable, readonly) NSData * operationalCertificate;
@property (nonatomic, copy, readonly, nullable) NSData * operationalCertificate;

/**
* Operational keypair to use. If operationalCertificate is not nil, the public
Expand All @@ -184,7 +184,7 @@ NS_ASSUME_NONNULL_BEGIN
* will for that certificated will be determined as described in the
* documentation for nodeId.
*/
@property (strong, nonatomic, nullable) id<MTRKeypair> operationalKeypair;
@property (nonatomic, strong, nullable) id<MTRKeypair> operationalKeypair;

- (instancetype)init NS_UNAVAILABLE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface MTRDeviceControllerStartupParams ()
// We want to be able to write to operationalCertificate in
// MTRDeviceControllerStartupParamsInternal.
@property (strong, nonatomic, nullable) NSData * operationalCertificate;
@property (nonatomic, copy, nullable) NSData * operationalCertificate;

// Init method that just copies the values of all our ivars.
- (instancetype)initWithParams:(MTRDeviceControllerStartupParams *)params;
Expand All @@ -45,14 +45,14 @@ NS_ASSUME_NONNULL_BEGIN
@interface MTRDeviceControllerStartupParamsInternal : MTRDeviceControllerStartupParams

// Fabric table we can use to do things like allocate operational keys.
@property (nonatomic, readonly) chip::FabricTable * fabricTable;
@property (nonatomic, assign, readonly) chip::FabricTable * fabricTable;

// Fabric index we're starting on. Only has a value when starting on an
// existing fabric.
@property (nonatomic, readonly) chip::Optional<chip::FabricIndex> fabricIndex;
@property (nonatomic, assign, readonly) chip::Optional<chip::FabricIndex> fabricIndex;

// Key store we're using with our fabric table, for sanity checks.
@property (nonatomic, readonly) chip::Crypto::OperationalKeystore * keystore;
@property (nonatomic, assign, readonly) chip::Crypto::OperationalKeystore * keystore;

/**
* Helper method that checks that our keypairs match our certificates.
Expand Down
22 changes: 11 additions & 11 deletions src/darwin/Framework/CHIP/MTRSetupPayload.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,24 @@ typedef NS_ENUM(NSUInteger, MTROptionalQRCodeInfoType) {
};

@interface MTROptionalQRCodeInfo : NSObject
@property (nonatomic, strong) NSNumber * infoType;
@property (nonatomic, strong) NSNumber * tag;
@property (nonatomic, strong) NSNumber * integerValue;
@property (nonatomic, strong) NSString * stringValue;
@property (nonatomic, copy) NSNumber * infoType;
@property (nonatomic, copy) NSNumber * tag;
@property (nonatomic, copy) NSNumber * integerValue;
@property (nonatomic, copy) NSString * stringValue;
@end

@interface MTRSetupPayload : NSObject

@property (nonatomic, strong) NSNumber * version;
@property (nonatomic, strong) NSNumber * vendorID;
@property (nonatomic, strong) NSNumber * productID;
@property (nonatomic, copy) NSNumber * version;
@property (nonatomic, copy) NSNumber * vendorID;
@property (nonatomic, copy) NSNumber * productID;
@property (nonatomic, assign) MTRCommissioningFlow commissioningFlow;
@property (nonatomic, assign) MTRRendezvousInformationFlags rendezvousInformation;
@property (nonatomic, strong) NSNumber * discriminator;
@property (nonatomic) BOOL hasShortDiscriminator;
@property (nonatomic, strong) NSNumber * setUpPINCode;
@property (nonatomic, copy) NSNumber * discriminator;
@property (nonatomic, assign) BOOL hasShortDiscriminator;
@property (nonatomic, copy) NSNumber * setUpPINCode;

@property (nonatomic, strong) NSString * serialNumber;
@property (nonatomic, copy) NSString * serialNumber;
- (nullable NSArray<MTROptionalQRCodeInfo *> *)getAllOptionalVendorData:(NSError * __autoreleasing *)error;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ NS_ASSUME_NONNULL_BEGIN
return self;
}

- (id)copyWithZone:(nullable NSZone *)zone;
{
auto other = [[MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Params alloc] init];

{{#zcl_command_arguments}}
other.{{asStructPropertyName label}} = self.{{asStructPropertyName label}};
{{/zcl_command_arguments}}
other.timedInvokeTimeoutMs = self.timedInvokeTimeoutMs;

return other;
}

- (NSString *)description
{
NSString *descriptionString = [NSString stringWithFormat:@"<%@: {{#zcl_command_arguments}}{{asStructPropertyName label}}:%@; {{/zcl_command_arguments}}>", NSStringFromClass([self class]) {{#zcl_command_arguments}},{{#if isArray}}_{{asStructPropertyName label}}{{else if (isOctetString type)}}[_{{asStructPropertyName label}} base64EncodedStringWithOptions:0]{{else}}_{{asStructPropertyName label}}{{/if}}{{/zcl_command_arguments}}];
Expand Down
16 changes: 8 additions & 8 deletions src/darwin/Framework/CHIP/templates/MTRCommandPayloadsObjc.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ NS_ASSUME_NONNULL_BEGIN

{{#zcl_clusters}}
{{#zcl_commands}}
@interface MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Params : NSObject
@interface MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Params : NSObject <NSCopying>
{{#zcl_command_arguments}}

{{! Override the getter name because some of our properties start with things
like "new" or "init" }}
@property (strong, nonatomic{{#unless (isStrEqual (asGetterName label) (asStructPropertyName label))}}, getter={{asGetterName label}}{{/unless}}) {{asObjectiveCType type parent.parent.name}} {{asStructPropertyName label}};
@property (nonatomic, copy{{#unless (isStrEqual (asGetterName label) (asStructPropertyName label))}}, getter={{asGetterName label}}{{/unless}}) {{asObjectiveCType type parent.parent.name}} {{asStructPropertyName label}};
{{/zcl_command_arguments}}
/**
* Controls whether the command is a timed command (using Timed Invoke).
*
*
* If nil (the default value), a regular invoke is done for commands that do
* not require a timed invoke and a timed invoke with some default timed request
* timeout is done for commands that require a timed invoke.
Expand All @@ -26,12 +26,12 @@ NS_ASSUME_NONNULL_BEGIN
* desired security properties but large enough that it will allow a round-trip
* from the sever to the client (for the status response and actual invoke
* request) within the timeout window.
*
*/
@property (strong, nonatomic, nullable) NSNumber * timedInvokeTimeoutMs;
*
*/
@property (nonatomic, copy, nullable) NSNumber * timedInvokeTimeoutMs;

- (instancetype)init;
- (instancetype)init;
- (id)copyWithZone:(nullable NSZone *)zone;
@end
{{/zcl_commands}}
{{/zcl_clusters}}
Expand Down
22 changes: 22 additions & 0 deletions src/darwin/Framework/CHIP/templates/MTRStructsObjc-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,17 @@ NS_ASSUME_NONNULL_BEGIN
return self;
}

- (id)copyWithZone:(nullable NSZone *)zone
{
auto other = [[MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}} alloc] init];

{{#zcl_struct_items}}
other.{{asStructPropertyName label}} = self.{{asStructPropertyName label}};
{{/zcl_struct_items}}

return other;
}

- (NSString *)description
{
NSString *descriptionString = [NSString stringWithFormat:@"<%@: {{#zcl_struct_items}}{{asStructPropertyName label}}:%@; {{/zcl_struct_items}}>", NSStringFromClass([self class]){{#zcl_struct_items}},{{#if isArray}}_{{asStructPropertyName label}}{{else if (isOctetString type)}}[_{{asStructPropertyName label}} base64EncodedStringWithOptions:0]{{else}}_{{asStructPropertyName label}}{{/if}}{{/zcl_struct_items}}];
Expand All @@ -39,6 +50,17 @@ NS_ASSUME_NONNULL_BEGIN
return self;
}

- (id)copyWithZone:(nullable NSZone *)zone
{
auto other = [[MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Event alloc] init];

{{#zcl_event_fields}}
other.{{asStructPropertyName name}} = self.{{asStructPropertyName name}};
{{/zcl_event_fields}}

return other;
}

- (NSString *)description
{
NSString *descriptionString = [NSString stringWithFormat:@"<%@: {{#zcl_event_fields}}{{asStructPropertyName name}}:%@; {{/zcl_event_fields}}>", NSStringFromClass([self class]){{#zcl_event_fields}},{{#if isArray}}_{{asStructPropertyName name}}{{else if (isOctetString type)}}[_{{asStructPropertyName name}} base64EncodedStringWithOptions:0]{{else}}_{{asStructPropertyName name}}{{/if}}{{/zcl_event_fields}}];
Expand Down
14 changes: 9 additions & 5 deletions src/darwin/Framework/CHIP/templates/MTRStructsObjc.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,27 @@ NS_ASSUME_NONNULL_BEGIN

{{#zcl_clusters}}
{{#zcl_structs}}
@interface MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}} : NSObject
@interface MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}} : NSObject <NSCopying>
{{! Override the getter name because some of our properties start with things
like "new" or "init" }}
{{#zcl_struct_items}}
@property (strong, nonatomic{{#unless (isStrEqual (asGetterName label) (asStructPropertyName label))}}, getter={{asGetterName label}}{{/unless}}) {{asObjectiveCType type parent.parent.name}} {{asStructPropertyName label}};
@property (nonatomic, copy{{#unless (isStrEqual (asGetterName label) (asStructPropertyName label))}}, getter={{asGetterName label}}{{/unless}}) {{asObjectiveCType type parent.parent.name}} {{asStructPropertyName label}};
{{/zcl_struct_items}}
- (instancetype)init;

- (instancetype)init;
- (id)copyWithZone:(nullable NSZone *)zone;
@end

{{/zcl_structs}}

{{#zcl_events}}
@interface MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Event : NSObject
@interface MTR{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Event : NSObject <NSCopying>
{{#zcl_event_fields}}
@property (strong, nonatomic{{#unless (isStrEqual (asGetterName name) (asStructPropertyName name))}}, getter={{asGetterName name}}{{/unless}}) {{asObjectiveCType type parent.parent.name}} {{asStructPropertyName name}};
@property (nonatomic, copy{{#unless (isStrEqual (asGetterName name) (asStructPropertyName name))}}, getter={{asGetterName name}}{{/unless}}) {{asObjectiveCType type parent.parent.name}} {{asStructPropertyName name}};
{{/zcl_event_fields}}

- (instancetype)init;
- (id)copyWithZone:(nullable NSZone *)zone;
@end

{{/zcl_events}}
Expand Down
Loading

0 comments on commit 540f3c6

Please sign in to comment.