[asterisk-dev] [Code Review] Converting the code to use masq_park to do call parking

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/29/#review281
-----------------------------------------------------------


I think you have already committed this.  If so, please edit the commit message to include the following text, so that it will be recorded in svn history, and also have this review request closed out.

Review: http://reviewboard.digium.com/r/29/

- Russell


On 2008-12-17 13:18:55, Steve Murphy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/29/
> -----------------------------------------------------------
> 
> (Updated 2008-12-17 13:18:55)
> 
> 
> Review request for Asterisk Developers, Russell Bryant, Joshua Colp, Mark Michelson, and Jeff Peeler.
> 
> 
> Summary
> -------
> 
> this diff is against 1.4; My notes on the bug (13820) are:
> 
> ...So, Russell advised using the masq_park_call stuff instead of park_call
> stuff. which I did, and entirely got rid of the KEEPALIVE and NO_HANGUP_PEER
> stuff in the res_features.c and app_dial and app_queue code, simplifying things
> and making them more crash-resistant.
> 
> 
> Testing:
> 
> I noted that there are more than one way to park a channel:
> 
> I.   Request a park via the manager interface: this uses the masq_park_call
>      facility already (which is good).
> II.  Via blind xfer feature to parking exten) -- this I modified to
>      use masq_park_call_announce().
> III. Via *3 (one-touch park feature)... this used masq_park_call_announce
>      in one case and the park_call facility in the other. Now, it just does the
>      masq_park_call thing.
> IV . Via the Park() app (for instance, DAHDI hookflash, dial 700)... 
>      This calls park_call_exec() I did not update this to use the
>      masq_park_call facility, because I wasn't certain that this would
>      be of benefit. I'll research it out and see.
> 
> There are call sequences that are good to test:
> 
> 1. A calls B, A Parks B, B hangs up while A is getting parking stall announcement.
> 2.           "         , B hangs up after A gets announcemnt, but before parking expires.
> 3.           "         , B waits, and A is redialed, rebridged
> 4.           "         , B is picked up by C
> 5. A calls B, B parks A, A hangs up while B is getting parking stall announcement.
> 6.           "         , A hangs up after B gets announcemnt, but before parking expires.
> 7.           "         , A waits, and B is redialed, rebridged
> 8.           "         , A is picked up by C
> 
> ===========
> Differences:
> ===========
> 
> The major difference is the channel name during the hangup extension execution.
> 
> In the one-touch, blind xfer to parking exten, and Park() call, (II, III, and IV)
> cases, I note that the channel name ( ${CHANNEL} )  is set to:
> 
> A Parks B: DAHDI/1-1  (where A is DAHDI/1) which is good.
> 
> B Parks A: Parked/DAHDI/1-1<ZOMBIE>   (where B was DAHDI/2)
> 
> This isn't ideal; I think that most users would love it if the hangup for B where not
> celebrated with a h-exten execution at all; and rather, that the h-exten would run when
> channel 1 actually hung up in this case.
> 
> Any ideas?
> 
> Also, I did an experiment with using the masq_park_call in the park_call_exec()
> for the Park() app. It made no difference, so I decided to not add a layer or
> function call to the pile, and left it alone.
> 
> 
> This addresses bug http://bugs.digium.com/view.php?id=13820.
>     http://bugs.digium.com/view.php?id=http://bugs.digium.com/view.php?id=13820
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/apps/app_dial.c 165179 
>   /branches/1.4/apps/app_queue.c 165179 
>   /branches/1.4/include/asterisk/pbx.h 165179 
>   /branches/1.4/res/res_features.c 165179 
> 
> Diff: http://reviewboard.digium.com/r/29/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steve
> 
>