[Asterisk-code-review] chan sip: Send stasis message on invite with replaces (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/10448 )

Change subject: chan_sip:  Send stasis message on invite with replaces
......................................................................


Patch Set 1:

(1 comment)

> > Patch Set 1:
 > >
 > > > (2 comments)
 > >  >
 > >  > Eventually a similar patch will need to be done for
 > >  > res_pjsip_refer.c as it will not send the pickup event either.
 > >
 > > Jasper,
 > >
 > > Were you planning on responding to Richard's feedback?
 > >
 > > Thanks,
 > > Matthew Fredrickson
 > 
 > Matthew,
 > 
 > I am having trouble understanding your second comment about the
 > ConfBridge that is not returned in ast_bridge_transfer_acquire_bridge()
 > but is a bridge by the definition of ast_channel_get_bridge().
 > 
 > I see that ast_bridge_transfer_acquire_bridge has an additional
 > check on a masquerade flag that is related to ConfBridge, but am
 > unsure how that is relevant in the handle_invite_replaces context.
 > 
 > Should I wrap the ast_send_call_pickup_stasis_message call with an
 > if statement on the output of ast_channel_get_bridge()?
 > 
 > Regards,
 > Jasper Hafkenscheid

I think I've found a better way to do the pickup since just sending the CEL event does not do several other things things such as connected line updates to affected parties and reporting answered elsewhere to the ringing endpoint.

https://gerrit.asterisk.org/#/c/10448/1/channels/chan_sip.c
File channels/chan_sip.c:

https://gerrit.asterisk.org/#/c/10448/1/channels/chan_sip.c@25686
PS1, Line 25686: 	} else {
               : 		RAII_VAR(struct ast_channel_snapshot *, chan_snapshot, NULL, ao2_cleanup);
               : 		RAII_VAR(struct ast_channel_snapshot *, target_snapshot, NULL, ao2_cleanup);
               : 
               : 		chan_snapshot = ast_channel_snapshot_create(c);
               : 		target_snapshot = ast_channel_snapshot_get_latest(ast_channel_uniqueid(replaces_chan));
               : 
               : 		ast_channel_move(replaces_chan, c);
               : 
               : 		ast_send_call_pickup_stasis_message(replaces_chan, chan_snapshot, target_snapshot);
               : 		ast_hangup(c);
               : 	}
Replacing the marked code (the else clause) with the following should be the complete patch needed to do the pickup:

} else {
	int pickedup;

	ast_channel_lock(replaces_chan);
	pickedup = ast_can_pickup(replaces_chan) && !ast_do_pickup(c, replaces_chan);
	ast_channel_unlock(replaces_chan);
	if (!pickedup) {
		ast_channel_move(replaces_chan, c);
	}
	ast_hangup(c);
}

So you don't need to make send_call_picup_stasis_message() public anymore.



-- 
To view, visit https://gerrit.asterisk.org/10448
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb1442027a3ce6ae55faca47bc095e53972f947a
Gerrit-Change-Number: 10448
Gerrit-PatchSet: 1
Gerrit-Owner: Jasper Hafkenscheid <jasper.hafkenscheid at wearespindle.com>
Gerrit-Reviewer: Jasper Hafkenscheid <jasper.hafkenscheid at wearespindle.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 18:04:47 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181101/4057254d/attachment.html>