[asterisk-dev] [Code Review] 2545: res_parking: Generate extensions when parkext is set for a parking lot. Generate extension to dial parker on parked call timeout.

[jrose]

> On June 6, 2013, 4:11 p.m., Mark Michelson wrote:
> > /team/group/bridge_construction/res/parking/parking_bridge_features.c, lines 475-484
> > <https://reviewboard.asterisk.org/r/2545/diff/6/?file=39198#file39198line475>
> >
> >     The way this if is constructed, then if the extension exists and the extension's registrar is the BASE_REGISTRAR, then you will call ast_add_extension2_nolock(). Is this intended? It seems more likely that if the extension already exists that you would not want to try to create it a second time.

Nope, this is absolutely intentional. Nothing comes along afterwards to clean up extensions registered in the park-dial context when we don't need them anymore, so if an extension exists and it was registered by the BASE_REGISTRAR we need to overwrite it. Note that ast_add_extension2 has an overwrite value which is raised for this call.

Strictly speaking though, things won't change a lot here. Really the only thing that can change between one timeout and another for an extension registered by parking in park-dial is the comebackdialtime.

This is essentially what features.c did as well, only it doesn't bother checking for an existing extension which is wrong in my opinion. Extensions registered by other modules should be preserved and not overridden by the parking system.

I've added a comment to the code to clarify this.


> On June 6, 2013, 4:11 p.m., Mark Michelson wrote:
> > /team/group/bridge_construction/res/res_parking.c, lines 947-948
> > <https://reviewboard.asterisk.org/r/2545/diff/6/?file=39202#file39202line947>
> >
> >     This is a very long line to print. Consider either shortening the message or adding one or more embedded newlines.

ast_log(LOG_ERROR, "Extension registration failed. Previously configured lot extensions and can not be safely restored.\n");

I think Matt suggested that I avoid multi-line log messages.


- jrose


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


On June 5, 2013, 8:50 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2545/
> -----------------------------------------------------------
> 
> (Updated June 5, 2013, 8:50 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-21645
>     https://issues.asterisk.org/jira/browse/ASTERISK-21645
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Extension generation and removal for:
> Park a call to the parkext in the parking context
> Retrieve parked calls from the space numbers in the parking context
> Hints for ParkedCall extensions
> 
> Extension generation (but no removal yet) for:
> Dial the peer which parked the call on parking timeout in the park-dial context
> 
> Extension removal for the park-dial extensions will eventually be handled when reloads are implemented. Since they can't reliably be said to belong to
> the parking lot, they are registered to the base registrar for the parking system instead.
> 
> Applying bridge features in the way features.c did (as arguments to dial) is no longer necessary on account of how features are handled now.
> 
> Each parking lot has its own registrar which is 'res_parking/<parking lot name>'
> The registrar used for non-lot-specific extensions is just 'res_parking'
> 
> When a parking lot is destroyed or reloaded, all of its registered extensions are flushed and rebuilt.
> 
> Note: ParkedCall extensions no longer belong to the parked user involved with them unlike how old parking works. This means that if a parking lot is reloaded and the new configuration specifies that the slot the old call was parked at no longer exists, the extension to retrieve that call will go away even though the call is still parked there. This will resolve itself when the parked call times out (provided timeout hasn't been disabled), and manually created ParkedCall extensions can still retrieve that call if the parking space option isn't provided.
> 
> 
> Diffs
> -----
> 
>   /team/group/bridge_construction/CHANGES 390539 
>   /team/group/bridge_construction/include/asterisk/pbx.h 390539 
>   /team/group/bridge_construction/main/pbx.c 390539 
>   /team/group/bridge_construction/res/parking/parking_bridge.c 390539 
>   /team/group/bridge_construction/res/parking/parking_bridge_features.c 390539 
>   /team/group/bridge_construction/res/parking/parking_controller.c 390539 
>   /team/group/bridge_construction/res/parking/parking_devicestate.c PRE-CREATION 
>   /team/group/bridge_construction/res/parking/res_parking.h 390539 
>   /team/group/bridge_construction/res/res_parking.c 390539 
> 
> Diff: https://reviewboard.asterisk.org/r/2545/diff/
> 
> 
> Testing
> -------
> 
> Checked that extensions are generated and destroyed for Park and ParkedCall as expected with multiple parking lots.
> Overlaps are currently allowed. If two parking lots occupy the same space and parkext exclusive is in use, it's possible for one parking lot's extensions to be overwritten in favor of another. When parking lot extensions are overwritten by other parking lot extensions, warnings are issued.
> Checked that extensions are generated and appropriately called in park-dial when a parked call times out.
> Checked that the timeout is set to the parking lot's parkdialtimeout value.
> 
> 
> Thanks,
> 
> jrose
> 
>

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