Summary:	ASTERISK-12457: [patch] Add various app_dial features to app_bridge
Reporter:	Tim Ringenbach at Asteria Solutions Group (tim_ringenbach)	Labels:
Date Opened:	2008-07-25 15:33:50	Date Closed:	2009-10-21 18:50:59
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Resources/NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_bridge_options_r138998.diff ( 1) app_bridge_options.diff
Description:	This patch adds all of app_dial's features that seemed appropriate to app_bridge. It also adds an x flag that causes the callee to be disconnected at the end of the bridge instead of restarted in the dialplan. I've not tested all the options, only the T and x flags, and even then I was testing it against 1.4.
Comments:	By: Mark Michelson (mmichelson) 2008-08-19 18:34:43 I gave the patch a look. It seems fine with two exceptions. One is that the default time limit that is used if the argument to L() is invalid. You have set the default to be 360 ms, which would be a very short call :). The other thing is the not threadsafe use of pbx_builtin_getvar_helper. While the entirety of the Asterisk codebase hasn't converted to use the result of that function safely, we are trying to move in the direction of using it properly. If you're not sure what I'm talking about, see the second bullet point in doc/janitor-projects.txt in the Asterisk source directory. By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2008-08-19 21:06:22 Thanks for looking at it. I just uploaded a new version of the patch if those two issues corrected. The patch is against r138998. I haven't yet had a chance to test this version beyond that it compiles cleanly. By: Mark Michelson (mmichelson) 2008-08-21 13:09:34 The patch looks fine by me now. I'm moving this to "ready for testing" in the hopes that it gets tested, just in case there's a subtle problem I didn't see. Thanks for the patch! By: Leif Madsen (lmadsen) 2008-11-20 09:18:36.000-0600 This seems like a fine candidate for reviewboard! Seems unfortunately we're not going to find any testers for this right now :( By: Russell Bryant (russell) 2008-12-11 16:19:52.000-0600 I'm fine with this going in in general, but I have a couple of comments ... 1) Someone will need to update this for xmldocs 2) It looks like there is probably a lot of code duplication realted to call limit handling.  It would be nice to centralize this to an API call of some kind. By: Leif Madsen (lmadsen) 2009-02-02 16:49:37.000-0600 Tim, any chance of maybe doing number 2? If so, then I'll take care of number 1 (I need to learn how to do that xmldocs stuff anyways :)) By: Tim Ringenbach at Asteria Solutions Group (tim_ringenbach) 2009-02-02 17:10:35.000-0600 I'm not opposed to it. I'm not really sure how to go about abstracting that though. I'd appreciate some suggestion of where it should live and what it should look like. Or to put it another way, if someone will write the patch for the .h part of number 2, I'll do the .c part. By: Leif Madsen (lmadsen) 2009-05-12 20:01:46 Dropping this down to 'confirmed' since it looks like the patch needs to be updated with the comments that Russell has noted. By: Digium Subversion (svnbot) 2009-09-24 15:32:11 Repository: asterisk Revision: 220344 U   trunk/apps/app_dial.c U   trunk/include/asterisk/features.h U   trunk/main/features.c ------------------------------------------------------------------------ r220344 | jpeeler | 2009-09-24 15:32:11 -0500 (Thu, 24 Sep 2009) | 13 lines Add bridge related dial flags to the bridge app Most of the functionality here is gained simply by setting the feature flag on the bridge config. However, the dial limit functionality has been moved from app_dial to the features code and has been made public so both app_dial and the bridge app can use it. (closes issue ASTERISK-12457) Reported by: tim_ringenbach Patches:      app_bridge_options_r138998.diff uploaded by tim ringenbach (license 540),      modified by me ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=220344 By: Digium Subversion (svnbot) 2009-09-24 15:32:46 Repository: asterisk Revision: 220347 _U  branches/1.6.2/ ------------------------------------------------------------------------ r220347 | jpeeler | 2009-09-24 15:32:46 -0500 (Thu, 24 Sep 2009) | 19 lines Blocked revisions 220344 via svnmerge ........  r220344 | jpeeler | 2009-09-24 15:29:51 -0500 (Thu, 24 Sep 2009) | 13 lines    Add bridge related dial flags to the bridge app    Most of the functionality here is gained simply by setting the feature flag  on the bridge config. However, the dial limit functionality has been moved from  app_dial to the features code and has been made public so both app_dial and  the bridge app can use it.    (closes issue ASTERISK-12457)  Reported by: tim_ringenbach  Patches:        app_bridge_options_r138998.diff uploaded by tim ringenbach (license 540),        modified by me ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=220347