[asterisk-dev] [Code Review] 4123: app_agent_pool: Make agent alert interruptable by DTMF.

[rmudgett]

> On Nov. 5, 2014, 11:24 a.m., Mark Michelson wrote:
> > /branches/12/main/bridge_channel.c, lines 2155-2160
> > <https://reviewboard.asterisk.org/r/4123/diff/2/?file=68729#file68729line2155>
> >
> >     It's probably worth printing a warning message if this case occurs. Told to write something but there's nothing in the queue? Weird.

Added warning message: "Weird.  No frame from bridge for %s to process?"
This is in addition to the assertion check that was already there.  FRACK on!


> On Nov. 5, 2014, 11:24 a.m., Mark Michelson wrote:
> > /branches/12/main/bridge_channel.c, line 2365
> > <https://reviewboard.asterisk.org/r/4123/diff/2/?file=68729#file68729line2365>
> >
> >     Should be <= instead of <

Why?  If ms is zero I'll be assigning zero to it.


> On Nov. 5, 2014, 11:24 a.m., Mark Michelson wrote:
> > /branches/12/main/bridge_channel.c, lines 1512-1514
> > <https://reviewboard.asterisk.org/r/4123/diff/2/?file=68729#file68729line1512>
> >
> >     Since this is only a type declaration and intended to provide a compile-time error if the sizes of the arrays are different, could this go someplace else than here? It seems out of place here.
> >     
> >     Also, why ARRAY_LEN instead of sizeof()?

I cannot do it elsewhere.  I need a struct ast_bridge_channel variable and a struct ast_bridge_hook_dtmf variable to reference the appropriate struct members.

I used ARRAY_LEN() to be more explicit that I was checking the number of array elements; not the size of the elements themselves.  Though sizeof would have worked just as well in this case.


- rmudgett


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


On Nov. 3, 2014, 11:31 a.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4123/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2014, 11:31 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24257 and ASTERISK-24447
>     https://issues.asterisk.org/jira/browse/ASTERISK-24257
>     https://issues.asterisk.org/jira/browse/ASTERISK-24447
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch can be broken into two logical changes.  The first is to allow the DTMF feature hooks to collect digits while passing frames from the bridge (ASTERISK-24447).  The secod is to allow an agent pool agent to interrupt the alerting playback file with DTMF (ASTERISK-24257).  The agent interrupting the alerting playback builds on the ASTERISK-24447 part because it knows what digit interrupted the playback and needs to be able to pass that digit to the DTMF hook digit collection code.
> 
> * Made collecting DTMF digits for the DTMF feature hooks not block receiving frames from the bridge. (Changes in bridge_channel.c and bridge_channel.h)
> 
> * Made collecting DTMF digits possible by other bridge hooks if there is a need.
> 
> * Made agent able to interrupt the alerting beep playback with DTMF.  Any digit can interrupt if the call does not need to be acknowledged.  Only the first digit of the acknowledgement can interrupt if the call needs to be acknowledged. (Changes in app_agent_pool.c)
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/bridge_channel.c 427128 
>   /branches/12/include/asterisk/bridge_channel.h 427128 
>   /branches/12/apps/app_agent_pool.c 427128 
> 
> Diff: https://reviewboard.asterisk.org/r/4123/diff/
> 
> 
> Testing
> -------
> 
> With the patch
> 1) Agent can interrupt the beep playback with any digit if an acknowledgement is not required.
> 2) Agent can interrupt the beep playback with the first digit of the configured acknowledgement.  Other digits are ignored.  The acknowledgement could be one or more digits.
> 3) Agent can still wait for the playback to end.
> 
> 4) Collecting DTMF digits for the DTMF feature hooks does not block receiving frames from the bridge.  Tested by using ControlPlayback application through a local channel chain that had other DTMF feature hooks starting with '*'.  When I pressed the '*', the rewinding of the playback was delayed by the digit collection interdigit timeout time but the audio was not discarded during the interdigit timeout time.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141105/b16a9bdd/attachment-0001.html>