Address Darwin API review comments on MTRThreadOperationalDataset. (#23392)
* Address Darwin API review comments on MTRThreadOperationalDataset.
* Improve documentation
* Actually use our size constants in the error-checks we do.
* Make the properties non-nullable, since we always have them after successful
init.
This is a re-landing of PR #22562 but modified to preserve old APIs.
* Address review comment.
diff --git a/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.h b/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.h
index 5c8bb60..fa2e6b7 100644
--- a/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.h
+++ b/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.h
@@ -19,52 +19,66 @@
NS_ASSUME_NONNULL_BEGIN
+/**
+ * MTRThreadOperationalDataset allows converting between an "expanded" view of
+ * the dataset (with the separate fields) and a single-blob NSData view.
+ *
+ * The latter can be used to pass Thread network credentials via
+ * MTRCommissioningParameters.
+ */
@interface MTRThreadOperationalDataset : NSObject
/**
- * The expected lengths of each of the NSData fields in the CHIPThreadOperationalDataset
- *
- * initWithNetworkName must be provided NSData fields with at least these lengths otherwise
- * the object will fail to init.
+ * The expected lengths of each of the NSData fields in the MTRThreadOperationalDataset
*/
extern size_t const MTRSizeThreadNetworkName;
-extern size_t const MTRSizeThreadExtendedPanId;
+extern size_t const MTRSizeThreadExtendedPanId MTR_NEWLY_DEPRECATED("Please use MTRSizeThreadExtendedPANID");
+extern size_t const MTRSizeThreadExtendedPANID MTR_NEWLY_AVAILABLE;
extern size_t const MTRSizeThreadMasterKey;
extern size_t const MTRSizeThreadPSKc;
+extern size_t const MTRSizeThreadPANID MTR_NEWLY_AVAILABLE;
/**
- * The Thread Network name
+ * The Thread Network name
*/
-@property (nonatomic, nullable, copy, readonly) NSString * networkName;
+@property (nonatomic, copy, readonly) NSString * networkName;
/**
- * The Thread Network extendended PAN ID
+ * The Thread Network extendended PAN ID
*/
-@property (nonatomic, nullable, copy, readonly) NSData * extendedPANID;
+@property (nonatomic, copy, readonly) NSData * extendedPANID;
/**
- * The 16 byte Master Key
+ * The 16 byte Master Key
*/
-@property (nonatomic, nullable, copy, readonly) NSData * masterKey;
+@property (nonatomic, copy, readonly) NSData * masterKey;
/**
- * The Thread PSKc
+ * The Thread PSKc
*/
-@property (nonatomic, nullable, copy, readonly) NSData * PSKc;
+@property (nonatomic, copy, readonly) NSData * PSKc;
/**
- * The Thread network channel. Always an unsigned 16-bit integer.
+ * The Thread network channel. Always an unsigned 16-bit integer.
*/
@property (nonatomic, copy, readonly) NSNumber * channelNumber MTR_NEWLY_AVAILABLE;
/**
- * A uint16_t stored as 2-bytes in host order representing the Thread PAN ID
+ * A uint16_t stored as 2-bytes in host order representing the Thread PAN ID
*/
-@property (nonatomic, nullable, copy, readonly) NSData * panID;
+@property (nonatomic, copy, readonly) NSData * panID;
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
/**
- * Create a Thread Operational Dataset object with the individual network fields.
- * This initializer will return nil if any of the NSData fields don't match the expected size.
+ * Create a Thread Operational Dataset object with the individual network
+ * fields.
*
- * Note: The panID is expected to be a uint16_t stored as 2-bytes in host order
+ * @param extendedPANID Must be MTRSizeThreadExtendedPANID bytes. Otherwise nil
+ * will be returned.
+ * @param masterKey Must be MTRSizeThreadMasterKey bytes. Otherwise nil will be
+ * returned.
+ * @param PSKc Must be MTRSizeThreadPSKc bytes. Otherwise nil will be returned.
+ * @param channelNumber Must be an unsigned 16-bit value.
+ * @param panID Must be MTRSizeThreadPANID bytes. Otherwise nil will be
+ * returned. In particular, it's expected to be a 16-bit unsigned
+ * integer stored as 2 bytes in host order.
*/
- (nullable instancetype)initWithNetworkName:(NSString *)networkName
extendedPANID:(NSData *)extendedPANID
@@ -74,13 +88,14 @@
panID:(NSData *)panID MTR_NEWLY_AVAILABLE;
/**
- * Create a Thread Operational Dataset object with a RCP formatted active operational dataset.
- * This initializer will return nil if the input data cannot be parsed correctly
+ * Create a Thread Operational Dataset object with a RCP formatted active operational dataset.
+ * This initializer will return nil if the input data cannot be parsed correctly
*/
- (nullable instancetype)initWithData:(NSData *)data;
/**
* Get the underlying data that represents the Thread Active Operational Dataset
+ * This can be used for the threadOperationalDataset of MTRCommissioningParameters.
*/
- (NSData *)data;
diff --git a/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.mm b/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.mm
index fe8a6d0..d8831ce 100644
--- a/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.mm
+++ b/src/darwin/Framework/CHIP/MTRThreadOperationalDataset.mm
@@ -22,9 +22,11 @@
#include <lib/support/ThreadOperationalDataset.h>
size_t const MTRSizeThreadNetworkName = chip::Thread::kSizeNetworkName;
-size_t const MTRSizeThreadExtendedPanId = chip::Thread::kSizeExtendedPanId;
+size_t const MTRSizeThreadExtendedPANID = chip::Thread::kSizeExtendedPanId;
+size_t const MTRSizeThreadExtendedPanId = MTRSizeThreadExtendedPANID;
size_t const MTRSizeThreadMasterKey = chip::Thread::kSizeMasterKey;
size_t const MTRSizeThreadPSKc = chip::Thread::kSizePSKc;
+size_t const MTRSizeThreadPANID = 2; // Thread's PAN ID is 2 bytes
@interface MTRThreadOperationalDataset ()
@@ -62,43 +64,41 @@
_cppThreadOperationalDataset.Clear();
_cppThreadOperationalDataset.SetNetworkName([self.networkName cStringUsingEncoding:NSUTF8StringEncoding]);
- if (![self _checkDataLength:self.extendedPANID expectedLength:chip::Thread::kSizeExtendedPanId]) {
+ if (![self _checkDataLength:self.extendedPANID expectedLength:MTRSizeThreadExtendedPANID]) {
MTR_LOG_ERROR("Invalid ExtendedPANID");
return NO;
}
- uint8_t extendedPanId[chip::Thread::kSizeExtendedPanId];
- [self.extendedPANID getBytes:&extendedPanId length:chip::Thread::kSizeExtendedPanId];
+ uint8_t extendedPanId[MTRSizeThreadExtendedPANID];
+ [self.extendedPANID getBytes:&extendedPanId length:MTRSizeThreadExtendedPANID];
_cppThreadOperationalDataset.SetExtendedPanId(extendedPanId);
- if (![self _checkDataLength:self.masterKey expectedLength:chip::Thread::kSizeMasterKey]) {
+ if (![self _checkDataLength:self.masterKey expectedLength:MTRSizeThreadMasterKey]) {
MTR_LOG_ERROR("Invalid MasterKey");
return NO;
}
- uint8_t masterKey[chip::Thread::kSizeMasterKey];
- [self.masterKey getBytes:&masterKey length:chip::Thread::kSizeMasterKey];
+ uint8_t masterKey[MTRSizeThreadMasterKey];
+ [self.masterKey getBytes:&masterKey length:MTRSizeThreadMasterKey];
_cppThreadOperationalDataset.SetMasterKey(masterKey);
- if (![self _checkDataLength:self.PSKc expectedLength:chip::Thread::kSizePSKc]) {
+ if (![self _checkDataLength:self.PSKc expectedLength:MTRSizeThreadPSKc]) {
MTR_LOG_ERROR("Invalid PKSc");
return NO;
}
- uint8_t PSKc[chip::Thread::kSizePSKc];
- [self.PSKc getBytes:&PSKc length:chip::Thread::kSizePSKc];
+ uint8_t PSKc[MTRSizeThreadPSKc];
+ [self.PSKc getBytes:&PSKc length:MTRSizeThreadPSKc];
_cppThreadOperationalDataset.SetPSKc(PSKc);
_cppThreadOperationalDataset.SetChannel([self.channelNumber unsignedShortValue]);
// Thread's PAN ID is 2 bytes
- if (![self _checkDataLength:self.panID expectedLength:2]) {
+ if (![self _checkDataLength:self.panID expectedLength:MTRSizeThreadPANID]) {
MTR_LOG_ERROR("Invalid PAN ID");
return NO;
}
- uint16_t * valuePtr = (uint16_t *) [self.panID bytes];
- if (valuePtr == nullptr) {
- return NO;
- }
+ uint16_t panID;
+ memcpy(&panID, [self.panID bytes], MTRSizeThreadPANID);
// The underlying CPP class assumes Big Endianness for the panID
- _cppThreadOperationalDataset.SetPanId(CFSwapInt16HostToBig(*valuePtr));
+ _cppThreadOperationalDataset.SetPanId(CFSwapInt16HostToBig(panID));
return YES;
}
@@ -124,7 +124,7 @@
// len+1 for null termination
char networkName[MTRSizeThreadNetworkName + 1];
uint8_t pskc[MTRSizeThreadPSKc];
- uint8_t extendedPANID[MTRSizeThreadExtendedPanId];
+ uint8_t extendedPANID[MTRSizeThreadExtendedPANID];
uint8_t masterKey[MTRSizeThreadMasterKey];
uint16_t panID;
uint16_t channel;
@@ -137,7 +137,7 @@
panID = CFSwapInt16BigToHost(panID);
return [self initWithNetworkName:[NSString stringWithUTF8String:networkName]
- extendedPANID:[NSData dataWithBytes:extendedPANID length:MTRSizeThreadExtendedPanId]
+ extendedPANID:[NSData dataWithBytes:extendedPANID length:MTRSizeThreadExtendedPANID]
masterKey:[NSData dataWithBytes:masterKey length:MTRSizeThreadMasterKey]
PSKc:[NSData dataWithBytes:pskc length:MTRSizeThreadPSKc]
channelNumber:@(channel)
diff --git a/src/darwin/Framework/CHIPTests/MTRThreadOperationalDatasetTests.mm b/src/darwin/Framework/CHIPTests/MTRThreadOperationalDatasetTests.mm
index de15aa2..59f34f8 100644
--- a/src/darwin/Framework/CHIPTests/MTRThreadOperationalDatasetTests.mm
+++ b/src/darwin/Framework/CHIPTests/MTRThreadOperationalDatasetTests.mm
@@ -38,7 +38,7 @@
const uint16_t panID = 0x28f4;
MTRThreadOperationalDataset * dataset = [[MTRThreadOperationalDataset alloc]
initWithNetworkName:@"TestNetwork"
- extendedPANID:[NSData dataWithBytes:&extendedPANID length:MTRSizeThreadExtendedPanId]
+ extendedPANID:[NSData dataWithBytes:&extendedPANID length:MTRSizeThreadExtendedPANID]
masterKey:[NSData dataWithBytes:&masterKey length:MTRSizeThreadMasterKey]
PSKc:[NSData dataWithBytes:&PKSc length:MTRSizeThreadPSKc]
channelNumber:@(25)
