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

Dmitry Andrianov dimas at dataart.com
Thu Dec 18 08:37:56 CST 2008


I personally think that equal sign is a bad choice - it looks like double assignment and is a bit misleading.
Depending on what characters are allowed for the variable names it could be:

Set(ORIGINATE_VAR.MYVAR=foo)
Set(ORIGINATE_VAR:MYVAR=foo)
Set(ORIGINATE_VAR/MYVAR=foo)
Set(MYVAR at ORIGINATE_VAR=foo)

Regards,
Dmitry Andrianov

-----Original Message-----
From: asterisk-dev-bounces at lists.digium.com [mailto:asterisk-dev-bounces at lists.digium.com] On Behalf Of Russell Bryant
Sent: Thursday, December 18, 2008 5:26 PM
To: Jared Smith; Russell Bryant; Mark Michelson; Asterisk Developers
Subject: Re: [asterisk-dev] [Code Review] New Application: Originate



> 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)
>

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)


- 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
>
>


_______________________________________________
--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



More information about the asterisk-dev mailing list