fix(camera): add Barton session metadata - #264
Open
kfundecmcsa wants to merge 1 commit into
Open
Conversation
- Attach bartonSessionId metadata to remoteSdp and remoteIceCandidates so the client can correlate incoming WebRTC updates to the correct Barton session. Previously, when incoming WebRTC updates arrived, we inferred the session from the active streaming state. That worked when only one client was active, but it became ambiguous once multiple clients could be streaming at the same time. - To resolve that ambiguity, use the camera's webRTCSessionID to infer the Barton session directly when it is available.
kfundecmcsa
requested review from
cleithner-comcast,
mkkoch,
rchowdcmcsa and
tleacmcsa
as code owners
August 12, 2026 18:36
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the SBMD camera WebRTC driver logic to attach session-correlation metadata to incoming WebRTC signaling updates, so clients can attribute remote SDP / ICE / end events to the correct Barton session when multiple sessions may be active.
Changes:
- Add metadata assertions to WebRTC unit tests for
remoteSdp,remoteIceCandidates, andwebrtcErrorupdates (including “unknown” session cases). - Introduce
findSessionIdByWebRTCSessionID()and use it to resolve the Barton session from camera-providedwebRTCSessionIDfor incoming ICE candidates and end-session events. - Add defensive handling for null/undefined session maps in
findStreamingSessionId().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| core/test/src/SbmdCameraWebrtcTest.cpp | Extends tests to validate that incoming WebRTC updates emit session metadata (including unknown-session fallbacks). |
| core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js | Adds WebRTCSessionID→sessionId lookup and attaches session metadata to WebRTC resource updates to disambiguate multi-session scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
963
to
967
| var sessionsJson = args.supplements.transientData[TD_SESSIONS]; | ||
| var sessions = parseSessions(sessionsJson); | ||
| var sessionId = findStreamingSessionId(sessions); | ||
| var metadata = {sessionId: sessionId || 'unknown'}; | ||
|
|
Comment on lines
1004
to
+1007
| var sessionsJson = args.supplements.transientData[TD_SESSIONS]; | ||
| var sessions = parseSessions(sessionsJson); | ||
| var sessionId = findStreamingSessionId(sessions); | ||
| var metadata = {sessionId: sessionId || 'unknown'}; |
| var sessions = parseSessions(sessionsJson); | ||
| var sessionId = findSessionIdByWebRTCSessionID(sessions, webRTCSessionID); | ||
|
|
||
| var metadata = {sessionId: sessionId || 'unknown'}; |
| EXPECT_TRUE(ur->metadata->find("reason") != std::string::npos); | ||
| } | ||
|
|
||
| TEST_F(SbmdCameraWebrtcTest, HandleIncomingEndWithUnknownWebRTCSessionIDReturnsError) |
Comment on lines
338
to
+342
| function findStreamingSessionId(sessions) { | ||
| if (sessions === null || sessions === undefined) | ||
| { | ||
| return null; | ||
| } |
cleithner-comcast
approved these changes
Aug 12, 2026
cleithner-comcast
left a comment
cleithner-comcast
left a comment
Contributor
There was a problem hiding this comment.
Any needed changes in the reference app? I suppose that you'd never have multiple clients with the reference app in the current increment so likely this metadata can just be ignored.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, when incoming WebRTC updates arrived, we inferred the session from the active streaming state. That worked when only one client was active, but it became ambiguous once multiple clients could be streaming at the same time.