Darwin: Consistently use ivars in MTRDeviceController and a few other places

examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm

@@ -12,15 +12,14 @@
 static NSString * const kOperationalCredentialsIssuerKeypairStorage = @"ChipToolOpCredsCAKey";
 static NSString * const kOperationalCredentialsIPK = @"ChipToolOpCredsIPK";
 
-@interface CHIPToolKeypair ()
-@property (nonatomic) chip::Crypto::P256Keypair mKeyPair;
-@property (nonatomic) chip::Crypto::P256Keypair mIssuer;
-@property (nonatomic) NSData * ipk;
-@property (atomic) uint32_t mNow;
-@property (nonatomic, readonly) SecKeyRef mPublicKey;
-@end
+@implementation CHIPToolKeypair {
+    chip::Crypto::P256Keypair _mKeyPair;
+    chip::Crypto::P256Keypair _mIssuer;
+    NSData * _ipk;
+    uint32_t _mNow;
+    SecKeyRef _mPublicKey;
+}
 
-@implementation CHIPToolKeypair
 - (instancetype)init
 {
     if (self = [super init]) {

examples/darwin-framework-tool/commands/pairing/DeviceControllerDelegateBridge.mm

@@ -19,9 +19,6 @@
 #include "DeviceControllerDelegateBridge.h"
 #import <Matter/Matter.h>
 
-@interface CHIPToolDeviceControllerDelegate ()
-@end
-
 @implementation CHIPToolDeviceControllerDelegate
 - (void)controller:(MTRDeviceController *)controller statusUpdate:(MTRCommissioningStatus)status
 {

src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm

@@ -15,6 +15,10 @@
  *    limitations under the License.
  */
 
+// NOTE: This class was not intended to be part of the public Matter API;
+// internally this class has been replaced by MTRAsyncWorkQueue. This code
+// remains here simply to preserve API/ABI compatibility.
+
 #import <dispatch/dispatch.h>
 #import <os/lock.h>
 

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -2744,12 +2744,10 @@ - (id)copyWithZone:(NSZone *)zone
 
 @end
 
-@interface MTREventReport () {
+@implementation MTREventReport {
     NSNumber * _timestampValue;
 }
-@end
 
-@implementation MTREventReport
 + (void)initialize
 {
     // One of our init methods ends up doing Platform::MemoryAlloc.

src/darwin/Framework/CHIP/MTRDeviceController.mm

@@ -101,31 +101,27 @@
 typedef id (^SyncWorkQueueBlockWithReturnValue)(void);
 typedef BOOL (^SyncWorkQueueBlockWithBoolReturnValue)(void);
 
-@interface MTRDeviceController () {
+@implementation MTRDeviceController {
     // Atomic because it can be touched from multiple threads.
     std::atomic<chip::FabricIndex> _storedFabricIndex;
-}
-
-// queue used to serialize all work performed by the MTRDeviceController
-@property (atomic, readonly) dispatch_queue_t chipWorkQueue;
 
-@property (readonly) chip::Controller::DeviceCommissioner * cppCommissioner;
-@property (readonly) chip::Credentials::PartialDACVerifier * partialDACVerifier;
-@property (readonly) chip::Credentials::DefaultDACVerifier * defaultDACVerifier;
-@property (readonly) MTRDeviceControllerDelegateBridge * deviceControllerDelegateBridge;
-@property (readonly) MTROperationalCredentialsDelegate * operationalCredentialsDelegate;
-@property (readonly) MTRP256KeypairBridge signingKeypairBridge;
-@property (readonly) MTRP256KeypairBridge operationalKeypairBridge;
-@property (readonly) MTRDeviceAttestationDelegateBridge * deviceAttestationDelegateBridge;
-@property (readonly) MTRDeviceControllerFactory * factory;
-@property (readonly) NSMutableDictionary * nodeIDToDeviceMap;
-@property (readonly) os_unfair_lock deviceMapLock; // protects nodeIDToDeviceMap
-@property (readonly) MTRCommissionableBrowser * commissionableBrowser;
-@property (readonly) MTRAttestationTrustStoreBridge * attestationTrustStoreBridge;
+    // queue used to serialize all work performed by the MTRDeviceController
+    dispatch_queue_t _chipWorkQueue;
 
-@end
-
-@implementation MTRDeviceController
+    chip::Controller::DeviceCommissioner * _cppCommissioner;
+    chip::Credentials::PartialDACVerifier * _partialDACVerifier;
+    chip::Credentials::DefaultDACVerifier * _defaultDACVerifier;
+    MTRDeviceControllerDelegateBridge * _deviceControllerDelegateBridge;
+    MTROperationalCredentialsDelegate * _operationalCredentialsDelegate;
+    MTRP256KeypairBridge _signingKeypairBridge;
+    MTRP256KeypairBridge _operationalKeypairBridge;
+    MTRDeviceAttestationDelegateBridge * _deviceAttestationDelegateBridge;
+    MTRDeviceControllerFactory * _factory;
+    NSMutableDictionary * _nodeIDToDeviceMap;
+    os_unfair_lock _deviceMapLock; // protects nodeIDToDeviceMap
+    MTRCommissionableBrowser * _commissionableBrowser;
+    MTRAttestationTrustStoreBridge * _attestationTrustStoreBridge;
+}
 
 - (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters error:(NSError * __autoreleasing *)error
 {
@@ -249,7 +245,7 @@ - (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
 
 - (BOOL)isRunning
 {
-    return self.cppCommissioner != nullptr;
+    return _cppCommissioner != nullptr;
 }
 
 - (void)shutdown
@@ -271,8 +267,8 @@ - (void)cleanupAfterStartup
     // while calling out into arbitrary invalidation code, snapshot the list of
     // devices before we start invalidating.
     os_unfair_lock_lock(&_deviceMapLock);
-    NSArray<MTRDevice *> * devices = [self.nodeIDToDeviceMap allValues];
-    [self.nodeIDToDeviceMap removeAllObjects];
+    NSArray<MTRDevice *> * devices = [_nodeIDToDeviceMap allValues];
+    [_nodeIDToDeviceMap removeAllObjects];
     os_unfair_lock_unlock(&_deviceMapLock);
 
     for (MTRDevice * device in devices) {
@@ -590,7 +586,7 @@ - (BOOL)setupCommissioningSessionWithPayload:(MTRSetupPayload *)payload
 
         chip::NodeId nodeId = [newNodeID unsignedLongLongValue];
         self->_operationalCredentialsDelegate->SetDeviceID(nodeId);
-        auto errorCode = self.cppCommissioner->EstablishPASEConnection(nodeId, [pairingCode UTF8String]);
+        auto errorCode = self->_cppCommissioner->EstablishPASEConnection(nodeId, [pairingCode UTF8String]);
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
@@ -612,7 +608,7 @@ - (BOOL)setupCommissioningSessionWithDiscoveredDevice:(MTRCommissionableBrowserR
             auto pinCode = static_cast<uint32_t>(payload.setupPasscode.unsignedLongValue);
             params.Value().SetSetupPINCode(pinCode);
 
-            errorCode = self.cppCommissioner->EstablishPASEConnection(nodeId, params.Value());
+            errorCode = self->_cppCommissioner->EstablishPASEConnection(nodeId, params.Value());
         } else {
             // Try to get a QR code if possible (because it has a better
             // discriminator, etc), then fall back to manual code if that fails.
@@ -630,7 +626,7 @@ - (BOOL)setupCommissioningSessionWithDiscoveredDevice:(MTRCommissionableBrowserR
                     continue;
                 }
 
-                errorCode = self.cppCommissioner->EstablishPASEConnection(
+                errorCode = self->_cppCommissioner->EstablishPASEConnection(
                     nodeId, [pairingCode UTF8String], chip::Controller::DiscoveryType::kDiscoveryNetworkOnly, resolutionData);
                 if (CHIP_NO_ERROR != errorCode) {
                     break;
@@ -745,7 +741,7 @@ - (BOOL)commissionNodeWithID:(NSNumber *)nodeID
 
         chip::NodeId deviceId = [nodeID unsignedLongLongValue];
         self->_operationalCredentialsDelegate->SetDeviceID(deviceId);
-        auto errorCode = self.cppCommissioner->Commission(deviceId, params);
+        auto errorCode = self->_cppCommissioner->Commission(deviceId, params);
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
@@ -762,7 +758,7 @@ - (BOOL)continueCommissioningDevice:(void *)device
             : chip::Credentials::AttestationVerificationResult::kSuccess;
 
         auto deviceProxy = static_cast<chip::DeviceProxy *>(device);
-        auto errorCode = self.cppCommissioner->ContinueCommissioningAfterDeviceAttestation(deviceProxy,
+        auto errorCode = self->_cppCommissioner->ContinueCommissioningAfterDeviceAttestation(deviceProxy,
             ignoreAttestationFailure ? chip::Credentials::AttestationVerificationResult::kSuccess : lastAttestationResult);
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
@@ -774,7 +770,7 @@ - (BOOL)cancelCommissioningForNodeID:(NSNumber *)nodeID error:(NSError * __autor
 {
     auto block = ^BOOL {
         self->_operationalCredentialsDelegate->ResetDeviceID();
-        auto errorCode = self.cppCommissioner->StopPairing([nodeID unsignedLongLongValue]);
+        auto errorCode = self->_cppCommissioner->StopPairing([nodeID unsignedLongLongValue]);
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorStopPairing error:error];
     };
 
@@ -784,7 +780,7 @@ - (BOOL)cancelCommissioningForNodeID:(NSNumber *)nodeID error:(NSError * __autor
 - (BOOL)startBrowseForCommissionables:(id<MTRCommissionableBrowserDelegate>)delegate queue:(dispatch_queue_t)queue
 {
     auto block = ^BOOL {
-        VerifyOrReturnValue(self.commissionableBrowser == nil, NO);
+        VerifyOrReturnValue(self->_commissionableBrowser == nil, NO);
 
         auto commissionableBrowser = [[MTRCommissionableBrowser alloc] initWithDelegate:delegate controller:self queue:queue];
         VerifyOrReturnValue([commissionableBrowser start], NO);
@@ -799,9 +795,9 @@ - (BOOL)startBrowseForCommissionables:(id<MTRCommissionableBrowserDelegate>)dele
 - (BOOL)stopBrowseForCommissionables
 {
     auto block = ^BOOL {
-        VerifyOrReturnValue(self.commissionableBrowser != nil, NO);
+        VerifyOrReturnValue(self->_commissionableBrowser != nil, NO);
 
-        auto commissionableBrowser = self.commissionableBrowser;
+        auto commissionableBrowser = self->_commissionableBrowser;
         VerifyOrReturnValue([commissionableBrowser stop], NO);
 
         self->_commissionableBrowser = nil;
@@ -845,15 +841,15 @@ - (MTRBaseDevice *)baseDeviceForNodeID:(NSNumber *)nodeID
 - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
 {
     os_unfair_lock_lock(&_deviceMapLock);
-    MTRDevice * deviceToReturn = self.nodeIDToDeviceMap[nodeID];
+    MTRDevice * deviceToReturn = _nodeIDToDeviceMap[nodeID];
     if (!deviceToReturn) {
         deviceToReturn = [[MTRDevice alloc] initWithNodeID:nodeID controller:self];
         // If we're not running, don't add the device to our map.  That would
         // create a cycle that nothing would break.  Just return the device,
         // which will be in exactly the state it would be in if it were created
         // while we were running and then we got shut down.
         if ([self isRunning]) {
-            self.nodeIDToDeviceMap[nodeID] = deviceToReturn;
+            _nodeIDToDeviceMap[nodeID] = deviceToReturn;
         }
     }
     os_unfair_lock_unlock(&_deviceMapLock);
@@ -864,12 +860,13 @@ - (MTRDevice *)deviceForNodeID:(NSNumber *)nodeID
 - (void)removeDevice:(MTRDevice *)device
 {
     os_unfair_lock_lock(&_deviceMapLock);
-    MTRDevice * deviceToRemove = self.nodeIDToDeviceMap[device.nodeID];
+    auto * nodeID = device.nodeID;
+    MTRDevice * deviceToRemove = _nodeIDToDeviceMap[nodeID];
     if (deviceToRemove == device) {
         [deviceToRemove invalidate];
-        self.nodeIDToDeviceMap[device.nodeID] = nil;
+        _nodeIDToDeviceMap[nodeID] = nil;
     } else {
-        MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", device, device.nodeID.unsignedLongLongValue);
+        MTR_LOG_ERROR("Error: Cannot remove device %p with nodeID %llu", device, nodeID.unsignedLongLongValue);
     }
     os_unfair_lock_unlock(&_deviceMapLock);
 }
@@ -935,7 +932,7 @@ - (NSData * _Nullable)attestationChallengeForDeviceID:(NSNumber *)deviceID
     auto block = ^NSData *
     {
         chip::CommissioneeDeviceProxy * deviceProxy;
-        auto errorCode = self.cppCommissioner->GetDeviceBeingCommissioned([deviceID unsignedLongLongValue], &deviceProxy);
+        auto errorCode = self->_cppCommissioner->GetDeviceBeingCommissioned([deviceID unsignedLongLongValue], &deviceProxy);
         VerifyOrReturnValue(![MTRDeviceController checkForError:errorCode logMsg:kErrorGetCommissionee error:nil], nil);
 
         uint8_t challengeBuffer[chip::Crypto::kAES_CCM128_Key_Length];
@@ -1098,7 +1095,7 @@ - (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceComm
             return;
         }
 
-        block(self.cppCommissioner);
+        block(self->_cppCommissioner);
     });
 }
 
@@ -1208,7 +1205,7 @@ - (void)operationalInstanceAdded:(chip::NodeId)nodeID
     // Don't use deviceForNodeID here, because we don't want to create the
     // device if it does not already exist.
     os_unfair_lock_lock(&_deviceMapLock);
-    MTRDevice * device = self.nodeIDToDeviceMap[@(nodeID)];
+    MTRDevice * device = _nodeIDToDeviceMap[@(nodeID)];
     os_unfair_lock_unlock(&_deviceMapLock);
 
     if (device == nil) {
@@ -1400,7 +1397,7 @@ - (BOOL)pairDevice:(uint64_t)deviceID
         VerifyOrReturnValue(![MTRDeviceController checkForError:errorCode logMsg:kErrorSetupCodeGen error:error], NO);
 
         self->_operationalCredentialsDelegate->SetDeviceID(deviceID);
-        errorCode = self.cppCommissioner->EstablishPASEConnection(deviceID, manualPairingCode.c_str());
+        errorCode = self->_cppCommissioner->EstablishPASEConnection(deviceID, manualPairingCode.c_str());
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
@@ -1421,7 +1418,7 @@ - (BOOL)pairDevice:(uint64_t)deviceID
         self->_operationalCredentialsDelegate->SetDeviceID(deviceID);
 
         auto params = chip::RendezvousParameters().SetSetupPINCode(setupPINCode).SetPeerAddress(peerAddress);
-        auto errorCode = self.cppCommissioner->EstablishPASEConnection(deviceID, params);
+        auto errorCode = self->_cppCommissioner->EstablishPASEConnection(deviceID, params);
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
@@ -1432,7 +1429,7 @@ - (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPa
 {
     auto block = ^BOOL {
         self->_operationalCredentialsDelegate->SetDeviceID(deviceID);
-        auto errorCode = self.cppCommissioner->EstablishPASEConnection(deviceID, [onboardingPayload UTF8String]);
+        auto errorCode = self->_cppCommissioner->EstablishPASEConnection(deviceID, [onboardingPayload UTF8String]);
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorPairDevice error:error];
     };
 
@@ -1468,7 +1465,7 @@ - (BOOL)openPairingWindow:(uint64_t)deviceID duration:(NSUInteger)duration error
 
     auto block = ^BOOL {
         auto errorCode = chip::Controller::AutoCommissioningWindowOpener::OpenBasicCommissioningWindow(
-            self.cppCommissioner, deviceID, chip::System::Clock::Seconds16(static_cast<uint16_t>(duration)));
+            self->_cppCommissioner, deviceID, chip::System::Clock::Seconds16(static_cast<uint16_t>(duration)));
         return ![MTRDeviceController checkForError:errorCode logMsg:kErrorOpenPairingWindow error:error];
     };
 
@@ -1508,7 +1505,7 @@ - (NSString *)openPairingWindowWithPIN:(uint64_t)deviceID
     auto block = ^NSString *
     {
         chip::SetupPayload setupPayload;
-        auto errorCode = chip::Controller::AutoCommissioningWindowOpener::OpenCommissioningWindow(self.cppCommissioner, deviceID,
+        auto errorCode = chip::Controller::AutoCommissioningWindowOpener::OpenCommissioningWindow(self->_cppCommissioner, deviceID,
             chip::System::Clock::Seconds16(static_cast<uint16_t>(duration)), chip::Crypto::kSpake2p_Min_PBKDF_Iterations,
             static_cast<uint16_t>(discriminator), chip::MakeOptional(static_cast<uint32_t>(setupPIN)), chip::NullOptional,
             setupPayload);

src/darwin/Framework/CHIP/MTRThreadOperationalDataset.mm

@@ -29,14 +29,9 @@
 size_t const MTRSizeThreadPSKc = chip::Thread::kSizePSKc;
 size_t const MTRSizeThreadPANID = 2; // Thread's PAN ID is 2 bytes
 
-@interface MTRThreadOperationalDataset ()
-
-@property (readonly) chip::Thread::OperationalDataset cppThreadOperationalDataset;
-@property (nonatomic, copy) NSNumber * channelNumber;
-
-@end
-
-@implementation MTRThreadOperationalDataset
+@implementation MTRThreadOperationalDataset {
+    chip::Thread::OperationalDataset _cppThreadOperationalDataset;
+}
 
 - (instancetype _Nullable)initWithNetworkName:(NSString *)networkName
                                 extendedPANID:(NSData *)extendedPANID
@@ -157,7 +152,7 @@ @implementation MTRThreadOperationalDataset (Deprecated)
 
 - (void)setChannel:(uint16_t)channel
 {
-    self.channelNumber = @(channel);
+    _channelNumber = @(channel);
 }
 
 - (uint16_t)channel