[asterisk-dev] [Code Review] 3537: chan_sip+CEL: Add missing ANSWER and PICKUP events to INVITE/w/replaces pickup (for asterisk 12)

[Mark Michelson]

> On May 22, 2014, 2:12 p.m., Matt Jordan wrote:
> > /branches/12/channels/chan_sip.c, lines 25146-25151
> > <https://reviewboard.asterisk.org/r/3537/diff/1/?file=58434#file58434line25146>
> >
> >     Not a finding, just a thought for your testing.
> >     
> >     Under the hood, ast_do_pickup will eventually call ast_channel_move on the same two channels. From the perspective of "moving channels around", this ends up being the same.
> >     
> >     What is interesting about this change is that instead of just moving the channels around, we will now interpret _all_ INVITE w/ replaces that target a channel not in a bridge as a pickup attempt. That may be the correct interpretation - in which case, this patch should be fine. If that isn't true, then we'd have to have some other way to determine the intent of the INVITE w/ replaces.

The only time I could foresee this situation not being a call pickup would be if Alice places a call to Bob, and Carol sends an INVITE to replace Alice. In that scenario, Carol is not picking up the call but is instead taking Alice's place on the call to Bob.

Probably the best way to handle this is
if (bridge) {
    /* Do what's already in your patch */
} else if (p->outgoing_call) {
    /* Do the pickup as you have it in the patch */
} else {
    /* Do an ast_channel_move as it was prior to your patch */
}

That would at least make behavior correct in the odd case; however, I'm not sure if there would be other missing events if something like that were done.


- Mark


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


On May 13, 2014, 3:10 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3537/
> -----------------------------------------------------------
> 
> (Updated May 13, 2014, 3:10 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22977
>     https://issues.asterisk.org/jira/browse/ASTERISK-22977
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The fixes from https://reviewboard.asterisk.org/r/3073/ (11r413838) won't work in Asterisk 12.
> 
> This is a quick stab at making it work.
> 
> BEWARE! This is completely untested. Preliminary feedback is welcome.
> This will have to sit here a while until I find some time to properly test how this pans out in 12.
> 
> 
> Diffs
> -----
> 
>   /branches/12/channels/chan_sip.c 413840 
> 
> Diff: https://reviewboard.asterisk.org/r/3537/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140529/66cb217a/attachment.html>