Ensure that MTRDevice invokes provide a timed invoke timeout when nee…

…ded. (#29855) (#29871)

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.

src/darwin/Framework/CHIP/MTRCommandTimedCheck.h

@@ -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

src/darwin/Framework/CHIP/MTRDefines_Internal.h

@@ -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

src/darwin/Framework/CHIP/MTRDevice.h

@@ -126,32 +126,56 @@ 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
- *                      as described in the MTRDeviceResponseHandler.
- *                      The attribute 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 commandFields command fields object. If not nil, the object must be a data-value
+ *                      NSDictionary object as described in the MTRDeviceResponseHandler
+ *                      documentation. The value must be a Structure, i.e., the NSDictionary
+ *                      MTRTypeKey key must have the value MTRStructureValueType.
  *
- * 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
+ *                      If commandFields is nil, it will be treated as a Structure with no fields.
  *
- * @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.
+ * @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:
  *
- * @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.
+ *                       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.
+ *
+ *                       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.
+ *
+ *                       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:(NSDictionary<NSString *, id> * _Nullable)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

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -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"
@@ -1087,6 +1089,33 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
     [_asyncWorkQueue enqueueWorkItem:workItem descriptionWithFormat:@"write %@ %@ %@", endpointID, clusterID, attributeID];
 }
 
+- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
+                          clusterID:(NSNumber *)clusterID
+                          commandID:(NSNumber *)commandID
+                      commandFields:(NSDictionary<NSString *, id> * _Nullable)commandFields
+                     expectedValues:(NSArray<NSDictionary<NSString *, id> *> * _Nullable)expectedValues
+              expectedValueInterval:(NSNumber * _Nullable)expectedValueInterval
+                              queue:(dispatch_queue_t)queue
+                         completion:(MTRDeviceResponseHandler)completion
+{
+    if (commandFields == nil) {
+        commandFields = @{
+            MTRTypeKey : MTRStructureValueType,
+            MTRValueKey : @[],
+        };
+    }
+
+    [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
@@ -1133,6 +1162,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)];

src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt

@@ -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>
@@ -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}}
 
@@ -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}}
 

src/darwin/Framework/CHIP/templates/MTRClusters-src.zapt

@@ -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>
@@ -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}}
 
@@ -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}}
 

src/darwin/Framework/CHIP/templates/MTRCommandTimedCheck-src.zapt

@@ -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;
+        }
+    }
+}

src/darwin/Framework/CHIP/templates/templates.json

@@ -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"
         }
     ]
 }