Summary:	ASTERISK-06714: Features beginning with '*' dont work for chan_agent
Reporter:	Dinesh Nair (alphaque)	Labels:
Date Opened:	2006-04-06 08:44:11	Date Closed:	2006-04-15 17:39:31
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_agent
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) agent-endcall.patch ( 1) agent-endcall-trunk.patch ( 2) chan_agent.c.diff
Description:	When Queue() is called with the 'wWtT' options, blindxfer, atxfer and automon are made available to agents with the dtmf keypresses as configured in features.conf. however, if any of these features are activated by a first dtmf of '*', the call is hungup. this is due to the fact that chan_agent hangs up the agent and the call when it detects a '*' instead of passing the entire dtmf string to res_features.so. the attached patch adds a new option to agents.conf, endcall, which acts this way. if endcall=yes, then '*' is used to end a call as is the behaviour now. this is the default, if no endcall is found in agents.conf. if endcall=no, then '*' is ignored. ****** ADDITIONAL INFORMATION ****** this is for 1.2.6, since that is the version we're working with. however, it shouldnt be too difficult to apply this patch to TRUNK as well. i know features are not accepted into 1.2, however this is here for those who do need to use automon, atxfer, blindxfer with a starting '*' on this branch. if this patch/feature is accepted, i'd like to explore using the value of endcall as the DTMF needed to end the call, allowing us to move away from the '*' hardcoded within chan_agent.c. the attached patch patches both chan_agent.c and agents.conf.sample.
Comments:	By: Andrey S Pankov (casper) 2006-04-06 17:21:03 Did you redefined "disconnect => 123" in [featuremap] section of features.conf not to be default ('*')? Or do I miss smth? By: Dinesh Nair (alphaque) 2006-04-06 21:03:48 it's not about redefining disconnect in features.conf, since that wasnt the cause of this. one way of working around this is to not use '*' to prefix any feature in features.conf, but that's a workaround. the attached patch has chan_agent ignoring the '*' if endcall=no in agents.conf. By: Andrey S Pankov (casper) 2006-04-07 10:55:40 Now I see you point... sorry. :) Please try the patch attached. Disclaimer is on file. Now chan_agent should honor features.conf settings. Can you confirm that with the patch this works as expected. By: Dinesh Nair (alphaque) 2006-04-08 09:33:25 your patch works as expected. however, this will change the behaviour for those who're used to having agents use '*' to end the call. the patch i submitted keeps this functionality as default but provides a new agents.conf parameter, endcall, which turns this off (to follow the disconnect setting in features.conf) if desired. plus, the default option is the existing behaviour, and this preserves backwards compatibility. By: Andrey S Pankov (casper) 2006-04-08 11:10:20 I don't see the need to maintain backward compatibility in this case. First, trunk is a development branch and a note about chan_agent '*' change may be added into UPGRADE.txt. Second, '*' is default disconnect sequence even if not configured. By: Dinesh Nair (alphaque) 2006-04-10 05:25:09 in that case, i guess this can be committed. By: BJ Weschke (bweschke) 2006-04-10 21:45:17 alphaque - I like your patch and the recommendation to be able to change the currently hardcoded digit. Why not create a endcall digit config parameter (defaults to '*') that can be observed when endcall=yes? I think this is candidate for /trunk, but probably a little too invasive for 1.2.X. By: Andrey S Pankov (casper) 2006-04-11 01:01:40 bweschke: if you read the description you'll probably see that the solution as proposed by alphaque is wrong. If the patch is commited endcall=yes will mean "ignore any configured feature beginning with *". If someone wants to configure separate sequence for agents disconnect he should use applicationmap/DYNAMIC_FEATURES (I hope this can be done with current implementation, can't it?) By: Dinesh Nair (alphaque) 2006-04-11 01:44:04 thanks, bweschke. initially, (according to the patch i've submitted), endcall takes values of yes or no only, with yes turning on the default chan_agent behaviour of using '*' and endcall=no ignoring the '*' and falling back to whatever has been defined in features.conf. an extension of this, as i've mentioned in Additional Information, would be to take the following values for endcall: no - ignore '*' and fallback to whatever's in features.conf any other value ([0-9*#]) would be the digit sequence used to end the call, overriding what's in features.conf and only for chan_agent. By: Dinesh Nair (alphaque) 2006-04-11 01:47:36 casper: i think you're misunderstanding what the current behaviour of chan_agent is and what my patch does. currently, the '*' to end the call is hardcoded in chan_agent, making any feature starting with '*' in features.conf unavailable for agent calls. my patch allows the user to set endcall=no in agents.conf, thus ignoring the hardcoded '*' in chan_agent and falling back to whatever has been defined in features.conf. so when you say, "If the patch is commited endcall=yes will mean "ignore any configured feature beginning with *"." that is exactly the default behaviour of chan_agent now without the patch. By: Andrey S Pankov (casper) 2006-04-11 02:15:59 > so when you say, "If the patch is commited endcall=yes will mean "ignore any > configured feature beginning with *"." that is exactly the default behaviour of > chan_agent now without the patch. Yes, it is. I mean the documentation you provided in the patch. And the default behavior is to ignore features.conf configuration. I'd like to get rid of the hardcoded '*' value at all and use disconnect to configure global disconnect sequence and applicationmaps to configure custom disconnect sequence for agents (or whatever/whoever else). Noway setting endcall to "no" will force ignoring of '*' or inability to end a call. In default features.conf configuration agents will still be able to use '*' as disconnect sequence if not configured otherwise. The problem is not in who's patch to commit. Even if it is mine I'd be happy if you get a karma point for that report and your efforts in fixing the issue. By: Dinesh Nair (alphaque) 2006-04-11 02:32:58 > Yes, it is. I mean the documentation you provided in the patch. And the > default behavior is to ignore features.conf configuration. I'd like to get rid if the semantics in the patch to agents.conf.sample were wrong, then by all means do change them to something more descriptive. > of the hardcoded '*' value at all and use disconnect to configure global > disconnect sequence and applicationmaps to configure custom disconnect this is just as fine, it's just that this may break backwards compatibility, which is why i'm suggesting something which eases this in. > The problem is not in who's patch to commit. Even if it is mine I'd be happy > if you get a karma point for that report and your efforts in fixing the issue. it's not about karma points, casper. By: Andrey S Pankov (casper) 2006-04-11 03:39:49 > it's not about karma points, casper. :) I stop the debate on the issue for now... it should be obvious enough from my earlier posts that my patch is for trunk and is the right way (IMO of course) to do it. As to 1.2 I'd consider this more like a feature, not a bug... and you should know that 1.2 is bugfix only branch. By: BJ Weschke (bweschke) 2006-04-11 07:25:51 casper: the problem you're going to have with your patch is that if you're not using callback mode in chan_agent (agents are always 'connected') you're not going to be able to signal a disconnect anymore by pulling out this code completely from chan_agent. That, in itself, I think is going to cause some problems. By: BJ Weschke (bweschke) 2006-04-11 09:34:55 alphaque: your patch fails to merge correct against current /trunk. Can you please update it? By: Dinesh Nair (alphaque) 2006-04-15 04:31:42 patch which merges cleanly against trunk (revision 20292) is attached as agent-endcall-trunk.patch. for those using 1.2.6 and 1.2.7.1, agent-endcall.patch works in those branch releases. By: Russell Bryant (russell) 2006-04-15 17:39:19 I have merged the patch that implements endcall into the trunk, with some small modifications.