[Home]

Summary:ASTERISK-17424: [patch] ParkedCall() does not update Connected Line information
Reporter:Philippe Lindheimer (p_lindheimer)Labels:
Date Opened:2011-02-17 10:27:09.000-0600Date Closed:2011-08-26 13:27:30
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Features/Parking
Versions:1.8.2 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) features.patch
Description:When picking up a parked call, neither party has their connected line information updated. The expected behavior is that upon picking up the parked call from the parking slot, the two sides of the bridge would see each other's CID information.

****** ADDITIONAL INFORMATION ******

Patch being attached with fix for this bug.
Comments:By: Richard Mudgett (rmudgett) 2011-02-17 12:20:56.000-0600

From just looking at this patch:
1) There is no need to ast_strdupa() pu_name and chan_name since they are only used for the debug message.  Just reference them there since you have both channels locked.

2) Running a macro can take awhile so you may need to pass in the other channel so it can go into autoservice.  However, you cannot have the channels locked at that time.

3) The interception macros probably need to be run in both directions.

By: Philippe Lindheimer (p_lindheimer) 2011-02-17 12:28:48.000-0600

Thanks for the feedback. As my Asterisk coding is pretty 'blind' I'm sure there is room for improvement.

1) I thought that may be the case, I just copied this code from the call Pickup() app in the same file which did that (but maybe was using those values for something else).

2) I'll leave this to whom ever get's assigned the bug as that is beyond my understanding of the code. If you want to show what needs to be changed to do that, I will be very happy to update my patch so that it gets tested.

3) I don't understand what this means. In case it refers to the need to update the connected line info to BOTH channels, that is being done from my testing. If something else, please provide an example and again, I'll be happy to further test.



By: Richard Mudgett (rmudgett) 2011-02-17 12:57:35.000-0600

regarding above:

2) Using ast_connected_line_copy_from_caller() to perform a deep copy to allow you to unlock the channels before invoking the interception macro.
General outline of steps:
2a) ast_party_connected_line_init()
2b) ast_connected_line_copy_from_caller()
2c) unlock channels
2d) ast_channel_connected_line_macro()
2e) ast_party_connected_line_free()

3) The interception macro probably needs to be run on both channels because you are sending connected line information to both channels.  Each channel could define its own interception macro.

Note that there are caller/callee connected line interception macro flavors.  If party A calls party B then party A is the caller and party B is the callee.  The original intent was that the caller macro would be run on party A's channel
when party B's connected line information changed.  The callee macro would be run on party B's channel when party A's connected line information changed.

In the parked call pickup scenario, who is the caller and who is the callee?
I think the caller should be defined as the parked call being picked up and the callee as the call picking up the parked call.

By: Philippe Lindheimer (p_lindheimer) 2011-02-17 13:09:58.000-0600

rmudgett,

concerning your note on caller vs. callee, I agree that conceptually that is usually the case, typical scenario being:

- outside call comes in (caller)
- call is answered
- call is parked
- original caller is picked up, thus by the callee as you describe.

However another common scenario is:

- I originate a call
- I say 'hold on got to send you to someone else"
- I park the call
- I announce to someone else to pickup the call (or walks somewhere else to take it elsewhere)
- Call is picked up, at this point, either the roles are reversed, or now both callers are the callees

So … with that said, it is not clear to me what the difference is wrt to to the internal functioning of connected call. For the purpose of call parking though, if a 'standard' needs to be defined then I would agree with your original comment and what ever implementation implications that has.

As far as the other notes, they go a bit beyond what would be efficient for me to try to implement at this time. As soon as there is a corrected patch with the preferred way to handle this I'll be happy to start testing with that one. I don't think I'm the right person to take it beyond this point (unless there is an example elsewhere that does exactly this in which case I can grab that and try it out).

By: Richard Mudgett (rmudgett) 2011-02-17 13:40:16.000-0600

Yes, I agree that the caller/callee designation is unfortunate since it only works well in the limited sense of A calls B.  I picked the parked call caller/callee designations that way because I thought that would be the most common scenario.  Call transfers have a similar problem with role reversals.

I think the best practice for connected line interception macros is to create only one macro that works in both directions.  You then set the caller/callee to the same macro.

By: black187 (black187) 2011-04-22 01:28:16

I've tried this patch, and it works. Could you please include it in the next release of Asterisk.

The CONNECTEDLINE feature was the final step of Asterisk to have all the features of ISDN PBX (regarding number presentations), so it would be great to have it enabled for all Asterisk features (parking lot...).

By: Richard Mudgett (rmudgett) 2011-08-26 13:27:30.888-0500

Part of the multiple parking lot issues fixed connected line to be updated when the parked call is retrieved.

r332117 | rmudgett | 2011-08-16 12:23:08 -0500 (Tue, 16 Aug 2011) | 147 lines

Merged revisions 332101 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/10

................
 r332101 | rmudgett | 2011-08-16 12:17:28 -0500 (Tue, 16 Aug 2011) | 140 lines

 Merged revisions 332100 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.8

 ........
   r332100 | rmudgett | 2011-08-16 11:31:36 -0500 (Tue, 16 Aug 2011) | 133 lines

   Fix multiple parking issues.