Skip to content

Commit

Permalink
Move the controllerDataStore property to MTRDeviceController_Concrete. (
Browse files Browse the repository at this point in the history
#36331)

Non-concrete controllers are not initialized with a datastore.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 15, 2024
1 parent 650d533 commit 1431134
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#import "MTRCertificates.h"
#import "MTRConversion.h"
#import "MTRDeviceControllerStartupParams_Internal.h"
#import "MTRDeviceController_Concrete.h"
#import "MTRDeviceController_Internal.h"
#import "MTRLogging_Internal.h"
#import "MTRP256KeypairBridge.h"
Expand Down Expand Up @@ -596,7 +597,7 @@ - (instancetype)initForExistingFabric:(FabricTable *)fabricTable
return self;
}

- (instancetype)initForNewController:(MTRDeviceController *)controller
- (instancetype)initForNewController:(MTRDeviceController_Concrete *)controller
fabricTable:(chip::FabricTable *)fabricTable
keystore:(chip::Crypto::OperationalKeystore *)keystore
advertiseOperational:(BOOL)advertiseOperational
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
#include <lib/core/DataModelTypes.h>
#include <lib/core/Optional.h>

// MTRDeviceController_Concrete.h imports this header, so we can't import it.
@class MTRDeviceController_Concrete;

namespace chip {
class FabricTable;

Expand Down Expand Up @@ -157,7 +160,7 @@ NS_ASSUME_NONNULL_BEGIN
/**
* Initialize for controller bringup with per-controller storage.
*/
- (instancetype)initForNewController:(MTRDeviceController *)controller
- (instancetype)initForNewController:(MTRDeviceController_Concrete *)controller
fabricTable:(chip::FabricTable *)fabricTable
keystore:(chip::Crypto::OperationalKeystore *)keystore
advertiseOperational:(BOOL)advertiseOperational
Expand Down
6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#import "MTRAsyncWorkQueue.h"
#import "MTRDeviceConnectionBridge.h"
#import "MTRDeviceControllerDataStore.h"
#import "MTRDeviceControllerStartupParams_Internal.h"

#include <credentials/FabricTable.h>
Expand Down Expand Up @@ -233,6 +234,11 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, readonly) MTRAsyncWorkQueue<MTRDeviceController *> * concurrentSubscriptionPool;

/**
* The per-controller data store this controller was initialized with, if any.
*/
@property (nonatomic, readonly, nullable) MTRDeviceControllerDataStore * controllerDataStore;

@end

NS_ASSUME_NONNULL_END
6 changes: 0 additions & 6 deletions src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#import "MTRBaseDevice.h"
#import "MTRDeviceClusterData.h"
#import "MTRDeviceController.h"
#import "MTRDeviceControllerDataStore.h"
#import "MTRDeviceControllerDelegate.h"
#import "MTRDeviceStorageBehaviorConfiguration.h"

Expand Down Expand Up @@ -65,11 +64,6 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, readonly, nullable) NSNumber * compressedFabricID;

/**
* The per-controller data store this controller was initialized with, if any.
*/
@property (nonatomic, readonly, nullable) MTRDeviceControllerDataStore * controllerDataStore;

/**
* OTA delegate and its queue, if this controller supports OTA. Either both
* will be non-nil or both will be nil.
Expand Down
26 changes: 14 additions & 12 deletions src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ @interface MTRDevice_Concrete ()
@property (nonatomic, readwrite, nullable, copy) NSNumber * estimatedSubscriptionLatency;
@property (nonatomic, readwrite, assign) BOOL suspended;

// nullable because technically _deviceController is nullable.
@property (nonatomic, readonly, nullable) MTRDeviceController_Concrete * _concreteController;

@end

typedef void (^MTRDeviceAttributeReportHandler)(NSArray * _Nonnull);
Expand Down Expand Up @@ -901,7 +904,7 @@ - (void)_triggerResubscribeWithReason:(NSString *)reason nodeLikelyReachable:(BO
} else if (_internalDeviceState == MTRInternalDeviceStateSubscribing && nodeLikelyReachable) {
// If we have reason to suspect that the node is now reachable and we haven’t established a
// CASE session yet, let’s consider it to be stalled and invalidate the pairing session.
[[self _concreteController] asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) {
[self._concreteController asyncGetCommissionerOnMatterQueue:^(Controller::DeviceCommissioner * commissioner) {
auto caseSessionMgr = commissioner->CASESessionMgr();
VerifyOrDie(caseSessionMgr != nullptr);
caseSessionMgr->ReleaseSession(commissioner->GetPeerScopedId(self->_nodeID.unsignedLongLongValue));
Expand Down Expand Up @@ -1245,7 +1248,7 @@ - (void)_scheduleSubscriptionPoolWork:(dispatch_block_t)workBlock inNanoseconds:

workBlock();
}];
[[[self _concreteController] concurrentSubscriptionPool] enqueueWorkItem:workItem description:description];
[self._concreteController.concurrentSubscriptionPool enqueueWorkItem:workItem description:description];
MTR_LOG("%@ - enqueued in the subscription pool", self);
};

Expand Down Expand Up @@ -1547,7 +1550,7 @@ - (void)_persistClusterData
// storage implementation, which will try to read them later. Make sure
// we snapshot the state here instead of handing out live copies.
NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> * clusterData = [self _clusterDataToPersistSnapshot];
[_deviceController.controllerDataStore storeClusterData:clusterData forNodeID:_nodeID];
[self._concreteController.controllerDataStore storeClusterData:clusterData forNodeID:_nodeID];
for (MTRClusterPath * clusterPath in _clusterDataToPersist) {
[_persistedClusterData setObject:_clusterDataToPersist[clusterPath] forKey:clusterPath];
[_persistedClusters addObject:clusterPath];
Expand Down Expand Up @@ -2140,7 +2143,7 @@ - (void)_reconcilePersistedClustersWithStorage

NSMutableSet * clusterPathsToRemove = [NSMutableSet set];
for (MTRClusterPath * clusterPath in _persistedClusters) {
MTRDeviceClusterData * data = [_deviceController.controllerDataStore getStoredClusterDataForNodeID:_nodeID endpointID:clusterPath.endpoint clusterID:clusterPath.cluster];
MTRDeviceClusterData * data = [self._concreteController.controllerDataStore getStoredClusterDataForNodeID:_nodeID endpointID:clusterPath.endpoint clusterID:clusterPath.cluster];
if (!data) {
[clusterPathsToRemove addObject:clusterPath];
}
Expand Down Expand Up @@ -2175,13 +2178,13 @@ - (nullable MTRDeviceClusterData *)_clusterDataForPath:(MTRClusterPath *)cluster
return nil;
}

NSAssert(_deviceController.controllerDataStore != nil,
NSAssert(self._concreteController.controllerDataStore != nil,
@"How can _persistedClusters have an entry if we have no persistence?");
NSAssert(_persistedClusterData != nil,
@"How can _persistedClusterData not exist if we have persisted clusters?");

// Page in the stored value for the data.
MTRDeviceClusterData * data = [_deviceController.controllerDataStore getStoredClusterDataForNodeID:_nodeID endpointID:clusterPath.endpoint clusterID:clusterPath.cluster];
MTRDeviceClusterData * data = [self._concreteController.controllerDataStore getStoredClusterDataForNodeID:_nodeID endpointID:clusterPath.endpoint clusterID:clusterPath.cluster];
MTR_LOG("%@ cluster path %@ cache miss - load from storage success %@", self, clusterPath, MTR_YES_NO(data));
if (data != nil) {
[_persistedClusterData setObject:data forKey:clusterPath];
Expand Down Expand Up @@ -2456,7 +2459,7 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason
MATTER_LOG_METRIC_BEGIN(kMetricMTRDeviceInitialSubscriptionSetup);

// Call directlyGetSessionForNode because the subscription setup already goes through the subscription pool queue
[[self _concreteController]
[self._concreteController
directlyGetSessionForNode:_nodeID.unsignedLongLongValue
completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error,
Expand Down Expand Up @@ -3429,7 +3432,7 @@ - (void)_removeClusters:(NSSet<MTRClusterPath *> *)clusterPathsToRemove
[_persistedClusterData removeObjectForKey:path];
[_clusterDataToPersist removeObjectForKey:path];
if (doRemoveFromDataStore) {
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:path.endpoint clusterID:path.cluster];
[self._concreteController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:path.endpoint clusterID:path.cluster];
}
}
}
Expand All @@ -3443,7 +3446,7 @@ - (void)_removeAttributes:(NSSet<NSNumber *> *)attributes fromCluster:(MTRCluste
}
// Just clear out the NSCache entry for this cluster, so we'll load it from storage as needed.
[_persistedClusterData removeObjectForKey:clusterPath];
[self.deviceController.controllerDataStore removeAttributes:attributes fromCluster:clusterPath forNodeID:self.nodeID];
[self._concreteController.controllerDataStore removeAttributes:attributes fromCluster:clusterPath forNodeID:self.nodeID];
}

- (void)_pruneEndpointsIn:(MTRDeviceDataValueDictionary)previousPartsListValue
Expand All @@ -3464,7 +3467,7 @@ - (void)_pruneEndpointsIn:(MTRDeviceDataValueDictionary)previousPartsListValue
}
}
[self _removeClusters:clusterPathsToRemove doRemoveFromDataStore:NO];
[self.deviceController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:endpoint];
[self._concreteController.controllerDataStore clearStoredClusterDataForNodeID:self.nodeID endpointID:endpoint];

[_deviceController asyncDispatchToMatterQueue:^{
std::lock_guard lock(self->_lock);
Expand Down Expand Up @@ -3792,7 +3795,7 @@ - (void)_storePersistedDeviceData
{
os_unfair_lock_assert_owner(&self->_lock);

auto datastore = _deviceController.controllerDataStore;
auto datastore = self._concreteController.controllerDataStore;
if (datastore == nil) {
// No way to store.
return;
Expand Down Expand Up @@ -4207,7 +4210,6 @@ - (void)controllerResumed
[self _ensureSubscriptionForExistingDelegates:@"Controller resumed"];
}

// nullable because technically _deviceController is nullable.
- (nullable MTRDeviceController_Concrete *)_concreteController
{
// We know our _deviceController is actually an MTRDeviceController_Concrete, since that's what
Expand Down

0 comments on commit 1431134

Please sign in to comment.