Update CommandHandler to support batch commands

src/app/CommandHandler.cpp

@@ -593,7 +593,7 @@ CHIP_ERROR CommandHandler::PrepareCommand(const ConcreteCommandPath & aResponseC
     // At this point application supports Batch Invoke Commands since CommandPathRegistry has more than 1 entry,
     // but application is calling the deprecated PrepareCommand. We have no way to determine the associated CommandRef
     // to put into the InvokeResponse.
-    VerifyOrDieWithMsg(countOfPathRegistryEntries > 1, DataManagement,
+    VerifyOrDieWithMsg(countOfPathRegistryEntries == 1, DataManagement,
                        "Seemingly device supports batch commands, but is calling the deprecated PrepareCommand API");
 
     auto commandPathRegistryEntry = GetCommandPathRegistry().GetFirstEntry();
@@ -614,6 +614,8 @@ CHIP_ERROR CommandHandler::PrepareInvokeResponseCommand(const CommandPathRegistr
     //
     VerifyOrReturnError(mState == State::Idle || mState == State::AddedCommand, CHIP_ERROR_INCORRECT_STATE);
 
+    // TODO(#30453): See if we can pass this back up the stack so caller can provide this instead of taking up
+    // space in CommandHanlder.
     mRefForResponse = apCommandPathRegistryEntry.ref;
 
     MoveToState(State::Preparing);

src/app/CommandHandler.h

@@ -156,15 +156,23 @@ class CommandHandler : public Messaging::ExchangeDelegate
     // Previously we kept adding arguments with default values individually as parameters. This is because there
     // is legacy code outside of the SDK that would call PrepareCommand. With the new PrepareInvokeResponseCommand
     // replacing PrepareCommand, we took this opportunity to create a new parameter structure to make it easier to
-    // add new parameters without there needing to be an ever increase parameter list with defaults.
+    // add new parameters without there needing to be an ever increasing parameter list with defaults.
     struct InvokeResponseParameters
     {
-        InvokeResponseParameters(ConcreteCommandPath aRequestCommandPath, bool aStartOrEndDataStruct = true) :
-            mRequestCommandPath(aRequestCommandPath), mStartOrEndDataStruct(aStartOrEndDataStruct)
-        {}
+        InvokeResponseParameters(ConcreteCommandPath aRequestCommandPath) : mRequestCommandPath(aRequestCommandPath) {}
+
+        InvokeResponseParameters & SetStartOrEndDataStruct(bool aStartOrEndDataStruct)
+        {
+            mStartOrEndDataStruct = aStartOrEndDataStruct;
+            return *this;
+        }
 
         ConcreteCommandPath mRequestCommandPath;
-        bool mStartOrEndDataStruct;
+        /**
+         * Should the method this is being provided to start/end the TLV container for the CommandFields element
+         * within CommandDataIB.
+         */
+        bool mStartOrEndDataStruct = true;
     };
 
     class TestOnlyMarker
@@ -200,7 +208,8 @@ class CommandHandler : public Messaging::ExchangeDelegate
                                 System::PacketBufferHandle && payload, bool isTimedInvoke);
 
     /**
-     * Checks that all CommandDataIB within InvokeRequests is correct as per spec.
+     * Checks that all CommandDataIB within InvokeRequests satisfy the spec's general
+     * constraints for CommandDataIB.
      *
      * This also builds a registry that to ensure that all commands can be responded
      * to with the data required as per spec.
@@ -233,14 +242,16 @@ class CommandHandler : public Messaging::ExchangeDelegate
      * `CommandDataIB`.
      *
      * This call will fail if CommandHandler is already in the middle of building a
-     * CommandStatusIB or CommandDataID (ei something has called Perpare*, without
+     * CommandStatusIB or CommandDataIB (i.e. something has called Prepare*, without
      * calling Finish*), or is already sending InvokeResponseMessage.
      *
      * Upon success, the caller is expected to call `FinishCommand` once they have added
-     * all Data into Fields element of CommandDataIB.
+     * all the fields into the CommandFields element of CommandDataIB.
      *
      * @param [in] aResponseCommandPath the concrete response path that we are sending to Requester.
-     * @param [in] aPrepareParameters struct containing paramters needs for preparing a command, such as request path.
+     * @param [in] aPrepareParameters struct containing paramters needs for preparing a command. Data
+     *             such as request path, and if method should start the CommandField element within
+     *             CommandDataIB.
      */
     CHIP_ERROR PrepareInvokeResponseCommand(const ConcreteCommandPath & aResponseCommandPath,
                                             const InvokeResponseParameters & aPrepareParameters);
@@ -254,12 +265,12 @@ class CommandHandler : public Messaging::ExchangeDelegate
      * Caller must have first successfully called `PrepareInvokeResponseCommand`.
      *
      * @param [in] aEndDataStruct end the TLV container for the CommandFields element within
-     *             CommandDataIB.
+     *             CommandDataIB. This should match the boolean passed into Prepare*.
      *
      * @return CHIP_ERROR_INCORRECT_STATE
      *                      If device has not previously successfully called
      *                      `PrepareInvokeResponseCommand`.
-     * @retval CHIP_ERROR_BUFFER_TOO_SMALL
+     * @return CHIP_ERROR_BUFFER_TOO_SMALL
      *                      If writing the values needed to finish the InvokeReponseIB
      *                      with the current contents of the InvokeResponseMessage
      *                      would exceed the limit. When this error occurs, it is possible
@@ -271,18 +282,20 @@ class CommandHandler : public Messaging::ExchangeDelegate
      * @return other        Other TLVWriter related errors. Typically occurs if
      *                      `GetCommandDataIBTLVWriter()` was called and used incorrectly.
      */
+    // TODO(#30453): We should be able to eliminate the chances of OOM issues with reserve.
+    // This will be completed in a follow up PR.
     CHIP_ERROR FinishCommand(bool aEndDataStruct = true);
 
     /**
      * This will add a new CommandStatusIB element into InvokeResponses. It will put the
      * aCommandPath into the CommandPath element within CommandStatusIB.
      *
      * This call will fail if CommandHandler is already in the middle of building a
-     * CommandStatusIB or CommandDataID (ei something has called Perpare*, without
+     * CommandStatusIB or CommandDataIB (i.e. something has called Prepare*, without
      * calling Finish*), or is already sending InvokeResponseMessage.
      *
-     * Upon success, the caller is expected to call `FinishStatus` once they have added
-     * data into Fields element of CommandStatusIB.
+     * Upon success, the caller is expected to call `FinishStatus` once they have encoded
+     * StatusIB.
      *
      * @param [in] aCommandPath the concrete path of the command we are responding to.
      */
@@ -503,7 +516,9 @@ class CommandHandler : public Messaging::ExchangeDelegate
         // Return early in case of requests targeted to a group, since they should not add a response.
         VerifyOrReturnValue(!IsGroupRequest(), CHIP_NO_ERROR);
 
-        InvokeResponseParameters prepareParams(aRequestCommandPath, /* aStartOrEndDataStruct = */ false);
+        InvokeResponseParameters prepareParams(aRequestCommandPath);
+        prepareParams.SetStartOrEndDataStruct(false);
+
         ConcreteCommandPath responsePath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId,
                                              CommandData::GetCommandId() };
         ReturnErrorOnFailure(PrepareInvokeResponseCommand(responsePath, prepareParams));