Merge branch 'master' into door_lock_aliro_command_handling

examples/all-clusters-app/nxp/mw320/main.cpp

@@ -1152,12 +1152,10 @@ void task_test_main(void * param)
             PRINTF("--> update CurrentPosition [%d] \r\n", value);
             Clusters::Switch::Attributes::CurrentPosition::Set(1, value);
 #ifdef SUPPORT_MANUAL_CTRL
-#error                                                                                                                             \
-    "This code thinks it's setting the OnOff attribute, but it's actually setting the NumberOfPositions attribute!  And passing the wrong size for either case.  Figure out what it's trying to do."
-            // sync-up the Light attribute (for test event, OO.M.ManuallyControlled)
-            PRINTF("--> update [Clusters::Switch::Id]: OnOff::Id [%d] \r\n", value);
-            emAfWriteAttribute(1, Clusters::Switch::Id, Clusters::OnOff::Attributes::OnOff::Id, (uint8_t *) &value, sizeof(value),
-                               true, false);
+#error "Not implemented"
+            // TODO: previous code was trying to write a OnOff cluster attribute id to a switch attribute, generally
+            //       not working. Determine if this should maybe be
+            //       OnOff::Attributes::OnOff::Set(1, is_on) or similar
 #endif // SUPPORT_MANUAL_CTRL
 
             need2sync_sw_attr = false;

examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.h

@@ -31,5 +31,6 @@ CHIP_ERROR LogAttributeAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNum
 CHIP_ERROR LogCommandAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNumber * commandId, id result);
 CHIP_ERROR LogAttributeErrorAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNumber * attributeId, NSError * error);
 CHIP_ERROR LogCommandErrorAsJSON(NSNumber * endpointId, NSNumber * clusterId, NSNumber * commandId, NSError * error);
+CHIP_ERROR LogGetCommissionerNodeId(NSNumber * nodeId);
 void SetDelegate(RemoteDataModelLoggerDelegate * delegate);
 }; // namespace RemoteDataModelLogger

examples/darwin-framework-tool/commands/common/RemoteDataModelLogger.mm

@@ -34,6 +34,7 @@
 constexpr char kErrorIdKey[] = "error";
 constexpr char kClusterErrorIdKey[] = "clusterError";
 constexpr char kValueKey[] = "value";
+constexpr char kNodeIdKey[] = "nodeId";
 
 constexpr char kBase64Header[] = "base64:";
 
@@ -191,5 +192,17 @@ CHIP_ERROR LogCommandErrorAsJSON(NSNumber * endpointId, NSNumber * clusterId, NS
     return LogError(value, status);
 }
 
+CHIP_ERROR LogGetCommissionerNodeId(NSNumber * value)
+{
+    VerifyOrReturnError(gDelegate != nullptr, CHIP_NO_ERROR);
+
+    Json::Value rootValue;
+    rootValue[kValueKey] = Json::Value();
+    rootValue[kValueKey][kNodeIdKey] = [value unsignedLongLongValue];
+
+    auto valueStr = JsonToString(rootValue);
+    return gDelegate->LogJSON(valueStr.c_str());
+}
+
 void SetDelegate(RemoteDataModelLoggerDelegate * delegate) { gDelegate = delegate; }
 }; // namespace RemoteDataModelLogger

examples/darwin-framework-tool/commands/pairing/GetCommissionerNodeIdCommand.mm

@@ -19,15 +19,17 @@
 #import <Matter/Matter.h>
 
 #include "GetCommissionerNodeIdCommand.h"
+#include "RemoteDataModelLogger.h"
 
 CHIP_ERROR GetCommissionerNodeIdCommand::RunCommand()
 {
     auto * controller = CurrentCommissioner();
     VerifyOrReturnError(nil != controller, CHIP_ERROR_INCORRECT_STATE);
 
-    ChipLogProgress(
-        chipTool, "Commissioner Node Id 0x" ChipLogFormatX64, ChipLogValueX64(controller.controllerNodeId.unsignedLongLongValue));
+    auto controllerNodeId = controller.controllerNodeId;
+    ChipLogProgress(chipTool, "Commissioner Node Id 0x" ChipLogFormatX64, ChipLogValueX64(controllerNodeId.unsignedLongLongValue));
 
+    ReturnErrorOnFailure(RemoteDataModelLogger::LogGetCommissionerNodeId(controllerNodeId));
     SetCommandExitStatus(CHIP_NO_ERROR);
     return CHIP_NO_ERROR;
 }

src/app/CommandHandler.h

@@ -360,24 +360,7 @@ class CommandHandler
     template <typename CommandData>
     CHIP_ERROR AddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
     {
-        // This method, templated with CommandData, captures all the components needs
-        // from CommandData with as little code as possible. This in theory should
-        // reduce compiled code size.
-        //
-        // TODO(#30453): Verify the accuracy of the theory outlined below.
-        //
-        // Theory on code reduction: Previously, non-essential code was unnecessarily
-        // templated, leading to compilation and duplication N times. The lambda
-        // function below mitigates this issue by isolating only the code segments
-        // that genuinely require templating, thereby minimizing duplicate compiled
-        // code.
-        ConcreteCommandPath responsePath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId,
-                                             CommandData::GetCommandId() };
-        auto encodeCommandDataClosure    = [&](TLV::TLVWriter & writer) -> CHIP_ERROR {
-            return DataModel::Encode(writer, TLV::ContextTag(CommandDataIB::Tag::kFields), aData);
-        };
-        return TryAddingResponse(
-            [&]() -> CHIP_ERROR { return TryAddResponseData(aRequestCommandPath, responsePath, encodeCommandDataClosure); });
+        return TryAddingResponse([&]() -> CHIP_ERROR { return TryAddResponseData(aRequestCommandPath, aData); });
     }
 
     /**
@@ -567,18 +550,21 @@ class CommandHandler
     CHIP_ERROR AddStatusInternal(const ConcreteCommandPath & aCommandPath, const StatusIB & aStatus);
 
     /**
-     * If this function fails, it may leave our TLV buffer in an inconsistent state.  Callers should snapshot as needed before
-     * calling this function, and roll back as needed afterward.
+     * Non-templated function called before DataModel::Encode when attempting to add a response,
+     * which does all the work needed before encoding the actual type-dependent data into the buffer.
      *
-     * @param [in] aRequestCommandPath the concrete path of the command we are
-     *             responding to.
-     * @param [in] aResponseCommandPath the concrete command response path.
-     * @param [in] encodeCommandDataFunction A lambda function responsible for
-     *             encoding the CommandData field.
+     * **Important:** If this function fails, the TLV buffer may be left in an inconsistent state.
+     * Callers should create snapshots as necessary before invoking this function and implement
+     * rollback mechanisms if needed.
+     *
+     * **Usage:** This function is intended to be called exclusively by TryAddResponseData. It was
+     * factored out to optimize code size.
+     *
+     * @param aRequestCommandPath  The concrete path of the command being responded to.
+     * @param aResponseCommandPath The concrete path of the command response.
      */
-    template <typename Function>
-    CHIP_ERROR TryAddResponseData(const ConcreteCommandPath & aRequestCommandPath, const ConcreteCommandPath & aResponseCommandPath,
-                                  Function && encodeCommandDataFunction)
+    CHIP_ERROR TryAddResponseDataPreEncode(const ConcreteCommandPath & aRequestCommandPath,
+                                           const ConcreteCommandPath & aResponseCommandPath)
     {
         // Return early in case of requests targeted to a group, since they should not add a response.
         VerifyOrReturnValue(!IsGroupRequest(), CHIP_NO_ERROR);
@@ -587,11 +573,39 @@ class CommandHandler
         prepareParams.SetStartOrEndDataStruct(false);
 
         ScopedChange<bool> internalCallToAddResponse(mInternalCallToAddResponseData, true);
-        ReturnErrorOnFailure(PrepareInvokeResponseCommand(aResponseCommandPath, prepareParams));
+        return PrepareInvokeResponseCommand(aResponseCommandPath, prepareParams);
+    }
+
+    // TODO(#31627): It would be awesome if we could remove this template all together.
+    /**
+     * If this function fails, it may leave our TLV buffer in an inconsistent state.
+     * Callers should snapshot as needed before calling this function, and roll back
+     * as needed afterward.
+     *
+     * @param [in] aRequestCommandPath the concrete path of the command we are
+     *             responding to.
+     * @param [in] aData the data for the response.
+     */
+    template <typename CommandData>
+    CHIP_ERROR TryAddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
+    {
+        // This method, templated with CommandData, captures all the components needs
+        // from CommandData with as little code as possible.
+        //
+        // Previously, non-essential code was unnecessarily templated, leading to
+        // compilation and duplication N times. By isolating only the code segments
+        // that genuinely require templating, minimizes duplicate compiled code.
+        ConcreteCommandPath responseCommandPath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId,
+                                                    CommandData::GetCommandId() };
+        ReturnErrorOnFailure(TryAddResponseDataPreEncode(aRequestCommandPath, responseCommandPath));
         TLV::TLVWriter * writer = GetCommandDataIBTLVWriter();
         VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE);
-        ReturnErrorOnFailure(encodeCommandDataFunction(*writer));
+        ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(CommandDataIB::Tag::kFields), aData));
 
+        // FinishCommand technically should be refactored out as it is not a command that needs templating.
+        // But, because there is only a single function call, keeping it here takes less code. If there is
+        // ever more code between DataModel::Encode and the end of this function, it should be broken out into
+        // TryAddResponseDataPostEncode.
         return FinishCommand(/* aEndDataStruct = */ false);
     }
 

src/app/EventLogging.h

@@ -54,6 +54,10 @@ class EventLogger : public EventLoggingDelegate
  * context tags to be interpreted within the schema identified by
  * `ClusterID` and `EventId`.
  *
+ * The consumer has to either lock the Matter stack lock or queue the event to
+ * the Matter event queue when using LogEvent. This function is not safe to call
+ * outside of the main Matter processing context.
+ *
  * LogEvent has 2 variant, one for fabric-scoped events and one for non-fabric-scoped events.
  * @param[in] aEventData  The event cluster object
  * @param[in] aEndpoint    The current cluster's Endpoint Id

src/app/chip_data_model.gni

@@ -356,6 +356,12 @@ template("chip_data_model") {
           "${_app_root}/clusters/${cluster}/${cluster}.h",
           "${_app_root}/clusters/${cluster}/EnergyReportingTestEventTriggerHandler.h",
         ]
+      } else if (cluster == "thread-network-diagnostics-server") {
+        sources += [
+          "${_app_root}/clusters/${cluster}/${cluster}.cpp",
+          "${_app_root}/clusters/${cluster}/thread-network-diagnostics-provider.cpp",
+          "${_app_root}/clusters/${cluster}/thread-network-diagnostics-provider.h",
+        ]
       } else {
         sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
       }