[asterisk-dev] [Code Review] 2655: OneTouchRecord: Make so Monitor/MixMonitor can be toggled/started/stopped.

[jrose]

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

Ship it!


Minor grievances aside, this looks fine.


/trunk/bridges/bridge_builtin_features.c
<https://reviewboard.asterisk.org/r/2655/#comment17816>

    start_stop strikes me as a less than self-explanatory name for this.  Maybe consider 'toggle_mode' or 'toggle_behavior'
    
    This comment applies to the structs ast_bridge_features_automonitor and ast_bridge_features_automixmonitor as well.



/trunk/bridges/bridge_builtin_features.c
<https://reviewboard.asterisk.org/r/2655/#comment17815>

    I worry that sending fake confirmation sounds is simply going to lead to confusion and bug reports. There are a number of situations I think that would call for some beeping sound that is more questioning than beep and less critical than beep-err. Something that goes 'ehhh'. beep-huh
    
    But that's not here or now.


- jrose


On July 3, 2013, 5:18 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2655/
> -----------------------------------------------------------
> 
> (Updated July 3, 2013, 5:18 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The OneTouchRecord feature has historically been a toggle.  This patch adds the ability to make the OneTouchRecord hook optionally start/stop recording only.  If OneTouchRecord is already doing what is requested then only the invoker hears the courtesy tone and/or start/stop recording message.
> 
> The new feature is written so it we could easily add explicit start/stop recording DTMF hooks for Monitor and MixMonitor.
> 
> The majority of the changes in bridge_builtin_features.c is a refactoring of the OneTouchRecord code (Monitor and MixMonitor versions) so it is easy to direct the toggle/start/stop functionality.
> 
> 
> Diffs
> -----
> 
>   /trunk/bridges/bridge_builtin_features.c 393531 
>   /trunk/include/asterisk/bridging_features.h 393531 
> 
> Diff: https://reviewboard.asterisk.org/r/2655/diff/
> 
> 
> Testing
> -------
> 
> Tested with the chan_agent replacement functionality project.  I was able to start recording when the agent connected with a caller and I was able to fake starting a recording if it was already started before the agent connected with a caller.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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