[asterisk-dev] [Code Review] 2648: func_channel: set dtmf_features

[jrose]

> On June 28, 2013, 2:37 p.m., Joshua Colp wrote:
> > /trunk/main/bridging_basic.c, line 79
> > <https://reviewboard.asterisk.org/r/2648/diff/1/?file=40549#file40549line79>
> >
> >     I'm torn these days on being tolerant of invalid configurations. In this case the user mistyped or thinks a feature exists that doesn't and the end result behavior will differ from what they expect... if a failure gets passed all the way back up, they'll notice it immediately.
> 
> Matt Jordan wrote:
>     I'd say this should behave the same as Dial or Bridge if you specify an invalid feature letter. If those WARN and continue this should as well; otherwise this should fail out.

Confirmed that they continue on invalid characters, so I'll continue. They don't warn, but I'm going to go ahead and warn anyway. They are application options, so it's not quite as cut and dry as what you should expect for a much more limited and consistent set of options like in this case.

Error messages for failed allocation and characters outside of the uppercase range have been moved into the func_channel part of the code..


> On June 28, 2013, 2:37 p.m., Joshua Colp wrote:
> > /trunk/main/bridging_basic.c, line 78
> > <https://reviewboard.asterisk.org/r/2648/diff/1/?file=40549#file40549line78>
> >
> >     if (raise

fixed


> On June 28, 2013, 2:37 p.m., Joshua Colp wrote:
> > /trunk/main/bridging_basic.c, line 48
> > <https://reviewboard.asterisk.org/r/2648/diff/1/?file=40549#file40549line48>
> >
> >     Misnamed I'd say...

changed to set_feature_flag_from_char, but I don't really think it was misnamed all that much in the first place. It can't really lower them after all, it's make hot only.


- jrose


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


On June 27, 2013, 11:14 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2648/
> -----------------------------------------------------------
> 
> (Updated June 27, 2013, 11:14 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-21876
>     https://issues.asterisk.org/jira/browse/ASTERISK-21876
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Allows setting dtmf features via a channel set function (namely CHANNEL(dtmf_features)
> 
> 
> Diffs
> -----
> 
>   /trunk/funcs/func_channel.c 393075 
>   /trunk/include/asterisk/bridging_basic.h 393075 
>   /trunk/main/bridging_basic.c 393075 
> 
> Diff: https://reviewboard.asterisk.org/r/2648/diff/
> 
> 
> Testing
> -------
> 
> Stuck two channels in the following extension:
> 
> exten => 10,1,NoOp
> exten => 10,n,Answer()
> exten => 10,n,Set(CHANNEL(dtmf_features)=wktx)
> exten => 10,n,Wait(50)
> 
> Then bridged them together with the manager bridge command and checked to see if both channels could park, transfer, and attended transfer.
> 
> It's worth noting that the dial application overrides this datastore entirely, so these don't get applied to dial as-is. This might need to be addressed.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130628/98f4eb9c/attachment.htm>