Decouple ember functions from wifi diagnostic cluster (#37097)

* Decouple ember functions from general diagnostic cluster * Address review comment
project-chip · Jan 21, 2025 · 2fed176 · 2fed176
1 parent bdbd217
commit 2fed176
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 123 deletions.
diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/IMClusterCommandHandler.cpp b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/IMClusterCommandHandler.cpp
@@ -1748,47 +1748,6 @@ Protocols::InteractionModel::Status DispatchServerCommand(CommandHandler * apCom
 
 } // namespace ValveConfigurationAndControl
 
-namespace WiFiNetworkDiagnostics {
-
-Protocols::InteractionModel::Status DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandPath & aCommandPath,
-                                                          TLV::TLVReader & aDataTlv)
-{
-    CHIP_ERROR TLVError = CHIP_NO_ERROR;
-    bool wasHandled     = false;
-    {
-        switch (aCommandPath.mCommandId)
-        {
-        case Commands::ResetCounts::Id: {
-            Commands::ResetCounts::DecodableType commandData;
-            TLVError = DataModel::Decode(aDataTlv, commandData);
-            if (TLVError == CHIP_NO_ERROR)
-            {
-                wasHandled = emberAfWiFiNetworkDiagnosticsClusterResetCountsCallback(apCommandObj, aCommandPath, commandData);
-            }
-            break;
-        }
-        default: {
-            // Unrecognized command ID, error status will apply.
-            ChipLogError(Zcl, "Unknown command " ChipLogFormatMEI " for cluster " ChipLogFormatMEI,
-                         ChipLogValueMEI(aCommandPath.mCommandId), ChipLogValueMEI(aCommandPath.mClusterId));
-            return Protocols::InteractionModel::Status::UnsupportedCommand;
-        }
-        }
-    }
-
-    if (CHIP_NO_ERROR != TLVError || !wasHandled)
-    {
-        ChipLogProgress(Zcl, "Failed to dispatch command, TLVError=%" CHIP_ERROR_FORMAT, TLVError.Format());
-        return Protocols::InteractionModel::Status::InvalidCommand;
-    }
-
-    // We use success as a marker that no special handling is required
-    // This is to avoid having a std::optional which uses slightly more code.
-    return Protocols::InteractionModel::Status::Success;
-}
-
-} // namespace WiFiNetworkDiagnostics
-
 namespace WindowCovering {
 
 Protocols::InteractionModel::Status DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandPath & aCommandPath,
@@ -1964,9 +1923,6 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, TLV:
     case Clusters::ValveConfigurationAndControl::Id:
         errorStatus = Clusters::ValveConfigurationAndControl::DispatchServerCommand(apCommandObj, aCommandPath, aReader);
         break;
-    case Clusters::WiFiNetworkDiagnostics::Id:
-        errorStatus = Clusters::WiFiNetworkDiagnostics::DispatchServerCommand(apCommandObj, aCommandPath, aReader);
-        break;
     case Clusters::WindowCovering::Id:
         errorStatus = Clusters::WindowCovering::DispatchServerCommand(apCommandObj, aCommandPath, aReader);
         break;

diff --git a/scripts/tools/zap/tests/outputs/lighting-app/app-templates/IMClusterCommandHandler.cpp b/scripts/tools/zap/tests/outputs/lighting-app/app-templates/IMClusterCommandHandler.cpp
@@ -969,47 +969,6 @@ Protocols::InteractionModel::Status DispatchServerCommand(CommandHandler * apCom
 
 } // namespace ThreadNetworkDiagnostics
 
-namespace WiFiNetworkDiagnostics {
-
-Protocols::InteractionModel::Status DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandPath & aCommandPath,
-                                                          TLV::TLVReader & aDataTlv)
-{
-    CHIP_ERROR TLVError = CHIP_NO_ERROR;
-    bool wasHandled     = false;
-    {
-        switch (aCommandPath.mCommandId)
-        {
-        case Commands::ResetCounts::Id: {
-            Commands::ResetCounts::DecodableType commandData;
-            TLVError = DataModel::Decode(aDataTlv, commandData);
-            if (TLVError == CHIP_NO_ERROR)
-            {
-                wasHandled = emberAfWiFiNetworkDiagnosticsClusterResetCountsCallback(apCommandObj, aCommandPath, commandData);
-            }
-            break;
-        }
-        default: {
-            // Unrecognized command ID, error status will apply.
-            ChipLogError(Zcl, "Unknown command " ChipLogFormatMEI " for cluster " ChipLogFormatMEI,
-                         ChipLogValueMEI(aCommandPath.mCommandId), ChipLogValueMEI(aCommandPath.mClusterId));
-            return Protocols::InteractionModel::Status::UnsupportedCommand;
-        }
-        }
-    }
-
-    if (CHIP_NO_ERROR != TLVError || !wasHandled)
-    {
-        ChipLogProgress(Zcl, "Failed to dispatch command, TLVError=%" CHIP_ERROR_FORMAT, TLVError.Format());
-        return Protocols::InteractionModel::Status::InvalidCommand;
-    }
-
-    // We use success as a marker that no special handling is required
-    // This is to avoid having a std::optional which uses slightly more code.
-    return Protocols::InteractionModel::Status::Success;
-}
-
-} // namespace WiFiNetworkDiagnostics
-
 } // namespace Clusters
 
 void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, TLV::TLVReader & aReader, CommandHandler * apCommandObj)
@@ -1054,9 +1013,6 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, TLV:
     case Clusters::ThreadNetworkDiagnostics::Id:
         errorStatus = Clusters::ThreadNetworkDiagnostics::DispatchServerCommand(apCommandObj, aCommandPath, aReader);
         break;
-    case Clusters::WiFiNetworkDiagnostics::Id:
-        errorStatus = Clusters::WiFiNetworkDiagnostics::DispatchServerCommand(apCommandObj, aCommandPath, aReader);
-        break;
     default:
         ChipLogError(Zcl, "Unknown cluster " ChipLogFormatMEI, ChipLogValueMEI(aCommandPath.mClusterId));
         errorStatus = Protocols::InteractionModel::Status::UnsupportedCluster;

diff --git a/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp b/src/app/clusters/wifi-network-diagnostics-server/wifi-network-diagnostics-server.cpp
@@ -23,6 +23,8 @@
 #include <app/AttributeAccessInterface.h>
 #include <app/AttributeAccessInterfaceRegistry.h>
 #include <app/CommandHandler.h>
+#include <app/CommandHandlerInterface.h>
+#include <app/CommandHandlerInterfaceRegistry.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/EventLogging.h>
 #include <app/util/attribute-storage.h>
@@ -40,11 +42,15 @@ using chip::DeviceLayer::GetDiagnosticDataProvider;
 
 namespace {
 
-class WiFiDiagosticsAttrAccess : public AttributeAccessInterface
+class WiFiDiagosticsGlobalInstance : public AttributeAccessInterface, public CommandHandlerInterface
 {
 public:
     // Register for the WiFiNetworkDiagnostics cluster on all endpoints.
-    WiFiDiagosticsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), WiFiNetworkDiagnostics::Id) {}
+    WiFiDiagosticsGlobalInstance(DiagnosticDataProvider & diagnosticProvider) :
+        AttributeAccessInterface(Optional<EndpointId>::Missing(), WiFiNetworkDiagnostics::Id),
+        CommandHandlerInterface(Optional<EndpointId>::Missing(), WiFiNetworkDiagnostics::Id),
+        mDiagnosticProvider(diagnosticProvider)
+    {}
 
     CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
@@ -57,14 +63,20 @@ class WiFiDiagosticsAttrAccess : public AttributeAccessInterface
     CHIP_ERROR ReadWiFiVersion(AttributeValueEncoder & aEncoder);
     CHIP_ERROR ReadChannelNumber(AttributeValueEncoder & aEncoder);
     CHIP_ERROR ReadWiFiRssi(AttributeValueEncoder & aEncoder);
+
+    void InvokeCommand(HandlerContext & ctx) override;
+
+    void HandleResetCounts(HandlerContext & ctx, const Commands::ResetCounts::DecodableType & commandData);
+
+    DiagnosticDataProvider & mDiagnosticProvider;
 };
 
 template <typename T, typename Type>
-CHIP_ERROR WiFiDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (DiagnosticDataProvider::*getter)(T &), Type & data,
-                                                     AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::ReadIfSupported(CHIP_ERROR (DiagnosticDataProvider::*getter)(T &), Type & data,
+                                                         AttributeValueEncoder & aEncoder)
 {
     T value;
-    CHIP_ERROR err = (DeviceLayer::GetDiagnosticDataProvider().*getter)(value);
+    CHIP_ERROR err = (mDiagnosticProvider.*getter)(value);
 
     if (err == CHIP_NO_ERROR)
     {
@@ -78,13 +90,13 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (DiagnosticDataP
     return aEncoder.Encode(data);
 }
 
-CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiBssId(AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::ReadWiFiBssId(AttributeValueEncoder & aEncoder)
 {
     Attributes::Bssid::TypeInfo::Type bssid;
 
     uint8_t bssidBytes[chip::DeviceLayer::kMaxHardwareAddrSize];
     MutableByteSpan bssidSpan(bssidBytes);
-    if (DeviceLayer::GetDiagnosticDataProvider().GetWiFiBssId(bssidSpan) == CHIP_NO_ERROR)
+    if (mDiagnosticProvider.GetWiFiBssId(bssidSpan) == CHIP_NO_ERROR)
     {
         if (!bssidSpan.empty())
         {
@@ -101,12 +113,12 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiBssId(AttributeValueEncoder & aEnco
     return aEncoder.Encode(bssid);
 }
 
-CHIP_ERROR WiFiDiagosticsAttrAccess::ReadSecurityType(AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::ReadSecurityType(AttributeValueEncoder & aEncoder)
 {
     Attributes::SecurityType::TypeInfo::Type securityType;
     SecurityTypeEnum value = SecurityTypeEnum::kUnspecified;
 
-    if (DeviceLayer::GetDiagnosticDataProvider().GetWiFiSecurityType(value) == CHIP_NO_ERROR)
+    if (mDiagnosticProvider.GetWiFiSecurityType(value) == CHIP_NO_ERROR)
     {
         securityType.SetNonNull(value);
         ChipLogProgress(Zcl, "The current type of Wi-Fi security used: %d", to_underlying(value));
@@ -119,12 +131,12 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadSecurityType(AttributeValueEncoder & aE
     return aEncoder.Encode(securityType);
 }
 
-CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiVersion(AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::ReadWiFiVersion(AttributeValueEncoder & aEncoder)
 {
     Attributes::WiFiVersion::TypeInfo::Type version;
     WiFiVersionEnum value = WiFiVersionEnum::kUnknownEnumValue;
 
-    if (DeviceLayer::GetDiagnosticDataProvider().GetWiFiVersion(value) == CHIP_NO_ERROR)
+    if (mDiagnosticProvider.GetWiFiVersion(value) == CHIP_NO_ERROR)
     {
         version.SetNonNull(value);
         ChipLogProgress(Zcl, "The current 802.11 standard version in use by the Node: %d", to_underlying(value));
@@ -137,12 +149,12 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiVersion(AttributeValueEncoder & aEn
     return aEncoder.Encode(version);
 }
 
-CHIP_ERROR WiFiDiagosticsAttrAccess::ReadChannelNumber(AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::ReadChannelNumber(AttributeValueEncoder & aEncoder)
 {
     Attributes::ChannelNumber::TypeInfo::Type channelNumber;
     uint16_t value = 0;
 
-    if (DeviceLayer::GetDiagnosticDataProvider().GetWiFiChannelNumber(value) == CHIP_NO_ERROR)
+    if (mDiagnosticProvider.GetWiFiChannelNumber(value) == CHIP_NO_ERROR)
     {
         channelNumber.SetNonNull(value);
         ChipLogProgress(Zcl, "The channel that Wi-Fi communication is currently operating on is: %d", value);
@@ -155,12 +167,12 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadChannelNumber(AttributeValueEncoder & a
     return aEncoder.Encode(channelNumber);
 }
 
-CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiRssi(AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::ReadWiFiRssi(AttributeValueEncoder & aEncoder)
 {
     Attributes::Rssi::TypeInfo::Type rssi;
     int8_t value = 0;
 
-    if (DeviceLayer::GetDiagnosticDataProvider().GetWiFiRssi(value) == CHIP_NO_ERROR)
+    if (mDiagnosticProvider.GetWiFiRssi(value) == CHIP_NO_ERROR)
     {
         rssi.SetNonNull(value);
         ChipLogProgress(Zcl, "The current RSSI of the Node’s Wi-Fi radio in dB: %d", value);
@@ -174,9 +186,7 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiRssi(AttributeValueEncoder & aEncod
     return aEncoder.Encode(rssi);
 }
 
-WiFiDiagosticsAttrAccess gAttrAccess;
-
-CHIP_ERROR WiFiDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
+CHIP_ERROR WiFiDiagosticsGlobalInstance::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     if (aPath.mClusterId != WiFiNetworkDiagnostics::Id)
     {
@@ -240,6 +250,25 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPat
     return CHIP_NO_ERROR;
 }
 
+void WiFiDiagosticsGlobalInstance::InvokeCommand(HandlerContext & handlerContext)
+{
+    switch (handlerContext.mRequestPath.mCommandId)
+    {
+    case Commands::ResetCounts::Id:
+        CommandHandlerInterface::HandleCommand<Commands::ResetCounts::DecodableType>(
+            handlerContext, [this](HandlerContext & ctx, const auto & commandData) { HandleResetCounts(ctx, commandData); });
+        break;
+    }
+}
+
+void WiFiDiagosticsGlobalInstance::HandleResetCounts(HandlerContext & ctx, const Commands::ResetCounts::DecodableType & commandData)
+{
+    mDiagnosticProvider.ResetWiFiNetworkDiagnosticsCounts();
+    ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::Status::Success);
+}
+
+WiFiDiagosticsGlobalInstance gWiFiDiagosticsInstance(DeviceLayer::GetDiagnosticDataProvider());
+
 } // anonymous namespace
 
 namespace chip {
@@ -316,18 +345,10 @@ void WiFiDiagnosticsServer::OnConnectionStatusChanged(uint8_t connectionStatus)
 } // namespace app
 } // namespace chip
 
-bool emberAfWiFiNetworkDiagnosticsClusterResetCountsCallback(app::CommandHandler * commandObj,
-                                                             const app::ConcreteCommandPath & commandPath,
-                                                             const Commands::ResetCounts::DecodableType & commandData)
-{
-    DeviceLayer::GetDiagnosticDataProvider().ResetWiFiNetworkDiagnosticsCounts();
-    commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::Success);
-
-    return true;
-}
-
 void MatterWiFiNetworkDiagnosticsPluginServerInitCallback()
 {
-    AttributeAccessInterfaceRegistry::Instance().Register(&gAttrAccess);
+    AttributeAccessInterfaceRegistry::Instance().Register(&gWiFiDiagosticsInstance);
+    CommandHandlerInterfaceRegistry::Instance().RegisterCommandHandler(&gWiFiDiagosticsInstance);
+
     GetDiagnosticDataProvider().SetWiFiDiagnosticsDelegate(&WiFiDiagnosticsServer::Instance());
 }
diff --git a/src/app/common/templates/config-data.yaml b/src/app/common/templates/config-data.yaml
@@ -52,6 +52,7 @@ CommandHandlerInterfaceOnlyClusters:
     - General Commissioning
     - General Diagnostics
     - Software Diagnostics
+    - Wi-Fi Network Diagnostics
 
 # We need a more configurable way of deciding which clusters have which init functions....
 # See /~https://github.com/project-chip/connectedhomeip/issues/4369

diff --git a/zzz_generated/app-common/app-common/zap-generated/callback.h b/zzz_generated/app-common/app-common/zap-generated/callback.h