[asterisk-dev] [Code Review] PreDial - Ability to run dialplan on callee channel and caller channel right before actual Dial

[Russell Bryant]

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


The XML docs in app_dial need to be updated for this new feature.  Also add a line to the CHANGES file.


trunk/apps/app_dial.c
<https://reviewboard.asterisk.org/r/1229/#comment7312>

    make this const



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1229/#comment7314>

    Use size_t instead of int for the buffer lengths.
    
    I would actually really like this all to be ast_str ... but that's just for bonus points.



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1229/#comment7315>

    Using sscanf() is preferred over atoi() to be more explicit about checking for valid input.



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1229/#comment7313>

    Use ast_copy_string() instead of strncpy().
    
    Just FYI, the way this is written doesn't guarantee that these strings will be null terminated.  strncpy() is weird.  Just replacing strncpy with ast_copy_string will fix it up though.



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1229/#comment7316>

    \retval should be used when documenting individual return values like this.  \return is used when writing just a sentence/paragraph that describes the return value(s).



trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/1229/#comment7317>

    Why is this function needed?  Is there anything wrong with just doing this the way the GoSub() option for app_dial works?


- Russell


On 2011-05-24 15:38:57, kobaz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1229/
> -----------------------------------------------------------
> 
> (Updated 2011-05-24 15:38:57)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> PreDial
>   
>   Say SIP/abc is calling SIP/def
>   You have: Dial(SIP/abc)
>   SIP/abc-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 'I' 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,,I(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 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);
>   }
> }
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   trunk/apps/app_dial.c 320708 
>   trunk/include/asterisk/pbx.h 320708 
>   trunk/main/pbx.c 320708 
> 
> Diff: https://reviewboard.asterisk.org/r/1229/diff
> 
> 
> Testing
> -------
> 
> context predial {
>   s => {
>     NoOp(I'm Here!);
>   }
> }
> 
>  Dial(SIP/def,,I(predial,s,1))
>    run predial on callee channel
> 
>  Dial(SIP/def&SIP/ghi&SIP/qrx,,I(predial,s,1))
>   runs predial on all three callee channels
> 
>  Dial(SIP/def,,B(predial,s,1))
>    runs predial on caller channel
> 
>  Dial(SIP/def,,B(predial,s,1)I(predial,s,1))
>    runs predial on callee channel and caller channel
> 
> 
> Thanks,
> 
> kobaz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110527/e818ad5c/attachment-0001.htm>