<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2963/">https://reviewboard.asterisk.org/r/2963/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The code itself is good.
The problem I'm having with this change is that the "uri_dialplan" option needs both a better name and a better explanation. The big problem I have with the name is that the dialplan isn't involved at all. The Dial or Queue applications get involved by handling the redirection at that level of code. The distinction between uri_dialplan and uri_pjsip is really hard to explain to a typical Asterisk user. Given that the distinction is hard to make, it may be worth documenting why you would want to use uri_dialplan instead of uri_pjsip. The biggest reason I can see to let control go back to Dial() is so that channel redirecting information will be set on the new outbound channel. Other than that, it appears to be a lot of the same operations that were performed on the original outgoing channel (surprisingly, predial subroutines don't get run on call forward channels).
My big problem is that I don't really have a good suggestion for a new name. Perhaps "uri_asterisk" or "uri_core" to indicate that it's not being handled by PJSIP?</pre>
<br />
<p>- Mark</p>
<br />
<p>On October 26th, 2013, 1:47 p.m. UTC, Joshua Colp wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Joshua Colp.</div>
<p style="color: grey;"><i>Updated Oct. 26, 2013, 1:47 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">chan_pjsip currently supports only one method for handling redirects: It takes the user portion of the target and places it into the call forwarding target as a local extension. This is fine for calling end-user devices but is not suitable for some situations involving other SIP servers (*cough* Microsoft Lync *cough*). The attached patch makes the behavior configurable and adds two other options: "uri_dialplan" and "uri_pjsip".
The uri_dialplan option returns the URI as the call forwarding target and instructs the dial process to dial it using the original endpoint. This is the equivalent of the "promiscredir" option in chan_sip.
The uri_pjsip option handles the redirect completely within chan_pjsip itself. This allows multiple targets to be tried if need be, and also reduces the amount of work the core has to do (no channel teardown and dialing again, the same channel is used).
As all of these may be useful for people and implementing them is relatively easy I've done so.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Placed calls to a target with each option, confirmed that they work as expected.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/12/include/asterisk/res_pjsip.h <span style="color: grey">(402063)</span></li>
<li>/branches/12/res/res_pjsip.c <span style="color: grey">(402063)</span></li>
<li>/branches/12/res/res_pjsip/pjsip_configuration.c <span style="color: grey">(402063)</span></li>
<li>/branches/12/res/res_pjsip_session.c <span style="color: grey">(402063)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2963/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>