Add CommandSender callback for chip-repl to better handle path-specif…

…ic command errors
project-chip · Dec 14, 2023 · f92e7d7 · f92e7d7
1 parent 7378446
commit f92e7d7
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 8 deletions.
diff --git a/src/controller/python/chip/clusters/Command.py b/src/controller/python/chip/clusters/Command.py
@@ -149,7 +149,13 @@ def _handleResponse(self, path: CommandPath, index: int, status: Status, respons
             self._handleError(status, 0, IndexError(f"CommandSenderCallback has given us an unexpected index value {index}"))
             return
 
-        if (len(response) == 0):
+        if status.IMStatus != chip.interaction_model.Status.Success:
+            try:
+                self._responses[index] = chip.interaction_model.InteractionModelError(
+                    chip.interaction_model.Status(status.IMStatus), status.ClusterStatus)
+            except AttributeError as ex:
+                self._handleError(status, 0, ex)
+        elif (len(response) == 0):
             self._responses[index] = None
         else:
             # If a type hasn't been assigned, let's auto-deduce it.
@@ -173,9 +179,7 @@ def handleResponse(self, path: CommandPath, index: int, status: Status, response
 
     def _handleError(self, imError: Status, chipError: PyChipError, exception: Exception):
         if self._future.done():
-            # TODO Right now this even callback happens if there was a real IM Status error on one command.
-            # We need to update OnError to allow providing a CommandRef that we can try associating with it.
-            logger.exception(f"Recieved another error, but we have sent error. imError:{imError}, chipError {chipError}")
+            logger.exception(f"Recieved another error, but we been have sent error. imError:{imError}, chipError {chipError}")
             return
         if exception:
             self._future.set_exception(exception)

diff --git a/src/controller/python/chip/clusters/command.cpp b/src/controller/python/chip/clusters/command.cpp
@@ -120,10 +120,26 @@ class CommandSenderCallback : public CommandSender::Callback
             return;
         }
 
-        gOnCommandSenderResponseCallback(
-            mAppContext, aPath.mEndpointId, aPath.mClusterId, aPath.mCommandId, index, to_underlying(aStatus.mStatus),
-            aStatus.mClusterStatus.HasValue() ? aStatus.mClusterStatus.Value() : chip::python::kUndefinedClusterStatus, buffer,
-            size);
+        gOnCommandSenderResponseCallback(mAppContext, aPath.mEndpointId, aPath.mClusterId, aPath.mCommandId, index,
+                                         to_underlying(aStatus.mStatus),
+                                         aStatus.mClusterStatus.ValueOr(chip::python::kUndefinedClusterStatus), buffer, size);
+    }
+
+    void OnPathSpecificError(CommandSender * apCommandSender, const ConcreteCommandPath & aPath, const StatusIB & aStatusIB,
+                             const CommandSender::AdditionalResponseData & aAdditionalResponseData) override
+    {
+        // For legacy specific reasons when we are not processing a batch command we simply forward this to the OnError callback
+        // for more information on why see https://github.com/project-chip/connectedhomeip/issues/30991.
+        if (!mIsBatchedCommands)
+        {
+            OnError(apCommandSender, aStatusIB.ToChipError());
+            return;
+        }
+
+        // While the API contract for OnResponseWithAdditionalData states that it should only be called when aStatusIB indicates
+        // success, for chip-repl the callback for batch commands is written in manner that supports accepting both success and
+        // error StatusIB for batch commands, as such we just call that.
+        OnResponseWithAdditionalData(apCommandSender, aPath, aStatusIB, nullptr, aAdditionalResponseData);
     }
 
     void OnError(const CommandSender * apCommandSender, CHIP_ERROR aProtocolError) override