<p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/9558">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/9558/2//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9558/2//COMMIT_MSG@21">Patch Set #2, Line 21:</a> <code style="font-family:monospace,monospace">This patch suppress the connected line update on attended transfer</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">s/suppress/suppresses/</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9558/2//COMMIT_MSG@23">Patch Set #2, Line 23:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">And also suppress the connected line update on "DTMF Swap"<br>between the target and the transferee.<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Suppressing the connected line updates when Alice swaps between Bob and Charlie is wrong. Alice must get the updates when switching between Bob and Charlie. As a consequence Bob and Charlie must also get updates even if they *seem* redundant. Any connected line interception routines you setup could change them however you want including to what was sent earlier to the channel driver. Truly redundant updates are blocked before being passed to the channel driver.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I was pointing out in my earlier comment that when Alice switches between Bob and Charlie your lie to Charlie would get erased. Your lie to Charlie can *only* be done for the *initial* connection between Alice and Charlie. Any subsequent connected line updates cannot be suppressed.</p><p style="white-space: pre-wrap; word-wrap: break-word;"><br>You asked why there is a difference between blind transfers and attended transfers. In the blind transfer case Alice tells Bob to call Charlie directly. With a blind transfer Alice has nothing to do with actually calling Charlie so your dialplan works as you want. In the attended transfer case Alice calls Charlie then transfers Charlie to Bob. When Alice completes transferring Charlie to Bob a connected line update *must* be generated because Charlie was talking to Alice and is now talking with Bob. Your dialplan does not work as you want in this case because it cannot handle *any* connected line updates. Your dialplan is fragile.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/9558/2/main/bridge_basic.c">File main/bridge_basic.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9558/2/main/bridge_basic.c@3433">Patch Set #2, Line 3433:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> /* We increase the refcount of the transfer target because ast_bridge_impart() will<br> * steal the reference we already have. We need to keep a reference, so the only<br> * choice is to give it a bump<br> */<br> ast_channel_ref(props->transfer_target);<br> if (ast_bridge_impart(props->target_bridge, props->transfer_target, NULL, NULL,<br> AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {<br> ast_log(LOG_ERROR, "Channel %s: Unable to place transfer target into bridge.\n",<br> ast_channel_name(bridge_channel->chan));<br> stream_failsound(props->transferer);<br> ast_bridge_channel_write_unhold(bridge_channel);<br> ast_hangup(props->transfer_target);<br> props->transfer_target = NULL;<br> attended_transfer_properties_shutdown(props);<br> return 0;<br> }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">You need to redo this patch to *only* suppress the *initial* connected line update. To suppress that connected line update we need to get Alice into the target bridge before the local channel connected to Charlie joins the target bridge with the AST_BRIDGE_IMPART_INHIBIT_JOIN_COLP flag.</p><ul><li>This bridge impart needs to be moved to attended_transfer_monitor_thread().</li><li>The impart needs to be done after Alice is moved to the target bridge with bridge_move() in attended_transfer_monitor_thread().</li><li>The impart needs to be done with the AST_BRIDGE_IMPART_INHIBIT_JOIN_COLP flag added.</li><li>bridge_move() needs to be modified to not call bridge_do_move if the passed in channel is already in the destination bridge.</li><li>Some off-nominal code needs to be fixed-up because of the relocated code.</li></ul></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/9558">change 9558</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/9558"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I346652661949b6611c23e431ede0dbea1be3017a </div>
<div style="display:none"> Gerrit-Change-Number: 9558 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 01 Aug 2018 21:10:09 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>