[asterisk-dev] [Code Review] 4477: app_confbridge (11): file playback blocks dtmf

[Matt Jordan]

> On March 18, 2015, 12:39 p.m., rmudgett wrote:
> > branches/11/apps/app_confbridge.c, line 676
> > <https://reviewboard.asterisk.org/r/4477/diff/2/?file=72633#file72633line676>
> >
> >     Please get out of the habit of assigning in if tests:
> >     
> >     digit = ast_stream_and_wait(...)
> >     if (digit < 0)
> >

As an aside: it is not against our coding guidelines to do this.

Yes, it may be preferable to not do the assignment in the conditional:
* It's usually more readable
* It's usually safer (although a compiler will typically catch it when you mess it up

However, this is a personal stylistic point. People don't have to necessarily agree on this particular issue.


- Matt


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


On March 17, 2015, 1 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4477/
> -----------------------------------------------------------
> 
> (Updated March 17, 2015, 1 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24864
>     https://issues.asterisk.org/jira/browse/ASTERISK-24864
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Attempting to execute DTMF in a confbridge while file playback (prompt, announcement, etc) is occurring is not allowed. You have to wait until the sound file has completed before entering DTMF. This patch fixes it so that app_confbridge now monitors for dtmf key presses during file playback. If a key is pressed playback stops and it executes the matched menu option.
> 
> 
> Diffs
> -----
> 
>   branches/11/apps/confbridge/include/confbridge.h 432991 
>   branches/11/apps/confbridge/conf_state_multi_marked.c 432991 
>   branches/11/apps/app_confbridge.c 432991 
> 
> Diff: https://reviewboard.asterisk.org/r/4477/diff/
> 
> 
> Testing
> -------
> 
> Manual testing done. Setup a basic conference bridge that allowed both regular and admin users to enter. Ran through various menu options to make sure the sound file playback would stop (no longer have to wait) and a new option was executed when appropriate. Also ran the app_confbridge testsuite tests to make sure they still passed.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150318/283530d9/attachment.html>