<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/1515/">https://reviewboard.asterisk.org/r/1515/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On , <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">First, submit the patch through our bug tracker to make sure we have a proper license agreement. Also, please fill in the form of the first page here in review board.
Secondly, I can't make sense of having a "sip connect" setting. I would like to propose that you separate the path work, which we already have a patch for, from the gin work. Some of your changes of id's should be handled by the dial plan (adding a + can be done there for e.164 phone numbers). Some of them may not be doable and we need to discuss it.
So instead of a big patch, please break it up and let us review smaller patches.
I can not accept this as part of asterisk as it stands, but really want to see the functionality in asterisk. Let's continue working on this. Thank you very much for contributing this code!
/O</pre>
</blockquote>
<p>On November 10th, 2011, 8:19 a.m., <b>Andrew Olmsted</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi Olle,
I worked on this patch with Neeharika and appreciate your comments. We are currently testing e.164 in the dial plan and if this is successful I will take it out of the patch.
For the RFC 6140 changes for GIN support do you have a recommendation on being able to turn this on? We are considering a RCF6140 flag on a per-peer basis, but you said you didn't see the point to a sip connect setting.
Taking e.164 out of the patch will already make it much smaller, and doing the path work with the previously submitted patch will make it smaller still. Hopefully reducing the patch this much will make it easier to review. Should we resubmit the patch here after implementing your recommendations?
Thanks,
Andrew</pre>
</blockquote>
<p>On November 10th, 2011, 8:32 a.m., <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Andrew - can you please verify that the Path: header support is written by you and not the patch we already have in the issue tracker? licensing is important for us. Thanks.
I would propose we do take the code in bit by bit, starting with the Path code.
/O</pre>
</blockquote>
<p>On November 10th, 2011, 8:35 a.m., <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In regards of a SIP connect setting - no. We need to refer to standards. And we need to be able to configure a peer's E.164 phone numbers to do this right. We have no such configuration today, as everything is in the dial plan. What happens if we place a call from the dial plan to a number not associated with the GIN registration? Should it be refused by the SIP channel?
Many architecture things to discuss here.</pre>
</blockquote>
<p>On November 10th, 2011, 8:44 a.m., <b>Andrew Olmsted</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Olle; the path code in this patch was written by me. I was working against 1.8.0 for a reference implementation at the time and was not aware that there was already a patch in trunk. I will take a look at the patch in trunk to see what duplication exists; is this the patch you were referring to: https://reviewboard.asterisk.org/r/991/ (from schmidts later in this thread)?</pre>
</blockquote>
<p>On November 10th, 2011, 8:53 a.m., <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ok. I just wanted to be very clear on the license. THanks for confirming. Yes, the patch in 991 is the one I referred to. It's waiting for an update or a replacement. If you can extract a similar patch for just Path and add it as a separate issue and review that's a good first step. Nothing is in trunk yet.</pre>
</blockquote>
<p>On November 10th, 2011, 10:19 a.m., <b>Andrew Olmsted</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm, I am a bit confused looking at patch 991 because the copy_route and build_path are exactly what I have implemented down to the comments. These functions definitely exist as present in this patch in my source tree and I cannot fathom how they came to be there if not be me. I did the majority of the coding in January of this year so it isn't entirely fresh in my mind.
The only thing I can think of is I was searching for a function to do this and came across patch 991. I really don't remember this but it is the only thing that makes sense. Sorry for the confusion, but I cannot claim ownership of this section of code.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ok. Great to have that cleared up. I also suffer from memory loss. Let's work on 991 together and get that stuff committed, since nothing has happened since my review.
You can reach me on oej (at) edvina.net as email if you want to discuss. Over e-mail we can exchange chat/skype handles.
/O</pre>
<br />
<p>- Olle E</p>
<br />
<p>On October 18th, 2011, 3:11 p.m., Neeharika Allanki wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Neeharika Allanki.</div>
<p style="color: grey;"><i>Updated Oct. 18, 2011, 3:11 p.m.</i></p>
<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;">This Chan-SIP patch brings Asterisk into compliance with the SIPconnect1.1. SIPconnect1.1 is a newly released SIP Forum specification that describes a common set of signaling and media interworking procedures for the SIP Trunk interface between a SIP-based IP-PBX and a SIP-enabled Service Provider network. This patch, coupled with specific Asterisk configuration settings, will enable Asterisk to comply with the normative SIP-PBX requirements specified in SIPconnect1.1.
The patch diff listings being submitted are against Asterisk version 1.8.11.The patch itself has been tested against the 1.8.0 version of Asterisk for the following SIPconnect1.1 functions/capabilities:
Security
-TLS
-SIP Digest
Registration (RFC 6140)
-Basic GIN registration
-did not test the GIN interactions with the GRUU and reg-event package extensions)
Calling features
-Basic DID/DOD calls
-Calling name/number delivery with and without privacy
-Early media
-Call Forwarding
-Call Transfer (attended and blind)
-Emergency calls
-DTMF relay</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-18705">ASTERISK-18705</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/channels/chan_sip.c <span style="color: grey">(333472)</span></li>
<li>/trunk/channels/sip/include/sip.h <span style="color: grey">(333472)</span></li>
<li>/trunk/configs/sip.conf.sample <span style="color: grey">(333472)</span></li>
<li>/trunk/include/asterisk/strings.h <span style="color: grey">(333472)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1515/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>