diff --git a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp index 315bf2f3711f35..e8296fa478d95d 100644 --- a/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp +++ b/examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp @@ -57,6 +57,7 @@ void FabricSyncAddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_E DeviceManager::Instance().UpdateLastUsedNodeId(mBridgeNodeId); DeviceManager::Instance().SubscribeRemoteFabricBridge(); + DeviceManager::Instance().InitCommissionerControl(); if (DeviceManager::Instance().IsLocalBridgeReady()) { @@ -284,7 +285,7 @@ void FabricSyncDeviceCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERRO if (err == CHIP_NO_ERROR) { - DeviceManager::Instance().AddSyncedDevice(Device(mAssignedNodeId, mRemoteEndpointId)); + DeviceManager::Instance().AddSyncedDevice(SyncedDevice(mAssignedNodeId, mRemoteEndpointId)); } else { diff --git a/examples/fabric-admin/device_manager/DeviceManager.cpp b/examples/fabric-admin/device_manager/DeviceManager.cpp index 9f40f92a87c411..7e40a8196555ea 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.cpp +++ b/examples/fabric-admin/device_manager/DeviceManager.cpp @@ -72,39 +72,35 @@ void DeviceManager::UpdateLastUsedNodeId(NodeId nodeId) void DeviceManager::SetRemoteBridgeNodeId(chip::NodeId nodeId) { mRemoteBridgeNodeId = nodeId; - - if (mRemoteBridgeNodeId != kUndefinedNodeId) - { - mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); - } + ChipLogProgress(NotSpecified, "Set remote bridge NodeId:" ChipLogFormatX64, ChipLogValueX64(mRemoteBridgeNodeId)); } -void DeviceManager::AddSyncedDevice(const Device & device) +void DeviceManager::AddSyncedDevice(const SyncedDevice & device) { mSyncedDevices.insert(device); ChipLogProgress(NotSpecified, "Added synced device: NodeId:" ChipLogFormatX64 ", EndpointId %u", ChipLogValueX64(device.GetNodeId()), device.GetEndpointId()); } -Device * DeviceManager::FindDeviceByEndpoint(EndpointId endpointId) +SyncedDevice * DeviceManager::FindDeviceByEndpoint(EndpointId endpointId) { for (auto & device : mSyncedDevices) { if (device.GetEndpointId() == endpointId) { - return const_cast(&device); + return const_cast(&device); } } return nullptr; } -Device * DeviceManager::FindDeviceByNode(NodeId nodeId) +SyncedDevice * DeviceManager::FindDeviceByNode(NodeId nodeId) { for (auto & device : mSyncedDevices) { if (device.GetNodeId() == nodeId) { - return const_cast(&device); + return const_cast(&device); } } return nullptr; @@ -116,8 +112,8 @@ void DeviceManager::RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId) RemoveSynchronizedDevice(scopedNodeId); #endif - NodeId nodeId = scopedNodeId.GetNodeId(); - Device * device = FindDeviceByNode(nodeId); + NodeId nodeId = scopedNodeId.GetNodeId(); + SyncedDevice * device = FindDeviceByNode(nodeId); if (device == nullptr) { ChipLogProgress(NotSpecified, "No device found with NodeId:" ChipLogFormatX64, ChipLogValueX64(nodeId)); @@ -129,6 +125,18 @@ void DeviceManager::RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId) ChipLogValueX64(device->GetNodeId()), device->GetEndpointId()); } +void DeviceManager::InitCommissionerControl() +{ + if (mRemoteBridgeNodeId != kUndefinedNodeId) + { + mCommissionerControl.Init(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, kAggregatorEndpointId); + } + else + { + ChipLogError(NotSpecified, "Failed to initialize the Commissioner Control delegate"); + } +} + void DeviceManager::OpenDeviceCommissioningWindow(ScopedNodeId scopedNodeId, uint32_t iterations, uint16_t commissioningTimeoutSec, uint16_t discriminator, const ByteSpan & salt, const ByteSpan & verifier) { @@ -212,10 +220,10 @@ void DeviceManager::UnpairLocalFabricBridge() void DeviceManager::SubscribeRemoteFabricBridge() { - ChipLogProgress(NotSpecified, "Start subscription to the remote bridge.") + ChipLogProgress(NotSpecified, "Start subscription to the remote bridge."); - CHIP_ERROR error = mBridgeSubscriber.StartSubscription(PairingManager::Instance().CurrentCommissioner(), - mRemoteBridgeNodeId, kAggregatorEndpointId); + CHIP_ERROR error = mBridgeSubscriber.StartSubscription(PairingManager::Instance().CurrentCommissioner(), mRemoteBridgeNodeId, + kAggregatorEndpointId); if (error != CHIP_NO_ERROR) { @@ -354,7 +362,7 @@ void DeviceManager::HandleAttributePartsListUpdate(TLV::TLVReader & data) std::vector removedEndpoints; // Note: We're using vectors and manual searches instead of set operations - // because we need to work with the Device objects in mSyncedDevices, + // because we need to work with the SyncedDevice objects in mSyncedDevices, // not just their EndpointIds. This approach allows us to access the full // Device information when processing changes. @@ -381,15 +389,16 @@ void DeviceManager::HandleAttributePartsListUpdate(TLV::TLVReader & data) for (const auto & endpoint : addedEndpoints) { // print to console - fprintf(stderr, "A new device is added on Endpoint: %u\n", endpoint); + fprintf(stderr, "A new endpoint %u is added on the remote bridge\n", endpoint); } // Process removed endpoints for (const auto & endpoint : removedEndpoints) { - ChipLogProgress(NotSpecified, "Endpoint removed: %u", endpoint); + // print to console + fprintf(stderr, "Endpoint %u removed from the remote bridge\n", endpoint); - Device * device = FindDeviceByEndpoint(endpoint); + SyncedDevice * device = FindDeviceByEndpoint(endpoint); if (device == nullptr) { @@ -467,6 +476,7 @@ void DeviceManager::HandleCommandResponse(const app::ConcreteCommandPath & path, if (path.mClusterId == app::Clusters::CommissionerControl::Id && path.mCommandId == app::Clusters::CommissionerControl::Commands::ReverseOpenCommissioningWindow::Id) { + VerifyOrDie(path.mEndpointId == kAggregatorEndpointId); HandleReverseOpenCommissioningWindow(data); } } diff --git a/examples/fabric-admin/device_manager/DeviceManager.h b/examples/fabric-admin/device_manager/DeviceManager.h index 594fcfc5745703..8c273ee53d3af7 100644 --- a/examples/fabric-admin/device_manager/DeviceManager.h +++ b/examples/fabric-admin/device_manager/DeviceManager.h @@ -32,15 +32,15 @@ constexpr uint32_t kDefaultSetupPinCode = 20202021; constexpr uint16_t kDefaultLocalBridgePort = 5540; constexpr uint16_t kResponseTimeoutSeconds = 30; -class Device +class SyncedDevice { public: - Device(chip::NodeId nodeId, chip::EndpointId endpointId) : mNodeId(nodeId), mEndpointId(endpointId) {} + SyncedDevice(chip::NodeId nodeId, chip::EndpointId endpointId) : mNodeId(nodeId), mEndpointId(endpointId) {} chip::NodeId GetNodeId() const { return mNodeId; } chip::EndpointId GetEndpointId() const { return mEndpointId; } - bool operator<(const Device & other) const + bool operator<(const SyncedDevice & other) const { return mNodeId < other.mNodeId || (mNodeId == other.mNodeId && mEndpointId < other.mEndpointId); } @@ -81,10 +81,15 @@ class DeviceManager bool IsLocalBridgeReady() const { return mLocalBridgeNodeId != chip::kUndefinedNodeId; } - void AddSyncedDevice(const Device & device); + void AddSyncedDevice(const SyncedDevice & device); void RemoveSyncedDevice(chip::ScopedNodeId scopedNodeId); + /** + * @brief Initializes the CommissionerControl for fabric sync setup process. + */ + void InitCommissionerControl(); + /** * @brief Determines whether a given nodeId corresponds to the "current bridge device," either local or remote. * @@ -177,8 +182,8 @@ class DeviceManager void HandleCommandResponse(const chip::app::ConcreteCommandPath & path, chip::TLV::TLVReader & data); - Device * FindDeviceByEndpoint(chip::EndpointId endpointId); - Device * FindDeviceByNode(chip::NodeId nodeId); + SyncedDevice * FindDeviceByEndpoint(chip::EndpointId endpointId); + SyncedDevice * FindDeviceByNode(chip::NodeId nodeId); private: void RequestCommissioningApproval(); @@ -193,8 +198,6 @@ class DeviceManager void HandleReverseOpenCommissioningWindow(chip::TLV::TLVReader & data); - static DeviceManager sInstance; - chip::NodeId mLastUsedNodeId = 0; // The Node ID of the remote bridge used for Fabric-Sync @@ -207,7 +210,7 @@ class DeviceManager // This represents the bridge within its own ecosystem. chip::NodeId mLocalBridgeNodeId = chip::kUndefinedNodeId; - std::set mSyncedDevices; + std::set mSyncedDevices; bool mInitialized = false; uint64_t mRequestId = 0; diff --git a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp index 092c2505d19d73..9274b98d4a0d52 100644 --- a/examples/fabric-admin/device_manager/DeviceSynchronization.cpp +++ b/examples/fabric-admin/device_manager/DeviceSynchronization.cpp @@ -142,13 +142,16 @@ void DeviceSynchronizer::OnReportEnd() void DeviceSynchronizer::OnDone(app::ReadClient * apReadClient) { + ChipLogProgress(NotSpecified, "Synchronization complete for NodeId:" ChipLogFormatX64, ChipLogValueX64(mNodeId)); + #if defined(PW_RPC_ENABLED) if (mState == State::ReceivedResponse && !DeviceManager::Instance().IsCurrentBridgeDevice(mNodeId)) { GetUniqueId(); if (mState == State::GettingUid) { - // GetUniqueId was successful and we rely on callback to call SynchronizationCompleteAddDevice. + ChipLogProgress(NotSpecified, + "GetUniqueId was successful and we rely on callback to call SynchronizationCompleteAddDevice."); return; } SynchronizationCompleteAddDevice(); @@ -206,6 +209,8 @@ void DeviceSynchronizer::StartDeviceSynchronization(Controller::DeviceController mNodeId = nodeId; + ChipLogProgress(NotSpecified, "Start device synchronization for NodeId:" ChipLogFormatX64, ChipLogValueX64(mNodeId)); + #if defined(PW_RPC_ENABLED) mCurrentDeviceData = chip_rpc_SynchronizedDevice_init_default; mCurrentDeviceData.has_id = true; @@ -272,6 +277,7 @@ void DeviceSynchronizer::GetUniqueId() void DeviceSynchronizer::SynchronizationCompleteAddDevice() { VerifyOrDie(mState == State::ReceivedResponse || mState == State::GettingUid); + ChipLogProgress(NotSpecified, "Synchronization complete and add device"); #if defined(PW_RPC_ENABLED) AddSynchronizedDevice(mCurrentDeviceData); diff --git a/examples/fabric-admin/device_manager/PairingManager.cpp b/examples/fabric-admin/device_manager/PairingManager.cpp index 8474d78f023343..8d985ac553f010 100644 --- a/examples/fabric-admin/device_manager/PairingManager.cpp +++ b/examples/fabric-admin/device_manager/PairingManager.cpp @@ -285,6 +285,12 @@ void PairingManager::OnPairingDeleted(CHIP_ERROR err) void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) { + if (mPairingDelegate) + { + mPairingDelegate->OnCommissioningComplete(nodeId, err); + SetPairingDelegate(nullptr); + } + if (err == CHIP_NO_ERROR) { // print to console @@ -308,12 +314,6 @@ void PairingManager::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err) } ChipLogProgress(NotSpecified, "Device commissioning Failure: %s", ErrorStr(err)); } - - if (mPairingDelegate) - { - mPairingDelegate->OnCommissioningComplete(nodeId, err); - SetPairingDelegate(nullptr); - } } void PairingManager::OnReadCommissioningInfo(const Controller::ReadCommissioningInfo & info) diff --git a/examples/fabric-admin/rpc/RpcServer.cpp b/examples/fabric-admin/rpc/RpcServer.cpp index 05fb9492248131..f59a2729dbfa84 100644 --- a/examples/fabric-admin/rpc/RpcServer.cpp +++ b/examples/fabric-admin/rpc/RpcServer.cpp @@ -55,7 +55,7 @@ struct ScopedNodeIdHasher } }; -class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate +class FabricAdmin final : public rpc::FabricAdmin, public admin::PairingDelegate, public IcdManager::Delegate { public: void OnCheckInCompleted(const app::ICDClientInfo & clientInfo) override @@ -96,6 +96,33 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate } } + void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) override + { + if (mNodeId != deviceId) + { + ChipLogError(NotSpecified, + "Tried to pair a non-bridge device (0x:" ChipLogFormatX64 ") with result: %" CHIP_ERROR_FORMAT, + ChipLogValueX64(deviceId), err.Format()); + return; + } + else + { + ChipLogProgress(NotSpecified, "Reverse commission succeeded for NodeId:" ChipLogFormatX64, ChipLogValueX64(mNodeId)); + } + + if (err == CHIP_NO_ERROR) + { + DeviceManager::Instance().SetRemoteBridgeNodeId(deviceId); + } + else + { + ChipLogError(NotSpecified, "Failed to pair bridge device (0x:" ChipLogFormatX64 ") with error: %" CHIP_ERROR_FORMAT, + ChipLogValueX64(deviceId), err.Format()); + } + + mNodeId = kUndefinedNodeId; + } + pw::Status OpenCommissioningWindow(const chip_rpc_DeviceCommissioningWindowInfo & request, chip_rpc_OperationStatus & response) override { @@ -146,13 +173,14 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate if (error == CHIP_NO_ERROR) { - NodeId nodeId = DeviceManager::Instance().GetNextAvailableNodeId(); + mNodeId = DeviceManager::Instance().GetNextAvailableNodeId(); + ; // After responding with RequestCommissioningApproval to the node where the client initiated the // RequestCommissioningApproval, you need to wait for it to open a commissioning window on its bridge. usleep(kCommissionPrepareTimeMs * 1000); - - DeviceManager::Instance().PairRemoteDevice(nodeId, code.c_str()); + PairingManager::Instance().SetPairingDelegate(this); + DeviceManager::Instance().PairRemoteDevice(mNodeId, code.c_str()); } else { @@ -224,6 +252,8 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate Platform::Delete(data); } + NodeId mNodeId = chip::kUndefinedNodeId; + // Modifications to mPendingCheckIn should be done on the MatterEventLoop thread // otherwise we would need a mutex protecting this data to prevent race as this // data is accessible by both RPC thread and Matter eventloop.