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

add transfer action type #10

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

Conversation

byoungdale
Copy link

@byoungdale byoungdale commented Aug 8, 2019

This change implements a "transfer" action option. It only implements a "blind" transfer in the sense that it just sends the REFER and hangs up once it get the final NOTIFY with 200 OK. I think in theory you could construct an "attended" transfer scenario by adding a "hold" action, and setting up another call to be the transfer target. This is my first try at C++ and I reused a lot of the code that was already there, so please let me know if something is off or needs improvement.

I also added examples to the README on how to set up the new action.

@byoungdale byoungdale changed the title Transfer feature branch adding Transfer action Aug 8, 2019
@byoungdale byoungdale changed the title adding Transfer action adding transfer action Aug 8, 2019
@byoungdale byoungdale changed the title adding transfer action add transfer action type Aug 8, 2019
@jchavanton
Copy link
Owner

Hi sorry for the delay, I just made sure I will receive email notifications from now on.

I will test and review asap, thank you !

@jchavanton
Copy link
Owner

interesting, however the transfer command is blocking everything else and affecting all the calls.
I think we should find a way to transfer specific calls only, so that it is still possible to do multiple calls in parallel doing different things.

@jchavanton
Copy link
Owner

jchavanton commented Aug 25, 2019

One option would be to add call_group that may contain zero, one or more calls.
Then we could apply specific actions to group.

Example :

   <action type="accept" label="receive_and_transfer" call_group="will_transfer"
      account="VP_ENV_CALLEE_USERNAME"
      wait_until="DISCONNECTED"
      play="../voice_ref_files/reference_8000.wav"
      code="200" reason="OK"
      ring_duration="5"
      rtp_stats
    />
    <!-- note: call ends once transfer is complete -->
    <action type="transfer" call_group="will_transfer" to_uri="12015555555@target.com"/>
  </action>

What do you think ?

@byoungdale
Copy link
Author

Ah, ok. I did not realize that. So, any new calls coming into this scenario are blocked when the transfer is happening? Or it is blocking the whole voip_patrol process?

@jchavanton
Copy link
Owner

jchavanton commented Sep 9, 2019

It will block further action execution :

while (!completed) {

call coming in will not block

Copy link
Collaborator

@kalmik kalmik left a comment

Choose a reason for hiding this comment

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

Another idea came up in my mind but I don't know if it is a good one 😛 .

If we have a scope that defines a call behavior like this

<action type="accept" label="receive_and_transfer"
      account="VP_ENV_CALLEE_USERNAME"
      play="../voice_ref_files/reference_8000.wav"
      code="200" reason="OK"
      ring_duration="5"
      rtp_stats
>
    <behavior>
        <action type="wait" until="CONNECTED">
        <action type="dtmf" data="123">
        <action type="wait" ms="1000">
        <action type="hold">
        <action type="wait" ms="1000">
        <action type="unhold">
        <action type="transfer" to_uri="12015555555@target.com"/>
    </behavior>
</action>

It make things easy to improve more features like (DTMF, HOLD, UNHOLD)
What do you guys think about?

} else if (ci.state == PJSIP_INV_STATE_CONFIRMED) {
CallOpParam prm(true);
try {
if (to_uri.empty() ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You have already checked it before starting the loop! Are you still needing this?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, no, I think that was just an oversight.

@jchavanton
Copy link
Owner

jchavanton commented Sep 23, 2019

I thinks this is making a lot of sense, with relatively little work we should be able to refactor Config::process to support behavior, this would mean executing a branch (or a sublist) of actions.

It would execute them until a wait and enter something like wait in behavior the difference would be that when the condition to come back from the wait is met, we continue the execution of thebehavior. It does also imply and that voip_patrol will need to be able to support several parallel wait, the main one and any number of wait in behavior (or they could be the same) .

Not sure about the best way to refactor Config::process to keep things as simple as possible.

Optionally, the design/refactoring could support nested behavior ...

@jchavanton
Copy link
Owner

I am still looking at integrating this work after we applying modifications based on the discussions. (behavior / action_group)
I just think merging it as it may confusing because it is blocking.
When I/we find the time, we can work from this branch.

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