<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/1825/">https://reviewboard.asterisk.org/r/1825/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 22nd, 2012, 10:46 a.m., <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1825/diff/1/?file=26499#file26499line432" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/include/asterisk/utils.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int ast_pthread_create_detached_stack(pthread_t *thread, pthread_attr_t *attr, void*(*start_routine)(void *),</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">432</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> \brief s String within which to replace characters</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">\param, not \brief</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ah, thanks. Fixed.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 22nd, 2012, 10:46 a.m., <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1825/diff/1/?file=26500#file26500line7707" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/features.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int bridge_exec(struct ast_channel *chan, const char *data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7707</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_goto_if_exists</span><span class="p">(</span><span class="n">final_dest_chan</span><span class="p">,</span> <span class="n">caller_context</span><span class="p">,</span> <span class="n">caller_extension</span><span class="p">,</span> <span class="n">caller_priority</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7708</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* Then perform the goto */</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7709</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">ast_parseable_goto</span><span class="p">(</span><span class="n">final_dest_chan</span><span class="p">,</span> <span class="n">opt_args</span><span class="p">[</span><span class="n">OPT_ARG_CALLEE_GO_ON</span><span class="p">])</span> <span class="o">==</span> <span class="n">AST_PBX_SUCCESS</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Doing this is redundant. Both ast_goto_if_exists() and ast_parseable_goto() do the same thing. The difference is that ast_goto_if_exists() takes a separated context, exten, and priority whereas ast_parseable_goto() takes a comma-separated string.
Take a look at how the F() option is implemented in app_dial().</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I should fill you in on why I'm doing this. It's not really redundant, the first goto just reorients the channel. I was wondering how appropriate it was to handle that this way though.
Because this is a bridge and not a dial or a queue, the channel being connected to doesn't have the same starting point as the channel that called bridge (when a channel is created with dial or queue, it does have the same exten/context/priority). Instead, its context, extension, and priority are all set to whatever it was before hand. The purpose of using goto_if_exists before using ast_parseable goto here is to set the starting point to be the same as the channel that called the bridge. The alternative would be to check how many arguments there were to OPT_ARG_CALLEE_GO_ON and then manually sort out what is specified and add the leftovers that weren't specified. This is much easier and probably faster as well.</pre>
<br />
<p>- jrose</p>
<br />
<p>On March 22nd, 2012, 9:37 a.m., jrose 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, Mark Michelson and Matt Jordan.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated March 22, 2012, 9:37 a.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;">Similar to the F option added to queue, this adds a new option to the bridge application to transfer the party that gets bridged upon (awkwardly worded maybe) to a position in the dialplan specified by the bridge F option from the perspective of the channel that bridged.
Also moves a shared function from a couple of places for app_dial and app_queue into utils.</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;">A sip phone would dial the following extension:
exten => 045,1,NoOp(BindBape)
exten => 045,2,Answer()
exten => 045,3,Set(GLOBAL(CHANBAPE)=${CHANNEL})
exten => 045,4,Playback(tt-weasels)
exten => 045,5,Wait(30)
exten => 045,6,HangUp()
>From here, a second phone would dial any of these extensions:
exten => 046,1,NoOp(BrideBape)
exten => 046,2,Answer()
exten => 046,3,Bridge(${CHANBAPE}, F(5))
exten => 046,4,Playback(tt-monkeys)
exten => 046,5,NoOp(Finish!)
exten => 046,6,NoOp(Finish More!)
exten => 047,1,NoOp(BridgeBape2)
exten => 047,2,Answer()
exten => 047,3,Bridge(${CHANBAPE}, F)
exten => 047,4,Playback(tt-allbusy)
exten => 047,5,NoOp(BridgeBape2 finish)
exten => 048,1,NoOp(BridgeBape3)
exten => 048,2,Answer()
exten => 048,3,Bridge(${CHANBAPE}, F(049,4))
exten => 048,4,NoOp(Failure)
exten => 050,1,NoOp(BrigeBape4)
exten => 050,2,Answer()
exten => 050,3,Bridge(${CHANBAPE}, F(bridgetestcontext,20,1))
Also this stuff:
exten => 049,1,NoOp(Failure)
exten => 049,2,NoOp(Failure)
exten => 049,3,NoOp(Failure)
exten => 049,4,Playback(tt-weasels)
exten => 049,5,NoOp(BridgeBape3 Success)
[bridgetestcontext]
exten => 20,1,NoOp(Successful bape4)
exten => 20,n,Playback(tt-somethingwrong)
For each of these cases, I just checked to make sure that hanging up the bridger resulted in the bridged party getting sent to the expected position.
There will be automated tests added for bridging in general as well as for this feature into testsuite soon as well.</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-19282">ASTERISK-19282</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/apps/app_dial.c <span style="color: grey">(360188)</span></li>
<li>/trunk/apps/app_queue.c <span style="color: grey">(360188)</span></li>
<li>/trunk/include/asterisk/utils.h <span style="color: grey">(360188)</span></li>
<li>/trunk/main/features.c <span style="color: grey">(360188)</span></li>
<li>/trunk/main/utils.c <span style="color: grey">(360188)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1825/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>