Skip to content

Commit 56deee7

Browse files
authored
[WebRTC] WebRTCSessionID should start from 0 (project-chip#40494)
* [WebRTC] WebRTCSessionID should start from 0 * Address review comment * Address review comments
1 parent 1a50d44 commit 56deee7

File tree

7 files changed

+36
-19
lines changed

7 files changed

+36
-19
lines changed

src/app/clusters/webrtc-transport-provider-server/webrtc-transport-provider-server.cpp

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -224,25 +224,33 @@ WebRTCSessionStruct * WebRTCTransportProviderServer::CheckForMatchingSession(Han
224224
return session;
225225
}
226226

227-
uint16_t WebRTCTransportProviderServer::GenerateSessionId()
227+
CHIP_ERROR WebRTCTransportProviderServer::GenerateSessionId(uint16_t & outSessionId)
228228
{
229-
static uint16_t lastSessionId = 1;
229+
static uint16_t nextSessionId = 0;
230+
uint16_t candidateId = 0;
230231

231-
do
232+
// Try at most kMaxSessionId+1 attempts to find a free ID
233+
// This ensures we never loop infinitely even if all IDs are somehow in use
234+
for (uint16_t attempts = 0; attempts <= kMaxSessionId; attempts++)
232235
{
233-
uint16_t candidateId = lastSessionId++;
236+
candidateId = nextSessionId++;
234237

235238
// Handle wrap-around per spec
236-
if (lastSessionId > kMaxSessionId)
239+
if (nextSessionId > kMaxSessionId)
237240
{
238-
lastSessionId = 1;
241+
nextSessionId = 0;
239242
}
240243

241244
if (FindSession(candidateId) == nullptr)
242245
{
243-
return candidateId;
246+
outSessionId = candidateId;
247+
return CHIP_NO_ERROR;
244248
}
245-
} while (true);
249+
}
250+
251+
// All session IDs are in use
252+
ChipLogError(Zcl, "All session IDs are in use! Cannot generate new session ID.");
253+
return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
246254
}
247255

248256
// Command Handlers
@@ -337,7 +345,15 @@ void WebRTCTransportProviderServer::HandleSolicitOffer(HandlerContext & ctx, con
337345

338346
// Prepare the arguments for the delegate.
339347
Delegate::OfferRequestArgs args;
340-
args.sessionId = GenerateSessionId();
348+
uint16_t sessionId;
349+
CHIP_ERROR err = GenerateSessionId(sessionId);
350+
if (err != CHIP_NO_ERROR)
351+
{
352+
ChipLogError(Zcl, "HandleSolicitOffer: Cannot generate session ID: %" CHIP_ERROR_FORMAT, err.Format());
353+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::ClusterStatusCode(err));
354+
return;
355+
}
356+
args.sessionId = sessionId;
341357
args.streamUsage = req.streamUsage;
342358
args.videoStreamId = req.videoStreamID;
343359
args.audioStreamId = req.audioStreamID;
@@ -546,8 +562,16 @@ void WebRTCTransportProviderServer::HandleProvideOffer(HandlerContext & ctx, con
546562
return;
547563
}
548564

549-
// Generate new sessiond id
550-
args.sessionId = GenerateSessionId();
565+
// Generate new session id
566+
uint16_t sessionId;
567+
err = GenerateSessionId(sessionId);
568+
if (err != CHIP_NO_ERROR)
569+
{
570+
ChipLogError(Zcl, "HandleProvideOffer: Cannot generate session ID: %" CHIP_ERROR_FORMAT, err.Format());
571+
ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Protocols::InteractionModel::ClusterStatusCode(err));
572+
return;
573+
}
574+
args.sessionId = sessionId;
551575
}
552576

553577
args.streamUsage = req.streamUsage;

src/app/clusters/webrtc-transport-provider-server/webrtc-transport-provider-server.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ class WebRTCTransportProviderServer : public AttributeAccessInterface, public Co
333333
WebRTCSessionStruct * CheckForMatchingSession(HandlerContext & ctx, uint16_t sessionId);
334334
UpsertResultEnum UpsertSession(const WebRTCSessionStruct & session);
335335
void RemoveSession(uint16_t sessionId);
336-
uint16_t GenerateSessionId();
336+
CHIP_ERROR GenerateSessionId(uint16_t & outSessionId);
337337

338338
// Command Handlers
339339
void HandleSolicitOffer(HandlerContext & ctx, const Commands::SolicitOffer::DecodableType & req);

src/python_testing/TC_WEBRTCP_2_1.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ async def test_TC_WebRTCProvider_2_1(self):
162162
resp = await self.send_single_cmd(cmd=cmd, endpoint=endpoint, payloadCapability=ChipDeviceCtrl.TransportPayloadCapability.LARGE_PAYLOAD)
163163
asserts.assert_equal(type(resp), Clusters.WebRTCTransportProvider.Commands.SolicitOfferResponse,
164164
"Incorrect response type")
165-
asserts.assert_not_equal(resp.webRTCSessionID, 0, "webrtcSessionID in SolicitOfferResponse should not be 0.")
166165
asserts.assert_true(resp.deferredOffer, "Expected 'deferredOffer' to be True.")
167166

168167
self.step(7)

src/python_testing/TC_WEBRTCP_2_2.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ async def test_TC_WebRTCProvider_2_2(self):
108108
resp = await self.send_single_cmd(cmd=cmd, endpoint=endpoint, payloadCapability=ChipDeviceCtrl.TransportPayloadCapability.LARGE_PAYLOAD)
109109
asserts.assert_equal(type(resp), Clusters.WebRTCTransportProvider.Commands.SolicitOfferResponse,
110110
"Incorrect response type")
111-
asserts.assert_not_equal(resp.webRTCSessionID, 0, "webrtcSessionID in SolicitOfferResponse should not be 0.")
112111
asserts.assert_false(resp.deferredOffer, "Expected 'deferredOffer' to be False.")
113112

114113
# TODO: Enable this check after integrating with Camera AvStreamManager

src/python_testing/TC_WEBRTCP_2_3.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,6 @@ async def test_TC_WebRTCProvider_2_3(self):
213213
resp = await self.send_single_cmd(cmd=cmd, endpoint=endpoint, payloadCapability=ChipDeviceCtrl.TransportPayloadCapability.LARGE_PAYLOAD)
214214
asserts.assert_equal(type(resp), Clusters.WebRTCTransportProvider.Commands.ProvideOfferResponse,
215215
"Incorrect response type")
216-
asserts.assert_not_equal(resp.webRTCSessionID, 0, "webRTCSessionID in ProvideOfferResponse should not be 0.")
217216

218217
self.step(9)
219218
# Verify CurrentSessions contains the new session

src/python_testing/TC_WEBRTCP_2_4.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,6 @@ async def test_TC_WebRTCProvider_2_4(self):
158158
saved_session_id = resp.webRTCSessionID
159159
asserts.assert_equal(videoStreamID, resp.videoStreamID, "Video stream ID does not match that in the command")
160160
asserts.assert_equal(audioStreamID, resp.audioStreamID, "Audio stream ID does not match that in the command")
161-
asserts.assert_not_equal(
162-
saved_session_id, 0, "Allocated WebRTCSessionID must be non‑zero"
163-
)
164161

165162
self.step(5)
166163
current_sessions = await self.read_single_attribute_check_success(

src/python_testing/TC_WEBRTCP_2_5.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ async def test_TC_WebRTCProvider_2_5(self):
146146
resp = await self.send_single_cmd(cmd=cmd, endpoint=endpoint, payloadCapability=ChipDeviceCtrl.TransportPayloadCapability.LARGE_PAYLOAD)
147147
asserts.assert_equal(type(resp), Clusters.WebRTCTransportProvider.Commands.SolicitOfferResponse,
148148
"Incorrect response type")
149-
asserts.assert_not_equal(resp.webRTCSessionID, 0, "webrtcSessionID in SolicitOfferResponse should not be 0.")
150149
asserts.assert_false(resp.deferredOffer, "Expected 'deferredOffer' to be False.")
151150

152151
# TODO: Enable this check after integrating with Camera AvStreamManager

0 commit comments

Comments
 (0)