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 TinyPhone::Join (can be used to join two specific calls together) #81

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

pdesaulniers-vertisoft
Copy link
Contributor

@pdesaulniers-vertisoft pdesaulniers-vertisoft commented Nov 24, 2023

This PR implements TinyPhone::Join and the following endpoint: /calls/<int>/join/<int>

This method is very similar to TinyPhone::Conference. It can be used to initiate a conference between an active call and a second, specific call (as opposed to TinyPhone::Conference, which joins all ongoing calls together).

This method can be used to join two specific calls.

It is similar to `TinyPhone::Conference`, except it applies to a specific call, rather than all current calls.
This endpoint exposes `TinyPhone::Join`.
@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

@pdesaulniers-vertisoft pdesaulniers-vertisoft marked this pull request as draft November 24, 2023 17:59
@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

@pdesaulniers-vertisoft pdesaulniers-vertisoft marked this pull request as ready for review November 24, 2023 19:23
@kingster
Copy link
Member

Hi @pdesaulniers-vertisoft one thing that I see missing before we can merge this capability is how to unjoin calls.

Second, what would be the behaviour if the join request is made with a 3rd call when call 1 & 2 are already joined. IMO this would need some tracking of bridged (joined) calls and that is the reason conference today works on linking all calls, similar to what smartphones offers.

@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

@pdesaulniers-vertisoft
Copy link
Contributor Author

pdesaulniers-vertisoft commented Nov 25, 2023

Hello,

one thing that I see missing before we can merge this capability is how to unjoin calls.

OK, I added the unjoin endpoint.

what would be the behaviour if the join request is made with a 3rd call when call 1 & 2 are already joined. IMO this would need some tracking of bridged (joined) calls and that is the reason conference today works on linking all calls, similar to what smartphones offers.

Yes, the current behavior of join/unjoin is not very practical when the conference should contain more than 2 calls...

In that case, would it be reasonable to bridge call_to_join_id with all the currently 'active' calls where current_media.status != PJSUA_CALL_MEDIA_LOCAL_HOLD && current_media.status != PJSUA_CALL_MEDIA_NONE ? I think this approach would be a bit easier to implement than keeping track of bridged calls...

Also, if we use this approach, I suppose it would make sense to change the API route from /calls/<call_id>/join/<call_to_join_id> to /calls/<call_to_join_id>/join, meaning: "join <call_to_join_id> to the current conference"

@AppVeyorBot
Copy link

1 similar comment
@AppVeyorBot
Copy link

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