Draft: proper floating up of version#1081
Conversation
ChangesetThe following package versions will be affected by this PR:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the signal client’s v1 join-request generation to report the correct SDK identity (and version) in ClientInfo, rather than always reporting Rust, which improves cross-language wrapper attribution while keeping the existing v0 query-param behavior intact.
Changes:
- Added
client_info_sdk_for_name()to map SDK name strings (e.g.,"cpp","C++","react-native") intoproto::client_info::Sdk. - Updated v1
JoinRequestconstruction to setclient_info.sdkbased onSignalOptions.sdk_options.sdk. - Added unit tests covering SDK-name mapping and verifying SDK/version reporting for both v0 URL params and v1 join_request payload.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "node" => proto::client_info::Sdk::Node, | ||
| "unreal" => proto::client_info::Sdk::Unreal, | ||
| "esp32" => proto::client_info::Sdk::Esp32, | ||
| _ => proto::client_info::Sdk::Unknown, |
There was a problem hiding this comment.
suggestion: It might be a good idea to log a warning on _ => for visibility.
| "flutter" => proto::client_info::Sdk::Flutter, | ||
| "go" => proto::client_info::Sdk::Go, | ||
| "unity" => proto::client_info::Sdk::Unity, | ||
| "react_native" | "react-native" | "reactnative" => proto::client_info::Sdk::ReactNative, |
There was a problem hiding this comment.
suggestion: I would just match against the "canonical" string the SFU checks for when this is provided via URL parameter (for non-single peer connection).
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
some comments, lgtm after addressing them
| --- | ||
| livekit-api: patch | ||
| livekit: patch | ||
| livekit-ffi: patch |
There was a problem hiding this comment.
I only see changes touching the livekit-api, but not other two crates
| "node" => proto::client_info::Sdk::Node, | ||
| "unreal" => proto::client_info::Sdk::Unreal, | ||
| "esp32" => proto::client_info::Sdk::Esp32, | ||
| _ => proto::client_info::Sdk::Unknown, |
There was a problem hiding this comment.
how about C# ? do we have something like proto::client_info::Sdk::CSharp ?
There was a problem hiding this comment.
Currently that is covered by proto::client_info::Sdk::Unity.
| options | ||
| } | ||
|
|
||
| fn decode_join_request_param(param: &str) -> proto::JoinRequest { |
There was a problem hiding this comment.
is this function used by test only ? any way to be more explicit if that is the case ?
PR description
Report the corect client fclient_info_sdk_for_name
Breaking changes
I dont believe there are breaking changes
Testing
Unit tests inline