Changing AcceptedCommands Interface

docs/upgrading.md

@@ -107,3 +107,87 @@ To use default attribute persistence, you need to pass in a
 `PersistentStorageDelegate` to `CodegenDataModelProviderInstance`. See example
 changes in [36658](https://github.com/project-chip/connectedhomeip/pull/36658)
 ).
+
+### Decoupling of Command Handler Interface from Ember
+
+#### EnumerateGeneratedCommands
+
+Changed the old callback based iteration, into a ListBuilder based approach for
+the Enumeration of Generated Commands
+
+`CommandHandlerInterface::EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)`
+becomes
+`CommandHandlerInterface::EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, ListBuilder<CommandId> & builder)`
+
+Changes for implementation
+
+-   old
+
+```cpp
+   for (auto && cmd : { ScanNetworksResponse::Id, NetworkConfigResponse::Id, ConnectNetworkResponse::Id })
+   {
+       VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
+   }
+```
+
+-   new
+
+```cpp
+   ReturnErrorOnFailure(
+        builder.AppendElements({ ScanNetworksResponse::Id, NetworkConfigResponse::Id, ConnectNetworkResponse::Id })
+   )
+```
+
+#### EnumerateAcceptedCommands
+
+Expanded `EnumerateAcceptedCommands` Interface to provide the Access Information
+(Attributes and Qualities) using a ListBuilder
+
+`CommandHandlerInterface::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)`
+becomes
+`CommandHandlerInterface::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, ListBuilder<AcceptedCommandEntry> & builder)`
+
+Changes for implementation:
+
+-   Old
+
+```cpp
+    for (auto && cmd : {
+             Disable::Id,
+             EnableCharging::Id,
+         })
+    {
+        VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
+    }
+
+    if (HasFeature(Feature::kV2x))
+    {
+        VerifyOrExit(callback(EnableDischarging::Id, context) == Loop::Continue, /**/);
+    }
+```
+
+-   new
+
+```cpp
+    using QF   = DataModel::CommandQualityFlags;
+    using Priv = Access::Privilege;
+
+    ReturnErrorOnFailure(builder.AppendElements({
+        { Disable::Id,        QF::kTimed, Priv::kOperate },
+        { EnableCharging::Id, QF::kTimed, Priv::kOperate },
+    }));
+
+    if (HasFeature(Feature::kV2x))
+    {
+        ReturnErrorOnFailure(builder.EnsureAppendCapacity(1));
+        ReturnErrorOnFailure(
+            builder.Append({ EnableDischarging::Id, QF::kTimed, Priv::kOperate})
+        );
+    }
+
+```
+
+Important Notes:
+
+Use `EnsureAppendCapacity` before `ListBuilder::Append` to prevent buffer
+overflow when appending a single element, this function never allocates.

src/app/CommandHandlerInterface.h

@@ -21,14 +21,15 @@
 #include <app/CommandHandler.h>
 #include <app/ConcreteClusterPath.h>
 #include <app/ConcreteCommandPath.h>
+#include <app/data-model-provider/MetadataList.h>
+#include <app/data-model-provider/MetadataTypes.h>
 #include <app/data-model/Decode.h>
 #include <app/data-model/List.h> // So we can encode lists
 #include <lib/core/DataModelTypes.h>
 #include <lib/support/Iterators.h>
 
 namespace chip {
 namespace app {
-
 /*
  * This interface permits applications to register a server-side command handler
  * at run-time for a given cluster. The handler can either be configured to handle all endpoints
@@ -122,7 +123,8 @@ class CommandHandlerInterface
      * This is used by callbacks that just look for a particular value in the
      * list.
      */
-    virtual CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
+    virtual CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                                 DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder)
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }
@@ -146,7 +148,7 @@ class CommandHandlerInterface
      * This is used by callbacks that just look for a particular value in the
      * list.
      */
-    virtual CHIP_ERROR EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
+    virtual CHIP_ERROR EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, DataModel::ListBuilder<CommandId> & builder)
     {
         return CHIP_ERROR_NOT_IMPLEMENTED;
     }

src/app/clusters/device-energy-management-server/device-energy-management-server.cpp

@@ -104,49 +104,51 @@ CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValu
 }
 
 // CommandHandlerInterface
-CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
+CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                               DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder)
 {
     using namespace Commands;
+    using Priv = Access::Privilege;
+
+    ReturnErrorOnFailure(builder.EnsureAppendCapacity(8)); // Ensure we have capacity for all possible commands
 
     if (HasFeature(Feature::kPowerAdjustment))
     {
-        for (auto && cmd : {
-                 PowerAdjustRequest::Id,
-                 CancelPowerAdjustRequest::Id,
-             })
-        {
-            VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
-        }
+        ReturnErrorOnFailure(builder.AppendElements({
+            { PowerAdjustRequest::Id, {}, Priv::kOperate },      //
+            { CancelPowerAdjustRequest::Id, {}, Priv::kOperate } //
+        }));
     }
 
     if (HasFeature(Feature::kStartTimeAdjustment))
     {
-        VerifyOrExit(callback(StartTimeAdjustRequest::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append({ StartTimeAdjustRequest::Id, {}, Priv::kOperate }));
     }
 
     if (HasFeature(Feature::kPausable))
     {
-        VerifyOrExit(callback(PauseRequest::Id, context) == Loop::Continue, /**/);
-        VerifyOrExit(callback(ResumeRequest::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.AppendElements({
+            { PauseRequest::Id, {}, Priv::kOperate }, //
+            { ResumeRequest::Id, {}, Priv::kOperate } //
+        }));
     }
 
     if (HasFeature(Feature::kForecastAdjustment))
     {
-        VerifyOrExit(callback(ModifyForecastRequest::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append({ ModifyForecastRequest::Id, {}, Priv::kOperate }));
     }
 
     if (HasFeature(Feature::kConstraintBasedAdjustment))
     {
-        VerifyOrExit(callback(RequestConstraintBasedForecast::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append({ RequestConstraintBasedForecast::Id, {}, Priv::kOperate }));
     }
 
     if (HasFeature(Feature::kStartTimeAdjustment) || HasFeature(Feature::kForecastAdjustment) ||
         HasFeature(Feature::kConstraintBasedAdjustment))
     {
-        VerifyOrExit(callback(CancelRequest::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append({ CancelRequest::Id, {}, Priv::kOperate }));
     }
 
-exit:
     return CHIP_NO_ERROR;
 }
 

src/app/clusters/device-energy-management-server/device-energy-management-server.h

@@ -235,7 +235,8 @@ class Instance : public AttributeAccessInterface, public CommandHandlerInterface
 
     // CommandHandlerInterface
     void InvokeCommand(HandlerContext & handlerContext) override;
-    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override;
+    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                         DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder) override;
 
     Protocols::InteractionModel::Status CheckOptOutAllowsRequest(AdjustmentCauseEnum adjustmentCause);
     void HandlePowerAdjustRequest(HandlerContext & ctx, const Commands::PowerAdjustRequest::DecodableType & commandData);

src/app/clusters/energy-evse-server/energy-evse-server.cpp

@@ -182,41 +182,37 @@ CHIP_ERROR Instance::Write(const ConcreteDataAttributePath & aPath, AttributeVal
 }
 
 // CommandHandlerInterface
-CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
+CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                               DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder)
 {
     using namespace Commands;
+    using QF   = DataModel::CommandQualityFlags;
+    using Priv = Access::Privilege;
+    ReturnErrorOnFailure(builder.EnsureAppendCapacity(7));
 
-    for (auto && cmd : {
-             Disable::Id,
-             EnableCharging::Id,
-         })
-    {
-        VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
-    }
+    ReturnErrorOnFailure(builder.AppendElements({
+        { Disable::Id, QF::kTimed, Priv::kOperate },        //
+        { EnableCharging::Id, QF::kTimed, Priv::kOperate }, //
+    }));
 
     if (HasFeature(Feature::kV2x))
     {
-        VerifyOrExit(callback(EnableDischarging::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append({ EnableDischarging::Id, QF::kTimed, Priv::kOperate }));
     }
 
     if (HasFeature(Feature::kChargingPreferences))
     {
-        for (auto && cmd : {
-                 SetTargets::Id,
-                 GetTargets::Id,
-                 ClearTargets::Id,
-             })
-        {
-            VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
-        }
+        ReturnErrorOnFailure(builder.AppendElements({
+            { SetTargets::Id, QF::kTimed, Priv::kOperate },   //
+            { GetTargets::Id, QF::kTimed, Priv::kOperate },   //
+            { ClearTargets::Id, QF::kTimed, Priv::kOperate }, //
+        }));
     }
 
     if (SupportsOptCmd(OptionalCommands::kSupportsStartDiagnostics))
     {
-        callback(StartDiagnostics::Id, context);
+        ReturnErrorOnFailure(builder.Append({ StartDiagnostics::Id, QF::kTimed, Priv::kOperate }));
     }
-
-exit:
     return CHIP_NO_ERROR;
 }
 

src/app/clusters/energy-evse-server/energy-evse-server.h

@@ -200,7 +200,8 @@ class Instance : public AttributeAccessInterface, public CommandHandlerInterface
 
     // CommandHandlerInterface
     void InvokeCommand(HandlerContext & handlerContext) override;
-    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override;
+    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                         DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder) override;
 
     void HandleDisable(HandlerContext & ctx, const Commands::Disable::DecodableType & commandData);
     void HandleEnableCharging(HandlerContext & ctx, const Commands::EnableCharging::DecodableType & commandData);

src/app/clusters/microwave-oven-control-server/microwave-oven-control-server.cpp

@@ -16,14 +16,14 @@
  *
  */
 
-#include "app/data-model-provider/MetadataList.h"
 #include <app-common/zap-generated/attributes/Accessors.h>
 #include <app/AttributeAccessInterfaceRegistry.h>
 #include <app/CommandHandlerInterfaceRegistry.h>
 #include <app/ConcreteClusterPath.h>
 #include <app/InteractionModelEngine.h>
 #include <app/clusters/microwave-oven-control-server/microwave-oven-control-server.h>
 #include <app/clusters/mode-base-server/mode-base-server.h>
+#include <app/data-model-provider/MetadataList.h>
 #include <app/data-model-provider/MetadataTypes.h>
 #include <app/reporting/reporting.h>
 #include <app/util/attribute-storage.h>

src/app/clusters/network-commissioning/network-commissioning.cpp

@@ -1360,64 +1360,56 @@ void Instance::OnFailSafeTimerExpired()
     }
 }
 
-CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
+CHIP_ERROR Instance::EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                               DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder)
 {
     using namespace Clusters::NetworkCommissioning::Commands;
+    using Priv = Access::Privilege;
 
-    if (mFeatureFlags.Has(Feature::kThreadNetworkInterface))
-    {
-        for (auto && cmd : {
-                 ScanNetworks::Id,
-                 AddOrUpdateThreadNetwork::Id,
-                 RemoveNetwork::Id,
-                 ConnectNetwork::Id,
-                 ReorderNetwork::Id,
-             })
-        {
-            VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
-        }
-    }
-    else if (mFeatureFlags.Has(Feature::kWiFiNetworkInterface))
+    static constexpr size_t kNetworkCommands = 7; // Count of max possible commands assuming all features
+    ReturnErrorOnFailure(builder.EnsureAppendCapacity(kNetworkCommands));
+
+    bool hasThread = mFeatureFlags.Has(Feature::kThreadNetworkInterface);
+    bool hasWifi   = mFeatureFlags.Has(Feature::kWiFiNetworkInterface);
+
+    if (hasThread || hasWifi)
     {
-        for (auto && cmd : {
-                 ScanNetworks::Id,
-                 AddOrUpdateWiFiNetwork::Id,
-                 RemoveNetwork::Id,
-                 ConnectNetwork::Id,
-                 ReorderNetwork::Id,
-             })
-        {
-            VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
-        }
+        ReturnErrorOnFailure(builder.AppendElements({
+            { ScanNetworks::Id, {}, Priv::kAdminister },   //
+            { RemoveNetwork::Id, {}, Priv::kAdminister },  //
+            { ConnectNetwork::Id, {}, Priv::kAdminister }, //
+            { ReorderNetwork::Id, {}, Priv::kAdminister }, //
+        }));
+        ReturnErrorOnFailure(
+            builder.Append({ hasThread ? AddOrUpdateThreadNetwork::Id : AddOrUpdateWiFiNetwork::Id, {}, Priv::kAdminister }));
     }
 
     if (mFeatureFlags.Has(Feature::kPerDeviceCredentials))
     {
-        VerifyOrExit(callback(QueryIdentity::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append({ QueryIdentity::Id, {}, Priv::kAdminister }));
     }
 
-exit:
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR Instance::EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context)
+CHIP_ERROR Instance::EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, DataModel::ListBuilder<CommandId> & builder)
 {
     using namespace Clusters::NetworkCommissioning::Commands;
 
+    static constexpr size_t kCommands = 4; // Count of max possible commands assuming all features
+    ReturnErrorOnFailure(builder.EnsureAppendCapacity(kCommands));
+
     if (mFeatureFlags.HasAny(Feature::kWiFiNetworkInterface, Feature::kThreadNetworkInterface))
     {
-        for (auto && cmd : { ScanNetworksResponse::Id, NetworkConfigResponse::Id, ConnectNetworkResponse::Id })
-        {
-            VerifyOrExit(callback(cmd, context) == Loop::Continue, /**/);
-        }
+        ReturnErrorOnFailure(
+            builder.AppendElements({ ScanNetworksResponse::Id, NetworkConfigResponse::Id, ConnectNetworkResponse::Id }));
     }
 
     if (mFeatureFlags.Has(Feature::kPerDeviceCredentials))
     {
-        VerifyOrExit(callback(QueryIdentityResponse::Id, context) == Loop::Continue, /**/);
+        ReturnErrorOnFailure(builder.Append(QueryIdentityResponse::Id));
     }
 
-exit:
     return CHIP_NO_ERROR;
 }
 

src/app/clusters/network-commissioning/network-commissioning.h

@@ -59,8 +59,10 @@ class Instance : public CommandHandlerInterface,
 
     // CommandHandlerInterface
     void InvokeCommand(HandlerContext & ctx) override;
-    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override;
-    CHIP_ERROR EnumerateGeneratedCommands(const ConcreteClusterPath & cluster, CommandIdCallback callback, void * context) override;
+    CHIP_ERROR EnumerateAcceptedCommands(const ConcreteClusterPath & cluster,
+                                         DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder) override;
+    CHIP_ERROR EnumerateGeneratedCommands(const ConcreteClusterPath & cluster,
+                                          DataModel::ListBuilder<CommandId> & builder) override;
 
     // AttributeAccessInterface
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;