[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/#review258
-----------------------------------------------------------


My only comment is that I would like to see this diff updated to completely remove AST_PBX_KEEPALIVE.  With the change from review 98, you should be able to accomplish that.

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