Summary:	ASTERISK-04156: [patch] make WaitForSilence return value meaningful
Reporter:	paradise (paradise)	Labels:
Date Opened:	2005-05-13 04:35:27	Date Closed:	2008-01-15 15:37:52.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_waitforsilence.patch.txt ( 1) waitstatus_rev3.diff.txt
Description:	WaitForSilence return value doesn't show whether it has detected silence or not. it just returns 0 regardless of silence detection status. now it returns 1 if silence is detected and 0 if not. -1 on error or hangup. ****** ADDITIONAL INFORMATION ****** Disclaimer on file. Paradise Dove (pardove@gmail.com)
Comments:	By: paradise (paradise) 2005-05-13 04:38:42 sorry, the bug summary should be started with [patch] By: Kevin P. Fleming (kpfleming) 2005-05-14 19:30:12 The result value from a dialplan application is not exposed in the dialplan, so how do you expect this to be valuable? The only way you could tell whether the app found silence or timed out would be for the app to set a channel variable with a result string in it (not a numeric value, please). I'd be willing to see a variable called WAITSTATUS with values 'TIMEOUT' or 'SILENCE' as options. By: paradise (paradise) 2005-05-14 23:43:41 Yes, you're right. i didn't noticed that, because i'm using it in an AGI, so here the result value is more useful as AGIs can't access the Channel Vars. By: Clod Patry (junky) 2005-05-18 23:40:48 What do you mean with "AGIs can't access the Channel Vars."? AGI can access channel variables, without problem. kpflemming: why not returning number? I can take care of a patch setting WAITSTATUS with possible values to TIMEOUT or SILENCE depends of what that app detected. so in AGI, you will be GET VARIABLE WAITSTATUS and you'll know what's going on, is it what you looking for? By: Kevin P. Fleming (kpfleming) 2005-05-19 00:08:13 Returning a number as a dialplan application under normal circumstances (not an AGI) is not useful, because the dialplan will only continue executing if zero is returned, and if non-zero is returned it will hang up the call. Setting a channel variable with the result is a more useful technique. By: Russell Bryant (russell) 2005-05-19 01:28:17 I believe a return value greater than zero implies that the call will continue at that extension. See pbx_builtin_waitexten as a reference example for this. By: Kevin P. Fleming (kpfleming) 2005-05-19 01:35:09 Yeah, that's right, sorry for the confusion :-) By: Michael Jerris (mikej) 2005-05-19 08:27:12 This may be a new found use for functions.  If we add a show field to the function definitions, we could create a funcion wrapper for some of these other applications (right in the app file) that does not show up in show functions (because you would not want to see them in the dialplan) but could be exposed to be used in AGI's to return specified values and not have to set a bunch of channel vars.  Thoughts? By: Clod Patry (junky) 2005-05-19 12:17:29 That patch is innapropriate, cause even if a timeout occurs, you still are in do_wait function. So that function always returns 1 or 0 if you hangups that channel. If we create an WAITSTATUS with 2 options: TIMEOUT and SILENCE is it what you looking for exactly? By: Clod Patry (junky) 2005-05-20 20:55:54 waitstatus.diff.txt is a patch that set a channel variable (WAITSTATUS) to TIMEOUT or SILENCE depends if that app exits with silence or not. By: Clod Patry (junky) 2005-05-20 23:34:14 rev2 is to prevent ast_frfree to be called twice. Btw, what happens if we run ast_frfree twice? Apparently there's no problem. By: Mark Spencer (markster) 2005-05-23 01:03:43 status is set to NULL and then strcpy'd? By: Clod Patry (junky) 2005-05-23 05:35:42 sure, cause im doing that after to this pointer: strcpy(status, "SILENCE"); or strcpy(status, "TIMEOUT"); which set status to SILENCE or TIMEOUT. What's wrong? By: Michael Jerris (mikej) 2005-05-23 06:18:23 There is nothing allocated for status to store the string. By: Clod Patry (junky) 2005-05-23 06:53:48 i've changed your things about status. Plus, i've changed my strcpy to ast_copy_string to be fluent with all the recent changes. By: Michael Jerris (mikej) 2005-05-25 13:16:26 We need functionality testing on this. By: Mark Spencer (markster) 2005-05-25 13:16:40 Just needs functionality testing. By: Clod Patry (junky) 2005-05-26 07:07:13 paradise: we're waiting your GO here :) By: paradise (paradise) 2005-05-26 11:55:16 i'm going to test it and will let you know the result soon. thanks! By: Michael Jerris (mikej) 2005-06-02 13:56:13 paradise, any results yet? By: Clod Patry (junky) 2005-06-08 07:11:22 paradise: any development here??? We need feedbacks to commit it or to fix it. Thanks. By: Clod Patry (junky) 2005-06-09 11:59:08 By: Kevin P. Fleming (kpfleming) 2005-06-09 17:02:28 Committed to CVS HEAD, thanks! By: Digium Subversion (svnbot) 2008-01-15 15:37:52.000-0600 Repository: asterisk Revision: 5888 U   trunk/apps/app_waitforsilence.c ------------------------------------------------------------------------ r5888 | kpfleming | 2008-01-15 15:37:52 -0600 (Tue, 15 Jan 2008) | 2 lines add WAITSTATUS channel variable output to WaitForSilence() application (bug ASTERISK-4156) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=5888