Skip to content

Commit

Permalink
Remove various un-necessary code from MTRDevice. (project-chip#36132)
Browse files Browse the repository at this point in the history
* Remove various un-necessary code from MTRDevice.

The _deviceInternalStateChanged declaration was duplicated in
MTRDevice_Concrete and only needed there.

MTRDeviceClusterData implementation can move into its own file.

setPersistedClusterData: is only used on MTRDevice_Concrete instances, so its
declaration can move to MTRDevice_Concrete and the unused implementation in
MTRDevice can be removed.

setPersistedDeviceData: is only used on MTRDevice_Concrete instances, so its
declaration can move to MTRDevice_Concrete and the unused implementation in
MTRDevice can be removed.

Now _setLastInitialSubscribeLatency and _updateAttributeDependentDescriptionData
on MTRDevice are unused and can be removed.

At this point, _informationalVendorID, _informationalProductID,
_networkFeatures, _vid, _pid, _allNetworkFeatures, and _descriptionLock are all
unused on MTRDevice and can be removed.

Now _informationalNumberAtAttributePath is unused on MTRDevice and can be
removed.

_endpointList is also unused on MTRDevice and can be removed.

deviceCachePrimed is implemented by both MTRDevice_XPC and MTRDevice_Concrete,
so the implementation on MTRDevice can be removed.

unitTestGetClusterDataForPath, unitTestGetPersistedClusters,
unitTestClusterHasBeenPersisted, unitTestResetSubscription,
unitTestAttributesReportedSinceLastCheck, unitTestClearClusterData,
unitTestInjectEventReport, unitTestInjectAttributeReport, and
unitTestAttributeCount are only used on MTRDevice_Concrete, so the duplicate
implementations on MTRDevice can be removed.

At this point, _cachedAttributeValueForPath is unused on MTRDevice and can be
removed.

Now _getCachedDataVersions is unused on MTRDevice and can be removed.

deviceUsesThread is only used by MTRDeviceController_Concrete on
MTRDevice_Concrete instances and so can move to MTRDevice_Concrete entirely.
Then _deviceUsesThread becomes unused on MTRDevice and can be removed.

Then _clusterDataForPath is unused on MTRDevice and can be removed.

Now _reconcilePersistedClustersWithStorage is unused on MTRDevice and can be
removed.

Also, _knownClusters is unused on MTRDevice and can be removed.

setStorageBehaviorConfiguration is only used from MTRDeviceController_Concrete
(which has an MTRDevice_Concrete) and from unit tests (which have an
MTRDevice_Concrete, though they don't know it), so it can be removed from
MTRDevice.  Then _resetStorageBehaviorState also becomes unused and can be
removed.

At this point, _scheduleClusterDataPersistence is not used on MTRDevice and can
be removed.  This makes _persistClusterDataAsNeeded and _deviceCachePrimed
unused, and they can also be removed.

At this point, _persistClusterData, _deviceIsReportingExcessively,
_reportToPersistenceDelayTimeAfterMutiplier,
_reportToPersistenceDelayTimeMaxAfterMutiplier, _mostRecentReportTimes, and
_clusterDataPersistenceFirstScheduledTime are not used on MTRDevice and can be
removed.

Now _dataStoreExists, _clusterDataToPersistSnapshot,
_storageBehaviorConfiguration, _reportToPersistenceDelayCurrentMultiplier,
_deviceReportingExcessivelyStartTime, and _persistedClusters are unused on
MTRDevice and can be removed.

At this point _clusterDataToPersist and _persistedClusterData are unused on
MTRDevice and can be removed.

_systemTimeChangeObserverToken was never actually used on MTRDevice (it's only
used on MTRDevice_Concrete) and can be removed.

The MTRDeviceAttributeReportHandler typedef is unused on MTRDevice and can be
removed.

kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription and
ENABLE_CONNECTIVITY_MONITORING are unused on MTRDevice and can be removed.

assertChipStackLockedByCurrentThread is not used in MTRDevice, so the
LockTracker.h include can be removed.

MTRAsyncWorkQueue, MTRAttributeIsSpecified, MTRCommandNeedsTimedInvoke,
MTRDeviceConnectivityMonitor, MTRDeviceControllerOverXPC, MTRDecodeEventPayload,
are all unused in MTRDevice, so the imports of the corresponding headers can be
removed.

arrayOfNumbersFromAttributeValue is now unused on MTRDevice and can be removed.

The fabricIndex property is write-only on both MTRDevice and MTRDevice_Concrete
and can be removed on both.

_dataValueWithoutDataVersion is unused on MTRDevice and can be removed.

* Move MTRDeviceClusterData into a separate header/implementation file.
  • Loading branch information
bzbarsky-apple authored Oct 18, 2024
1 parent 76e9e14 commit 2574847
Show file tree
Hide file tree
Showing 15 changed files with 261 additions and 905 deletions.
871 changes: 11 additions & 860 deletions src/darwin/Framework/CHIP/MTRDevice.mm

Large diffs are not rendered by default.

38 changes: 38 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceClusterData.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import <Foundation/Foundation.h>

#import "MTRDefines_Internal.h"
#import "MTRDeviceDataValueDictionary.h"

NS_ASSUME_NONNULL_BEGIN

/**
* Information about a cluster: data version and known attribute values.
*/
MTR_TESTABLE
@interface MTRDeviceClusterData : NSObject <NSSecureCoding, NSCopying>
@property (nonatomic, nullable) NSNumber * dataVersion;
@property (nonatomic, readonly) NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * attributes; // attributeID => data-value dictionary

- (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute;
- (void)removeValueForAttribute:(NSNumber *)attribute;

- (nullable instancetype)initWithDataVersion:(NSNumber * _Nullable)dataVersion attributes:(NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _Nullable)attributes;
@end

NS_ASSUME_NONNULL_END
122 changes: 122 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceClusterData.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import "MTRDeviceClusterData.h"
#import "MTRLogging_Internal.h"
#import "MTRUtilities.h"

static NSString * const sDataVersionKey = @"dataVersion";
static NSString * const sAttributesKey = @"attributes";

@implementation MTRDeviceClusterData {
NSMutableDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _attributes;
}

- (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute
{
_attributes[attribute] = value;
}

- (void)removeValueForAttribute:(NSNumber *)attribute
{
[_attributes removeObjectForKey:attribute];
}

- (NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> *)attributes
{
return _attributes;
}

+ (BOOL)supportsSecureCoding
{
return YES;
}

- (NSString *)description
{
return [NSString stringWithFormat:@"<MTRDeviceClusterData: dataVersion %@ attributes count %lu>", _dataVersion, static_cast<unsigned long>(_attributes.count)];
}

- (nullable instancetype)init
{
return [self initWithDataVersion:nil attributes:nil];
}

// Attributes dictionary is: attributeID => data-value dictionary
- (nullable instancetype)initWithDataVersion:(NSNumber * _Nullable)dataVersion attributes:(NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _Nullable)attributes
{
self = [super init];
if (self == nil) {
return nil;
}

_dataVersion = [dataVersion copy];
_attributes = [NSMutableDictionary dictionaryWithCapacity:attributes.count];
[_attributes addEntriesFromDictionary:attributes];

return self;
}

- (nullable instancetype)initWithCoder:(NSCoder *)decoder
{
self = [super init];
if (self == nil) {
return nil;
}

_dataVersion = [decoder decodeObjectOfClass:[NSNumber class] forKey:sDataVersionKey];
if (_dataVersion != nil && ![_dataVersion isKindOfClass:[NSNumber class]]) {
MTR_LOG_ERROR("MTRDeviceClusterData got %@ for data version, not NSNumber.", _dataVersion);
return nil;
}

static NSSet * const sAttributeValueClasses = [NSSet setWithObjects:[NSDictionary class], [NSArray class], [NSData class], [NSString class], [NSNumber class], nil];
_attributes = [decoder decodeObjectOfClasses:sAttributeValueClasses forKey:sAttributesKey];
if (_attributes != nil && ![_attributes isKindOfClass:[NSDictionary class]]) {
MTR_LOG_ERROR("MTRDeviceClusterData got %@ for attributes, not NSDictionary.", _attributes);
return nil;
}

return self;
}

- (void)encodeWithCoder:(NSCoder *)coder
{
[coder encodeObject:self.dataVersion forKey:sDataVersionKey];
[coder encodeObject:self.attributes forKey:sAttributesKey];
}

- (id)copyWithZone:(NSZone *)zone
{
return [[MTRDeviceClusterData alloc] initWithDataVersion:_dataVersion attributes:_attributes];
}

- (BOOL)isEqualToClusterData:(MTRDeviceClusterData *)otherClusterData
{
return MTREqualObjects(_dataVersion, otherClusterData.dataVersion)
&& MTREqualObjects(_attributes, otherClusterData.attributes);
}

- (BOOL)isEqual:(id)object
{
if ([object class] != [self class]) {
return NO;
}

return [self isEqualToClusterData:object];
}

@end
4 changes: 3 additions & 1 deletion src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
#import <Matter/MTRDefines.h>
#import <Matter/MTRDeviceController.h>
#import <Matter/MTRDeviceControllerStorageDelegate.h>
#import <Matter/MTRDevice_Internal.h>

#import "MTRDeviceClusterData.h"
#import "MTRDevice_Internal.h"

#include <lib/core/CHIPError.h>

Expand Down
25 changes: 22 additions & 3 deletions src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,7 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
{
os_unfair_lock_assert_owner(self.deviceMapLock);

MTRDevice * deviceToReturn = [[MTRDevice_Concrete alloc] initWithNodeID:nodeID controller:self];
MTRDevice_Concrete * deviceToReturn = [[MTRDevice_Concrete 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
Expand Down Expand Up @@ -1190,7 +1190,16 @@ - (MTRDevice *)_setupDeviceForNodeID:(NSNumber *)nodeID prefetchedClusterData:(N
std::lock_guard lock(*self.deviceMapLock);
NSMutableDictionary<NSNumber *, NSNumber *> * deviceAttributeCounts = [NSMutableDictionary dictionary];
for (NSNumber * nodeID in self.nodeIDToDeviceMap) {
deviceAttributeCounts[nodeID] = @([[self.nodeIDToDeviceMap objectForKey:nodeID] unitTestAttributeCount]);
MTRDevice * device = [self.nodeIDToDeviceMap objectForKey:nodeID];

// TODO: Can we not just assume this isKindOfClass test is true? Would be
// really nice if we had compile-time checking for this somehow...
if (![device isKindOfClass:MTRDevice_Concrete.class]) {
continue;
}

auto * concreteDevice = static_cast<MTRDevice_Concrete *>(device);
deviceAttributeCounts[nodeID] = @([concreteDevice unitTestAttributeCount]);
}
return deviceAttributeCounts;
}
Expand Down Expand Up @@ -1389,9 +1398,19 @@ - (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConn
// Get the corresponding MTRDevice object to determine if the case/subscription pool is to be used
MTRDevice * device = [self deviceForNodeID:@(nodeID)];

// TODO: Can we not just assume this isKindOfClass test is true? Would be
// really nice if we had compile-time checking for this somehow...
if (![device isKindOfClass:MTRDevice_Concrete.class]) {
MTR_LOG_ERROR("%@ somehow has %@ instead of MTRDevice_Concrete for node ID 0x%016llX (%llu)", self, device, nodeID, nodeID);
completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
return;
}

auto * concreteDevice = static_cast<MTRDevice_Concrete *>(device);

// In the case that this device is known to use thread, queue this with subscription attempts as well, to
// help with throttling Thread traffic.
if ([device deviceUsesThread]) {
if ([concreteDevice deviceUsesThread]) {
MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0)];
[workItem setReadyHandler:^(id _Nonnull context, NSInteger retryCount, MTRAsyncWorkCompletionBlock _Nonnull workItemCompletion) {
MTRInternalDeviceConnectionCallback completionWrapper = ^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#import <os/lock.h>

#import "MTRBaseDevice.h"
#import "MTRDeviceClusterData.h"
#import "MTRDeviceController.h"
#import "MTRDeviceControllerDataStore.h"
#import "MTRDeviceControllerDelegate.h"
Expand Down
23 changes: 23 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceDataValueDictionary.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

typedef NSDictionary<NSString *, id> * MTRDeviceDataValueDictionary;

NS_ASSUME_NONNULL_END
18 changes: 18 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice_Concrete.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#import <Foundation/Foundation.h>
#import <Matter/MTRDevice.h>

#import "MTRDeviceClusterData.h"
#import "MTRDeviceController_Concrete.h"

NS_ASSUME_NONNULL_BEGIN
Expand All @@ -31,6 +32,23 @@ NS_ASSUME_NONNULL_BEGIN
// false-positives, for example due to compressed fabric id collisions.
- (void)nodeMayBeAdvertisingOperational;

// Method to insert persisted cluster data
// Contains data version information and attribute values.
- (void)setPersistedClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData;

// Method to insert persisted data that pertains to the whole device.
- (void)setPersistedDeviceData:(NSDictionary<NSString *, id> *)data;

// Returns whether this MTRDevice_Concrete uses Thread for communication
- (BOOL)deviceUsesThread;

// For use from MTRDeviceController_Concrete when setting up a device instance.
- (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;

#ifdef DEBUG
- (NSUInteger)unitTestAttributeCount;
#endif

@end

NS_ASSUME_NONNULL_END
5 changes: 3 additions & 2 deletions src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#import "MTRDeviceConnectivityMonitor.h"
#import "MTRDeviceControllerOverXPC.h"
#import "MTRDeviceController_Internal.h"
#import "MTRDeviceDataValueDictionary.h"
#import "MTRDevice_Concrete.h"
#import "MTRDevice_Internal.h"
#import "MTRError_Internal.h"
Expand All @@ -55,6 +56,8 @@
#import <platform/LockTracker.h>
#import <platform/PlatformManager.h>

static NSString * const sLastInitialSubscribeLatencyKey = @"lastInitialSubscribeLatency";

// allow readwrite access to superclass properties
@interface MTRDevice_Concrete ()

Expand Down Expand Up @@ -208,7 +211,6 @@ @interface MTRDevice_Concrete ()
// whatever lock protects the time sync bits.
@property (nonatomic, readonly) os_unfair_lock timeSyncLock;

@property (nonatomic) chip::FabricIndex fabricIndex;
@property (nonatomic) NSMutableArray<NSDictionary<NSString *, id> *> * unreportedEvents;
@property (nonatomic) BOOL receivingReport;
@property (nonatomic) BOOL receivingPrimingReport;
Expand Down Expand Up @@ -367,7 +369,6 @@ - (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceControlle
if (self = [super initForSubclassesWithNodeID:nodeID controller:controller]) {
_timeSyncLock = OS_UNFAIR_LOCK_INIT;
_descriptionLock = OS_UNFAIR_LOCK_INIT;
_fabricIndex = controller.fabricIndex;
_queue
= dispatch_queue_create("org.csa-iot.matter.framework.device.workqueue", DISPATCH_QUEUE_SERIAL_WITH_AUTORELEASE_POOL);
_expectedValueCache = [NSMutableDictionary dictionary];
Expand Down
37 changes: 0 additions & 37 deletions src/darwin/Framework/CHIP/MTRDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ NS_ASSUME_NONNULL_BEGIN

@class MTRAsyncWorkQueue;

typedef NSDictionary<NSString *, id> * MTRDeviceDataValueDictionary;

typedef void (^MTRDevicePerformAsyncBlock)(MTRBaseDevice * baseDevice);

typedef NS_ENUM(NSUInteger, MTRInternalDeviceState) {
Expand All @@ -52,20 +50,6 @@ typedef NS_ENUM(NSUInteger, MTRInternalDeviceState) {
MTRInternalDeviceStateLaterSubscriptionEstablished = 4,
};

/**
* Information about a cluster: data version and known attribute values.
*/
MTR_TESTABLE
@interface MTRDeviceClusterData : NSObject <NSSecureCoding, NSCopying>
@property (nonatomic, nullable) NSNumber * dataVersion;
@property (nonatomic, readonly) NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * attributes; // attributeID => data-value dictionary

- (void)storeValue:(MTRDeviceDataValueDictionary _Nullable)value forAttribute:(NSNumber *)attribute;
- (void)removeValueForAttribute:(NSNumber *)attribute;

- (nullable instancetype)initWithDataVersion:(NSNumber * _Nullable)dataVersion attributes:(NSDictionary<NSNumber *, MTRDeviceDataValueDictionary> * _Nullable)attributes;
@end

// Consider moving utility classes to their own file
#pragma mark - Utility Classes

Expand Down Expand Up @@ -157,22 +141,6 @@ MTR_DIRECT_MEMBERS
@property (nonatomic) dispatch_queue_t queue;
@property (nonatomic, readonly) MTRAsyncWorkQueue<MTRDevice *> * asyncWorkQueue;

// Method to insert persisted cluster data
// Contains data version information and attribute values.
- (void)setPersistedClusterData:(NSDictionary<MTRClusterPath *, MTRDeviceClusterData *> *)clusterData;

// Method to insert persisted data that pertains to the whole device.
- (void)setPersistedDeviceData:(NSDictionary<NSString *, id> *)data;

#ifdef DEBUG
- (NSUInteger)unitTestAttributeCount;
#endif

- (void)setStorageBehaviorConfiguration:(MTRDeviceStorageBehaviorConfiguration *)storageBehaviorConfiguration;

// Returns whether this MTRDevice uses Thread for communication
- (BOOL)deviceUsesThread;

#pragma mark - MTRDevice functionality to deal with delegates.

// Returns YES if any non-null delegates were found
Expand Down Expand Up @@ -208,11 +176,6 @@ MTR_DIRECT_MEMBERS
static NSString * const kDefaultSubscriptionPoolSizeOverrideKey = @"subscriptionPoolSizeOverride";
static NSString * const kTestStorageUserDefaultEnabledKey = @"enableTestStorage";

// ex-MTRDeviceClusterData constants
static NSString * const sDataVersionKey = @"dataVersion";
static NSString * const sAttributesKey = @"attributes";
static NSString * const sLastInitialSubscribeLatencyKey = @"lastInitialSubscribeLatency";

// Declared inside platform, but noting here for reference
// static NSString * const kSRPTimeoutInMsecsUserDefaultKey = @"SRPTimeoutInMSecsOverride";

Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/MTRDevice_XPC.mm
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#import "MTRDeviceControllerStartupParams_Internal.h"
#import "MTRDeviceController_Concrete.h"
#import "MTRDeviceController_XPC.h"
#import "MTRDeviceDataValueDictionary.h"
#import "MTRDevice_Concrete.h"
#import "MTRDevice_Internal.h"
#import "MTRDevice_XPC_Internal.h"
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#import <Matter/Matter.h>

#import "MTRCommandPayloadExtensions_Internal.h"
#import "MTRDeviceClusterData.h"
#import "MTRDeviceControllerLocalTestStorage.h"
#import "MTRDeviceStorageBehaviorConfiguration.h"
#import "MTRDeviceTestDelegate.h"
Expand Down
Loading

0 comments on commit 2574847

Please sign in to comment.