[Home]

Summary:ASTERISK-09451: [patch] "fixing" the saying of queue holdtime (for less than 2 minutes)
Reporter:Caio Begotti (caio1982)Labels:
Date Opened:2007-05-15 21:38:36Date Closed:2008-02-14 18:23:20.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) queue_announce.diff
( 1) queue_announce5.diff
Description:I've found that when you're waiting in a queue, and has the announce options configured, Asterisk only reports the ammount of time after 2 minutes of waiting. That's kind of weird, I didn't understand why not to say when you're waiting for just "1 minute". It becomes a little weird when you set up the round seconds options in queues.conf and gets something like "less than 2 minutes, 37 seconds".

I wrote this very simple patch that makes app_queue say all the minutes and seconds possible, including the singular form for 1 minute or second. I'm not a programmer at all, I just used some copy & paste and I've been testing it in my lab in the last two days without problem. Could someone please review it and tell me if it's plain wrong, nonsense or something?

I just don't know the procedure triggered when a patch "creates" a new sound file. The patch did its job, fixed a problem of mine :-P

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

Anyone can track the end of thread that started it all here:
http://www.mail-archive.com/asterisk-dev@lists.digium.com/msg26958.html

It is, by the way, somewhat related to issue 9514 that has been already solved.
Comments:By: Jason Parker (jparker) 2007-05-17 11:36:36

There is also another reference to sound_lessthan on line 2559 in svn trunk.

If we remove that one also (it's also used for hold time), we could actually just remove that whole variable.
By: Joshua C. Colp (jcolp) 2007-05-17 11:56:34

We'll also need to get the sounds recorded for this if we decide to put it in.
By: Tilghman Lesher (tilghman) 2007-08-27 17:29:42

We can obliviate the need for "minute" and "second", if you consider that anything under two minutes can be stated in seconds, like "90 seconds".  1 "second" is not needed, either, because by the time you say "we estimate that your call will be answered in", it's longer than 1 second.  So you'd never hear such a prompt to completion (and even if you did, it would sound VERY weird).

Even telling someone their call is likely to be answered in ten seconds sounds weird, and we probably should not get any more exact than to the nearest 30 second mark.
By: Caio Begotti (caio1982) 2007-08-27 21:01:34

About obliviating the need for minute and second (singular) I totally agree, yeah. But I think it wouldn't hurt to let people get less than 30 seconds estimatives, though. I don't have enough spare time to update the diff until the weekend, so feel free to do it on your own, you guys are much more faster and experienced coders.


By: Caio Begotti (caio1982) 2007-12-21 20:57:03.000-0600

Keeping my own house cleaned, I uploaded a new patch that finish my initial idea and even solves a bug introduced after this bug was opened:

- Completely removes the "lessthan 2 minutes" thing, as it's much worse than changing it.

For example: VERBOSE[28314] logger.c:     -- Hold time for 555 is 1 minutes 26 seconds
expands to "less than two minutes twenty six seconds" *that* is very weird. It simply should be "one minute twenty six seconds".

- Add a queue-minute audio file to substitute the above lessthan, so it will indeed say correct hold times less than 2 minutes. One audio file out, one audio file in, no big deal I think.

- Removes the option to round hold times == 1 as it introduces a bug where you could possibly hear something like "3 minutes 1 seconds", plural, but Asterisk does not provide this sound nor will (as far as I understood). This was introduced on revision 64263 by Qwell, after this bug was opened.

This patch I just uploaded solves that. Corydon already said that is stupid to say "1 second" as it's too specific and I wholly agree :-)

For the sake of documentation, I tested it and it really happens on trunk:

  -- <SIP/111-01843000> Playing 'queue-thereare.gsm' (language 'en')
  -- <SIP/111-01843000> Playing 'digits/4.gsm' (language 'en')
  -- <SIP/111-01843000> Playing 'queue-callswaiting.gsm' (language 'en')
  -- Hold time for 555 is 2 minutes 1 seconds
  -- <SIP/111-01843000> Playing 'queue-holdtime.gsm' (language 'en')
  -- <SIP/111-01843000> Playing 'digits/2.gsm' (language 'en')
  -- <SIP/111-01843000> Playing 'queue-minutes.gsm' (language 'en')
  -- <SIP/111-01843000> Playing 'digits/1.gsm' (language 'en')
  -- <SIP/111-01843000> Playing 'queue-seconds.gsm' (language 'en')
  -- Told SIP/111-01843000 in 555 their queue position (which was 4)
  -- <SIP/111-01843000> Playing 'queue-thankyou.gsm' (language 'en')

Any objections?

PS: please, can someone update the bug's description to trunk, revision 94593?


By: Caio Begotti (caio1982) 2007-12-21 21:07:31.000-0600

By the way, I'm not sure what to describe in the CHANGES file so a bug marshall might prefer to do that, in case this patch is fine as is, of course.
By: Caio Begotti (caio1982) 2008-01-04 10:32:31.000-0600

Ping?
By: Caio Begotti (caio1982) 2008-01-18 06:16:49.000-0600

Gentlemen, anyone? Ping? The current patch started applying with offsets and I'm afraid it will be rejected soon so I'll have to update it anyway without previous review.

Maybe this issue could make it into 1.6 before the first release?
By: Mark Michelson (mmichelson) 2008-02-13 14:28:26.000-0600

This has sat around for too long, and so I have assigned it to myself and will get this committed as soon as possible.
By: Digium Subversion (svnbot) 2008-02-14 14:42:45.000-0600

Repository: asterisk
Revision: 103687

U   trunk/UPGRADE.txt
U   trunk/apps/app_queue.c
U   trunk/configs/queues.conf.sample

------------------------------------------------------------------------
r103687 | mmichelson | 2008-02-14 14:42:44 -0600 (Thu, 14 Feb 2008) | 10 lines

Change the queue holdtime announcement to happen at any interval (not just greater than two minutes). Remove
the saying of less-than for holdtime announcements since it can lead to awkward holdtime announcements. Using
'1' as a queue-round-seconds value is no longer valid.

(closes issue ASTERISK-9451)
Reported by: caio1982
Patches:
     queue_announce5.diff uploaded by caio1982 (license 22)
 Tested by: caio1982, putnopvut

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=103687
By: Digium Subversion (svnbot) 2008-02-14 18:23:20.000-0600

Repository: asterisk
Revision: 103708

_U  team/murf/bug11210/
U   team/murf/bug11210/UPGRADE.txt
U   team/murf/bug11210/apps/app_externalivr.c
U   team/murf/bug11210/apps/app_queue.c
U   team/murf/bug11210/apps/app_voicemail.c
U   team/murf/bug11210/channels/chan_iax2.c
U   team/murf/bug11210/configs/queues.conf.sample
U   team/murf/bug11210/configure
U   team/murf/bug11210/configure.ac
U   team/murf/bug11210/doc/tex/imapstorage.tex
U   team/murf/bug11210/funcs/func_cdr.c
U   team/murf/bug11210/include/asterisk/autoconfig.h.in
U   team/murf/bug11210/res/res_agi.c
U   team/murf/bug11210/res/res_musiconhold.c

------------------------------------------------------------------------
r103708 | murf | 2008-02-14 18:23:17 -0600 (Thu, 14 Feb 2008) | 187 lines

Merged revisions 103658,103662,103668,103677,103682,103685,103687,103689,103691,103694,103699-103700,103702,103704-103705 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r103658 | mmichelson | 2008-02-13 08:47:25 -0700 (Wed, 13 Feb 2008) | 10 lines

1. Deprecate SetMusicOnHold and WaitMusicOnHold.
2. Add a duration parameter to MusicOnHold

(closes issue ASTERISK-11358)
Reported by: dimas
Patches:
     v2-moh.patch uploaded by dimas (license 88)
 Tested by: dimas


................
r103662 | jpeeler | 2008-02-13 14:04:31 -0700 (Wed, 13 Feb 2008) | 6 lines

(closes issue ASTERISK-11286)
Reported by: ctooley
Patches:
     additional_eivr_commands.patch uploaded by ctooley (license 136)
Tested by: ctooley

................
r103668 | oej | 2008-02-14 03:19:09 -0700 (Thu, 14 Feb 2008) | 2 lines

Formatting fixes

................
r103677 | qwell | 2008-02-14 11:39:51 -0700 (Thu, 14 Feb 2008) | 11 lines

Add periodic jitter stats to CLI and manager.

(closes issue ASTERISK-7968)
Reported by: stevedavies
Patches:
     jblogging-trunk.patch uploaded by stevedavies
     jblogging-trunk_wmgrevent.patch uploaded by johann8384
     updated_jbloggin-trunk_mgrevent.patch uploaded by johann8384 (license 190)
     (with additional changes by me)
Tested by: stevedavies, johann8384

................
r103682 | jpeeler | 2008-02-14 12:47:39 -0700 (Thu, 14 Feb 2008) | 1 line

a few syntax changes and safer code
................
r103685 | qwell | 2008-02-14 12:52:21 -0700 (Thu, 14 Feb 2008) | 13 lines

Merged revisions 103683 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r103683 | qwell | 2008-02-14 13:51:10 -0600 (Thu, 14 Feb 2008) | 5 lines

Document the 'l' option to the CDR() function.
(Thanks voipgate for pointing out the option, and Leif for providing text for it.)

Closes issue ASTERISK-11166.

........

................
r103687 | mmichelson | 2008-02-14 13:46:00 -0700 (Thu, 14 Feb 2008) | 10 lines

Change the queue holdtime announcement to happen at any interval (not just greater than two minutes). Remove
the saying of less-than for holdtime announcements since it can lead to awkward holdtime announcements. Using
'1' as a queue-round-seconds value is no longer valid.

(closes issue ASTERISK-9451)
Reported by: caio1982
Patches:
     queue_announce5.diff uploaded by caio1982 (license 22)
 Tested by: caio1982, putnopvut

................
r103689 | mmichelson | 2008-02-14 13:58:30 -0700 (Thu, 14 Feb 2008) | 17 lines

Merged revisions 103688 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r103688 | mmichelson | 2008-02-14 14:55:48 -0600 (Thu, 14 Feb 2008) | 9 lines

Fix the new message count if delete=yes when using IMAP storage.

(closes issue ASTERISK-10917)
Reported by: jaroth
Patches:
     deleteflag_v2.patch uploaded by jaroth (license 50)
 Tested by: jaroth


........

................
r103691 | mmichelson | 2008-02-14 14:04:37 -0700 (Thu, 14 Feb 2008) | 11 lines

Merged revisions 103690 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r103690 | mmichelson | 2008-02-14 15:03:02 -0600 (Thu, 14 Feb 2008) | 3 lines

Fix build for non-IMAP builds


........

................
r103694 | qwell | 2008-02-14 14:21:53 -0700 (Thu, 14 Feb 2008) | 8 lines

Modify ldap autoconf function, so that an incorrect ldap library is not found on Solaris (it is incompatible).
Also removes second check for awk, which causes Solaris to find an incompatible version of awk.

(closes issue ASTERISK-11290)
Reported by: snuffy
Patches:
     bug-11829.diff uploaded by snuffy (license 35)

................
r103699 | mmichelson | 2008-02-14 16:35:21 -0700 (Thu, 14 Feb 2008) | 20 lines

Blocked revisions 103698 via svnmerge

........
r103698 | mmichelson | 2008-02-14 17:30:17 -0600 (Thu, 14 Feb 2008) | 13 lines

Change to the configure logic regarding IMAP. Prior to this commit, if you wished to configure
Asterisk with IMAP support, you would use the --with-imap configure switch in one of the following
two ways:
--with-imap=/some/directory would look in the directory specified for a UW IMAP source installation
--with-imap would assume that you had imap-2004g installed in .. relative to the Asterisk source

With this set of changes the two above options still work the same, but there are two new behaviors, too.
--with-imap=system will assume that you have -libc-client.so where you store your shared objects and will
           attempt to find c-client headers in your include path either in the imap or c-client directory.
If either of the two original methods of specifying the imap option should fail, then the check for --with-imap
=system will be performed in addition. It is only after this "system" check that failure can happen.


........

................
r103700 | mmichelson | 2008-02-14 16:39:47 -0700 (Thu, 14 Feb 2008) | 5 lines

See commit message for svn revision 103698. This behavior is the same as what is described
there. The difference is that trunk already had the --with-imap=system option, but it only
checked the include path for headers in the imap directory and not also the c-client directory.


................
r103702 | mmichelson | 2008-02-14 16:48:12 -0700 (Thu, 14 Feb 2008) | 10 lines

Blocked revisions 103701 via svnmerge

........
r103701 | mmichelson | 2008-02-14 17:44:17 -0600 (Thu, 14 Feb 2008) | 3 lines

Update documentation regarding configuration of IMAP


........

................
r103704 | mmichelson | 2008-02-14 16:49:54 -0700 (Thu, 14 Feb 2008) | 10 lines

Blocked revisions 103703 via svnmerge

........
r103703 | mmichelson | 2008-02-14 17:49:24 -0600 (Thu, 14 Feb 2008) | 3 lines

Make a small clarification in the documentation


........

................
r103705 | mmichelson | 2008-02-14 16:51:49 -0700 (Thu, 14 Feb 2008) | 3 lines

Trunk version of 1.4's imap documentation updates


................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=103708