Skip to content

Commit 6b682e4

Browse files
committedSep 26, 2024
Remove some more unused bits in MTRDeviceController.
Controller's downloadLogFromNodeWithID is only used from MTRBaseDevice, which is not expected to be used with a non-concrete controller. Unfortunately, nothing actually prevents an MTRBaseDevice beign created against a non-concrete controller, so we can't just move this API to MTRDeviceController_Concrete. With the implementation of downloadLogFromNodeWithID removed, _factory is unused and can be removed. Also, with this implementation removed the factory's downloadLogFromNodeWithID can take MTRDeviceController_Concrete, as can the diagnostic log downloader. Similarly, getSessionFromNode is only used from MTRBaseDevice and MTRCallbackBridgeBase's ActionWithNodeID. And ActionWithNodeID is only used from DispatchAction, which is only called from MTRBaseClusters and MTRBaseDevice, none of which is expected to work with a non-concrete controller. So this implementation can also be removed. At that point directlyGetSessionForNode is only used from MTRDevice_Concrete, so we can just move it to MTRDeviceController_Concrete. getSessionForCommissioneeDevice is only used from ActionWithPASEDevice, which is only used from DispatchAction, like getSessionFromNode. So this implementation can also be removed. At this point asyncDispatchToMatterQueue is only used from: * MTRBaseDevice * MTRClusterStateCacheContainer, where it's used on a controller coming from MTRBaseDevice. * MTRDevice_Concrete, where it's being used on a concrete controller. * MTRDiagnosticLogsDownloader, where it's being used on a concrete controller. * MTRServerAttribute/MTRServerCluster/MTRServerEndpoint, which are not usable with non-concrete controllers as things stand. * MTROTAProviderDelegateBridge, where its being used on a concrete controller. So the MTRDeviceController implementation of asyncDispatchToMatterQueue can also be removed.
1 parent 8323604 commit 6b682e4

9 files changed

+39
-88
lines changed
 

‎src/darwin/Framework/CHIP/MTRDeviceController.mm

+10-71
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ @implementation MTRDeviceController {
126126
MTRDeviceControllerDelegateBridge * _deviceControllerDelegateBridge;
127127
MTROperationalCredentialsDelegate * _operationalCredentialsDelegate;
128128
MTRDeviceAttestationDelegateBridge * _deviceAttestationDelegateBridge;
129-
MTRDeviceControllerFactory * _factory;
130129
os_unfair_lock _underlyingDeviceMapLock;
131130
MTRCommissionableBrowser * _commissionableBrowser;
132131
MTRAttestationTrustStoreBridge * _attestationTrustStoreBridge;
@@ -554,65 +553,14 @@ - (BOOL)checkIsRunning:(NSError * __autoreleasing *)error
554553

555554
- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
556555
{
557-
// Get the corresponding MTRDevice object to determine if the case/subscription pool is to be used
558-
MTRDevice * device = [self deviceForNodeID:@(nodeID)];
559-
560-
// In the case that this device is known to use thread, queue this with subscription attempts as well, to
561-
// help with throttling Thread traffic.
562-
if ([device deviceUsesThread]) {
563-
MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)];
564-
[workItem setReadyHandler:^(id _Nonnull context, NSInteger retryCount, MTRAsyncWorkCompletionBlock _Nonnull workItemCompletion) {
565-
MTRInternalDeviceConnectionCallback completionWrapper = ^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
566-
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
567-
completion(exchangeManager, session, error, retryDelay);
568-
workItemCompletion(MTRAsyncWorkComplete);
569-
};
570-
[self directlyGetSessionForNode:nodeID completion:completionWrapper];
571-
}];
572-
573-
[_concurrentSubscriptionPool enqueueWorkItem:workItem descriptionWithFormat:@"device controller getSessionForNode nodeID: 0x%016llX", nodeID];
574-
} else {
575-
[self directlyGetSessionForNode:nodeID completion:completion];
576-
}
577-
}
578-
579-
- (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
580-
{
581-
[self
582-
asyncGetCommissionerOnMatterQueue:^(chip::Controller::DeviceCommissioner * commissioner) {
583-
auto connectionBridge = new MTRDeviceConnectionBridge(completion);
584-
585-
// MTRDeviceConnectionBridge always delivers errors async via
586-
// completion.
587-
connectionBridge->connect(commissioner, nodeID);
588-
}
589-
errorHandler:^(NSError * error) {
590-
completion(nullptr, chip::NullOptional, error, nil);
591-
}];
556+
MTR_ABSTRACT_METHOD();
557+
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
592558
}
593559

594560
- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion
595561
{
596-
[self
597-
asyncGetCommissionerOnMatterQueue:^(chip::Controller::DeviceCommissioner * commissioner) {
598-
chip::CommissioneeDeviceProxy * deviceProxy;
599-
CHIP_ERROR err = commissioner->GetDeviceBeingCommissioned(deviceID, &deviceProxy);
600-
if (err != CHIP_NO_ERROR) {
601-
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:err], nil);
602-
return;
603-
}
604-
605-
chip::Optional<chip::SessionHandle> session = deviceProxy->GetSecureSession();
606-
if (!session.HasValue() || !session.Value()->AsSecureSession()->IsPASESession()) {
607-
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
608-
return;
609-
}
610-
611-
completion(deviceProxy->GetExchangeManager(), session, nil, nil);
612-
}
613-
errorHandler:^(NSError * error) {
614-
completion(nullptr, chip::NullOptional, error, nil);
615-
}];
562+
MTR_ABSTRACT_METHOD();
563+
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
616564
}
617565

618566
- (MTRTransportType)sessionTransportTypeForDevice:(MTRBaseDevice *)device
@@ -665,10 +613,8 @@ - (void)asyncGetCommissionerOnMatterQueue:(void (^)(chip::Controller::DeviceComm
665613

666614
- (void)asyncDispatchToMatterQueue:(dispatch_block_t)block errorHandler:(nullable MTRDeviceErrorHandler)errorHandler
667615
{
668-
auto adapter = ^(chip::Controller::DeviceCommissioner *) {
669-
block();
670-
};
671-
[self asyncGetCommissionerOnMatterQueue:adapter errorHandler:errorHandler];
616+
MTR_ABSTRACT_METHOD();
617+
errorHandler([MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
672618
}
673619

674620
- (void)syncRunOnWorkQueue:(SyncWorkQueueBlock)block error:(NSError * __autoreleasing *)error
@@ -751,17 +697,10 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
751697
queue:(dispatch_queue_t)queue
752698
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
753699
{
754-
[self asyncDispatchToMatterQueue:^() {
755-
[self->_factory downloadLogFromNodeWithID:nodeID
756-
controller:self
757-
type:type
758-
timeout:timeout
759-
queue:queue
760-
completion:completion];
761-
}
762-
errorHandler:^(NSError * error) {
763-
completion(nil, error);
764-
}];
700+
MTR_ABSTRACT_METHOD();
701+
dispatch_async(queue, ^{
702+
completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
703+
});
765704
}
766705

767706
- (NSArray<MTRAccessGrant *> *)accessGrantsForClusterPath:(MTRClusterPath *)clusterPath

‎src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm

+1-1
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,7 @@ - (nullable NSNumber *)neededReadPrivilegeForClusterID:(NSNumber *)clusterID att
10991099
}
11001100

11011101
- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
1102-
controller:(MTRDeviceController *)controller
1102+
controller:(MTRDeviceController_Concrete *)controller
11031103
type:(MTRDiagnosticLogType)type
11041104
timeout:(NSTimeInterval)timeout
11051105
queue:(dispatch_queue_t)queue

‎src/darwin/Framework/CHIP/MTRDeviceControllerFactory_Internal.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ MTR_DIRECT_MEMBERS
8383
* Download log of the desired type from the device.
8484
*/
8585
- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
86-
controller:(MTRDeviceController *)controller
86+
controller:(MTRDeviceController_Concrete *)controller
8787
type:(MTRDiagnosticLogType)type
8888
timeout:(NSTimeInterval)timeout
8989
queue:(dispatch_queue_t)queue

‎src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h

+8
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#import <Matter/Matter.h>
2121

22+
#import "MTRDeviceConnectionBridge.h"
2223
#import "MTRDeviceControllerStartupParams_Internal.h"
2324

2425
NS_ASSUME_NONNULL_BEGIN
@@ -109,6 +110,13 @@ NS_ASSUME_NONNULL_BEGIN
109110
*/
110111
- (void)clearPendingShutdown;
111112

113+
/**
114+
* Since getSessionForNode now enqueues by the subscription pool for Thread
115+
* devices, MTRDevice_Concrete needs a direct non-queued access because it already
116+
* makes use of the subscription pool.
117+
*/
118+
- (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
119+
112120
@end
113121

114122
NS_ASSUME_NONNULL_END

‎src/darwin/Framework/CHIP/MTRDeviceController_Internal.h

-7
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,6 @@ NS_ASSUME_NONNULL_BEGIN
243243
- (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(nullable NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)prefetchedClusterData;
244244
- (void)removeDevice:(MTRDevice *)device;
245245

246-
/**
247-
* Since getSessionForNode now enqueues by the subscription pool for Thread
248-
* devices, MTRDevice needs a direct non-queued access because it already
249-
* makes use of the subscription pool.
250-
*/
251-
- (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
252-
253246
@end
254247

255248
/**

‎src/darwin/Framework/CHIP/MTRDevice_Concrete.mm

+9-1
Original file line numberDiff line numberDiff line change
@@ -2388,7 +2388,7 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
23882388
MATTER_LOG_METRIC_BEGIN(kMetricMTRDeviceInitialSubscriptionSetup);
23892389

23902390
// Call directlyGetSessionForNode because the subscription setup already goes through the subscription pool queue
2391-
[_deviceController
2391+
[[self _concreteController]
23922392
directlyGetSessionForNode:_nodeID.unsignedLongLongValue
23932393
completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
23942394
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error,
@@ -4138,6 +4138,14 @@ - (void)controllerResumed
41384138
[self _ensureSubscriptionForExistingDelegates:@"Controller resumed"];
41394139
}
41404140

4141+
// nullable because technically _deviceController is nullable.
4142+
- (nullable MTRDeviceController_Concrete *)_concreteController
4143+
{
4144+
// We know our _deviceController is actually an MTRDeviceController_Concrete, since that's what
4145+
// gets passed to initWithNodeID.
4146+
return static_cast<MTRDeviceController_Concrete *>(_deviceController);
4147+
}
4148+
41414149
@end
41424150

41434151
/* BEGIN DRAGONS: Note methods here cannot be renamed, and are used by private callers, do not rename, remove or modify behavior here */

‎src/darwin/Framework/CHIP/MTRDevice_Internal.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ MTR_DIRECT_MEMBERS
118118
NSNumber * _nodeID;
119119

120120
// Our controller. Declared nullable because our property is, though in
121-
// practice it does not look like we ever set it to nil.
121+
// practice it does not look like we ever set it to nil. If this changes,
122+
// fix _concreteController on MTRDevice_Concrete accordingly.
122123
MTRDeviceController * _Nullable _deviceController;
123124
}
124125

‎src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#import <Matter/MTRDeviceController.h>
2121
#import <Matter/MTRDiagnosticLogsType.h>
2222

23+
#import "MTRDeviceController_Concrete.h"
24+
2325
namespace chip {
2426
namespace bdx {
2527
class BDXTransferServerDelegate;
@@ -33,14 +35,14 @@ NS_ASSUME_NONNULL_BEGIN
3335

3436
// Must be called on Matter queue
3537
- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
36-
controller:(MTRDeviceController *)controller
38+
controller:(MTRDeviceController_Concrete *)controller
3739
type:(MTRDiagnosticLogType)type
3840
timeout:(NSTimeInterval)timeout
3941
queue:(dispatch_queue_t)queue
4042
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion;
4143

4244
// Must be called on Matter queue
43-
- (void)abortDownloadsForController:(MTRDeviceController *)controller;
45+
- (void)abortDownloadsForController:(MTRDeviceController_Concrete *)controller;
4446

4547
@end
4648

‎src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm

+4-4
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ - (MTRDownload * _Nullable)add:(MTRDiagnosticLogType)type
8686
completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
8787
done:(void (^)(MTRDownload * finishedDownload))done;
8888

89-
- (void)abortDownloadsForController:(MTRDeviceController *)controller;
89+
- (void)abortDownloadsForController:(MTRDeviceController_Concrete *)controller;
9090

9191
@end
9292

@@ -354,7 +354,7 @@ - (MTRDownload * _Nullable)add:(MTRDiagnosticLogType)type
354354
return download;
355355
}
356356

357-
- (void)abortDownloadsForController:(MTRDeviceController *)controller
357+
- (void)abortDownloadsForController:(MTRDeviceController_Concrete *)controller
358358
{
359359
assertChipStackLockedByCurrentThread();
360360

@@ -408,7 +408,7 @@ - (void)dealloc
408408
}
409409

410410
- (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
411-
controller:(MTRDeviceController *)controller
411+
controller:(MTRDeviceController_Concrete *)controller
412412
type:(MTRDiagnosticLogType)type
413413
timeout:(NSTimeInterval)timeout
414414
queue:(dispatch_queue_t)queue
@@ -462,7 +462,7 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID
462462
}
463463
}
464464

465-
- (void)abortDownloadsForController:(MTRDeviceController *)controller;
465+
- (void)abortDownloadsForController:(MTRDeviceController_Concrete *)controller;
466466
{
467467
assertChipStackLockedByCurrentThread();
468468

0 commit comments

Comments
 (0)