[Fabric-Admin] Refactor to use API methods instead of PushCommand (1/3) (#35614)

yufengwangca · andy31415 · web-flow · commit 010decd199af · 2024-09-18T20:12:52.000Z
* [Fabric-Admin] Refactor to use API methods instead of PushCommand to talk to SDK

* Address review comments

* Update examples/fabric-admin/device_manager/PairingManager.h

Co-authored-by: Andrei Litvin &lt;andy314@gmail.com&gt;

* Update per review comments

* Update commissioningTimeout to commissioningTimeoutSec

---------

Co-authored-by: Andrei Litvin &lt;andy314@gmail.com&gt;
diff --git a/examples/fabric-admin/BUILD.gn b/examples/fabric-admin/BUILD.gn
@@ -88,6 +88,8 @@ static_library("fabric-admin-utils") {
     "device_manager/DeviceSubscriptionManager.h",
     "device_manager/DeviceSynchronization.cpp",
     "device_manager/DeviceSynchronization.h",
+    "device_manager/PairingManager.cpp",
+    "device_manager/PairingManager.h",
     "device_manager/UniqueIdGetter.cpp",
     "device_manager/UniqueIdGetter.h",
   ]
diff --git a/examples/fabric-admin/commands/common/CHIPCommand.cpp b/examples/fabric-admin/commands/common/CHIPCommand.cpp
@@ -21,6 +21,7 @@
 #include "IcdManager.h"
 #include <controller/CHIPDeviceControllerFactory.h>
 #include <credentials/attestation_verifier/FileAttestationTrustStore.h>
+#include <device_manager/PairingManager.h>
 #include <lib/core/CHIPConfig.h>
 #include <lib/core/CHIPVendorIdentifiers.hpp>
 #include <lib/support/CodeUtils.h>
@@ -181,6 +182,8 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
     mCredIssuerCmds->SetCredentialIssuerOption(CredentialIssuerCommands::CredentialIssuerOptions::kAllowTestCdSigningKey,
                                                allowTestCdSigningKey);
 
+    PairingManager::Instance().Init(&CurrentCommissioner());
+
     return CHIP_NO_ERROR;
 }
 
diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp
@@ -344,15 +344,7 @@ CHIP_ERROR FabricSyncDeviceCommand::RunCommand(EndpointId remoteId)
         return CHIP_NO_ERROR;
     }
 
-    OpenCommissioningWindowCommand * openCommand =
-        static_cast<OpenCommissioningWindowCommand *>(CommandMgr().GetCommandByName("pairing", "open-commissioning-window"));
-
-    if (openCommand == nullptr)
-    {
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
-
-    openCommand->RegisterDelegate(this);
+    PairingManager::Instance().SetOpenCommissioningWindowDelegate(this);
 
     DeviceMgr().OpenRemoteDeviceCommissioningWindow(remoteId);
 
diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h
@@ -19,8 +19,8 @@
 #pragma once
 
 #include <commands/common/CHIPCommand.h>
-#include <commands/pairing/OpenCommissioningWindowCommand.h>
 #include <commands/pairing/PairingCommand.h>
+#include <device_manager/PairingManager.h>
 
 // Constants
 constexpr uint32_t kCommissionPrepareTimeMs = 500;
diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.cpp
@@ -70,14 +70,6 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand()
 void OpenCommissioningWindowCommand::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err,
                                                                        chip::SetupPayload payload)
 {
-    OpenCommissioningWindowCommand * self = static_cast<OpenCommissioningWindowCommand *>(context);
-    if (self->mDelegate)
-    {
-        self->mDelegate->OnCommissioningWindowOpened(remoteId, err, payload);
-        self->UnregisterDelegate();
-    }
-
-    LogErrorOnFailure(err);
     OnOpenBasicCommissioningWindowResponse(context, remoteId, err);
 }
 
diff --git a/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h b/examples/fabric-admin/commands/pairing/OpenCommissioningWindowCommand.h
@@ -22,13 +22,6 @@
 #include <controller/CommissioningWindowOpener.h>
 #include <lib/support/CHIPMem.h>
 
-class CommissioningWindowDelegate
-{
-public:
-    virtual void OnCommissioningWindowOpened(chip::NodeId deviceId, CHIP_ERROR err, chip::SetupPayload payload) = 0;
-    virtual ~CommissioningWindowDelegate()                                                                      = default;
-};
-
 class OpenCommissioningWindowCommand : public CHIPCommand
 {
 public:
@@ -57,9 +50,6 @@ class OpenCommissioningWindowCommand : public CHIPCommand
                     "params if absent");
     }
 
-    void RegisterDelegate(CommissioningWindowDelegate * delegate) { mDelegate = delegate; }
-    void UnregisterDelegate() { mDelegate = nullptr; }
-
     /////////// CHIPCommand Interface /////////
     CHIP_ERROR RunCommand() override;
 
@@ -71,7 +61,6 @@ class OpenCommissioningWindowCommand : public CHIPCommand
     NodeId mNodeId;
     chip::EndpointId mEndpointId;
     chip::Controller::CommissioningWindowOpener::CommissioningWindowOption mCommissioningWindowOption;
-    CommissioningWindowDelegate * mDelegate = nullptr;
     uint16_t mCommissioningWindowTimeout;
     uint32_t mIteration;
     uint16_t mDiscriminator;
diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp
@@ -20,6 +20,7 @@
 
 #include <commands/interactive/InteractiveCommands.h>
 #include <crypto/RandUtils.h>
+#include <device_manager/PairingManager.h>
 #include <lib/support/StringBuilder.h>
 
 #include <cstdio>
@@ -30,13 +31,12 @@ using namespace chip::app::Clusters;
 
 namespace {
 
-constexpr uint16_t kWindowTimeout              = 300;
-constexpr uint16_t kIteration                  = 1000;
-constexpr uint16_t kSubscribeMinInterval       = 0;
-constexpr uint16_t kSubscribeMaxInterval       = 60;
-constexpr uint16_t kAggragatorEndpointId       = 1;
-constexpr uint16_t kMaxDiscriminatorLength     = 4095;
-constexpr uint8_t kEnhancedCommissioningMethod = 1;
+constexpr uint16_t kWindowTimeout          = 300;
+constexpr uint16_t kIteration              = 1000;
+constexpr uint16_t kSubscribeMinInterval   = 0;
+constexpr uint16_t kSubscribeMaxInterval   = 60;
+constexpr uint16_t kAggragatorEndpointId   = 1;
+constexpr uint16_t kMaxDiscriminatorLength = 4095;
 
 } // namespace
 
@@ -115,37 +115,39 @@ void DeviceManager::RemoveSyncedDevice(NodeId nodeId)
                     ChipLogValueX64(device->GetNodeId()), device->GetEndpointId());
 }
 
-void DeviceManager::OpenDeviceCommissioningWindow(NodeId nodeId, uint32_t commissioningTimeout, uint32_t iterations,
-                                                  uint32_t discriminator, const char * saltHex, const char * verifierHex)
+void DeviceManager::OpenDeviceCommissioningWindow(NodeId nodeId, uint32_t commissioningTimeoutSec, uint32_t iterations,
+                                                  uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier)
 {
-    ChipLogProgress(NotSpecified, "Open the commissioning window of device with NodeId:" ChipLogFormatX64, ChipLogValueX64(nodeId));
+    ChipLogProgress(NotSpecified, "Opening commissioning window for Node ID: " ChipLogFormatX64, ChipLogValueX64(nodeId));
 
     // Open the commissioning window of a device within its own fabric.
-    StringBuilder<kMaxCommandSize> commandBuilder;
-
-    commandBuilder.Add("pairing open-commissioning-window ");
-    commandBuilder.AddFormat("%lu %d %d %d %d %d --salt hex:%s --verifier hex:%s", nodeId, kRootEndpointId,
-                             kEnhancedCommissioningMethod, commissioningTimeout, iterations, discriminator, saltHex, verifierHex);
-
-    PushCommand(commandBuilder.c_str());
+    CHIP_ERROR err = PairingManager::Instance().OpenCommissioningWindow(nodeId, kRootEndpointId, commissioningTimeoutSec,
+                                                                        iterations, discriminator, salt, verifier);
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(NotSpecified, "Failed to open commissioning window: %s", ErrorStr(err));
+    }
 }
 
 void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpointId)
 {
     // Open the commissioning window of a device from another fabric via its fabric bridge.
     // This method constructs and sends a command to open the commissioning window for a device
     // that is part of a different fabric, accessed through a fabric bridge.
-    StringBuilder<kMaxCommandSize> commandBuilder;
 
     // Use random discriminator to have less chance of collision.
     uint16_t discriminator =
         Crypto::GetRandU16() % (kMaxDiscriminatorLength + 1); // Include the upper limit kMaxDiscriminatorLength
 
-    commandBuilder.Add("pairing open-commissioning-window ");
-    commandBuilder.AddFormat("%lu %d %d %d %d %d", mRemoteBridgeNodeId, remoteEndpointId, kEnhancedCommissioningMethod,
-                             kWindowTimeout, kIteration, discriminator);
+    ByteSpan emptySalt;
+    ByteSpan emptyVerifier;
 
-    PushCommand(commandBuilder.c_str());
+    CHIP_ERROR err = PairingManager::Instance().OpenCommissioningWindow(mRemoteBridgeNodeId, remoteEndpointId, kWindowTimeout,
+                                                                        kIteration, discriminator, emptySalt, emptyVerifier);
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(NotSpecified, "Failed to open commissioning window: %s", ErrorStr(err));
+    }
 }
 
 void DeviceManager::PairRemoteFabricBridge(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp,
@@ -421,6 +423,7 @@ void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data)
 {
     CommissionerControl::Commands::ReverseOpenCommissioningWindow::DecodableType value;
     CHIP_ERROR error = app::DataModel::Decode(data, value);
+
     if (error != CHIP_NO_ERROR)
     {
         ChipLogError(NotSpecified, "Failed to decode command response value. Error: %" CHIP_ERROR_FORMAT, error.Format());
@@ -432,18 +435,12 @@ void DeviceManager::HandleReverseOpenCommissioningWindow(TLV::TLVReader & data)
     ChipLogProgress(NotSpecified, "  commissioningTimeout: %u", value.commissioningTimeout);
     ChipLogProgress(NotSpecified, "  discriminator: %u", value.discriminator);
     ChipLogProgress(NotSpecified, "  iterations: %u", value.iterations);
+    ChipLogProgress(NotSpecified, "  PAKEPasscodeVerifier size: %lu", value.PAKEPasscodeVerifier.size());
+    ChipLogProgress(NotSpecified, "  salt size: %lu", value.salt.size());
 
-    char verifierHex[Crypto::kSpake2p_VerifierSerialized_Length * 2 + 1];
-    Encoding::BytesToHex(value.PAKEPasscodeVerifier.data(), value.PAKEPasscodeVerifier.size(), verifierHex, sizeof(verifierHex),
-                         Encoding::HexFlags::kNullTerminate);
-    ChipLogProgress(NotSpecified, "  PAKEPasscodeVerifier: %s", verifierHex);
-
-    char saltHex[Crypto::kSpake2p_Max_PBKDF_Salt_Length * 2 + 1];
-    Encoding::BytesToHex(value.salt.data(), value.salt.size(), saltHex, sizeof(saltHex), Encoding::HexFlags::kNullTerminate);
-    ChipLogProgress(NotSpecified, "  salt: %s", saltHex);
-
-    OpenDeviceCommissioningWindow(mLocalBridgeNodeId, value.commissioningTimeout, value.iterations, value.discriminator, saltHex,
-                                  verifierHex);
+    OpenDeviceCommissioningWindow(mLocalBridgeNodeId, value.commissioningTimeout, value.iterations, value.discriminator,
+                                  ByteSpan(value.salt.data(), value.salt.size()),
+                                  ByteSpan(value.PAKEPasscodeVerifier.data(), value.PAKEPasscodeVerifier.size()));
 }
 
 void DeviceManager::HandleAttributeData(const app::ConcreteDataAttributePath & path, TLV::TLVReader & data)
diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h
@@ -94,18 +94,18 @@ class DeviceManager : public PairingDelegate
      * This function initiates the process to open the commissioning window for a device identified by the given node ID.
      *
      * @param nodeId               The ID of the node that should open the commissioning window.
-     * @param commissioningTimeout The time in seconds before the commissioning window closes. This value determines
+     * @param commissioningTimeoutSec The time in seconds before the commissioning window closes. This value determines
      *                             how long the commissioning window remains open for incoming connections.
      * @param iterations           The number of PBKDF (Password-Based Key Derivation Function) iterations to use
      *                             for deriving the PAKE (Password Authenticated Key Exchange) verifier.
      * @param discriminator        The device-specific discriminator, determined during commissioning, which helps
      *                             to uniquely identify the device among others.
-     * @param saltHex              The hexadecimal-encoded salt used in the cryptographic operations for commissioning.
-     * @param verifierHex          The hexadecimal-encoded PAKE verifier used to authenticate the commissioning process.
+     * @param salt                 The salt used in the cryptographic operations for commissioning.
+     * @param verifier             The PAKE verifier used to authenticate the commissioning process.
      *
      */
-    void OpenDeviceCommissioningWindow(chip::NodeId nodeId, uint32_t commissioningTimeout, uint32_t iterations,
-                                       uint32_t discriminator, const char * saltHex, const char * verifierHex);
+    void OpenDeviceCommissioningWindow(chip::NodeId nodeId, uint32_t commissioningTimeoutSec, uint32_t iterations,
+                                       uint16_t discriminator, const chip::ByteSpan & salt, const chip::ByteSpan & verifier);
 
     /**
      * @brief Open the commissioning window of a device from another fabric via its fabric bridge.
diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp
diff --git a/examples/fabric-admin/device_manager/PairingManager.h b/examples/fabric-admin/device_manager/PairingManager.h
diff --git a/examples/fabric-admin/rpc/RpcServer.cpp b/examples/fabric-admin/rpc/RpcServer.cpp

Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,8 @@ static_library("fabric-admin-utils") {`
`88`	`88`	`"device_manager/DeviceSubscriptionManager.h",`
`89`	`89`	`"device_manager/DeviceSynchronization.cpp",`
`90`	`90`	`"device_manager/DeviceSynchronization.h",`
	`91`	`+ "device_manager/PairingManager.cpp",`
	`92`	`+ "device_manager/PairingManager.h",`
`91`	`93`	`"device_manager/UniqueIdGetter.cpp",`
`92`	`94`	`"device_manager/UniqueIdGetter.h",`
`93`	`95`	`]`
Original file line number	Diff line number	Diff line change
`@@ -70,14 +70,6 @@ CHIP_ERROR OpenCommissioningWindowCommand::RunCommand()`
`70`	`70`	`void OpenCommissioningWindowCommand::OnOpenCommissioningWindowResponse(void * context, NodeId remoteId, CHIP_ERROR err,`
`71`	`71`	`chip::SetupPayload payload)`
`72`	`72`	`{`
`73`		`- OpenCommissioningWindowCommand * self = static_cast<OpenCommissioningWindowCommand *>(context);`
`74`		`- if (self->mDelegate)`
`75`		`- {`
`76`		`- self->mDelegate->OnCommissioningWindowOpened(remoteId, err, payload);`
`77`		`- self->UnregisterDelegate();`
`78`		`- }`
`79`		`-`
`80`		`- LogErrorOnFailure(err);`
`81`	`73`	`OnOpenBasicCommissioningWindowResponse(context, remoteId, err);`
`82`	`74`	`}`
`83`	`75`