Summary:	ASTERISK-16235: [patch] Bug in reporting queue hold time
Reporter:	Andrey Solovyev (corruptor)	Labels:
Date Opened:	2010-06-11 03:52:54	Date Closed:	2010-07-16 16:16:51
Priority:	Trivial	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_queue
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) holdesecs_bug.diff ( 1) queues.conf
Description:	I've noticed that asterisk incorrectly reports estimate hold time to callers if the hold time between more than 1 minute and less then 2 minutes. Asterisk correctly calculates avg minutes and avg seconds but play incorrect time. Example: -- Hold time for 6 is 1 minute(s) 30 seconds    -- <SIP/1110-000000a8> Playing 'queue-holdtime.slin' (language 'ru')    -- Nobody picked up in 10000 ms    -- Stopped music on hold on SIP/1108-000000a6    -- <SIP/1108-000000a6> Playing 'queue-youarenext.slin' (language 'ru')    -- <SIP/1110-000000a8> Playing 'digits/1.slin' (language 'ru')    -- <SIP/1110-000000a8> Playing 'queue-minute.slin' (language 'ru')    -- <SIP/1110-000000a8> Playing 'digits/90.slin' (language 'ru')    -- <SIP/1110-000000a8> Playing 'queue-seconds.slin' (language 'ru') We hear 1 minute 90 seconds which is wrong ****** ADDITIONAL INFORMATION ****** I've made simple patch to resolve this issue. It seems that earlier the hold time less than 2 minutes was reported in seconds but i think "1 minute 30 seconds" is better.
Comments:	By: Leif Madsen (lmadsen) 2010-06-14 10:26:25 Can you also provide the configuration of your queues.conf? I'd like to verify this is a bug and not a configuration issue. By: Andrey Solovyev (corruptor) 2010-06-15 03:15:22 I've uploaded queues.conf I've noticed that announce-holdtime and announce-frequency options are listed twice in my conf but I anyway the last value should be set and that shouldn't affect the bug. If we look through the code we see that minutes are reported always if 1 or more (avgholdmins >= 1 ) and then if avgholdmins = 1 the value avgholdmins * 60 + avgholdsecs is reported. So if asterisk calculates holdtime 1 min and 30 sec it reports "1 minute" and then it reports 1 * 60 + 30 = 90 seconds which is wrong. By: David Woolley (davidw) 2010-06-15 07:07:26 The order of precedence of multiple entries in configuration files is not well defined.  Some parts of the code scan the file and act on each variable, in which case you get the last setting, but some parts of the code go through a list of variables and search the file for each one in turn, in which case you get the first occurrence.  This is considered a feature; you should not rely on one behaviour or the other. By: Leif Madsen (lmadsen) 2010-06-15 11:32:40 Great, thanks for the information. From what you've described it appears as though you're correct. By: Leif Madsen (lmadsen) 2010-06-15 11:45:20 Also agreed with davidw; if you're relying on some matching order when you define something multiple times, you're likely to get failures. By: Digium Subversion (svnbot) 2010-07-16 16:16:07 Repository: asterisk Revision: 277488 U   trunk/apps/app_queue.c ------------------------------------------------------------------------ r277488 | jpeeler | 2010-07-16 16:16:07 -0500 (Fri, 16 Jul 2010) | 10 lines Fix reporting estimated queue hold time. Just say the number of seconds (after minutes) rather than doing some incorrect calculation with respect to minutes. (closes issue ASTERISK-16235) Reported by: corruptor Patches:      holdesecs_bug.diff uploaded by corruptor (license 253) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=277488 By: Digium Subversion (svnbot) 2010-07-16 16:16:50 Repository: asterisk Revision: 277489 _U  branches/1.6.2/ U   branches/1.6.2/apps/app_queue.c ------------------------------------------------------------------------ r277489 | jpeeler | 2010-07-16 16:16:50 -0500 (Fri, 16 Jul 2010) | 17 lines Merged revisions 277488 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........  r277488 | jpeeler | 2010-07-16 16:16:08 -0500 (Fri, 16 Jul 2010) | 10 lines    Fix reporting estimated queue hold time.    Just say the number of seconds (after minutes) rather than doing some incorrect  calculation with respect to minutes.    (closes issue ASTERISK-16235)  Reported by: corruptor  Patches:        holdesecs_bug.diff uploaded by corruptor (license 253) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=277489