Skip to content

Commit 39402ec

Browse files
tehampsonrestyled-commits
authored andcommitted
Add expiry to KeepActive as per spec into fabric-admin example (project-chip#35099)
* Add expiry to KeepActive as per spec into fabric-admin example * Restyled by clang-format * Self Review fix * Restyled by clang-format * Name fix * Address PR comments * Address PR comments * Restyled by clang-format * Address PR comments --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 929c08e commit 39402ec

File tree

1 file changed

+40
-12
lines changed

1 file changed

+40
-12
lines changed

examples/fabric-admin/rpc/RpcServer.cpp

+40-12
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,32 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
4747
public:
4848
void OnCheckInCompleted(const chip::app::ICDClientInfo & clientInfo) override
4949
{
50-
chip::NodeId nodeId = clientInfo.peer_node.GetNodeId();
51-
auto it = mPendingKeepActiveTimesMs.find(nodeId);
52-
VerifyOrReturn(it != mPendingKeepActiveTimesMs.end());
53-
// TODO(#33221): We also need a mechanism here to drop KeepActive
54-
// request if they were recieved over 60 mins ago.
55-
uint32_t stayActiveDurationMs = it->second;
50+
// Needs for accessing mPendingCheckIn
51+
assertChipStackLockedByCurrentThread();
52+
NodeId nodeId = clientInfo.peer_node.GetNodeId();
53+
auto it = mPendingCheckIn.find(nodeId);
54+
VerifyOrReturn(it != mPendingCheckIn.end());
55+
56+
KeepActiveDataForCheckIn checkInData = it->second;
57+
// Removed from pending map as check-in from this node has occured and we will handle the pending KeepActive
58+
// request.
59+
mPendingCheckIn.erase(nodeId);
60+
61+
auto timeNow = System::SystemClock().GetMonotonicTimestamp();
62+
if (timeNow > checkInData.mRequestExpiryTimestamp)
63+
{
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);
68+
return;
69+
}
5670

5771
// TODO(#33221): If there is a failure in sending the message this request just gets dropped.
5872
// Work to see if there should be update to spec on whether some sort of failure later on
5973
// Should be indicated in some manner, or identify a better recovery mechanism here.
60-
mPendingKeepActiveTimesMs.erase(nodeId);
61-
6274
auto onDone = [=](uint32_t promisedActiveDuration) { ActiveChanged(nodeId, promisedActiveDuration); };
63-
CHIP_ERROR err = StayActiveSender::SendStayActiveCommand(stayActiveDurationMs, clientInfo.peer_node,
75+
CHIP_ERROR err = StayActiveSender::SendStayActiveCommand(checkInData.mStayActiveDurationMs, clientInfo.peer_node,
6476
chip::app::InteractionModelEngine::GetInstance(), onDone);
6577
if (err != CHIP_NO_ERROR)
6678
{
@@ -147,10 +159,24 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
147159

148160
void ScheduleSendingKeepActiveOnCheckIn(chip::NodeId nodeId, uint32_t stayActiveDurationMs)
149161
{
150-
mPendingKeepActiveTimesMs[nodeId] = stayActiveDurationMs;
162+
// Needs for accessing mPendingCheckIn
163+
assertChipStackLockedByCurrentThread();
164+
165+
auto timeNow = System::SystemClock().GetMonotonicTimestamp();
166+
// Spec says we should expire the request 60 mins after we get it
167+
System::Clock::Timestamp expiryTimestamp = timeNow + System::Clock::Seconds64(60 * 60);
168+
KeepActiveDataForCheckIn checkInData = { .mStayActiveDurationMs = stayActiveDurationMs,
169+
.mRequestExpiryTimestamp = expiryTimestamp };
170+
mPendingCheckIn[nodeId] = checkInData;
151171
}
152172

153173
private:
174+
struct KeepActiveDataForCheckIn
175+
{
176+
uint32_t mStayActiveDurationMs = 0;
177+
System::Clock::Timestamp mRequestExpiryTimestamp;
178+
};
179+
154180
struct KeepActiveWorkData
155181
{
156182
KeepActiveWorkData(FabricAdmin * fabricAdmin, chip::NodeId nodeId, uint32_t stayActiveDurationMs) :
@@ -169,8 +195,10 @@ class FabricAdmin final : public rpc::FabricAdmin, public IcdManager::Delegate
169195
chip::Platform::Delete(data);
170196
}
171197

172-
// Modifications to mPendingKeepActiveTimesMs should be done on the MatterEventLoop thread
173-
std::map<chip::NodeId, uint32_t> mPendingKeepActiveTimesMs;
198+
// Modifications to mPendingCheckIn should be done on the MatterEventLoop thread
199+
// otherwise we would need a mutex protecting this data to prevent race as this
200+
// data is accessible by both RPC thread and Matter eventloop.
201+
std::unordered_map<NodeId, KeepActiveDataForCheckIn> mPendingCheckIn;
174202
};
175203

176204
FabricAdmin fabric_admin_service;

0 commit comments

Comments
 (0)