[asterisk-dev] [Code Review]: Improvements to Asterisk parking (Repost of review 963)

[Mitch Sharp]

> On Jan. 17, 2012, 10:11 a.m., Mark Michelson wrote:
> >

After writing this patch it was brought to my attention that the majority of it could already be accomplished via the dial plan, although it does take some playing to realize exactly how it work. The options from this patch would make these features easier to manage and understand I believe.

With the changes in 10 and trunk, mohclass is now a per-parkinglot option and the park-dial context issue was fixed. It would be nice to have comebacktoorigin be a per-parkinglot option as well, as well as a few of the other featuers added.

Below is a link to the dialplan equivalent:
http://files.bluecrow.net/asterisk/patches/asterisk-10.0.0-park-features-comebackcontext-via-dialplan-workaround.txt

Last time, there didn't seem like there was any interest in these features so I didn't pursue it and have since stopped using the patch on my production systems so I didn't have to go through the headache of updating the patch for each release. I would be glad to bring it current if it will actually be used... if not I won't waste my time.


> On Jan. 17, 2012, 10:11 a.m., Mark Michelson wrote:
> > /trunk/main/features.c, line 4641
> > <https://reviewboard.asterisk.org/r/1674/diff/1/?file=23203#file23203line4641>
> >
> >     This was from Mitch's original patch. It seems like this is a bit backwards though. This is setting PARKER to the name of the channel being parked, as opposed to the one doing the parking. Is this intentional?

Nope not intentional... complete oversight... should be PARKEE or PARKED_DEVICE or UNPARKED_DEVICE or something more appropriate.


- Mitch


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


On Jan. 17, 2012, 10:09 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1674/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2012, 10:09 a.m.)
> 
> 
> Review request for Asterisk Developers, rmudgett and Mitch Sharp.
> 
> 
> Summary
> -------
> 
> Many moons ago, Mitch Sharp posted some updates for Asterisk parking (https://reviewboard.asterisk.org/r/963/). Here is the copy and pasted text from that review description:
> ------
> I posted a patch to issues a few weeks ago that duplicated some existing features.  (https://issues.asterisk.org/view.php?id=17947)
> 
> I have reworked it to extend the existing parking behavior.
> 
> This patch does the following:
> - Make comebacktoorigin a per-parkinglot option.
> - Make parkedmusicclass a per-parkinglot option, and make sure parkinglot->parking_con_dial gets set to "park-dial" for additional parking lots instead of being left NULL (already have a patch in issues for this for as a fix for 1.8.0-beta4, https://issues.asterisk.org/view.php?id=17946)
> - Adds a per-parkinglot option comebackcontext, which defaults to the usual 'parkedcallstimeout' context.  Allows the user to handle timed out calls for different parking lots in different contexts.
> - Adds a per-parkinglot option comebackdialtime, which allows user to override the dial time specified in automatically created extensions when comebacktoorigin=yes.
> - Adds channel variable PARKER to timed out calls, so user can easily dial the original device that parked the call if comebacktoorigin=no.
> - Update CLI command 'features show' to include the comebacktoorigin, comebackcontext and comebackdialtime values.
> - Updated /configs/features.conf.sample
> 
> This patch doesn't change any default behavior.
> 
> Looking forward to some feedback.
> ------
> 
> It got some review feedback, but there was never a new version of the patch posted. I've taken the original patch and updated it to apply to Asterisk trunk as it is now.
> 
> As far as I can tell, I've got the patch behaving exactly how it was as originally written. There is a spot that seems odd to me, and I'll highlight it in the diff.
> 
> I've included Richard in this review since he knows parking a lot better than most people do at the moment since he did some refactoring of it recently. 
> 
> 
> This addresses bug ASTERISK-16643.
>     https://issues.asterisk.org/jira/browse/ASTERISK-16643
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/features.conf.sample 350960 
>   /trunk/main/features.c 350960 
> 
> Diff: https://reviewboard.asterisk.org/r/1674/diff
> 
> 
> Testing
> -------
> 
> I have set up parking lots and ensured that the new options take effect when set and that the defaults are used when not set.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120118/0785ed1b/attachment.htm>