[asterisk-dev] [Code Review] 4549: ARI: Add the ability to intercept hold and raise an event

[Matt Jordan]

> On March 31, 2015, 12:23 p.m., Mark Michelson wrote:
> > One thing to take into consideration here is that there are some places within Asterisk where we will send an AST_CONTROL_UNHOLD frame on a channel, even though it may not currently be on hold. This means you may send some unhold ARI events that don't match up with a previous hold event. This is probably worth documenting somewhere so that ARI application writers know what they might have to deal with.
> 
> Matt Jordan wrote:
>     In what circumstances do we do that?
> 
> Mark Michelson wrote:
>     I was thinking of transfer code in particular. The transfer code does not know whether the channel being transferred is on hold or not, but it will send an unhold frame anyway.

Hm. That might actually be a slight issue in the transfer code. The channel structure has a property on it that is ostensibly supposed to track if a channel is on hold or not, ast_channel_hold_state. This maps to the hold_state property on the channel:

        int hold_state;                                                 /*!< Current Hold/Unhold state */

We should be able to look at that to see if the channel is currently on hold before queuing up an unhold frame.

bridge_channel does do this when a channel leaves the bridge:

        /* Complete any active hold before exiting the bridge. */
        if (ast_channel_hold_state(bridge_channel->chan) == AST_CONTROL_HOLD) {
                ast_debug(1, "Channel %s simulating UNHOLD for bridge end.\n",
                        ast_channel_name(bridge_channel->chan));
                ast_indicate(bridge_channel->chan, AST_CONTROL_UNHOLD);
        }

While there are places where the bridging framework sends an unhold to the channel, they appear to all be matched with a hold frame. Which particular place are you thinking of?


- Matt


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


On March 27, 2015, 10:19 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4549/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 10:19 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Bugs: ASTERISK-24922
>     https://issues.asterisk.org/jira/browse/ASTERISK-24922
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> For some applications - such as SLA - a phone pressing hold should not behave in the fashion that the Asterisk core would like it to. Instead, the hold action has some application specific behaviour associated with it - such as disconnecting the channel that initiated the hold; only playing MoH to channels in the bridge if the channels are of a particular type, etc.
> 
> One way of accomplishing this is to use a framehook to intercept the hold/unhold frames, raise an event, and eat the frame. Tasty. The patch attached to this issue accomplished that as a new dialplan function, HOLD_INTERCEPT.
> 
> In addition:
> * ARI now queues hold/unhold frames instead of indicating frames directly. This allows for the Stasis hold/unhold messages to be raised.
> * Some general cleanup of raising hold/unhold Stasis messages was done, including removing some RAII_VAR usage.
> 
> 
> Diffs
> -----
> 
>   /branches/13/rest-api/api-docs/events.json 433677 
>   /branches/13/res/stasis/control.c 433677 
>   /branches/13/res/stasis/app.c 433677 
>   /branches/13/res/ari/ari_model_validators.c 433677 
>   /branches/13/res/ari/ari_model_validators.h 433677 
>   /branches/13/main/stasis_channels.c 433677 
>   /branches/13/main/manager_channels.c 433677 
>   /branches/13/main/channel.c 433677 
>   /branches/13/main/bridge_channel.c 433677 
>   /branches/13/funcs/func_holdintercept.c PRE-CREATION 
>   /branches/13/CHANGES 433677 
> 
> Diff: https://reviewboard.asterisk.org/r/4549/diff/
> 
> 
> Testing
> -------
> 
> See Gerrit reviews:
> 
> https://gerrit.asterisk.org/#/c/16
> https://gerrit.asterisk.org/#/c/17
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

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