Skip to content

Commit

Permalink
Ensure that MTRDevice invokes provide a timed invoke timeout when nee…
Browse files Browse the repository at this point in the history
…ded. (#29855)

If we know the command being invoked needs a timed invoke, and the caller did
not provide a timed invoke timeout, go ahead and use a default one.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Dec 14, 2023
1 parent 12d9254 commit 1068790
Show file tree
Hide file tree
Showing 13 changed files with 1,421 additions and 50 deletions.
26 changes: 26 additions & 0 deletions src/darwin/Framework/CHIP/MTRCommandTimedCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Copyright (c) 2023 Project CHIP Authors
* All rights reserved.
*
* 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.
*/

#pragma once

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

BOOL MTRCommandNeedsTimedInvoke(NSNumber * aClusterID, NSNumber * aCommandID);

NS_ASSUME_NONNULL_END
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTRDefines_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,6 @@
_Pragma("clang diagnostic pop")
typedef struct {} variable_hidden_by_mtr_hide;
// clang-format on

// Default timed interaction timeout, in ms, if another one is not provided.
#define MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS 10000
50 changes: 36 additions & 14 deletions src/darwin/Framework/CHIP/MTRDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,32 +126,54 @@ typedef NS_ENUM(NSUInteger, MTRDeviceState) {
/**
* Invoke a command with a designated command path
*
* @param commandFields command fields object. The object must be a data-value NSDictionary object
* @param commandFields command fields object. The object must be a data-value NSDictionary object
* as described in the MTRDeviceResponseHandler.
* The attribute must be a Structure, i.e.,
* The value must be a Structure, i.e.,
* the NSDictionary MTRTypeKey key must have the value MTRStructureValueType.
*
* @param expectedValues array of dictionaries containing the expected values in the same format as
* attribute read completion handler. Requires MTRAttributePathKey values.
* See MTRDeviceResponseHandler definition for dictionary details.
* The expectedValues and expectedValueInterval arguments need to be both
* nil or both non-nil, or both will be both ignored.
* @param expectedValues The expected values of attributes that will be affected by the command, if
* any. If these are provided, the relevant attributes will have the provided
* values when read until one of the following happens:
*
* TODO: document better the expectedValues is how this command is expected to change attributes when read, and that the next
* readAttribute will get these values
* 1. Something (another invoke or a write) sets different expected values.
* 2. expectedValueInterval elapses without the device reporting the
* attributes changing their values to the expected values.
* 3. The command invoke fails.
* 4. The device reports some other values for these attributes.
*
* @param expectedValueInterval maximum interval in milliseconds during which reads of the attribute will return the value being
* written. If the value is less than 1, both this value and expectedValues will be ignored.
If this value is greater than UINT32_MAX, it will be clamped to UINT32_MAX.
* The dictionaries in this array are expected to be response-value
* dictionaries as documented in the documentation of
* MTRDeviceResponseHandler, and each one must have an MTRAttributePathKey.
*
* @param timeout timeout in milliseconds for timed invoke, or nil. This value must be within [1, UINT16_MAX], and will be clamped
* to this range.
* The expectedValues and expectedValueInterval arguments need to be both
* nil or both non-nil, or both will be both ignored.
*
* @param expectedValueInterval maximum interval in milliseconds during which reads of the
* attributes that had expected values provided will return the
* expected values. If the value is less than 1, both this value and
* expectedValues will be ignored. If this value is greater than
* UINT32_MAX, it will be clamped to UINT32_MAX.
*
* @param completion response handler will receive either values or error. A
* path-specific error status from the command invocation
* will result in an error being passed to the completion, so
* values will only be passed in when the command succeeds.
*
* If values are passed, the array length will always be 1 and the single
* response-value in it will have an MTRCommandPathKey. If the command
* response is just a success status, there will be no MTRDataKey. If the
* command response has data fields, there will be an MTRDataKey, whose value
* will be of type MTRStructureValueType and describe the response payload.
*/
- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
commandID:(NSNumber *)commandID
commandFields:(id)commandFields
expectedValues:(NSArray<NSDictionary<NSString *, id> *> * _Nullable)expectedValues
expectedValueInterval:(NSNumber * _Nullable)expectedValueInterval
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion MTR_NEWLY_AVAILABLE;

- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
commandID:(NSNumber *)commandID
Expand Down
26 changes: 26 additions & 0 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#import "MTRBaseSubscriptionCallback.h"
#import "MTRCluster.h"
#import "MTRClusterConstants.h"
#import "MTRCommandTimedCheck.h"
#import "MTRDefines_Internal.h"
#import "MTRDeviceController_Internal.h"
#import "MTRDevice_Internal.h"
#import "MTRError_Internal.h"
Expand Down Expand Up @@ -1066,6 +1068,26 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
[_asyncWorkQueue enqueueWorkItem:workItem descriptionWithFormat:@"write %@ %@ %@", endpointID, clusterID, attributeID];
}

- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
commandID:(NSNumber *)commandID
commandFields:(id)commandFields
expectedValues:(NSArray<NSDictionary<NSString *, id> *> * _Nullable)expectedValues
expectedValueInterval:(NSNumber * _Nullable)expectedValueInterval
queue:(dispatch_queue_t)queue
completion:(MTRDeviceResponseHandler)completion
{
[self invokeCommandWithEndpointID:endpointID
clusterID:clusterID
commandID:commandID
commandFields:commandFields
expectedValues:expectedValues
expectedValueInterval:expectedValueInterval
timedInvokeTimeout:nil
queue:queue
completion:completion];
}

- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
clusterID:(NSNumber *)clusterID
commandID:(NSNumber *)commandID
Expand Down Expand Up @@ -1112,6 +1134,10 @@ - (void)_invokeCommandWithEndpointID:(NSNumber *)endpointID
serverSideProcessingTimeout = [serverSideProcessingTimeout copy];
timeout = [timeout copy];

if (timeout == nil && MTRCommandNeedsTimedInvoke(clusterID, commandID)) {
timeout = @(MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS);
}

NSDate * cutoffTime;
if (timeout) {
cutoffTime = [NSDate dateWithTimeIntervalSinceNow:(timeout.doubleValue / 1000)];
Expand Down
7 changes: 4 additions & 3 deletions src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#import "MTRStructsObjc.h"
#import "NSStringSpanConversion.h"
#import "NSDataSpanConversion.h"
#import "MTRDefines_Internal.h"

#include <app-common/zap-generated/cluster-objects.h>
#include <app/util/im-client-callbacks.h>
Expand Down Expand Up @@ -97,7 +98,7 @@ MTR{{cluster}}Cluster{{command}}Params
auto * timedInvokeTimeoutMs = params.timedInvokeTimeoutMs;
{{#if mustUseTimedInvoke}}
if (timedInvokeTimeoutMs == nil) {
timedInvokeTimeoutMs = @(10000);
timedInvokeTimeoutMs = @(MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS);
}
{{/if}}

Expand Down Expand Up @@ -175,9 +176,9 @@ MTR{{cluster}}Cluster{{command}}Params
timedWriteTimeout.SetValue(params.timedWriteTimeout.unsignedShortValue);
}
}
{{#if mustUseTimedInvoke}}
{{#if mustUseTimedWrite}}
if (!timedWriteTimeout.HasValue()) {
timedWriteTimeout.SetValue(10000);
timedWriteTimeout.SetValue(MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS);
}
{{/if}}

Expand Down
5 changes: 3 additions & 2 deletions src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#import "MTRStructsObjc.h"
#import "MTRCommandPayloadsObjc.h"
#import "MTRLogging_Internal.h"
#import "MTRDefines_Internal.h"

#include <app-common/zap-generated/cluster-objects.h>
#include <platform/CHIPDeviceLayer.h>
Expand Down Expand Up @@ -79,7 +80,7 @@ MTR{{cluster}}Cluster{{command}}Params
auto * timedInvokeTimeoutMs = params.timedInvokeTimeoutMs;
{{#if mustUseTimedInvoke}}
if (timedInvokeTimeoutMs == nil) {
timedInvokeTimeoutMs = @(10000);
timedInvokeTimeoutMs = @(MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS);
}
{{/if}}

Expand Down Expand Up @@ -133,7 +134,7 @@ MTR{{cluster}}Cluster{{command}}Params
NSNumber *timedWriteTimeout = params.timedWriteTimeout;
{{#if mustUseTimedWrite}}
if (!timedWriteTimeout) {
timedWriteTimeout = @(10000);
timedWriteTimeout = @(MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS);
}
{{/if}}

Expand Down
51 changes: 51 additions & 0 deletions src/darwin/Framework/CHIP/templates/MTRCommandTimedCheck-src.zapt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
{{> header excludeZapComment=true}}

#import "MTRCommandTimedCheck.h"

#include <app-common/zap-generated/ids/Commands.h>
#include <app-common/zap-generated/ids/Clusters.h>

using namespace chip;
using namespace chip::app;

{{#zcl_clusters}}
{{#if (isSupported (asUpperCamelCase name preserveAcronyms=true))}}
static BOOL CommandNeedsTimedInvokeIn{{asUpperCamelCase name preserveAcronyms=true}}Cluster(AttributeId aAttributeId)
{
using namespace Clusters::{{asUpperCamelCase name}};
switch (aAttributeId) {
{{#zcl_commands}}
{{#if (and (isSupported (asUpperCamelCase ../name preserveAcronyms=true) attribute=(asUpperCamelCase name preserveAcronyms=true))
mustUseTimedInvoke)}}
case Commands::{{asUpperCamelCase name}}::Id: {
return YES;
}
{{/if}}
{{/zcl_commands}}
default: {
return NO;
}
}
}
{{/if}}
{{/zcl_clusters}}

BOOL MTRCommandNeedsTimedInvoke(NSNumber * _Nonnull aClusterID, NSNumber * _Nonnull aCommandID)
{
ClusterId clusterID = static_cast<ClusterId>(aClusterID.unsignedLongLongValue);
CommandId commandID = static_cast<CommandId>(aCommandID.unsignedLongLongValue);

switch (clusterID)
{
{{#zcl_clusters}}
{{#if (isSupported (asUpperCamelCase name preserveAcronyms=true))}}
case Clusters::{{asUpperCamelCase name}}::Id: {
return CommandNeedsTimedInvokeIn{{asUpperCamelCase name preserveAcronyms=true}}Cluster(commandID);
}
{{/if}}
{{/zcl_clusters}}
default: {
return NO;
}
}
}
5 changes: 5 additions & 0 deletions src/darwin/Framework/CHIP/templates/templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@
"path": "MTRAttributeSpecifiedCheck-src.zapt",
"name": "Function to check if attribute is specified",
"output": "src/darwin/Framework/CHIP/zap-generated/MTRAttributeSpecifiedCheck.mm"
},
{
"path": "MTRCommandTimedCheck-src.zapt",
"name": "Function to check if command needs timed invoke",
"output": "src/darwin/Framework/CHIP/zap-generated/MTRCommandTimedCheck.mm"
}
]
}
Loading

0 comments on commit 1068790

Please sign in to comment.