[asterisk-dev] [Code Review] 2393: Add directed pickup to features

[wedhorn]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2393/
-----------------------------------------------------------

(Updated May 23, 2013, 9:47 p.m.)


Review request for Asterisk Developers.


Changes
-------

Okay, another patch. This is a fairly comprehensive refactor to minimise the code changes but to include all (and actually more) functionality. The reason I've done this is to try and get my point across that this is a fairly small addition to the existing ast_pickup_call function.

The difference between this and the previous patch are:
 - chanvar used to identify exten;
 - code fully embedded in ast_pickup_call rather than duplicating functions;
 - callgroup is checked against pickupgroup and dirpickupgroup; and
 - additional named directed pickup group variable.

I've pondered running it through the dialplan, and sure it can be done, but it really doesn't change much of the channel code (apart from doing a dial rather than calling ast_pickup_call). I can't envisage a reasonable method of how to determine whether an extension can pickup another extension without using the callgroup type parameters. Further, I think that the pickupgroup is likely to be too limiting in may circumstances. An example may be that a device's pickupgroup is the immediately surrounding (physically located) callgroups but the device can directed pickup all devices within earshot. While this is doable in a dialplan, it would likely be long/convoluted and difficult to maintain. Including this in the device configuration is obviously easier because  physically moving the device would only entail a fixup in the devices configuration (ie change callgroup, pickupgroup and dirpickupgroup).

Maybe there's is a way to do this reasonably in the dialplan, and if so, here's a relatively simple example. 20 callgroups spread out over 4 floors of a building. Circular layout for simplicity. Each group can pickup calls in it own callgroup and those located physically adjacent. So 5 callgroups on a floor, group 1 can pickup 5,1 and 2. Group 2 can pickup 1,2,3 etc. Devices can directed pickup any device on the floor. With this patch the device configuration would be:
callgroup 1
pickupgroupt 5,1,2
dirpickupgroup 1-5.

Now for the spanner, once an extension has been given to that person, they retain the extension as they phsically move from area to area. As such, the extensions within each callgroup are fairly random. What reasonable way is there to do this sort of matching in the dialplan with keeping a db of all the various groupings. Why would such an approach be better than simply adding another pickupgroup to the device configuration?

I have an issue with using a chanvar to provide the exten to pickup because there's going to be a lot of iterations through the chanvars in a busy environment through the way the pickup callback works. This could be remedied by adding a dedicated variable to the chan structure, or as previously done, having an explicit directed_pickup function and setting the chan->exten. I've used the chanvar for simplicity in this patch in an attempt to highligh that functionally this is a small addition to the existing ast_pickup_call arrangements.


Repository: Asterisk


Description
-------

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'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.


Diffs (updated)
-----

  /trunk/channels/chan_skinny.c 389422 
  /trunk/include/asterisk/channel.h 389422 
  /trunk/main/channel_internal_api.c 389422 
  /trunk/main/features.c 389422 

Diff: https://reviewboard.asterisk.org/r/2393/diff/


Testing
-------

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.


Thanks,

wedhorn

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130523/150db36b/attachment.htm>