From 1742574e4203585edc14c8e113c3551377e2f39e Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Wed, 6 Nov 2024 01:13:26 -0800 Subject: [PATCH] [Fabric-Sync] use namespace to isolate admin and bridge (#36374) --- examples/fabric-sync/admin/DeviceManager.cpp | 3 +++ examples/fabric-sync/admin/DeviceManager.h | 4 ++++ examples/fabric-sync/admin/PairingManager.cpp | 6 +++++- examples/fabric-sync/admin/PairingManager.h | 4 ++++ .../include/BridgedAdministratorCommissioning.h | 4 ++++ .../fabric-sync/bridge/include/BridgedDevice.h | 4 ++++ .../include/BridgedDeviceBasicInformationImpl.h | 4 ++++ .../bridge/include/BridgedDeviceManager.h | 12 ++++++++---- .../src/BridgedAdministratorCommissioning.cpp | 4 ++++ .../fabric-sync/bridge/src/BridgedDevice.cpp | 4 ++++ .../src/BridgedDeviceBasicInformationImpl.cpp | 4 ++++ .../bridge/src/BridgedDeviceManager.cpp | 15 ++++++++++----- examples/fabric-sync/main.cpp | 2 +- examples/fabric-sync/shell/AddBridgeCommand.cpp | 13 +++++++------ examples/fabric-sync/shell/AddBridgeCommand.h | 2 +- .../fabric-sync/shell/RemoveBridgeCommand.cpp | 16 ++++++++-------- examples/fabric-sync/shell/RemoveBridgeCommand.h | 2 +- 17 files changed, 76 insertions(+), 27 deletions(-) diff --git a/examples/fabric-sync/admin/DeviceManager.cpp b/examples/fabric-sync/admin/DeviceManager.cpp index 79f41d5dc11c17..3d2055444f92a8 100644 --- a/examples/fabric-sync/admin/DeviceManager.cpp +++ b/examples/fabric-sync/admin/DeviceManager.cpp @@ -26,6 +26,7 @@ using namespace chip; +namespace admin { // Define the static member DeviceManager DeviceManager::sInstance; @@ -109,3 +110,5 @@ void DeviceManager::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err) ChipLogProgress(NotSpecified, "Synced device with NodeId:" ChipLogFormatX64 " has been removed.", ChipLogValueX64(deviceId)); } + +} // namespace admin diff --git a/examples/fabric-sync/admin/DeviceManager.h b/examples/fabric-sync/admin/DeviceManager.h index 5615df54a9cf32..866e0c20f6c333 100644 --- a/examples/fabric-sync/admin/DeviceManager.h +++ b/examples/fabric-sync/admin/DeviceManager.h @@ -23,6 +23,8 @@ #include #include +namespace admin { + class DeviceManager : public PairingDelegate { public: @@ -97,3 +99,5 @@ inline DeviceManager & DeviceMgr() } return DeviceManager::sInstance; } + +} // namespace admin diff --git a/examples/fabric-sync/admin/PairingManager.cpp b/examples/fabric-sync/admin/PairingManager.cpp index 06c1f2b99b7336..32317b4fa30e3a 100644 --- a/examples/fabric-sync/admin/PairingManager.cpp +++ b/examples/fabric-sync/admin/PairingManager.cpp @@ -29,6 +29,8 @@ using namespace ::chip; using namespace ::chip::Controller; +namespace admin { + namespace { CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload) @@ -451,7 +453,7 @@ void PairingManager::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E if (err == CHIP_NO_ERROR) { // print to console - fprintf(stderr, "Device with Node ID: " ChipLogFormatX64 "has been successfully removed.\n", ChipLogValueX64(nodeId)); + fprintf(stderr, "Device with Node ID: " ChipLogFormatX64 " has been successfully removed.\n", ChipLogValueX64(nodeId)); } else { @@ -548,3 +550,5 @@ CHIP_ERROR PairingManager::UnpairDevice(NodeId nodeId) } }); } + +} // namespace admin diff --git a/examples/fabric-sync/admin/PairingManager.h b/examples/fabric-sync/admin/PairingManager.h index 07cba7488154f8..5ef8a0761f7f28 100644 --- a/examples/fabric-sync/admin/PairingManager.h +++ b/examples/fabric-sync/admin/PairingManager.h @@ -24,6 +24,8 @@ #include #include +namespace admin { + // Constants constexpr uint16_t kMaxManualCodeLength = 22; @@ -201,3 +203,5 @@ class PairingManager : public chip::Controller::DevicePairingDelegate, chip::Platform::UniquePtr mCurrentFabricRemover; chip::Callback::Callback mCurrentFabricRemoveCallback; }; + +} // namespace admin diff --git a/examples/fabric-sync/bridge/include/BridgedAdministratorCommissioning.h b/examples/fabric-sync/bridge/include/BridgedAdministratorCommissioning.h index 06fd9026d44225..7b5711d39557e9 100644 --- a/examples/fabric-sync/bridge/include/BridgedAdministratorCommissioning.h +++ b/examples/fabric-sync/bridge/include/BridgedAdministratorCommissioning.h @@ -20,6 +20,8 @@ #include #include +namespace bridge { + /** * @brief CADMIN cluster implementation for handling attribute interactions of bridged device endpoints. * @@ -56,3 +58,5 @@ class BridgedAdministratorCommissioning : public chip::app::AttributeAccessInter // to reflect this change. chip::app::AttributeAccessInterface * mOriginalAttributeInterface = nullptr; }; + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/include/BridgedDevice.h b/examples/fabric-sync/bridge/include/BridgedDevice.h index d2c5a64b9ef4b7..ed5c9710624225 100644 --- a/examples/fabric-sync/bridge/include/BridgedDevice.h +++ b/examples/fabric-sync/bridge/include/BridgedDevice.h @@ -23,6 +23,8 @@ #include +namespace bridge { + class BridgedDevice { public: @@ -90,3 +92,5 @@ class BridgedDevice BridgedAttributes mAttributes; AdminCommissioningAttributes mAdminCommissioningAttributes; }; + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/include/BridgedDeviceBasicInformationImpl.h b/examples/fabric-sync/bridge/include/BridgedDeviceBasicInformationImpl.h index 23403438ab2be8..f57f7d0362ae73 100644 --- a/examples/fabric-sync/bridge/include/BridgedDeviceBasicInformationImpl.h +++ b/examples/fabric-sync/bridge/include/BridgedDeviceBasicInformationImpl.h @@ -18,6 +18,8 @@ #include #include +namespace bridge { + class BridgedDeviceBasicInformationImpl : public chip::app::AttributeAccessInterface { public: @@ -30,3 +32,5 @@ class BridgedDeviceBasicInformationImpl : public chip::app::AttributeAccessInter CHIP_ERROR Read(const chip::app::ConcreteReadAttributePath & path, chip::app::AttributeValueEncoder & encoder) override; CHIP_ERROR Write(const chip::app::ConcreteDataAttributePath & path, chip::app::AttributeValueDecoder & decoder) override; }; + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/include/BridgedDeviceManager.h b/examples/fabric-sync/bridge/include/BridgedDeviceManager.h index 127898fc5b2fab..6d172767662a69 100644 --- a/examples/fabric-sync/bridge/include/BridgedDeviceManager.h +++ b/examples/fabric-sync/bridge/include/BridgedDeviceManager.h @@ -24,6 +24,8 @@ #include +namespace bridge { + class BridgedDeviceManager { public: @@ -52,9 +54,9 @@ class BridgedDeviceManager * * @param dev A pointer to the device to be added. * @param parentEndpointId The parent endpoint ID. Defaults to an invalid endpoint ID. - * @return int The index of the dynamic endpoint if successful, nullopt otherwise + * @return uint16_t The index of the dynamic endpoint if successful, nullopt otherwise */ - std::optional AddDeviceEndpoint(std::unique_ptr dev, + std::optional AddDeviceEndpoint(std::unique_ptr dev, chip::EndpointId parentEndpointId = chip::kInvalidEndpointId); /** @@ -100,9 +102,9 @@ class BridgedDeviceManager * found, it removes the dynamic endpoint. * * @param scopedNodeId The ScopedNodeId of the device to be removed. - * @return unsigned of the index of the removed dynamic endpoint if successful, nullopt otherwise. + * @return uint16_t of the index of the removed dynamic endpoint if successful, nullopt otherwise. */ - std::optional RemoveDeviceByScopedNodeId(chip::ScopedNodeId scopedNodeId); + std::optional RemoveDeviceByScopedNodeId(chip::ScopedNodeId scopedNodeId); /** * Finds the device with the given unique id (if any) @@ -134,3 +136,5 @@ inline BridgedDeviceManager & BridgeDeviceMgr() { return BridgedDeviceManager::sInstance; } + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp b/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp index a0d87cbb3b81ef..3299df75a9039e 100644 --- a/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp +++ b/examples/fabric-sync/bridge/src/BridgedAdministratorCommissioning.cpp @@ -26,6 +26,8 @@ using namespace chip::app; using namespace chip::app::Clusters; using namespace chip::app::Clusters::AdministratorCommissioning; +namespace bridge { + CHIP_ERROR BridgedAdministratorCommissioning::Init() { // We expect initialization after emberAfInit(). This allows us to unregister the existing @@ -79,3 +81,5 @@ CHIP_ERROR BridgedAdministratorCommissioning::Read(const ConcreteReadAttributePa return CHIP_NO_ERROR; } + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/src/BridgedDevice.cpp b/examples/fabric-sync/bridge/src/BridgedDevice.cpp index f462d1cce6efd5..bd88c89145930e 100644 --- a/examples/fabric-sync/bridge/src/BridgedDevice.cpp +++ b/examples/fabric-sync/bridge/src/BridgedDevice.cpp @@ -27,6 +27,8 @@ using namespace chip; using namespace chip::app::Clusters::Actions; +namespace bridge { + BridgedDevice::BridgedDevice(ScopedNodeId scopedNodeId) { mReachable = false; @@ -116,3 +118,5 @@ void BridgedDevice::SetAdminCommissioningAttributes(const AdminCommissioningAttr } }); } + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp b/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp index 7fa48a4a158ddd..d021671c8bf1bd 100644 --- a/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp +++ b/examples/fabric-sync/bridge/src/BridgedDeviceBasicInformationImpl.cpp @@ -29,6 +29,8 @@ using namespace ::chip; using namespace ::chip::app; using namespace ::chip::app::Clusters; +namespace bridge { + CHIP_ERROR BridgedDeviceBasicInformationImpl::Read(const ConcreteReadAttributePath & path, AttributeValueEncoder & encoder) { // Registration is done for the bridged device basic information only @@ -105,3 +107,5 @@ CHIP_ERROR BridgedDeviceBasicInformationImpl::Write(const ConcreteDataAttributeP return CHIP_ERROR_INVALID_ARGUMENT; } + +} // namespace bridge diff --git a/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp b/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp index 0265f912027ca0..09c1b643c00efb 100644 --- a/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp +++ b/examples/fabric-sync/bridge/src/BridgedDeviceManager.cpp @@ -45,6 +45,8 @@ using namespace chip::Transport; using namespace chip::DeviceLayer; using namespace chip::app::Clusters; +namespace bridge { + namespace { constexpr uint8_t kMaxRetries = 10; @@ -184,7 +186,7 @@ void BridgedDeviceManager::Init() mCurrentEndpointId = mFirstDynamicEndpointId; } -std::optional BridgedDeviceManager::AddDeviceEndpoint(std::unique_ptr dev, +std::optional BridgedDeviceManager::AddDeviceEndpoint(std::unique_ptr dev, chip::EndpointId parentEndpointId) { EmberAfEndpointType * ep = dev->IsIcd() ? &sIcdBridgedNodeEndpoint : &sBridgedNodeEndpoint; @@ -194,6 +196,8 @@ std::optional BridgedDeviceManager::AddDeviceEndpoint(std::unique_ptr< // TODO: this shares data version among different clusters, which seems incorrect const chip::Span & dataVersionStorage = Span(sBridgedNodeDataVersions); + assertChipStackLockedByCurrentThread(); + if (dev->GetBridgedAttributes().uniqueId.empty()) { dev->SetUniqueId(GenerateUniqueId()); @@ -204,7 +208,7 @@ std::optional BridgedDeviceManager::AddDeviceEndpoint(std::unique_ptr< return std::nullopt; } - for (unsigned index = 0; index < CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; index++) + for (uint16_t index = 0; index < CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; index++) { if (mDevices[index]) { @@ -213,7 +217,6 @@ std::optional BridgedDeviceManager::AddDeviceEndpoint(std::unique_ptr< for (int retryCount = 0; retryCount < kMaxRetries; retryCount++) { - DeviceLayer::StackLock lock; dev->SetEndpointId(mCurrentEndpointId); dev->SetParentEndpointId(parentEndpointId); CHIP_ERROR err = @@ -327,9 +330,9 @@ BridgedDevice * BridgedDeviceManager::GetDeviceByScopedNodeId(chip::ScopedNodeId return nullptr; } -std::optional BridgedDeviceManager::RemoveDeviceByScopedNodeId(chip::ScopedNodeId scopedNodeId) +std::optional BridgedDeviceManager::RemoveDeviceByScopedNodeId(chip::ScopedNodeId scopedNodeId) { - for (unsigned index = 0; index < CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; ++index) + for (uint16_t index = 0; index < CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT; ++index) { if (mDevices[index] && mDevices[index]->GetScopedNodeId() == scopedNodeId) { @@ -343,3 +346,5 @@ std::optional BridgedDeviceManager::RemoveDeviceByScopedNodeId(chip::S } return std::nullopt; } + +} // namespace bridge diff --git a/examples/fabric-sync/main.cpp b/examples/fabric-sync/main.cpp index 45e0ce388ef0ff..3cd5ab8c20f1c9 100644 --- a/examples/fabric-sync/main.cpp +++ b/examples/fabric-sync/main.cpp @@ -99,7 +99,7 @@ int main(int argc, char * argv[]) Shell::RegisterCommands(); #endif - CHIP_ERROR err = PairingManager::Instance().Init(GetDeviceCommissioner()); + CHIP_ERROR err = admin::PairingManager::Instance().Init(GetDeviceCommissioner()); if (err != CHIP_NO_ERROR) { ChipLogProgress(NotSpecified, "Failed to init PairingManager: %s ", ErrorStr(err)); diff --git a/examples/fabric-sync/shell/AddBridgeCommand.cpp b/examples/fabric-sync/shell/AddBridgeCommand.cpp index d02ad9f58dba11..09246c11528e7e 100644 --- a/examples/fabric-sync/shell/AddBridgeCommand.cpp +++ b/examples/fabric-sync/shell/AddBridgeCommand.cpp @@ -48,11 +48,12 @@ void AddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) if (err == CHIP_NO_ERROR) { - DeviceMgr().SetRemoteBridgeNodeId(mBridgeNodeId); + admin::DeviceMgr().SetRemoteBridgeNodeId(mBridgeNodeId); + ChipLogProgress(NotSpecified, "Successfully paired bridge device: NodeId: " ChipLogFormatX64, ChipLogValueX64(mBridgeNodeId)); - DeviceMgr().UpdateLastUsedNodeId(mBridgeNodeId); + admin::DeviceMgr().UpdateLastUsedNodeId(mBridgeNodeId); } else { @@ -65,16 +66,16 @@ void AddBridgeCommand::OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) CHIP_ERROR AddBridgeCommand::RunCommand() { - if (DeviceMgr().IsFabricSyncReady()) + if (admin::DeviceMgr().IsFabricSyncReady()) { // print to console fprintf(stderr, "Remote Fabric Bridge has already been configured.\n"); - return CHIP_ERROR_BUSY; + return CHIP_NO_ERROR; } - PairingManager::Instance().SetCommissioningDelegate(this); + admin::PairingManager::Instance().SetCommissioningDelegate(this); - return DeviceMgr().PairRemoteFabricBridge(mBridgeNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); + return admin::DeviceMgr().PairRemoteFabricBridge(mBridgeNodeId, mSetupPINCode, mRemoteAddr, mRemotePort); } } // namespace commands diff --git a/examples/fabric-sync/shell/AddBridgeCommand.h b/examples/fabric-sync/shell/AddBridgeCommand.h index d4b2c16f1e21d9..48c1830260421e 100644 --- a/examples/fabric-sync/shell/AddBridgeCommand.h +++ b/examples/fabric-sync/shell/AddBridgeCommand.h @@ -23,7 +23,7 @@ namespace commands { -class AddBridgeCommand : public Command, public CommissioningDelegate +class AddBridgeCommand : public Command, public admin::CommissioningDelegate { public: AddBridgeCommand(chip::NodeId nodeId, uint32_t setupPINCode, const char * remoteAddr, uint16_t remotePort); diff --git a/examples/fabric-sync/shell/RemoveBridgeCommand.cpp b/examples/fabric-sync/shell/RemoveBridgeCommand.cpp index 9e8f1ed5fa8cd5..2eaaf6fc34476f 100644 --- a/examples/fabric-sync/shell/RemoveBridgeCommand.cpp +++ b/examples/fabric-sync/shell/RemoveBridgeCommand.cpp @@ -35,9 +35,10 @@ void RemoveBridgeCommand::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err) if (err == CHIP_NO_ERROR) { - DeviceMgr().SetRemoteBridgeNodeId(kUndefinedNodeId); - ChipLogProgress(NotSpecified, "Successfully removed bridge device: NodeId: " ChipLogFormatX64, - ChipLogValueX64(mBridgeNodeId)); + admin::DeviceMgr().SetRemoteBridgeNodeId(kUndefinedNodeId); + + // print to console + fprintf(stderr, "Successfully removed bridge device: NodeId: " ChipLogFormatX64 "\n", ChipLogValueX64(mBridgeNodeId)); } else { @@ -50,21 +51,20 @@ void RemoveBridgeCommand::OnDeviceRemoved(NodeId deviceId, CHIP_ERROR err) CHIP_ERROR RemoveBridgeCommand::RunCommand() { - NodeId bridgeNodeId = DeviceMgr().GetRemoteBridgeNodeId(); + NodeId bridgeNodeId = admin::DeviceMgr().GetRemoteBridgeNodeId(); if (bridgeNodeId == kUndefinedNodeId) { // print to console fprintf(stderr, "Remote Fabric Bridge is not configured yet, nothing to remove.\n"); - return CHIP_ERROR_BUSY; + return CHIP_NO_ERROR; } mBridgeNodeId = bridgeNodeId; - PairingManager::Instance().SetPairingDelegate(this); - DeviceMgr().UnpairRemoteFabricBridge(); + admin::PairingManager::Instance().SetPairingDelegate(this); - return CHIP_NO_ERROR; + return admin::DeviceMgr().UnpairRemoteFabricBridge(); } } // namespace commands diff --git a/examples/fabric-sync/shell/RemoveBridgeCommand.h b/examples/fabric-sync/shell/RemoveBridgeCommand.h index 401a5390be8fd9..b0e4b33d52ce8b 100644 --- a/examples/fabric-sync/shell/RemoveBridgeCommand.h +++ b/examples/fabric-sync/shell/RemoveBridgeCommand.h @@ -23,7 +23,7 @@ namespace commands { -class RemoveBridgeCommand : public Command, public PairingDelegate +class RemoveBridgeCommand : public Command, public admin::PairingDelegate { public: void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) override;