[Home]

Summary:ASTERISK-03644: [patch] Cancel an in-progress attended transfer.
Reporter:Anthony Minessale (anthm)Labels:
Date Opened:2005-03-07 13:08:15.000-0600Date Closed:2008-01-15 15:39:22.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_features
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) features0305_rev0.diff
( 1) features0305_rev1.diff
( 2) features0305_rev2.diff
( 3) features0305_rev10.diff
( 4) features0305_rev3.diff
( 5) features0305_rev4.diff
( 6) features0305_rev5.diff
( 7) features0305_rev6.diff
( 8) features0305_rev7.diff
( 9) features0305_rev8.diff
(10) features0305_rev9.diff
(11) folsson_cancel_patch.diff
(12) suggested_rev9.diff
Description:This patch makes attended transfers honor the disconnect code found in the [featuremap] section of features.conf during the attempt.  This means while you are waiting for an answer when you dial the transfee's extension you can dial this code any time and immediatly cancel the attempt and return to the original bridge.


****** ADDITIONAL INFORMATION ******

Disclaimer on file
anthmct@yahoo.com
Comments:By: Anthony Minessale (anthm) 2005-03-12 13:11:51.000-0600

updated (little tweak necessary.)
By: Anthony Minessale (anthm) 2005-03-14 18:49:08.000-0600

doh forgot to declare my new function static
By: Mark Spencer (markster) 2005-03-15 00:02:56.000-0600

1.  Why does this code automatically send ringback without waiting for it to come from the other side in the features request_and_dial?  If you call something which doesn't have ringback, this definitely does the "wrong thing"...  Instead, you need to be bridiging audio bidirectionally (see app_dial).  In fact, a potentially easier way to do this might be to actually call app_dial, you think?

2.  Does this code properly handle an empty disconnect code?

3.  You can't just ignore the outbound channel for 100ms at a time.  You have to actually perform a waitfor_n and multiplex the input of the two (remember meetme?)

4.  Why are we declaring a CDR on the outbound leg?  Shouldn't that be the responsibility of the calling function if we want it?

Looks like a promising start though.
By: Brian West (bkw918) 2005-03-17 21:08:26.000-0600

anthm?  

/b
By: Anthony Minessale (anthm) 2005-03-18 09:26:32.000-0600

I'm not so sure,

1 of the channels isnt up yet so you cant read from it you can only see if it's up or not the caller is the only chan you are required to entertain.

Just to be sure i put in a waitfor_n and the caller chan exclusively won even if i alternated thier placement in the array.

the rest you tell me this ringback stuff never works right so I usually do it to be safe. and I don't care if you cut either part.
By: Paul Hales (paulh) 2005-03-23 22:17:11.000-0600

We are looking forward to this function being added to CVS-HEAD.
By: Mark Spencer (markster) 2005-03-23 23:13:55.000-0600

Perhaps the loop from app_dial called wait_for_answer() would be helpful.  Maybe even some of its logic could be brought into a function that could be more generally useful?
By: Anthony Minessale (anthm) 2005-03-24 09:05:31.000-0600

Here's what I was thinking earlier:

What if we take everything about app_dial that relates to placing a call and make it a function either designed to take a pointer to an array of dialstrings or a varargs and the calling channel.

struct ast_channel *ast_dial(struct ast_channel *caller, int format, char **dialstr, size_t len)

struct ast_channel *ast_dial(struct ast_channel *caller, int format,...)

since app_dial needs to be moved to the new arg parser that would be a good time to try and abstract the core features. then one could make thier own app_dial with a few lines of code if the so choose.
By: Mark Spencer (markster) 2005-03-26 01:41:48.000-0600

Calling/waiting for an answer seems logical here.  Might even be able to unify a bunch of code from app_dial and app_queue...
By: Anthony Minessale (anthm) 2005-04-20 13:45:26

This does work as is though, it solves the problem for the customer who is happy now, could we add it in the meantime while plotting a way to use an alternate function/api shift, rework app_dial so it's not killed by the langoleers?
By: Paul Hales (paulh) 2005-05-03 21:49:41

I would really appreciate this being added to CVS as I now have 2 customers using it, and I don't want it being killed by an update to Asterisk.
By: Kevin P. Fleming (kpfleming) 2005-05-04 16:34:54

I agree, this should probably go in as it stands right now (assuming it still works as intended, but it should), since any API changes for dialing are not going to happen in a short time period.

Any objections?
By: Anthony Minessale (anthm) 2005-05-23 19:16:02

updated
By: Filip Olsson (folsson) 2005-05-24 07:28:07

I would think that the objections markster had about ignoring the outbound call is justified:

If the outbound channel gets AST_CONTROL_PROGRESS we would start getting a lot of frames on that channel. The result of it being that we never get the AST_CONTROL_ANSWER (or any of the other control-frames), because we have a bunch of audio-frames to read before we get to the good part. We timeout before we have read all the frames before AST_CONTROL_ANSWER.

anthm: this was the problem I talked about last night on IRC. It doesn't have anything to do with the indications.
By: Anthony Minessale (anthm) 2005-05-24 08:59:32

Well you found yourself in one of asterisk's catch 22's if the system was not designed around trying to use 1 thread where there should be 2 you would have each channel in a thread and you could easily do this stuff if you poll the 2 channels the caller's channel is never seen because it is not sending audio yet and dtmf does not trigger the poll
By: Filip Olsson (folsson) 2005-05-24 10:18:15

Anyway, I've got it working now. Works like a charm.

As markster said I used ast_waitfor_n in something very like what's found in wait_for_answer().

The patch isn't very clean since I use my own ast_app_dtget that doesn't care for the ast_extension_exist()-things.
By: Filip Olsson (folsson) 2005-05-24 11:24:21

Uploaded the patch on anthm's request. There's probably a bunch of problems of doing it like this, but it works.
By: Anthony Minessale (anthm) 2005-05-24 12:30:31

changed it around a little does it still work?
By: Anthony Minessale (anthm) 2005-05-24 14:19:53

rev7 has been tested and tries to compromise a working soulution give it a review and get it in.
By: Dave Ryan (newsole2) 2005-05-24 17:34:23

with rev7 I now get

May 24 15:33:03 NOTICE[20198]: res_features.c:960 ast_feature_request_and_dial: We exceeded our AT-timeout

and hangs up....

but still has not fixed main problem I was refered to here for

On Zap it works fine.... but on IAX I get no Busy or ring tones.... and it takes 60 seconds from time finished dialing till it spawns new channel on server and tries to dial.
By: Anthony Minessale (anthm) 2005-05-24 19:32:15

ok let's try rev8
By: Filip Olsson (folsson) 2005-05-25 16:38:48

I've tried this stuff and it works like it should.

I really think we should break out wait_for_answer() into a more general function (as previously stated). That piece of code is in too many places with small variations. And soon we have another one?
By: Magnus Drougge (minstrel) 2005-05-30 00:37:55

Hello!

It's not possible to get back to the original caller (transferee) once the new peer has answered (unless that person hangs up before you do). That causes problems when you want to back out from answering machines and such.

It is also not possible to 'convert' an attended transfer into a blind one by hanging up while waiting for an answer, something that I believe should be supported.

I patched res_features.c with patch rev 8 and made a few alterations to builtin_atxfer() and ast_feature_request_and_dial() that allows the two scenarious stated above. But I am not very familiar with the internals of asterisk and thus have no idea if I'm causing memory leaks or inconsistencies with my solution.

Can I send the resulting diff to any of you guys for inspection and possibly have it included in patch rev 9?
By: Andrew Kohlsmith (akohlsmith) 2005-05-30 05:45:03

Good catch; I would have assumed that both of these things worked just as the nature of the beast!  I'll give rev9 a shot here.
By: Magnus Drougge (minstrel) 2005-05-30 06:42:41

I've added my suggested changes as suggested_rev9.diff, please have a look at it and then delete it if/when you have included them in the real rev9.

And as I said earlier, I don't have in-depth knowledge of asterisk. These changes works for me but I don't know if they leak or deviate from any standard. So don't trust them.


By: Paul Hales (paulh) 2005-05-30 18:01:48

Great to see this item being tested and worked with - I know of 3 setups using this patch.
By: Clod Patry (junky) 2005-05-30 18:05:51

Can i delete all files prior on suggested_rev9.diff?
By: Michael Jerris (mikej) 2005-05-30 19:26:21

junky, definately don't delete rev8 at least until anthm can take a look at the rev9 that somone else put on the bug.
By: Magnus Drougge (minstrel) 2005-06-20 02:45:24

Have you had the time to review my suggested changes yet?
By: Anthony Minessale (anthm) 2005-06-20 08:53:03

see if this has the same effect.
I think you had an overcomplicated if and || in your patch this *should* be the same effect let me know.  If not tell me what you were trying to accomplish.  I got the impression that the transferer still being up after the call was automatic grounds to hangup on the transfer dest.
By: Michael Jerris (mikej) 2005-06-23 14:01:17

from dev conf:  change the collection of digits for the code to reset when it gets to the point where it could not be the right code, instead of when it gets to the number of digits for the code.
By: Kevin P. Fleming (kpfleming) 2005-06-23 18:11:42

Committed to CVS HEAD with some minor mods, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:39:22.000-0600

Repository: asterisk
Revision: 5991

U   trunk/res/res_features.c

------------------------------------------------------------------------
r5991 | kpfleming | 2008-01-15 15:39:21 -0600 (Tue, 15 Jan 2008) | 2 lines

support cancellation of attended transfers using the defined disconnect code (bug ASTERISK-3644 with minor mods)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5991