[Home]

Summary:ASTERISK-05130: [patch] [post 1.2] Addition of multiple configurable periodic announcements
Reporter:j (j)Labels:
Date Opened:2005-09-22 16:52:14Date Closed:2006-01-17 14:00:08.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_queue
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_queue.c.patch_2005-12-02-correct
( 1) app_queue.c-patch-12-21-05
( 2) app_queues_bt.txt
Description:My company was in need of the ability to use multiple (different) periodic announcements while in our queue.

 I modified app_queue.c to be able to do so.

 You can put up to 5 (default) periodic announcements into the queue.conf file, and they will play in succession just like it did before, only now looping through all of the defined announcements.
 If more than 5 are defined, it simply drops the remaining ones in the conf.
 The actual number allowed I made a define called MAX_PERIODIC_ANNOUCEMENTS at the top of the file, and set it to 5. This could obviously be tweaked.


  In the conf you can now do this:
[queue]
periodic-announce-frequency=25
periodic-announce=soundFile1
periodic-announce=soundFile2
periodic-announce=soundFile3

 and it will play those three in succession with the frequency timeout in between them.

 I hope I followed the guidelines of submitting a patch correctly. If not please let me know exactly what I did wrong so that I won't do it again.

 Obviously I give full permissions to do what ever you want with this patch.

 Thanks very much!

j
Comments:By: twisted (twisted) 2005-09-22 17:45:49

any patches filed in the system must have a disclaimer on file to be included in CVS.
By: j (j) 2005-09-22 19:01:51

Ok couple things:

 I noticed, much to my chagrin, that the format was indeed incorrect. I fixed the formatting to use proper indentation and actual \t tab characters (darn you nedit! hehe)
 I also applied the patch to tonight's cvs (09-22-05), so it should patch very cleanly. Took out some stuff I forgot to take out, but mostly importantly fixed a bug in the original code:

 The say_periodic_announcement function was returning -1 if the time had not elapsed yet for a caller. It seemed to work fine for the first caller, but subsequent callers were kicked immediately out of the queue because it thought the caller had hung up. I changed the return value to 0, which seems to be correct and has been tested on the very latest cvs with at least two callers and once agent.

 So, please disreguard the first patch file (delete it if you can please), as it is incorrect.

 Oh yeah.
 The disclaimer is signed, and will be faxed to you very shortly. You will have it by morning. Sorry about that <blush>.

 Also; thanks for the quick response.

 Cheers!

 j


By: Russell Bryant (russell) 2005-09-22 23:06:28

Please post another note here to verify that your disclaimer has been faxed.  Thanks!
By: j (j) 2005-09-23 08:50:04

The disclaimer has been faxed and confirmed to have arrived.

 Thanks :)

j
By: j (j) 2005-09-26 16:37:52

Sorry for all the notes guys, hope I'm not getting on anyone's nerves.

 I've updated the patch for todays CVS.
 The reason I did that was that I re-worked the code so that instead of supplying the same directive in the configuration file multiple times, you can supply multiple sound files on one line, separated by a | (pipe character). The reason for this was that the old way was incompatible with RealTime, and this is compatible.

Before:
periodic-announce=SoundFile1
periodic-announce=SoundFile2
periodic-announce=SoundFile3

Now:
periodic-announce=SoundFile1|SoundFile2|Soundfile3

You might want to double check my code, just to make sure there isn't a more efficient way to do it.

last update, I swear :)

Thanks

j
By: Michael Jerris (mikej) 2005-12-01 13:53:05.000-0600

Can we please get an updated patch for this for current svn trunk.  Thanks.
By: j (j) 2005-12-02 08:28:11.000-0600

Sure thing :)
By: j (j) 2005-12-02 08:33:41.000-0600

I uploaded the wrong file. The one with "-correct" is the correct patch file.

 Please forgive, it's early :(
By: Jason Parker (jparker) 2005-12-20 01:38:59.000-0600

Which of the uploaded files can be deleted?  My guess is everything except app_queue.c.patch_2005-12-02-correct - does this sound right?

I'll try to test this one tomorrow.
By: j (j) 2005-12-20 07:09:56.000-0600

app_queue.c.patch_2005-12-02-correct is the latest correct patch file.

Everything else can go.
 Sorry for the superfluous file uploads.

j
By: Jason Parker (jparker) 2005-12-20 17:25:14.000-0600

Patch causes crash with realtime queues.

After adding debug statements, it appears to happen on this line:

ast_copy_string(q->sound_periodicannounce[i], buff, sizeof(q->sound_periodicannounce[i]));

Just tested, also happens if there is no "|", so, this line too:

ast_copy_string(q->sound_periodicannounce[i], val, sizeof(q->sound_periodicannounce[i]));

Backtrace attached.


By: Jason Parker (jparker) 2005-12-20 17:42:33.000-0600

It appears to be because i is uninitialized.  It worked for me when I changed it to:
int i = 0;

Besides that, it works alright.
By: j (j) 2005-12-21 07:17:32.000-0600

Huh.
I've been using the code in that patch in a production environment with an average consistent call load of about 30 calls with spikes up to 60-80 calls for more than a month now. All calls end up in a queue using realtime.

 I went back to check the code in use and sure enough it had i initialized to 0. I'm not sure why it didn't make it in the patch. Sorry.

 I double checked the patch to the code I'm using and that seems to be the only inconsistency.

 Is there still a problem if there's no pipe character in the config, or was that related to the uninitialized int counter?

j
By: Jason Parker (jparker) 2005-12-21 09:41:06.000-0600

Everything seemed to work fine after I set i = 0.

Can you upload a new patch?
By: j (j) 2005-12-21 11:29:25.000-0600

Yep. No problem.

I made the patch from today's svn. It compiled cleanly.

j
By: Matt O'Gorman (mogorman) 2006-01-17 13:59:02.000-0600

commited into trunk 8152