[Home]

Summary:ASTERISK-01984: [patch] allow the transfer keys from app_dial's 't' and 'T' and hangup key 'H' to be configured via features.conf
Reporter:twisted (twisted)Labels:
Date Opened:2004-07-09 21:40:26Date Closed:2011-06-07 14:10:24
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) take17.txt
( 1) take18.txt
( 2) take19.txt
Description:Don't be afraid, this patch modifies res_features.c, app_dial.c, and updates features.conf.sample.

This patch allows you to define your keypresses to use inline transfer options to be single, double, or double with  configurable timeout.  This also allows you to change the hangup key (single key only).   This is based on CVS RC1 as of 07/16/04.    



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

Enjoy!
Comments:By: zoa (zoa) 2004-07-10 15:01:22

although i'm not using a lot of transfering, i really like this idea.
Is a transfer.conf file a good place to put such things ? shouldnt we need 1 big file for all such things ?

By: twisted (twisted) 2004-07-10 15:55:35

the problem is, i couldn't really think of anywhere it really belongs..

By: cmslaght (cmslaght) 2004-07-10 23:38:50

This is cool since my bank requires me to hit '#' after I have entered in my account number.  Everytime I would hit the '#' key, asterisk would want me to transfer the call.  I will let you know if I run into any problems.

By: twisted (twisted) 2004-07-11 00:37:05

Okay.. i've updated this patch, the new version will let you use two digits keypresses for transfer, and lets you set the timeout. See the updated sample file for details. Also, i forgot to mention, there are a few minor formatting cleanups in the second patch. :)

edited on: 07-11-04 00:24

By: twisted (twisted) 2004-07-12 20:21:24

Okay.. if you have previously applied the second patch ( the one with double digit support), PLEASE destroy your copy of res/res_parking.c and reapply the patch as of today.  I noticed a couple bugs with the second patch, and i have fixed them

Thank you!

By: schurig (schurig) 2004-07-13 08:17:48

Could you:

rename transfer.conf into asterisk.conf
rename [general] into [transfer]


I think we're getting more and more little *.conf files. Maybe this is wanted. But if not, we should start doing something :-)

By: twisted (twisted) 2004-07-13 11:31:32

Honestly, I'd like to see us merge several different configuration files into one, but I had a conversation with Mark yesterday, that we need to continue to figure out what we're going to do about the overload of config files...   stay tuned

By: twisted (twisted) 2004-07-15 12:37:33

Updated configurable-transfer.txt patch to apply cleanly to current cvs as of 7/15/04.  

Mark - please comment on this on what you would like to see changed/reason we cannot go into CVS.   thanks!

By: twisted (twisted) 2004-07-17 17:55:45

Updated to CVS RC1 from 07/17/04.  Thanks for adjusting res_parking.c, parking.h, and parking.conf to the new 'features' set!

edited on: 07-17-04 20:44

By: twisted (twisted) 2004-07-23 12:17:54

Okay guys... for-review.txt is the most recent markster-reiview..   It should function just as well as the other patch up here.  Please test..   I've seen some comment on the mailing lists about wanting something similar in CVS... You need to voice your testing results here, and also, your support!

By: twisted (twisted) 2004-07-23 12:55:08

Updated again to match cvs as of 7/23/04

By: John Todd (jtodd) 2004-07-23 14:44:23

Patch applies cleanly, and seems to work as advertised.

It may be worth noting that the doubledigits= and digittimeout=  MUST be set in order for the system to recognize the double-keypress.

By: Matt Florell (mflorell) 2004-07-23 15:26:00

Well it gets me half way there, pressing ## does work with this patch, but pressing # once doesn't send a hash tone out, it gets muted while it waits to hear another tone. Any way around this so you can press a single hash and have it send a tone through? That's the way the original doublehash patch worked and that's how we need this one to work.

By: Matt Florell (mflorell) 2004-07-23 16:39:14

It is not passing the digit out if there is only one pressed. I have the digittimeout set to 400, even setting it to 10 did nothing, I have tried this on two separate SIP phones and on two separate Asterisk servers and I am running CVS from 2 hours ago with the patch posted this afternoon on both. the single hash is not sent or detected as being sent on any internal or external IVR that I have tried. Any idea why it isn't passing on the digit for me?

By: twisted (twisted) 2004-07-23 18:02:57

try passitall.txt ... i found the problem. :)  Don't set your digit timeout to 10, you'll NEVER get that second keypress in 10ms.

By: twisted (twisted) 2004-07-23 18:10:47

mflorell - also, you can find me on irc as twisted if you would like to discuss this. :)

By: schurig (schurig) 2004-07-26 03:03:52

The patch doesn't apply cleanly anymore, there are now rejects in res_features.c

By: Matt Florell (mflorell) 2004-07-26 09:10:37

Detail of rejects:
patching file asterisk/res/res_features.c
Hunk ASTERISK-6 FAILED at 297.
Hunk ASTERISK-7 succeeded at 402 (offset 1 line).
Hunk ASTERISK-9 succeeded at 438 (offset 1 line).
Hunk ASTERISK-11 succeeded at 524 (offset 1 line).
Hunk ASTERISK-13 succeeded at 797 (offset 1 line).
1 out of 18 hunks FAILED -- saving rejects to file asterisk/res/res_features.c.rej

By: Matt Florell (mflorell) 2004-07-28 08:19:58

Is anything being done to fix this patch on recent CVS?

Has this been integrated into the code base?

Where does this bug stand?

By: twisted (twisted) 2004-07-28 09:16:00

you know, I hate to say this, but i'm not the only one who can update this to current.  I have put countless hours into this trying to make it work, and get it working the way it should.   Rather than complaining, why not try to fix it yourself?  Some of us get paid for our time elsewhere, so we have to focus on our paying jobs.

By: Matt Florell (mflorell) 2004-07-28 09:47:50

I am very sorry if I offended, I did not want to duplicate any efforts. I am extremely appreciative for your work on this patch, and I did mean what I said on the mailing list. I will buy dinner at Astricon for whoever gets this into the CVS working(or something of equivelent value if you aren't going)

If noone is going to update this patch I will attempt to on my own, but what steps do you have to go through in order to get something like this integrated into the CVS? I am unfamiliar with the inner workings of the code validation system within the core Asterisk developer group.

Any help would be greatly appreciated.

By: twisted (twisted) 2004-07-28 10:20:18

No offense taken..  I just have quite a few projects going on..   Does cvs not merge with this patch applied ( to a previous checkout, of course? )  I will attempt of shove this back in later today, but I just wanted to make aware to everyone who keeps track of the bugnotes, that we are a community, and as such, we can all stand to learn and help pick up where others may not have the time to, and hey, some may even be able to do it better.  

Give me a few hours to get into the swing of my day, and I'll see if I can't re-merge this with current cvs.  (as a favor, of course ;) )

By: patrickdk (patrickdk) 2004-07-28 11:56:28

Patch for current cvs, Passitall.diff. heh, Have fun.

By: patrickdk (patrickdk) 2004-07-28 12:30:40

well, got too fancy with passitall.diff, so made passitall.diff2

By: twisted (twisted) 2004-07-28 14:15:47

Glad someone has picked up the ball... I was attempting to flat out re-write it today, and have decided to drop this project until I can improve on my C more.

PatrickDK, it's your ballgame. :)

By: twisted (twisted) 2004-07-28 14:20:23

PatrickDK, i hate to tell you this, but your patch does not compile (the second version, at least).

By: Matt Florell (mflorell) 2004-07-28 14:26:17

passitall.diff2 didn't compile for me(CVS 30 minutes ago):

res_features.c: In function `ast_bridge_call':
res_features.c:479: syntax error before "else"
make[1]: *** [res_features.o] Error 1
make[1]: Leaving directory `/usr/local/ast_src/asterisk/res'
make: *** [subdirs] Error 1

By: patrickdk (patrickdk) 2004-07-28 17:27:50

Opps, had a extra close bracket in here

By: twisted (twisted) 2004-07-28 17:42:43

Okay.. I know I said i dropped it, but PatrickDK, you broke it :P   I'm taking over again.... all of the files on this page are being deleted, and i'm uploading the newer one.

By: twisted (twisted) 2004-07-28 18:26:40

take11 takes us back to the great functionality we had before the recent changes to res_features.c  ... it works - but i'm going to fix it even though it's not broken.  Stay tuned for cleaner code. :P

By: twisted (twisted) 2004-07-28 20:38:05

take12 brings us to a whole new level of parsing the configs... yay.  it also prevents us from being stupid and trying to use non-dtmf keys as transfer keys..hehe..

In take12 we also now autoservice our transferee, so that they aren't left out in the cold.   Still working on frame storage, that part wants to prove difficult.

By: John Todd (jtodd) 2004-07-29 21:22:28

OK, tested by me again against today's CVS.  I got a slight hiccup on the features file, but probably very minor.  Transfer works like this (assuming # as our transfer key): press once, and it's played to the remote side.  Press a second time within the allotted delay interval, and the "transfer?" prompt is heard.

[root@ms1 asterisk]# patch -p0 </tmp/patch1.patch
patching file apps/app_dial.c
patching file configs/features.conf.sample
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file configs/features.conf.sample.rej
patching file res/res_features.c
[root@ms1 asterisk]#

By: twisted (twisted) 2004-07-29 23:36:59

if you take a look at your features.conf.sample.rej, you'll see what failed, and I have a feeling I know what it is ;)

By: twisted (twisted) 2004-07-29 23:43:12

also, were you stating that you had a single # as your transfer key, or a double?    If it's a double, the patch is working great. if it's a single, something's funky... however, in my lab, it works perfectly.

By: John Todd (jtodd) 2004-07-30 14:32:54

I was pressing # twice in order to get the "transfer" prompt.  The first press of # transmitted to the other side, and the second # got me the "transfer" prompt.

I've mailed you the .rej file to save space here.

By: tclark (tclark) 2004-08-01 12:23:04

take14 applies clean to cvs head as of Aug 1,
1 small chg that might be nice is in parse_transfer_opts
if transfer is set to some 2 digit combo then set
astdigittimeout=500, that way just transfer=## will work w/o setting
digittimeout

markster
any chance this might see the light of day in cvs ?

edited on: 08-01-04 12:13

By: cmslaght (cmslaght) 2004-08-01 15:24:24

I applied the patch and setup the feature so that '##' activated the transfer option.  That works however, I called myself (and my bank) and when I hit '#' (only once), asterisk actually passes it twice out loud after the timeout.

By: Matt Florell (mflorell) 2004-08-01 22:13:25

I'm having the same problem as cmslaght. pressing # once yields two # tones to be played out after the timeout. Experienced this with a Polycom IP600, Grandstream BT-102 and Sipura SPA-2000.

By: twisted (twisted) 2004-08-03 12:12:30

take15.txt brings up to current cvs as of 8/03/04

next tackle is the double passing of the first transfer digit.

By: twisted (twisted) 2004-08-03 12:28:25

take16 takes care of the doubled up digits if you let the timeout expire or if you hit the first digit then another (wrong) digit.   It should also be current to cvs.

By: tclark (tclark) 2004-08-03 13:41:50

take16 does not apply clean
this line has a dangling */
+*/ dotransfer=0;
also any reason not to default in astdigittimeout in parse_transfer_opts
when the transfer has some 2 digit value
like
if (astdigittimeout==0)
 astdigittimeout=500;
??

By: twisted (twisted) 2004-08-03 13:49:10

there's a difference between a clean apply, and a wrecked build.  I've removed the dangling */   it was an artifact of a test.  

your other concern is listed here
+                       if((sscanf(var->value, "%d", &timeout) != 1) || (timeout < 500)) {
+                               ast_log(LOG_WARNING, "No human can press two keys in %d. Setting to half a second.\n", timeo
ut);
+                               astdigittimeout = 500;
+                       } else
+                               astdigittimeout = timeout;

if we don't return something,  or it's less than 500, we set it to 500.

By: twisted (twisted) 2004-08-03 13:55:22

mind you, that notice will be changed to something a little more professional, but for my testing purposes, it works.

By: tclark (tclark) 2004-08-03 14:16:04

Err no my other concern is not when 'digittimeout' is set
its on the test above this
when you set a value for 'transfer' and it is a 2 digit transfer you can
set default 'astdigittimeout' to 500,
that way if you dont set a value in the features.conf for 'digittimeout' and only set the transfer to '##' the feature still works in most cases :)

By: twisted (twisted) 2004-08-03 14:36:39

I just initialized it at 500 instead of 0.  That should do what you were asking.

By: jas_williams (jas_williams) 2004-08-05 09:14:42

Just installed take16 this patch works great for me when can we see this merged with cvs, Markster are you listening.


Jason

By: tclark (tclark) 2004-08-06 11:47:50

FYI: current cvs 08/06/04 with r1.7 res_features causes a cpl
of minor cvs conflicts i have a diff here but thought you might
want post a new one since your maintaining this :)

By: twisted (twisted) 2004-08-13 17:59:47

Take 17 is current as of 8/13/04 @ 17:42pm CDT

By: cmslaght (cmslaght) 2004-08-16 18:26:31

I installed take17.  I have my system setup so that "##" is the transfer key.  When I press a single "#", I head two "##".  Anyone else?  Just updated on Sunday with Take 17 and latest CVS.

By: twisted (twisted) 2004-08-16 20:12:20

OOPS... I know what happened there... It's a simple fix, I'll take care of it tomorrow..

By: twisted (twisted) 2004-08-17 19:21:54

there we go.  fixed the double-passing problem again... sorry bout that :P

By: ltropiano (ltropiano) 2004-08-21 17:35:34

any chance getting this into Asterisk 1.0?

By: twisted (twisted) 2004-08-22 04:21:32

take 19 just makes a couple non-functional changes to how we deal with not getting a value for digittimeout.

By: letherglov (letherglov) 2004-08-22 19:54:16

what's left to get this applied to HEAD?

By: twisted (twisted) 2004-09-14 11:33:40

And here's the word we've all been waiting for.   Since the only word i have recieved from markster on this is something to the effect of "it may work, but it's not correct", and no assitance forward, I am no longer going to jump through the hoops of keeping this current.   If someone else would like to pick this up, keep it up to date with cvs, and bug markster to find out what's wrong, or even re-write parts of this to hopefully match what the thought of of "correctness" is, you have 24 hours to do so before I remove the files associated with this bug, and close it, forever to rest in piece.

I'm sorry that it has come to this, but after so many times of hearing "sure, i'll comment on what needs to be changed", and offering to show me what's not right here, and nothing happening, it's not worth my time any longer.  This is not a statement like it was before where I 'gave up' and came back on it, this is the real deal.

The only other way I will keep working on this is if i recieve the appropriate comments/suggestions/pointers from the persons that I need to actually get this submitted.  Otherwise, the 24 hour clock starts now.

By: tclark (tclark) 2004-09-14 12:00:16

This is an autocratic democracy right I beleive
this requires a little email lobby to markster@digium.com
for at a miniumum 1 response to this lengthy patch from markster :)
...

any agreement here ? I can paste a template email if any one in favour :)

By: abatista (abatista) 2004-09-14 12:09:51

Just a quick note on my support for this bug to be included into the CVS. It's something that has been needed for asterisk from the very beginning.

We should support this type of items. It's makes asterisk more of a user friendly system.

By: Matt Florell (mflorell) 2004-09-14 12:17:29

I'd like to voice my support as well, I've been asking for something like this to be put into CVS for over 9 months now, and this latest patch is the best structured and most flexible method of doing this I've seen.

Many thanks to twisted on his efforts, and I'll certainly send an email to markster when the template email gets posted.

By: Mark Spencer (markster) 2004-09-14 15:38:15

As twisted said, I definitely like the idea of what this patch supplies -- in fact, that's why "parking" was renamed "features" in the first place.

This patch has not yet been in a state that seems reasonable for merger yet, but I'll try to put some work into it this evening.

By: twisted (twisted) 2004-09-14 16:16:59

Thanks markster. :)    I want you to know that I understand you're busy, and appreciate your efforts tremendously, but since i'm still relatively new to the coding world, I require a little guidance to be able to understand some aspects of what's required, and so forth.   Times are getting a little busy for me as of late, which is why I came to the conclusing of shutting down if nobody wanted to pick it back up.   I'll wait for your next move now. ;)

By: twisted (twisted) 2004-09-15 11:33:45

cshaw, you're right, this isn't the place for this.   You need to add a seperate feature request.

By: jamieg (jamieg) 2004-09-16 23:18:35

I would like to just put my hand up on this going into the Main CVS tree ASAP too.  I have a few Clients I implemented this for and they are VERY happy with it.

Especially for the Grandstream USERS.

Just a note here on this feature.  I just noticed
NEW FEATURE 0002460
http://bugs.digium.com/bug_view_advanced_page.php?bug_id=0002460
Which is ATTENDANT transfer based on the # button. (When I have time I will give it a go.)
Only just saw it.  This is another FEATURE I have BADLY wanted as the cheaper Grandstream SIP phones are VERY bad when you try to use the actual "TRANSFER" key. (If you make an error, the call you are trying to transfer can get stuck on MOH with NO way to retreve them back.  NOT GOOD) So I set it up so my clients with Grandstreams use # transfer, but then you loose IVR capabilities on the phones.  Also a show stopper.  ## was the trick.  But then...

Business NEEDS attendant transfer, thats how it works..

I made some comments to the List about ## transfer and that it should be UPDATED to allow ATTENDANT transfers to, ie, like a ##=blind-Transfer, #0=Attendant-Transfer

I really think  a DOUBLE key escape is needed to give the user IVR capabilities, however, if implementing this, the Attendant transfer additional feature should be considered and may as well be worked on at the same time.

Thanks for listening.
James

By: cshaw59 (cshaw59) 2004-09-17 22:08:11

Added a new patch against today's CVS to hold us over until Mark gets time to work on it!

P.S please ignore the first one, I missed a '/' and it doesn't work....

Enjoy!

-Chris

edited on: 09-17-04 22:16

By: Anthony Minessale (anthm) 2004-09-20 16:42:25

see bug 2460 where I have tried to add my own humble implementation
of this feature to my attended xfer patch..

By: Anthony Minessale (anthm) 2004-09-20 16:44:51

cshaw59  sorry i was trying to clean up that
extra file you had and deleted both by accident =(
can you put it back thx...

edited on: 09-20-04 16:45

By: cshaw59 (cshaw59) 2004-09-20 18:20:55

No problem Anthm,

It'll have to be after I get off work though, about 6:00PST :(

By: twisted (twisted) 2004-09-25 02:59:46

Well... since this never really got the attention from the gods that it deserved, i'm closing it in favor of anthm's attempt on ASTERISK-2425