[Home]

Summary:ASTERISK-01946: App_Voicemail externnotify always returns 1
Reporter:umarsear (umarsear)Labels:
Date Opened:2004-07-03 16:16:14Date Closed:2008-01-15 15:01:49.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_voicmail.c.patch
Description:When using externnotify option in app_voicemail, it always return message count as one, even when there is no voice mail.

Tested against the latest CVS - 07-03-2004
Comments:By: nicolasg (nicolasg) 2004-07-03 18:05:07

Running version from 06/29 (same version from app_voicemail.c and app.c) and it works fine, it returns the correct number of mails. I can try on monday with latest CVS. Are you sure you are specifying the context for the mailbox?
By: umarsear (umarsear) 2004-07-03 18:25:15

I am using MySQL for SIP friends, and do specify the context in there. There is obviously no option there to specify a mailbox.

I am testing using a simple bash script below

#! /bin/bash
echo Context : $1 Mailbox $2 Messages $3  >>/home/usear/test.txt

The output I see in file test.txt is

Context : default Mailbox 2248599@default Messages 1
Context : default Mailbox 2248599@default Messages 1

I may very well be missing something. Guidence will be welcomed and I will do my part in trying to document this feature better once I have a clear understanding.
By: Rob Gagnon (rgagnon) 2004-07-06 02:49:20

looking at the code, the values fed to run_externnotify() will only ever send a 1 or a 0 by design to the script it calls.

The prototype looks like this:
static void run_externnotify(char *context, char *extension, int numvoicemails)

but the "numvoicemails" is a mis-nomer.  It should read something along the lines of "hasvoicemail".

It appears that this is simply a true/false flag indicating whether or not any voicemail are present.

However, someone of higher decision-making power may want to look and decide if the value sent to run_externnotify() should change from one int, to two, and then use the output from ast_app_messagecount() instead of the output from ast_app_hasvoicemail() for the input to run_externnotify.

With that done, you could send both the number of new messages, and the number of old messages as well to the externnotify script.

If someone wants this done, I could whip up a patch pretty quick as I've been in app_voicemail.c code for quite a while--- Just say the word.

edited on: 07-06-04 02:41
By: umarsear (umarsear) 2004-07-06 04:34:07

rgagnon, I see your point and to be honest I was expecting it to be a 1 or 0 (true/false). However in practice it appears to be sending a 1 (True !) even when the INBOX is empty.

I am happy/willing to do further tests to prove/disprove the case if required.
By: nicolasg (nicolasg) 2004-07-06 08:51:38

Umarsear: I could not test with latest CVS yet, and I'm not using Mysql SIP friends. Did you look into your /var/spool/asterisk/voicemail/default/2248599/INBOX directory to see what's in there when you have no voicemails? It should be empty. The mailbox you want to check is number 2248599?

rgagnon: You are right, the variable in run_externnotify should be named 'hasvoicemail' instead of 'numvoicemail'. IMO The app_hasvoicemail routine is there for a reason: its lighter to the disk drive. If you have hundreds of mailboxes and count the number of old/new messages every time, a considerable work is pushed to the drives. If you really want to count the number of messages, you can ask for them in your external script, or in my case, using the manager.
By: Rob Gagnon (rgagnon) 2004-07-06 11:28:21

umarsear, I just tested this with your simple script above (the echo statement)

when I left an email for myself, it reported 1.

I then logged in as myself, deleted all my messages, and hungup.  It reported 0 as expected.

Are you sure your mailbox on the disk is actually empty?

I should note that in my test, app_voicemail.c is v1.126 in CVS, and pbx.c is at v1.135 (both current as of this posting)

edited on: 07-06-04 11:16
By: umarsear (umarsear) 2004-07-06 13:58:21

Tested everything again and was getting the same result. Looked at the folder as suggested and sure enough the folder had some messages left behind.

As it happens app_voicemail looks for the msg0000 file where as externnotify feature uses any msgxxxx.txt file in the INBOX.

Although the issue I raised does not really exist and the INBOX was not in a clean state I think it might be better to have consistancy. What I mean is that they to features use the same method to decide if there is a message waiting or not.

I thank you guys for your efforts, and apologies for not being more thorough in the first place.

Umar.

Btw. The reason I want to use this feature is that I have integerated the asterisk voicemail feature with an external VPBX, I currently use an app (written in pascal) to traverse through the mailboxes pereodically and send a message waiting sip message to the VPBX. This will improve/simplify thing.
By: Matthew Fredrickson (mattf) 2004-07-06 16:01:46

Yeah, it would be very trivial to change this to behave like it is supposed to.  But would it be a feature that people would be interested in having?
By: Rob Gagnon (rgagnon) 2004-07-06 16:08:09

Probably it would be worthwhile for someone to change app_hasnewvoicemail.c to use one, or both of:  ast_app_messagecount(), and ast_app_has_voicemail() within the module for the applications "HasNewVoiceMail"  and "HasVoiceMail"

This would make the functionality more uniform.  My guess now is that the app_hasnewvoicemail.c file came before the two functions now within app.c
By: Matthew Fredrickson (mattf) 2004-07-06 16:45:57

I just uploaded a patch that fixes the behavior of the externnotify feature in app_voicemail.c to correctly report the number of (new) voicemails waiting on a phone.  If someone could test it, that would be great.

(Ugh, looks like I didn't spell voicemail correctly in the patch name) :-)
By: umarsear (umarsear) 2004-07-06 17:21:26

Quick test and it seems to be reporting the correct number of messages in the mail box. Just debating though if it was better it reporting 0 or 1 if there was a lower overhead.

Either way is fine by me though.

Thanks

Umar.
By: Rob Gagnon (rgagnon) 2004-07-06 17:27:11

Actually, the program could be modified more, as every call to "run_externnotify()" supplied "ext_context" as well as the return value from "ast_app_hasvoicemail(ext_context)"

There really is no need for the last parameter.  The run_externnotify() function could simply call ast_app_hasvoicemail on its own.

Also, you should watch out returning a number non-zero or non-one now, for backward compatibility.

I would suggest leaving the 0 / 1 flag in the position it is in, and simply adding more data to the end of the args list.

Thus, $3 is still 0/1 to a script. but $4 and $5 could be added new as "$4=#new voicemails" and "$5=#old voicemails"
By: umarsear (umarsear) 2004-07-07 15:48:53

My recomendation would be that this fix be available as a patch to anyone that needs it. In general I would say that a true/false option is fine.

My only other concern was that app_voicemail obviously looks for msg0000 and if it does not find it, it assumes that there are no new messages. Where as externnotify will return one if there is a file that matches the mask of ".txt". which effectively means that if a mailbox is in an unclean state than the two sources will return different results.

What I mean to say I suppose is that instead of searching for a file that matches a mask, search for msg0000.txt (which is what I believe app_voicemail does) to determine the result of externnotify.

secondly I suppose the name of the parameter could be changed from numvoicemails to something more accurate like, hasmessage or hasvoicemail.
By: Matthew Fredrickson (mattf) 2004-07-07 17:50:36

It was orginally written to send the number of messages that were available, not to send a boolean "available" or "unavailable".

Presumably, it would make more sense to correct it's behavior so that it reports the number of messages available.  Existing code would be easy to implement interpreting it as a boolean value (i.e. if(x) mail_is_there(); else no_mail()) but you would have the additional functionality of the knowledge of how many messages are available.
By: Mark Spencer (markster) 2004-07-08 03:41:06

Do you already have a disclaimer on file for this patch?
By: Mark Spencer (markster) 2004-07-08 03:42:06

nevermind, it's matt's patch :)
By: Mark Spencer (markster) 2004-07-08 03:45:53

Eh, patch didn't apply alnyway -- did some restructuring and stuff.
By: Digium Subversion (svnbot) 2008-01-15 15:01:49.000-0600

Repository: asterisk
Revision: 3389

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r3389 | markster | 2008-01-15 15:01:48 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge matt's voicemail patch with some restructuring (bug ASTERISK-1946)

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

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