@@ -42,29 +42,40 @@ namespace {
4242
4343#if defined(PW_RPC_FABRIC_ADMIN_SERVICE) && PW_RPC_FABRIC_ADMIN_SERVICE
4444
45+ struct ScopedNodeIdHasher
46+ {
47+ std::size_t operator ()(const chip::ScopedNodeId & scopedNodeId) const
48+ {
49+ std::size_t h1 = std::hash<uint64_t >{}(scopedNodeId.GetFabricIndex ());
50+ std::size_t h2 = std::hash<uint64_t >{}(scopedNodeId.GetNodeId ());
51+ // Bitshifting h2 reduces collisions when fabricIndex == nodeId.
52+ return h1 ^ (h2 << 1 );
53+ }
54+ };
55+
4556class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
4657{
4758public:
4859 void OnCheckInCompleted (const app::ICDClientInfo & clientInfo) override
4960 {
5061 // Accessing mPendingCheckIn should only be done while holding ChipStackLock
5162 assertChipStackLockedByCurrentThread ();
52- NodeId nodeId = clientInfo.peer_node . GetNodeId () ;
53- auto it = mPendingCheckIn .find (nodeId );
63+ ScopedNodeId scopedNodeId = clientInfo.peer_node ;
64+ auto it = mPendingCheckIn .find (scopedNodeId );
5465 VerifyOrReturn (it != mPendingCheckIn .end ());
5566
5667 KeepActiveDataForCheckIn checkInData = it->second ;
5768 // Removed from pending map as check-in from this node has occured and we will handle the pending KeepActive
5869 // request.
59- mPendingCheckIn .erase (nodeId );
70+ mPendingCheckIn .erase (scopedNodeId );
6071
6172 auto timeNow = System::SystemClock ().GetMonotonicTimestamp ();
6273 if (timeNow > checkInData.mRequestExpiryTimestamp )
6374 {
64- ChipLogError (
65- NotSpecified,
66- " ICD check-in for device we have been waiting, came after KeepActive expiry. Reqeust dropped for Node ID: 0x%lx "
67- nodeId );
75+ ChipLogError (NotSpecified,
76+ " ICD check-in for device we have been waiting, came after KeepActive expiry. Request dropped for ID: "
77+ " [%d:0x " ChipLogFormatX64 " ] "
78+ scopedNodeId. GetFabricIndex (), ChipLogValueX64 (scopedNodeId. GetNodeId ()) );
6879 return ;
6980 }
7081
@@ -74,7 +85,7 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
7485 // there is no mechanism for us to communicate with the client that sent out the KeepActive
7586 // command that there was a failure, we simply fail silently. After spec issue is
7687 // addressed, we can implement what spec defines here.
77- auto onDone = [=](uint32_t promisedActiveDuration) { ActiveChanged (nodeId , promisedActiveDuration); };
88+ auto onDone = [=](uint32_t promisedActiveDuration) { ActiveChanged (scopedNodeId , promisedActiveDuration); };
7889 CHIP_ERROR err = StayActiveSender::SendStayActiveCommand (checkInData.mStayActiveDurationMs , clientInfo.peer_node ,
7990 app::InteractionModelEngine::GetInstance (), onDone);
8091 if (err != CHIP_NO_ERROR)
@@ -86,7 +97,10 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
8697 pw::Status OpenCommissioningWindow (const chip_rpc_DeviceCommissioningWindowInfo & request,
8798 chip_rpc_OperationStatus & response) override
8899 {
89- NodeId nodeId = request.node_id ;
100+ VerifyOrReturnValue (request.has_id , pw::Status::InvalidArgument ());
101+ // TODO(#35875): OpenDeviceCommissioningWindow uses the same controller every time and doesn't currently accept
102+ // FabricIndex. For now we are dropping fabric index from the scoped node id.
103+ NodeId nodeId = request.id .node_id ;
90104 uint32_t commissioningTimeoutSec = request.commissioning_timeout ;
91105 uint32_t iterations = request.iterations ;
92106 uint16_t discriminator = request.discriminator ;
@@ -149,18 +163,19 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
149163
150164 pw::Status KeepActive (const chip_rpc_KeepActiveParameters & request, pw_protobuf_Empty & response) override
151165 {
152- ChipLogProgress (NotSpecified, " Received KeepActive request: 0x%lx, %u" node_id , request.stay_active_duration_ms );
153- // TODO(#33221): We should really be using ScopedNode, but that requires larger fix in communication between
154- // fabric-admin and fabric-bridge. For now we make the assumption that there is only one fabric used by
155- // fabric-admin.
166+ VerifyOrReturnValue (request.has_id , pw::Status::InvalidArgument ());
167+ ScopedNodeId scopedNodeId (request.id .node_id , request.id .fabric_index );
168+ ChipLogProgress (NotSpecified, " Received KeepActive request: Id[%d, 0x" " ], %u"
169+ scopedNodeId.GetFabricIndex (), ChipLogValueX64 (scopedNodeId.GetNodeId ()), request.stay_active_duration_ms );
170+
156171 KeepActiveWorkData * data =
157- Platform::New<KeepActiveWorkData>(this , request. node_id , request.stay_active_duration_ms , request.timeout_ms );
172+ Platform::New<KeepActiveWorkData>(this , scopedNodeId , request.stay_active_duration_ms , request.timeout_ms );
158173 VerifyOrReturnValue (data, pw::Status::Internal ());
159174 DeviceLayer::PlatformMgr ().ScheduleWork (KeepActiveWork, reinterpret_cast <intptr_t >(data));
160175 return pw::OkStatus ();
161176 }
162177
163- void ScheduleSendingKeepActiveOnCheckIn (NodeId nodeId , uint32_t stayActiveDurationMs, uint32_t timeoutMs)
178+ void ScheduleSendingKeepActiveOnCheckIn (ScopedNodeId scopedNodeId , uint32_t stayActiveDurationMs, uint32_t timeoutMs)
164179 {
165180 // Accessing mPendingCheckIn should only be done while holding ChipStackLock
166181 assertChipStackLockedByCurrentThread ();
@@ -170,14 +185,14 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
170185 KeepActiveDataForCheckIn checkInData = { .mStayActiveDurationMs = stayActiveDurationMs,
171186 .mRequestExpiryTimestamp = expiryTimestamp };
172187
173- auto it = mPendingCheckIn .find (nodeId );
188+ auto it = mPendingCheckIn .find (scopedNodeId );
174189 if (it != mPendingCheckIn .end ())
175190 {
176191 checkInData.mStayActiveDurationMs = std::max (checkInData.mStayActiveDurationMs , it->second .mStayActiveDurationMs );
177192 checkInData.mRequestExpiryTimestamp = std::max (checkInData.mRequestExpiryTimestamp , it->second .mRequestExpiryTimestamp );
178193 }
179194
180- mPendingCheckIn [nodeId ] = checkInData;
195+ mPendingCheckIn [scopedNodeId ] = checkInData;
181196 }
182197
183198private:
@@ -189,27 +204,29 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
189204
190205 struct KeepActiveWorkData
191206 {
192- KeepActiveWorkData (FabricAdmin * fabricAdmin, NodeId nodeId, uint32_t stayActiveDurationMs, uint32_t timeoutMs) :
193- mFabricAdmin (fabricAdmin), mNodeId (nodeId), mStayActiveDurationMs (stayActiveDurationMs), mTimeoutMs (timeoutMs)
207+ KeepActiveWorkData (FabricAdmin * fabricAdmin, ScopedNodeId scopedNodeId, uint32_t stayActiveDurationMs,
208+ uint32_t timeoutMs) :
209+ mFabricAdmin (fabricAdmin),
210+ mScopedNodeId (scopedNodeId), mStayActiveDurationMs (stayActiveDurationMs), mTimeoutMs (timeoutMs)
194211 {}
195212
196213 FabricAdmin * mFabricAdmin ;
197- NodeId mNodeId ;
214+ ScopedNodeId mScopedNodeId ;
198215 uint32_t mStayActiveDurationMs ;
199216 uint32_t mTimeoutMs ;
200217 };
201218
202219 static void KeepActiveWork (intptr_t arg)
203220 {
204221 KeepActiveWorkData * data = reinterpret_cast <KeepActiveWorkData *>(arg);
205- data->mFabricAdmin ->ScheduleSendingKeepActiveOnCheckIn (data->mNodeId , data->mStayActiveDurationMs , data->mTimeoutMs );
222+ data->mFabricAdmin ->ScheduleSendingKeepActiveOnCheckIn (data->mScopedNodeId , data->mStayActiveDurationMs , data->mTimeoutMs );
206223 Platform::Delete (data);
207224 }
208225
209226 // Modifications to mPendingCheckIn should be done on the MatterEventLoop thread
210227 // otherwise we would need a mutex protecting this data to prevent race as this
211228 // data is accessible by both RPC thread and Matter eventloop.
212- std::unordered_map<NodeId , KeepActiveDataForCheckIn> mPendingCheckIn ;
229+ std::unordered_map<ScopedNodeId , KeepActiveDataForCheckIn, ScopedNodeIdHasher > mPendingCheckIn ;
213230};
214231
215232FabricAdmin fabric_admin_service;
0 commit comments