Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing createSIPParticipant params #337

Merged
merged 5 commits into from
Nov 28, 2024
Merged

Add missing createSIPParticipant params #337

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
401930e
add missing create sip participant params
umarniz Nov 20, 2024
b9bb211
Fixed prettier issue
umarniz Nov 20, 2024
8da0535
Add changeset
umarniz Nov 20, 2024
3af97e5
Default ringingTimeout and maxCallDuration to be undefined
umarniz Nov 28, 2024
45a9390
Fix formatting issues
umarniz Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/early-forks-sin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'livekit-server-sdk': minor
---

SIP: Add missing params in CreateSIPParticipant
13 changes: 13 additions & 0 deletions packages/livekit-server-sdk/src/SipClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-FileCopyrightText: 2024 LiveKit, Inc.
//
// SPDX-License-Identifier: Apache-2.0
import { Duration } from '@bufbuild/protobuf';
import {
CreateSIPDispatchRuleRequest,
CreateSIPInboundTrunkRequest,
Expand Down Expand Up @@ -94,6 +95,9 @@ export interface CreateSipParticipantOptions {
playRingtone?: boolean; // Deprecated, use playDialtone instead
playDialtone?: boolean;
hidePhoneNumber?: boolean;
ringingTimeout?: number; // Duration in seconds
maxCallDuration?: number; // Duration in seconds
enableKrisp?: boolean;
}

export interface TransferSipParticipantOptions {
Expand Down Expand Up @@ -419,6 +423,9 @@ export class SipClient extends ServiceBase {
let playRingtone: boolean = false;
let playDialtone: boolean = false;
let hidePhoneNumber: boolean = false;
let ringingTimeout: number = 0;
let maxCallDuration: number = 0;
let enableKrisp: boolean = false;

if (opts !== undefined) {
participantIdentity = opts.participantIdentity || '';
Expand All @@ -428,6 +435,9 @@ export class SipClient extends ServiceBase {
playRingtone = opts.playRingtone || false;
playDialtone = opts.playDialtone || playRingtone; // Enable PlayDialtone if either PlayDialtone or playRingtone is set
hidePhoneNumber = opts.hidePhoneNumber || false;
ringingTimeout = opts.ringingTimeout || 0;
maxCallDuration = opts.maxCallDuration || 0;
enableKrisp = opts.enableKrisp || false;
}

const req = new CreateSIPParticipantRequest({
Expand All @@ -441,6 +451,9 @@ export class SipClient extends ServiceBase {
playRingtone: playDialtone,
playDialtone: playDialtone,
hidePhoneNumber: hidePhoneNumber,
ringingTimeout: new Duration({ seconds: BigInt(ringingTimeout) }),
maxCallDuration: new Duration({ seconds: BigInt(maxCallDuration) }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when it's not passed in, we should leave these as undefined instead of set to 0. this way the server is able to pick sane defaults, versus having different defaults from each client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidzhao thanks for the review, defaulted the 3 new params to be undefined if not specified

enableKrisp: enableKrisp,
}).toJson();

const data = await this.rpc.request(
Expand Down