[asterisk-dev] [Code Review] 2573: res_parking: Recreate the 'Park' manager action in the new parking system

[Matt Jordan]

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



/trunk/include/asterisk/bridging.h
<https://reviewboard.asterisk.org/r/2573/#comment17501>

    Keep this public, but leave it in features.h for now



/trunk/main/bridging.c
<https://reviewboard.asterisk.org/r/2573/#comment17502>

    This BUGBUG should still be with the previous function (I think)



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17505>

    This parameter is optional, so the required attribute should be false



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17503>

    "The channel to be called back if the <replaceable>timeout</replaceable> value specified is reached"



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17504>

    This parameter is optional, so the 'required' attribute should be false



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17506>

    And this is optional as well.



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17507>

    "The parking lot the channel should be parked in"



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17508>

    A description should be more verbose than the synopsis.
    
    Describe:
     * The various options and how they behave
     * The channel variables set by the AMI action
    



/trunk/res/parking/parking_manager.c
<https://reviewboard.asterisk.org/r/2573/#comment17509>

    This bothers me.
    
    Different entry points for parking will have different ways of supplying parameters. AMI will be different than dialplan. AGI would be different as well (and there is a long standing patch to have Park added to AGI). Having AMI, AGI, or other interfaces repack their parameters into dialplan syntax feels hackish.
    
    The parking API should provide a structure that defines the allowed configuration parameters. park_common_setup should take in this structure and configure the channels for parking based on that. It should be up to the various applications to parse out their configuration parameters into the structure and provide it to the common setup routine.


- Matt Jordan


On June 17, 2013, 11:31 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2573/
> -----------------------------------------------------------
> 
> (Updated June 17, 2013, 11:31 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-21641
>     https://issues.asterisk.org/jira/browse/ASTERISK-21641
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch gets the 'Park' manager action back up and working. It supports all the arguments to the old park manager action and seems to work in the same way.
> This patch also fixes an issue with the 'Bridge' manager action where channels yanked from two party bridges would leave the bridge up in spite of there being only one channel in the bridge at that point.
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 392115 
>   /trunk/include/asterisk/bridging.h 392115 
>   /trunk/main/bridging.c 392115 
>   /trunk/main/features.c 392115 
>   /trunk/res/parking/parking_manager.c 392115 
> 
> Diff: https://reviewboard.asterisk.org/r/2573/diff/
> 
> 
> Testing
> -------
> 
> Pulled channels from bridged and unbridged calls with the 'Park' and made sure the channel on the other end got torn down in each case. I did similar stuff with the 'Bridge' command.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130618/f5a9d261/attachment-0001.htm>