<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/2393/">https://reviewboard.asterisk.org/r/2393/</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 18th, 2013, 7:01 p.m. UTC, <b>rmudgett</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/2393/diff/3/?file=34817#file34817line8053" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/features.c</a>
    <span style="font-weight: normal;">

     (Diff revision 3)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <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">8040</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int ast_pickup_call_directed(struct ast_channel *chan)</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;">You might want to look at the Pickup application for finding the exten selects channels in ast_channel_iterator_by_exten_new()/ast_channel_by_exten_cb().  Your target may be in a macro.

Passing in the exten to search for set in the channel does seem a bit awkward for more general use.  Probably should also specify a context or you may get more target matches than intended.</pre>
 </blockquote>



 <p>On March 18th, 2013, 8:36 p.m. UTC, <b>wedhorn</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;">The idea is that the directed_callgroup will be set by (and only by) channel drivers. So to match callstate, callgroup, and exten should preclude any unwanted matches. Still, I&#39;ll have another look over the code.

Agree that passing in the chan with target exten already set is a bit awkward, but given that it&#39;s for developer use, seems reasonable.

I thought about including context (possibly even optionally). Given that they need to be in the same directed_callgroup, and that group pickup allows cross context pickups, thought this approach reasonable. Happy to include context, but would make it optional.</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&#39;ve had more of a dig around and don&#39;t think that a revision of the directed pickup method is warranted. The new function ast_pickup_call_directed is a copy of ast_pickup_call with the same general limitations, but providing an additional parameter to specify the exten to pickup. If there&#39;s an issue with the new method regarding what can be picked up, it is most likely a current bug with ast_pickup_call (probably in ast_can_pickup, which is shared by both ast_pickup_call and the pickup app).

Also I&#39;m having a hard time rationalising adding contexts to the directed pickup. Given that the only calls parsed for exten will be those specifically set to that particular directedCallGroup in the respective channel config (eg sip.conf), it seems redundant to include the pickup context. If the configurer want groups separated, they should be in separate directedCallGroups.

To reiterate, if you could pass in a matchall exten (eg *), ast_pickup_call_directed would pickup the same calls as ast_pickup_call.</pre>
<br />




<p>- wedhorn</p>


<br />
<p>On March 18th, 2013, 3:22 p.m. UTC, wedhorn 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Asterisk Developers.</div>
<div>By wedhorn.</div>


<p style="color: grey;"><i>Updated March 18, 2013, 3:22 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;">Added ast_pickup_call_directed to features. Included some refactoring of ast_pickup_call to reuse same code.

In order to use ast_pickup_call, the chan passed in must have the exten set of the intended target. Will only pickup the target if the exten&#39;s match and the pickupgroupdirected and callgroupdirected of the chans match.

Also add ast_channel_callgroupdirected and ast_channel_pickupgroupdirected functions (and set functions) including code to set both of these in skinny and sip.

Code to actually do a directed pickup included in skinny.</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;">Directed pickup to non ringing skinny and non ringing sip device (with matching and unset groups). Returns without picking up.

Directed pickup to ringing skinny and ringing sip device, with matching groups, picks up call, with unmatching groups return without pickup.</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>/trunk/CHANGES <span style="color: grey">(383304)</span></li>

 <li>/trunk/channels/chan_sip.c <span style="color: grey">(383304)</span></li>

 <li>/trunk/channels/chan_skinny.c <span style="color: grey">(383304)</span></li>

 <li>/trunk/channels/sip/include/sip.h <span style="color: grey">(383304)</span></li>

 <li>/trunk/configs/sip.conf.sample <span style="color: grey">(383304)</span></li>

 <li>/trunk/configs/skinny.conf.sample <span style="color: grey">(383304)</span></li>

 <li>/trunk/include/asterisk/channel.h <span style="color: grey">(383304)</span></li>

 <li>/trunk/include/asterisk/features.h <span style="color: grey">(383304)</span></li>

 <li>/trunk/main/channel_internal_api.c <span style="color: grey">(383304)</span></li>

 <li>/trunk/main/features.c <span style="color: grey">(383304)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/2393/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>