[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] Fix feature inheritance when using
From:       "Mark Michelson" <mmichelson () digium ! com>
Date:       2009-01-29 22:26:09
Message-ID: 20090129222609.8061.39059 () hotblack ! digium ! internal
[Download RAW message or body]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/138/#review345
-----------------------------------------------------------

Ship it!


This gets my approval, assuming that all testing has gone as expected.

- Mark


On 2009-01-29 16:01:50, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/138/
> -----------------------------------------------------------
> 
> (Updated 2009-01-29 16:01:50)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> When using builtin features like parking and transfers, the AST_FEATURE_* flags \
> would not be set correctly for all instances when either performing a builtin \
> attended transfer, or parking a call and getting the timeout callback.  Also, there \
> was no way on a per-call basis to specify what features someone should have on \
> picking up a parked call (since that doesn't involve the Dial() command).  There \
> was a global option for setting whether or not all users who pickup a parked call \
> should have AST_FEATURE_REDIRECT set, but nothing for DISCONNECT, AUTOMON, or \
> PARKCALL. 
> This patch:
> 1) adds the BRIDGE_FEATURES dialplan variable which can be set either in the \
> dialplan or with setvar in channels that support it.  This variable can be set to \
> any combination of 't', 'k', 'w', and 'h' (case insensitive matching of the \
> equivalent dial options), to set what features should be activated on this channel. \
> The patch moves the setting of the features datastores into the bridging code \
> instead of app_dial to help facilitate this. 
> 2) adds global options parkedcallparking, parkedcallhangup, and parkedcallrecording \
> to be similar to the parkedcalltransfers option for globally setting features. 
> 3) has builtin_atxfer call builtin_parkcall if being transfered to the parking \
> extension since tracking everything through multiple masquerades, etc. is difficult \
> and error-prone 
> 4) attempts to fix all cases of return calls from parking and completed builtin \
> transfers not having the correct permissions 
> 
> Diffs
> -----
> 
> branches/1.4/CHANGES 172062 
> branches/1.4/apps/app_dial.c 172062 
> branches/1.4/configs/features.conf.sample 172062 
> branches/1.4/doc/channelvariables.txt 172062 
> branches/1.4/include/asterisk/global_datastores.h 172062 
> branches/1.4/main/global_datastores.c 172062 
> branches/1.4/res/res_features.c 172062 
> 
> Diff: http://reviewboard.digium.com/r/138/diff
> 
> 
> Testing
> -------
> 
> I think I've tested the combinations of
> A calls B, (A or B) (builtin or SIP) (blind or attended) transfers (B or A) to (C \
> or parking) and tested parking either timing out or being picked up.
> 
> The case I used was having tT in the Dials to the extensions with an optional k or \
> K and tested the inheritance of the parking feature. 
> There are a lot of cases and it is certainly possible that I missed some, or typed \
> the wrong extension and thought I was testing something that I wasn't, etc.  So I \
> would *really* appreciate it if some other people could review this and test it out \
> before committing.  Most of the stuff before hand was broken anyway, but still... 
> 
> Thanks,
> 
> Terry
> 
> 


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.com--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev


[prev in list] [next in list] [prev in thread] [next in thread]