Skip to content

Commit

Permalink
Fix API review comments for MTRAttributeCacheContainer.
Browse files Browse the repository at this point in the history
* Fix function naming to indicate we are reading a path (maybe multiple
  attributes), not just an attribute.
* Fix documentation.
* Rename to MTRClusterStateCacheContainer.

Fixes project-chip#22532

Addresses part of project-chip#22420
  • Loading branch information
bzbarsky-apple committed Sep 14, 2022
1 parent 6527ae3 commit 2defe67
Show file tree
Hide file tree
Showing 25 changed files with 8,705 additions and 8,463 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class SubscribeEvent : public ModelCommand {

[device subscribeWithQueue:callbackQueue
params:params
attributeCacheContainer:nil
clusterStateCacheContainer:nil
attributeReportHandler:^(NSArray * value) {
SetCommandExitStatus(CHIP_NO_ERROR);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ - (void)reportFromUserEnteredSettings
maxInterval:@(maxIntervalSeconds)];
[chipDevice subscribeWithQueue:dispatch_get_main_queue()
params:params
attributeCacheContainer:nil
clusterStateCacheContainer:nil
attributeReportHandler:^(NSArray * _Nullable reports) {
if (!reports)
return;
Expand Down
16 changes: 8 additions & 8 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ extern NSString * const MTRNullValueType;
extern NSString * const MTRStructureValueType;
extern NSString * const MTRArrayValueType;

@class MTRAttributeCacheContainer;
@class MTRClusterStateCacheContainer;
@class MTRReadParams;
@class MTRSubscribeParams;
@class MTRDeviceController;
Expand Down Expand Up @@ -173,13 +173,13 @@ extern NSString * const MTRArrayValueType;
* attempts fail.
*/
- (void)subscribeWithQueue:(dispatch_queue_t)queue
params:(MTRSubscribeParams *)params
attributeCacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(MTRDeviceErrorHandler)errorHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduled;
params:(MTRSubscribeParams *)params
clusterStateCacheContainer:(MTRClusterStateCacheContainer * _Nullable)clusterStateCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(MTRDeviceErrorHandler)errorHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduled;

/**
* Reads the given attribute path from the device.
Expand Down
36 changes: 18 additions & 18 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
* limitations under the License.
*/

#import "MTRAttributeCacheContainer_Internal.h"
#import "MTRAttributeTLVValueDecoder_Internal.h"
#import "MTRBaseDevice_Internal.h"
#import "MTRBaseSubscriptionCallback.h"
#import "MTRCallbackBridgeBase_internal.h"
#import "MTRCluster.h"
#import "MTRClusterStateCacheContainer_Internal.h"
#import "MTRError_Internal.h"
#import "MTREventTLVValueDecoder_Internal.h"
#import "MTRLogging.h"
Expand Down Expand Up @@ -299,13 +299,13 @@ - (void)invalidateCASESession
} // anonymous namespace

- (void)subscribeWithQueue:(dispatch_queue_t)queue
params:(MTRSubscribeParams *)params
attributeCacheContainer:(MTRAttributeCacheContainer * _Nullable)attributeCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(void (^)(NSError * error))errorHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduled
params:(MTRSubscribeParams *)params
clusterStateCacheContainer:(MTRClusterStateCacheContainer * _Nullable)clusterStateCacheContainer
attributeReportHandler:(MTRDeviceReportHandler _Nullable)attributeReportHandler
eventReportHandler:(MTRDeviceReportHandler _Nullable)eventReportHandler
errorHandler:(void (^)(NSError * error))errorHandler
subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduled
{
if (self.isPASEDevice) {
// We don't support subscriptions over PASE.
Expand Down Expand Up @@ -344,19 +344,19 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue

std::unique_ptr<SubscriptionCallback> callback;
std::unique_ptr<ReadClient> readClient;
std::unique_ptr<ClusterStateCache> attributeCache;
if (attributeCacheContainer) {
__weak MTRAttributeCacheContainer * weakPtr = attributeCacheContainer;
std::unique_ptr<ClusterStateCache> clusterStateCache;
if (clusterStateCacheContainer) {
__weak MTRClusterStateCacheContainer * weakPtr = clusterStateCacheContainer;
callback = std::make_unique<SubscriptionCallback>(queue, attributeReportHandler, eventReportHandler,
errorHandler, resubscriptionScheduled, subscriptionEstablished, ^{
MTRAttributeCacheContainer * container = weakPtr;
MTRClusterStateCacheContainer * container = weakPtr;
if (container) {
container.cppAttributeCache = nullptr;
container.cppClusterStateCache = nullptr;
}
});
attributeCache = std::make_unique<ClusterStateCache>(*callback.get());
clusterStateCache = std::make_unique<ClusterStateCache>(*callback.get());
readClient = std::make_unique<ReadClient>(InteractionModelEngine::GetInstance(), exchangeManager,
attributeCache->GetBufferedCallback(), ReadClient::InteractionType::Subscribe);
clusterStateCache->GetBufferedCallback(), ReadClient::InteractionType::Subscribe);
} else {
callback = std::make_unique<SubscriptionCallback>(queue, attributeReportHandler, eventReportHandler,
errorHandler, resubscriptionScheduled, subscriptionEstablished, nil);
Expand All @@ -382,10 +382,10 @@ - (void)subscribeWithQueue:(dispatch_queue_t)queue
return;
}

if (attributeCacheContainer) {
attributeCacheContainer.cppAttributeCache = attributeCache.get();
if (clusterStateCacheContainer) {
clusterStateCacheContainer.cppClusterStateCache = clusterStateCache.get();
// ClusterStateCache will be deleted when OnDone is called or an error is encountered as well.
callback->AdoptAttributeCache(std::move(attributeCache));
callback->AdoptClusterStateCache(std::move(clusterStateCache));
}
// Callback and ReadClient will be deleted when OnDone is called or an error is
// encountered.
Expand Down
6 changes: 3 additions & 3 deletions src/darwin/Framework/CHIP/MTRBaseSubscriptionCallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac

// We need to exist to get a ReadClient, so can't take this as a constructor argument.
void AdoptReadClient(std::unique_ptr<chip::app::ReadClient> aReadClient) { mReadClient = std::move(aReadClient); }
void AdoptAttributeCache(std::unique_ptr<chip::app::ClusterStateCache> aAttributeCache)
void AdoptClusterStateCache(std::unique_ptr<chip::app::ClusterStateCache> aClusterStateCache)
{
mAttributeCache = std::move(aAttributeCache);
mClusterStateCache = std::move(aClusterStateCache);
}

protected:
Expand Down Expand Up @@ -142,7 +142,7 @@ class MTRBaseSubscriptionCallback : public chip::app::ClusterStateCache::Callbac
// error callback, but not both, by tracking whether we have a queued-up
// deletion.
std::unique_ptr<chip::app::ReadClient> mReadClient;
std::unique_ptr<chip::app::ClusterStateCache> mAttributeCache;
std::unique_ptr<chip::app::ClusterStateCache> mClusterStateCache;
bool mHaveQueuedDeletion = false;
OnDoneHandler _Nullable mOnDoneHandler = nil;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@

#import <Foundation/Foundation.h>

#import "MTRAttributeCacheContainer.h"
#import "MTRClusterStateCacheContainer.h"

NS_ASSUME_NONNULL_BEGIN

@interface MTRAttributeCacheContainer (XPC)
@interface MTRClusterStateCacheContainer (XPC)
- (void)setXPCConnection:(MTRDeviceControllerXPCConnection *)xpcConnection
controllerID:(id<NSCopying>)controllerID
deviceID:(NSNumber *)deviceID;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,27 @@ NS_ASSUME_NONNULL_BEGIN

@class MTRSubscribeParams;

@interface MTRAttributeCacheContainer : NSObject
@interface MTRClusterStateCacheContainer : NSObject

/**
* Reads an attribute with specific attribute path
* Reads the given attribute path from the cluster state cache inside
* this cache container.
*
* @param endpointID endpoint ID of the attribute
* @param clusterID cluster ID of the attribute
* @param attributeID attribute ID of the attribute
* @param endpointID endpoint ID of the attribute. nil means wildcard.
* @param clusterID cluster ID of the attribute nil means wildcard.
* @param attributeID attribute ID of the attribute. nil means wildcard.
* @param queue client queue to dispatch the completion handler through
* @param completion block to receive the result.
* "values" received by the block will have the same format of object as the one received by completion block
* of CHIPDevice readAttributeWithEndpointID:clusterID:attributeID:queue:completion method.
* "values" received by the block will have the same format of object as the one received by the completion block
* of the MTRBaseDevice readAttributePathWithEndpointID:clusterID:attributeID:queue:completion method.
*
* @note: not all combinations of wildcards might be supported.
*/
- (void)readAttributeWithEndpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion;
- (void)readAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#import <Foundation/Foundation.h>

#import "MTRAttributeCacheContainer_Internal.h"
#import "MTRClusterStateCacheContainer_Internal.h"
#import "MTRBaseDevice_Internal.h"
#import "MTRCluster.h"
#import "MTRDeviceControllerXPCConnection.h"
Expand All @@ -31,12 +31,12 @@

using namespace chip;

@implementation MTRAttributeCacheContainer
@implementation MTRClusterStateCacheContainer

- (instancetype)init
{
if ([super init]) {
_cppAttributeCache = nullptr;
_cppClusterStateCache = nullptr;
_shouldUseXPC = NO;
}
return self;
Expand All @@ -52,7 +52,7 @@ - (void)setXPCConnection:(MTRDeviceControllerXPCConnection *)xpcConnection
self.shouldUseXPC = YES;
}

static CHIP_ERROR AppendAttibuteValueToArray(
static CHIP_ERROR AppendAttributeValueToArray(
const chip::app::ConcreteAttributePath & path, chip::app::ClusterStateCache * cache, NSMutableArray * array)
{
chip::TLV::TLVReader reader;
Expand All @@ -78,11 +78,11 @@ static CHIP_ERROR AppendAttibuteValueToArray(
return err;
}

- (void)readAttributeWithEndpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion
- (void)readAttributePathWithEndpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion
{
__auto_type completionHandler = ^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
dispatch_async(queue, ^{
Expand All @@ -102,7 +102,7 @@ - (void)readAttributeWithEndpointID:(NSNumber * _Nullable)endpointID
[xpcConnection
getProxyHandleWithCompletion:^(dispatch_queue_t _Nonnull queue, MTRDeviceControllerXPCProxyHandle * _Nullable handle) {
if (handle) {
[handle.proxy readAttributeCacheWithController:controllerId
[handle.proxy readClusterStateCacheWithController:controllerId
nodeID:nodeId
endpointID:endpointID
clusterID:clusterID
Expand All @@ -127,7 +127,7 @@ - (void)readAttributeWithEndpointID:(NSNumber * _Nullable)endpointID
return;
}

if (!self.cppAttributeCache) {
if (!self.cppClusterStateCache) {
MTR_LOG_ERROR("Error: No attribute cache available to read from");
completionHandler(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
return;
Expand All @@ -136,39 +136,39 @@ - (void)readAttributeWithEndpointID:(NSNumber * _Nullable)endpointID
NSMutableArray * result = [[NSMutableArray alloc] init];
CHIP_ERROR err = CHIP_NO_ERROR;
if (endpointID == nil) {
err = self.cppAttributeCache->ForEachAttribute(
err = self.cppClusterStateCache->ForEachAttribute(
static_cast<chip::ClusterId>([clusterID unsignedLongValue]), [&](const app::ConcreteAttributePath & path) {
if (attributeID == nil
|| static_cast<chip::AttributeId>([attributeID unsignedLongValue]) == path.mAttributeId) {
(void) AppendAttibuteValueToArray(path, self.cppAttributeCache, result);
(void) AppendAttributeValueToArray(path, self.cppClusterStateCache, result);
}
return CHIP_NO_ERROR;
});
} else if (clusterID == nil) {
err = self.cppAttributeCache->ForEachCluster(
err = self.cppClusterStateCache->ForEachCluster(
static_cast<chip::EndpointId>([endpointID unsignedShortValue]), [&](chip::ClusterId enumeratedClusterId) {
(void) self.cppAttributeCache->ForEachAttribute(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
(void) self.cppClusterStateCache->ForEachAttribute(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
enumeratedClusterId, [&](const app::ConcreteAttributePath & path) {
if (attributeID == nil
|| static_cast<chip::AttributeId>([attributeID unsignedLongValue]) == path.mAttributeId) {
(void) AppendAttibuteValueToArray(path, self.cppAttributeCache, result);
(void) AppendAttributeValueToArray(path, self.cppClusterStateCache, result);
}
return CHIP_NO_ERROR;
});
return CHIP_NO_ERROR;
});
} else if (attributeID == nil) {
err = self.cppAttributeCache->ForEachAttribute(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
err = self.cppClusterStateCache->ForEachAttribute(static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
static_cast<chip::ClusterId>([clusterID unsignedLongValue]), [&](const app::ConcreteAttributePath & path) {
(void) AppendAttibuteValueToArray(path, self.cppAttributeCache, result);
(void) AppendAttributeValueToArray(path, self.cppClusterStateCache, result);
return CHIP_NO_ERROR;
});
} else {
app::ConcreteAttributePath path;
path.mEndpointId = static_cast<chip::EndpointId>([endpointID unsignedShortValue]);
path.mClusterId = static_cast<chip::ClusterId>([clusterID unsignedLongValue]);
path.mAttributeId = static_cast<chip::AttributeId>([attributeID unsignedLongValue]);
err = AppendAttibuteValueToArray(path, self.cppAttributeCache, result);
err = AppendAttributeValueToArray(path, self.cppClusterStateCache, result);
}
if (err == CHIP_NO_ERROR) {
completionHandler(result, nil);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@

#import <Foundation/Foundation.h>

#import "MTRAttributeCacheContainer.h"
#import "MTRClusterStateCacheContainer.h"
#import "MTRDeviceControllerOverXPC.h"

#include <app/ClusterStateCache.h>

NS_ASSUME_NONNULL_BEGIN

@interface MTRAttributeCacheContainer ()
@interface MTRClusterStateCacheContainer ()

@property (atomic, readwrite, nullable) chip::app::ClusterStateCache * cppAttributeCache;
@property (atomic, readwrite, nullable) chip::app::ClusterStateCache * cppClusterStateCache;
@property (nonatomic, readwrite, copy) NSNumber * deviceID;
@property (nonatomic, readwrite, weak, nullable) MTRDeviceControllerXPCConnection * xpcConnection;
@property (nonatomic, readwrite, strong, nullable) id<NSCopying> xpcControllerID;
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ - (void)subscribeWithMinInterval:(uint16_t)minInterval maxInterval:(uint16_t)max

std::unique_ptr<SubscriptionCallback> callback;
std::unique_ptr<ReadClient> readClient;
std::unique_ptr<ClusterStateCache> attributeCache;
std::unique_ptr<ClusterStateCache> clusterStateCache;
callback = std::make_unique<SubscriptionCallback>(
self.queue,
^(NSArray * value) {
Expand Down
12 changes: 6 additions & 6 deletions src/darwin/Framework/CHIP/MTRDeviceController+XPC.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ typedef void (^MTRValuesHandler)(id _Nullable values, NSError * _Nullable error)
/**
* Requests reading attribute cache
*/
- (void)readAttributeCacheWithController:(id _Nullable)controller
nodeID:(NSNumber *)nodeID
endpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
completion:(MTRValuesHandler)completion;
- (void)readClusterStateCacheWithController:(id _Nullable)controller
nodeID:(NSNumber *)nodeID
endpointID:(NSNumber * _Nullable)endpointID
clusterID:(NSNumber * _Nullable)clusterID
attributeID:(NSNumber * _Nullable)attributeID
completion:(MTRValuesHandler)completion;

@end

Expand Down
Loading

0 comments on commit 2defe67

Please sign in to comment.