<p> Attention is currently required from: Joshua Colp, Mark Petersen. </p>
<p>Patch set 5:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/16344">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</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/asterisk/+/16344?tab=comments">Patch Set #5:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">Just thought of something else...  What happens if you're using ARI to make the call?  That doesn't go through app_dial.<br></p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File apps/app_dial.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/asterisk/+/16344/comment/6e49c13e_7dcb9d6a">Patch Set #5, Line 2873:</a> <code style="font-family:monospace,monospace">                     cancelcause = ast_channel_hangupcause(chan);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">wait_for_answer is pretty complicated.  Are you sure just using peer and to is enough to positively indicate that the caller hung up?  You may want to do the determination in wait_for_answer and add a field to "pa" with the hangup cause.  There's already code in there that deals with the caller hanging up.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16344/comment/228c33c3_87a7cab0">Patch Set #5, Line 3360:</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;">ast_channel_hangupcause(chan) == AST_CAUSE_ANSWERED_ELSEWHERE<br>           || ast_test_flag64(&opts, OPT_CANCEL_ELSEWHERE)<br>           ? AST_CAUSE_ANSWERED_ELSEWHERE : cancelcause<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I know it was like this before but i had a hard time deciphering what this was doing.  Think you could move this before the hanguptree call and make it a bit clearer?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_rfc3326.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/asterisk/+/16344/comment/858982fc_6d8b932d">Patch Set #5, Line 80:</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;">       if (ast_channel_hangupcause(session->channel) == AST_CAUSE_NOTDEFINED &&<br>           !pjsip_method_cmp(&rdata->msg_info.msg->line.req.method, &pjsip_cancel_method)) {<br>                       ast_debug(1, "Channel '%s', setting hangup cause: %d\n", ast_channel_name(session->channel), AST_CAUSE_NORMAL_CLEARING);<br>                 ast_channel_hangupcause_set(session->channel, AST_CAUSE_NORMAL_CLEARING);<br>  }<br><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is this even still needed with the logic in Dial?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16344">change 16344</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/c/asterisk/+/16344"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: Ib653aec2282f55b59d87484391cc07c8e6612b89 </div>
<div style="display:none"> Gerrit-Change-Number: 16344 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Mark Petersen <bugs.digium.com@zombie.dk> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Mark Petersen <bugs.digium.com@zombie.dk> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 05 Oct 2021 13:54:55 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>