Add a way for Darwin invokes to specify a server-side processing time…

…out. (#24552)

The default 2s timeout is way too low for some commands (like ScanNetworks).

Fixes #24333

src/darwin/Framework/CHIP/MTRBaseClusterUtils.h

@@ -25,6 +25,7 @@
 #include <lib/core/DataModelTypes.h>
 #include <lib/core/TLV.h>
 #include <lib/support/CHIPMem.h>
+#include <system/SystemClock.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -378,11 +379,18 @@ template <typename InvokeBridgeType, typename ResponseType> class MTRInvokeCallb
     bool mCalledCallback = false;
 };
 
+/**
+ * timedInvokeTimeoutMs, if provided, is how long the server will wait for us to
+ * send the invoke after we sent the Timed Request message.
+ *
+ * invokeTimeout, if provided, will have possible MRP latency added to it and
+ * the result is how long we will wait for the server to respond.
+ */
 template <typename BridgeType, typename RequestDataType>
 CHIP_ERROR MTRStartInvokeInteraction(BridgeType * _Nonnull bridge, const RequestDataType & requestData,
     chip::Messaging::ExchangeManager & exchangeManager, const chip::SessionHandle & session,
     typename BridgeType::SuccessCallbackType successCb, MTRErrorCallback failureCb, chip::EndpointId endpoint,
-    chip::Optional<uint16_t> timedInvokeTimeoutMs)
+    chip::Optional<uint16_t> timedInvokeTimeoutMs, chip::Optional<chip::System::Clock::Timeout> invokeTimeout)
 {
     auto callback = chip::Platform::MakeUnique<MTRInvokeCallback<BridgeType, typename RequestDataType::ResponseType>>(
         bridge, successCb, failureCb);
@@ -395,7 +403,11 @@ CHIP_ERROR MTRStartInvokeInteraction(BridgeType * _Nonnull bridge, const Request
     chip::app::CommandPathParams commandPath(endpoint, 0, RequestDataType::GetClusterId(), RequestDataType::GetCommandId(),
         chip::app::CommandPathFlags::kEndpointIdValid);
     ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, requestData, timedInvokeTimeoutMs));
-    ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
+
+    if (invokeTimeout.HasValue()) {
+        invokeTimeout.SetValue(session->ComputeRoundTripTimeout(invokeTimeout.Value()));
+    }
+    ReturnErrorOnFailure(commandSender->SendCommandRequest(session, invokeTimeout));
 
     callback->AdoptCommandSender(std::move(commandSender));
     callback.release();

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -22,6 +22,7 @@
 #import "MTRCluster.h"
 #import "MTRClusterStateCacheContainer_Internal.h"
 #import "MTRCluster_Internal.h"
+#import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
 #import "MTREventTLVValueDecoder_Internal.h"
 #import "MTRLogging_Internal.h"
@@ -1132,7 +1133,11 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
 
             ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, MTRDataValueDictionaryDecodableType(commandFields),
                 (timeoutMs == nil) ? NullOptional : Optional<uint16_t>([timeoutMs unsignedShortValue])));
-            ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
+
+            // We don't have a way to communicate a non-default invoke timeout
+            // here for now.
+            // TODO: https://github.com/project-chip/connectedhomeip/issues/24563
+            ReturnErrorOnFailure(commandSender->SendCommandRequest(session, NullOptional));
 
             decoder.release();
             commandSender.release();

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

@@ -9,6 +9,7 @@
 #import "MTRCluster_Internal.h"
 #import "MTRClusterStateCacheContainer_Internal.h"
 #import "MTRCommandPayloadsObjc.h"
+#import "MTRDevice_Internal.h"
 #import "MTRStructsObjc.h"
 
 #include <lib/support/CHIPListUtils.h>
@@ -20,6 +21,9 @@ using chip::Callback::Cancelable;
 using namespace chip::app::Clusters;
 using chip::Messaging::ExchangeManager;
 using chip::SessionHandle;
+using chip::Optional;
+using chip::System::Clock::Timeout;
+using chip::System::Clock::Seconds16;
 
 // NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks): Linter is unable to locate the delete on these objects.
 {{#chip_client_clusters includeAll=true}}
@@ -80,13 +84,21 @@ MTR{{cluster}}Cluster{{command}}Params
       {{/if}}
       ^(ExchangeManager & exchangeManager, const SessionHandle & session, {{>callbackName}}CallbackType successCb, MTRErrorCallback failureCb, MTRCallbackBridgeBase * bridge) {
         auto * typedBridge = static_cast<MTR{{>callbackName}}CallbackBridge *>(bridge);
-        chip::Optional<uint16_t> timedInvokeTimeoutMs;
+        Optional<uint16_t> timedInvokeTimeoutMs;
+        Optional<Timeout> invokeTimeout;
         ListFreer listFreer;
         {{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::Type request;
         if (params != nil) {
           if (params.timedInvokeTimeoutMs != nil) {
+            params.timedInvokeTimeoutMs = MTRClampedNumber(params.timedInvokeTimeoutMs, @(1), @(UINT16_MAX));
             timedInvokeTimeoutMs.SetValue(params.timedInvokeTimeoutMs.unsignedShortValue);
           }
+          if (params.serverSideProcessingTimeout != nil) {
+            // Clamp to a number of seconds that will not overflow 32-bit
+            // int when converted to ms.
+            auto * serverSideProcessingTimeout = MTRClampedNumber(params.serverSideProcessingTimeout, @(0), @(UINT16_MAX));
+            invokeTimeout.SetValue(Seconds16(serverSideProcessingTimeout.unsignedShortValue));
+          }
         }
         {{#if mustUseTimedInvoke}}
         if (!timedInvokeTimeoutMs.HasValue()) {
@@ -107,7 +119,7 @@ MTR{{cluster}}Cluster{{command}}Params
           {{/last}}
         {{/chip_cluster_command_arguments}}
 
-        return MTRStartInvokeInteraction(typedBridge, request, exchangeManager, session, successCb, failureCb, self->_endpoint, timedInvokeTimeoutMs);
+        return MTRStartInvokeInteraction(typedBridge, request, exchangeManager, session, successCb, failureCb, self->_endpoint, timedInvokeTimeoutMs, invokeTimeout);
     });
     std::move(*bridge).DispatchAction(self.device);
 }

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

@@ -3,6 +3,7 @@
 #import <Foundation/Foundation.h>
 
 #import "MTRAsyncCallbackWorkQueue.h"
+#import "MTRBaseClusterUtils.h"
 #import "MTRBaseDevice_Internal.h"
 #import "MTRClusterConstants.h"
 #import "MTRClusters_Internal.h"
@@ -22,6 +23,9 @@ using chip::Callback::Cancelable;
 using namespace chip::app::Clusters;
 using chip::Messaging::ExchangeManager;
 using chip::SessionHandle;
+using chip::Optional;
+using chip::System::Clock::Timeout;
+using chip::System::Clock::Seconds16;
 
 static void MTRClustersLogEnqueue(NSString *logPrefix, MTRAsyncCallbackWorkQueue *workQueue) {
     MTR_LOG_INFO("%@ enqueueWorkItem %@", logPrefix, workQueue);
@@ -120,12 +124,22 @@ MTRCommandIDTypeCluster{{cluster}}Command{{command}}ID
             [workItem endWork];
           },
           ^(ExchangeManager & exchangeManager, const SessionHandle & session, {{>callbackName}}CallbackType successCb, MTRErrorCallback failureCb, MTRCallbackBridgeBase * bridge) {
-            chip::Optional<uint16_t> timedInvokeTimeoutMs;
+            auto * typedBridge = static_cast<MTR{{>callbackName}}CallbackBridge *>(bridge);
+            Optional<uint16_t> timedInvokeTimeoutMs;
+            Optional<Timeout> invokeTimeout;
             ListFreer listFreer;
             {{asUpperCamelCase parent.name}}::Commands::{{asUpperCamelCase name}}::Type request;
             if (timedInvokeTimeoutMsParam != nil) {
               timedInvokeTimeoutMs.SetValue(timedInvokeTimeoutMsParam.unsignedShortValue);
             }
+            if (params != nil) {
+              if (params.serverSideProcessingTimeout != nil) {
+                // Clamp to a number of seconds that will not overflow 32-bit
+                // int when converted to ms.
+                auto * serverSideProcessingTimeout = MTRClampedNumber(params.serverSideProcessingTimeout, @(0), @(UINT16_MAX));
+                invokeTimeout.SetValue(Seconds16(serverSideProcessingTimeout.unsignedShortValue));
+              }
+            }
             {{#if mustUseTimedInvoke}}
             if (!timedInvokeTimeoutMs.HasValue()) {
               timedInvokeTimeoutMs.SetValue(10000);
@@ -145,8 +159,7 @@ MTRCommandIDTypeCluster{{cluster}}Command{{command}}ID
               {{/last}}
             {{/chip_cluster_command_arguments}}
 
-            chip::Controller::{{asUpperCamelCase parent.name}}Cluster cppCluster(exchangeManager, session, self->_endpoint);
-            return cppCluster.InvokeCommand(request, bridge, successCb, failureCb, timedInvokeTimeoutMs);
+            return MTRStartInvokeInteraction(typedBridge, request, exchangeManager, session, successCb, failureCb, self->_endpoint, timedInvokeTimeoutMs, invokeTimeout);
         });
         std::move(*bridge).DispatchAction(baseDevice);
     };

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

@@ -19,6 +19,9 @@ NS_ASSUME_NONNULL_BEGIN
 {{#if (or (isStrEqual source "client")
           (wasIntroducedBeforeRelease "First major API revamp" (compatClusterNameRemapping parent.name) command=(compatCommandNameRemapping parent.name name)))}}
     _timedInvokeTimeoutMs = nil;
+{{/if}}
+{{#if (isStrEqual source "client")}}
+    _serverSideProcessingTimeout = nil;
 {{/if}}
   }
   return self;
@@ -35,6 +38,9 @@ NS_ASSUME_NONNULL_BEGIN
           (wasIntroducedBeforeRelease "First major API revamp" (compatClusterNameRemapping parent.name) command=(compatCommandNameRemapping parent.name name)))}}
   other.timedInvokeTimeoutMs = self.timedInvokeTimeoutMs;
 {{/if}}
+{{#if (isStrEqual source "client")}}
+  other.serverSideProcessingTimeout = self.serverSideProcessingTimeout;
+{{/if}}
 
   return other;
 }

src/darwin/Framework/CHIP/templates/MTRCommandPayloadsObjc.zapt

@@ -40,6 +40,15 @@ NS_ASSUME_NONNULL_BEGIN
  *
  */
 @property (nonatomic, copy, nullable) NSNumber * timedInvokeTimeoutMs;
+
+/**
+ * Controls how much time, in seconds, we will allow for the server to process the command.
+ *
+ * The command will then time out if that much time, plus an allowance for retransmits due to network failures, passes.
+ *
+ * If nil, the framework will try to select an appropriate timeout value itself.
+ */
+ @property (nonatomic, copy, nullable) NSNumber * serverSideProcessingTimeout;
 {{! This is using the pre-renaming names for the isAvailableBefore test, because the pre-rename things inherit
     from the post-rename ones and need to have this selector.}}
 {{else if (wasIntroducedBeforeRelease "First major API revamp" (compatClusterNameRemapping parent.name) command=(compatCommandNameRemapping parent.name name))}}