Summary:	ASTERISK-03189: [patch] Add the ability to pause (temporarily suspend) queue members
Reporter:	mjohnston (mjohnston)	Labels:
Date Opened:	2005-01-05 14:14:21.000-0600	Date Closed:	2008-01-15 15:22:44.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_queue
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) queue_pause_rev2.patch ( 1) queue_pause_rev3.patch ( 2) queue_pause.patch
Description:	This is a patch to app_queue.c that adds a "paused" flag to queue members.  Members with this flag set will not receive any calls, but will stay in the queue, so their round-robin status and other stats are preserved.  The patch also adds two ways to set the flag: a dialplan application called PauseQueueMember([queuename]|interface) that pauses the user when started, unpausing when hung up, and a simple QueuePause manager command. Disclaimer is on file. ****** ADDITIONAL INFORMATION ****** The paused flag is meant to implement the agent-requested "not-ready" feature available with other ACDs.  This is basically an extension of wrap-up time that lasts until the agent sets their phone to be "ready" again. The dialplan application is intended for use with Cisco phones; one of the speed-dial keys can be configured to call an extension that runs PauseQueueMember, thus acting like a hardware not-ready button.  I considered playing MOH in PauseQueueMember, but with the Cisco phones they will likely be running the app on speakerphone, so silence is more desirable. Persistent storage of the flag in and output in the QueueMemberStatus mgr event and a new QueueMemberPaused one are supported, as well as "show queues" and anywhere else it seemed appropriate.  The locking code in set_member_paused could use an experienced eye; I mostly cribbed it from the member removal code.  I'm especially concerned with whether it's okay to call manager_event with locks held.
Comments:	By: Kevin P. Fleming (kpfleming) 2005-01-05 15:36:14.000-0600 Some thoughts: - in set_member_paused, don't call dump_queue_members every time you pause a member, wait until you have processed all the queues (this is safe since you have the locks held) - in the code that restores queue members from the db, you can't error out if there is no "paused" data... you need to support restoring in the "old" format, or when people upgrade they will lose their persisted members - I'm personally not too keen on PauseQueueMember holding the channel open; I'd rather see PauseQueueMember/UnpauseQueueMember, just like you have implemented for the manager interface... you would then be able to simulate the current functionality in the dialplan by calling QueuePauseMember and waiting indefinitely for the user to do something, calling QueueUnpauseMember when they hang up. This way, if you want to supply MOH or some other audio while they are paused, you can do so... if not, the member can just dial another extension to unpause. - all of this activity will need to be logged into the queue log; without that, people monitoring their agents will not be able to track how much time (and how often) they "paused". Since you still deliver calls to the queue even if all the members are paused, this could be a serious issue for some call centers. - the locking in set_member_paused looks fine to me; yes, calling manager_event is fine, since it does not reference any locks owned by app_queue and there are no "global" locks in Asterisk If this patch has a chance of going in, I'd like to see it happen soon; I have a complete rebuild of app_queue coming that uses the new ASTOBJ infrastructure to drastically simplify the locking and object lifetime rules. By: mjohnston (mjohnston) 2005-01-06 11:47:23.000-0600 kpfleming: "in set_member_paused, don't call dump_queue_members every time you pause a member, wait until you have processed all the queues" I don't think that's a win.  Currently, each queue is dumped either 0 times (if the requested interface isn't in it or it's not requested) or 1 time (if the interface is in it.)  It's never dumped more than once per queue, since only one member can be passed in to set_member_paused.  If I move dump_queue_members out, I'll have to loop through the queues again, since dump_queue_members runs on one queue at a time, and either unconditionally dump every queue, or do the membership test again. Thanks for the tips - I've modified and tested the patch to load old or new format data and log to the queue log.  I also switched the applications over from PauseQueueMember only to PauseQueueMember/UnpauseQueueMember, since, as you suggest, that way is more flexible while still allowing the original pause-while-the-line-is-open functionality.  Updated patch attached. By: Kevin P. Fleming (kpfleming) 2005-01-06 14:11:11.000-0600 You are right, I was thinking dump_queue_members dumped all the queues, but it doesn't. I'll look over your new patch when I get a few minutes; thanks for following up! By: Kevin P. Fleming (kpfleming) 2005-01-06 18:29:18.000-0600 Looks good to me; I like the feature, and the implementation appears to be sound. Now we just wait to see if Mark agrees :-) By: mjohnston (mjohnston) 2005-01-07 09:43:53.000-0600 One more note: I had been using " " to avoid an empty-format-string warning, but I see the right way to do it is to use "%s", "".  This should probably be changed - it occurs only in two places in set_member_paused.  I can make a new patch if desired. By: Kevin P. Fleming (kpfleming) 2005-01-07 10:07:56.000-0600 Sure, go ahead and send the revised patch. It's always best if the patch sitting in the bug is ready to apply to CVS with no changes (and since there were other changes to app_queue merged last night, your patch may not apply cleanly any longer). By: mjohnston (mjohnston) 2005-01-10 12:33:08.000-0600 Rev 3 of the patch, updated to app_queue.c 1.114 (no changes were needed.)  This patch also fixes the empty format strings mentioned in the bugnotes. By: jmls (jmls) 2005-01-10 15:04:54.000-0600 is there any way of being able to display the member status (ready / not ready) on a cisco 7940 / 7960 phone ? By: mjohnston (mjohnston) 2005-01-11 09:02:40.000-0600 jmls: I'd love to know that myself. :)  I have a couple of possibilities in mind here.  The first would be to name one of the line keys "Not Ready", and have it insta-dial a given extension (this is where I'm stuck; it seems speed dial keys pick up your other line to dial.)  You could hit the button to go to Not Ready, which would show the regular 'in-call' status screen with Not Ready highlighted by the line key, and hit the same key to drop the call and go back to Ready. If that doesn't work out, I'm considering using the Services key, with its connection to a Web server kicking off a manager PauseAgent command.  The trouble is knowing when Services has been dismissed.  I'm considering using a custom Web server and just holding the connection open, so the phone can drop the connection when the user closes Services and (by doing so) returns to Ready status. Apart from that, I can't think of anything; it's a shame that the SIP images are so crippled on these sets.  If some of the restrictions are relaxed in the future, it'll allow more flexibility to display status. By: Mark Spencer (markster) 2005-01-20 22:11:44.000-0600 Added to CVS with small changes, thanks!  Feel free to reopen if i introduced any problems with my minor changes. By: Digium Subversion (svnbot) 2008-01-15 15:22:44.000-0600 Repository: asterisk Revision: 4861 U   trunk/apps/app_queue.c ------------------------------------------------------------------------ r4861 | markster | 2008-01-15 15:22:43 -0600 (Tue, 15 Jan 2008) | 2 lines Merge mjohnston's pause/unpause (bug ASTERISK-3189) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=4861