Skip to content

Commit

Permalink
Minor cleanup around read/subscribe(To)AttributePaths.
Browse files Browse the repository at this point in the history
1) Use initWithArray:copyItems:YES when copying arrays, instead of hand-rolling
   the copy.
2) For read, empty paths should just lead to an empty result, not an error, just
   like they would if the read actually happened.

Fixes #26169
  • Loading branch information
bzbarsky-apple committed Apr 20, 2023
1 parent e2341d8 commit 5c8a191
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 29 deletions.
11 changes: 6 additions & 5 deletions src/darwin/Framework/CHIP/MTRBaseDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,10 @@ MTR_NEWLY_AVAILABLE
*
* Lists of attribute and event paths to read can be provided via attributePaths and eventPaths.
*
* The completion will be called with an error if the input parameters are invalid (e.g., both attributePaths and eventPaths are
* empty.) or the entire read interaction fails. Otherwise it will be called with values, which may be empty (e.g. if no paths
* matched the wildcard paths passed in) or may include per-path errors if particular paths failed.
* The completion will be called with an error if the entire read interaction fails. Otherwise it
* will be called with an array of values. This array may be empty (e.g. if no paths matched the
* wildcard paths passed in, or if empty lists of paths were passed in) or may include per-path
* errors if particular paths failed.
*
* If the sum of the lengths of attributePaths and eventPaths exceeds 9, the read may fail due to the device not supporting that
* many read paths.
Expand Down Expand Up @@ -364,8 +365,8 @@ MTR_NEWLY_AVAILABLE
*
* Lists of attribute and event paths to subscribe to can be provided via attributePaths and eventPaths.
*
* The reportHandler will be called with an error if the inputs are invalid (e.g., both attributePaths and eventPaths are
* empty), or if the subscription fails entirely.
* The reportHandler will be called with an error if the subscription fails
* entirely (including when both attributePaths and eventPaths are empty).
*
* If the sum of the lengths of attributePaths and eventPaths exceeds 3, the subscribe may fail due to the device not supporting
* that many paths for a subscription.
Expand Down
39 changes: 15 additions & 24 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -912,26 +912,21 @@ - (void)readAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullable)attri
completion:(MTRDeviceResponseHandler)completion
{
if ((attributePaths == nil || [attributePaths count] == 0) && (eventPaths == nil || [eventPaths count] == 0)) {
// No paths, just return an empty array.
dispatch_async(queue, ^{
completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT]);
completion(@[], nil);
});
return;
}

NSMutableArray<MTRAttributeRequestPath *> * attributes = nil;
NSArray<MTRAttributeRequestPath *> * attributes = nil;
if (attributePaths != nil) {
attributes = [[NSMutableArray alloc] init];
for (MTRAttributeRequestPath * attributePath in attributePaths) {
[attributes addObject:[attributePath copy]];
}
attributes = [[NSArray alloc] initWithArray:attributePaths copyItems:YES];
}

NSMutableArray<MTREventRequestPath *> * events = nil;
NSArray<MTREventRequestPath *> * events = nil;
if (eventPaths != nil) {
events = [[NSMutableArray alloc] init];
for (MTRAttributeRequestPath * eventPath in eventPaths) {
[events addObject:[eventPath copy]];
}
events = [[NSArray alloc] initWithArray:eventPaths copyItems:YES];
}
params = (params == nil) ? nil : [params copy];
auto * bridge = new MTRDataValueDictionaryCallbackBridge(queue, completion,
Expand Down Expand Up @@ -1004,9 +999,9 @@ - (void)readAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullable)attri
chip::app::ReadPrepareParams readParams(session);
[params toReadPrepareParams:readParams];
readParams.mpAttributePathParamsList = attributePathParamsList.Get();
readParams.mAttributePathParamsListSize = [attributePaths count];
readParams.mAttributePathParamsListSize = [attributes count];
readParams.mpEventPathParamsList = eventPathParamsList.Get();
readParams.mEventPathParamsListSize = [eventPaths count];
readParams.mEventPathParamsListSize = [events count];

AttributePathParams * attributePathParamsListToFree = attributePathParamsList.Get();
EventPathParams * eventPathParamsListToFree = eventPathParamsList.Get();
Expand Down Expand Up @@ -1286,8 +1281,10 @@ - (void)subscribeToAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullabl
resubscriptionScheduled:(MTRDeviceResubscriptionScheduledHandler _Nullable)resubscriptionScheduled
{
if ((attributePaths == nil || [attributePaths count] == 0) && (eventPaths == nil || [eventPaths count] == 0)) {
// Per spec a server would respond InvalidAction to this, so just go
// ahead and do that.
dispatch_async(queue, ^{
reportHandler(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT]);
reportHandler(nil, [MTRError errorForIMStatus:StatusIB(Status::InvalidAction)]);
});
return;
}
Expand All @@ -1301,20 +1298,14 @@ - (void)subscribeToAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullabl
}

// Copy params before going async.
NSMutableArray<MTRAttributeRequestPath *> * attributes = nil;
NSArray<MTRAttributeRequestPath *> * attributes = nil;
if (attributePaths != nil) {
attributes = [[NSMutableArray alloc] init];
for (MTRAttributeRequestPath * attributePath in attributePaths) {
[attributes addObject:[attributePath copy]];
}
attributes = [[NSArray alloc] initWithArray:attributePaths copyItems:YES];
}

NSMutableArray<MTREventRequestPath *> * events = nil;
NSArray<MTREventRequestPath *> * events = nil;
if (eventPaths != nil) {
events = [[NSMutableArray alloc] init];
for (MTRAttributeRequestPath * eventPath in eventPaths) {
[events addObject:[eventPath copy]];
}
events = [[NSArray alloc] initWithArray:eventPaths copyItems:YES];
}

params = (params == nil) ? nil : [params copy];
Expand Down

0 comments on commit 5c8a191

Please sign in to comment.