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.

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

Implement TransferParticipant in SIP #425

Merged
merged 5 commits into from
Sep 25, 2024
Merged

Conversation

biglittlebigben
Copy link
Contributor

No description provided.

cmd/lk/sip.go Outdated
Name: "transfer",
Usage: "Transfer a SIP Participant",
Action: transferSIPParticipant,
ArgsUsage: RequestDesc[livekit.TransferSIPParticipantRequest](),
Copy link
Contributor

Choose a reason for hiding this comment

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

This API is pretty close to the ones we have on room participants in general, so maybe we should not require a JSON file here? Could be all 3 arguments instead: --room, --id (or how do we call participant identity in CLI?) and regular argument for the transfer URI.

Copy link
Contributor

Choose a reason for hiding this comment

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

++ on using named args. We usually use --identity for participants, and could map the rooms to --from and --to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be consistent with other commands, should we use --room, --identity --to? It may be confusing to have from be a room, and to a sip url

&cli.StringFlag{
Name: "to",
Required: true,
Usage: "`SIP URL` to transfer the call to. Use 'tel:<phone number>' to transfer to a phone",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the tel: prefix a part of SIP spec? Did I miss the parsing part in the REFER PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is supported and documented by Twilio. It's unclear if other providers support it as well. We can adjust the documentation as needed.

@biglittlebigben biglittlebigben merged commit 93657b7 into main Sep 25, 2024
3 checks passed
@biglittlebigben biglittlebigben deleted the benjamin/sip_transfer branch September 25, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants