[asterisk-dev] [Code Review] New Application: Originate

[Russell Bryant]

> On 2008-12-17 13:11:59, Mark Michelson wrote:
> > The code appears to be error-free by my eyes, but I do have a suggestion, though.
> > 
> > I'm wondering if it would be a good idea to use the return value from ast_pbx_outgoing_exten and ast_pbx_outgoing_app in conjunction with the output parameter "res" to play a sound back to the channel running app_originate to indicate if the call was successful. Something like "Originate successful," "Originate failed. Channel was busy," or "Originate failed. Channel could not be requested."
> > 
> > My inspection of the code shows that even though the two function calls are made with the "sync" variable set to 0, ast_pbx_outgoing_exten and ast_pbx_outgoing_app will only return once the channel specified as the first parameter for Originate() has either answered or is determined to be busy or something.
> > 
> > Feel free to reject this idea entirely or modify it in some way, but it seems like right now there's no way for someone originating a call in this manner to know if the originate actually succeeded or not.
> 
> Russell Bryant wrote:
>     This is a great suggestion.
>     
>     I do have one modification.  I'll take a look at what is available as a result code back from those API calls.  Whatever is available, I'd like to expose via ORIGINATE_RESULT channel variables.  That way, from the dialplan, you could play a sound as you suggest, or do whatever else the dialplan author may want to do.
>     
>     Thanks.
> 
> Jared Smith wrote:
>     Would it be worth it to have an additional parameter for being able to set one or more channel variables on the Originated call?  I've often used this for originations come from AMI, and I figured it might be useful here.
>     
>     The syntax is a bit ugly, but I imagine it looking something like:
>     
>     Originate(SIP/george,exten,default,123,1,accountnumber=543212345&agentnumber=424242&requestid=77321672)
>
> 
> Russell Bryant wrote:
>     Jared, I think that's a great suggestion for improvement.  I have added a todo note in the code so that the enhancement gets completed at some point.  I'm not so sure on the syntax, but we'll figure something out.  It could also be something like ...
>     
>     Set(ORIGINATE_VAR=MYVAR=foo)
>     Set(ORIGINATE_VAR=CALLERID(name)=Me)
>     Set(ORIGINATE_VAR=SOMETHING_ELSE=bar)
>     Originate(SIP/1234,exten,default,123,1)
>
> 
> Jared Smith wrote:
>     Wouldn't the second invocation of the Set(ORIGINATE_VAR=CALLERID(name)=Me) wipe out the MYVAR=foo?  I considered doing it with a specially named channel variable, but it gets messy for doing multiple variables.  I might suggest that if we still want to go that route we do something like:
>     
>     Set(ORIGINATE_VAR=MYVAR=foo&CALLERID(name)=Me&SOMETHING_ELSE=bar)
>     Originate(SIP/1234,exten,default,123,1)

You're right.  I typed that out in haste.  I meant to have stuff like ..

Set(ORIGINATE_VAR(MYVAR)=foo)

Each time you ran it, it would store these variables in a special datastore on the ast_channel.


- Russell


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


On 2008-12-17 15:51:01, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/95/
> -----------------------------------------------------------
> 
> (Updated 2008-12-17 15:51:01)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch introduces a new application, Originate.  I made a lot of changes to the original patch, so I figured it would be worth getting another set of eyes on the code before commit.
> 
> 
> This addresses bug 14075.
>     http://bugs.digium.com/view.php?id=14075
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 165329 
>   /trunk/apps/app_originate.c PRE-CREATION 
> 
> Diff: http://reviewboard.digium.com/r/95/diff
> 
> 
> Testing
> -------
> 
> Tested calls to 589 through 595 and got expected results.
> 
> exten => s,1,Answer
> exten => s,n,MusicOnHold()
> 
> exten => 589,1,Answer
> exten => 589,n,Originate(SIP/5004,exten,default,586,1)
> exten => 589,n,Playback(demo-congrats)
> 
> exten => 590,1,Answer
> exten => 590,n,Originate(SIP/5004,exten,default,586)
> exten => 590,n,Playback(demo-congrats)
> 
> exten => 591,1,Answer
> exten => 591,n,Originate(SIP/5004,exten,default)
> exten => 591,n,Playback(demo-congrats)
> 
> exten => 592,1,Answer
> exten => 592,n,Originate(SIP/5004,exten)
> exten => 592,n,Playback(demo-congrats)
> 
> exten => 593,1,Answer
> exten => 593,n,Originate(SIP/5004,app,Playback,demo-congrats)
> exten => 593,n,Playback(demo-congrats)
> 
> exten => 594,1,Answer
> exten => 594,n,Originate(SIP/5004,app,Echo)
> exten => 594,n,Playback(demo-congrats)
> 
> exten => 595,1,Answer
> exten => 595,n,Originate(SIP/5004,app)
> exten => 595,n,Playback(demo-congrats)
> 
> 
> Thanks,
> 
> Russell
> 
>