[asterisk-dev] [Code Review] PreDial continuation - Ability to run dialplan on callee and caller channels before Dial

[schmidts]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1878/#review6084
-----------------------------------------------------------

Ship it!


this changes looks solid to me. the things i noticed in the other review is solved in a fine way. only some minor questions from my side.  


/trunk/apps/app_dial.c
<https://reviewboard.asterisk.org/r/1878/#comment11172>

    is there a special need to alloce these both in the while loop? maybe it would be better to do this before the while.



/trunk/apps/app_dial.c
<https://reviewboard.asterisk.org/r/1878/#comment11173>

    why do you alloc number here? AFAIK will this be done in setvar_helper (ast_var_assign) itself.


best regards

- schmidts


On April 23, 2012, 1:19 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1878/
> -----------------------------------------------------------
> 
> (Updated April 23, 2012, 1:19 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This review takes over from the https://reviewboard.asterisk.org/r/1229/ predial review and addresses the concerns brought up by the last review there.
> 
> PreDial
>   
>   Say SIP/abc is calling SIP/def
>   You have: Dial(SIP/def)
>   SIP/def-123234 is created.  But how can you tell that from dialplan?
> 
>   You can use a pickup macro: M or U options to Dial(), but you have to wait till pickup to know.
>   'PreDial' new option 'b' to Dial(), will let you run dialplan on the newly created channel before it is connected to the end-device.
> 
>   New way:
>   Dial(SIP/def,,b(predial^s^1))
>   Dialplan will run on SIP/def-123234 and allow you to know right away what channel will be used, and you can set specific variables on that channel.
> 
> You can also run dialplan on the caller channel (option 'B') right before the dial, which is a great place to do a last microsecond UNLOCK to ensure good channel behavior.
> Example:  LOCK(foo)
>           do stuff
>           UNLOCK(foo)
>           Dial(SIP/abc)
> 
> With this above example, say SIP/123 and SIP/234 are running this dialplan.
> 
> SIP/123 locks foo
> SIP/123 unlocks foo
> due to some cpu load issue, SIP/123 takes its time getting to Dial(SIP/abc) and doesn't do it right away
> 
> Meanwhile... SIP/234 zips right by, lock 'foo' is already unlocked, it grabs the lock, does its thing and it gets to Dial(SIP/abc).  SIP/123 wakes up and finally gets to the Dial().  Now you have two channels dialing SIP/abc when there was supposed to be one.
> 
> If your intention is to ensure that Dial(SIP/abc) is only done one at a time, you may have unexpected behavior lurking.
> 
> New way:
>   LOCK(foo)
>   do stuff
>   Dial(SIP/abc,,B(unlock^s^1))
> 
> context unlock {
>   s => {
>     UNLOCK(foo);
>     Return;
>   }
> }
> 
> Now, under no circumstances can this dialplan be run through and execute the Dial unless lock 'foo' is released.
> 
> Obviously this doesn't ensure that you're not calling SIP/abc more than once (you would need more dialplan logic for that), but it will allow a dialplan coder to also put the Dial in the locked section to ensure tighter control.
> 
> 
> This addresses bug ASTERISK-19548.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19548
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 363306 
>   /trunk/apps/app_dial.c 363306 
> 
> Diff: https://reviewboard.asterisk.org/r/1878/diff
> 
> 
> Testing
> -------
> 
> context predial {
>   s => {
>     NoOp(I'm Here! ARGC=${ARGC} ARG1=${ARG1} ARG2=${ARG2});
>     Return;
>   }
> }
> 
>  Dial(SIP/def,,b(predial^s^1(callee^other)))
>    run predial on callee channel with two specified arguments
> 
>  Dial(SIP/def&SIP/ghi&SIP/qrx,,b(predial^s^1(callee^other)))
>   runs predial on all three callee channels with two specified arguments
> 
>  Dial(SIP/def,,B(predial^s^1(CALLER^extra)))
>    runs predial on caller channel with two specified arguments
> 
>  Dial(SIP/def,,B(predial^s^1(CALLER^extra))b(predial^s^1(callee^other)))
>    runs predial on callee channel with two specified arguments and caller channel with two specified arguments
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120427/73370db5/attachment.htm>