Skip to content

Commit 31d316a

Browse files
yufengwangcayufengwangca
committed
Address review comments
1 parent cf3a532 commit 31d316a

File tree

6 files changed

+32
-32
lines changed

6 files changed

+32
-32
lines changed

‎examples/fabric-admin/commands/clusters/ReportCommand.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ void ReportCommand::OnAttributeData(const app::ConcreteDataAttributePath & path,
4646

4747
LogErrorOnFailure(RemoteDataModelLogger::LogAttributeAsJSON(path, data));
4848

49-
DeviceMgr().HanldeAttributeChange(path, data);
49+
DeviceMgr().HandleAttributeChange(path, data);
5050
}
5151

5252
void ReportCommand::OnEventData(const app::EventHeader & eventHeader, TLV::TLVReader * data, const app::StatusIB * status)

‎examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp

+8-6
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ constexpr uint32_t kCommissionPrepareTimeMs = 500;
3737
constexpr uint16_t kMaxManaulCodeLength = 21;
3838
constexpr uint16_t kSubscribeMinInterval = 0;
3939
constexpr uint16_t kSubscribeMaxInterval = 60;
40+
constexpr uint16_t kRemoteBridgePort = 5540;
4041

4142
} // namespace
4243

@@ -80,14 +81,15 @@ CHIP_ERROR FabricSyncAddBridgeCommand::RunCommand(NodeId remoteId)
8081
}
8182

8283
char command[kMaxCommandSize];
83-
snprintf(command, sizeof(command), "pairing onnetwork %ld %d", remoteId, kSetupPinCode);
84+
snprintf(command, sizeof(command), "pairing already-discovered %ld %d %s %d", remoteId, kSetupPinCode,
85+
reinterpret_cast<const char *>(mRemoteAddr.data()), kRemoteBridgePort);
8486

85-
PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "onnetwork"));
87+
PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "already-discovered"));
8688

8789
if (pairingCommand == nullptr)
8890
{
8991
ChipLogError(NotSpecified, "Pairing onnetwork command is not available");
90-
return CHIP_ERROR_UNINITIALIZED;
92+
return CHIP_ERROR_NOT_IMPLEMENTED;
9193
}
9294

9395
pairingCommand->RegisterCommissioningDelegate(this);
@@ -128,7 +130,7 @@ CHIP_ERROR FabricSyncRemoveBridgeCommand::RunCommand()
128130
if (bridgeNodeId == kUndefinedNodeId)
129131
{
130132
// print to console
131-
fprintf(stderr, "Remote Fabric Bridge is not configured yet.");
133+
fprintf(stderr, "Remote Fabric Bridge is not configured yet, nothing to remove.");
132134
return CHIP_NO_ERROR;
133135
}
134136

@@ -142,7 +144,7 @@ CHIP_ERROR FabricSyncRemoveBridgeCommand::RunCommand()
142144
if (pairingCommand == nullptr)
143145
{
144146
ChipLogError(NotSpecified, "Pairing code command is not available");
145-
return CHIP_ERROR_UNINITIALIZED;
147+
return CHIP_ERROR_NOT_IMPLEMENTED;
146148
}
147149

148150
pairingCommand->RegisterPairingDelegate(this);
@@ -231,7 +233,7 @@ CHIP_ERROR FabricSyncDeviceCommand::RunCommand(EndpointId remoteId)
231233

232234
if (openCommand == nullptr)
233235
{
234-
return CHIP_ERROR_UNINITIALIZED;
236+
return CHIP_ERROR_NOT_IMPLEMENTED;
235237
}
236238

237239
openCommand->RegisterDelegate(this);

‎examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h

+2
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDeleg
3636
FabricSyncAddBridgeCommand(CredentialIssuerCommands * credIssuerCommands) : CHIPCommand("add-bridge", credIssuerCommands)
3737
{
3838
AddArgument("nodeid", 0, UINT64_MAX, &mNodeId);
39+
AddArgument("device-remote-ip", &mRemoteAddr);
3940
}
4041

4142
void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) override;
@@ -48,6 +49,7 @@ class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDeleg
4849
private:
4950
chip::NodeId mNodeId;
5051
chip::NodeId mBridgeNodeId;
52+
chip::ByteSpan mRemoteAddr;
5153

5254
CHIP_ERROR RunCommand(NodeId remoteId);
5355
};

‎examples/fabric-admin/commands/pairing/PairingCommand.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -538,9 +538,7 @@ void PairingCommand::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
538538
// print to console
539539
fprintf(stderr, "Device with Node ID: 0x%lx has been successfully removed.\n", nodeId);
540540

541-
#if defined(PW_RPC_ENABLED)
542-
RemoveSynchronizedDevice(nodeId);
543-
#endif
541+
// TODO: (#33973) Add RPC method RemoveSynchronizedDevice
544542
}
545543
else
546544
{

‎examples/fabric-admin/device_manager/DeviceManager.cpp

+14-21
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,12 @@
2626
using namespace chip;
2727
using namespace chip::app::Clusters;
2828

29-
namespace {
30-
31-
// Constants
32-
constexpr uint32_t kSyncPrepareTimeMs = 1000;
33-
34-
} // namespace
35-
3629
// Define the static member
3730
DeviceManager DeviceManager::sInstance;
3831

3932
void DeviceManager::Init()
4033
{
41-
// TODO: Init mLastUsedNodeId from chip config file
34+
// TODO: (#34113) Init mLastUsedNodeId from chip config file
4235
mLastUsedNodeId = 1;
4336
}
4437

@@ -93,19 +86,18 @@ Device * DeviceManager::FindDeviceByNode(NodeId nodeId)
9386
void DeviceManager::RemoveSyncedDevice(NodeId nodeId)
9487
{
9588
Device * device = FindDeviceByNode(nodeId);
96-
if (device != nullptr)
97-
{
98-
mSyncedDevices.erase(*device);
99-
ChipLogProgress(NotSpecified, "Removed synced device: NodeId:" ChipLogFormatX64 ", EndpointId %u",
100-
ChipLogValueX64(device->GetNodeId()), device->GetEndpointId());
101-
}
102-
else
89+
if (device == nullptr)
10390
{
10491
ChipLogProgress(NotSpecified, "No device found with NodeId:" ChipLogFormatX64, ChipLogValueX64(nodeId));
92+
return;
10593
}
94+
95+
mSyncedDevices.erase(*device);
96+
ChipLogProgress(NotSpecified, "Removed synced device: NodeId:" ChipLogFormatX64 ", EndpointId %u",
97+
ChipLogValueX64(device->GetNodeId()), device->GetEndpointId());
10698
}
10799

108-
void DeviceManager::HanldeAttributeChange(const app::ConcreteDataAttributePath & path, TLV::TLVReader * data)
100+
void DeviceManager::HandleAttributeChange(const app::ConcreteDataAttributePath & path, TLV::TLVReader * data)
109101
{
110102
if (path.mClusterId != Descriptor::Id || path.mAttributeId != Descriptor::Attributes::PartsList::Id)
111103
{
@@ -140,10 +132,15 @@ void DeviceManager::HanldeAttributeChange(const app::ConcreteDataAttributePath &
140132
return;
141133
}
142134

143-
// Compare newEndpoints with mSyncedEndpoints to determine added and removed endpoints
135+
// Compare newEndpoints with mSyncedDevices to determine added and removed endpoints
144136
std::vector<EndpointId> addedEndpoints;
145137
std::vector<EndpointId> removedEndpoints;
146138

139+
// Note: We're using vectors and manual searches instead of set operations
140+
// because we need to work with the Device objects in mSyncedDevices,
141+
// not just their EndpointIds. This approach allows us to access the full
142+
// Device information when processing changes.
143+
147144
// Find added endpoints
148145
for (const auto & endpoint : newEndpoints)
149146
{
@@ -172,8 +169,6 @@ void DeviceManager::HanldeAttributeChange(const app::ConcreteDataAttributePath &
172169
{
173170
char command[64];
174171
snprintf(command, sizeof(command), "fabricsync sync-device %d", endpoint);
175-
176-
usleep(kSyncPrepareTimeMs * 1000);
177172
PushCommand(command);
178173
}
179174
}
@@ -205,8 +200,6 @@ void DeviceManager::HanldeAttributeChange(const app::ConcreteDataAttributePath &
205200
}
206201

207202
pairingCommand->RegisterPairingDelegate(this);
208-
209-
usleep(kSyncPrepareTimeMs * 1000);
210203
PushCommand(command);
211204
}
212205
}

‎examples/fabric-admin/device_manager/DeviceManager.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class DeviceManager : public PairingDelegate
6767

6868
void RemoveSyncedDevice(chip::NodeId nodeId);
6969

70-
void HanldeAttributeChange(const chip::app::ConcreteDataAttributePath & path, chip::TLV::TLVReader * data);
70+
void HandleAttributeChange(const chip::app::ConcreteDataAttributePath & path, chip::TLV::TLVReader * data);
7171

7272
void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) override;
7373

@@ -80,6 +80,7 @@ class DeviceManager : public PairingDelegate
8080
chip::NodeId mRemoteBridgeNodeId = chip::kUndefinedNodeId;
8181
std::set<Device> mSyncedDevices;
8282
bool mAutoSyncEnabled = false;
83+
bool mInitialized = false;
8384

8485
Device * FindDeviceByEndpoint(chip::EndpointId endpointId);
8586
Device * FindDeviceByNode(chip::NodeId nodeId);
@@ -93,5 +94,9 @@ class DeviceManager : public PairingDelegate
9394
*/
9495
inline DeviceManager & DeviceMgr()
9596
{
97+
if (!DeviceManager::sInstance.mInitialized)
98+
{
99+
DeviceManager::sInstance.Init();
100+
}
96101
return DeviceManager::sInstance;
97102
}

0 commit comments

Comments
 (0)